Return enum from TransportSecurityState::CheckPublicKeyPins

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"
 
 #include <algorithm>
 #include <memory>
 #include <utility>
 
@@ skipping 638 matching lines @@
   STSState static_sts_state;
   PKPState unused;
   if (GetStaticDomainState(host, &static_sts_state, &unused) &&
       static_sts_state.ShouldUpgradeToSSL()) {
     return true;
   }
 
   return false;
 }
 
-bool TransportSecurityState::CheckPublicKeyPins(
+TransportSecurityState::PKPStatus TransportSecurityState::CheckPublicKeyPins(
     const HostPortPair& host_port_pair,
     bool is_issued_by_known_root,
     const HashValueVector& public_key_hashes,
     const X509Certificate* served_certificate_chain,
     const X509Certificate* validated_certificate_chain,
     const PublicKeyPinReportStatus report_status,
     std::string* pinning_failure_log) {
   // Perform pin validation only if the server actually has public key pins.
   if (!HasPublicKeyPins(host_port_pair.host())) {
-    return true;
+    return PKPStatus::OK;
   }
 
-  bool pins_are_valid = CheckPublicKeyPinsImpl(
+  PKPStatus pin_validity = CheckPublicKeyPinsImpl(
       host_port_pair, is_issued_by_known_root, public_key_hashes,
       served_certificate_chain, validated_certificate_chain, report_status,
       pinning_failure_log);
+
   // Don't track statistics when a local trust anchor would override the pinning
   // anyway.
   if (!is_issued_by_known_root)
-    return pins_are_valid;
+    return pin_validity;
 
-  if (!pins_are_valid) {
+  if (pin_validity == PKPStatus::VIOLATED) {
     LOG(ERROR) << *pinning_failure_log;
     ReportUMAOnPinFailure(host_port_pair.host());
   }
-
-  UMA_HISTOGRAM_BOOLEAN("Net.PublicKeyPinSuccess", pins_are_valid);
-  return pins_are_valid;
+  UMA_HISTOGRAM_BOOLEAN("Net.PublicKeyPinSuccess",
+                        pin_validity == PKPStatus::OK);
+  return pin_validity;
 }
 
 bool TransportSecurityState::HasPublicKeyPins(const std::string& host) {
   PKPState dynamic_state;
   if (GetDynamicPKPState(host, &dynamic_state))
     return dynamic_state.HasPublicKeyPins();
 
   STSState unused;
   PKPState static_pkp_state;
   if (GetStaticDomainState(host, &unused, &static_pkp_state)) {
@@ skipping 101 matching lines @@
 
     enabled_pkp_hosts_[HashHost(canonicalized_host)] = pkp_state;
   } else {
     const std::string hashed_host = HashHost(canonicalized_host);
     enabled_pkp_hosts_.erase(hashed_host);
   }
 
   DirtyNotify();
 }
 
-bool TransportSecurityState::CheckPinsAndMaybeSendReport(
+TransportSecurityState::PKPStatus
+TransportSecurityState::CheckPinsAndMaybeSendReport(
     const HostPortPair& host_port_pair,
     bool is_issued_by_known_root,
     const TransportSecurityState::PKPState& pkp_state,
     const HashValueVector& hashes,
     const X509Certificate* served_certificate_chain,
     const X509Certificate* validated_certificate_chain,
     const TransportSecurityState::PublicKeyPinReportStatus report_status,
     std::string* failure_log) {
   if (pkp_state.CheckPublicKeyPins(hashes, failure_log))
-    return true;
+    return PKPStatus::OK;
 
-  if (!report_sender_ || !is_issued_by_known_root ||
+  // Don't report violations for certificates that chain to local roots.
+  if (!is_issued_by_known_root)
+    return PKPStatus::BYPASSED;
+
+  if (!report_sender_ ||
       report_status != TransportSecurityState::ENABLE_PIN_REPORTS ||
       pkp_state.report_uri.is_empty()) {
-    return false;
+    return PKPStatus::VIOLATED;
   }
 
   DCHECK(pkp_state.report_uri.is_valid());
   // Report URIs should not be used if they are the same host as the pin
   // and are HTTPS, to avoid going into a report-sending loop.
   if (!IsReportUriValidForHost(pkp_state.report_uri, host_port_pair.host()))
-    return false;
+    return PKPStatus::VIOLATED;
 
   std::string serialized_report;
   std::string report_cache_key;
   if (!GetHPKPReport(host_port_pair, pkp_state, served_certificate_chain,
                      validated_certificate_chain, &serialized_report,
                      &report_cache_key)) {
-    return false;
+    return PKPStatus::VIOLATED;
   }
 
   // Limit the rate at which duplicate reports are sent to the same
   // report URI. The same report will not be sent within
   // |kTimeToRememberHPKPReportsMins|, which reduces load on servers and
   // also prevents accidental loops (a.com triggers a report to b.com
   // which triggers a report to a.com). See section 2.1.4 of RFC 7469.
   if (sent_reports_cache_.Get(report_cache_key, base::TimeTicks::Now()))
-    return false;
+    return PKPStatus::VIOLATED;
   sent_reports_cache_.Put(
       report_cache_key, true, base::TimeTicks::Now(),
       base::TimeTicks::Now() +
           base::TimeDelta::FromMinutes(kTimeToRememberHPKPReportsMins));
 
   report_sender_->Send(pkp_state.report_uri, serialized_report);
-  return false;
+  return PKPStatus::VIOLATED;
 }
 
 bool TransportSecurityState::GetStaticExpectCTState(
     const std::string& host,
     ExpectCTState* expect_ct_state) const {
   DCHECK(CalledOnValidThread());
 
   if (!IsBuildTimely())
     return false;
 
@@ skipping 246 matching lines @@
       "Net.PublicKeyPinFailureDomain", result.domain_id);
 }
 
 // static
 bool TransportSecurityState::IsBuildTimely() {
   const base::Time build_time = base::GetBuildTime();
   // We consider built-in information to be timely for 10 weeks.
   return (base::Time::Now() - build_time).InDays() < 70 /* 10 weeks */;
 }
 
-bool TransportSecurityState::CheckPublicKeyPinsImpl(
+TransportSecurityState::PKPStatus
+TransportSecurityState::CheckPublicKeyPinsImpl(
     const HostPortPair& host_port_pair,
     bool is_issued_by_known_root,
     const HashValueVector& hashes,
     const X509Certificate* served_certificate_chain,
     const X509Certificate* validated_certificate_chain,
     const PublicKeyPinReportStatus report_status,
     std::string* failure_log) {
   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;
-  }
+  bool found_state =
+      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 found_state is false, its an error.

[estark, 2016/06/21 18:31:28]

sorry, one more tiny nit: pipes around |found_state| and apostrophe on "its".
Though you could maybe just cut it off at "to have been called" and leave it at
that.

+  DCHECK(found_state);
   return CheckPinsAndMaybeSendReport(
       host_port_pair, is_issued_by_known_root, pkp_state, hashes,
       served_certificate_chain, validated_certificate_chain, report_status,
       failure_log);
 }
 
 bool TransportSecurityState::GetStaticDomainState(const std::string& host,
                                                   STSState* sts_state,
                                                   PKPState* pkp_state) const {
   DCHECK(CalledOnValidThread());
@@ skipping 252 matching lines @@
 TransportSecurityState::PKPStateIterator::PKPStateIterator(
     const TransportSecurityState& state)
     : iterator_(state.enabled_pkp_hosts_.begin()),
       end_(state.enabled_pkp_hosts_.end()) {
 }
 
 TransportSecurityState::PKPStateIterator::~PKPStateIterator() {
 }
 
 }  // namespace