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

1 files changed, 9 insertions, 103 deletions

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) {