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

net/http/http_stream_factory_impl_job.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/http/http_stream_factory_impl_job.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 1510 matching lines @@
   // RestartIgnoringLastError(). And the user will be asked interactively
   // before RestartIgnoringLastError() is ever called.
   server_ssl_config_.allowed_bad_certs.emplace_back(ssl_info_.cert,
                                                     ssl_info_.cert_status);
 
   int load_flags = request_info_.load_flags;
   if (session_->params().ignore_certificate_errors)
     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;
 }
 
 ClientSocketPoolManager::SocketGroupType
 HttpStreamFactoryImpl::Job::GetSocketGroup() const {
   std::string scheme = origin_url_.scheme();
   if (scheme == url::kHttpsScheme || scheme == url::kWssScheme)
     return ClientSocketPoolManager::SSL_GROUP;
 
   if (scheme == url::kFtpScheme)
@@ skipping 14 matching lines @@
 
   ConnectionAttempts socket_attempts = connection_->connection_attempts();
   if (connection_->socket()) {
     connection_->socket()->GetConnectionAttempts(&socket_attempts);
   }
 
   delegate_->AddConnectionAttemptsToRequest(this, socket_attempts);
 }
 
 }  // namespace net