Return the constructed certificate chain in X509Certificate::Verify()

net/base/x509_certificate_unittest.cc

Index: net/base/x509_certificate_unittest.cc
diff --git a/net/base/x509_certificate_unittest.cc b/net/base/x509_certificate_unittest.cc
index 7adad4f51322a7c67fd6c54dc7f0699818a271d2..ddc8ecd3243ae309a8656760e6a5d33eaa29abab 100644
--- a/net/base/x509_certificate_unittest.cc
+++ b/net/base/x509_certificate_unittest.cc
@@ -820,6 +820,148 @@ TEST(X509CertificateTest, IntermediateCertificates) {
   X509Certificate::FreeOSCertHandle(google_handle);
 }
 
+// Basic test for returning the chain in CertVerifyResult. Note that the
+// returned chain may just be a reflection of the originally supplied chain;
+// that is, if any unrecoverable errors occur, the default chain returned is

[wtc, 2011/07/26 19:32:29]

It's not clear what "unrecoverable errors" means here.  I'd
like remove the word "unrecoverable".

+// an exact copy of the certificate to be verified. The remaining
+// VerifyReturn* tests are used to ensure that the actual, verified chain is
+// being returned by Verify().
+TEST(X509CertificateTest, VerifyReturnChainBasic) {
+  FilePath certs_dir = GetTestCertsDirectory();
+  CertificateList certs = CreateCertificateListFromFile(
+      certs_dir, "x509_verify_results.chain.pem",
+      X509Certificate::FORMAT_AUTO);
+  ASSERT_EQ(3U, certs.size());
+
+  X509Certificate::OSCertHandles intermediates;
+  intermediates.push_back(certs[1]->os_cert_handle());
+  intermediates.push_back(certs[2]->os_cert_handle());
+
+  TestRootCerts::GetInstance()->Add(certs[2]);
+
+  scoped_refptr<X509Certificate> google_full_chain =
+      X509Certificate::CreateFromHandle(certs[0]->os_cert_handle(),
+                                        intermediates);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), google_full_chain);
+  ASSERT_EQ(2U, google_full_chain->GetIntermediateCertificates().size());
+
+  CertVerifyResult verify_result;
+  EXPECT_EQ(static_cast<X509Certificate*>(NULL), verify_result.verified_cert);
+  int error = google_full_chain->Verify("127.0.0.1", 0, &verify_result);
+  EXPECT_EQ(OK, error);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), verify_result.verified_cert);
+
+  EXPECT_NE(google_full_chain, verify_result.verified_cert);
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(
+      google_full_chain->os_cert_handle(),
+      verify_result.verified_cert->os_cert_handle()));
+  const X509Certificate::OSCertHandles& return_intermediates =
+      verify_result.verified_cert->GetIntermediateCertificates();
+  ASSERT_EQ(2U, return_intermediates.size());
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(return_intermediates[0],
+                                            certs[1]->os_cert_handle()));
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(return_intermediates[1],
+                                            certs[2]->os_cert_handle()));
+
+  TestRootCerts::GetInstance()->Clear();
+}
+
+// Test that the certificate returned in CertVerifyResult is able to reorder
+// certificates that are not ordered from end-entity to root. While this is
+// a protocol violation if sent during a TLS handshake, if multiple sources
+// of intermediate certificates are combined, it's possible that order may
+// not be maintained.
+TEST(X509CertificateTest, VerifyReturnChainProperlyOrders) {

[wtc, 2011/07/26 19:32:29]

ProperlyOrders => ProperlyOrdered ?

+  FilePath certs_dir = GetTestCertsDirectory();
+  CertificateList certs = CreateCertificateListFromFile(
+      certs_dir, "x509_verify_results.chain.pem",
+      X509Certificate::FORMAT_AUTO);
+  ASSERT_EQ(3U, certs.size());
+
+  // Construct the chain out of order.
+  X509Certificate::OSCertHandles intermediates;
+  intermediates.push_back(certs[2]->os_cert_handle());
+  intermediates.push_back(certs[1]->os_cert_handle());
+
+  TestRootCerts::GetInstance()->Add(certs[2]);
+
+  scoped_refptr<X509Certificate> google_full_chain =
+      X509Certificate::CreateFromHandle(certs[0]->os_cert_handle(),
+                                        intermediates);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), google_full_chain);
+  ASSERT_EQ(2U, google_full_chain->GetIntermediateCertificates().size());
+
+  CertVerifyResult verify_result;
+  EXPECT_EQ(static_cast<X509Certificate*>(NULL), verify_result.verified_cert);
+  int error = google_full_chain->Verify("127.0.0.1", 0, &verify_result);
+  EXPECT_EQ(OK, error);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), verify_result.verified_cert);
+
+  EXPECT_NE(google_full_chain, verify_result.verified_cert);
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(
+      google_full_chain->os_cert_handle(),
+      verify_result.verified_cert->os_cert_handle()));
+  const X509Certificate::OSCertHandles& return_intermediates =
+      verify_result.verified_cert->GetIntermediateCertificates();
+  ASSERT_EQ(2U, return_intermediates.size());
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(return_intermediates[0],
+                                            certs[1]->os_cert_handle()));
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(return_intermediates[1],
+                                            certs[2]->os_cert_handle()));
+
+  TestRootCerts::GetInstance()->Clear();
+}
+
+// Test that Verify() filters out certificates which are not related to
+// or part of the certificate/chain being verified.

