Refactor the assignment of CertVerifyResult::has_md2, etc.

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 1678 matching lines @@
 }
 
 class CertVerifyProcWeakDigestTest
     : public CertVerifyProcTest,
       public testing::WithParamInterface<WeakDigestTestData> {
  public:
   CertVerifyProcWeakDigestTest() {}
   virtual ~CertVerifyProcWeakDigestTest() {}
 };
 
-// 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 that the CertVerifyProc::Verify() properly surfaces the (weak) hashing
+// algorithms used in the chain.
 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_TRUE(root_cert);
-     test_root.Reset(root_cert.get());
+  scoped_refptr<X509Certificate> intermediate_cert;
+  scoped_refptr<X509Certificate> root_cert;
+
+  // Build |intermediates| as the full chain (including trust anchor).
+  X509Certificate::OSCertHandles intermediates;
+
+  if (data.intermediate_cert_filename) {
+    intermediate_cert =
+        ImportCertFromFile(certs_dir, data.intermediate_cert_filename);
+    ASSERT_TRUE(intermediate_cert);
+    intermediates.push_back(intermediate_cert->os_cert_handle());
   }
 
-  scoped_refptr<X509Certificate> intermediate_cert =
-      ImportCertFromFile(certs_dir, data.intermediate_cert_filename);
-  ASSERT_TRUE(intermediate_cert);
+  if (data.root_cert_filename) {
+    root_cert = ImportCertFromFile(certs_dir, data.root_cert_filename);
+    ASSERT_TRUE(root_cert);
+    intermediates.push_back(root_cert->os_cert_handle());
+  }
+
   scoped_refptr<X509Certificate> ee_cert =
       ImportCertFromFile(certs_dir, data.ee_cert_filename);
   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_TRUE(ee_chain);
 
   int flags = 0;
   CertVerifyResult verify_result;
-  Verify(ee_chain.get(), "127.0.0.1", flags, NULL, empty_cert_list_,
-         &verify_result);
+
+  // Use a mock CertVerifyProc that returns success with a verified_cert of
+  // |ee_chain|.
+  //
+  // This is sufficient for the purposes of this test, as the checking for weak
+  // hashing algorithms is done by CertVerifyProc::Verify().
+  scoped_refptr<CertVerifyProc> proc =
+      new MockCertVerifyProc(CertVerifyResult());
+  proc->Verify(ee_chain.get(), "127.0.0.1", std::string(), flags, NULL,

[Ryan Sleevi, 2017/01/10 01:53:24]

nullptr?

[eroman, 2017/01/10 02:48:46]

Done.

+               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);
 }
 
-// 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) \
-    INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator)
-
 // The signature algorithm of the root CA should not matter.
 const WeakDigestTestData kVerifyRootCATestData[] = {
     {"weak_digest_md5_root.pem", "weak_digest_sha1_intermediate.pem",
      "weak_digest_sha1_ee.pem", EXPECT_SHA1 | EXPECT_SHA1_LEAF},
-#if defined(USE_OPENSSL_CERTS) || defined(OS_WIN)
-    // MD4 is not supported by OS X / NSS
     {"weak_digest_md4_root.pem", "weak_digest_sha1_intermediate.pem",
      "weak_digest_sha1_ee.pem", EXPECT_SHA1 | EXPECT_SHA1_LEAF},
-#endif
     {"weak_digest_md2_root.pem", "weak_digest_sha1_intermediate.pem",
      "weak_digest_sha1_ee.pem", EXPECT_SHA1 | EXPECT_SHA1_LEAF},
 };
 INSTANTIATE_TEST_CASE_P(VerifyRoot,
                         CertVerifyProcWeakDigestTest,
                         testing::ValuesIn(kVerifyRootCATestData));
 
 // The signature algorithm of intermediates should be properly detected.
 const WeakDigestTestData kVerifyIntermediateCATestData[] = {
     {"weak_digest_sha1_root.pem", "weak_digest_md5_intermediate.pem",
      "weak_digest_sha1_ee.pem", EXPECT_MD5 | EXPECT_SHA1 | EXPECT_SHA1_LEAF},
-#if defined(USE_OPENSSL_CERTS) || defined(OS_WIN)
-    // MD4 is not supported by OS X / NSS
     {"weak_digest_sha1_root.pem", "weak_digest_md4_intermediate.pem",
      "weak_digest_sha1_ee.pem", EXPECT_MD4 | EXPECT_SHA1 | EXPECT_SHA1_LEAF},
-#endif
     {"weak_digest_sha1_root.pem", "weak_digest_md2_intermediate.pem",
      "weak_digest_sha1_ee.pem", EXPECT_MD2 | EXPECT_SHA1 | EXPECT_SHA1_LEAF},
 };
