Distrust publicly trusted SHA-1 certs

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"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/sha1.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "crypto/sha2.h"
 #include "net/base/net_errors.h"
 #include "net/cert/asn1_util.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/cert/cert_verifier.h"
 #include "net/cert/cert_verify_result.h"
 #include "net/cert/crl_set.h"
 #include "net/cert/crl_set_storage.h"
 #include "net/cert/test_root_certs.h"
 #include "net/cert/x509_certificate.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/gtest_util.h"
 #include "net/test/test_certificate_data.h"
 #include "net/test/test_data_directory.h"
 #include "testing/gmock/include/gmock/gmock.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
 
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif
+
 using net::test::IsError;
 using net::test::IsOk;
 
 using base::HexEncode;
 
 namespace net {
 
 namespace {
 
 const char kTLSFeatureExtensionHistogram[] =
@@ skipping 811 matching lines @@
   // However, if the CA is not well known, these should not be flagged:
   dummy_result.Reset();
   dummy_result.is_issued_by_known_root = false;
   verify_proc_ = new MockCertVerifyProc(dummy_result);
   error =
       Verify(cert.get(), "intranet", 0, NULL, empty_cert_list_, &verify_result);
   EXPECT_THAT(error, IsOk());
   EXPECT_FALSE(verify_result.cert_status & CERT_STATUS_NON_UNIQUE_NAME);
 }
 
-// Test that a SHA-1 certificate from a publicly trusted CA issued after
-// 1 January 2016 is rejected, but those issued before that date, or with
-// SHA-1 in the intermediate, is not rejected.
-TEST_F(CertVerifyProcTest, VerifyRejectsSHA1AfterDeprecation) {
+// While all SHA-1 certificates should be rejected, in the event that there
+// emerges some unexpected bug, test that the 'legacy' behaviour works
+// correctly - rejecting all SHA-1 certificates from publicly trusted CAs
+// that were issued after 1 January 2016, while still allowing those from
+// before that date, with SHA-1 in the intermediate, or from an enterprise
+// CA.
+//
+// TODO(rsleevi): This code should be removed in M57.
+TEST_F(CertVerifyProcTest, VerifyRejectsSHA1AfterDeprecationLegacyMode) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(CertVerifyProc::kSHA1LegacyMode);
+
   CertVerifyResult dummy_result;
   CertVerifyResult verify_result;
   int error = 0;
   scoped_refptr<X509Certificate> cert;
 
   // Publicly trusted SHA-1 leaf certificates issued before 1 January 2016
   // are accepted.
   verify_result.Reset();
   dummy_result.Reset();
   dummy_result.is_issued_by_known_root = true;
@@ skipping 617 matching lines @@
       "/System/Library/Keychains/SystemRootCertificates.keychain";
   ASSERT_TRUE(base::PathExists(base::FilePath(root_keychain_path)));
 
   SecKeychainRef keychain;
   OSStatus status = SecKeychainOpen(root_keychain_path, &keychain);
   ASSERT_EQ(errSecSuccess, status);
   CFRelease(keychain);
 }
 #endif
 
+bool AreSHA1IntermediatesAllowed() {
+#if defined(OS_WIN)
+  // TODO(rsleevi): Remove this once https://crbug.com/588789 is resolved
+  // for Windows 7/2008 users.
+  // Note: This must be kept in sync with cert_verify_proc.cc
+  return base::win::GetVersion() < base::win::VERSION_WIN8;

[Ryan Sleevi, 2016/11/09 01:02:14]

This is why it made it past bots

[davidben, 2016/11/09 01:04:28]

Oops. I should have been paying more attention. :-(

+#else
+  return false;
+#endif
+}
+
+TEST_F(CertVerifyProcTest, RejectsMD2) {
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), "ok_cert.pem"));
+  ASSERT_TRUE(cert);
+
+  CertVerifyResult result;
+  result.has_md2 = true;
+  verify_proc_ = new MockCertVerifyProc(result);
+
+  int flags = 0;
+  CertVerifyResult verify_result;
+  int error = Verify(cert.get(), "127.0.0.1", flags, nullptr /* crl_set */,
+                     empty_cert_list_, &verify_result);
+  EXPECT_THAT(error, IsError(ERR_CERT_INVALID));
+  EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_INVALID);
+}
+
+TEST_F(CertVerifyProcTest, RejectsMD4) {
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), "ok_cert.pem"));
+  ASSERT_TRUE(cert);
+
+  CertVerifyResult result;
+  result.has_md4 = true;
+  verify_proc_ = new MockCertVerifyProc(result);
+
+  int flags = 0;
+  CertVerifyResult verify_result;
+  int error = Verify(cert.get(), "127.0.0.1", flags, nullptr /* crl_set */,
+                     empty_cert_list_, &verify_result);
+  EXPECT_THAT(error, IsError(ERR_CERT_INVALID));
+  EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_INVALID);
+}
+
+TEST_F(CertVerifyProcTest, RejectsMD5) {
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), "ok_cert.pem"));
+  ASSERT_TRUE(cert);
+
+  CertVerifyResult result;
+  result.has_md5 = true;
+  verify_proc_ = new MockCertVerifyProc(result);
+
+  int flags = 0;
+  CertVerifyResult verify_result;
+  int error = Verify(cert.get(), "127.0.0.1", flags, nullptr /* crl_set */,
+                     empty_cert_list_, &verify_result);
+  EXPECT_THAT(error, IsError(ERR_CERT_WEAK_SIGNATURE_ALGORITHM));
+  EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_WEAK_SIGNATURE_ALGORITHM);
+}
+
+TEST_F(CertVerifyProcTest, RejectsPublicSHA1Leaves) {
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), "ok_cert.pem"));
+  ASSERT_TRUE(cert);
+
+  CertVerifyResult result;
+  result.has_sha1 = true;
+  result.has_sha1_leaf = true;
+  result.is_issued_by_known_root = true;
+  verify_proc_ = new MockCertVerifyProc(result);
+
+  int flags = 0;
+  CertVerifyResult verify_result;
+  int error = Verify(cert.get(), "127.0.0.1", flags, nullptr /* crl_set */,
+                     empty_cert_list_, &verify_result);
+  EXPECT_THAT(error, IsError(ERR_CERT_WEAK_SIGNATURE_ALGORITHM));
+  EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_WEAK_SIGNATURE_ALGORITHM);
+}
+
+TEST_F(CertVerifyProcTest, RejectsPublicSHA1IntermediatesUnlessAllowed) {
+  scoped_refptr<X509Certificate> cert(ImportCertFromFile(
+      GetTestCertsDirectory(), "39_months_after_2015_04.pem"));

[Ryan Sleevi, 2016/11/09 01:02:14]

I only changed this test, because the hope is for this test to be removed. It
does mean that the other tests, w/r/t using ok_cert.pem and setting
is_issued_by_known_root, are assuming that algorithm policies trump date
policies - but that's a reasonable expectation.

+  ASSERT_TRUE(cert);
+
+  CertVerifyResult result;
+  result.has_sha1 = true;
+  result.has_sha1_leaf = false;
+  result.is_issued_by_known_root = true;
+  verify_proc_ = new MockCertVerifyProc(result);
+
+  int flags = 0;
+  CertVerifyResult verify_result;
+  int error = Verify(cert.get(), "127.0.0.1", flags, nullptr /* crl_set */,
+                     empty_cert_list_, &verify_result);
+  if (AreSHA1IntermediatesAllowed()) {
+    EXPECT_THAT(error, IsOk());
+    EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_SHA1_SIGNATURE_PRESENT);
+  } else {
+    EXPECT_THAT(error, IsError(ERR_CERT_WEAK_SIGNATURE_ALGORITHM));
+    EXPECT_TRUE(verify_result.cert_status &
+                CERT_STATUS_WEAK_SIGNATURE_ALGORITHM);
+  }
+}
+
+TEST_F(CertVerifyProcTest, AcceptsPrivateSHA1) {
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), "ok_cert.pem"));
+  ASSERT_TRUE(cert);
+
+  CertVerifyResult result;
+  result.has_sha1 = true;
+  result.has_sha1_leaf = true;
+  result.is_issued_by_known_root = false;
+  verify_proc_ = new MockCertVerifyProc(result);
+
+  int flags = 0;
+  CertVerifyResult verify_result;
+  int error = Verify(cert.get(), "127.0.0.1", flags, nullptr /* crl_set */,
+                     empty_cert_list_, &verify_result);
+  EXPECT_THAT(error, IsOk());
+  EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_SHA1_SIGNATURE_PRESENT);
+}
+
 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 14 matching lines @@
 }
 
 class CertVerifyProcWeakDigestTest
     : public CertVerifyProcTest,
       public testing::WithParamInterface<WeakDigestTestData> {
  public:
   CertVerifyProcWeakDigestTest() {}
   virtual ~CertVerifyProcWeakDigestTest() {}
 };
 
