Change the HTTP cache to cache the entire certificate chain for SSL sites

When persisting an X509Certificate to a pickle, such as when storing to the HTTP cache, persist any intermediate certificates in addition to the end-entity certificate. This will allow the complete certificate chain to be displayed to the end user when viewing a cached entry, independent of whether a network request has been made to that site during the browsing session.

R=agl
BUG=7065
TEST=X509CertificateTest.Persist

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

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

8 files changed, 250 insertions, 119 deletions

diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc
index 91ed346..b371c97 100644
--- a/net/base/x509_certificate.cc
+++ b/net/base/x509_certificate.cc
@@ -14,6 +14,7 @@
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram.h"
+#include "base/pickle.h"
 #include "base/sha1.h"
 #include "base/string_piece.h"
 #include "base/string_util.h"
@@ -231,6 +232,52 @@ X509Certificate* X509Certificate::CreateFromBytes(const char* data,
   return cert;
 }
 
+// static
+X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
+                                                   void** pickle_iter,
+                                                   PickleType type) {
+  OSCertHandle cert_handle = ReadCertHandleFromPickle(pickle, pickle_iter);
+  OSCertHandles intermediates;
+
+  // Even if a certificate fails to parse, whether the server certificate in
+  // |cert_handle| or one of the optional intermediates, continue reading
+  // the data from |pickle| so that |pickle_iter| is kept in sync for any
+  // other reads the caller may perform after this method returns.
+  if (type == PICKLETYPE_CERTIFICATE_CHAIN) {
+    size_t num_intermediates;
+    if (!pickle.ReadSize(pickle_iter, &num_intermediates)) {
+      FreeOSCertHandle(cert_handle);
+      return NULL;
+    }
+
+    bool ok = !!cert_handle;
+    for (size_t i = 0; i < num_intermediates; ++i) {
+      OSCertHandle intermediate = ReadCertHandleFromPickle(pickle,
+                                                           pickle_iter);
+      // If an intermediate fails to load, it and any certificates after it
+      // will not be added. However, any intermediates that were successfully
+      // parsed before the failure can be safely returned.
+      ok &= !!intermediate;
+      if (ok) {
+        intermediates.push_back(intermediate);
+      } else if (intermediate) {
+        FreeOSCertHandle(intermediate);
+      }
+    }
+  }
+
+  if (!cert_handle)
+    return NULL;
+  X509Certificate* cert = CreateFromHandle(cert_handle, SOURCE_FROM_CACHE,
+                                           intermediates);
+  FreeOSCertHandle(cert_handle);
+  for (size_t i = 0; i < intermediates.size(); ++i)
+    FreeOSCertHandle(intermediates[i]);
+
+  return cert;
+}
+
+// static
 CertificateList X509Certificate::CreateCertificateListFromBytes(
     const char* data, int length, int format) {
   OSCertHandles certificates;
@@ -308,6 +355,26 @@ CertificateList X509Certificate::CreateCertificateListFromBytes(
   return results;
 }
 
+void X509Certificate::Persist(Pickle* pickle) {
+  DCHECK(cert_handle_);
+  if (!WriteCertHandleToPickle(cert_handle_, pickle)) {
+    NOTREACHED();
+    return;
+  }
+
+  if (!pickle->WriteSize(intermediate_ca_certs_.size())) {
+    NOTREACHED();
+    return;
+  }
+
+  for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i) {
+    if (!WriteCertHandleToPickle(intermediate_ca_certs_[i], pickle)) {
+      NOTREACHED();
+      return;
+    }
+  }
+}
+
 bool X509Certificate::HasExpired() const {
   return base::Time::Now() > valid_expiry();
 }
@@ -317,15 +384,11 @@ bool X509Certificate::Equals(const X509Certificate* other) const {
 }
 
 bool X509Certificate::HasIntermediateCertificate(OSCertHandle cert) {
-#if defined(OS_MACOSX) || defined(OS_WIN) || defined(USE_OPENSSL)
   for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i) {
     if (IsSameOSCert(cert, intermediate_ca_certs_[i]))
       return true;
   }
   return false;
