Return enum from TransportSecurityState::CheckPublicKeyPins
BUG=618775
Review-Url: https://codereview.chromium.org/2066603004
Cr-Commit-Position: refs/heads/master@{#401140}
diff --git a/net/http/http_security_headers_unittest.cc b/net/http/http_security_headers_unittest.cc
index 4c7bff3..e4904dd 100644
--- a/net/http/http_security_headers_unittest.cc
+++ b/net/http/http_security_headers_unittest.cc
@@ -710,9 +710,10 @@
std::string failure_log;
const bool is_issued_by_known_root = true;
HostPortPair domain_port(domain, 443);
- EXPECT_TRUE(state.CheckPublicKeyPins(
- domain_port, is_issued_by_known_root, hashes, nullptr, nullptr,
- TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::OK,
+ state.CheckPublicKeyPins(
+ domain_port, is_issued_by_known_root, hashes, nullptr, nullptr,
+ TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
TransportSecurityState::PKPState new_dynamic_pkp_state;
EXPECT_TRUE(state.GetDynamicPKPState(domain, &new_dynamic_pkp_state));
@@ -795,10 +796,11 @@
const bool is_issued_by_known_root = true;
HostPortPair domain_port(domain, 443);
- EXPECT_FALSE(state.CheckPublicKeyPins(
- domain_port, is_issued_by_known_root, new_static_pkp_state2.spki_hashes,
- nullptr, nullptr, TransportSecurityState::DISABLE_PIN_REPORTS,
- &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ domain_port, is_issued_by_known_root,
+ new_static_pkp_state2.spki_hashes, nullptr, nullptr,
+ TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
EXPECT_NE(0UL, failure_log.length());
}
@@ -831,9 +833,11 @@
std::string failure_log;
const bool is_issued_by_known_root = true;
HostPortPair domain_port(domain, 443);
- EXPECT_TRUE(state.CheckPublicKeyPins(
- domain_port, is_issued_by_known_root, saved_hashes, nullptr, nullptr,
- TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(
+ TransportSecurityState::PKPStatus::OK,
+ state.CheckPublicKeyPins(
+ domain_port, is_issued_by_known_root, saved_hashes, nullptr, nullptr,
+ TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
// Add an HPKP header, which should only update the dynamic state.
HashValue good_hash = GetTestHashValue(1, HASH_VALUE_SHA256);
@@ -853,9 +857,11 @@
EXPECT_TRUE(state.ShouldUpgradeToSSL(domain));
// The dynamic pins, which do not match |saved_hashes|, should take
// precedence over the static pins and cause the check to fail.
- EXPECT_FALSE(state.CheckPublicKeyPins(
- domain_port, is_issued_by_known_root, saved_hashes, nullptr, nullptr,
- TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(
+ TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ domain_port, is_issued_by_known_root, saved_hashes, nullptr, nullptr,
+ TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
}
// Tests that seeing an invalid HPKP header leaves the existing one alone.
@@ -880,9 +886,11 @@
std::string failure_log;
bool is_issued_by_known_root = true;
HostPortPair domain_port("example.com", 443);
- EXPECT_TRUE(state.CheckPublicKeyPins(
- domain_port, is_issued_by_known_root, ssl_info.public_key_hashes, nullptr,
- nullptr, TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::OK,
+ state.CheckPublicKeyPins(
+ domain_port, is_issued_by_known_root,
+ ssl_info.public_key_hashes, nullptr, nullptr,
+ TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
// Now assert an invalid one. This should fail.
EXPECT_FALSE(state.AddHPKPHeader(
@@ -891,9 +899,11 @@
// The old pins must still exist.
EXPECT_TRUE(state.HasPublicKeyPins("example.com"));
- EXPECT_TRUE(state.CheckPublicKeyPins(
- domain_port, is_issued_by_known_root, ssl_info.public_key_hashes, nullptr,
- nullptr, TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::OK,
+ state.CheckPublicKeyPins(
+ domain_port, is_issued_by_known_root,
+ ssl_info.public_key_hashes, nullptr, nullptr,
+ TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
}
}; // namespace net
diff --git a/net/http/transport_security_state.cc b/net/http/transport_security_state.cc
index 6ee15b0c..1348528 100644
--- a/net/http/transport_security_state.cc
+++ b/net/http/transport_security_state.cc
@@ -656,7 +656,7 @@
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,
@@ -666,25 +666,26 @@
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) {
@@ -806,7 +807,8 @@
DirtyNotify();
}
-bool TransportSecurityState::CheckPinsAndMaybeSendReport(
+TransportSecurityState::PKPStatus
+TransportSecurityState::CheckPinsAndMaybeSendReport(
const HostPortPair& host_port_pair,
bool is_issued_by_known_root,
const TransportSecurityState::PKPState& pkp_state,
@@ -816,26 +818,30 @@
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
@@ -844,14 +850,14 @@
// 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(
@@ -1118,7 +1124,8 @@
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,
@@ -1129,13 +1136,13 @@
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.
+ DCHECK(found_state);
return CheckPinsAndMaybeSendReport(
host_port_pair, is_issued_by_known_root, pkp_state, hashes,
served_certificate_chain, validated_certificate_chain, report_status,
diff --git a/net/http/transport_security_state.h b/net/http/transport_security_state.h
index aa92b43..168e879 100644
--- a/net/http/transport_security_state.h
+++ b/net/http/transport_security_state.h
@@ -99,6 +99,20 @@
std::map<std::string, STSState>::const_iterator end_;
};
+ // PKPStatus describes the result of a pinning check.
+ enum class PKPStatus {
+ // Pinning was enabled and the necessary pins were not present.
+ VIOLATED,
+
+ // Pinning was not enabled, or pinning was enabled and the certificate
+ // satisfied the pins.
+ OK,
+
+ // Pinning was enabled and the certificate did not satisfy the pins, but the
+ // violation was ignored due to local policy, such as a local trust anchor.
+ BYPASSED,
+ };
+
// A PKPState describes the public key pinning state.
class NET_EXPORT PKPState {
public:
@@ -247,13 +261,14 @@
// when is_issued_by_known_root is false.
bool ShouldSSLErrorsBeFatal(const std::string& host);
bool ShouldUpgradeToSSL(const std::string& host);
- bool CheckPublicKeyPins(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);
+ PKPStatus CheckPublicKeyPins(
+ 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);
bool HasPublicKeyPins(const std::string& host);
// Assign a |Delegate| for persisting the transport security state. If
@@ -399,7 +414,7 @@
static bool IsBuildTimely();
// Helper method for actually checking pins.
- bool CheckPublicKeyPinsImpl(
+ PKPStatus CheckPublicKeyPinsImpl(
const HostPortPair& host_port_pair,
bool is_issued_by_known_root,
const HashValueVector& hashes,
@@ -440,7 +455,7 @@
// |report_status| says to), this method sends an HPKP violation
// report containing |served_certificate_chain| and
// |validated_certificate_chain|.
- bool CheckPinsAndMaybeSendReport(
+ PKPStatus CheckPinsAndMaybeSendReport(
const HostPortPair& host_port_pair,
bool is_issued_by_known_root,
const TransportSecurityState::PKPState& pkp_state,
diff --git a/net/http/transport_security_state_unittest.cc b/net/http/transport_security_state_unittest.cc
index 009c7d6..e5c1e63 100644
--- a/net/http/transport_security_state_unittest.cc
+++ b/net/http/transport_security_state_unittest.cc
@@ -1270,44 +1270,49 @@
EXPECT_EQ(std::string(), mock_report_sender.latest_report());
std::string failure_log;
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log));
// No report should have been sent because of the DISABLE_PIN_REPORTS
// argument.
EXPECT_EQ(GURL(), mock_report_sender.latest_report_uri());
EXPECT_EQ(std::string(), mock_report_sender.latest_report());
- EXPECT_TRUE(state.CheckPublicKeyPins(
- host_port_pair, true, good_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::OK,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, good_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
// No report should have been sent because there was no violation.
EXPECT_EQ(GURL(), mock_report_sender.latest_report_uri());
EXPECT_EQ(std::string(), mock_report_sender.latest_report());
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, false, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::BYPASSED,
+ state.CheckPublicKeyPins(
+ host_port_pair, false, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
// No report should have been sent because the certificate chained to a
// non-public root.
EXPECT_EQ(GURL(), mock_report_sender.latest_report_uri());
EXPECT_EQ(std::string(), mock_report_sender.latest_report());
- EXPECT_TRUE(state.CheckPublicKeyPins(
- host_port_pair, false, good_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::OK,
+ state.CheckPublicKeyPins(
+ host_port_pair, false, good_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
// No report should have been sent because there was no violation, even though
// the certificate chained to a local trust anchor.
EXPECT_EQ(GURL(), mock_report_sender.latest_report_uri());
EXPECT_EQ(std::string(), mock_report_sender.latest_report());
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
// Now a report should have been sent. Check that it contains the
// right information.
@@ -1318,9 +1323,11 @@
cert1.get(), cert2.get(),
good_hashes));
mock_report_sender.Clear();
- EXPECT_FALSE(state.CheckPublicKeyPins(
- subdomain_host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(subdomain_host_port_pair, true, bad_hashes,
+ cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS,
+ &failure_log));
// Now a report should have been sent for the subdomain. Check that it
// contains the right information.
@@ -1367,9 +1374,10 @@
state.AddHPKP(kHost, expiry, true, good_hashes, report_uri);
std::string failure_log;
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
// Check that the UMA histogram was updated when the report failed to
// send.
@@ -1553,9 +1561,10 @@
// Trigger a violation and check that it sends a report.
std::string failure_log;
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
EXPECT_EQ(report_uri, mock_report_sender.latest_report_uri());
@@ -1601,17 +1610,19 @@
// Trigger a violation and check that it does not send a report
// because the report-uri is HTTPS and same-host as the pins.
std::string failure_log;
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
EXPECT_TRUE(mock_report_sender.latest_report_uri().is_empty());
// An HTTP report uri to the same host should be okay.
state.AddHPKP("example.test", expiry, true, good_hashes, http_report_uri);
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
EXPECT_EQ(http_report_uri, mock_report_sender.latest_report_uri());
}
@@ -1649,9 +1660,10 @@
EXPECT_EQ(std::string(), mock_report_sender.latest_report());
std::string failure_log;
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
// A report should have been sent. Check that it contains the
// right information.
@@ -1665,9 +1677,10 @@
// Now trigger the same violation; a duplicative report should not be
// sent.
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
EXPECT_EQ(GURL(), mock_report_sender.latest_report_uri());
EXPECT_EQ(std::string(), mock_report_sender.latest_report());
@@ -1675,9 +1688,10 @@
// should be sent.
GURL report_uri2("http://report-example2.test/test");
state.AddHPKP(kHost, expiry, true, good_hashes, report_uri2);
- EXPECT_FALSE(state.CheckPublicKeyPins(
- host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
+ EXPECT_EQ(TransportSecurityState::PKPStatus::VIOLATED,
+ state.CheckPublicKeyPins(
+ host_port_pair, true, bad_hashes, cert1.get(), cert2.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &failure_log));
EXPECT_EQ(report_uri2, mock_report_sender.latest_report_uri());
report = mock_report_sender.latest_report();
ASSERT_FALSE(report.empty());
diff --git a/net/quic/crypto/proof_verifier_chromium.cc b/net/quic/crypto/proof_verifier_chromium.cc
index dde1503..4598150 100644
--- a/net/quic/crypto/proof_verifier_chromium.cc
+++ b/net/quic/crypto/proof_verifier_chromium.cc
@@ -339,21 +339,29 @@
verify_details_->ct_verify_result.verified_scts, net_log_);
}
- if ((result == OK ||
- (IsCertificateError(result) && IsCertStatusMinorError(cert_status))) &&
- !transport_security_state_->CheckPublicKeyPins(
- HostPortPair(hostname_, port_),
- cert_verify_result.is_issued_by_known_root,
- cert_verify_result.public_key_hashes, cert_.get(),
- cert_verify_result.verified_cert.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS,
- &verify_details_->pinning_failure_log)) {
- if (cert_verify_result.is_issued_by_known_root) {
- verify_details_->cert_verify_result.cert_status |=
- CERT_STATUS_PINNED_KEY_MISSING;
- result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
- } else {
- verify_details_->pkp_bypassed = true;
+ if (transport_security_state_ &&
+ (result == OK ||
+ (IsCertificateError(result) && IsCertStatusMinorError(cert_status)))) {
+ TransportSecurityState::PKPStatus pin_validity =
+ transport_security_state_->CheckPublicKeyPins(
+ HostPortPair(hostname_, port_),
+ cert_verify_result.is_issued_by_known_root,
+ cert_verify_result.public_key_hashes, cert_.get(),
+ cert_verify_result.verified_cert.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS,
+ &verify_details_->pinning_failure_log);
+ switch (pin_validity) {
+ case TransportSecurityState::PKPStatus::VIOLATED:
+ result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
+ verify_details_->cert_verify_result.cert_status |=
+ CERT_STATUS_PINNED_KEY_MISSING;
+ break;
+ case TransportSecurityState::PKPStatus::BYPASSED:
+ verify_details_->pkp_bypassed = true;
+ // Fall through.
+ case TransportSecurityState::PKPStatus::OK:
+ // Do nothing.
+ break;
}
}
diff --git a/net/socket/ssl_client_socket_impl.cc b/net/socket/ssl_client_socket_impl.cc
index 60a8fae4..01acc54 100644
--- a/net/socket/ssl_client_socket_impl.cc
+++ b/net/socket/ssl_client_socket_impl.cc
@@ -1328,18 +1328,27 @@
}
const CertStatus cert_status = server_cert_verify_result_.cert_status;
- if ((result == OK ||
- (IsCertificateError(result) && IsCertStatusMinorError(cert_status))) &&
- !transport_security_state_->CheckPublicKeyPins(
- host_and_port_, server_cert_verify_result_.is_issued_by_known_root,
- server_cert_verify_result_.public_key_hashes, server_cert_.get(),
- server_cert_verify_result_.verified_cert.get(),
- TransportSecurityState::ENABLE_PIN_REPORTS, &pinning_failure_log_)) {
- if (server_cert_verify_result_.is_issued_by_known_root) {
- server_cert_verify_result_.cert_status |= CERT_STATUS_PINNED_KEY_MISSING;
- result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
- } else {
- pkp_bypassed_ = true;
+ if (transport_security_state_ &&
+ (result == OK ||
+ (IsCertificateError(result) && IsCertStatusMinorError(cert_status)))) {
+ TransportSecurityState::PKPStatus pin_validity =
+ transport_security_state_->CheckPublicKeyPins(
+ host_and_port_, server_cert_verify_result_.is_issued_by_known_root,
+ server_cert_verify_result_.public_key_hashes, server_cert_.get(),
+ server_cert_verify_result_.verified_cert.get(),
+ TransportSecurityState::ENABLE_PIN_REPORTS, &pinning_failure_log_);
+ switch (pin_validity) {
+ case TransportSecurityState::PKPStatus::VIOLATED:
+ server_cert_verify_result_.cert_status |=
+ CERT_STATUS_PINNED_KEY_MISSING;
+ result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
+ break;
+ case TransportSecurityState::PKPStatus::BYPASSED:
+ pkp_bypassed_ = true;
+ // Fall through.
+ case TransportSecurityState::PKPStatus::OK:
+ // Do nothing.
+ break;
}
}
diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc
index 6330774..48758ea 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -3316,6 +3316,7 @@
EXPECT_TRUE(sock_->IsConnected());
EXPECT_TRUE(ssl_info.pkp_bypassed);
+ EXPECT_FALSE(ssl_info.cert_status & CERT_STATUS_PINNED_KEY_MISSING);
}
TEST_F(SSLClientSocketTest, PKPEnforced) {
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 5b4530cc..fc1d94f 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -658,12 +658,12 @@
// DISABLE_PIN_REPORTS is set here because this check can fail in
// normal operation without being indicative of a misconfiguration or
// attack. Port is left at 0 as it is never used.
- if (ssl_info.is_issued_by_known_root &&
- !transport_security_state->CheckPublicKeyPins(
+ if (transport_security_state->CheckPublicKeyPins(
HostPortPair(new_hostname, 0), ssl_info.is_issued_by_known_root,
ssl_info.public_key_hashes, ssl_info.unverified_cert.get(),
ssl_info.cert.get(), TransportSecurityState::DISABLE_PIN_REPORTS,
- &pinning_failure_log)) {
+ &pinning_failure_log) ==
+ TransportSecurityState::PKPStatus::VIOLATED) {
return false;
}
