Refactor X509Certificate caching to cache the OS handle, rather than the X509Certificate

net/base/x509_certificate_unittest.cc

 // Copyright (c) 2010 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.
 
 #include "base/crypto/rsa_private_key.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/pickle.h"
 #include "net/base/cert_status_flags.h"
@@ skipping 410 matching lines @@
   ASSERT_NE(static_cast<X509Certificate*>(NULL), intermediate_cert);
 
   FilePath root_cert_path = certs_dir.AppendASCII("dod_root_ca_2_cert.der");
   TestRootCerts* root_certs = TestRootCerts::GetInstance();
   ASSERT_TRUE(root_certs->AddFromFile(root_cert_path));
 
   X509Certificate::OSCertHandles intermediates;
   intermediates.push_back(intermediate_cert->os_cert_handle());
   scoped_refptr<X509Certificate> cert_chain =
       X509Certificate::CreateFromHandle(server_cert->os_cert_handle(),
-                                        X509Certificate::SOURCE_FROM_NETWORK,
                                         intermediates);
 
   int flags = 0;
   CertVerifyResult verify_result;
   int error = cert_chain->Verify("www.us.army.mil", flags, &verify_result);
   EXPECT_EQ(OK, error);
   EXPECT_EQ(0, verify_result.cert_status);
   root_certs->Clear();
 }
 
-// Tests X509Certificate::Cache via X509Certificate::CreateFromHandle.  We
+// Tests X509CertificateCache via X509Certificate::CreateFromHandle.  We
 // call X509Certificate::CreateFromHandle several times and observe whether
 // it returns a cached or new X509Certificate object.
 //
 // All the OS certificate handles in this test are actually from the same
 // source (the bytes of a lone certificate), but we pretend that some of them
 // come from the network.
 TEST(X509CertificateTest, Cache) {
   X509Certificate::OSCertHandle google_cert_handle;
+  X509Certificate::OSCertHandle thawte_cert_handle;
 
-  // Add a certificate from the source SOURCE_LONE_CERT_IMPORT to our
-  // certificate cache.
+  // Add a single certificate to the certificate cache.
   google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
       reinterpret_cast<const char*>(google_der), sizeof(google_der));
   scoped_refptr<X509Certificate> cert1(X509Certificate::CreateFromHandle(
-      google_cert_handle, X509Certificate::SOURCE_LONE_CERT_IMPORT,
-      X509Certificate::OSCertHandles()));
+      google_cert_handle, X509Certificate::OSCertHandles()));
   X509Certificate::FreeOSCertHandle(google_cert_handle);
 
-  // Add a certificate from the same source (SOURCE_LONE_CERT_IMPORT).  This
-  // should return the cached certificate (cert1).
+  // Add the same certificate, but as a new handle.
   google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
       reinterpret_cast<const char*>(google_der), sizeof(google_der));
   scoped_refptr<X509Certificate> cert2(X509Certificate::CreateFromHandle(
-      google_cert_handle, X509Certificate::SOURCE_LONE_CERT_IMPORT,
-      X509Certificate::OSCertHandles()));
+      google_cert_handle, X509Certificate::OSCertHandles()));
   X509Certificate::FreeOSCertHandle(google_cert_handle);
 
-  EXPECT_EQ(cert1, cert2);
+  // A new X509Certificate should be returned.
+  EXPECT_NE(cert1.get(), cert2.get());
+  // But both instances should share the underlying OS certificate handle.
+  EXPECT_EQ(cert1->os_cert_handle(), cert2->os_cert_handle());
+  EXPECT_TRUE(cert1->HasIntermediateCertificates(
+      cert2->GetIntermediateCertificates()));
 
-  // Add a certificate from the network.  This should kick out the original
-  // cached certificate (cert1) and return a new certificate.
+  // Add the same certificate, but this time with an intermediate. This
+  // should result in the intermediate being cached. Note that this is not
+  // a legitimate chain, but is suitable for testing.
   google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
       reinterpret_cast<const char*>(google_der), sizeof(google_der));
+  thawte_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
+      reinterpret_cast<const char*>(thawte_der), sizeof(thawte_der));
+  X509Certificate::OSCertHandles intermediates;
+  intermediates.push_back(thawte_cert_handle);
   scoped_refptr<X509Certificate> cert3(X509Certificate::CreateFromHandle(
-      google_cert_handle, X509Certificate::SOURCE_FROM_NETWORK,
-      X509Certificate::OSCertHandles()));
+      google_cert_handle, intermediates));
   X509Certificate::FreeOSCertHandle(google_cert_handle);
+  X509Certificate::FreeOSCertHandle(thawte_cert_handle);
 
-  EXPECT_NE(cert1, cert3);
-
-  // Add one certificate from each source.  Both should return the new cached
-  // certificate (cert3).
-  google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
-      reinterpret_cast<const char*>(google_der), sizeof(google_der));
-  scoped_refptr<X509Certificate> cert4(X509Certificate::CreateFromHandle(
-      google_cert_handle, X509Certificate::SOURCE_FROM_NETWORK,
-      X509Certificate::OSCertHandles()));
-  X509Certificate::FreeOSCertHandle(google_cert_handle);
-
-  EXPECT_EQ(cert3, cert4);
-
-  google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
-      reinterpret_cast<const char*>(google_der), sizeof(google_der));
-  scoped_refptr<X509Certificate> cert5(X509Certificate::CreateFromHandle(
-      google_cert_handle, X509Certificate::SOURCE_FROM_NETWORK,
-      X509Certificate::OSCertHandles()));
-  X509Certificate::FreeOSCertHandle(google_cert_handle);
-
-  EXPECT_EQ(cert3, cert5);
+  // Test that the new certificate, even with intermediates, results in the
+  // same underlying handle being used.
+  EXPECT_EQ(cert1->os_cert_handle(), cert3->os_cert_handle());
+  // Though they use the same OS handle, the intermediates should be different.
+  EXPECT_FALSE(cert1->HasIntermediateCertificates(
+      cert3->GetIntermediateCertificates()));
 }
 
 TEST(X509CertificateTest, Pickle) {
  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);