-#else
-  return true;
-#endif
 }
 
 bool X509Certificate::HasIntermediateCertificates(const OSCertHandles& certs) {
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index 7f2c881..6376b43 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -79,8 +79,11 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   enum Source {
     SOURCE_UNUSED = 0,            // The source_ member is not used.
     SOURCE_LONE_CERT_IMPORT = 1,  // From importing a certificate without
-                                  // its intermediate CA certificates.
-    SOURCE_FROM_NETWORK = 2,      // From the network.
+                                  // any intermediate CA certificates.
+    SOURCE_FROM_CACHE = 2,        // From the disk cache - which contains
+                                  // intermediate CA certificates, but may be
+                                  // stale.
+    SOURCE_FROM_NETWORK = 3,      // From the network.
   };
 
   enum VerifyFlags {
@@ -110,6 +113,17 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
                   FORMAT_PKCS7,
   };
 
+  enum PickleType {
+    // When reading a certificate from a Pickle, the Pickle only contains a
+    // single certificate.
+    PICKLETYPE_SINGLE_CERTIFICATE,
+
+    // When reading a certificate from a Pickle, the Pickle contains the
+    // the certificate plus any certificates that were stored in
+    // |intermediate_ca_certificates_| at the time it was serialized.
+    PICKLETYPE_CERTIFICATE_CHAIN,
+  };
+
   // Creates a X509Certificate from the ground up.  Used by tests that simulate
   // SSL connections.
   X509Certificate(const std::string& subject, const std::string& issuer,
@@ -123,8 +137,8 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   // (http://crbug.com/7065).
   // The returned pointer must be stored in a scoped_refptr<X509Certificate>.
   static X509Certificate* CreateFromHandle(OSCertHandle cert_handle,
-      Source source,
-      const OSCertHandles& intermediates);
+                                           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
@@ -148,7 +162,8 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   //
   // The returned pointer must be stored in a scoped_refptr<X509Certificate>.
   static X509Certificate* CreateFromPickle(const Pickle& pickle,
-                                           void** pickle_iter);
+                                           void** pickle_iter,
+                                           PickleType type);
 
   // Parses all of the certificates possible from |data|. |format| is a
   // bit-wise OR of Format, indicating the possible formats the
@@ -389,6 +404,17 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
                                       const uint8* array,
                                       size_t array_byte_len);
 
+  // Reads a single certificate from |pickle| and returns a platform-specific
+  // certificate handle. The format of the certificate stored in |pickle| is
+  // not guaranteed to be the same across different underlying cryptographic
+  // libraries, nor acceptable to CreateFromBytes(). Returns an invalid
+  // handle, NULL, on failure.
+  static OSCertHandle ReadCertHandleFromPickle(const Pickle& pickle,
+                                               void** pickle_iter);
+
+  // Writes a single certificate to |pickle|. Returns false on failure.
+  static bool WriteCertHandleToPickle(OSCertHandle handle, Pickle* pickle);
+
   // The subject of the certificate.
   CertPrincipal subject_;
 
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index 033ddbf..4bb9adf 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -634,17 +634,6 @@ bool X509Certificate::IsIssuedByKnownRoot(CFArrayRef chain) {
 }
 
 // static
-X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
-                                                   void** pickle_iter) {
-  const char* data;
-  int length;
-  if (!pickle.ReadData(pickle_iter, &data, &length))
-    return NULL;
-
-  return CreateFromBytes(data, length);
-}
-
-// static
 X509Certificate* X509Certificate::CreateSelfSigned(
     crypto::RSAPrivateKey* key,
     const std::string& subject,
@@ -784,17 +773,6 @@ X509Certificate* X509Certificate::CreateSelfSigned(
      X509Certificate::OSCertHandles());
 }
 
-void X509Certificate::Persist(Pickle* pickle) {
-  CSSM_DATA cert_data;
-  OSStatus status = SecCertificateGetData(cert_handle_, &cert_data);
-  if (status) {
-    NOTREACHED();
-    return;
-  }
-
-  pickle->WriteData(reinterpret_cast<char*>(cert_data.Data), cert_data.Length);
-}
-
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   dns_names->clear();
 
@@ -1340,4 +1318,28 @@ CFArrayRef X509Certificate::CreateClientCertificateChain() const {
   return chain.release();
 }
 
