Certificate Transparency: Start tracking logs' state

chrome/browser/io_thread.cc

 // Copyright (c) 2012 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/io_thread.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/base64.h"
@@ skipping 25 matching lines @@
 #include "chrome/browser/data_usage/tab_id_annotator.h"
 #include "chrome/browser/net/async_dns_field_trial.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
 #include "chrome/browser/net/connect_interceptor.h"
 #include "chrome/browser/net/dns_probe_service.h"
 #include "chrome/browser/net/proxy_service_factory.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_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "components/data_usage/core/data_use_aggregator.h"
 #include "components/data_usage/core/data_use_amortizer.h"
 #include "components/data_usage/core/data_use_annotator.h"
 #include "components/metrics/metrics_service.h"
 #include "components/net_log/chrome_net_log.h"
 #include "components/policy/core/common/policy_service.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/proxy_config/pref_proxy_config_tracker.h"
 #include "components/variations/variations_associated_data.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/cookie_store_factory.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/user_agent.h"
 #include "net/base/external_estimate_provider.h"
 #include "net/base/host_mapping_rules.h"
 #include "net/base/network_quality_estimator.h"
 #include "net/base/sdch_manager.h"
 #include "net/cert/cert_verifier.h"
 #include "net/cert/cert_verify_proc.h"
 #include "net/cert/ct_known_logs.h"
 #include "net/cert/ct_log_verifier.h"
+#include "net/cert/ct_observer.h"
 #include "net/cert/ct_policy_enforcer.h"
 #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_observer.h"
+#include "net/cert/sth_reporter.h"
 #include "net/cookies/cookie_store.h"
 #include "net/dns/host_cache.h"
 #include "net/dns/host_resolver.h"
 #include "net/dns/mapped_host_resolver.h"
 #include "net/ftp/ftp_network_layer.h"
 #include "net/http/http_auth_filter.h"
 #include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_auth_preferences.h"
 #include "net/http/http_network_layer.h"
 #include "net/http/http_server_properties_impl.h"
@@ skipping 337 matching lines @@
     policy::PolicyService* policy_service,
     net_log::ChromeNetLog* net_log,
     extensions::EventRouterForwarder* extension_event_router_forwarder)
     : net_log_(net_log),
 #if defined(ENABLE_EXTENSIONS)
       extension_event_router_forwarder_(extension_event_router_forwarder),
 #endif
       globals_(NULL),
       is_spdy_allowed_by_policy_(true),
       is_quic_allowed_by_policy_(true),
+      sth_reporter_(nullptr),
       creation_time_(base::TimeTicks::Now()),
       weak_factory_(this) {
   scoped_refptr<base::SingleThreadTaskRunner> io_thread_proxy =
       BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO);
   auth_schemes_ = local_state->GetString(prefs::kAuthSchemes);
   negotiate_disable_cname_lookup_.Init(
       prefs::kDisableAuthNegotiateCnameLookup, local_state,
       base::Bind(&IOThread::UpdateNegotiateDisableCnameLookup,
                  base::Unretained(this)));
   negotiate_disable_cname_lookup_.MoveToThread(io_thread_proxy);
@@ skipping 72 matching lines @@
 
   BrowserThread::SetDelegate(BrowserThread::IO, this);
 }
 
 IOThread::~IOThread() {
   // This isn't needed for production code, but in tests, IOThread may
   // be multiply constructed.
   BrowserThread::SetDelegate(BrowserThread::IO, NULL);
 
   pref_proxy_config_tracker_->DetachFromPrefService();
+  DCHECK(sth_observers_.empty());
   DCHECK(!globals_);
 }
 
 IOThread::Globals* IOThread::globals() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   return globals_;
 }
 
 void IOThread::SetGlobalsForTesting(Globals* globals) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ skipping 328 matching lines @@
 
 void IOThread::CleanUp() {
   base::debug::LeakTracker<SafeBrowsingURLRequestContext>::CheckForLeaks();
 
 #if defined(USE_NSS_CERTS)
   net::ShutdownNSSHttpIO();
 #endif
 
   system_url_request_context_getter_ = NULL;
 
-  // Release objects that the net::URLRequestContext could have been pointing
-  // to.
+  // Since the cert_transparency_observer will be deleted first, unlink
+  // it from the cert_transparency_verifier by nullifying the observer.
+  globals()->cert_transparency_verifier->SetObserver(nullptr);
+
+  if (sth_reporter_) {
+    for (const auto& observer : sth_observers_) {
+      sth_reporter_->UnregisterObserver(observer);
+    }
+    // Make sure no registration happens after CleanUp
+    sth_reporter_ = nullptr;
+  }
+  sth_observers_.clear();
 
   // Shutdown the HistogramWatcher on the IO thread.
   net::NetworkChangeNotifier::ShutdownHistogramWatcher();
 
   // This must be reset before the ChromeNetLog is destroyed.
   network_change_observer_.reset();
 
   system_proxy_config_service_.reset();
   delete globals_;
   globals_ = NULL;
@@ skipping 323 matching lines @@
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
   globals_->system_proxy_service = ProxyServiceFactory::CreateProxyService(
       net_log_, globals_->proxy_script_fetcher_context.get(),
       globals_->system_network_delegate.get(),
       std::move(system_proxy_config_service_), command_line,
       quick_check_enabled_.GetValue());
 
   globals_->system_request_context.reset(
       ConstructSystemRequestContext(globals_, params_, net_log_));
+
+  globals_->cert_transparency_observer.reset(
+      new certificate_transparency::TreeStateTracker(globals_->ct_logs));
+  RegisterSTHObserver(globals_->cert_transparency_observer.get());
+  // The |cert_transparency_verifier| is the same one held by
+  // the |proxy_script_fetcher_context| and |system_request_context|,
+  // so no need to set the observer in their cert_transparency_verifiers.
+  globals_->cert_transparency_verifier->SetObserver(
+      globals_->cert_transparency_observer.get());
 }
 
 void IOThread::UpdateDnsClientEnabled() {
   globals()->host_resolver->SetDnsClientEnabled(*dns_client_enabled_);
 }
 
 // static
 void IOThread::NetworkSessionConfigurator::ConfigureQuicParams(
     const base::CommandLine& command_line,
     base::StringPiece quic_trial_group,
@@ skipping 153 matching lines @@
     const base::CommandLine& command_line) {
   if (command_line.HasSwitch(switches::kDisableQuicPortSelection))
     return false;
 
   if (command_line.HasSwitch(switches::kEnableQuicPortSelection))
     return true;
 
   return false;  // Default to disabling port selection on all channels.
 }
 
