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) {
+    verify_result->cert_status |= CERT_STATUS_WEAK_KEY;
+    return MapCertStatusToNetError(verify_result->cert_status);
+  }
+  for (OSCertHandles::const_iterator i = intermediate_ca_certs_.begin();
+       i != intermediate_ca_certs_.end(); ++i) {
+    GetPublicKeyInfo(*i, &size_bits, &type);
+    if (type == kPublicKeyTypeRSA && size_bits < 1023) {
+      verify_result->cert_status |= CERT_STATUS_WEAK_KEY;
+      return MapCertStatusToNetError(verify_result->cert_status);
+    }
+  }
+

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

I don't think this is the correct place to do it, for the same reason I didn't
put the MD2/MD4/MD5 checking here.

First reason is that this only iterates the server supplied chain. Except for OS
X, none of the other implementations care that much about the rest of the server
supplied certs being related.

Second reason is that, in the event of weak certs (particularly intermediaries),
it's possible they'll be replaced (or introduced) during AIA fetching. So this
can have both false negatives and false positives.

I think this should be moved to after VerifyInternal, and operate on
|verified_result->verified_cert|

Also, the same concerns raised re: md2/md4/md5 masking more serious errors may
apply here (if this is interstitialed) or may not.

   int rv = VerifyInternal(hostname, flags, crl_set, verify_result);
 
   // This check is done after VerifyInternal so that VerifyInternal can fill in