+// static
+X509Certificate::OSCertHandle
+X509Certificate::ReadCertHandleFromPickle(const Pickle& pickle,
+                                          void** pickle_iter) {
+  const char* data;
+  int length;
+  if (!pickle.ReadData(pickle_iter, &data, &length))
+    return NULL;
+
+  return CreateOSCertHandleFromBytes(data, length);
+}
+
+// static
+bool X509Certificate::WriteCertHandleToPickle(OSCertHandle cert_handle,
+                                              Pickle* pickle) {
+  CSSM_DATA cert_data;
+  OSStatus status = SecCertificateGetData(cert_handle, &cert_data);
+  if (status)
+    return false;
+
+  return pickle->WriteData(reinterpret_cast<char*>(cert_data.Data),
+                           cert_data.Length);
+}
+
 }  // namespace net
diff --git a/net/base/x509_certificate_nss.cc b/net/base/x509_certificate_nss.cc
index 56035fa..14004e0 100644
--- a/net/base/x509_certificate_nss.cc
+++ b/net/base/x509_certificate_nss.cc
@@ -650,17 +650,6 @@ void X509Certificate::Initialize() {
 }
 
 // static
-X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
-                                                   void** pickle_iter) {
-  const char* data;
-  int length;
-  if (!pickle.ReadData(pickle_iter, &data, &length))
-    return NULL;
-
-  return CreateFromBytes(data, length);
-}
-
-// static
 X509Certificate* X509Certificate::CreateSelfSigned(
     crypto::RSAPrivateKey* key,
     const std::string& subject,
@@ -762,11 +751,6 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   return x509_cert;
 }
 
-void X509Certificate::Persist(Pickle* pickle) {
-  pickle->WriteData(reinterpret_cast<const char*>(cert_handle_->derCert.data),
-                    cert_handle_->derCert.len);
-}
-
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   dns_names->clear();
 
@@ -1005,4 +989,24 @@ SHA1Fingerprint X509Certificate::CalculateFingerprint(
   return sha1;
 }
 
+// static
+X509Certificate::OSCertHandle
+X509Certificate::ReadCertHandleFromPickle(const Pickle& pickle,
+                                          void** pickle_iter) {
+  const char* data;
+  int length;
+  if (!pickle.ReadData(pickle_iter, &data, &length))
+    return NULL;
+
+  return CreateOSCertHandleFromBytes(data, length);
+}
+
+// static
+bool X509Certificate::WriteCertHandleToPickle(OSCertHandle cert_handle,
+                                              Pickle* pickle) {
+  return pickle->WriteData(
+      reinterpret_cast<const char*>(cert_handle->derCert.data),
+      cert_handle->derCert.len);
+}
+
 }  // namespace net
diff --git a/net/base/x509_certificate_openssl.cc b/net/base/x509_certificate_openssl.cc
index 52d9d4e..10ce266 100644
--- a/net/base/x509_certificate_openssl.cc
+++ b/net/base/x509_certificate_openssl.cc
@@ -385,17 +385,6 @@ X509Certificate::OSCertHandles X509Certificate::CreateOSCertHandlesFromBytes(
 }
 
 // static
-X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
-                                                   void** pickle_iter) {
-  const char* data;
-  int length;
-  if (!pickle.ReadData(pickle_iter, &data, &length))
-    return NULL;
-
-  return CreateFromBytes(data, length);
-}
-
-// static
 X509Certificate* X509Certificate::CreateSelfSigned(
     crypto::RSAPrivateKey* key,
     const std::string& subject,
@@ -405,15 +394,6 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   return NULL;
 }
 
-void X509Certificate::Persist(Pickle* pickle) {
-  DERCache der_cache;
-  if (!GetDERAndCacheIfNeeded(cert_handle_, &der_cache))
-      return;
-
-  pickle->WriteData(reinterpret_cast<const char*>(der_cache.data),
-                    der_cache.data_length);
-}
-
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   dns_names->clear();
 
@@ -533,4 +513,28 @@ bool X509Certificate::IsSameOSCert(X509Certificate::OSCertHandle a,
       memcmp(der_cache_a.data, der_cache_b.data, der_cache_a.data_length) == 0;
 }
 
+// static
+X509Certificate::OSCertHandle
+X509Certificate::ReadCertHandleFromPickle(const Pickle& pickle,
+                                          void** pickle_iter) {
+  const char* data;
+  int length;
+  if (!pickle.ReadData(pickle_iter, &data, &length))
+    return NULL;
+
+  return CreateOSCertHandleFromBytes(data, length);
+}
+
+// static
+bool X509Certificate::WriteCertHandleToPickle(OSCertHandle cert_handle,
+                                              Pickle* pickle) {
+  DERCache der_cache;
+  if (!GetDERAndCacheIfNeeded(cert_handle, &der_cache))
+    return false;
+
+  return pickle->WriteData(
+      reinterpret_cast<const char*>(der_cache.data),
+      der_cache.data_length);
+}
+
 } // namespace net
