Refactor TransportSecurityState.

net/base/x509_certificate.cc

Index: net/base/x509_certificate.cc
===================================================================
--- net/base/x509_certificate.cc	(revision 128526)
+++ net/base/x509_certificate.cc	(working copy)
@@ -4,6 +4,17 @@
 
 #include "net/base/x509_certificate.h"
 
+#if defined(USE_OPENSSL)
+#include <openssl/ecdsa.h>
+#include <openssl/ssl.h>
+#else  // !defined(USE_OPENSSL)
+#include <cryptohi.h>
+#include <hasht.h>
+#include <keyhi.h>
+#include <pk11pub.h>
+#include <nspr.h>
+#endif

[Ryan Sleevi, 2012/03/28 00:50:32]

These do not belong in x509_certificate.cc

If they were acceptable, they'd belong at line 42-44, as per
http://dev.chromium.org/developers/coding-style#TOC-Code-formatting

[palmer, 2012/04/10 23:25:51]

This and the other things removed; some kind of mishap.

+
 #include <stdlib.h>
 
 #include <algorithm>
@@ -11,6 +22,7 @@
 #include <string>
 #include <vector>
 
+#include "net/base/asn1_util.h"
 #include "base/base64.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
@@ -22,10 +34,15 @@
 #include "base/string_util.h"
 #include "base/synchronization/lock.h"
 #include "base/time.h"
+#include "crypto/sha2.h"

[Ryan Sleevi, 2012/03/28 00:50:32]

??

[palmer, 2012/04/10 23:25:51]

Done.

 #include "googleurl/src/url_canon_ip.h"
 #include "net/base/net_util.h"
 #include "net/base/pem_tokenizer.h"
 
+#if defined(USE_OPENSSL)
+#include "crypto/openssl_util.h"
+#endif

[Ryan Sleevi, 2012/03/28 00:50:32]

Nor this

[palmer, 2012/04/10 23:25:51]

Done.

+
 namespace net {
 
 namespace {
@@ -479,6 +496,23 @@
     dns_names->push_back(subject_.common_name);
 }
 
+// static
+bool X509Certificate::GetPublicKeyHash(const OSCertHandle& cert,

[Ryan Sleevi, 2012/03/28 00:50:32]

My gut is that because this builds directly on top of GetDEREncoded, it may not
belong in X509Certificate at all.

Do you anticipate further users of this beyond pinning, and could you explain
where/why/how?

[palmer, 2012/04/10 23:25:51]

Turns out we only need this for a unit test. Moved it there.

+                                       SHA1Fingerprint* fingerprint) {
+  std::string der_bytes;
+  if (!GetDEREncoded(cert, &der_bytes))

[Ryan Sleevi, 2012/03/28 00:50:32]

This requires making a copy of the |cert| data.

If you do think this is a re-usable method, then you should implement this
within each of the _platform files, which can use the raw data when applicable
(eg: NSS+Win, but not OS X, OpenSSL, or Android)

[palmer, 2012/04/10 23:25:51]

Done.

+    return false;
+
+  base::StringPiece spki;
+  if (!asn1::ExtractSPKIFromDERCert(der_bytes, &spki))
+    return false;
+
+  base::SHA1HashBytes(reinterpret_cast<const unsigned char*>(spki.data()),
+                      spki.size(), fingerprint->data);
+  return true;
+}
+
+
 bool X509Certificate::HasExpired() const {
   return base::Time::Now() > valid_expiry();
 }