Certificate Transparency: Wire the TreeStateTracker up.
This change puts into use the TreeStateTracker class by wiring it to:
* A global STHDistributor for receiving fresh STHs via the component
updater.
* The per-profile CTVerifier for receiving notification of verified
SCTs that should be checked for inclusion.
As discussed with sorin and rsleevi, the wiring of the STHSetComponentInstaller
is via a globally-accessible STHDistributor, created by the IOThread.
BUG=613501
Review-Url: https://codereview.chromium.org/2014573002
Cr-Commit-Position: refs/heads/master@{#397096}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index dfc340a..f079086b 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -117,6 +117,7 @@
"//components/browser_sync/common",
"//components/captive_portal",
"//components/certificate_reporting",
+ "//components/certificate_transparency",
"//components/cloud_devices/common",
"//components/component_updater",
"//components/content_settings/content/common",
diff --git a/chrome/browser/component_updater/sth_set_component_installer.cc b/chrome/browser/component_updater/sth_set_component_installer.cc
index 401a79f..c4f2ba5 100644
--- a/chrome/browser/component_updater/sth_set_component_installer.cc
+++ b/chrome/browser/component_updater/sth_set_component_installer.cc
@@ -15,6 +15,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/values.h"
#include "base/version.h"
+#include "chrome/browser/net/sth_distributor_provider.h"
#include "components/component_updater/component_updater_paths.h"
#include "components/safe_json/safe_json_parser.h"
#include "content/public/browser/browser_thread.h"
@@ -49,8 +50,8 @@
const char kSTHSetFetcherManifestName[] = "Signed Tree Heads";
STHSetComponentInstallerTraits::STHSetComponentInstallerTraits(
- std::unique_ptr<net::ct::STHObserver> sth_observer)
- : sth_observer_(std::move(sth_observer)) {}
+ net::ct::STHObserver* sth_observer)
+ : sth_observer_(sth_observer) {}
STHSetComponentInstallerTraits::~STHSetComponentInstallerTraits() {}
@@ -168,7 +169,7 @@
content::BrowserThread::PostTask(
content::BrowserThread::IO, FROM_HERE,
base::Bind(&net::ct::STHObserver::NewSTHObserved,
- base::Unretained(sth_observer_.get()), signed_tree_head));
+ base::Unretained(sth_observer_), signed_tree_head));
}
void STHSetComponentInstallerTraits::OnJsonParseError(
@@ -182,14 +183,13 @@
const base::FilePath& user_data_dir) {
DVLOG(1) << "Registering STH Set fetcher component.";
- // TODO(eranm): The next step in auditing CT logs (crbug.com/506227) is to
- // pass the distributor to the IOThread so it can be used in a per-profile
- // context for checking inclusion of SCTs.
- std::unique_ptr<net::ct::STHDistributor> distributor(
- new net::ct::STHDistributor());
+ net::ct::STHDistributor* distributor =
+ chrome_browser_net::GetGlobalSTHDistributor();
+ // The global STHDistributor should have been created by this point.
+ DCHECK(distributor);
std::unique_ptr<ComponentInstallerTraits> traits(
- new STHSetComponentInstallerTraits(std::move(distributor)));
+ new STHSetComponentInstallerTraits(distributor));
// |cus| will take ownership of |installer| during installer->Register(cus).
DefaultComponentInstaller* installer =
new DefaultComponentInstaller(std::move(traits));
diff --git a/chrome/browser/component_updater/sth_set_component_installer.h b/chrome/browser/component_updater/sth_set_component_installer.h
index 4223aa37..1279706 100644
--- a/chrome/browser/component_updater/sth_set_component_installer.h
+++ b/chrome/browser/component_updater/sth_set_component_installer.h
@@ -40,8 +40,7 @@
class STHSetComponentInstallerTraits : public ComponentInstallerTraits {
public:
// The |sth_distributor| will be notified each time a new STH is observed.
- explicit STHSetComponentInstallerTraits(
- std::unique_ptr<net::ct::STHObserver> sth_observer);
+ explicit STHSetComponentInstallerTraits(net::ct::STHObserver* sth_observer);
~STHSetComponentInstallerTraits() override;
private:
@@ -73,7 +72,10 @@
// STH parsing failed - do nothing.
void OnJsonParseError(const std::string& log_id, const std::string& error);
- std::unique_ptr<net::ct::STHObserver> sth_observer_;
+ // The observer is not owned by this class, so the code creating an instance
+ // of this class is expected to ensure the STHObserver lives as long as
+ // this class does. Typically the observer provided will be a global.
+ net::ct::STHObserver* sth_observer_;
DISALLOW_COPY_AND_ASSIGN(STHSetComponentInstallerTraits);
};
diff --git a/chrome/browser/component_updater/sth_set_component_installer_unittest.cc b/chrome/browser/component_updater/sth_set_component_installer_unittest.cc
index 5b6b101..d965fc0 100644
--- a/chrome/browser/component_updater/sth_set_component_installer_unittest.cc
+++ b/chrome/browser/component_updater/sth_set_component_installer_unittest.cc
@@ -44,9 +44,8 @@
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
- std::unique_ptr<StoringSTHObserver> observer(new StoringSTHObserver());
- observer_ = observer.get();
- traits_.reset(new STHSetComponentInstallerTraits(std::move(observer)));
+ observer_.reset(new StoringSTHObserver());
+ traits_.reset(new STHSetComponentInstallerTraits(observer_.get()));
}
void WriteSTHToFile(const std::string& sth_json,
@@ -82,8 +81,10 @@
content::TestBrowserThreadBundle thread_bundle_;
base::ScopedTempDir temp_dir_;
+ std::unique_ptr<StoringSTHObserver> observer_;
+ // |traits_| should be destroyed before the |observer_| as it holds a pointer
+ // to it.
std::unique_ptr<STHSetComponentInstallerTraits> traits_;
- StoringSTHObserver* observer_;
safe_json::TestingJsonParser::ScopedFactoryOverride factory_override_;
private:
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 5210f4b8..063d370 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -39,10 +39,12 @@
#include "chrome/browser/net/chrome_network_delegate.h"
#include "chrome/browser/net/dns_probe_service.h"
#include "chrome/browser/net/proxy_service_factory.h"
+#include "chrome/browser/net/sth_distributor_provider.h"
#include "chrome/common/channel_info.h"
#include "chrome/common/chrome_content_client.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
+#include "components/certificate_transparency/tree_state_tracker.h"
#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_prefs.h"
#include "components/data_usage/core/data_use_aggregator.h"
#include "components/data_usage/core/data_use_amortizer.h"
@@ -72,6 +74,8 @@
#include "net/cert/ct_verifier.h"
#include "net/cert/multi_log_ct_verifier.h"
#include "net/cert/multi_threaded_cert_verifier.h"
+#include "net/cert/sth_distributor.h"
+#include "net/cert/sth_observer.h"
#include "net/cookies/cookie_store.h"
#include "net/dns/host_cache.h"
#include "net/dns/host_resolver.h"
@@ -393,6 +397,9 @@
g_browser_process->metrics_service()->GetDataUseForwardingCallback();
}
+ chrome_browser_net::SetGlobalSTHDistributor(
+ std::unique_ptr<net::ct::STHDistributor>(new net::ct::STHDistributor()));
+
BrowserThread::SetDelegate(BrowserThread::IO, this);
}
@@ -403,6 +410,10 @@
pref_proxy_config_tracker_->DetachFromPrefService();
DCHECK(!globals_);
+
+ // Destroy the old distributor to check that the observers list it holds is
+ // empty.
+ chrome_browser_net::SetGlobalSTHDistributor(nullptr);
}
IOThread::Globals* IOThread::globals() {
@@ -607,6 +618,13 @@
// Add built-in logs
ct_verifier->AddLogs(globals_->ct_logs);
+ ct_tree_tracker_.reset(
+ new certificate_transparency::TreeStateTracker(globals_->ct_logs));
+ // Register the ct_tree_tracker_ as observer for new STHs.
+ RegisterSTHObserver(ct_tree_tracker_.get());
+ // Register the ct_tree_tracker_ as observer for verified SCTs.
+ globals_->cert_transparency_verifier->SetObserver(ct_tree_tracker_.get());
+
// TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
// is fixed.
tracked_objects::ScopedTracker tracking_profile10(
@@ -766,6 +784,14 @@
system_url_request_context_getter_ = NULL;
+ // Unlink the ct_tree_tracker_ from the global cert_transparency_verifier
+ // and unregister it from new STH notifications so it will take no actions
+ // on anything observed during CleanUp process.
+ globals()->cert_transparency_verifier->SetObserver(nullptr);
+ UnregisterSTHObserver(ct_tree_tracker_.get());
+
+ ct_tree_tracker_.reset();
+
// Release objects that the net::URLRequestContext could have been pointing
// to.
@@ -917,6 +943,14 @@
globals()->host_resolver->SetDnsClientEnabled(*dns_client_enabled_);
}
+void IOThread::RegisterSTHObserver(net::ct::STHObserver* observer) {
+ chrome_browser_net::GetGlobalSTHDistributor()->RegisterObserver(observer);
+}
+
+void IOThread::UnregisterSTHObserver(net::ct::STHObserver* observer) {
+ chrome_browser_net::GetGlobalSTHDistributor()->UnregisterObserver(observer);
+}
+
// static
net::URLRequestContext* IOThread::ConstructSystemRequestContext(
IOThread::Globals* globals,
diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h
index 2f1103d4..d6fff07 100644
--- a/chrome/browser/io_thread.h
+++ b/chrome/browser/io_thread.h
@@ -44,6 +44,10 @@
}
#endif // BUILDFLAG(ANDROID_JAVA_UI)
+namespace certificate_transparency {
+class TreeStateTracker;
+}
+
namespace chrome_browser_net {
class DnsProbeService;
}
@@ -81,6 +85,11 @@
class URLRequestContext;
class URLRequestContextGetter;
class URLRequestJobFactory;
+
+namespace ct {
+class STHObserver;
+}
+
} // namespace net
namespace net_log {
@@ -223,6 +232,12 @@
// Returns the callback for updating data use prefs.
const metrics::UpdateUsagePrefCallbackType& GetMetricsDataUseForwarder();
+ // Registers the |observer| for new STH notifications.
+ void RegisterSTHObserver(net::ct::STHObserver* observer);
+
+ // Un-registers the |observer|.
+ void UnregisterSTHObserver(net::ct::STHObserver* observer);
+
private:
// Provide SystemURLRequestContextGetter with access to
// InitSystemRequestContext().
@@ -305,6 +320,8 @@
// Observer that logs network changes to the ChromeNetLog.
std::unique_ptr<net::LoggingNetworkChangeObserver> network_change_observer_;
+ std::unique_ptr<certificate_transparency::TreeStateTracker> ct_tree_tracker_;
+
BooleanPrefMember system_enable_referrers_;
BooleanPrefMember dns_client_enabled_;
diff --git a/chrome/browser/net/sth_distributor_provider.cc b/chrome/browser/net/sth_distributor_provider.cc
new file mode 100644
index 0000000..97a109e
--- /dev/null
+++ b/chrome/browser/net/sth_distributor_provider.cc
@@ -0,0 +1,27 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/net/sth_distributor_provider.h"
+
+#include "base/lazy_instance.h"
+#include "net/cert/sth_distributor.h"
+
+namespace chrome_browser_net {
+
+namespace {
+base::LazyInstance<std::unique_ptr<net::ct::STHDistributor>>
+ global_sth_distributor = LAZY_INSTANCE_INITIALIZER;
+} // namespace
+
+void SetGlobalSTHDistributor(
+ std::unique_ptr<net::ct::STHDistributor> distributor) {
+ global_sth_distributor.Get().swap(distributor);
+}
+
+net::ct::STHDistributor* GetGlobalSTHDistributor() {
+ CHECK(global_sth_distributor.Get());
+ return global_sth_distributor.Get().get();
+}
+
+} // namespace chrome_browser_net
diff --git a/chrome/browser/net/sth_distributor_provider.h b/chrome/browser/net/sth_distributor_provider.h
new file mode 100644
index 0000000..b8e36676
--- /dev/null
+++ b/chrome/browser/net/sth_distributor_provider.h
@@ -0,0 +1,26 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_NET_STH_DISTRIBUTOR_PROVIDER_H_
+#define CHROME_BROWSER_NET_STH_DISTRIBUTOR_PROVIDER_H_
+
+#include <memory>
+
+namespace net {
+
+namespace ct {
+class STHDistributor;
+} // namespace ct
+
+} // namespace net
+
+namespace chrome_browser_net {
+
+void SetGlobalSTHDistributor(
+ std::unique_ptr<net::ct::STHDistributor> distributor);
+net::ct::STHDistributor* GetGlobalSTHDistributor();
+
+} // namespace chrome_browser_net
+
+#endif // CHROME_BROWSER_NET_STH_DISTRIBUTOR_PROVIDER_H_
diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc
index a3bda6c..11c06609c1 100644
--- a/chrome/browser/profiles/profile_io_data.cc
+++ b/chrome/browser/profiles/profile_io_data.cc
@@ -58,6 +58,7 @@
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "components/about_handler/about_protocol_handler.h"
+#include "components/certificate_transparency/tree_state_tracker.h"
#include "components/content_settings/core/browser/content_settings_provider.h"
#include "components/content_settings/core/browser/cookie_settings.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
@@ -81,6 +82,7 @@
#include "content/public/browser/resource_context.h"
#include "net/base/keygen_handler.h"
#include "net/cert/cert_verifier.h"
+#include "net/cert/ct_log_verifier.h"
#include "net/cert/multi_log_ct_verifier.h"
#include "net/cookies/canonical_cookie.h"
#include "net/http/http_network_session.h"
@@ -1150,7 +1152,16 @@
new net::MultiLogCTVerifier());
ct_verifier->AddLogs(io_thread_globals->ct_logs);
main_request_context_->set_cert_transparency_verifier(ct_verifier.get());
+
+ ct_tree_tracker_.reset(new certificate_transparency::TreeStateTracker(
+ io_thread_globals->ct_logs));
+ ct_verifier->SetObserver(ct_tree_tracker_.get());
+
cert_transparency_verifier_ = std::move(ct_verifier);
+ io_thread->RegisterSTHObserver(ct_tree_tracker_.get());
+ ct_tree_tracker_unregistration_ =
+ base::Bind(&IOThread::UnregisterSTHObserver, base::Unretained(io_thread),
+ ct_tree_tracker_.get());
InitializeInternal(std::move(network_delegate), profile_params_.get(),
protocol_handlers, std::move(request_interceptors));
@@ -1299,6 +1310,12 @@
void ProfileIOData::DestroyResourceContext() {
resource_context_.reset();
+ // Prevent the cert_transparency_observer_ from getting any more
+ // notifications by severing the link between it and the
+ // cert_transparency_verifier_ and unregistering it from new STH
+ // notifications.
+ cert_transparency_verifier_->SetObserver(nullptr);
+ ct_tree_tracker_unregistration_.Run();
}
std::unique_ptr<net::HttpNetworkSession>
diff --git a/chrome/browser/profiles/profile_io_data.h b/chrome/browser/profiles/profile_io_data.h
index 116d98b..6b00aa8 100644
--- a/chrome/browser/profiles/profile_io_data.h
+++ b/chrome/browser/profiles/profile_io_data.h
@@ -54,6 +54,10 @@
class ResourcePrefetchPredictorObserver;
}
+namespace certificate_transparency {
+class TreeStateTracker;
+}
+
namespace content_settings {
class CookieSettings;
}
@@ -610,6 +614,10 @@
mutable DevToolsNetworkControllerHandle network_controller_handle_;
+ mutable std::unique_ptr<certificate_transparency::TreeStateTracker>
+ ct_tree_tracker_;
+ mutable base::Closure ct_tree_tracker_unregistration_;
+
const Profile::ProfileType profile_type_;
DISALLOW_COPY_AND_ASSIGN(ProfileIOData);
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 8698d038..c446ccf2 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2183,6 +2183,8 @@
'browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h',
'browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.cc',
'browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h',
+ 'browser/net/sth_distributor_provider.cc',
+ 'browser/net/sth_distributor_provider.h',
'browser/net/timed_cache.cc',
'browser/net/timed_cache.h',
'browser/net/url_info.cc',
@@ -3327,6 +3329,7 @@
'../components/components.gyp:autofill_content_browser',
'../components/components.gyp:browsing_data',
'../components/components.gyp:certificate_reporting',
+ '../components/components.gyp:certificate_transparency',
'../components/components.gyp:contextual_search_browser',
'../components/components.gyp:cookie_config',
'../components/components.gyp:data_reduction_proxy_content_browser',
