Implement SHA-256 fingerprint support

net/base/cert_verify_proc.cc

 // Copyright (c) 2012 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/cert_verify_proc.h"
 
 #include "base/metrics/histogram.h"
 #include "base/sha1.h"
 #include "build/build_config.h"
 #include "net/base/cert_status_flags.h"
@@ skipping 199 matching lines @@
                                   arraysize(kComodoSerials) + 1);
         return true;
       }
     }
   }
 
   return false;
 }
 
 // static
+// NOTE: This implementation assumes and enforces that the hashes are SHA1.
 bool CertVerifyProc::IsPublicKeyBlacklisted(
-    const std::vector<SHA1Fingerprint>& public_key_hashes) {
+    const std::vector<HashValueVector>& public_key_hashes) {
   static const unsigned kNumHashes = 9;
   static const uint8 kHashes[kNumHashes][base::kSHA1Length] = {
     // Subject: CN=DigiNotar Root CA
     // Issuer: CN=Entrust.net x2 and self-signed
     {0x41, 0x0f, 0x36, 0x36, 0x32, 0x58, 0xf3, 0x0b, 0x34, 0x7d,
      0x12, 0xce, 0x48, 0x63, 0xe4, 0x33, 0x43, 0x78, 0x06, 0xa8},
     // Subject: CN=DigiNotar Cyber CA
     // Issuer: CN=GTE CyberTrust Global Root
     {0xc4, 0xf9, 0x66, 0x37, 0x16, 0xcd, 0x5e, 0x71, 0xd6, 0x95,
      0x0b, 0x5f, 0x33, 0xce, 0x04, 0x1c, 0x95, 0xb4, 0x35, 0xd1},
@@ skipping 24 matching lines @@
     // 2021 GMT.
     {0xe1, 0x2d, 0x89, 0xf5, 0x6d, 0x22, 0x76, 0xf8, 0x30, 0xe6,
      0xce, 0xaf, 0xa6, 0x6c, 0x72, 0x5c, 0x0b, 0x41, 0xa9, 0x32},
     // Cyberoam CA certificate. Private key leaked, but this certificate would
     // only have been installed by Cyberoam customers. The certificate expires
     // in 2036, but we can probably remove in a couple of years (2014).
     {0xd9, 0xf5, 0xc6, 0xce, 0x57, 0xff, 0xaa, 0x39, 0xcc, 0x7e,
      0xd1, 0x72, 0xbd, 0x53, 0xe0, 0xd3, 0x07, 0x83, 0x4b, 0xd1},
   };
 
+  const HashValueVector& sha1_hashes = public_key_hashes[HASH_VALUE_SHA1];

[Ryan Sleevi, 2012/08/11 01:39:55]

BUG: Accessing HASH_VALUE_SHA1 without checking that .size() >= HASH_VALUE_SHA1

Seems like you really want an std::map<HashValueTag, HashValueVector> here as
the arg, but for presumably perf, you're relying on the fact that a HashValueTag
is a viable index into an std::vector<>. I think that's really non-obvious, and
minimally requires more defensiveness.

My gut is that "adding DCHECKs" wouldn't be in itself sufficient

[palmer, 2012/08/14 19:40:42]

Well, the constructor guarantees that public_key_hashes has the right number of
items. Can't we count on that?

As I recall using a vector of vectors instead of a map of vectors was your
idea... :) I'm fine with changing it to a map if people think it's necessary or
better.

[Ryan Sleevi, 2012/08/14 20:02:51]

What constructor?

public_key_hashes is an input argument here. There's no guarantee that the
caller ensured it's in the proper form - or that it's not an empty vector.

That's what I meant here. Adding DCHECKs is great, but since it seems very
unlikely that caller's will be directly supplying hashes (as opposed to crafting
them from user/network input), it seems risky.
You could pass a (fixed-size) array of (dynamically-sized) vectors ;-) At least
then the compiler "should" warn, IIRC. More testing is needed to confirm I'm not
completely mistaken here

IsPublicKeyBlacklisted(const std::vector<HashValueVector>
(&public_key_hashes)[HASH_VALUES_TAG_COUNT])

That will pass a reference to a fixed-size array of 2 vectors. Beyond the
ugliness (which can probably be accomodated with a typedef), I don't recall what
the behaviour is for copy-assignment between two arrays - whether it performs a
memory-wise copy or if it invokes each members' copy constructor.

   for (unsigned i = 0; i < kNumHashes; i++) {
-    for (std::vector<SHA1Fingerprint>::const_iterator
-         j = public_key_hashes.begin(); j != public_key_hashes.end(); ++j) {
-      if (memcmp(j->data, kHashes[i], base::kSHA1Length) == 0)
+    for (HashValueVector::const_iterator j = sha1_hashes.begin();
+         j != sha1_hashes.end(); ++j) {
+      if (j->tag == HASH_VALUE_SHA1 &&
+          memcmp(j->data(), kHashes[i], base::kSHA1Length) == 0) {
         return true;
+      }
     }
   }
 
   return false;
 }
 
 }  // namespace net