-// Disabled on NSS - MD4 is not supported, and MD2 and MD5 are disabled.
-#if defined(USE_NSS_CERTS) || defined(OS_IOS)
-#define MAYBE_VerifyIntermediate DISABLED_VerifyIntermediate
-#else
-#define MAYBE_VerifyIntermediate VerifyIntermediate
-#endif
-WRAPPED_INSTANTIATE_TEST_CASE_P(
-    MAYBE_VerifyIntermediate,
-    CertVerifyProcWeakDigestTest,
-    testing::ValuesIn(kVerifyIntermediateCATestData));
+
+INSTANTIATE_TEST_CASE_P(VerifyIntermediate,
+                        CertVerifyProcWeakDigestTest,
+                        testing::ValuesIn(kVerifyIntermediateCATestData));
 
 // The signature algorithm of end-entity should be properly detected.
 const WeakDigestTestData kVerifyEndEntityTestData[] = {
   { "weak_digest_sha1_root.pem", "weak_digest_sha1_intermediate.pem",
     "weak_digest_md5_ee.pem", EXPECT_MD5 | EXPECT_SHA1 },
-#if defined(USE_OPENSSL_CERTS) || defined(OS_WIN)
-  // MD4 is not supported by OS X / NSS
   { "weak_digest_sha1_root.pem", "weak_digest_sha1_intermediate.pem",
     "weak_digest_md4_ee.pem", EXPECT_MD4 | EXPECT_SHA1 },
-#endif
   { "weak_digest_sha1_root.pem", "weak_digest_sha1_intermediate.pem",
     "weak_digest_md2_ee.pem", EXPECT_MD2 | EXPECT_SHA1 },
 };
-// Disabled on NSS - NSS caches chains/signatures in such a way that cannot
-// be cleared until NSS is cleanly shutdown, which is not presently supported
-// in Chromium.
-// OSX 10.12+ stops building the chain at the first weak digest.
-#if defined(USE_NSS_CERTS) || defined(OS_IOS) || defined(OS_MACOSX)
-#define MAYBE_VerifyEndEntity DISABLED_VerifyEndEntity
-#else
-#define MAYBE_VerifyEndEntity VerifyEndEntity
-#endif
-WRAPPED_INSTANTIATE_TEST_CASE_P(MAYBE_VerifyEndEntity,
-                                CertVerifyProcWeakDigestTest,
-                                testing::ValuesIn(kVerifyEndEntityTestData));
 
-// Incomplete chains should still report the status of the intermediate.
+INSTANTIATE_TEST_CASE_P(VerifyEndEntity,
+                        CertVerifyProcWeakDigestTest,
+                        testing::ValuesIn(kVerifyEndEntityTestData));
+
+// Incomplete chains do not report the status of the intermediate. This is
+// an implementation issue with CertVerifyProc::Verify(), as it does not
+// know whether the final intermediate is a trust anchor or not.

[Ryan Sleevi, 2017/01/10 01:53:24]

This comment doesn't seem entirely accurate - incomplete chains do report the
status of the intermediate, just not the *last* intermediate.

You sort of capture this in 1812-1815

[eroman, 2017/01/10 02:48:46]

