net: move importing a DER certificate chain into X509Certificate.

This is a prelude to having SSLHostInfo be able to kick off
certificate validations.

The vector of intermediate certificates is added on Linux because,
otherwise, nothing is holding a reference to them. Previously, the
nss_fd_ was holding a reference. However, without the vector holding
references, CreateFromDERChain deletes them all at the end and NSS has
to AIA chase up the chain.

BUG=none
TEST=net_unittests

http://codereview.chromium.org/3920001

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

5 files changed, 152 insertions, 123 deletions

diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc
index e21a3bc..ad49f18 100644
--- a/net/base/x509_certificate.cc
+++ b/net/base/x509_certificate.cc
@@ -153,6 +153,100 @@ X509Certificate* X509Certificate::CreateFromHandle(
   return cert;
 }
 
+#if defined(OS_WIN)
+// See IsProblematicComodoEVCACert, below.
+// Issuer:
+// CN = AddTrust External CA Root
+// OU = AddTrust External TTP Network
+// O = AddTrust AB
+// C = SE
+//
+// This is the first 308 bytes of the certificate, which covers the serial
+// number, issuer and subject. It stops just short of the public key.
+static const uint8 kProblematicComodoEVCACert[] = {
+  0x30, 0x82, 0x04, 0xd5, 0x30, 0x82, 0x03, 0xbd, 0xa0, 0x03, 0x02, 0x01, 0x02,
+  0x02, 0x10, 0x79, 0x0a, 0x83, 0x4d, 0x48, 0x40, 0x6b, 0xab, 0x6c, 0x35, 0x2a,
+  0xd5, 0x1f, 0x42, 0x83, 0xfe, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86,
+  0xf7, 0x0d, 0x01, 0x01, 0x05, 0x05, 0x00, 0x30, 0x6f, 0x31, 0x0b, 0x30, 0x09,
+  0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x53, 0x45, 0x31, 0x14, 0x30, 0x12,
+  0x06, 0x03, 0x55, 0x04, 0x0a, 0x13, 0x0b, 0x41, 0x64, 0x64, 0x54, 0x72, 0x75,
+  0x73, 0x74, 0x20, 0x41, 0x42, 0x31, 0x26, 0x30, 0x24, 0x06, 0x03, 0x55, 0x04,
+  0x0b, 0x13, 0x1d, 0x41, 0x64, 0x64, 0x54, 0x72, 0x75, 0x73, 0x74, 0x20, 0x45,
+  0x78, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x20, 0x54, 0x54, 0x50, 0x20, 0x4e,
+  0x65, 0x74, 0x77, 0x6f, 0x72, 0x6b, 0x31, 0x22, 0x30, 0x20, 0x06, 0x03, 0x55,
+  0x04, 0x03, 0x13, 0x19, 0x41, 0x64, 0x64, 0x54, 0x72, 0x75, 0x73, 0x74, 0x20,
+  0x45, 0x78, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x20, 0x43, 0x41, 0x20, 0x52,
+  0x6f, 0x6f, 0x74, 0x30, 0x1e, 0x17, 0x0d, 0x30, 0x37, 0x30, 0x31, 0x30, 0x31,
+  0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x5a, 0x17, 0x0d, 0x32, 0x30, 0x30, 0x35,
+  0x33, 0x30, 0x31, 0x30, 0x34, 0x38, 0x33, 0x38, 0x5a, 0x30, 0x73, 0x31, 0x0b,
+  0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x47, 0x42, 0x31, 0x1b,
+  0x30, 0x19, 0x06, 0x03, 0x55, 0x04, 0x08, 0x13, 0x12, 0x47, 0x72, 0x65, 0x61,
+  0x74, 0x65, 0x72, 0x20, 0x4d, 0x61, 0x6e, 0x63, 0x68, 0x65, 0x73, 0x74, 0x65,
+  0x72, 0x31, 0x10, 0x30, 0x0e, 0x06, 0x03, 0x55, 0x04, 0x07, 0x13, 0x07, 0x53,
+  0x61, 0x6c, 0x66, 0x6f, 0x72, 0x64, 0x31, 0x1a, 0x30, 0x18, 0x06, 0x03, 0x55,
+  0x04, 0x0a, 0x13, 0x11, 0x43, 0x4f, 0x4d, 0x4f, 0x44, 0x4f, 0x20, 0x43, 0x41,
+  0x20, 0x4c, 0x69, 0x6d, 0x69, 0x74, 0x65, 0x64, 0x31, 0x19, 0x30, 0x17, 0x06,
+  0x03, 0x55, 0x04, 0x03, 0x13, 0x10, 0x43, 0x4f, 0x4d, 0x4f, 0x44, 0x4f, 0x20,
+  0x45, 0x56, 0x20, 0x53, 0x47, 0x43, 0x20, 0x43, 0x41,
+};
+
+// A certificate for COMODO EV SGC CA, issued by AddTrust External CA Root,
+// causes CertGetCertificateChain to report CERT_TRUST_IS_NOT_VALID_FOR_USAGE.
+// It seems to be caused by the szOID_APPLICATION_CERT_POLICIES extension in
+// that certificate.
+//
+// This function is used in the workaround for http://crbug.com/43538
+static bool IsProblematicComodoEVCACert(base::StringPiece der_cert) {
+  return der_cert.size() >= sizeof(kProblematicComodoEVCACert) &&
+         memcmp(der_cert.data(), kProblematicComodoEVCACert,
+                sizeof(kProblematicComodoEVCACert)) == 0;
+}
+#endif
+
+#if defined(OS_WIN)
+static X509Certificate::OSCertHandle CreateOSCert(base::StringPiece der_cert) {
+  X509Certificate::OSCertHandle cert_handle = NULL;
+  BOOL ok = CertAddEncodedCertificateToStore(
+      X509Certificate::cert_store(), X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
+      reinterpret_cast<const BYTE*>(der_cert.data()), der_cert.size(),
+      CERT_STORE_ADD_USE_EXISTING, &cert_handle);
+  return ok ? cert_handle : NULL;
+}
+#else
+static X509Certificate::OSCertHandle CreateOSCert(base::StringPiece der_cert) {
+  return X509Certificate::CreateOSCertHandleFromBytes(
+      const_cast<char*>(der_cert.data()), der_cert.size());
+}
+#endif
+
+// static
+X509Certificate* X509Certificate::CreateFromDERCertChain(
+    const std::vector<base::StringPiece>& der_certs) {
+  if (der_certs.size() == 0)
+    return NULL;
+
+  X509Certificate::OSCertHandles intermediate_ca_certs;
+  for (size_t i = 1; i < der_certs.size(); i++) {
+#if defined(OS_WIN)
+    if (IsProblematicComodoEVCACert(der_certs[i]))
+      continue;
+#endif
+    OSCertHandle handle = CreateOSCert(der_certs[i]);
+    DCHECK(handle);
+    intermediate_ca_certs.push_back(handle);
+  }
+
+  OSCertHandle handle = CreateOSCert(der_certs[0]);
+  DCHECK(handle);
+  X509Certificate* cert =
+      CreateFromHandle(handle, SOURCE_FROM_NETWORK, intermediate_ca_certs);
+  FreeOSCertHandle(handle);
+  for (size_t i = 0; i < intermediate_ca_certs.size(); i++)
+    FreeOSCertHandle(intermediate_ca_certs[i]);
+
+  return cert;
+}
+
 // static
 X509Certificate* X509Certificate::CreateFromBytes(const char* data,
                                                   int length) {
@@ -249,11 +343,9 @@ X509Certificate::X509Certificate(OSCertHandle cert_handle,
                                  const OSCertHandles& intermediates)
     : cert_handle_(DupOSCertHandle(cert_handle)),
       source_(source) {
-#if defined(OS_MACOSX) || defined(OS_WIN) || defined(USE_OPENSSL)
   // Copy/retain the intermediate cert handles.
   for (size_t i = 0; i < intermediates.size(); ++i)
     intermediate_ca_certs_.push_back(DupOSCertHandle(intermediates[i]));
-#endif
   // Platform-specific initialization.
   Initialize();
 }
@@ -276,10 +368,8 @@ X509Certificate::~X509Certificate() {
   X509Certificate::Cache::GetInstance()->Remove(this);
   if (cert_handle_)
     FreeOSCertHandle(cert_handle_);
-#if defined(OS_MACOSX) || defined(OS_WIN)
   for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i)
     FreeOSCertHandle(intermediate_ca_certs_[i]);
-#endif
 }
 
 bool X509Certificate::HasExpired() const {
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index 577de92..a7eef9d 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -13,6 +13,7 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/ref_counted.h"
+#include "base/string_piece.h"
 #include "base/time.h"
 #include "net/base/x509_cert_types.h"
 
@@ -107,12 +108,20 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   // certificate cache prefers the handle from the network because our HTTP
   // cache isn't caching the corresponding intermediate CA certificates yet
   // (http://crbug.com/7065).
-  // The list of intermediate certificates is ignored under NSS (i.e. Linux.)
   // The returned pointer must be stored in a scoped_refptr<X509Certificate>.
   static X509Certificate* CreateFromHandle(OSCertHandle cert_handle,
       Source source,
       const OSCertHandles& intermediates);
 
+  // Create an X509Certificate from a chain of DER encoded certificates. The
+  // first certificate in the chain is the end-entity certificate to which a
+  // handle is returned. The other certificates in the chain are intermediate
+  // certificates. See the comment for |CreateFromHandle| about the |source|
+  // argument.
+  // The returned pointer must be stored in a scoped_refptr<X509Certificate>.
+  static X509Certificate* CreateFromDERCertChain(
+      const std::vector<base::StringPiece>& der_certs);
+
   // Create an X509Certificate from the DER-encoded representation.
   // Returns NULL on failure.
   //
@@ -173,14 +182,12 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   // now.
   bool HasExpired() const;
 
-#if defined(OS_MACOSX) || defined(OS_WIN) || defined(USE_OPENSSL)
   // Returns intermediate certificates added via AddIntermediateCertificate().
   // Ownership follows the "get" rule: it is the caller's responsibility to
   // retain the elements of the result.
   const OSCertHandles& GetIntermediateCertificates() const {
     return intermediate_ca_certs_;
   }
-#endif
 
   // Returns true if I already contain the given intermediate cert.
   bool HasIntermediateCertificate(OSCertHandle cert);
@@ -213,6 +220,17 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   CFArrayRef CreateClientCertificateChain() const;
 #endif
 
+#if defined(OS_WIN)
+  // Returns a handle to a global, in-memory certificate store. We use it for
+  // two purposes:
+  // 1. Import server certificates into this store so that we can verify and
+  //    display the certificates using CryptoAPI.
+  // 2. Copy client certificates from the "MY" system certificate store into
+  //    this store so that we can close the system store when we finish
+  //    searching for client certificates.
+  static HCERTSTORE cert_store();
+#endif
+
   // Verifies the certificate against the given hostname.  Returns OK if
   // successful or an error code upon failure.
   //
@@ -292,11 +310,9 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   // A handle to the certificate object in the underlying crypto library.
   OSCertHandle cert_handle_;
 
-#if defined(OS_MACOSX) || defined(OS_WIN) || defined(USE_OPENSSL)
   // Untrusted intermediate certificates associated with this certificate
-  // that may be needed for chain building. (NSS impl does not need these.)
+  // that may be needed for chain building.
   OSCertHandles intermediate_ca_certs_;
-#endif
 
 #if defined(OS_MACOSX)
   // Blocks multiple threads from verifying the cert simultaneously.
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index 380ff3c..1236f85 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -575,6 +575,33 @@ void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
     dns_names->push_back(subject_.common_name);
 }
 
