Add the ability to send Expect-Staple reports.

net/http/transport_security_state_unittest.cc

Index: net/http/transport_security_state_unittest.cc
diff --git a/net/http/transport_security_state_unittest.cc b/net/http/transport_security_state_unittest.cc
index 20c5619555b49809c75dd4c8fe26238f6fedf622..e86c74b5bf59cff2162671a26c910f68f0d830a9 100644
--- a/net/http/transport_security_state_unittest.cc
+++ b/net/http/transport_security_state_unittest.cc
@@ -240,6 +240,94 @@ void CheckHPKPReport(
       validated_certificate_chain, report_validated_certificate_chain));
 }
 
+void CheckExpectStapleReport(const std::string& report,
+                             const HostPortPair& host_port_pair,
+                             const SSLInfo& ssl_info,
+                             const std::string& ocsp_response,
+                             const std::string& response_status,
+                             const std::string& cert_status) {
+  std::unique_ptr<base::Value> value(base::JSONReader::Read(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(host_port_pair.host(), report_hostname);
+
+  int report_port;
+  EXPECT_TRUE(report_dict->GetInteger("port", &report_port));
+  EXPECT_EQ(host_port_pair.port(), report_port);
+
+  std::string report_response_status;
+  EXPECT_TRUE(
+      report_dict->GetString("response-status", &report_response_status));
+  EXPECT_EQ(response_status, report_response_status);
+
+  std::string report_ocsp_response;
+  bool has_ocsp_response =
+      report_dict->GetString("ocsp-response", &report_ocsp_response);
+
+  if (!ocsp_response.empty()) {
+    EXPECT_TRUE(has_ocsp_response);
+    std::string decoded_ocsp_response;
+    EXPECT_TRUE(
+        base::Base64Decode(report_ocsp_response, &decoded_ocsp_response));
+    EXPECT_EQ(ocsp_response, decoded_ocsp_response);
+  } else {
+    EXPECT_FALSE(has_ocsp_response);
+  }
+
+  std::string report_cert_status;
+  bool has_cert_status =
+      report_dict->GetString("cert-status", &report_cert_status);
+  if (!cert_status.empty()) {
+    EXPECT_TRUE(has_cert_status);
+    EXPECT_EQ(cert_status, report_cert_status);
+  } else {
+    EXPECT_FALSE(has_cert_status);
+  }
+
+  base::ListValue* report_served_certificate_chain;
+  bool has_served_chain = report_dict->GetList(
+      "served-certificate-chain", &report_served_certificate_chain);
+
+  base::ListValue* report_validated_certificate_chain;
+  bool has_validated_chain = report_dict->GetList(
+      "validated-certificate-chain", &report_validated_certificate_chain);
+
+  if (ssl_info.is_issued_by_known_root) {
+    EXPECT_TRUE(has_served_chain);
+    EXPECT_NO_FATAL_FAILURE(CompareCertificateChainWithList(
+        ssl_info.unverified_cert, report_served_certificate_chain));
+
+    EXPECT_TRUE(has_validated_chain);
+    EXPECT_NO_FATAL_FAILURE(CompareCertificateChainWithList(
+        ssl_info.cert, report_validated_certificate_chain));
+  } else {
+    EXPECT_FALSE(has_served_chain);
+    EXPECT_FALSE(has_validated_chain);
+  }
+}
+
+void CheckExpectStaple(TransportSecurityState* state,
+                       MockCertificateReportSender* reporter,
+                       const SSLInfo& ssl_info,
+                       const std::string& ocsp_response,
+                       const std::string& response_status,
+                       const std::string& cert_status) {
+  HostPortPair host_port(kExpectStapleStaticHostname, 443);

[Ryan Sleevi, 2016/07/19 19:11:05]

Where does this come from and why? The test lacks documentation, so it was
unclear. It sounds like this is baked directly into the test data?

[dadrian, 2016/07/19 21:21:46]

Yup, it's baked into the preload list itself as a test domain. I added a comment
explaining the choice of domains.

+  state->SetReportSender(reporter);
+  state->ProcessExpectStaple(host_port, ssl_info, ocsp_response);
+  EXPECT_EQ(GURL(kExpectStapleStaticReportURI), reporter->latest_report_uri());
+  std::string serialized_report = reporter->latest_report();
+  EXPECT_NO_FATAL_FAILURE(
+      CheckExpectStapleReport(serialized_report, host_port, ssl_info,
+                              ocsp_response, response_status, cert_status));
+}
+
 }  // namespace
 
 class TransportSecurityStateTest : public testing::Test {
@@ -1902,6 +1990,167 @@ TEST_F(TransportSecurityStateTest, ExpectCTReporter) {
   EXPECT_EQ(GURL(kExpectCTStaticReportURI), reporter.report_uri());
 }
 
+static const struct ExpectStapleErrorResponseData {
+  OCSPVerifyResult::ResponseStatus response_status;
+  std::string response_status_string;
+} kExpectStapleReportData[] = {
+    {OCSPVerifyResult::MISSING, "MISSING"},
+    {OCSPVerifyResult::ERROR_RESPONSE, "ERROR_RESPONSE"},
+    {OCSPVerifyResult::BAD_PRODUCED_AT, "BAD_PRODUCED_AT"},
+    {OCSPVerifyResult::NO_MATCHING_RESPONSE, "NO_MATCHING_RESPONSE"},
+    {OCSPVerifyResult::INVALID_DATE, "INVALID_DATE"},
+    {OCSPVerifyResult::PARSE_RESPONSE_ERROR, "PARSE_RESPONSE_ERROR"},
+    {OCSPVerifyResult::PARSE_RESPONSE_DATA_ERROR, "PARSE_RESPONSE_DATA_ERROR"},
+};
+
+class ExpectStapleErrorResponseTest
+    : public TransportSecurityStateTest,
+      public testing::WithParamInterface<ExpectStapleErrorResponseData> {};
+
+TEST_P(ExpectStapleErrorResponseTest, CheckResponseStatusSerialization) {

[Ryan Sleevi, 2016/07/19 19:11:05]

Improve high-level docs for the test

[dadrian, 2016/07/19 21:21:46]

Done.

+  TransportSecurityState state;
+  TransportSecurityStateTest::EnableStaticExpectStaple(&state);
+  MockCertificateReportSender reporter;
+  ExpectStapleErrorResponseData test = GetParam();
+
+  std::string ocsp_response;
+  if (test.response_status != OCSPVerifyResult::MISSING)
+    ocsp_response = "dummy_response";
+
+  // Two dummy certs to use as the server-sent and validated chains. The
+  // contents don't matter.
+  scoped_refptr<X509Certificate> cert1 =
+      ImportCertFromFile(GetTestCertsDirectory(), "test_mail_google_com.pem");
+  scoped_refptr<X509Certificate> cert2 =
+      ImportCertFromFile(GetTestCertsDirectory(), "expired_cert.pem");
+
+  SSLInfo ssl_info;
+  ssl_info.cert = cert1;
+  ssl_info.unverified_cert = cert2;
+  ssl_info.ocsp_result.response_status = test.response_status;
+
+  ssl_info.is_issued_by_known_root = true;
+  ASSERT_NO_FATAL_FAILURE(
+      CheckExpectStaple(&state, &reporter, ssl_info, ocsp_response,
+                        test.response_status_string, std::string()));
+
+  ssl_info.is_issued_by_known_root = false;
+  ASSERT_NO_FATAL_FAILURE(
+      CheckExpectStaple(&state, &reporter, ssl_info, ocsp_response,
+                        test.response_status_string, std::string()));

[Ryan Sleevi, 2016/07/19 19:11:05]

These tests (2081 - 2089) could benefit from improved documentation - it's
unclear why you're testing both or what you're testing for or why.

Are you testing an implementation detail? Is it guaranteed by API contract that
we send regardless? Why :)

(Same applies to other tests)

[dadrian, 2016/07/19 21:21:46]

Done.

+}
+
+INSTANTIATE_TEST_CASE_P(ExpectStaple,
+                        ExpectStapleErrorResponseTest,
+                        testing::ValuesIn(kExpectStapleReportData));
+
+static const struct ExpectStapleErrorCertStatusData {
+  OCSPRevocationStatus revocation_status;
+  std::string cert_status_string;
+} kExpectStapleErrorCertStatusData[] = {
+    {OCSPRevocationStatus::REVOKED, "REVOKED"},
+    {OCSPRevocationStatus::UNKNOWN, "UNKNOWN"},
+};
+
+class ExpectStapleErrorCertStatusTest
+    : public TransportSecurityStateTest,
+      public testing::WithParamInterface<ExpectStapleErrorCertStatusData> {};
+
+TEST_P(ExpectStapleErrorCertStatusTest, CheckCertStatusSerialization) {

[Ryan Sleevi, 2016/07/19 19:11:04]

Improve high-level docs for the test

[dadrian, 2016/07/19 21:21:46]

Done.

+  TransportSecurityState state;
+  TransportSecurityStateTest::EnableStaticExpectStaple(&state);
+  MockCertificateReportSender reporter;
+  ExpectStapleErrorCertStatusData test = GetParam();
+  std::string ocsp_response = "dummy_response";
+
+  // Two dummy certs to use as the server-sent and validated chains. The
+  // contents don't matter.
+  scoped_refptr<X509Certificate> cert1 =
+      ImportCertFromFile(GetTestCertsDirectory(), "test_mail_google_com.pem");
+  scoped_refptr<X509Certificate> cert2 =
+      ImportCertFromFile(GetTestCertsDirectory(), "expired_cert.pem");
+
+  SSLInfo ssl_info;
+  ssl_info.cert = cert1;
+  ssl_info.unverified_cert = cert2;
+  // |response_status| must be set to PROVIDED for |revocation_status| to have
+  // meaning.
+  ssl_info.ocsp_result.response_status = OCSPVerifyResult::PROVIDED;
+  ssl_info.ocsp_result.revocation_status = test.revocation_status;
+
+  ssl_info.is_issued_by_known_root = true;
+  ASSERT_NO_FATAL_FAILURE(CheckExpectStaple(&state, &reporter, ssl_info,
+                                            ocsp_response, "PROVIDED",
+                                            test.cert_status_string));
+
+  ssl_info.is_issued_by_known_root = false;
+  ASSERT_NO_FATAL_FAILURE(CheckExpectStaple(&state, &reporter, ssl_info,
+                                            ocsp_response, "PROVIDED",
+                                            test.cert_status_string));
+};
+
+INSTANTIATE_TEST_CASE_P(ExpectStaple,
+                        ExpectStapleErrorCertStatusTest,
+                        testing::ValuesIn(kExpectStapleErrorCertStatusData));
+
+TEST_F(TransportSecurityStateTest, ExpectStapleDoesNotReportValidStaple) {
+  TransportSecurityState state;
+  TransportSecurityStateTest::EnableStaticExpectStaple(&state);
+  MockCertificateReportSender reporter;
+  state.SetReportSender(&reporter);
+
+  HostPortPair host_port(kExpectStapleStaticHostname, 443);
+
+  // Two dummy certs to use as the server-sent and validated chains. The
+  // contents don't matter.
+  scoped_refptr<X509Certificate> cert1 =
+      ImportCertFromFile(GetTestCertsDirectory(), "test_mail_google_com.pem");
+  scoped_refptr<X509Certificate> cert2 =
+      ImportCertFromFile(GetTestCertsDirectory(), "expired_cert.pem");
+
+  SSLInfo ssl_info;
+  ssl_info.is_issued_by_known_root = true;
+  ssl_info.cert = cert1;
+  ssl_info.unverified_cert = cert2;
+  ssl_info.ocsp_result.response_status = OCSPVerifyResult::PROVIDED;
+  ssl_info.ocsp_result.revocation_status = OCSPRevocationStatus::GOOD;
+
+  std::string ocsp_response = "dummy response";
+
+  state.ProcessExpectStaple(host_port, ssl_info, ocsp_response);
+  EXPECT_EQ(GURL(), reporter.latest_report_uri());
+  EXPECT_TRUE(reporter.latest_report().empty());
+}
+
+TEST_F(TransportSecurityStateTest, ExpectStapleRequiresPreload) {
+  TransportSecurityState state;
+  TransportSecurityStateTest::EnableStaticExpectStaple(&state);
+  MockCertificateReportSender reporter;
+  state.SetReportSender(&reporter);
+
+  HostPortPair host_port("not-preloaded.badssl.com", 443);

[Ryan Sleevi, 2016/07/19 19:11:05]

Similar to my remarks above (about it being preloaded), this is unclear why it's
guaranteed it will *not* be preloaded.

It would make more sense to use an IANA-reserved hostname, rather than assuming
badssl won't ever set expect-staple with include-subdomains

That is, not-preloaded.host.example (for example)

[dadrian, 2016/07/19 21:21:46]

Done.

+
+  // Two dummy certs to use as the server-sent and validated chains. The
+  // contents don't matter.
+  scoped_refptr<X509Certificate> cert1 =
+      ImportCertFromFile(GetTestCertsDirectory(), "test_mail_google_com.pem");
+  scoped_refptr<X509Certificate> cert2 =
+      ImportCertFromFile(GetTestCertsDirectory(), "expired_cert.pem");
+
+  SSLInfo ssl_info;
+  ssl_info.is_issued_by_known_root = true;

[Ryan Sleevi, 2016/07/19 19:11:05]

By contrast with your other tests (and part of why I recommended more
documentation), it's unclear why it's OK not to fiddle with
is_issued_by_known_root in this test (and in
ExpectStapleDoesNotReportValidStaple)

If we accept the possible argument that you fiddle with report sending for the
first two since "reports might be disabled based on the value of
is_issued_by_known_root" (I suspect the probable answer), then wouldn't the
response here be to test both values in both subsequent tests as well?

[dadrian, 2016/07/19 21:21:46]

Done.

+  ssl_info.cert = cert1;
+  ssl_info.unverified_cert = cert2;
+  ssl_info.ocsp_result.response_status = OCSPVerifyResult::MISSING;
+
+  // Empty response
+  std::string ocsp_response;
+
+  state.ProcessExpectStaple(host_port, ssl_info, ocsp_response);
+  EXPECT_EQ(GURL(), reporter.latest_report_uri());
+  EXPECT_TRUE(reporter.latest_report().empty());
+}
+
 // Tests that TransportSecurityState always consults the RequireCTDelegate,
 // if supplied.
 TEST_F(TransportSecurityStateTest, RequireCTConsultsDelegate) {