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

net/base/x509_certificate.cc

Index: net/base/x509_certificate.cc
diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc
index 91ed346d010d3d504dab36fbb9daea7c31c3f8a8..b371c977360233002b37a687ac05a77dc1e0ce46 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.

[wtc, 2011/04/20 23:07:58]

If a certificate fails to parse, the subsequent data in
|pickle| may be invalid and |pickle_iter| may already be
out of sync.

Are you sure we should continue reading?

[Ryan Sleevi, 2011/04/20 23:59:10]

There are two reasons for why parsing may fail, and my goal was to gracefully
handle one of them.
1) The |pickle_iter| is out of sync. We've already thrown it even more out of
sync by the failed attempt to read a single certificate, so this cannot be
gracefully recovered.
2) The |pickle_iter| is in sync, but the underlying cryptographic library is no
longer able to parse the certificate. This indicates a failure of the library,
but not of the cache, and can be gracefully recovered with respect to the
pickle.

By handling #2 gracefully, the same error handling semantics of M11 and earlier
were preserved - the certificate data is always pulled out of the pickle,
regardless of the state of the underlying cryptographic library. This, in turn,
minimized any impact this change might have on M12, and in HttpResponseInfo
deserialization in general.

+  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;

[wtc, 2011/04/20 23:07:58]

Nit: say
  bool ok = (cert_handle != NULL);

+    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;

[wtc, 2011/04/20 23:07:58]

We should also return NULL if |ok| is false.

[Ryan Sleevi, 2011/04/20 23:59:10]

This contradicts the behaviour documented in line 258-259 (which can be
changed). |intermediates| would also need to be freed before returning (which
can also be changed)

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