-TEST_P(CertVerifyProcWeakDigestTest, Verify) {
+// Test that the underlying cryptographic library properly surfaces the
+// algorithms used in the chain. Some libraries, like NSS, don't return
+// the failing chain on error, and thus not all tests can be run.
+TEST_P(CertVerifyProcWeakDigestTest, VerifyDetectsAlgorithm) {
   WeakDigestTestData data = GetParam();
   base::FilePath certs_dir = GetTestCertsDirectory();
 
   ScopedTestRoot test_root;
   if (data.root_cert_filename) {
      scoped_refptr<X509Certificate> root_cert =
          ImportCertFromFile(certs_dir, data.root_cert_filename);
-     ASSERT_NE(static_cast<X509Certificate*>(NULL), root_cert.get());
+     ASSERT_TRUE(root_cert);
      test_root.Reset(root_cert.get());
   }
 
   scoped_refptr<X509Certificate> intermediate_cert =
       ImportCertFromFile(certs_dir, data.intermediate_cert_filename);
-  ASSERT_NE(static_cast<X509Certificate*>(NULL), intermediate_cert.get());
+  ASSERT_TRUE(intermediate_cert);
   scoped_refptr<X509Certificate> ee_cert =
       ImportCertFromFile(certs_dir, data.ee_cert_filename);
-  ASSERT_NE(static_cast<X509Certificate*>(NULL), ee_cert.get());
+  ASSERT_TRUE(ee_cert);
 
   X509Certificate::OSCertHandles intermediates;
   intermediates.push_back(intermediate_cert->os_cert_handle());
 
   scoped_refptr<X509Certificate> ee_chain =
       X509Certificate::CreateFromHandle(ee_cert->os_cert_handle(),
                                         intermediates);
-  ASSERT_NE(static_cast<X509Certificate*>(NULL), ee_chain.get());
+  ASSERT_TRUE(ee_chain);
 
   int flags = 0;
   CertVerifyResult verify_result;
-  int rv = Verify(ee_chain.get(),
-                  "127.0.0.1",
-                  flags,
-                  NULL,
-                  empty_cert_list_,
-                  &verify_result);
+  Verify(ee_chain.get(), "127.0.0.1", flags, NULL, empty_cert_list_,
+         &verify_result);
   EXPECT_EQ(!!(data.expected_algorithms & EXPECT_MD2), verify_result.has_md2);
   EXPECT_EQ(!!(data.expected_algorithms & EXPECT_MD4), verify_result.has_md4);
   EXPECT_EQ(!!(data.expected_algorithms & EXPECT_MD5), verify_result.has_md5);
   EXPECT_EQ(!!(data.expected_algorithms & EXPECT_SHA1), verify_result.has_sha1);
   EXPECT_EQ(!!(data.expected_algorithms & EXPECT_SHA1_LEAF),
             verify_result.has_sha1_leaf);
-
-  EXPECT_FALSE(verify_result.is_issued_by_additional_trust_anchor);
-
-  // Ensure that MD4 and MD2 are tagged as invalid.
-  if (data.expected_algorithms & (EXPECT_MD2 | EXPECT_MD4)) {
-    EXPECT_EQ(CERT_STATUS_INVALID,
-              verify_result.cert_status & CERT_STATUS_INVALID);
-  }
-
-  // Ensure that MD5 is flagged as weak.
-  if (data.expected_algorithms & EXPECT_MD5) {
-    EXPECT_EQ(
-        CERT_STATUS_WEAK_SIGNATURE_ALGORITHM,
-        verify_result.cert_status & CERT_STATUS_WEAK_SIGNATURE_ALGORITHM);
-  }
-
-  // If a root cert is present, then check that the chain was rejected if any
-  // weak algorithms are present. This is only checked when a root cert is
-  // present because the error reported for incomplete chains with weak
-  // algorithms depends on which implementation was used to validate (NSS,
-  // OpenSSL, CryptoAPI, Security.framework) and upon which weak algorithm
-  // present (MD2, MD4, MD5).
-  if (data.root_cert_filename) {
-    if (data.expected_algorithms & (EXPECT_MD2 | EXPECT_MD4)) {
-      EXPECT_THAT(rv, IsError(ERR_CERT_INVALID));
-    } else if (data.expected_algorithms & EXPECT_MD5) {
-      EXPECT_THAT(rv, IsError(ERR_CERT_WEAK_SIGNATURE_ALGORITHM));
-    } else {
-      EXPECT_THAT(rv, IsOk());
-    }
-  }
 }
 
 // Unlike TEST/TEST_F, which are macros that expand to further macros,
 // INSTANTIATE_TEST_CASE_P is a macro that expands directly to code that
 // stringizes the arguments. As a result, macros passed as parameters (such as
 // prefix or test_case_name) will not be expanded by the preprocessor. To work
 // around this, indirect the macro for INSTANTIATE_TEST_CASE_P, so that the
 // pre-processor will expand macros such as MAYBE_test_name before
 // instantiating the test.
 #define WRAPPED_INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator) \
@@ skipping 230 matching lines @@
 }
 #endif  // defined(OS_MACOSX) && !defined(OS_IOS)
 
 // Tests that CertVerifyProc records a histogram correctly when a
 // certificate chaining to a private root contains the TLS feature
 // extension and does not have a stapled OCSP response.
 TEST_F(CertVerifyProcTest, HasTLSFeatureExtensionUMA) {
   base::HistogramTester histograms;
   scoped_refptr<X509Certificate> cert(
       ImportCertFromFile(GetTestCertsDirectory(), "tls_feature_extension.pem"));
+  ASSERT_TRUE(cert);
   CertVerifyResult result;
   result.is_issued_by_known_root = false;
-  result.verified_cert = cert;
   verify_proc_ = new MockCertVerifyProc(result);
 
   histograms.ExpectTotalCount(kTLSFeatureExtensionHistogram, 0);
   histograms.ExpectTotalCount(kTLSFeatureExtensionOCSPHistogram, 0);
 
   int flags = 0;
   CertVerifyResult verify_result;
   int error = Verify(cert.get(), "127.0.0.1", flags, NULL, empty_cert_list_,
                      &verify_result);
   EXPECT_EQ(OK, error);
   histograms.ExpectTotalCount(kTLSFeatureExtensionHistogram, 1);
   histograms.ExpectBucketCount(kTLSFeatureExtensionHistogram, true, 1);
   histograms.ExpectTotalCount(kTLSFeatureExtensionOCSPHistogram, 1);
   histograms.ExpectBucketCount(kTLSFeatureExtensionOCSPHistogram, false, 1);
 }
 
 // Tests that CertVerifyProc records a histogram correctly when a
 // certificate chaining to a private root contains the TLS feature
 // extension and does have a stapled OCSP response.
 TEST_F(CertVerifyProcTest, HasTLSFeatureExtensionWithStapleUMA) {
   base::HistogramTester histograms;
   scoped_refptr<X509Certificate> cert(
       ImportCertFromFile(GetTestCertsDirectory(), "tls_feature_extension.pem"));
+  ASSERT_TRUE(cert);
   CertVerifyResult result;
   result.is_issued_by_known_root = false;
-  result.verified_cert = cert;
   verify_proc_ = new MockCertVerifyProc(result);
 
   histograms.ExpectTotalCount(kTLSFeatureExtensionHistogram, 0);
   histograms.ExpectTotalCount(kTLSFeatureExtensionOCSPHistogram, 0);
 
   int flags = 0;
   CertVerifyResult verify_result;
   int error =
       VerifyWithOCSPResponse(cert.get(), "127.0.0.1", "dummy response", flags,
                              NULL, empty_cert_list_, &verify_result);
   EXPECT_EQ(OK, error);
   histograms.ExpectTotalCount(kTLSFeatureExtensionHistogram, 1);
   histograms.ExpectBucketCount(kTLSFeatureExtensionHistogram, true, 1);
   histograms.ExpectTotalCount(kTLSFeatureExtensionOCSPHistogram, 1);
   histograms.ExpectBucketCount(kTLSFeatureExtensionOCSPHistogram, true, 1);
 }
 
 // Tests that CertVerifyProc records a histogram correctly when a
 // certificate chaining to a private root does not contain the TLS feature
 // extension.
 TEST_F(CertVerifyProcTest, DoesNotHaveTLSFeatureExtensionUMA) {
   base::HistogramTester histograms;
   scoped_refptr<X509Certificate> cert(
       ImportCertFromFile(GetTestCertsDirectory(), "ok_cert.pem"));
+  ASSERT_TRUE(cert);
   CertVerifyResult result;
   result.is_issued_by_known_root = false;
-  result.verified_cert = cert;
   verify_proc_ = new MockCertVerifyProc(result);
 
   histograms.ExpectTotalCount(kTLSFeatureExtensionHistogram, 0);
   histograms.ExpectTotalCount(kTLSFeatureExtensionOCSPHistogram, 0);
 
   int flags = 0;
   CertVerifyResult verify_result;
   int error = Verify(cert.get(), "127.0.0.1", flags, NULL, empty_cert_list_,
                      &verify_result);
   EXPECT_EQ(OK, error);
   histograms.ExpectTotalCount(kTLSFeatureExtensionHistogram, 1);
   histograms.ExpectBucketCount(kTLSFeatureExtensionHistogram, false, 1);
   histograms.ExpectTotalCount(kTLSFeatureExtensionOCSPHistogram, 0);
 }
 
 // Tests that CertVerifyProc does not record a histogram when a
 // certificate contains the TLS feature extension but chains to a public
 // root.
 TEST_F(CertVerifyProcTest, HasTLSFeatureExtensionWithPublicRootUMA) {
   base::HistogramTester histograms;
   scoped_refptr<X509Certificate> cert(
       ImportCertFromFile(GetTestCertsDirectory(), "tls_feature_extension.pem"));
+  ASSERT_TRUE(cert);
   CertVerifyResult result;
   result.is_issued_by_known_root = true;
-  result.verified_cert = cert;
   verify_proc_ = new MockCertVerifyProc(result);
 
   histograms.ExpectTotalCount(kTLSFeatureExtensionHistogram, 0);
 
   int flags = 0;
   CertVerifyResult verify_result;
   int error = Verify(cert.get(), "127.0.0.1", flags, NULL, empty_cert_list_,
                      &verify_result);
   EXPECT_EQ(OK, error);
   histograms.ExpectTotalCount(kTLSFeatureExtensionHistogram, 0);
   histograms.ExpectTotalCount(kTLSFeatureExtensionOCSPHistogram, 0);
 }
 
 }  // namespace net