Done.

 const WeakDigestTestData kVerifyIncompleteIntermediateTestData[] = {
     {NULL, "weak_digest_md5_intermediate.pem", "weak_digest_sha1_ee.pem",
-     EXPECT_MD5 | EXPECT_SHA1 | EXPECT_SHA1_LEAF},
-#if defined(USE_OPENSSL_CERTS) || defined(OS_WIN)
-    // MD4 is not supported by OS X / NSS
+     EXPECT_SHA1 | EXPECT_SHA1_LEAF},
     {NULL, "weak_digest_md4_intermediate.pem", "weak_digest_sha1_ee.pem",
-     EXPECT_MD4 | EXPECT_SHA1 | EXPECT_SHA1_LEAF},
-#endif
+     EXPECT_SHA1 | EXPECT_SHA1_LEAF},
     {NULL, "weak_digest_md2_intermediate.pem", "weak_digest_sha1_ee.pem",
-     EXPECT_MD2 | EXPECT_SHA1 | EXPECT_SHA1_LEAF},
+     EXPECT_SHA1 | EXPECT_SHA1_LEAF},
 };
-// Disabled on NSS - libpkix does not return constructed chains on error,
-// preventing us from detecting/inspecting the verified chain.
-#if defined(USE_NSS_CERTS) || defined(OS_IOS)
-#define MAYBE_VerifyIncompleteIntermediate \
-    DISABLED_VerifyIncompleteIntermediate
-#else
-#define MAYBE_VerifyIncompleteIntermediate VerifyIncompleteIntermediate
-#endif
-WRAPPED_INSTANTIATE_TEST_CASE_P(
+
+INSTANTIATE_TEST_CASE_P(
     MAYBE_VerifyIncompleteIntermediate,
     CertVerifyProcWeakDigestTest,
     testing::ValuesIn(kVerifyIncompleteIntermediateTestData));
 
-// Incomplete chains should still report the status of the end-entity.
+// Incomplete chains should report the status of the end-entity.
+// Note: really each of these tests should also expect EXPECT_SHA1. However
+// CertVerifyProc::Verify() is unable to distinguish that this is an
+// intermediate and not a trust anchor, so this intermediate is treated like a
+// trust anchor.
 const WeakDigestTestData kVerifyIncompleteEETestData[] = {
-  { NULL, "weak_digest_sha1_intermediate.pem", "weak_digest_md5_ee.pem",
-    EXPECT_MD5 | EXPECT_SHA1 },
-#if defined(USE_OPENSSL_CERTS) || defined(OS_WIN)
-  // MD4 is not supported by OS X / NSS
-  { NULL, "weak_digest_sha1_intermediate.pem", "weak_digest_md4_ee.pem",
-    EXPECT_MD4 | EXPECT_SHA1 },
-#endif
-  { NULL, "weak_digest_sha1_intermediate.pem", "weak_digest_md2_ee.pem",
-    EXPECT_MD2 | EXPECT_SHA1 },
+    {NULL, "weak_digest_sha1_intermediate.pem", "weak_digest_md5_ee.pem",
+     EXPECT_MD5},
+    {NULL, "weak_digest_sha1_intermediate.pem", "weak_digest_md4_ee.pem",
+     EXPECT_MD4},
+    {NULL, "weak_digest_sha1_intermediate.pem", "weak_digest_md2_ee.pem",
+     EXPECT_MD2},
 };
-// Disabled on NSS - libpkix does not return constructed chains on error,
-// preventing us from detecting/inspecting the verified chain.
-// OSX 10.12+ stops building the chain at the first weak digest.
-#if defined(USE_NSS_CERTS) || defined(OS_IOS) || defined(OS_MACOSX)
-#define MAYBE_VerifyIncompleteEndEntity DISABLED_VerifyIncompleteEndEntity
-#else
-#define MAYBE_VerifyIncompleteEndEntity VerifyIncompleteEndEntity
-#endif
-WRAPPED_INSTANTIATE_TEST_CASE_P(
-    MAYBE_VerifyIncompleteEndEntity,
-    CertVerifyProcWeakDigestTest,
-    testing::ValuesIn(kVerifyIncompleteEETestData));
+
+INSTANTIATE_TEST_CASE_P(VerifyIncompleteEndEntity,
+                        CertVerifyProcWeakDigestTest,
+                        testing::ValuesIn(kVerifyIncompleteEETestData));
 
 // Differing algorithms between the intermediate and the EE should still be
 // reported.
 const WeakDigestTestData kVerifyMixedTestData[] = {
   { "weak_digest_sha1_root.pem", "weak_digest_md5_intermediate.pem",
     "weak_digest_md2_ee.pem", EXPECT_MD2 | EXPECT_MD5 },
   { "weak_digest_sha1_root.pem", "weak_digest_md2_intermediate.pem",
     "weak_digest_md5_ee.pem", EXPECT_MD2 | EXPECT_MD5 },
-#if defined(USE_OPENSSL_CERTS) || defined(OS_WIN)
-  // MD4 is not supported by OS X / NSS
   { "weak_digest_sha1_root.pem", "weak_digest_md4_intermediate.pem",
     "weak_digest_md2_ee.pem", EXPECT_MD2 | EXPECT_MD4 },
-#endif
 };
-// NSS does not support MD4 and does not enable MD2 by default, making all
-// permutations invalid.
-// OSX 10.12+ stops building the chain at the first weak digest.
-#if defined(USE_NSS_CERTS) || defined(OS_IOS) || defined(OS_MACOSX)
-#define MAYBE_VerifyMixed DISABLED_VerifyMixed
-#else
-#define MAYBE_VerifyMixed VerifyMixed
-#endif
-WRAPPED_INSTANTIATE_TEST_CASE_P(
-    MAYBE_VerifyMixed,
-    CertVerifyProcWeakDigestTest,
-    testing::ValuesIn(kVerifyMixedTestData));
+
+INSTANTIATE_TEST_CASE_P(VerifyMixed,
+                        CertVerifyProcWeakDigestTest,
+                        testing::ValuesIn(kVerifyMixedTestData));
+
+// The EE is a trusted certificate. Even though it uses weak hashes, these
+// should not be reported.
+const WeakDigestTestData kVerifyTrustedEETestData[] = {
+    {NULL, NULL, "weak_digest_md5_ee.pem", 0},
+    {NULL, NULL, "weak_digest_md4_ee.pem", 0},
+    {NULL, NULL, "weak_digest_md2_ee.pem", 0},
+    {NULL, NULL, "weak_digest_sha1_ee.pem", 0},
+};
+
+INSTANTIATE_TEST_CASE_P(VerifyTrustedEE,
+                        CertVerifyProcWeakDigestTest,
+                        testing::ValuesIn(kVerifyTrustedEETestData));
 
 // For the list of valid hostnames, see
 // net/cert/data/ssl/certificates/subjectAltName_sanity_check.pem
 static const struct CertVerifyProcNameData {
   const char* hostname;
   bool valid;  // Whether or not |hostname| matches a subjectAltName.
 } kVerifyNameData[] = {
   { "127.0.0.1", false },  // Don't match the common name
   { "127.0.0.2", true },  // Matches the iPAddress SAN (IPv4)
   { "FE80:0:0:0:0:0:0:1", true },  // Matches the iPAddress SAN (IPv6)
@@ skipping 41 matching lines @@
                      &verify_result);
   if (data.valid) {
     EXPECT_THAT(error, IsOk());
     EXPECT_FALSE(verify_result.cert_status & CERT_STATUS_COMMON_NAME_INVALID);
   } else {
     EXPECT_THAT(error, IsError(ERR_CERT_COMMON_NAME_INVALID));
     EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_COMMON_NAME_INVALID);
   }
 }
 
-WRAPPED_INSTANTIATE_TEST_CASE_P(
-    VerifyName,
-    CertVerifyProcNameTest,
-    testing::ValuesIn(kVerifyNameData));
+INSTANTIATE_TEST_CASE_P(VerifyName,
+                        CertVerifyProcNameTest,
+                        testing::ValuesIn(kVerifyNameData));
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
 // Test that CertVerifyProcMac reacts appropriately when Apple's certificate
 // verifier rejects a certificate with a fatal error. This is a regression
 // test for https://crbug.com/472291.
 // (Since 10.12, this causes a recoverable error instead of a fatal one.)
 // TODO(mattm): Try to find a different way to cause a fatal error that works
 // on 10.12.
 TEST_F(CertVerifyProcTest, LargeKey) {
   // Load root_ca_cert.pem into the test root store.
@@ skipping 110 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(OK, error);
   histograms.ExpectTotalCount(kTLSFeatureExtensionHistogram, 0);
   histograms.ExpectTotalCount(kTLSFeatureExtensionOCSPHistogram, 0);
 }
 
 }  // namespace net