Add URLRequest unit tests for HPKP violation reports

net/url_request/url_request_unittest.cc

Index: net/url_request/url_request_unittest.cc
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index ecff043877c54d88952a34bbccdcfb1eb4b96ee5..ef4aa76dd8a12890be41de7c3d7a2cf6dbe66bc1 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -20,6 +20,7 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/format_macros.h"
+#include "base/json/json_reader.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -35,6 +36,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/histogram_tester.h"
 #include "base/thread_task_runner_handle.h"
+#include "base/values.h"
 #include "net/base/chunked_upload_data_stream.h"
 #include "net/base/elements_upload_data_stream.h"
 #include "net/base/load_flags.h"
@@ -116,6 +118,7 @@ namespace {
 const base::string16 kChrome(ASCIIToUTF16("chrome"));
 const base::string16 kSecret(ASCIIToUTF16("secret"));
 const base::string16 kUser(ASCIIToUTF16("user"));
+const char kHPKPReportUri[] = "https://hpkp-report.test";
 
 const base::FilePath::CharType kTestFilePath[] =
     FILE_PATH_LITERAL("net/data/url_request_unittest");
@@ -618,6 +621,27 @@ class TestURLRequestContextWithProxy : public TestURLRequestContext {
   ~TestURLRequestContextWithProxy() override {}
 };
 
+// A mock ReportSender that just remembers the latest report
+// URI and report to be sent.
+class MockCertificateReportSender
+    : public TransportSecurityState::ReportSender {
+ public:
+  MockCertificateReportSender() {}
+  ~MockCertificateReportSender() override {}
+
+  void Send(const GURL& report_uri, const std::string& report) override {
+    latest_report_uri_ = report_uri;
+    latest_report_ = report;
+  }
+
+  const GURL& latest_report_uri() { return latest_report_uri_; }
+  const std::string& latest_report() { return latest_report_; }
+
+ private:
+  GURL latest_report_uri_;
+  std::string latest_report_;
+};
+
 }  // namespace
 
 // Inherit PlatformTest since we require the autorelease pool on Mac OS X.