+class GlobalCertStore {
+ public:
+  HCERTSTORE cert_store() {
+    return cert_store_;
+  }
+
+ private:
+  friend struct DefaultSingletonTraits<GlobalCertStore>;
+
+  GlobalCertStore()
+      : cert_store_(CertOpenStore(CERT_STORE_PROV_MEMORY, 0, NULL, 0, NULL)) {
+  }
+
+  ~GlobalCertStore() {
+    CertCloseStore(cert_store_, 0 /* flags */);
+  }
+
+  const HCERTSTORE cert_store_;
+
+  DISALLOW_COPY_AND_ASSIGN(GlobalCertStore);
+};
+
+// static
+HCERTSTORE X509Certificate::cert_store() {
+  return Singleton<GlobalCertStore>::get()->cert_store();
+}
+
 int X509Certificate::Verify(const std::string& hostname,
                             int flags,
                             CertVerifyResult* verify_result) const {
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 324f50c..abbd2fa 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -321,46 +321,6 @@ void LogFailedNSSFunction(const BoundNetLog& net_log,
 
 #if defined(OS_WIN)
 
-// A certificate for COMODO EV SGC CA, issued by AddTrust External CA Root,
-// causes CertGetCertificateChain to report CERT_TRUST_IS_NOT_VALID_FOR_USAGE.
-// It seems to be caused by the szOID_APPLICATION_CERT_POLICIES extension in
-// that certificate.
-//
-// This function is used in the workaround for http://crbug.com/43538
-bool IsProblematicComodoEVCACert(const CERTCertificate& cert) {
-  // Issuer:
-  // CN = AddTrust External CA Root
-  // OU = AddTrust External TTP Network
-  // O = AddTrust AB
-  // C = SE
-  static const uint8 kIssuer[] = {
-    0x30, 0x6f, 0x31, 0x0b, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04,
-    0x06, 0x13, 0x02, 0x53, 0x45, 0x31, 0x14, 0x30, 0x12, 0x06,
-    0x03, 0x55, 0x04, 0x0a, 0x13, 0x0b, 0x41, 0x64, 0x64, 0x54,
-    0x72, 0x75, 0x73, 0x74, 0x20, 0x41, 0x42, 0x31, 0x26, 0x30,
-    0x24, 0x06, 0x03, 0x55, 0x04, 0x0b, 0x13, 0x1d, 0x41, 0x64,
-    0x64, 0x54, 0x72, 0x75, 0x73, 0x74, 0x20, 0x45, 0x78, 0x74,
-    0x65, 0x72, 0x6e, 0x61, 0x6c, 0x20, 0x54, 0x54, 0x50, 0x20,
-    0x4e, 0x65, 0x74, 0x77, 0x6f, 0x72, 0x6b, 0x31, 0x22, 0x30,
-    0x20, 0x06, 0x03, 0x55, 0x04, 0x03, 0x13, 0x19, 0x41, 0x64,
-    0x64, 0x54, 0x72, 0x75, 0x73, 0x74, 0x20, 0x45, 0x78, 0x74,
-    0x65, 0x72, 0x6e, 0x61, 0x6c, 0x20, 0x43, 0x41, 0x20, 0x52,
-    0x6f, 0x6f, 0x74
-  };
-
-  // Serial number: 79:0A:83:4D:48:40:6B:AB:6C:35:2A:D5:1F:42:83:FE.
-  static const uint8 kSerialNumber[] = {
-    0x79, 0x0a, 0x83, 0x4d, 0x48, 0x40, 0x6b, 0xab, 0x6c, 0x35,
-    0x2a, 0xd5, 0x1f, 0x42, 0x83, 0xfe
-  };
-
-  return cert.derIssuer.len == sizeof(kIssuer) &&
-         memcmp(cert.derIssuer.data, kIssuer, cert.derIssuer.len) == 0 &&
-         cert.serialNumber.len == sizeof(kSerialNumber) &&
-         memcmp(cert.serialNumber.data, kSerialNumber,
-                cert.serialNumber.len) == 0;
-}
-
 // This callback is intended to be used with CertFindChainInStore. In addition
 // to filtering by extended/enhanced key usage, we do not show expired
 // certificates and require digital signature usage in the key usage
@@ -399,11 +359,6 @@ BOOL WINAPI ClientCertFindCallback(PCCERT_CONTEXT cert_context,
 
 }  // namespace
 
-#if defined(OS_WIN)
-// static
-HCERTSTORE SSLClientSocketNSS::cert_store_ = NULL;
-#endif
-
 SSLClientSocketNSS::SSLClientSocketNSS(ClientSocketHandle* transport_socket,
                                        const std::string& hostname,
                                        const SSLConfig& ssl_config,
@@ -1050,28 +1005,6 @@ bool SSLClientSocketNSS::SetSendBufferSize(int32 size) {
   return transport_->socket()->SetSendBufferSize(size);
 }
 
-#if defined(OS_WIN)
-// static
-X509Certificate::OSCertHandle SSLClientSocketNSS::CreateOSCert(
-    const SECItem& der_cert) {
-  // TODO(wtc): close cert_store_ at shutdown.
-  if (!cert_store_)
-    cert_store_ = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, NULL, 0, NULL);
-
-  X509Certificate::OSCertHandle cert_handle = NULL;
-  BOOL ok = CertAddEncodedCertificateToStore(
-      cert_store_, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
-      der_cert.data, der_cert.len, CERT_STORE_ADD_USE_EXISTING, &cert_handle);
-  return ok ? cert_handle : NULL;
-}
-#elif defined(OS_MACOSX)
-// static
-X509Certificate::OSCertHandle SSLClientSocketNSS::CreateOSCert(
-    const SECItem& der_cert) {
-  return X509Certificate::CreateOSCertHandleFromBytes(
-      reinterpret_cast<char*>(der_cert.data), der_cert.len);
-}
-#endif
 
 X509Certificate *SSLClientSocketNSS::UpdateServerCert() {
   // We set the server_cert_ from HandshakeCallback(), but this handler
@@ -1080,47 +1013,23 @@ X509Certificate *SSLClientSocketNSS::UpdateServerCert() {
   if (server_cert_ == NULL) {
     server_cert_nss_ = SSL_PeerCertificate(nss_fd_);
     if (server_cert_nss_) {
-#if defined(OS_MACOSX) || defined(OS_WIN)
-      // Get each of the intermediate certificates in the server's chain.
-      // These will be added to the server's X509Certificate object, making
-      // them available to X509Certificate::Verify() for chain building.
-      X509Certificate::OSCertHandles intermediate_ca_certs;
-      X509Certificate::OSCertHandle cert_handle = NULL;
+#if defined(OS_WIN) || defined(OS_MACOSX)
+      std::vector<base::StringPiece> der_certs;
       CERTCertList* cert_list = CERT_GetCertChainFromCert(
           server_cert_nss_, PR_Now(), certUsageSSLCA);
       if (cert_list) {
         for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list);
              !CERT_LIST_END(node, cert_list);
              node = CERT_LIST_NEXT(node)) {
-          if (node->cert == server_cert_nss_)
-            continue;
-#if defined(OS_WIN)
-          // Work around http://crbug.com/43538 by not importing the
-          // problematic COMODO EV SGC CA certificate.  CryptoAPI will
-          // download a good certificate for that CA, issued by COMODO
-          // Certification Authority, using the AIA extension in the server
-          // certificate.
-          if (IsProblematicComodoEVCACert(*node->cert))
-            continue;
-#endif
-          cert_handle = CreateOSCert(node->cert->derCert);
-          DCHECK(cert_handle);
-          intermediate_ca_certs.push_back(cert_handle);
+          der_certs.push_back(base::StringPiece(
+                reinterpret_cast<const char*>(node->cert->derCert.data),
+                node->cert->derCert.len));
         }
+        server_cert_ = X509Certificate::CreateFromDERCertChain(der_certs);
         CERT_DestroyCertList(cert_list);
       }
-
-      // Finally create the X509Certificate object.
-      cert_handle = CreateOSCert(server_cert_nss_->derCert);
-      DCHECK(cert_handle);
-      server_cert_ = X509Certificate::CreateFromHandle(
-          cert_handle,
-          X509Certificate::SOURCE_FROM_NETWORK,
-          intermediate_ca_certs);
-      X509Certificate::FreeOSCertHandle(cert_handle);
-      for (size_t i = 0; i < intermediate_ca_certs.size(); ++i)
-        X509Certificate::FreeOSCertHandle(intermediate_ca_certs[i]);
 #else
+      // TODO(agl): this should use SSL_PeerCertificateChain
       server_cert_ = X509Certificate::CreateFromHandle(
           server_cert_nss_,
           X509Certificate::SOURCE_FROM_NETWORK,
@@ -1714,10 +1623,6 @@ SECStatus SSLClientSocketNSS::ClientAuthHandler(
 
   PCCERT_CHAIN_CONTEXT chain_context = NULL;
 
-  // TODO(wtc): close cert_store_ at shutdown.
-  if (!cert_store_)
-    cert_store_ = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, NULL, 0, NULL);
-
   for (;;) {
     // Find a certificate chain.
     chain_context = CertFindChainInStore(my_cert_store,
@@ -1739,7 +1644,8 @@ SECStatus SSLClientSocketNSS::ClientAuthHandler(
     // Copy it to our own certificate store, so that we can close the "MY"
     // certificate store before returning from this function.
     PCCERT_CONTEXT cert_context2;
-    BOOL ok = CertAddCertificateContextToStore(cert_store_, cert_context,
+    BOOL ok = CertAddCertificateContextToStore(X509Certificate::cert_store(),
+                                               cert_context,
                                                CERT_STORE_ADD_USE_EXISTING,
                                                &cert_context2);
     if (!ok) {
diff --git a/net/socket/ssl_client_socket_nss.h b/net/socket/ssl_client_socket_nss.h
index 56d16b2..098ef75 100644
--- a/net/socket/ssl_client_socket_nss.h
+++ b/net/socket/ssl_client_socket_nss.h
@@ -222,16 +222,6 @@ class SSLClientSocketNSS : public SSLClientSocket {
   bool predicted_npn_proto_used_;
 
   scoped_ptr<SSLHostInfo> ssl_host_info_;
-
-#if defined(OS_WIN)
-  // A CryptoAPI in-memory certificate store.  We use it for two purposes:
-  // 1. Import server certificates into this store so that we can verify and
-  //    display the certificates using CryptoAPI.
-  // 2. Copy client certificates from the "MY" system certificate store into
-  //    this store so that we can close the system store when we finish
-  //    searching for client certificates.
-  static HCERTSTORE cert_store_;
-#endif
 };
 
 }  // namespace net