net/base/x509_certificate_unittest.cc - Issue 2944008: Refactor X509Certificate caching to cache the OS handle, rather than the X509Certificate

Unified Diff: net/base/x509_certificate_unittest.cc

Issue 2944008: Refactor X509Certificate caching to cache the OS handle, rather than the X509Certificate (Closed)

Patch Set: Removed unnecessary bits Created 9 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: net/base/x509_certificate_unittest.cc

diff --git a/net/base/x509_certificate_unittest.cc b/net/base/x509_certificate_unittest.cc

index 702db5704c1e961e3d19ab9e85d047366c26729b..6151351273cf4e8971ca03d874c15bc7df4f9611 100644

--- a/net/base/x509_certificate_unittest.cc

+++ b/net/base/x509_certificate_unittest.cc

@@ -475,7 +475,6 @@ TEST(X509CertificateTest, IntermediateCARequireExplicitPolicy) {

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;

@@ -510,7 +509,6 @@ TEST(X509CertificateTest, DISABLED_GlobalSignR3EVTest) {

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

CertVerifyResult verify_result;

@@ -539,7 +537,6 @@ TEST(X509CertificateTest, TestKnownRoot) {

intermediates.push_back(intermediate_cert->os_cert_handle());

scoped_refptr<X509Certificate> cert_chain =

X509Certificate::CreateFromHandle(cert->os_cert_handle(),

- X509Certificate::SOURCE_FROM_NETWORK,

intermediates);

int flags = 0;

@@ -615,7 +612,6 @@ TEST(X509CertificateTest, PublicKeyHashes) {

intermediates.push_back(intermediate_cert->os_cert_handle());

scoped_refptr<X509Certificate> cert_chain =

X509Certificate::CreateFromHandle(cert->os_cert_handle(),

- X509Certificate::SOURCE_FROM_NETWORK,

intermediates);

int flags = 0;

@@ -663,7 +659,7 @@ TEST(X509CertificateTest, InvalidKeyUsage) {

#endif

}

-// 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.

wtc 2011/07/17 01:55:32 The second sentence of this paragraph should be re

@@ -672,57 +668,49 @@ TEST(X509CertificateTest, InvalidKeyUsage) {

// come from the network.

wtc 2011/07/17 01:55:32 Delete this paragraph.

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.

wtc 2011/07/17 01:55:32 Note: NSS should return the same handle here.

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()));

wtc 2011/07/17 01:55:32 Since cert1 and cert2 don't have intermediate CA c

- // 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()));

- X509Certificate::FreeOSCertHandle(google_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()));

+ google_cert_handle, intermediates));

X509Certificate::FreeOSCertHandle(google_cert_handle);

+ X509Certificate::FreeOSCertHandle(thawte_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) {

@@ -735,13 +723,8 @@ TEST(X509CertificateTest, Pickle) {

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

ASSERT_NE(static_cast<X509Certificate*>(NULL), cert.get());

X509Certificate::FreeOSCertHandle(google_cert_handle);

@@ -798,7 +781,6 @@ TEST(X509CertificateTest, Policy) {

EXPECT_TRUE(policy.HasDeniedCert());

}

-#if defined(OS_MACOSX) || defined(OS_WIN)

TEST(X509CertificateTest, IntermediateCertificates) {

scoped_refptr<X509Certificate> webkit_cert(

X509Certificate::CreateFromBytes(

@@ -819,8 +801,7 @@ TEST(X509CertificateTest, IntermediateCertificates) {

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()));

@@ -830,8 +811,7 @@ TEST(X509CertificateTest, IntermediateCertificates) {

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

wtc 2011/07/17 01:55:32 Delete this comment and check, just like you delet

@@ -844,20 +824,9 @@ TEST(X509CertificateTest, IntermediateCertificates) {

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

« net/base/x509_certificate_mac.cc ('K') | « net/base/x509_certificate_nss.cc ('k') | net/base/x509_certificate_win.cc » ('j') | net/socket/ssl_client_socket_nss.cc » ('J')