Refactor the assignment of CertVerifyResult::has_md2, etc.

net/cert/cert_verify_proc_unittest.cc

Index: net/cert/cert_verify_proc_unittest.cc
diff --git a/net/cert/cert_verify_proc_unittest.cc b/net/cert/cert_verify_proc_unittest.cc
index c6bb0e71bc5f8fc2701fdc030ef9c6f8bffd02ad..8ffcbd96e24415c2239b8f182ab3da9aa976ab5c 100644
--- a/net/cert/cert_verify_proc_unittest.cc
+++ b/net/cert/cert_verify_proc_unittest.cc
@@ -1696,31 +1696,35 @@ class 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;
+  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());
+  }
+
   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());
+    root_cert = ImportCertFromFile(certs_dir, data.root_cert_filename);
+    ASSERT_TRUE(root_cert);
+    intermediates.push_back(root_cert->os_cert_handle());
   }
 
-  scoped_refptr<X509Certificate> intermediate_cert =
-      ImportCertFromFile(certs_dir, data.intermediate_cert_filename);
-  ASSERT_TRUE(intermediate_cert);
   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);
@@ -1728,8 +1732,16 @@ TEST_P(CertVerifyProcWeakDigestTest, VerifyDetectsAlgorithm) {
 
   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);
@@ -1738,25 +1750,12 @@ TEST_P(CertVerifyProcWeakDigestTest, VerifyDetectsAlgorithm) {
             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},
 };
@@ -1768,99 +1767,64 @@ INSTANTIATE_TEST_CASE_P(VerifyRoot,
 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.
@@ -1869,24 +1833,26 @@ const WeakDigestTestData kVerifyMixedTestData[] = {
     "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
@@ -1948,10 +1914,9 @@ TEST_P(CertVerifyProcNameTest, VerifyCertName) {
   }
 }
 
-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