Support SHA-256 in public key pins for HTTPS.

net/base/x509_cert_types.h

Index: net/base/x509_cert_types.h
===================================================================
--- net/base/x509_cert_types.h	(revision 142157)
+++ net/base/x509_cert_types.h	(working copy)
@@ -8,10 +8,12 @@
 
 #include <string.h>
 
+#include <algorithm>
 #include <set>
 #include <string>
 #include <vector>
 
+#include "base/logging.h"
 #include "base/string_piece.h"
 #include "build/build_config.h"
 #include "net/base/net_export.h"
@@ -37,13 +39,6 @@
   unsigned char data[20];
 };
 
-// In the future there will be a generic Fingerprint type, with at least two
-// implementations: SHA1 and SHA256. See http://crbug.com/117914. Until that
-// work is done (in a separate patch) this typedef bridges the gap.
-typedef SHA1Fingerprint Fingerprint;
-
-typedef std::vector<Fingerprint> FingerprintVector;
-
 class NET_EXPORT SHA1FingerprintLessThan {
  public:
   bool operator() (const SHA1Fingerprint& lhs,
@@ -52,6 +47,96 @@
   }
 };
 
+struct NET_EXPORT SHA256Fingerprint {
+  bool Equals(const SHA256Fingerprint& other) const {
+    return memcmp(data, other.data, sizeof(data)) == 0;
+  }
+
+  unsigned char data[32];
+};
+
+class NET_EXPORT SHA256FingerprintLessThan {
+ public:
+  bool operator() (const SHA256Fingerprint& lhs,
+                   const SHA256Fingerprint& rhs) const {
+    return memcmp(lhs.data, rhs.data, sizeof(lhs.data)) < 0;
+  }
+};
+
+enum FingerprintTag {
+  FINGERPRINT_SHA1,
+  FINGERPRINT_SHA256,
+};
+
+struct NET_EXPORT Fingerprint {

[wtc, 2012/06/18 17:00:29]

It would be nice to come up with a better name than Fingerprint.

The type has been renamed once, from
X509Certificate::SHA1Fingerprint to SHA1Fingerprint.
If we shorten it again to just "Fingerprint", it is no longer
descriptive enough to someone unfamiliar with the code.

I think we should use something like HashResult, HashValue,
or HashFingerprint.

+  bool Equals(const Fingerprint& other) const {
+    if (tag != other.tag)
+      return false;
+    switch (tag) {
+      case FINGERPRINT_SHA1:
+        return fingerprint.sha1.Equals(other.fingerprint.sha1);
+        break;
+      case FINGERPRINT_SHA256:
+        return fingerprint.sha256.Equals(other.fingerprint.sha256);
+        break;
+      default:
+        DCHECK(false) << "Unknown FingerprintTag " << tag;

[Ryan Sleevi, 2012/06/15 21:44:00]

NOTREACHED() rather than DCHECK(false). You can still stream error messages
though.

[palmer, 2012/06/15 22:40:49]

Done.

+        return false;
+    }
+  }
+
+  size_t size() const {
+    switch (tag) {
+      case FINGERPRINT_SHA1:
+        return sizeof(fingerprint.sha1.data);
+        break;
+      case FINGERPRINT_SHA256:
+        return sizeof(fingerprint.sha256.data);
+        break;
+      default:
+        DCHECK(false) << "Unknown FingerprintTag " << tag;
+        return false;

[Ryan Sleevi, 2012/06/15 21:44:00]

return 0;

[palmer, 2012/06/15 22:40:49]

Done.

+    }
+  }
+
+  unsigned char* data() const {

[Ryan Sleevi, 2012/06/15 21:44:00]

This should either be:

const unsigned char* data() const { <-- logical const correct
or
unsigned char* data() { <-- non-const

If you need to provide two versions, that's more preferable than the const-cast.

[palmer, 2012/06/15 22:40:49]

Done.

+    switch (tag) {
+      case FINGERPRINT_SHA1:
+        return const_cast<unsigned char*>(fingerprint.sha1.data);
+        break;
+      case FINGERPRINT_SHA256:
+        return const_cast<unsigned char*>(fingerprint.sha256.data);
+        break;
+      default:
+        DCHECK(false) << "Unknown FingerprintTag " << tag;
+        return NULL;
+    }
+  }
+
+  FingerprintTag tag;
+
+  union {
+    SHA1Fingerprint sha1;
+    SHA256Fingerprint sha256;
+  } fingerprint;
+};
+
+class NET_EXPORT FingerprintLessThan {
+ public:
+  bool operator() (const Fingerprint& lhs,
+                   const Fingerprint& rhs) const {

[Ryan Sleevi, 2012/06/15 21:44:00]

You should compare their types & sizes first, before doing the memory
comparison.

[palmer, 2012/06/15 22:40:49]

What should I do if they don't match? This is for use in sorting
FingerprintVectors, for example. Basically I'm treating this like strcmp. Doing
e.g. NOTREACHED for lhs.tag != rhs.tag doesn't seem appropriate.

[Ryan Sleevi, 2012/06/15 22:51:45]

Compare their tags/sizes.

SHA1 sorts lexically SHA256.
Within SHA1, hashes are sorted based memcmp
Within SHA256, hashes are sorted based on memcmp

simply:
if (lhs.tag != rhs.tag)
  return lhs.tag < rhs.tag;
if (lhs.size != rhs.size)  // not strictly needed?
  return lhs.size < rhs.size;  // not strictly needed?
return memcmp(lhs.data(), rhs.data(), lhs_size)

[Ryan Sleevi, 2012/06/15 22:54:49]

sorry, the last one would be

return memcmp(lhs.data(), rhs.data(), lhs_size) < 0;

+    size_t lhs_size = lhs.size();
+    size_t rhs_size = rhs.size();
+    int r = memcmp(lhs.data(), rhs.data(), std::min(lhs_size, rhs_size));
+
+    if (r == 0 && lhs_size != rhs_size)
+      return lhs_size < rhs_size;
+    return r < 0;
+  }
+};
+
+typedef std::vector<Fingerprint> FingerprintVector;
+
 // IsSHA1HashInSortedArray returns true iff |hash| is in |array|, a sorted
 // array of SHA1 hashes.
 bool NET_EXPORT IsSHA1HashInSortedArray(const SHA1Fingerprint& hash,