Implement SHA-256 fingerprint support

net/base/transport_security_state.cc

Index: net/base/transport_security_state.cc
===================================================================
--- net/base/transport_security_state.cc	(revision 151057)
+++ net/base/transport_security_state.cc	(working copy)
@@ -16,7 +16,6 @@
 #endif
 
 #include <algorithm>
-#include <utility>
 
 #include "base/base64.h"
 #include "base/logging.h"
@@ -33,6 +32,7 @@
 #include "googleurl/src/gurl.h"
 #include "net/base/dns_util.h"
 #include "net/base/ssl_info.h"
+#include "net/base/x509_cert_types.h"
 #include "net/base/x509_certificate.h"
 #include "net/http/http_util.h"
 
@@ -225,69 +225,78 @@
   return pair;
 }
 
-// TODO(palmer): Support both sha256 and sha1. This will require additional
-// infrastructure code changes and can come in a later patch.
-//
 // static
 bool TransportSecurityState::ParsePin(const std::string& value,
-                                      SHA1Fingerprint* out) {
+                                      HashValue* out) {
   StringPair slash = Split(Strip(value), '/');
-  if (slash.first != "sha1")
+
+  if (slash.first == "sha1")
+    out->tag = HASH_VALUE_SHA1;
+  else if (slash.first == "sha256")
+    out->tag = HASH_VALUE_SHA256;
+  else
     return false;
 
   std::string decoded;
   if (!base::Base64Decode(slash.second, &decoded) ||
-      decoded.size() != arraysize(out->data)) {
+      decoded.size() != out->size()) {
     return false;
   }
 
-  memcpy(out->data, decoded.data(), arraysize(out->data));
+  memcpy(out->data(), decoded.data(), out->size());
   return true;
 }
 
 static bool ParseAndAppendPin(const std::string& value,
-                      FingerprintVector* fingerprints) {
+                              HashValueVector* fingerprints) {
   // The base64'd fingerprint MUST be a quoted-string. 20 bytes base64'd is 28
-  // characters; 32 bytes base64'd is 44 characters. TODO(palmer): Support
-  // SHA256.
+  // characters; 32 bytes base64'd is 44 characters.
   size_t size = value.size();
-  if (size != 30 || value[0] != '"' || value[size - 1] != '"')
+  if ((size != 30 && size != 46) || value[0] != '"' || value[size - 1] != '"')

[hshi1, 2012/08/11 01:05:40]

Having hard-coded numerical values (such as 30 and 46) is awkward. Moreover I
feel that this size check (30 or 46 characters) is rather redundant. The
decoded.size() check a few lines below already verifies that the hash is of the
corrected length.

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

Done.

     return false;
 
   std::string unquoted = HttpUtil::Unquote(value);
   std::string decoded;
-  SHA1Fingerprint fp;
+  HashValue fp;
 
-  if (!base::Base64Decode(unquoted, &decoded) ||
-      decoded.size() != arraysize(fp.data)) {
+  // This code has to assume that 32 bytes is SHA-256 and 20 bytes is SHA-1.
+  // Currently, those are the only two possibilities, so the assumption is
+  // valid.
+  if (!base::Base64Decode(unquoted, &decoded))
     return false;
-  }
 
-  memcpy(fp.data, decoded.data(), arraysize(fp.data));
+  if (decoded.size() == 20)

[hshi1, 2012/08/11 01:05:40]

Nit: would it be better to use base::kSHA1Length instead of 20?

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

Done.

+    fp.tag = HASH_VALUE_SHA1;
+  else if (decoded.size() == 32)

[hshi1, 2012/08/11 01:05:40]

Nit: would it be better to use crypto::kSHA256Length instead of 32?

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

Done.

+    fp.tag = HASH_VALUE_SHA256;
+  else
+    return false;
+
+  memcpy(fp.data(), decoded.data(), fp.size());
   fingerprints->push_back(fp);
   return true;
 }
 
