Refuse to accept certificate chains containing any RSA public key smaller

net/base/x509_certificate_unittest.cc

 // Copyright (c) 2011 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 "base/file_path.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/pickle.h"
 #include "base/sha1.h"
 #include "base/string_number_conversions.h"
@@ skipping 593 matching lines @@
   int flags = X509Certificate::VERIFY_REV_CHECKING_ENABLED |
               X509Certificate::VERIFY_EV_CERT;
   int error = cert_chain->Verify("2029.globalsign.com", flags, NULL,
                                  &verify_result);
   if (error == OK)
     EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_IS_EV);
   else
     EXPECT_EQ(ERR_CERT_DATE_INVALID, error);
 }
 
+// Currently, only RSA and DSA keys are checked for weakness, and our example
+// weak size is 768. These could change in the future.
+static bool IsWeakKeyType(const std::string& key_type) {
+  size_t pos = key_type.find("-");
+  std::string size = key_type.substr(0, pos);
+  std::string type = key_type.substr(pos + 1);
+
+  if (type == "rsa" || type == "dsa")
+    return size == "768";
+
+  return false;

[wtc, 2011/12/14 00:18:03]

Nit: same here -- I think a comment to document this function
may return false negatives is useful.

+}
+
+TEST(X509CertificateTest, RejectWeakKeys) {
+  FilePath certs_dir = GetTestCertsDirectory();
+  typedef std::vector<std::string> Strings;
+  Strings key_types;
+
+  // generate-weak-test-chains.sh currently has:
+  //     key_types="768-rsa 1024-rsa 2048-rsa prime256v1-ecdsa"
+  // We must use the same key types here. The filenames generated look like:
+  //     2048-rsa-ee-by-768-rsa-intermediate.pem
+  key_types.push_back("768-rsa");
+  key_types.push_back("1024-rsa");
+  key_types.push_back("2048-rsa");
+  key_types.push_back("prime256v1-ecdsa");
+
+  // Add the root that signed the intermediates for this test.
+  scoped_refptr<X509Certificate> root_cert =
+      ImportCertFromFile(certs_dir, "2048-rsa-root.pem");
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), root_cert);
+  TestRootCerts::GetInstance()->Add(root_cert.get());

[Ryan Sleevi, 2011/12/13 23:54:16]

Possible flakiness BUG: Make sure to call TestRootCerts::GetInstance()->Clear()
at the end of this test. Leaving a spare root around can cause unexpected issues
with path building.

I suppose I should make a scoped helper here to automagically
TestRootCerts::Clear() [or have the test fixture do it in TearDown)

