Process Public-Key-Pin-Report-Only headers

net/http/transport_security_state.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/transport_security_state.h"
 
 #if defined(USE_OPENSSL)
 #include <openssl/ecdsa.h>
 #include <openssl/ssl.h>
 #else  // !defined(USE_OPENSSL)
@@ skipping 117 matching lines @@
   report.Set("known-pins", known_pin_list.Pass());
 
   if (!base::JSONWriter::Write(report, serialized_report)) {
     LOG(ERROR) << "Failed to serialize HPKP violation report.";
     return false;
   }
 
   return true;
 }
 
+bool CheckPinsAndMaybeSendReport(
+    const HostPortPair& host_port_pair,
+    const TransportSecurityState::PKPState& pkp_state,
+    const HashValueVector& hashes,
+    const X509Certificate* served_certificate_chain,
+    const X509Certificate* validated_certificate_chain,
+    const TransportSecurityState::PublicKeyPinReportStatus report_status,
+    TransportSecurityState::ReportSender* report_sender,
+    std::string* failure_log) {
+  if (pkp_state.CheckPublicKeyPins(hashes, failure_log))
+    return true;
+
+  if (!report_sender ||
+      report_status != TransportSecurityState::ENABLE_PIN_REPORTS ||
+      pkp_state.report_uri.is_empty()) {
+    return false;
+  }
+
+  DCHECK(pkp_state.report_uri.is_valid());
+
+  std::string serialized_report;
+
+  if (!GetHPKPReport(host_port_pair, pkp_state, served_certificate_chain,
+                     validated_certificate_chain, &serialized_report)) {
+    return false;
+  }
+
+  report_sender->Send(pkp_state.report_uri, serialized_report);
+
+  return false;
+}
+
 std::string HashesToBase64String(const HashValueVector& hashes) {
   std::string str;
   for (size_t i = 0; i != hashes.size(); ++i) {
     if (i != 0)
       str += ",";
     str += hashes[i].ToString();
   }
   return str;
 }
 
@@ skipping 718 matching lines @@
 void TransportSecurityState::AddHPKP(const std::string& host,
                                      const base::Time& expiry,
                                      bool include_subdomains,
                                      const HashValueVector& hashes,
                                      const GURL& report_uri) {
   DCHECK(CalledOnValidThread());
   AddHPKPInternal(host, base::Time::Now(), expiry, include_subdomains, hashes,
                   report_uri);
 }
 
+bool TransportSecurityState::ProcessHPKPReportOnlyHeader(
+    const HostPortPair& host_port_pair,
+    const std::string& value,
+    const SSLInfo& ssl_info) {
+  DCHECK(CalledOnValidThread());
+
+  base::Time now = base::Time::Now();
+  HashValueVector spki_hashes;
+  GURL report_uri;
+  std::string unused_failure_log;
+
+  if (!ParseHPKPReportOnlyHeader(value, &spki_hashes, &report_uri) ||
+      !report_uri.is_valid() || report_uri.is_empty())
+    return false;
+
+  PKPState pkp_state;
+  pkp_state.last_observed = now;
+  pkp_state.include_subdomains = false;
+  pkp_state.spki_hashes = spki_hashes;
+  pkp_state.report_uri = report_uri;
+  pkp_state.domain = DNSDomainToString(CanonicalizeHost(host_port_pair.host()));
+
+  // Do not perform pin validation if the cert chains up to a known
+  // root.
+  if (!ssl_info.is_issued_by_known_root)

[Ryan Sleevi, 2015/07/30 01:52:16]

I guess I'm a little nervous to see this check sprinkled through, although it
already exists with CheckPublicKeyPins. I'm just wondering out loud if we can
push this check to a more central place somewhere, just to ensure there's never
any code that *forgets* to do it.

That is, if something called CheckPinsAndMaybeSendReport and didn't check this.

This is mostly academic - that is, I trust the people working on the code and I
trust the review process - just wondering if we can make the check harder to
avoid / more obvious. If you had forgot to put this here, I don't know if I
would have caught it during review, for example.

[estark, 2015/07/31 00:49:44]

Yeah, I agree that it would be have been easy to forget. The only thing I can
think of is to pass it through to CheckPinsAndMaybeSendReport(). We could just
have that be the one place where it's checked, the downside being a lack of
short-circuiting inside CheckPublicKeyPins() (i.e. we wouldn't be able to exit
early in the local-root case). Or we could check it both outside and inside
CheckPinsAndMaybeSendReport(), so CheckPinsAndMaybeSendReport() would be the
final canonical safety check, but callers can check it to do an early-exit if
they want to. The former (just always do the check inside
CheckPinsAndMaybeSendReport() and give up on early-exiting) seems less weird to
me.

+    return true;
+
+  CheckPinsAndMaybeSendReport(
+      host_port_pair, pkp_state, ssl_info.public_key_hashes,
+      ssl_info.unverified_cert.get(), ssl_info.cert.get(), ENABLE_PIN_REPORTS,
+      report_sender_, &unused_failure_log);
+  return true;
+}
+
 // static
 bool TransportSecurityState::IsGooglePinnedProperty(const std::string& host) {
   PreloadResult result;
   return DecodeHSTSPreload(host, &result) && result.has_pins &&
          kPinsets[result.pinset_id].accepted_pins == kGoogleAcceptableCerts;
 }
 
 // static
 void TransportSecurityState::ReportUMAOnPinFailure(const std::string& host) {
   PreloadResult result;
@@ skipping 34 matching lines @@
   PKPState pkp_state;
   STSState unused;
 
   if (!GetDynamicPKPState(host_port_pair.host(), &pkp_state) &&
       !GetStaticDomainState(host_port_pair.host(), &unused, &pkp_state)) {
     // HasPublicKeyPins should have returned true in order for this method
     // to have been called, so if we fall through to here, it's an error.
     return false;
   }
 
-  if (pkp_state.CheckPublicKeyPins(hashes, failure_log))
-    return true;
-
-  if (!report_sender_ || report_status != ENABLE_PIN_REPORTS ||
-      pkp_state.report_uri.is_empty()) {
-    return false;
-  }
-
-  DCHECK(pkp_state.report_uri.is_valid());
-
-  std::string serialized_report;
-
-  if (!GetHPKPReport(host_port_pair, pkp_state, served_certificate_chain,
-                     validated_certificate_chain, &serialized_report)) {
-    return false;
-  }
-
-  report_sender_->Send(pkp_state.report_uri, serialized_report);
-
-  return false;
+  return CheckPinsAndMaybeSendReport(
+      host_port_pair, pkp_state, hashes, served_certificate_chain,
+      validated_certificate_chain, report_status, report_sender_, failure_log);
 }
 
 bool TransportSecurityState::GetStaticDomainState(const std::string& host,
                                                   STSState* sts_state,
                                                   PKPState* pkp_state) const {
   DCHECK(CalledOnValidThread());
 
   sts_state->upgrade_mode = STSState::MODE_FORCE_HTTPS;
   sts_state->include_subdomains = false;
   pkp_state->include_subdomains = false;
@@ skipping 215 matching lines @@
 TransportSecurityState::PKPStateIterator::PKPStateIterator(
     const TransportSecurityState& state)
     : iterator_(state.enabled_pkp_hosts_.begin()),
       end_(state.enabled_pkp_hosts_.end()) {
 }
 
 TransportSecurityState::PKPStateIterator::~PKPStateIterator() {
 }
 
 }  // namespace