diff --git a/net/base/x509_certificate_unittest.cc b/net/base/x509_certificate_unittest.cc
index f49e035..89ea5825 100644
--- a/net/base/x509_certificate_unittest.cc
+++ b/net/base/x509_certificate_unittest.cc
@@ -670,17 +670,40 @@ TEST(X509CertificateTest, Cache) {
 }
 
 TEST(X509CertificateTest, Pickle) {
-  scoped_refptr<X509Certificate> cert1(X509Certificate::CreateFromBytes(
-      reinterpret_cast<const char*>(google_der), sizeof(google_der)));
+  X509Certificate::OSCertHandle google_cert_handle =
+      X509Certificate::CreateOSCertHandleFromBytes(
+          reinterpret_cast<const char*>(google_der), sizeof(google_der));
+  X509Certificate::OSCertHandle thawte_cert_handle =
+      X509Certificate::CreateOSCertHandleFromBytes(
+          reinterpret_cast<const char*>(thawte_der), sizeof(thawte_der));
+
+  X509Certificate::OSCertHandles intermediates;
+  intermediates.push_back(thawte_cert_handle);
+  // Faking SOURCE_LONE_CERT_IMPORT so that when the pickled certificate is
+  // read, it successfully evicts |cert| from the X509Certificate::Cache.
+  // This will be fixed when http://crbug.com/49377 is fixed.
+  scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromHandle(
+      google_cert_handle,
+      X509Certificate::SOURCE_LONE_CERT_IMPORT,
+      intermediates);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), cert.get());
+
+  X509Certificate::FreeOSCertHandle(google_cert_handle);
+  X509Certificate::FreeOSCertHandle(thawte_cert_handle);
 
   Pickle pickle;
-  cert1->Persist(&pickle);
+  cert->Persist(&pickle);
 
   void* iter = NULL;
-  scoped_refptr<X509Certificate> cert2(
-      X509Certificate::CreateFromPickle(pickle, &iter));
-
-  EXPECT_EQ(cert1, cert2);
+  scoped_refptr<X509Certificate> cert_from_pickle =
+      X509Certificate::CreateFromPickle(
+          pickle, &iter, X509Certificate::PICKLETYPE_CERTIFICATE_CHAIN);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), cert_from_pickle);
+  EXPECT_NE(cert.get(), cert_from_pickle.get());
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(
+      cert->os_cert_handle(), cert_from_pickle->os_cert_handle()));
+  EXPECT_TRUE(cert->HasIntermediateCertificates(
+      cert_from_pickle->GetIntermediateCertificates()));
 }
 
 TEST(X509CertificateTest, Policy) {
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index a1a3eae..fd5076d 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -550,29 +550,6 @@ bool X509Certificate::IsIssuedByKnownRoot(PCCERT_CHAIN_CONTEXT chain_context) {
 }
 
 // static
-X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
-                                                   void** pickle_iter) {
-  const char* data;
-  int length;
-  if (!pickle.ReadData(pickle_iter, &data, &length))
-    return NULL;
-
-  OSCertHandle cert_handle = NULL;
-  if (!CertAddSerializedElementToStore(
-      NULL,  // the cert won't be persisted in any cert store
-      reinterpret_cast<const BYTE*>(data), length,
-      CERT_STORE_ADD_USE_EXISTING, 0, CERT_STORE_CERTIFICATE_CONTEXT_FLAG,
-      NULL, reinterpret_cast<const void **>(&cert_handle)))
-    return NULL;
-
-  X509Certificate* cert = CreateFromHandle(cert_handle,
-                                           SOURCE_LONE_CERT_IMPORT,
-                                           OSCertHandles());
-  FreeOSCertHandle(cert_handle);
-  return cert;
-}
-
-// static
 X509Certificate* X509Certificate::CreateSelfSigned(
     crypto::RSAPrivateKey* key,
     const std::string& subject,
@@ -635,23 +612,6 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   return cert;
 }
 
