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) {

[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
   // 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