Refactor TransportSecurityState.

net/base/x509_certificate.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/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>
 #include <map>
 #include <string>
 #include <vector>
 
+#include "net/base/asn1_util.h"
 #include "base/base64.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram.h"
 #include "base/pickle.h"
 #include "base/sha1.h"
 #include "base/string_piece.h"
 #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 {
 
 // Indicates the order to use when trying to decode binary data, which is
 // based on (speculation) as to what will be most common -> least common
 const X509Certificate::Format kFormatDecodePriority[] = {
   X509Certificate::FORMAT_SINGLE_CERTIFICATE,
   X509Certificate::FORMAT_PKCS7
 };
@@ skipping 433 matching lines @@
     }
   }
 }
 
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   GetSubjectAltName(dns_names, NULL);
   if (dns_names->empty())
     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();
 }
 
 bool X509Certificate::Equals(const X509Certificate* other) const {
   return IsSameOSCert(cert_handle_, other->cert_handle_);
 }
 
 // static
 bool X509Certificate::VerifyHostname(
@@ skipping 190 matching lines @@
     RemoveFromCache(cert_handle_);
     FreeOSCertHandle(cert_handle_);
   }
   for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i) {
     RemoveFromCache(intermediate_ca_certs_[i]);
     FreeOSCertHandle(intermediate_ca_certs_[i]);
   }
 }
 
 }  // namespace net