CertVerifyProcMac: Add Keychain re-ordering hack, check CRLsets in path pruning loop.

net/cert/cert_verify_proc_unittest.cc

 // Copyright (c) 2012 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 "net/cert/cert_verify_proc.h"
 
 #include <vector>
 
 #include "base/callback_helpers.h"
 #include "base/files/file_path.h"
@@ skipping 15 matching lines @@
 #include "net/cert/test_root_certs.h"
 #include "net/cert/x509_certificate.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/test_certificate_data.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(OS_ANDROID)
 #include "base/android/build_info.h"
 #endif
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+#include "net/cert/test_keychain_search_list_mac.h"
+#endif
+
 using base::HexEncode;
 
 namespace net {
 
 namespace {
 
 // A certificate for www.paypal.com with a NULL byte in the common name.
 // From http://www.gossamer-threads.com/lists/fulldisc/full-disclosure/70363
 unsigned char paypal_null_fingerprint[] = {
   0x4c, 0x88, 0x9e, 0x28, 0xd7, 0x7a, 0x44, 0x1e, 0x13, 0xf2, 0x6a, 0xba,
@@ skipping 92 matching lines @@
   bool SupportsAdditionalTrustAnchors() {
     return verify_proc_->SupportsAdditionalTrustAnchors();
   }
 
   // Returns true if the underlying CertVerifyProc supports integrating CRLSets
   // into path building logic, such as allowing the selection of alternatively
   // valid paths when one or more are revoked. As the goal is to integrate this
   // into all platforms, this is a temporary, test-only flag to centralize the
   // conditionals in tests.
   bool SupportsCRLSetsInPathBuilding() {
 #if defined(OS_WIN) || defined(USE_NSS_CERTS)

[Ryan Sleevi, 2016/06/30 01:49:16]

Can this be set for OS_MACOSX now? If not, should we update the comment
explaining why? Is there a way to fold in the new code to the existing
unittests?

I'm trying to think how we can make the tests cross-platform, since we totally
desire the results to be cross-platform.

[mattm, 2016/06/30 03:20:52]

It still fails, since adding CRLSet checking to the chopping loop only helps in
cases where chopping things off the list of intermediates allows it to build a
better path. 
It could work if the test was changed to only give the bad path and see if it
can fetch the correct path via AIA from a test server, though that's more work
to set up. I could look into that if you think it's worthwhile.

     return true;
 #else
     return false;
 #endif
   }
 
   int Verify(X509Certificate* cert,
              const std::string& hostname,
              int flags,
              CRLSet* crl_set,
@@ skipping 1174 matching lines @@
     EXPECT_TRUE(testcase.expected_intermediate->Equals(intermediate.get()))
         << "Expected: " << testcase.expected_intermediate->subject().common_name
         << " issued by " << testcase.expected_intermediate->issuer().common_name
         << "; Got: " << intermediate->subject().common_name << " issued by "
         << intermediate->issuer().common_name;
   }
 }
 
 #endif
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+// Test that a CRLSet blocking one of the intermediates supplied by the server
+// can be worked around by the chopping workaround for path building. (Once the
+// supplied chain is chopped back to just the target, a better path can be
+// found out-of-band. Normally that would be by AIA fetching, for the purposes
+// of this test the better path is supplied by a test keychain.)
+//
+// In this test, there are two possible paths to validate a leaf (A):
+// 1. A(B) -> B(C) -> C(E) -> E(E)
+// 2. A(B) -> B(F) -> F(E) -> E(E)
+//
+// A(B) -> B(C) -> C(E) is supplied to the verifier.
+// B(F) and F(E) are supplied in a test keychain.
+// C is blocked by a CRLset.
+//
+// The verifier should rollback until it just tries A(B) alone, at which point
+// it will pull B(F) & F(E) from the keychain and succeed.
+TEST_F(CertVerifyProcTest, MacCRLIntermediate) {
+  const char* const kPath2Files[] = {
+      "multi-root-A-by-B.pem", "multi-root-B-by-C.pem", "multi-root-C-by-E.pem",
+      "multi-root-E-by-E.pem"};
+  CertificateList path_2_certs;
+  ASSERT_NO_FATAL_FAILURE(LoadCertificateFiles(kPath2Files, &path_2_certs));
+
+  const char* const kPath3Files[] = {
+      "multi-root-A-by-B.pem", "multi-root-B-by-F.pem", "multi-root-F-by-E.pem",
+      "multi-root-E-by-E.pem"};
+
+  CertificateList path_3_certs;
+  ASSERT_NO_FATAL_FAILURE(LoadCertificateFiles(kPath3Files, &path_3_certs));
+
+  // Add E as trust anchor.
+  ScopedTestRoot test_root_E(path_3_certs[3].get());  // E-by-E
+
+  X509Certificate::OSCertHandles intermediates;
+  intermediates.push_back(path_2_certs[1]->os_cert_handle());  // B-by-C
+  intermediates.push_back(path_2_certs[2]->os_cert_handle());  // C-by-E
+  scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromHandle(
+      path_3_certs[0]->os_cert_handle(), intermediates);
+  ASSERT_TRUE(cert);
+
+  std::unique_ptr<TestKeychainSearchList> test_keychain_search_list(
+      TestKeychainSearchList::Create());
+  ASSERT_TRUE(test_keychain_search_list);
+
+  base::FilePath keychain_path(
+      GetTestCertsDirectory().AppendASCII("multi-root-BFE.keychain"));
+  // SecKeychainOpen does not fail if the file doesn't exist, so assert it here
+  // for easier debugging.
+  ASSERT_TRUE(base::PathExists(keychain_path));
+  SecKeychainRef keychain;
+  OSStatus status =
+      SecKeychainOpen(keychain_path.MaybeAsASCII().c_str(), &keychain);
+  ASSERT_EQ(errSecSuccess, status);
+  ASSERT_TRUE(keychain);
+  base::ScopedCFTypeRef<SecKeychainRef> scoped_keychain(keychain);
+  test_keychain_search_list->AddKeychain(keychain);
+
+  scoped_refptr<CRLSet> crl_set;
+  std::string crl_set_bytes;
+  // CRL which blocks C by SPKI.
+  EXPECT_TRUE(base::ReadFileToString(
+      GetTestCertsDirectory().AppendASCII("multi-root-crlset-C.raw"),
+      &crl_set_bytes));
+  ASSERT_TRUE(CRLSetStorage::Parse(crl_set_bytes, &crl_set));
+
+  int flags = 0;
+  CertVerifyResult verify_result;
+  int error = Verify(cert.get(), "127.0.0.1", flags, crl_set.get(),
+                     empty_cert_list_, &verify_result);
+
+  ASSERT_EQ(OK, error);
+  ASSERT_EQ(0U, verify_result.cert_status);
+  ASSERT_TRUE(verify_result.verified_cert.get());
+
+  const X509Certificate::OSCertHandles& verified_intermediates =
+      verify_result.verified_cert->GetIntermediateCertificates();
+  ASSERT_EQ(3U, verified_intermediates.size());
+
+  scoped_refptr<X509Certificate> intermediate =
+      X509Certificate::CreateFromHandle(verified_intermediates[1],
+                                        X509Certificate::OSCertHandles());
+  ASSERT_TRUE(intermediate);
+
+  scoped_refptr<X509Certificate> expected_intermediate = path_3_certs[2];
+  EXPECT_TRUE(expected_intermediate->Equals(intermediate.get()))
+      << "Expected: " << expected_intermediate->subject().common_name
+      << " issued by " << expected_intermediate->issuer().common_name
+      << "; Got: " << intermediate->subject().common_name << " issued by "
+      << intermediate->issuer().common_name;
+}
+
+// Test that if a keychain is present which trusts a less-desirable root (ex,
+// one using SHA1), that the keychain reordering hack will cause the better
+// root in the System Roots to be used instead.
+TEST_F(CertVerifyProcTest, MacKeychainReordering) {
+  // Note: target cert expires Apr  2 23:59:59 2018 GMT
+  scoped_refptr<X509Certificate> cert = CreateCertificateChainFromFile(
+      GetTestCertsDirectory(), "tripadvisor-verisign-chain.pem",
+      X509Certificate::FORMAT_AUTO);
+  ASSERT_TRUE(cert);
+
+  // Create a test keychain search list that will Always Trust the SHA1
+  // cross-signed VeriSign Class 3 Public Primary Certification Authority - G5
+  std::unique_ptr<TestKeychainSearchList> test_keychain_search_list(
+      TestKeychainSearchList::Create());
+  ASSERT_TRUE(test_keychain_search_list);
+
+  base::FilePath keychain_path(GetTestCertsDirectory().AppendASCII(
+      "verisign_class3_g5_crosssigned-trusted.keychain"));
+  // SecKeychainOpen does not fail if the file doesn't exist, so assert it here
+  // for easier debugging.
+  ASSERT_TRUE(base::PathExists(keychain_path));
+  SecKeychainRef keychain;
+  OSStatus status =
+      SecKeychainOpen(keychain_path.MaybeAsASCII().c_str(), &keychain);
+  ASSERT_EQ(errSecSuccess, status);
+  ASSERT_TRUE(keychain);
+  base::ScopedCFTypeRef<SecKeychainRef> scoped_keychain(keychain);
+  test_keychain_search_list->AddKeychain(keychain);
+
+  int flags = 0;
+  CertVerifyResult verify_result;
+  int error = Verify(cert.get(), "www.tripadvisor.com", flags,
+                     nullptr /* crl_set */, empty_cert_list_, &verify_result);
+
+  ASSERT_EQ(OK, error);
+  EXPECT_EQ(0U, verify_result.cert_status);
+  EXPECT_FALSE(verify_result.has_sha1);
+  ASSERT_TRUE(verify_result.verified_cert.get());
+
+  const X509Certificate::OSCertHandles& verified_intermediates =
+      verify_result.verified_cert->GetIntermediateCertificates();
+  ASSERT_EQ(2U, verified_intermediates.size());
+}
+#endif
+
 enum ExpectedAlgorithms {
   EXPECT_MD2 = 1 << 0,
   EXPECT_MD4 = 1 << 1,
   EXPECT_MD5 = 1 << 2,
   EXPECT_SHA1 = 1 << 3,
   EXPECT_SHA1_LEAF = 1 << 4,
 };
 
 struct WeakDigestTestData {
   const char* root_cert_filename;
@@ skipping 330 matching lines @@
   int flags = 0;
   CertVerifyResult verify_result;
   int error = Verify(cert.get(), "127.0.0.1", flags, NULL, empty_cert_list_,
                      &verify_result);
   EXPECT_EQ(ERR_CERT_INVALID, error);
   EXPECT_EQ(CERT_STATUS_INVALID, verify_result.cert_status);
 }
 #endif  // defined(OS_MACOSX) && !defined(OS_IOS)
 
 }  // namespace net