+      google_cert_handle, intermediates);
 
   X509Certificate::FreeOSCertHandle(google_cert_handle);
   X509Certificate::FreeOSCertHandle(thawte_cert_handle);
 
   Pickle pickle;
   cert->Persist(&pickle);
 
   void* iter = NULL;
   scoped_refptr<X509Certificate> cert_from_pickle =
       X509Certificate::CreateFromPickle(
@@ skipping 35 matching lines @@
   EXPECT_TRUE(policy.HasDeniedCert());
 
   policy.Allow(webkit_cert.get());
 
   EXPECT_EQ(policy.Check(google_cert.get()), CertPolicy::DENIED);
   EXPECT_EQ(policy.Check(webkit_cert.get()), CertPolicy::ALLOWED);
   EXPECT_TRUE(policy.HasAllowedCert());
   EXPECT_TRUE(policy.HasDeniedCert());
 }
 
-#if defined(OS_MACOSX) || defined(OS_WIN)
 TEST(X509CertificateTest, IntermediateCertificates) {
   scoped_refptr<X509Certificate> webkit_cert(
       X509Certificate::CreateFromBytes(
           reinterpret_cast<const char*>(webkit_der), sizeof(webkit_der)));
 
   scoped_refptr<X509Certificate> thawte_cert(
       X509Certificate::CreateFromBytes(
           reinterpret_cast<const char*>(thawte_der), sizeof(thawte_der)));
 
   scoped_refptr<X509Certificate> paypal_cert(
       X509Certificate::CreateFromBytes(
           reinterpret_cast<const char*>(paypal_null_der),
           sizeof(paypal_null_der)));
 
   X509Certificate::OSCertHandle google_handle;
   // Create object with no intermediates:
   google_handle = X509Certificate::CreateOSCertHandleFromBytes(
       reinterpret_cast<const char*>(google_der), sizeof(google_der));
   X509Certificate::OSCertHandles intermediates1;
   scoped_refptr<X509Certificate> cert1;
-  cert1 = X509Certificate::CreateFromHandle(
-      google_handle, X509Certificate::SOURCE_FROM_NETWORK, intermediates1);
+  cert1 = X509Certificate::CreateFromHandle(google_handle, intermediates1);
   EXPECT_TRUE(cert1->HasIntermediateCertificates(intermediates1));
   EXPECT_FALSE(cert1->HasIntermediateCertificate(
       webkit_cert->os_cert_handle()));
 
   // Create object with 2 intermediates:
   X509Certificate::OSCertHandles intermediates2;
   intermediates2.push_back(webkit_cert->os_cert_handle());
   intermediates2.push_back(thawte_cert->os_cert_handle());
   scoped_refptr<X509Certificate> cert2;
-  cert2 = X509Certificate::CreateFromHandle(
-      google_handle, X509Certificate::SOURCE_FROM_NETWORK, intermediates2);
+  cert2 = X509Certificate::CreateFromHandle(google_handle, intermediates2);
 
   // The cache should have stored cert2 'cause it has more intermediates:
   EXPECT_NE(cert1, cert2);
 
   // Verify it has all the intermediates:
   EXPECT_TRUE(cert2->HasIntermediateCertificate(
       webkit_cert->os_cert_handle()));
   EXPECT_TRUE(cert2->HasIntermediateCertificate(
       thawte_cert->os_cert_handle()));
   EXPECT_FALSE(cert2->HasIntermediateCertificate(
       paypal_cert->os_cert_handle()));
 
-  // Create object with 1 intermediate:
-  X509Certificate::OSCertHandles intermediates3;
-  intermediates2.push_back(thawte_cert->os_cert_handle());
-  scoped_refptr<X509Certificate> cert3;
-  cert3 = X509Certificate::CreateFromHandle(
-      google_handle, X509Certificate::SOURCE_FROM_NETWORK, intermediates3);
-
-  // The cache should have returned cert2 'cause it has more intermediates:
-  EXPECT_EQ(cert3, cert2);
-
   // Cleanup
   X509Certificate::FreeOSCertHandle(google_handle);
 }
-#endif
 
 #if defined(OS_MACOSX)
 TEST(X509CertificateTest, IsIssuedBy) {
   FilePath certs_dir = GetTestCertsDirectory();
 
   // Test a client certificate from MIT.
   scoped_refptr<X509Certificate> mit_davidben_cert(
       ImportCertFromFile(certs_dir, "mit.davidben.der"));
   ASSERT_NE(static_cast<X509Certificate*>(NULL), mit_davidben_cert);
 
@@ skipping 104 matching lines @@
 
     for (size_t j = 0; j < 20; ++j)
       EXPECT_EQ(expected_fingerprint[j], actual_fingerprint.data[j]);
   }
 }
 
 INSTANTIATE_TEST_CASE_P(, X509CertificateParseTest,
                         testing::ValuesIn(FormatTestData));
 
 }  // namespace net