Refuse to accept certificate chains containing any RSA public key smaller

net/base/x509_certificate.cc

Index: net/base/x509_certificate.cc
===================================================================
--- net/base/x509_certificate.cc	(revision 110129)
+++ net/base/x509_certificate.cc	(working copy)
@@ -588,6 +588,23 @@
     return ERR_CERT_REVOKED;
   }
 
+  // Check for weak keys (in the entire chain) first.
+  size_t size_bits = 0;
+  PublicKeyType type = kPublicKeyTypeUnknown;
+  GetPublicKeyInfo(cert_handle_, &size_bits, &type);
+  if (type == kPublicKeyTypeRSA && size_bits < 1023) {

[Ryan Sleevi, 2011/11/16 23:40:54]

As AGL noted over on the Mozilla bug, the '1023' is likely an artifact of
OpenSSL reporting, not the actual key size.

We should be able to use 1024 here.

+    verify_result->cert_status |= CERT_STATUS_WEAK_KEY;
+    return MapCertStatusToNetError(verify_result->cert_status);

[Ryan Sleevi, 2011/11/16 23:40:54]

Rather than duplicating this logic with the lines on 603/604, just use a bool
and have a single exit point from this function for week keys?

+  }
+  for (OSCertHandles::const_iterator i = intermediate_ca_certs_.begin();
+       i != intermediate_ca_certs_.end(); ++i) {
+    GetPublicKeyInfo(*i, &size_bits, &type);

[Ryan Sleevi, 2011/11/16 23:40:54]

bug: If GetPublicKeyInfo fails, |type| is not mutated, but |size_bits| is. This
will cause unknown types to be rejected as weak keys. Intentional? Should this
API return a bool?

It's less of a concern if moved to after verification.

+    if (type == kPublicKeyTypeRSA && size_bits < 1023) {
+      verify_result->cert_status |= CERT_STATUS_WEAK_KEY;
+      return MapCertStatusToNetError(verify_result->cert_status);
+    }
+  }
+
   int rv = VerifyInternal(hostname, flags, crl_set, verify_result);
 
   // This check is done after VerifyInternal so that VerifyInternal can fill in