Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4325)

Unified Diff: chrome/browser/ssl/chrome_expect_ct_reporter_unittest.cc

Issue 2970913002: Implement CORS preflights for Expect-CT reports (Closed)
Patch Set: meacer nits Created 3 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/ssl/chrome_expect_ct_reporter.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ssl/chrome_expect_ct_reporter_unittest.cc
diff --git a/chrome/browser/ssl/chrome_expect_ct_reporter_unittest.cc b/chrome/browser/ssl/chrome_expect_ct_reporter_unittest.cc
index 755444ab3a84b658e51f69774cc3481fb5b9e445..34ef988ff6f4ed8eb46c1745690db5693ce7d3b6 100644
--- a/chrome/browser/ssl/chrome_expect_ct_reporter_unittest.cc
+++ b/chrome/browser/ssl/chrome_expect_ct_reporter_unittest.cc
@@ -9,7 +9,6 @@
#include "base/base64.h"
#include "base/command_line.h"
#include "base/json/json_reader.h"
-#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
#include "base/test/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
@@ -19,6 +18,8 @@
#include "net/cert/ct_serialization.h"
#include "net/cert/signed_certificate_timestamp_and_status.h"
#include "net/test/cert_test_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_response.h"
#include "net/test/test_data_directory.h"
#include "net/test/url_request/url_request_failed_job.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
@@ -50,6 +51,10 @@ class TestCertificateReportSender : public net::ReportSender {
latest_report_uri_ = report_uri;
serialized_report.CopyToString(&latest_serialized_report_);
content_type.CopyToString(&latest_content_type_);
+ if (!report_callback_.is_null()) {
+ EXPECT_EQ(expected_report_uri_, latest_report_uri_);
+ report_callback_.Run();
+ }
}
const GURL& latest_report_uri() const { return latest_report_uri_; }
@@ -62,10 +67,26 @@ class TestCertificateReportSender : public net::ReportSender {
return latest_serialized_report_;
}
+ // Can be called to wait for a single report, which is expected to be sent to
+ // |report_uri|. Returns immediately if a report has already been sent in the
+ // past.
+ void WaitForReport(const GURL& report_uri) {
+ if (!latest_report_uri_.is_empty()) {
+ EXPECT_EQ(report_uri, latest_report_uri_);
+ return;
+ }
+ base::RunLoop run_loop;
+ report_callback_ = run_loop.QuitClosure();
+ expected_report_uri_ = report_uri;
+ run_loop.Run();
+ }
+
private:
GURL latest_report_uri_;
std::string latest_content_type_;
std::string latest_serialized_report_;
+ base::Closure report_callback_;
+ GURL expected_report_uri_;
};
// Constructs a net::SignedCertificateTimestampAndStatus with the given
@@ -299,14 +320,97 @@ class ChromeExpectCTReporterWaitTest : public ::testing::Test {
DISALLOW_COPY_AND_ASSIGN(ChromeExpectCTReporterWaitTest);
};
+// A test fixture that responds properly to CORS preflights so that reports can
+// be successfully sent to test_server().
+class ChromeExpectCTReporterTest : public ::testing::Test {
+ public:
+ ChromeExpectCTReporterTest()
+ : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {}
+ ~ChromeExpectCTReporterTest() override {}
+
+ void SetUp() override {
+ report_server_.RegisterRequestHandler(
+ base::Bind(&ChromeExpectCTReporterTest::HandleReportPreflight,
+ base::Unretained(this)));
+ ASSERT_TRUE(report_server_.Start());
+ }
+
+ std::unique_ptr<net::test_server::HttpResponse> HandleReportPreflight(
+ const net::test_server::HttpRequest& request) {
+ num_requests_++;
+ if (!requests_callback_.is_null()) {
+ requests_callback_.Run();
+ }
+ std::unique_ptr<net::test_server::BasicHttpResponse> http_response(
+ new net::test_server::BasicHttpResponse());
+ http_response->set_code(net::HTTP_OK);
+ for (const auto& cors_header : cors_headers_) {
+ http_response->AddCustomHeader(cors_header.first, cors_header.second);
+ }
+ return http_response;
+ }
+
+ void WaitForReportPreflight() {
+ if (num_requests_ >= 1) {
+ return;
+ }
+ base::RunLoop run_loop;
+ requests_callback_ = run_loop.QuitClosure();
+ run_loop.Run();
+ }
+
+ protected:
+ const net::EmbeddedTestServer& test_server() { return report_server_; }
+
+ // Tests that reports are not sent when the CORS preflight request returns the
+ // header field |preflight_header_name| with value given by
+ // |preflight_header_bad_value|, and that reports are successfully sent when
+ // it has value given by |preflight_header_good_value|.
+ void TestForReportPreflightFailure(
+ ChromeExpectCTReporter* reporter,
+ TestCertificateReportSender* sender,
+ const net::HostPortPair& host_port,
+ const net::SSLInfo& ssl_info,
+ const std::string& preflight_header_name,
+ const std::string& preflight_header_bad_value,
+ const std::string& preflight_header_good_value) {
+ cors_headers_[preflight_header_name] = preflight_header_bad_value;
+ const GURL fail_report_uri = test_server().GetURL("/report1");
+ reporter->OnExpectCTFailed(
+ host_port, fail_report_uri, base::Time(), ssl_info.cert.get(),
+ ssl_info.unverified_cert.get(), ssl_info.signed_certificate_timestamps);
+ WaitForReportPreflight();
+ EXPECT_TRUE(sender->latest_report_uri().is_empty());
+ EXPECT_TRUE(sender->latest_serialized_report().empty());
+
+ // Set the proper header value and send a dummy report. The test will fail
+ // if the previous OnExpectCTFailed() call unexpectedly resulted in a
+ // report, as WaitForReport() would see the previous report to /report1
+ // instead of the expected report to /report2.
+ const GURL successful_report_uri = test_server().GetURL("/report2");
+ cors_headers_[preflight_header_name] = preflight_header_good_value;
+ reporter->OnExpectCTFailed(
+ host_port, successful_report_uri, base::Time(), ssl_info.cert.get(),
+ ssl_info.unverified_cert.get(), ssl_info.signed_certificate_timestamps);
+ sender->WaitForReport(successful_report_uri);
+ EXPECT_EQ(successful_report_uri, sender->latest_report_uri());
+ }
+
+ private:
+ content::TestBrowserThreadBundle thread_bundle_;
+ net::EmbeddedTestServer report_server_;
+ uint32_t num_requests_ = 0;
+ base::Closure requests_callback_;
+ std::map<std::string, std::string> cors_headers_{
+ {"Access-Control-Allow-Origin", "*"},
+ {"Access-Control-Allow-Methods", "GET,POST"},
+ {"Access-Control-Allow-Headers", "content-type,another-header"}};
+};
+
} // namespace
// Test that no report is sent when the feature is not enabled.
-TEST(ChromeExpectCTReporterTest, FeatureDisabled) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitAndDisableFeature(features::kExpectCTReporting);
-
- base::MessageLoop message_loop;
+TEST_F(ChromeExpectCTReporterTest, FeatureDisabled) {
base::HistogramTester histograms;
histograms.ExpectTotalCount(kSendHistogramName, 0);
@@ -324,20 +428,39 @@ TEST(ChromeExpectCTReporterTest, FeatureDisabled) {
net::GetTestCertsDirectory(), "localhost_cert.pem");
net::HostPortPair host_port("example.test", 443);
- GURL report_uri("http://example-report.test");
- reporter.OnExpectCTFailed(host_port, report_uri, base::Time(),
- ssl_info.cert.get(), ssl_info.unverified_cert.get(),
- ssl_info.signed_certificate_timestamps);
- EXPECT_TRUE(sender->latest_report_uri().is_empty());
- EXPECT_TRUE(sender->latest_serialized_report().empty());
+ {
+ const GURL report_uri = test_server().GetURL("/report1");
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndDisableFeature(features::kExpectCTReporting);
- histograms.ExpectTotalCount(kSendHistogramName, 0);
+ reporter.OnExpectCTFailed(
+ host_port, report_uri, base::Time(), ssl_info.cert.get(),
+ ssl_info.unverified_cert.get(), ssl_info.signed_certificate_timestamps);
+ EXPECT_TRUE(sender->latest_report_uri().is_empty());
+ EXPECT_TRUE(sender->latest_serialized_report().empty());
+
+ histograms.ExpectTotalCount(kSendHistogramName, 0);
+ }
+
+ // Enable the feature and send a dummy report. The test will fail if the
+ // previous OnExpectCTFailed() call unexpectedly resulted in a report, as the
+ // WaitForReport() would see the previous report to /report1 instead of the
+ // expected report to /report2.
+ {
+ const GURL report_uri = test_server().GetURL("/report2");
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(features::kExpectCTReporting);
+ reporter.OnExpectCTFailed(
+ host_port, report_uri, base::Time(), ssl_info.cert.get(),
+ ssl_info.unverified_cert.get(), ssl_info.signed_certificate_timestamps);
+ sender->WaitForReport(report_uri);
+ EXPECT_EQ(report_uri, sender->latest_report_uri());
+ }
}
// Test that no report is sent if the report URI is empty.
-TEST(ChromeExpectCTReporterTest, EmptyReportURI) {
- base::MessageLoop message_loop;
+TEST_F(ChromeExpectCTReporterTest, EmptyReportURI) {
base::HistogramTester histograms;
histograms.ExpectTotalCount(kSendHistogramName, 0);
@@ -385,8 +508,7 @@ TEST_F(ChromeExpectCTReporterWaitTest, SendReportFailure) {
}
// Test that a sent report has the right format.
-TEST(ChromeExpectCTReporterTest, SendReport) {
- base::MessageLoop message_loop;
+TEST_F(ChromeExpectCTReporterTest, SendReport) {
base::HistogramTester histograms;
histograms.ExpectTotalCount(kFailureHistogramName, 0);
histograms.ExpectTotalCount(kSendHistogramName, 0);
@@ -454,26 +576,104 @@ TEST(ChromeExpectCTReporterTest, SendReport) {
net::ct::SCT_STATUS_OK,
&ssl_info.signed_certificate_timestamps);
- net::HostPortPair host_port("example.test", 443);
- GURL report_uri("http://example-report.test");
-
const char kExpirationTimeStr[] = "2017-01-01T00:00:00.000Z";
base::Time expiration;
ASSERT_TRUE(
base::Time::FromUTCExploded({2017, 1, 0, 1, 0, 0, 0, 0}, &expiration));
+ const GURL report_uri = test_server().GetURL("/report");
+
// Check that the report is sent and contains the correct information.
- reporter.OnExpectCTFailed(host_port, report_uri, expiration,
- ssl_info.cert.get(), ssl_info.unverified_cert.get(),
+ reporter.OnExpectCTFailed(net::HostPortPair::FromURL(report_uri), report_uri,
+ expiration, ssl_info.cert.get(),
+ ssl_info.unverified_cert.get(),
ssl_info.signed_certificate_timestamps);
+
+ // A CORS preflight request should be sent before the actual report.
+ WaitForReportPreflight();
+ sender->WaitForReport(report_uri);
+
EXPECT_EQ(report_uri, sender->latest_report_uri());
EXPECT_FALSE(sender->latest_serialized_report().empty());
- EXPECT_EQ("application/json; charset=utf-8", sender->latest_content_type());
- ASSERT_NO_FATAL_FAILURE(
- CheckExpectCTReport(sender->latest_serialized_report(), host_port,
- kExpirationTimeStr, ssl_info));
+ EXPECT_EQ("application/expect-ct-report+json; charset=utf-8",
+ sender->latest_content_type());
+ ASSERT_NO_FATAL_FAILURE(CheckExpectCTReport(
+ sender->latest_serialized_report(),
+ net::HostPortPair::FromURL(report_uri), kExpirationTimeStr, ssl_info));
histograms.ExpectTotalCount(kFailureHistogramName, 0);
histograms.ExpectTotalCount(kSendHistogramName, 1);
histograms.ExpectBucketCount(kSendHistogramName, true, 1);
}
+
+// Test that no report is sent when the CORS preflight returns an invalid
+// Access-Control-Allow-Origin.
+TEST_F(ChromeExpectCTReporterTest, BadCORSPreflightResponseOrigin) {
+ TestCertificateReportSender* sender = new TestCertificateReportSender();
+ net::TestURLRequestContext context;
+ ChromeExpectCTReporter reporter(&context);
+ reporter.report_sender_.reset(sender);
+ EXPECT_TRUE(sender->latest_report_uri().is_empty());
+ EXPECT_TRUE(sender->latest_serialized_report().empty());
+
+ net::SSLInfo ssl_info;
+ ssl_info.cert =
+ net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
+ ssl_info.unverified_cert = net::ImportCertFromFile(
+ net::GetTestCertsDirectory(), "localhost_cert.pem");
+
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(features::kExpectCTReporting);
+ EXPECT_TRUE(sender->latest_serialized_report().empty());
+ ASSERT_NO_FATAL_FAILURE(TestForReportPreflightFailure(
+ &reporter, sender, net::HostPortPair("example.test", 443), ssl_info,
+ "Access-Control-Allow-Origin", "https://another-origin.test", "null"));
+}
+
+// Test that no report is sent when the CORS preflight returns an invalid
+// Access-Control-Allow-Methods.
+TEST_F(ChromeExpectCTReporterTest, BadCORSPreflightResponseMethods) {
+ TestCertificateReportSender* sender = new TestCertificateReportSender();
+ net::TestURLRequestContext context;
+ ChromeExpectCTReporter reporter(&context);
+ reporter.report_sender_.reset(sender);
+ EXPECT_TRUE(sender->latest_report_uri().is_empty());
+ EXPECT_TRUE(sender->latest_serialized_report().empty());
+
+ net::SSLInfo ssl_info;
+ ssl_info.cert =
+ net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
+ ssl_info.unverified_cert = net::ImportCertFromFile(
+ net::GetTestCertsDirectory(), "localhost_cert.pem");
+
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(features::kExpectCTReporting);
+ EXPECT_TRUE(sender->latest_serialized_report().empty());
+ ASSERT_NO_FATAL_FAILURE(TestForReportPreflightFailure(
+ &reporter, sender, net::HostPortPair("example.test", 443), ssl_info,
+ "Access-Control-Allow-Methods", "GET,HEAD,POSSSST", "POST"));
+}
+
+// Test that no report is sent when the CORS preflight returns an invalid
+// Access-Control-Allow-Headers.
+TEST_F(ChromeExpectCTReporterTest, BadCORSPreflightResponseHeaders) {
+ TestCertificateReportSender* sender = new TestCertificateReportSender();
+ net::TestURLRequestContext context;
+ ChromeExpectCTReporter reporter(&context);
+ reporter.report_sender_.reset(sender);
+ EXPECT_TRUE(sender->latest_report_uri().is_empty());
+ EXPECT_TRUE(sender->latest_serialized_report().empty());
+
+ net::SSLInfo ssl_info;
+ ssl_info.cert =
+ net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
+ ssl_info.unverified_cert = net::ImportCertFromFile(
+ net::GetTestCertsDirectory(), "localhost_cert.pem");
+
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(features::kExpectCTReporting);
+ EXPECT_TRUE(sender->latest_serialized_report().empty());
+ ASSERT_NO_FATAL_FAILURE(TestForReportPreflightFailure(
+ &reporter, sender, net::HostPortPair("example.test", 443), ssl_info,
+ "Access-Control-Allow-Headers", "Not-Content-Type", "Content-Type"));
+}
« no previous file with comments | « chrome/browser/ssl/chrome_expect_ct_reporter.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698