+void IOThread::RegisterSTHReporter(net::ct::STHReporter* reporter) {
+  DCHECK(globals());
+  // A sanity check to make sure the observer was created by now.
+  DCHECK(globals()->cert_transparency_observer.get());

[Ryan Sleevi, 2016/04/27 23:05:59]

Why? The justification for why this is done is unclear.

[Eran Messeri, 2016/05/03 15:00:12]

Removed, this was to verify assumptions during development.
(Note this file was removed in the next patchset)

+
+  sth_reporter_ = reporter;
+  // Register all observers that were created before the reporter was.
+  for (const auto& observer : sth_observers_) {
+    sth_reporter_->RegisterObserver(observer);
+  }
+}
+
+void IOThread::RegisterSTHObserver(net::ct::STHObserver* observer) {
+  sth_observers_.insert(observer);
+  // If a reporter was set, also register this observer with it.
+  // Otherwise it will be registered in RegisterSTHReporter with all
+  // other pending observers.
+  if (sth_reporter_) {

[Ryan Sleevi, 2016/04/27 23:05:59]

Dominant style in this file seems to be no braces for one-lines.

[Eran Messeri, 2016/05/03 15:00:12]

Done.
(Note this file was removed in the next patchset)

+    sth_reporter_->RegisterObserver(observer);
+  }
+}
+
+void IOThread::UnregisterSTHObserver(net::ct::STHObserver* observer) {
+  DCHECK_NE(sth_observers_.count(observer), 0u);

[Ryan Sleevi, 2016/04/27 23:05:59]

Why? It's clear that you intend to make sure the observer is already registered,
but why don't you check during register if an object already has been registered
once before?

It's not clear what pattern(s) you're concerned about, because it feels
asymmetric.

[Eran Messeri, 2016/05/03 15:00:12]

I'd like to verify that the object was (1) registered only once and (2) that
everything that's unregistered was at one point registered.

To verify (1) I've added DCHECK_EQ(sth_observers_.count(observer), 0u) to
RegisterSTHObserver.

Does the DCHECK here now make sense?

+  sth_observers_.erase(observer);
+  if (sth_reporter_) {
+    sth_reporter_->UnregisterObserver(observer);
+  }
+}

[Ryan Sleevi, 2016/04/27 23:05:59]

Is this level of reciprocity (set a reporter it registers observers, set an
observer it registers reporters) necessary? It seems to make the API itself
somewhat confusing.

For example, what if someone does RegisterSTHReporter after an sth_reporter_ is
set? None of the observers are unregistered from the old reporter.

Is that a BUG? I don't know, because it's unclear what the right API usage
scenario is or should be.

What if I register an STH observer, set a new reporter, then unregister? That
seems like it would give to a BUG (the new sth_reporter_ doesn't have the
observer registered).

Is that a BUG? I don't know, because it's unclear what the right API usage
scenario is or should be.

[Eran Messeri, 2016/05/03 15:00:12]

Good point. I have:
(1) Added a DCHECK to verify that the sth_reporter_ is not set in
RegisterSTHReporter. Current design is to have exactly one sth_reporter_ during
the lifetime of IOThread (as I don't see a need to change it).
(2) Clarified in the documentation that RegisterSTHReporter is expected to be
called only once, all observers registered with RegisterSTHObserver before it
was called will be registered and all observers registered with
RegisterSTHObserver after it was called will be registered immediately.

+
 // static
 net::QuicTagVector
 IOThread::NetworkSessionConfigurator::GetQuicConnectionOptions(
     const base::CommandLine& command_line,
     const VariationParameters& quic_trial_params) {
   if (command_line.HasSwitch(switches::kQuicConnectionOptions)) {
     return net::QuicUtils::ParseQuicConnectionOptions(
         command_line.GetSwitchValueASCII(switches::kQuicConnectionOptions));
   }
 
@@ skipping 375 matching lines @@
   // TODO(rtenneti): We should probably use HttpServerPropertiesManager for the
   // system URLRequestContext too. There's no reason this should be tied to a
   // profile.
   return context;
 }
 
 const metrics::UpdateUsagePrefCallbackType&
 IOThread::GetMetricsDataUseForwarder() {
   return metrics_data_use_forwarder_;
 }