-void X509Certificate::Persist(Pickle* pickle) {
-  DCHECK(cert_handle_);
-  DWORD length;
-  if (!CertSerializeCertificateStoreElement(cert_handle_, 0,
-      NULL, &length)) {
-    NOTREACHED();
-    return;
-  }
-  BYTE* data = reinterpret_cast<BYTE*>(pickle->BeginWriteData(length));
-  if (!CertSerializeCertificateStoreElement(cert_handle_, 0,
-      data, &length)) {
-    NOTREACHED();
-    length = 0;
-  }
-  pickle->TrimWriteData(length);
-}
-
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   dns_names->clear();
   if (cert_handle_) {
@@ -1044,4 +1004,45 @@ SHA1Fingerprint X509Certificate::CalculateFingerprint(
   return sha1;
 }
 
+// static
+X509Certificate::OSCertHandle
+X509Certificate::ReadCertHandleFromPickle(const Pickle& pickle,
+                                          void** pickle_iter) {
+  const char* data;
+  int length;
+  if (!pickle.ReadData(pickle_iter, &data, &length))
+    return NULL;
+
+  OSCertHandle cert_handle = NULL;
+  if (!CertAddSerializedElementToStore(
+          NULL,  // the cert won't be persisted in any cert store
+          reinterpret_cast<const BYTE*>(data), length,
+          CERT_STORE_ADD_USE_EXISTING, 0, CERT_STORE_CERTIFICATE_CONTEXT_FLAG,
+          NULL, reinterpret_cast<const void **>(&cert_handle))) {
+    return NULL;
+  }
+
+  return cert_handle;
+}
+
+// static
+bool X509Certificate::WriteCertHandleToPickle(OSCertHandle cert_handle,
+                                              Pickle* pickle) {
+  DWORD length = 0;
+  if (!CertSerializeCertificateStoreElement(cert_handle, 0, NULL, &length))
+    return false;
+
+  std::vector<BYTE> buffer(length);
+  // Serialize |cert_handle| in a way that will preserve any extended
+  // attributes set on the handle, such as the location to the certificate's
+  // private key.
+  if (!CertSerializeCertificateStoreElement(cert_handle, 0, &buffer[0],
+                                            &length)) {
+    return false;
+  }
+
+  return pickle->WriteData(reinterpret_cast<const char*>(&buffer[0]),
+                           length);
+}
+
 }  // namespace net
diff --git a/net/http/http_response_info.cc b/net/http/http_response_info.cc
index fd76462..49090e0 100644
--- a/net/http/http_response_info.cc
+++ b/net/http/http_response_info.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -22,7 +22,10 @@ namespace net {
 // serialized HttpResponseInfo.
 enum {
   // The version of the response info used when persisting response info.
-  RESPONSE_INFO_VERSION = 1,
+  RESPONSE_INFO_VERSION = 2,
+
+  // The minimum version supported for deserializing response info.
+  RESPONSE_INFO_MINIMUM_VERSION = 1,
 
   // We reserve up to 8 bits for the version number.
   RESPONSE_INFO_VERSION_MASK = 0xFF,
@@ -108,7 +111,8 @@ bool HttpResponseInfo::InitFromPickle(const Pickle& pickle,
   if (!pickle.ReadInt(&iter, &flags))
     return false;
   int version = flags & RESPONSE_INFO_VERSION_MASK;
-  if (version != RESPONSE_INFO_VERSION) {
+  if (version < RESPONSE_INFO_MINIMUM_VERSION ||
+      version > RESPONSE_INFO_VERSION) {
     DLOG(ERROR) << "unexpected response info version: " << version;
     return false;
   }
@@ -131,8 +135,12 @@ bool HttpResponseInfo::InitFromPickle(const Pickle& pickle,
 
   // read ssl-info
   if (flags & RESPONSE_INFO_HAS_CERT) {
-    ssl_info.cert =
-        X509Certificate::CreateFromPickle(pickle, &iter);
+    // Version 1 only serialized only the end-entity certificate,
+    // while subsequent versions include the entire chain.
+    X509Certificate::PickleType type = (version == 1) ?
+        X509Certificate::PICKLETYPE_SINGLE_CERTIFICATE :
+        X509Certificate::PICKLETYPE_CERTIFICATE_CHAIN;
+    ssl_info.cert = X509Certificate::CreateFromPickle(pickle, &iter, type);
   }
   if (flags & RESPONSE_INFO_HAS_CERT_STATUS) {
     int cert_status;