Add --ignore-certificate-errors-spki-list switch and UMA histogram.

net/http/http_stream_factory_impl_job.cc

Index: net/http/http_stream_factory_impl_job.cc
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index d50d24fe13707a6db60bb9cf4537ce6273b2a6aa..f481c9d0975136928b47e556bf6ba119c846fe64 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -1528,6 +1528,30 @@ int HttpStreamFactoryImpl::Job::HandleCertificateError(int error) {
     load_flags |= LOAD_IGNORE_ALL_CERT_ERRORS;
   if (ssl_socket->IgnoreCertError(error, load_flags))
     return OK;
+
+  // Ignore errors for certificates that chain up to switch-whitelisted certs.
+  if (!session_->params().ignore_certificate_error_spki_list.empty()) {
+    // TODO(martinkr): Should we also include check the chain constructed by NSS
+    // in case validation was successful (i.e. ssl_config_.public_key_hashes)?

[Ryan Sleevi, 2017/03/16 23:19:14]

I'm not sure why we would - but did I miss something?

[martinkr, 2017/03/28 23:16:02]

I guess it might be possible that the cert validates ok but fails hostname
matching, with some intermediate or root in the chain not sent by the server
(and hence not in GetIntermediateCertificates()), and the user wants to ignore
errors based on that CA? Not sure how realistic that is though.

+    HashValueVector hashes;
+    hashes.push_back(
+        HashValue(net::X509Certificate::CalculatePublicKeyHashSHA256(
+            ssl_info_.unverified_cert->os_cert_handle())));
+    for (const net::X509Certificate::OSCertHandle& intermediate :
+         ssl_info_.unverified_cert->GetIntermediateCertificates()) {
+      hashes.push_back(HashValue(
+          // Does this need a non-NSS implementation?
+          net::X509Certificate::CalculatePublicKeyHashSHA256(intermediate)));
+    }
+    for (const HashValue& hash : hashes) {
+      const std::string spki = hash.ToString().substr(7);  // Strip 'sha256/'.

[Ryan Sleevi, 2017/03/16 23:19:14]

This ends up forcing multiple string allocation/copies. Any reason that
ignore_certificate_error_spki_list shouldn't just be a std::set<HashValue> ? Or
better yet, base::flat_set<> since this list should always be small?

Further, if you went with |hashes| as a base::flat_set<>, then you could do

auto first1 = session_params().ignore_[...].begin();
auto last1 = session_params().ignore_[...].end();
auto first2 = hashes.begin();
auto last2 = hashes.end();
while (first1 != last1 && first2 != last2) {
  if (*first1 < *first2) ++first1;
  elseif (*first2 < *first1) ++first2;
  else return true;
}
return false;


That will tell you if any element in [first2, last2) is in [first1, last1) with
a single linear scan of both.

(This is effectively std::set_intersection, except returning if at least one
item is found)

[martinkr, 2017/03/28 23:16:02]

I just assumed these sets to be tiny in all cases, I guess, but you're right.

Done.

+      if (session_->params().ignore_certificate_error_spki_list.find(spki) !=
+          session_->params().ignore_certificate_error_spki_list.end()) {
+        return OK;
+      }
+    }
+  }
+
   return error;
 }