-struct FingerprintsEqualPredicate {
-  explicit FingerprintsEqualPredicate(const SHA1Fingerprint& fingerprint) :
+struct HashValuesEqualPredicate {
+  explicit HashValuesEqualPredicate(const HashValue& fingerprint) :
       fingerprint_(fingerprint) {}
 
-  bool operator()(const SHA1Fingerprint& other) const {
+  bool operator()(const HashValue& other) const {
     return fingerprint_.Equals(other);
   }
 
-  const SHA1Fingerprint& fingerprint_;
+  const HashValue& fingerprint_;
 };
 
 // Returns true iff there is an item in |pins| which is not present in
 // |from_cert_chain|. Such an SPKI hash is called a "backup pin".
-static bool IsBackupPinPresent(const FingerprintVector& pins,
-                               const FingerprintVector& from_cert_chain) {
-  for (FingerprintVector::const_iterator
+static bool IsBackupPinPresent(const HashValueVector& pins,
+                               const HashValueVector& from_cert_chain) {
+  for (HashValueVector::const_iterator
        i = pins.begin(); i != pins.end(); ++i) {
-    FingerprintVector::const_iterator j =
+    HashValueVector::const_iterator j =
         std::find_if(from_cert_chain.begin(), from_cert_chain.end(),
-                     FingerprintsEqualPredicate(*i));
+                     HashValuesEqualPredicate(*i));
       if (j == from_cert_chain.end())
         return true;
   }
@@ -295,14 +304,15 @@
   return false;
 }
 
-static bool HashesIntersect(const FingerprintVector& a,
-                            const FingerprintVector& b) {
-  for (FingerprintVector::const_iterator
-       i = a.begin(); i != a.end(); ++i) {
-    FingerprintVector::const_iterator j =
-        std::find_if(b.begin(), b.end(), FingerprintsEqualPredicate(*i));
-      if (j != b.end())
-        return true;
+// Returns true if the intersection of |a| and |b| is not empty. If either
+// |a| or |b| is empty, returns false.
+static bool HashesIntersect(const HashValueVector& a,
+                            const HashValueVector& b) {
+  for (HashValueVector::const_iterator i = a.begin(); i != a.end(); ++i) {
+    HashValueVector::const_iterator j =
+        std::find_if(b.begin(), b.end(), HashValuesEqualPredicate(*i));
+    if (j != b.end())
+      return true;
   }
 
   return false;
@@ -313,17 +323,30 @@
 // backup pin is a pin intended for disaster recovery, not day-to-day use, and
 // thus must be absent from the certificate chain. The Public-Key-Pins header
 // specification requires both.
-static bool IsPinListValid(const FingerprintVector& pins,
+static bool IsPinListValid(const HashValueVector& pins,
                            const SSLInfo& ssl_info) {
+  // Fast fail: 1 live + 1 backup = at least 2 pins. (Check for actual
+  // liveness and backupness below.)
   if (pins.size() < 2)
     return false;
 
-  const FingerprintVector& from_cert_chain = ssl_info.public_key_hashes;
-  if (from_cert_chain.empty())
+  // Site operators might pin a key using either the SHA-1 or SHA-256 hash
+  // of the SPKI. So check for success using either hash function.
+  //
+  // TODO(palmer): Make this generic so that it works regardless of what
+  // HashValueTags are defined in the future.
+  const HashValueVector& from_cert_chain_sha1 =
+      ssl_info.public_key_hashes[HASH_VALUE_SHA1];
+  const HashValueVector& from_cert_chain_sha256 =
+      ssl_info.public_key_hashes[HASH_VALUE_SHA256];
+
+  if (from_cert_chain_sha1.empty() && from_cert_chain_sha256.empty())
     return false;
 
-  return IsBackupPinPresent(pins, from_cert_chain) &&
-      HashesIntersect(pins, from_cert_chain);
+  return (IsBackupPinPresent(pins, from_cert_chain_sha1) ||
+          IsBackupPinPresent(pins, from_cert_chain_sha256)) &&
+         (HashesIntersect(pins, from_cert_chain_sha1) ||
+          HashesIntersect(pins, from_cert_chain_sha256));
 }
 
 // "Public-Key-Pins" ":"
@@ -335,7 +358,7 @@
     const SSLInfo& ssl_info) {
   bool parsed_max_age = false;
   int max_age_candidate = 0;
-  FingerprintVector pins;
+  HashValueVector pins;
 
   std::string source = value;
 
@@ -356,11 +379,10 @@
       if (max_age_candidate > kMaxHSTSAgeSecs)
         max_age_candidate = kMaxHSTSAgeSecs;
       parsed_max_age = true;
-    } else if (LowerCaseEqualsASCII(equals.first, "pin-sha1")) {
+    } else if (LowerCaseEqualsASCII(equals.first, "pin-sha1") ||
+               LowerCaseEqualsASCII(equals.first, "pin-sha256")) {
       if (!ParseAndAppendPin(equals.second, &pins))
         return false;
-    } else if (LowerCaseEqualsASCII(equals.first, "pin-sha256")) {
-      // TODO(palmer)
     } else {
       // Silently ignore unknown directives for forward compatibility.
     }
@@ -376,7 +398,7 @@
 
   dynamic_spki_hashes.clear();
   if (max_age_candidate > 0) {
-    for (FingerprintVector::const_iterator i = pins.begin();
+    for (HashValueVector::const_iterator i = pins.begin();
          i != pins.end(); ++i) {
       dynamic_spki_hashes.push_back(*i);
     }
@@ -483,8 +505,8 @@
 }
 
 static bool AddHash(const std::string& type_and_base64,
-                    FingerprintVector* out) {
-  SHA1Fingerprint hash;
+                    HashValueVector* out) {
+  HashValue hash;
 
   if (!TransportSecurityState::ParsePin(type_and_base64, &hash))
     return false;
@@ -750,13 +772,13 @@
 }
 
 static std::string HashesToBase64String(
-    const FingerprintVector& hashes) {
+    const HashValueVector& hashes) {
   std::vector<std::string> hashes_strs;
-  for (FingerprintVector::const_iterator
+  for (HashValueVector::const_iterator
        i = hashes.begin(); i != hashes.end(); i++) {
     std::string s;
-    const std::string hash_str(reinterpret_cast<const char*>(i->data),
-                               sizeof(i->data));
+    const std::string hash_str(reinterpret_cast<const char*>(i->data()),
+                               i->size());
     base::Base64Encode(hash_str, &s);
     hashes_strs.push_back(s);
   }
@@ -774,23 +796,48 @@
 }
 
 bool TransportSecurityState::DomainState::IsChainOfPublicKeysPermitted(
-    const FingerprintVector& hashes) const {
-  if (HashesIntersect(bad_static_spki_hashes, hashes)) {
-    LOG(ERROR) << "Rejecting public key chain for domain " << domain
-               << ". Validated chain: " << HashesToBase64String(hashes)
-               << ", matches one or more bad hashes: "
-               << HashesToBase64String(bad_static_spki_hashes);
+    const std::vector<HashValueVector>& hashes) const {
+  // Validate that hashes is not empty. By the time this code is called (in
+  // production), that should never happen, but it's good to be defensive.
+  // And, hashes *can* be empty in some test scenarios.
+  bool empty = true;
+  for (size_t i = 0; i < hashes.size(); ++i) {
+    if (hashes[i].size()) {
+      empty = false;
+      break;
+    }
+  }
+  if (empty) {
+    LOG(ERROR) << "Rejecting empty public key chain for public-key-pinned "
+                  "domain " << domain;
     return false;
   }
 
-  if (!(dynamic_spki_hashes.empty() && static_spki_hashes.empty()) &&
-      !HashesIntersect(dynamic_spki_hashes, hashes) &&
-      !HashesIntersect(static_spki_hashes, hashes)) {
+  bool found_good_pin = false;
+  for (size_t i = 0; i < hashes.size(); ++i) {
+    if (HashesIntersect(bad_static_spki_hashes, hashes[i])) {
+      LOG(ERROR) << "Rejecting public key chain for domain " << domain
+                 << ". Validated chain: " << HashesToBase64String(hashes[i])
+                 << ", matches one or more bad hashes: "
+                 << HashesToBase64String(bad_static_spki_hashes);
+      return false;
+    }
+
+    if (HashesIntersect(dynamic_spki_hashes, hashes[i]) ||
+        HashesIntersect(static_spki_hashes, hashes[i])) {
+      found_good_pin = true;
+    }
+  }
+
+  if (!found_good_pin) {
+    std::vector<std::string> base64s;
+    for (size_t i = 0; i < hashes.size(); ++i)
+      base64s.push_back(HashesToBase64String(hashes[i]));
+
     LOG(ERROR) << "Rejecting public key chain for domain " << domain
-               << ". Validated chain: " << HashesToBase64String(hashes)
+               << ". Validated chain: " << JoinString(base64s, ',')
                << ", expected: " << HashesToBase64String(dynamic_spki_hashes)
                << " or: " << HashesToBase64String(static_spki_hashes);
-
     return false;
   }