@@ -5361,6 +5385,8 @@ TEST_F(URLRequestTestHTTP, STSNotProcessedOnIP) {
 // PKPState present because header rejected).
 #if defined(OS_ANDROID)

[davidben, 2015/08/05 18:23:51]

Maybe for a separate CL, but I'm pretty sure you can just enable these now.
Stuff has been moved around. I think Android will do HPKP, but not preloaded
pins.

[estark, 2015/08/06 05:50:07]

Ack: filed a bug, crbug.com/517311

 #define MAYBE_ProcessPKP DISABLED_ProcessPKP
+#define MAYBE_ProcessPKPAndSendReport DISABLED_ProcessPKPAndSendReport
+#define MAYBE_ProcessPKPReportOnly DISABLED_ProcessPKPReportOnly
 #else
 #define MAYBE_ProcessPKP ProcessPKP
 #endif
@@ -5368,6 +5394,7 @@ TEST_F(URLRequestTestHTTP, STSNotProcessedOnIP) {
 // Tests that enabling HPKP on a domain does not affect the HSTS
 // validity/expiration.
 TEST_F(URLRequestTestHTTP, MAYBE_ProcessPKP) {
+  GURL report_uri(kHPKPReportUri);
   SpawnedTestServer::SSLOptions ssl_options(
       SpawnedTestServer::SSLOptions::CERT_COMMON_NAME_IS_DOMAIN);
   SpawnedTestServer https_test_server(SpawnedTestServer::TYPE_HTTPS,
@@ -5397,9 +5424,149 @@ TEST_F(URLRequestTestHTTP, MAYBE_ProcessPKP) {
   EXPECT_FALSE(sts_state.include_subdomains);
   EXPECT_FALSE(pkp_state.include_subdomains);
   EXPECT_TRUE(pkp_state.HasPublicKeyPins());
+  EXPECT_EQ(report_uri, pkp_state.report_uri);
   EXPECT_NE(sts_state.expiry, pkp_state.expiry);
 }
 
+// Tests that reports get sent on HPKP violations when a report-uri is set.
+TEST_F(URLRequestTestHTTP, MAYBE_ProcessPKPAndSendReport) {
+  GURL report_uri(kHPKPReportUri);
+  SpawnedTestServer::SSLOptions ssl_options(
+      SpawnedTestServer::SSLOptions::CERT_COMMON_NAME_IS_DOMAIN);
+  SpawnedTestServer https_test_server(SpawnedTestServer::TYPE_HTTPS,
+                                      ssl_options,
+                                      base::FilePath(kTestFilePath));
+
+  ASSERT_TRUE(https_test_server.Start());
+
+  std::string test_server_hostname = https_test_server.GetURL("").host();
+
+  // Set up a pin for |test_server_hostname|.
+  TransportSecurityState security_state;
+  const base::Time current_time(base::Time::Now());
+  const base::Time expiry = current_time + base::TimeDelta::FromSeconds(1000);
+  HashValueVector hashes;
+  HashValue hash1;
+  HashValue hash2;
+  ASSERT_TRUE(hash1.FromString("sha1/m9lHYJYke9k0GtVZ+bXSQYE8nDI="));
+  ASSERT_TRUE(hash2.FromString("sha1/o5OZxATDsgmwgcIfIWIneMJ0jkw="));

[davidben, 2015/08/05 18:23:51]

These values aren't significant, right? Maybe do something like nab this
function:
https://code.google.com/p/chromium/codesearch#chromium/src/net/http/http_secu...

(Or perhaps just 111[..]111=?)

I was going to ask at first if we could come up with a better story for keeping
the pins and certificates in sync (compute them from the certificate or
something?), but it seems this doesn't matter because the CertVerifier is also
mocked.

[estark, 2015/08/06 05:50:07]

Done.

+  hashes.push_back(hash1);
+  hashes.push_back(hash2);
+  security_state.AddHPKP(test_server_hostname, expiry,
+                         false, /* include subdomains */
+                         hashes, report_uri);
+
+  MockCertificateReportSender mock_report_sender;
+  security_state.SetReportSender(&mock_report_sender);
+
+  // Set up a MockCertVerifier to trigger a violation of the previously
+  // set pin.
+  base::FilePath certificate_file = ssl_options.GetCertificateFile();
+  scoped_refptr<X509Certificate> cert = ImportCertFromFile(
+      GetTestCertsDirectory(), certificate_file.BaseName().value());
+  ASSERT_TRUE(cert);
+
+  MockCertVerifier cert_verifier;
+  CertVerifyResult verify_result;
+  verify_result.verified_cert = cert;
+  verify_result.is_issued_by_known_root = true;
+  HashValue hash3;
+  ASSERT_TRUE(hash3.FromString("sha1/4BjDjn8v2lWeUFQnqSs0BgbIcrU="));
+  verify_result.public_key_hashes.push_back(hash3);
+  cert_verifier.AddResultForCert(cert.get(), verify_result, OK);
+
+  TestNetworkDelegate network_delegate;
+  TestURLRequestContext context(true);
+  context.set_transport_security_state(&security_state);
+  context.set_network_delegate(&network_delegate);
+  context.set_cert_verifier(&cert_verifier);
+  context.Init();
+
+  // Now send a request to trigger the violation.
+  TestDelegate d;
+  scoped_ptr<URLRequest> violating_request(
+      context.CreateRequest(https_test_server.GetURL("files/hpkp-headers.html"),

[davidben, 2015/08/05 18:23:51]

Probably best use simple.html or something, since that file has its own HPKP
headers. (Which get ignored because they don't match the mocked
public_key_hashes, but still.)

[estark, 2015/08/06 05:50:06]

Done.

+                            DEFAULT_PRIORITY, &d));
+  violating_request->Start();
+  base::RunLoop().Run();
+
+  // Check that a report was sent.
+  EXPECT_EQ(report_uri, mock_report_sender.latest_report_uri());
+  ASSERT_FALSE(mock_report_sender.latest_report().empty());
+  scoped_ptr<base::Value> value(
+      base::JSONReader::Read(mock_report_sender.latest_report()));
+  ASSERT_TRUE(value);
+  ASSERT_TRUE(value->IsType(base::Value::TYPE_DICTIONARY));
+  base::DictionaryValue* report_dict;
+  ASSERT_TRUE(value->GetAsDictionary(&report_dict));
+  std::string report_hostname;
+  EXPECT_TRUE(report_dict->GetString("hostname", &report_hostname));
+  EXPECT_EQ(test_server_hostname, report_hostname);
+}
+
+// Tests that reports get sent on requests with
+// Public-Key-Pins-Report-Only headers.
+TEST_F(URLRequestTestHTTP, MAYBE_ProcessPKPReportOnly) {

[davidben, 2015/08/05 18:23:51]

Perhaps add a test that also asserts there is no report if it matches? This
would make sure we update those header files when the certs rotate.

(A little sad we're adding another one, but yours isn't the first of those
files.)

[estark, 2015/08/06 05:50:07]

Done.

+  GURL report_uri(kHPKPReportUri);
+  SpawnedTestServer::SSLOptions ssl_options(
+      SpawnedTestServer::SSLOptions::CERT_COMMON_NAME_IS_DOMAIN);
+  SpawnedTestServer https_test_server(SpawnedTestServer::TYPE_HTTPS,
+                                      ssl_options,
+                                      base::FilePath(kTestFilePath));
+
+  ASSERT_TRUE(https_test_server.Start());
+
+  std::string test_server_hostname = https_test_server.GetURL("").host();
+
+  TransportSecurityState security_state;
+  MockCertificateReportSender mock_report_sender;
+  security_state.SetReportSender(&mock_report_sender);
+
+  // Set up a MockCertVerifier to violate the pin in the Report-Only
+  // header.
+  base::FilePath certificate_file = ssl_options.GetCertificateFile();
+  scoped_refptr<X509Certificate> cert = ImportCertFromFile(
+      GetTestCertsDirectory(), certificate_file.BaseName().value());
+  ASSERT_TRUE(cert);
+
+  MockCertVerifier cert_verifier;
+  CertVerifyResult verify_result;
+  verify_result.verified_cert = cert;
+  verify_result.is_issued_by_known_root = true;
+  HashValue hash;
+  ASSERT_TRUE(hash.FromString("sha1/4BjDjn8v2lWeUFQnqSs0BgbIcrU="));

[davidben, 2015/08/05 18:23:51]

Ditto about making fake values obvious.

[estark, 2015/08/06 05:50:06]

Done.

+  verify_result.public_key_hashes.push_back(hash);
+  cert_verifier.AddResultForCert(cert.get(), verify_result, OK);
+
+  TestNetworkDelegate network_delegate;
+  TestURLRequestContext context(true);
+  context.set_transport_security_state(&security_state);
+  context.set_network_delegate(&network_delegate);
+  context.set_cert_verifier(&cert_verifier);
+  context.Init();
+
+  // Now send a request to trigger the violation.
+  TestDelegate d;
+  scoped_ptr<URLRequest> violating_request(context.CreateRequest(
+      https_test_server.GetURL("files/hpkp-headers-report-only.html"),
+      DEFAULT_PRIORITY, &d));
+  violating_request->Start();
+  base::RunLoop().Run();
+
+  // Check that a report was sent.
+  EXPECT_EQ(report_uri, mock_report_sender.latest_report_uri());
+  ASSERT_FALSE(mock_report_sender.latest_report().empty());
+  scoped_ptr<base::Value> value(
+      base::JSONReader::Read(mock_report_sender.latest_report()));
+  ASSERT_TRUE(value);
+  ASSERT_TRUE(value->IsType(base::Value::TYPE_DICTIONARY));
+  base::DictionaryValue* report_dict;
+  ASSERT_TRUE(value->GetAsDictionary(&report_dict));
+  std::string report_hostname;
+  EXPECT_TRUE(report_dict->GetString("hostname", &report_hostname));
+  EXPECT_EQ(test_server_hostname, report_hostname);
+}
+
 TEST_F(URLRequestTestHTTP, PKPNotProcessedOnIP) {
   SpawnedTestServer https_test_server(SpawnedTestServer::TYPE_HTTPS,
                                       SpawnedTestServer::SSLOptions(),