Add unittests for the detection of md[2,4,5] when verifying certificates

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 6dab44f18c67f810fc28c0cdbe32f70174145caa..b228a064131e6affb6dbb30ed35261932994c5f1 100644
--- a/net/base/x509_certificate_unittest.cc
+++ b/net/base/x509_certificate_unittest.cc
@@ -1247,15 +1247,10 @@ struct CertificateNameVerifyTestData {
   const char* ip_addrs;
 };
 
-// Required by valgrind on mac, otherwise it complains when using its default
-// printer:
-// UninitCondition
-// Conditional jump or move depends on uninitialised value(s)
-// ...
-// snprintf
-// testing::(anonymous namespace)::PrintByteSegmentInObjectTo
-// testing::internal2::TypeWithoutFormatter
-// ...
+// GTest 'magic' pretty-printer, so that if/when a test fails, it knows how
+// to output the parameter that was passed. Without this, it will simply
+// attempt to print out the first twenty bytes of the object, which depending
+// on platform and alignment, may result in an invalid read.
 void PrintTo(const CertificateNameVerifyTestData& data, std::ostream* os) {
   ASSERT_TRUE(data.hostname && data.common_name);
   // Using StringPiece to allow for optional fields being NULL.
@@ -1449,4 +1444,201 @@ TEST_P(X509CertificateNameVerifyTest, VerifyHostname) {
 INSTANTIATE_TEST_CASE_P(, X509CertificateNameVerifyTest,
                         testing::ValuesIn(kNameVerifyTestData));
 
+// Not implemented on Mac or OpenSSL - http://crbug.com/101123
+#if defined(USE_NSS) || defined(OS_WIN)
+
+struct WeakDigestTestData {
+  const char* root_cert_filename;
+  const char* intermediate_cert_filename;
+  const char* ee_cert_filename;
+  bool expected_has_md5;
+  bool expected_has_md4;
+  bool expected_has_md2;
+  bool expected_has_md5_ca;
+  bool expected_has_md2_ca;
+};
+
+// GTest 'magic' pretty-printer, so that if/when a test fails, it knows how
+// to output the parameter that was passed. Without this, it will simply
+// attempt to print out the first twenty bytes of the object, which depending
+// on platform and alignment, may result in an invalid read.
+void PrintTo(const WeakDigestTestData& data, std::ostream* os) {
+  *os << "root: "
+      << (data.root_cert_filename ? data.root_cert_filename : "none")
+      << "; intermediate: " << data.intermediate_cert_filename
+      << "; end-entity: " << data.ee_cert_filename;
+}
+
+class X509CertificateWeakDigestTest
+    : public testing::TestWithParam<WeakDigestTestData> {
+ public:
+  X509CertificateWeakDigestTest() {}
+
+  virtual void TearDown() {
+    TestRootCerts::GetInstance()->Clear();
+  }
+};
+
+TEST_P(X509CertificateWeakDigestTest, Verify) {
+  WeakDigestTestData data = GetParam();
+  FilePath certs_dir = GetTestCertsDirectory();
+
+  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);
+     TestRootCerts::GetInstance()->Add(root_cert.get());
+  }
+
+  scoped_refptr<X509Certificate> intermediate_cert =
+      ImportCertFromFile(certs_dir, data.intermediate_cert_filename);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), intermediate_cert);
+  scoped_refptr<X509Certificate> ee_cert =
+      ImportCertFromFile(certs_dir, data.ee_cert_filename);
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), 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);
+
+  int flags = 0;
+  CertVerifyResult verify_result;
+  ee_chain->Verify("127.0.0.1", flags, NULL, &verify_result);
+  EXPECT_EQ(data.expected_has_md5, verify_result.has_md5);
+  EXPECT_EQ(data.expected_has_md4, verify_result.has_md4);
+  EXPECT_EQ(data.expected_has_md2, verify_result.has_md2);
+  EXPECT_EQ(data.expected_has_md5_ca, verify_result.has_md5_ca);
+  EXPECT_EQ(data.expected_has_md2_ca, verify_result.has_md2_ca);
+}
+
+// Wrapper for the real GTest implementation so that macro expansion for tests
+// such as MAYBE_Foo can happen the same way it does for the other GTest
+// macros, like TEST or TEST_F.

[wtc, 2011/10/31 23:48:00]

The need for this wrapper macro is not obvious to people
unfamiliar with C preprocessor macro expansion.  Perhaps
you can point out that each macro is expanded only once,
so if a macro's argument is itself a macro, we need this
wrapper trick to expand the macro's argument.

How does TEST or TEST_F handle this?  Perhaps it's better
to fix INSTANTIATE_TEST_CASE_P in the long term?

[Ryan Sleevi, 2011/11/01 04:06:46]

I expanded the comment to explain. TEST/TEST_F are both calls to macros, hence a
chance for expansion, while TEST_P/INSTANTIATE_TEST_CASE_P both expand to code
that works to stringize the arguments (thus no extra expansion). 

I've reported this upstream at
http://groups.google.com/group/googletestframework (their issue tracker requests
bugs be reported there, vs the tracker). This was something so trivial that it
didn't seem a big deal, since this was the first time we've run into it, but no
harm fixing upstream as well.

+#define WRAPPED_INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator) \

[palmer, 2011/10/28 18:45:05]

I admit I don't understand this, because I don't know much/anything about how
GTest works.

The content of the tests, including disabling some on NSS, LGTM.