+
+  // Now test each chain.
+  for (Strings::const_iterator ee_type = key_types.begin();
+       ee_type != key_types.end(); ++ee_type) {
+    for (Strings::const_iterator signer_type = key_types.begin();
+         signer_type != key_types.end(); ++signer_type) {
+      std::string basename = *ee_type + "-ee-by-" + *signer_type +
+          "-intermediate.pem";
+      scoped_refptr<X509Certificate> ee_cert =
+          ImportCertFromFile(certs_dir, basename);
+      ASSERT_NE(static_cast<X509Certificate*>(NULL), ee_cert);
+
+      basename = *signer_type + "-intermediate.pem";
+      scoped_refptr<X509Certificate> intermediate =
+          ImportCertFromFile(certs_dir, basename);
+      ASSERT_NE(static_cast<X509Certificate*>(NULL), intermediate);
+
+      X509Certificate::OSCertHandles intermediates;
+      intermediates.push_back(intermediate->os_cert_handle());
+      scoped_refptr<X509Certificate> cert_chain =
+          X509Certificate::CreateFromHandle(ee_cert->os_cert_handle(),
+                                            intermediates);
+
+      CertVerifyResult verify_result;
+      int error = cert_chain->Verify("127.0.0.1", 0, NULL, &verify_result);
+
+      if (IsWeakKeyType(*ee_type) || IsWeakKeyType(*signer_type)) {
+        EXPECT_NE(OK, error);
+        EXPECT_EQ(CERT_STATUS_WEAK_KEY,
+                  verify_result.cert_status & CERT_STATUS_WEAK_KEY);
+      } else {
+        EXPECT_EQ(OK, error);
+        EXPECT_EQ(0U, verify_result.cert_status & CERT_STATUS_WEAK_KEY);
+      }
+    }
+  }
+}
+
 // Test for bug 94673.
 TEST(X509CertificateTest, GoogleDigiNotarTest) {
   FilePath certs_dir = GetTestCertsDirectory();
 
   scoped_refptr<X509Certificate> server_cert =
       ImportCertFromFile(certs_dir, "google_diginotar.pem");
   ASSERT_NE(static_cast<X509Certificate*>(NULL), server_cert);
 
   scoped_refptr<X509Certificate> intermediate_cert =
       ImportCertFromFile(certs_dir, "diginotar_public_ca_2025.pem");
@@ skipping 96 matching lines @@
   EXPECT_TRUE(X509Certificate::GetDEREncoded(cert->os_cert_handle(),
                                              &derBytes));
 
   base::StringPiece spkiBytes;
   EXPECT_TRUE(asn1::ExtractSPKIFromDERCert(derBytes, &spkiBytes));
 
   uint8 hash[base::kSHA1Length];
   base::SHA1HashBytes(reinterpret_cast<const uint8*>(spkiBytes.data()),
                       spkiBytes.size(), hash);
 
-  EXPECT_TRUE(0 == memcmp(hash, nistSPKIHash, sizeof(hash)));
+  EXPECT_EQ(0, memcmp(hash, nistSPKIHash, sizeof(hash)));
 }
 
 TEST(X509CertificateTest, ExtractCRLURLsFromDERCert) {
   FilePath certs_dir = GetTestCertsDirectory();
   scoped_refptr<X509Certificate> cert =
       ImportCertFromFile(certs_dir, "nist.der");
   ASSERT_NE(static_cast<X509Certificate*>(NULL), cert);
 
   std::string derBytes;
   EXPECT_TRUE(X509Certificate::GetDEREncoded(cert->os_cert_handle(),
@@ skipping 634 matching lines @@
     { true, "f", "f" },
     { false, "h", "i" },
     { true, "bar.foo.com", "*.foo.com" },
     { true, "www.test.fr", "common.name",
         "*.test.com,*.test.co.uk,*.test.de,*.test.fr" },
     { true, "wwW.tESt.fr",  "common.name",
         ",*.*,*.test.de,*.test.FR,www" },
     { false, "f.uk", ".uk" },
     { false, "w.bar.foo.com", "?.bar.foo.com" },
     { false, "www.foo.com", "(www|ftp).foo.com" },
-    { false, "www.foo.com", "www.foo.com#" }, // # = null char.
+    { false, "www.foo.com", "www.foo.com#" },  // # = null char.
     { false, "www.foo.com", "", "www.foo.com#*.foo.com,#,#" },
     { false, "www.house.example", "ww.house.example" },
     { false, "test.org", "", "www.test.org,*.test.org,*.org" },
     { false, "w.bar.foo.com", "w*.bar.foo.com" },
     { false, "www.bar.foo.com", "ww*ww.bar.foo.com" },
     { false, "wwww.bar.foo.com", "ww*ww.bar.foo.com" },
     { true, "wwww.bar.foo.com", "w*w.bar.foo.com" },
     { false, "wwww.bar.foo.com", "w*w.bar.foo.c0m" },
     { true, "WALLY.bar.foo.com", "wa*.bar.foo.com" },
     { true, "wally.bar.foo.com", "*Ly.bar.foo.com" },
@@ skipping 117 matching lines @@
   }
 
   if (test_data.ip_addrs) {
     // Build up the certificate IP address list.
     std::string ip_addrs_line(test_data.ip_addrs);
     std::vector<std::string> ip_addressses_ascii;
     base::SplitString(ip_addrs_line, ',', &ip_addressses_ascii);
     for (size_t i = 0; i < ip_addressses_ascii.size(); ++i) {
       std::string& addr_ascii = ip_addressses_ascii[i];
       ASSERT_NE(0U, addr_ascii.length());
-      if (addr_ascii[0] == 'x') { // Hex encoded address
+      if (addr_ascii[0] == 'x') {  // Hex encoded address
         addr_ascii.erase(0, 1);
         std::vector<uint8> bytes;
         EXPECT_TRUE(base::HexStringToBytes(addr_ascii, &bytes))
             << "Could not parse hex address " << addr_ascii << " i = " << i;
         ip_addressses.push_back(std::string(reinterpret_cast<char*>(&bytes[0]),
                                             bytes.size()));
         ASSERT_EQ(16U, ip_addressses.back().size()) << i;
       } else {  // Decimal groups
         std::vector<std::string> decimals_ascii;
         base::SplitString(addr_ascii, '.', &decimals_ascii);
@@ skipping 217 matching lines @@
 #define MAYBE_VerifyMixed DISABLED_VerifyMixed
 #else
 #define MAYBE_VerifyMixed VerifyMixed
 #endif
 WRAPPED_INSTANTIATE_TEST_CASE_P(
     MAYBE_VerifyMixed,
     X509CertificateWeakDigestTest,
     testing::ValuesIn(kVerifyMixedTestData));
 
 }  // namespace net