net: don't pass the CRLSet in the SSLConfig.

The SSLConfig was a poor choice of location to carry the CRLSet because the
CRLSet can be updated while Chrome is running, but the SSLConfig is relatively
static and is cached in several places in the code.

This change causes the locations which call X509Certificate::Verify to grab a
new reference to the current CRLSet.

BUG=none
TEST=compiles


Review URL: http://codereview.chromium.org/9044011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@116720 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 37 insertions, 17 deletions

diff --git a/net/base/ssl_config_service.cc b/net/base/ssl_config_service.cc
index 6319407..fba5373 100644
--- a/net/base/ssl_config_service.cc
+++ b/net/base/ssl_config_service.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -6,6 +6,7 @@
 
 #include "base/lazy_instance.h"
 #include "base/memory/ref_counted.h"
+#include "base/synchronization/lock.h"
 #include "net/base/crl_set.h"
 #include "net/base/ssl_config_service_defaults.h"
 #include "net/base/ssl_false_start_blacklist.h"
@@ -61,8 +62,29 @@ bool SSLConfigService::IsKnownFalseStartIncompatibleServer(
 static bool g_cached_info_enabled = false;
 static bool g_false_start_enabled = true;
 static bool g_dns_cert_provenance_checking = false;
-base::LazyInstance<scoped_refptr<CRLSet>,
-                   base::LeakyLazyInstanceTraits<scoped_refptr<CRLSet> > >
+
+// GlobalCRLSet holds a reference to the global CRLSet. It simply wraps a lock
+// around a scoped_refptr so that getting a reference doesn't race with
+// updating the CRLSet.
+class GlobalCRLSet {
+ public:
+  void Set(const scoped_refptr<CRLSet>& new_crl_set) {
+    base::AutoLock locked(lock_);
+    crl_set_ = new_crl_set;
+  }
+
+  scoped_refptr<CRLSet> Get() const {
+    base::AutoLock locked(lock_);
+    return crl_set_;
+  }
+
+ private:
+  scoped_refptr<CRLSet> crl_set_;
+  mutable base::Lock lock_;
+};
+
+base::LazyInstance<GlobalCRLSet,
+                   base::LeakyLazyInstanceTraits<GlobalCRLSet> >
     g_crl_set = LAZY_INSTANCE_INITIALIZER;
 
 // static
@@ -87,12 +109,13 @@ bool SSLConfigService::dns_cert_provenance_checking_enabled() {
 
 // static
 void SSLConfigService::SetCRLSet(scoped_refptr<CRLSet> crl_set) {
-  g_crl_set.Get() = crl_set;
+  // Note: this can be called concurently with GetCRLSet().
+  g_crl_set.Get().Set(crl_set);
 }
 
 // static
 scoped_refptr<CRLSet> SSLConfigService::GetCRLSet() {
-  return g_crl_set.Get();
+  return g_crl_set.Get().Get();
 }
 
 void SSLConfigService::EnableCachedInfo() {
diff --git a/net/base/ssl_config_service.h b/net/base/ssl_config_service.h
index 02b74ad..5a17750 100644
--- a/net/base/ssl_config_service.h
+++ b/net/base/ssl_config_service.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -105,8 +105,6 @@ struct NET_EXPORT SSLConfig {
   std::vector<std::string> next_protos;
 
   scoped_refptr<X509Certificate> client_cert;
-
-  scoped_refptr<CRLSet> crl_set;
 };
 
 // The interface for retrieving the SSL configuration.  This interface
diff --git a/net/base/ssl_config_service_defaults.cc b/net/base/ssl_config_service_defaults.cc
index 18918b9..c4c7e55 100644
--- a/net/base/ssl_config_service_defaults.cc
+++ b/net/base/ssl_config_service_defaults.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -12,7 +12,6 @@ SSLConfigServiceDefaults::SSLConfigServiceDefaults() {
 void SSLConfigServiceDefaults::GetSSLConfig(SSLConfig* config) {
   *config = default_config_;
   SetSSLConfigFlags(config);
-  config->crl_set = GetCRLSet();
 }
 
 SSLConfigServiceDefaults::~SSLConfigServiceDefaults() {
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 815ce94..654b467 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -1714,7 +1714,7 @@ int SSLClientSocketNSS::DoVerifyCert(int result) {
   server_cert_verify_result_ = &local_server_cert_verify_result_;
   return verifier_->Verify(
       server_cert_, host_and_port_.host(), flags,
-      ssl_config_.crl_set,
+      SSLConfigService::GetCRLSet(),
       &local_server_cert_verify_result_,
       base::Bind(&SSLClientSocketNSS::OnHandshakeIOComplete,
                  base::Unretained(this)),
diff --git a/net/socket/ssl_host_info.cc b/net/socket/ssl_host_info.cc
index ad9165c..bc4a43e 100644
--- a/net/socket/ssl_host_info.cc
+++ b/net/socket/ssl_host_info.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -8,6 +8,7 @@
 #include "base/metrics/histogram.h"
 #include "base/pickle.h"
 #include "base/string_piece.h"
+#include "net/base/crl_set.h"
 #include "net/base/ssl_config_service.h"
 #include "net/base/x509_certificate.h"
 #include "net/socket/ssl_client_socket.h"
@@ -112,8 +113,9 @@ bool SSLHostInfo::ParseInner(const std::string& data) {
       VLOG(1) << "Kicking off verification for " << hostname_;
       verification_start_time_ = base::TimeTicks::Now();
       verification_end_time_ = base::TimeTicks();
+      scoped_refptr<CRLSet> crl_set(SSLConfigService::GetCRLSet());
       int rv = verifier_.Verify(
-          cert_.get(), hostname_, flags, crl_set_, &cert_verify_result_,
+          cert_.get(), hostname_, flags, crl_set, &cert_verify_result_,
           base::Bind(&SSLHostInfo::VerifyCallback, weak_factory_.GetWeakPtr()),
           // TODO(willchan): Figure out how to use NetLog here.
           BoundNetLog());
diff --git a/net/socket/ssl_host_info.h b/net/socket/ssl_host_info.h
index 406dae9..34fb0ef 100644
--- a/net/socket/ssl_host_info.h
+++ b/net/socket/ssl_host_info.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -20,7 +20,6 @@
 
 namespace net {
 
-class CRLSet;
 class X509Certificate;
 struct SSLConfig;
 
@@ -121,7 +120,6 @@ class NET_EXPORT_PRIVATE SSLHostInfo {
   // These three members are taken from the SSLConfig.
   bool rev_checking_enabled_;
   bool verify_ev_cert_;
-  scoped_refptr<CRLSet> crl_set_;
   base::TimeTicks verification_start_time_;
   base::TimeTicks verification_end_time_;
   CertVerifyResult cert_verify_result_;