Refuse to accept certificate chains containing any RSA public key smaller

net/base/x509_certificate.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 "net/base/x509_certificate.h"
 
 #include <stdlib.h>
 
 #include <algorithm>
 #include <map>
@@ skipping 570 matching lines @@
                             CRLSet* crl_set,
                             CertVerifyResult* verify_result) const {
   verify_result->Reset();
   verify_result->verified_cert = const_cast<X509Certificate*>(this);
 
   if (IsBlacklisted()) {
     verify_result->cert_status |= CERT_STATUS_REVOKED;
     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
   // the list of public key hashes.
   if (IsPublicKeyBlacklisted(verify_result->public_key_hashes)) {
     verify_result->cert_status |= CERT_STATUS_REVOKED;
     rv = MapCertStatusToNetError(verify_result->cert_status);
   }
 
   return rv;
@@ skipping 197 matching lines @@
 bool X509Certificate::IsSHA1HashInSortedArray(const SHA1Fingerprint& hash,
                                               const uint8* array,
                                               size_t array_byte_len) {
   DCHECK_EQ(0u, array_byte_len % base::kSHA1Length);
   const size_t arraylen = array_byte_len / base::kSHA1Length;
   return NULL != bsearch(hash.data, array, arraylen, base::kSHA1Length,
                          CompareSHA1Hashes);
 }
 
 }  // namespace net