+    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", false, false, false, false, false },
+  { "weak_digest_md4_root.pem", "weak_digest_sha1_intermediate.pem",
+    "weak_digest_sha1_ee.pem", false, false, false, false, false },
+  { "weak_digest_md2_root.pem", "weak_digest_sha1_intermediate.pem",
+    "weak_digest_sha1_ee.pem", false, false, false, false, false },
+};
+INSTANTIATE_TEST_CASE_P(VerifyRoot, X509CertificateWeakDigestTest,
+                        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", true, false, false, true, false },
+// NSS does not support MD4 and does not enable MD2 by policy.
+#if !defined(USE_NSS)
+  { "weak_digest_sha1_root.pem", "weak_digest_md4_intermediate.pem",
+    "weak_digest_sha1_ee.pem", false, true, false, false, false },
+  { "weak_digest_sha1_root.pem", "weak_digest_md2_intermediate.pem",
+    "weak_digest_sha1_ee.pem", false, false, true, false, true },
+#endif
+};
+INSTANTIATE_TEST_CASE_P(VerifyIntermediate, X509CertificateWeakDigestTest,
+                        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", true, false, false, false, false },
+// NSS does not support MD4 and does not enable MD2 by policy.
+#if !defined(USE_NSS)
+  { "weak_digest_sha1_root.pem", "weak_digest_sha1_intermediate.pem",
+    "weak_digest_md4_ee.pem", false, true, false, false, false },
+  { "weak_digest_sha1_root.pem", "weak_digest_sha1_intermediate.pem",
+    "weak_digest_md2_ee.pem", false, false, true, false, false },
+#endif
+};
+// 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.
+#if defined(USE_NSS)
+#define MAYBE_VerifyEndEntity DISABLED_VerifyEndEntity
+#else
+#define MAYBE_VerifyEndEntity VerifyEndEntity
+#endif
+WRAPPED_INSTANTIATE_TEST_CASE_P(MAYBE_VerifyEndEntity,
+                                X509CertificateWeakDigestTest,
+                                testing::ValuesIn(kVerifyEndEntityTestData));
+
+// Incomplete chains should still report the status of the intermediate.
+const WeakDigestTestData kVerifyIncompleteIntermediateTestData[] = {
+  { NULL, "weak_digest_md5_intermediate.pem", "weak_digest_sha1_ee.pem",
+    true, false, false, true, false },
+  { NULL, "weak_digest_md4_intermediate.pem", "weak_digest_sha1_ee.pem",
+    false, true, false, false, false },
+  { NULL, "weak_digest_md2_intermediate.pem", "weak_digest_sha1_ee.pem",
+    false, false, true, false, true },
+};
+// Disabled on Windows - http://crbug.com/101123. The Windows implementation
+// does not report the status of the last intermediate for incomplete chains.
+// Disabled on NSS - libpkix does not return constructed chains on error,
+// preventing us from detecting/inspecting the verified chain.
+#if defined(OS_WIN) || defined(USE_NSS)
+#define MAYBE_VerifyIncompleteIntermediate \
+    DISABLED_VerifyIncompleteIntermediate
+#else
+#define MAYBE_VerifyIncompleteIntermediate VerifyIncompleteIntermediate
+#endif
+WRAPPED_INSTANTIATE_TEST_CASE_P(
+    MAYBE_VerifyIncompleteIntermediate,
+    X509CertificateWeakDigestTest,
+    testing::ValuesIn(kVerifyIncompleteIntermediateTestData));
+
+// Incomplete chains should still report the status of the end-entity.
+const WeakDigestTestData kVerifyIncompleteEETestData[] = {
+  { NULL, "weak_digest_sha1_intermediate.pem", "weak_digest_md5_ee.pem",
+    true, false, false, false, false },
+  { NULL, "weak_digest_sha1_intermediate.pem", "weak_digest_md4_ee.pem",
+    false, true, false, false, false },
+  { NULL, "weak_digest_sha1_intermediate.pem", "weak_digest_md2_ee.pem",
+    false, false, true, false, false },
+};
+// Disabled on NSS - libpkix does not return constructed chains on error,
+// preventing us from detecting/inspecting the verified chain.
+#if defined(USE_NSS)
+#define MAYBE_VerifyIncompleteEndEntity DISABLED_VerifyIncompleteEndEntity
+#else
+#define MAYBE_VerifyIncompleteEndEntity VerifyIncompleteEndEntity
+#endif
+WRAPPED_INSTANTIATE_TEST_CASE_P(
+    MAYBE_VerifyIncompleteEndEntity,
+    X509CertificateWeakDigestTest,
+    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", true, false, true, true, false },
+  { "weak_digest_sha1_root.pem", "weak_digest_md2_intermediate.pem",
+    "weak_digest_md5_ee.pem", true, false, true, false, true },
+  { "weak_digest_sha1_root.pem", "weak_digest_md4_intermediate.pem",
+    "weak_digest_md2_ee.pem", false, true, true, false, false },
+};
+// NSS does not support MD4 and does not enable MD2 by policy, making all
+// permutations invalid.
+#if defined(USE_NSS)
+#define MAYBE_VerifyMixed DISABLED_VerifyMixed
+#else
+#define MAYBE_VerifyMixed VerifyMixed
+#endif
+WRAPPED_INSTANTIATE_TEST_CASE_P(
+    MAYBE_VerifyMixed,
+    X509CertificateWeakDigestTest,
+    testing::ValuesIn(kVerifyMixedTestData));
+
+#endif  // defined(USE_NSS) || defined(OS_WIN)
+
 }  // namespace net