[wtc, 2011/07/26 19:32:29]

Nit: remove '/'?

+TEST(X509CertificateTest, VerifyReturnChainFiltersUnrelatedCerts) {
+  FilePath certs_dir = GetTestCertsDirectory();
+  CertificateList certs = CreateCertificateListFromFile(
+      certs_dir, "x509_verify_results.chain.pem",
+      X509Certificate::FORMAT_AUTO);
+  ASSERT_EQ(3U, certs.size());
+  TestRootCerts::GetInstance()->Add(certs[2]);
+
+  scoped_refptr<X509Certificate> unrelated_dod_certificate =
+      ImportCertFromFile(certs_dir, "dod_ca_17_cert.der");
+  scoped_refptr<X509Certificate> unrelated_dod_certificate2 =
+      ImportCertFromFile(certs_dir, "dod_root_ca_2_cert.der");
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), unrelated_dod_certificate);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), unrelated_dod_certificate2);
+
+  // Interject unrelated certificates into the list of intermediates.
+  X509Certificate::OSCertHandles intermediates;
+  intermediates.push_back(unrelated_dod_certificate->os_cert_handle());
+  intermediates.push_back(certs[1]->os_cert_handle());
+  intermediates.push_back(unrelated_dod_certificate2->os_cert_handle());
+  intermediates.push_back(certs[2]->os_cert_handle());
+
+  scoped_refptr<X509Certificate> google_full_chain =
+      X509Certificate::CreateFromHandle(certs[0]->os_cert_handle(),
+                                        intermediates);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), google_full_chain);
+  ASSERT_EQ(4U, google_full_chain->GetIntermediateCertificates().size());
+
+  CertVerifyResult verify_result;
+  EXPECT_EQ(static_cast<X509Certificate*>(NULL), verify_result.verified_cert);
+  int error = google_full_chain->Verify("127.0.0.1", 0, &verify_result);
+  EXPECT_EQ(OK, error);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), verify_result.verified_cert);
+
+  EXPECT_NE(google_full_chain, verify_result.verified_cert);
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(
+      google_full_chain->os_cert_handle(),
+      verify_result.verified_cert->os_cert_handle()));
+  const X509Certificate::OSCertHandles& return_intermediates =
+      verify_result.verified_cert->GetIntermediateCertificates();
+  ASSERT_EQ(2U, return_intermediates.size());
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(return_intermediates[0],
+                                            certs[1]->os_cert_handle()));
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(return_intermediates[1],
+                                            certs[2]->os_cert_handle()));
+  TestRootCerts::GetInstance()->Clear();
+}
+
 #if defined(OS_MACOSX)
 TEST(X509CertificateTest, IsIssuedBy) {
   FilePath certs_dir = GetTestCertsDirectory();