Allow preloaded pins to contain report URIs; remove special-case reporting
This CL processes report URIs in preloaded pins and removes special-case code
for reporting pin violations on Google properties
(FraudulentCertificateReporter and its implementation
ChromeFraudulentCertificateReporter), in favor of a preloaded report
URI.
BUG=445793
Review URL: https://codereview.chromium.org/1267383002
Cr-Original-Commit-Position: refs/heads/master@{#342967}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: db949c345a8c561f45a2351daa06dc9c85671e88
diff --git a/http/transport_security_state.cc b/http/transport_security_state.cc
index 4db4696..724235b 100644
--- a/http/transport_security_state.cc
+++ b/http/transport_security_state.cc
@@ -1032,6 +1032,9 @@
return false;
const Pinset *pinset = &kPinsets[result.pinset_id];
+ if (pinset->report_uri != kNoReportURI)
+ pkp_state->report_uri = GURL(pinset->report_uri);
+
if (pinset->accepted_pins) {
const char* const* sha1_hash = pinset->accepted_pins;
while (*sha1_hash) {
diff --git a/http/transport_security_state_static.h b/http/transport_security_state_static.h
index 0a06a03..40ba72e 100644
--- a/http/transport_security_state_static.h
+++ b/http/transport_security_state_static.h
@@ -551,6 +551,7 @@
kSPKIHash_GeoTrustGlobal,
NULL,
};
+static const char kGoogleReportURI[] = "http://clients3.google.com/cert_upload_json";
static const char* const kTorAcceptableCerts[] = {
kSPKIHash_RapidSSL,
kSPKIHash_DigiCertEVRoot,
@@ -674,7 +675,7 @@
static const struct Pinset kPinsets[] = {
{kTestAcceptableCerts, kNoRejectedPublicKeys, kNoReportURI},
- {kGoogleAcceptableCerts, kNoRejectedPublicKeys, kNoReportURI},
+ {kGoogleAcceptableCerts, kNoRejectedPublicKeys, kGoogleReportURI},
{kTorAcceptableCerts, kNoRejectedPublicKeys, kNoReportURI},
{kTwitterComAcceptableCerts, kNoRejectedPublicKeys, kNoReportURI},
{kTwitterCDNAcceptableCerts, kNoRejectedPublicKeys, kNoReportURI},
diff --git a/http/transport_security_state_static.json b/http/transport_security_state_static.json
index 96e9a56..7add3a8 100644
--- a/http/transport_security_state_static.json
+++ b/http/transport_security_state_static.json
@@ -13,6 +13,8 @@
// static_spki_hashes: (list of strings) the set of allowed SPKIs hashes
// bad_static_spki_hashes: (optional list of strings) the set of forbidden
// SPKIs hashes
+// report_uri: (optional string) the URI to send violation reports to;
+// reports will be in the format defined in RFC 7469
//
// For a given pinset, a certificate is accepted if at least one of the
// "static_spki_hashes" SPKIs is found in the chain and none of the
@@ -41,7 +43,8 @@
"GoogleBackup2048",
"GoogleG2",
"GeoTrustGlobal"
- ]
+ ],
+ "report_uri": "http://clients3.google.com/cert_upload_json"
},
{
"name": "tor",
diff --git a/http/transport_security_state_unittest.cc b/http/transport_security_state_unittest.cc
index 4c63c06..66bbdc4 100644
--- a/http/transport_security_state_unittest.cc
+++ b/http/transport_security_state_unittest.cc
@@ -1414,4 +1414,57 @@
state.ProcessHPKPReportOnlyHeader(header, host_port_pair, ssl_info));
}
+// Tests that pinning violations on preloaded pins trigger reports when
+// the preloaded pin contains a report URI.
+TEST_F(TransportSecurityStateTest, PreloadedPKPReportUri) {
+ const char kPreloadedPinDomain[] = "www.google.com";
+ const uint16_t kPort = 443;
+ HostPortPair host_port_pair(kPreloadedPinDomain, kPort);
+
+ TransportSecurityState state;
+ MockCertificateReportSender mock_report_sender;
+ state.SetReportSender(&mock_report_sender);
+
+ ASSERT_TRUE(
+ TransportSecurityState::IsGooglePinnedProperty(kPreloadedPinDomain));
+ EnableStaticPins(&state);
+
+ TransportSecurityState::PKPState pkp_state;
+ TransportSecurityState::STSState unused_sts_state;
+ ASSERT_TRUE(state.GetStaticDomainState(kPreloadedPinDomain, &unused_sts_state,
+ &pkp_state));
+
+ GURL report_uri = pkp_state.report_uri;
+ ASSERT_TRUE(report_uri.is_valid());
+ ASSERT_FALSE(report_uri.is_empty());
+
+ // Two dummy certs to use as the server-sent and validated chains. The
+ // contents don't matter, as long as they are not the real google.com
+ // certs in the pins.
+ scoped_refptr<X509Certificate> cert1 =
+ ImportCertFromFile(GetTestCertsDirectory(), "test_mail_google_com.pem");
+ scoped_refptr<X509Certificate> cert2 =
+ ImportCertFromFile(GetTestCertsDirectory(), "expired_cert.pem");
+ ASSERT_TRUE(cert1);
+ ASSERT_TRUE(cert2);
+
+ HashValueVector bad_hashes;
+ for (size_t i = 0; kBadPath[i]; i++)
+ EXPECT_TRUE(AddHash(kBadPath[i], &bad_hashes));
+
+ // 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(report_uri, mock_report_sender.latest_report_uri());
+
+ std::string report = mock_report_sender.latest_report();
+ ASSERT_FALSE(report.empty());
+ ASSERT_NO_FATAL_FAILURE(CheckHPKPReport(
+ report, host_port_pair, pkp_state.include_subdomains, pkp_state.domain,
+ cert1.get(), cert2.get(), pkp_state.spki_hashes));
+}
+
} // namespace net
diff --git a/net.gypi b/net.gypi
index 3fabe95..4e5b41f 100644
--- a/net.gypi
+++ b/net.gypi
@@ -1184,7 +1184,6 @@
'url_request/data_protocol_handler.h',
'url_request/file_protocol_handler.cc',
'url_request/file_protocol_handler.h',
- 'url_request/fraudulent_certificate_reporter.h',
'url_request/ftp_protocol_handler.cc',
'url_request/ftp_protocol_handler.h',
'url_request/http_user_agent_settings.h',
diff --git a/url_request/certificate_report_sender.cc b/url_request/certificate_report_sender.cc
index fd5d11c..0ff637b 100644
--- a/url_request/certificate_report_sender.cc
+++ b/url_request/certificate_report_sender.cc
@@ -28,7 +28,15 @@
void CertificateReportSender::Send(const GURL& report_uri,
const std::string& report) {
scoped_ptr<URLRequest> url_request =
- CreateURLRequest(request_context_, report_uri);
+ request_context_->CreateRequest(report_uri, DEFAULT_PRIORITY, this);
+
+ int load_flags =
+ LOAD_BYPASS_CACHE | LOAD_DISABLE_CACHE | LOAD_DO_NOT_SEND_AUTH_DATA;
+ if (cookies_preference_ != SEND_COOKIES) {
+ load_flags |= LOAD_DO_NOT_SEND_COOKIES | LOAD_DO_NOT_SAVE_COOKIES;
+ }
+ url_request->SetLoadFlags(load_flags);
+
url_request->set_method("POST");
scoped_ptr<UploadElementReader> reader(
@@ -56,19 +64,4 @@
NOTREACHED();
}
-scoped_ptr<URLRequest> CertificateReportSender::CreateURLRequest(
- URLRequestContext* context,
- const GURL& report_uri) {
- scoped_ptr<URLRequest> request =
- context->CreateRequest(report_uri, DEFAULT_PRIORITY, this);
- int load_flags =
- LOAD_BYPASS_CACHE | LOAD_DISABLE_CACHE | LOAD_DO_NOT_SEND_AUTH_DATA;
- if (cookies_preference_ != SEND_COOKIES) {
- load_flags =
- load_flags | LOAD_DO_NOT_SEND_COOKIES | LOAD_DO_NOT_SAVE_COOKIES;
- }
- request->SetLoadFlags(load_flags);
- return request.Pass();
-}
-
} // namespace net
diff --git a/url_request/certificate_report_sender.h b/url_request/certificate_report_sender.h
index dac8a57..0f247c3 100644
--- a/url_request/certificate_report_sender.h
+++ b/url_request/certificate_report_sender.h
@@ -49,15 +49,6 @@
void OnReadCompleted(URLRequest* request, int bytes_read) override;
private:
- // Creates a URLRequest with which to send a certificate report to the
- // server.
- //
- // TODO(estark): inline this into Send() once
- // ChromeFraudulentCertificateReporter goes away.
- virtual scoped_ptr<URLRequest> CreateURLRequest(
- net::URLRequestContext* context,
- const GURL& report_uri);
-
net::URLRequestContext* const request_context_;
CookiesPreference cookies_preference_;
diff --git a/url_request/fraudulent_certificate_reporter.h b/url_request/fraudulent_certificate_reporter.h
deleted file mode 100644
index 8d5d60a..0000000
--- a/url_request/fraudulent_certificate_reporter.h
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright (c) 2011 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.
-
-#ifndef NET_URL_REQUEST_FRAUDULENT_CERTIFICATE_REPORTER_H_
-#define NET_URL_REQUEST_FRAUDULENT_CERTIFICATE_REPORTER_H_
-
-#include <string>
-
-namespace net {
-
-class SSLInfo;
-
-// FraudulentCertificateReporter is an interface for asynchronously
-// reporting certificate chains that fail the certificate pinning
-// check.
-class FraudulentCertificateReporter {
- public:
- virtual ~FraudulentCertificateReporter() {}
-
- // Sends a report to the report collection server containing the |ssl_info|
- // associated with a connection to |hostname|.
- virtual void SendReport(const std::string& hostname,
- const SSLInfo& ssl_info) = 0;
-};
-
-} // namespace net
-
-#endif // NET_URL_REQUEST_FRAUDULENT_CERTIFICATE_REPORTER_H_
-
diff --git a/url_request/url_request_context.cc b/url_request/url_request_context.cc
index fa0db79..b83cf02 100644
--- a/url_request/url_request_context.cc
+++ b/url_request/url_request_context.cc
@@ -21,7 +21,6 @@
host_resolver_(nullptr),
cert_verifier_(nullptr),
channel_id_service_(nullptr),
- fraudulent_certificate_reporter_(nullptr),
http_auth_handler_factory_(nullptr),
proxy_service_(nullptr),
network_delegate_(nullptr),
@@ -47,7 +46,6 @@
set_host_resolver(other->host_resolver_);
set_cert_verifier(other->cert_verifier_);
set_channel_id_service(other->channel_id_service_);
- set_fraudulent_certificate_reporter(other->fraudulent_certificate_reporter_);
set_http_auth_handler_factory(other->http_auth_handler_factory_);
set_proxy_service(other->proxy_service_);
set_ssl_config_service(other->ssl_config_service_.get());
diff --git a/url_request/url_request_context.h b/url_request/url_request_context.h
index a50225c..b9f1276 100644
--- a/url_request/url_request_context.h
+++ b/url_request/url_request_context.h
@@ -31,7 +31,6 @@
class ChannelIDService;
class CookieStore;
class CTVerifier;
-class FraudulentCertificateReporter;
class HostResolver;
class HttpAuthHandlerFactory;
class HttpTransactionFactory;
@@ -99,14 +98,6 @@
channel_id_service_ = channel_id_service;
}
- FraudulentCertificateReporter* fraudulent_certificate_reporter() const {
- return fraudulent_certificate_reporter_;
- }
- void set_fraudulent_certificate_reporter(
- FraudulentCertificateReporter* fraudulent_certificate_reporter) {
- fraudulent_certificate_reporter_ = fraudulent_certificate_reporter;
- }
-
// Get the proxy service for this context.
ProxyService* proxy_service() const { return proxy_service_; }
void set_proxy_service(ProxyService* proxy_service) {
@@ -239,7 +230,6 @@
HostResolver* host_resolver_;
CertVerifier* cert_verifier_;
ChannelIDService* channel_id_service_;
- FraudulentCertificateReporter* fraudulent_certificate_reporter_;
HttpAuthHandlerFactory* http_auth_handler_factory_;
ProxyService* proxy_service_;
scoped_refptr<SSLConfigService> ssl_config_service_;
diff --git a/url_request/url_request_context_storage.cc b/url_request/url_request_context_storage.cc
index e1452be..b334632 100644
--- a/url_request/url_request_context_storage.cc
+++ b/url_request/url_request_context_storage.cc
@@ -17,7 +17,6 @@
#include "net/log/net_log.h"
#include "net/proxy/proxy_service.h"
#include "net/ssl/channel_id_service.h"
-#include "net/url_request/fraudulent_certificate_reporter.h"
#include "net/url_request/http_user_agent_settings.h"
#include "net/url_request/url_request_backoff_manager.h"
#include "net/url_request/url_request_context.h"
@@ -55,13 +54,6 @@
channel_id_service_ = channel_id_service.Pass();
}
-void URLRequestContextStorage::set_fraudulent_certificate_reporter(
- FraudulentCertificateReporter* fraudulent_certificate_reporter) {
- context_->set_fraudulent_certificate_reporter(
- fraudulent_certificate_reporter);
- fraudulent_certificate_reporter_.reset(fraudulent_certificate_reporter);
-}
-
void URLRequestContextStorage::set_http_auth_handler_factory(
HttpAuthHandlerFactory* http_auth_handler_factory) {
context_->set_http_auth_handler_factory(http_auth_handler_factory);
diff --git a/url_request/url_request_context_storage.h b/url_request/url_request_context_storage.h
index a37e30a..fd3c4a2 100644
--- a/url_request/url_request_context_storage.h
+++ b/url_request/url_request_context_storage.h
@@ -15,7 +15,6 @@
class CertVerifier;
class ChannelIDService;
class CookieStore;
-class FraudulentCertificateReporter;
class FtpTransactionFactory;
class HostResolver;
class HttpAuthHandlerFactory;
@@ -50,8 +49,6 @@
void set_host_resolver(scoped_ptr<HostResolver> host_resolver);
void set_cert_verifier(CertVerifier* cert_verifier);
void set_channel_id_service(scoped_ptr<ChannelIDService> channel_id_service);
- void set_fraudulent_certificate_reporter(
- FraudulentCertificateReporter* fraudulent_certificate_reporter);
void set_http_auth_handler_factory(
HttpAuthHandlerFactory* http_auth_handler_factory);
void set_proxy_service(ProxyService* proxy_service);
@@ -83,7 +80,6 @@
scoped_ptr<CertVerifier> cert_verifier_;
// The ChannelIDService must outlive the HttpTransactionFactory.
scoped_ptr<ChannelIDService> channel_id_service_;
- scoped_ptr<FraudulentCertificateReporter> fraudulent_certificate_reporter_;
scoped_ptr<HttpAuthHandlerFactory> http_auth_handler_factory_;
scoped_ptr<ProxyService> proxy_service_;
// TODO(willchan): Remove refcounting on these members.
diff --git a/url_request/url_request_http_job.cc b/url_request/url_request_http_job.cc
index 2e55ca5..0a6632e 100644
--- a/url_request/url_request_http_job.cc
+++ b/url_request/url_request_http_job.cc
@@ -42,7 +42,6 @@
#include "net/proxy/proxy_info.h"
#include "net/ssl/ssl_cert_request_info.h"
#include "net/ssl/ssl_config_service.h"
-#include "net/url_request/fraudulent_certificate_reporter.h"
#include "net/url_request/http_user_agent_settings.h"
#include "net/url_request/url_request.h"
#include "net/url_request/url_request_backoff_manager.h"
@@ -919,18 +918,6 @@
const URLRequestContext* context = request_->context();
- if (result == ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN &&
- transaction_->GetResponseInfo() != NULL) {
- FraudulentCertificateReporter* reporter =
- context->fraudulent_certificate_reporter();
- if (reporter != NULL) {
- const SSLInfo& ssl_info = transaction_->GetResponseInfo()->ssl_info;
- const std::string& host = request_->url().host();
-
- reporter->SendReport(host, ssl_info);
- }
- }
-
if (result == OK) {
if (transaction_ && transaction_->GetResponseInfo()) {
SetProxyServer(transaction_->GetResponseInfo()->proxy_server);
