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

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

Issue 935663004: Add checkbox for reporting invalid TLS/SSL cert chains (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: revert accidental deletion (fixes failing CaptivePortal tests) Created 5 years, 9 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/ssl_blocking_page.cc ('k') | chrome/browser/ssl/ssl_error_handler.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ssl/ssl_browser_tests.cc
diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc
index 841c4b26442f909016a8f5502f65801ee813cde8..ba69fe78eb8b47aee9d8ebc1bb8ecb8fff4a3cc8 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -12,9 +12,13 @@
#include "base/strings/utf_string_conversions.h"
#include "base/time/time.h"
#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/interstitials/security_interstitial_page_test_utils.h"
+#include "chrome/browser/net/certificate_error_reporter.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/ping_manager.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/browser/ssl/ssl_blocking_page.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
@@ -49,6 +53,7 @@
#include "net/cert/x509_certificate.h"
#include "net/ssl/ssl_info.h"
#include "net/test/spawned_test_server/spawned_test_server.h"
+#include "net/url_request/url_request_context.h"
#if defined(USE_NSS)
#include "chrome/browser/net/nss_context.h"
@@ -58,6 +63,7 @@
using base::ASCIIToUTF16;
using chrome_browser_interstitials::SecurityInterstitialIDNTest;
+using chrome_browser_net::CertificateErrorReporter;
using content::InterstitialPage;
using content::NavigationController;
using content::NavigationEntry;
@@ -174,6 +180,48 @@ void CheckSecurityState(WebContents* tab,
AuthState::Check(*entry, expected_authentication_state);
}
+namespace CertificateReporting {
+
+enum OptIn { EXTENDED_REPORTING_OPT_IN, EXTENDED_REPORTING_DO_NOT_OPT_IN };
+
+enum Proceed { SSL_INTERSTITIAL_PROCEED, SSL_INTERSTITIAL_DO_NOT_PROCEED };
+
+enum ExpectReport { CERT_REPORT_EXPECTED, CERT_REPORT_NOT_EXPECTED };
+
+// This class is used to test invalid certificate chain reporting when
+// the user opts in to do so on the interstitial.
+class MockReporter : public CertificateErrorReporter {
+ public:
+ explicit MockReporter(net::URLRequestContext* request_context,
+ const GURL& upload_url,
+ CookiesPreference cookies_preference)
+ : CertificateErrorReporter(request_context,
+ upload_url,
+ cookies_preference) {}
+
+ void SendReport(CertificateErrorReporter::ReportType type,
+ const std::string& hostname,
+ const net::SSLInfo& ssl_info) override {
+ EXPECT_EQ(CertificateErrorReporter::REPORT_TYPE_EXTENDED_REPORTING, type);
+ latest_hostname_reported_ = hostname;
+ }
+
+ const std::string& latest_hostname_reported() {
+ return latest_hostname_reported_;
+ }
+
+ private:
+ std::string latest_hostname_reported_;
+};
+
+void SetUpMockReporter(SafeBrowsingService* safe_browsing_service,
+ MockReporter* reporter) {
+ safe_browsing_service->ping_manager()->SetCertificateErrorReporterForTesting(
+ scoped_ptr<CertificateErrorReporter>(reporter));
+}
+
+} // namespace CertificateReporting
+
} // namespace
class SSLUITest : public InProcessBrowserTest {
@@ -192,6 +240,24 @@ class SSLUITest : public InProcessBrowserTest {
SSLOptions(SSLOptions::CERT_EXPIRED),
net::GetWebSocketTestDataDirectory()) {}
+ void SetUpOnMainThread() override {
+ // Set up the mock reporter to track the hostnames that reports get
+ // sent for. The request_context argument is NULL here
+ // because the MockReporter doesn't actually use a
+ // request_context. (In order to pass a real request_context, the
+ // reporter would have to be constructed on the IO thread.)
+ reporter_ = new CertificateReporting::MockReporter(
+ NULL, GURL("http://example.test"),
+ CertificateReporting::MockReporter::DO_NOT_SEND_COOKIES);
+ scoped_refptr<SafeBrowsingService> safe_browsing_service =
+ g_browser_process->safe_browsing_service();
+ ASSERT_TRUE(safe_browsing_service);
+ content::BrowserThread::PostTask(
+ content::BrowserThread::IO, FROM_HERE,
+ base::Bind(CertificateReporting::SetUpMockReporter,
+ safe_browsing_service, reporter_));
+ }
+
void SetUpCommandLine(base::CommandLine* command_line) override {
// Browser will both run and display insecure content.
command_line->AppendSwitch(switches::kAllowRunningInsecureContent);
@@ -356,6 +422,57 @@ class SSLUITest : public InProcessBrowserTest {
page_with_unsafe_worker_path);
}
+ // Helper function for testing invalid certificate chain reporting.
+ void TestBrokenHTTPSReporting(
+ CertificateReporting::OptIn opt_in,
+ CertificateReporting::Proceed proceed,
+ CertificateReporting::ExpectReport expect_report,
+ Browser* browser) {
+ ASSERT_TRUE(https_server_expired_.Start());
+
+ // Opt in to sending reports for invalid certificate chains.
+ browser->profile()->GetPrefs()->SetBoolean(
+ prefs::kSafeBrowsingExtendedReportingEnabled,
+ opt_in == CertificateReporting::EXTENDED_REPORTING_OPT_IN);
+
+ ui_test_utils::NavigateToURL(browser, https_server_expired_.GetURL("/"));
+
+ WebContents* tab = browser->tab_strip_model()->GetActiveWebContents();
+ CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID,
+ AuthState::SHOWING_INTERSTITIAL);
+
+ // Set up a callback so that the test is notified when the report
+ // has been sent on the IO thread (or not sent).
+ base::RunLoop report_run_loop;
+ base::Closure report_callback = report_run_loop.QuitClosure();
+ SSLBlockingPage* interstitial_page = static_cast<SSLBlockingPage*>(
+ tab->GetInterstitialPage()->GetDelegateForTesting());
+ interstitial_page->SetCertificateReportCallbackForTesting(report_callback);
+
+ EXPECT_EQ(std::string(), reporter_->latest_hostname_reported());
+
+ // Leave the interstitial (either by proceeding or going back)
+ if (proceed == CertificateReporting::SSL_INTERSTITIAL_PROCEED) {
+ ProceedThroughInterstitial(tab);
+ } else {
+ // Click "Take me back"
+ InterstitialPage* interstitial_page = tab->GetInterstitialPage();
+ ASSERT_TRUE(interstitial_page);
+ interstitial_page->DontProceed();
+ }
+
+ // Wait until the report has been sent on the IO thread.
+ report_run_loop.Run();
+
+ if (expect_report == CertificateReporting::CERT_REPORT_EXPECTED) {
+ // Check that the mock reporter received a request to send a report.
+ EXPECT_EQ(https_server_expired_.GetURL("/").host(),
+ reporter_->latest_hostname_reported());
+ } else {
+ EXPECT_EQ(std::string(), reporter_->latest_hostname_reported());
+ }
+ }
+
net::SpawnedTestServer https_server_;
net::SpawnedTestServer https_server_expired_;
net::SpawnedTestServer https_server_mismatched_;
@@ -363,6 +480,7 @@ class SSLUITest : public InProcessBrowserTest {
private:
typedef net::SpawnedTestServer::SSLOptions SSLOptions;
+ CertificateReporting::MockReporter* reporter_;
DISALLOW_COPY_AND_ASSIGN(SSLUITest);
};
@@ -397,6 +515,17 @@ class SSLUITestIgnoreLocalhostCertErrors : public SSLUITest {
}
};
+class SSLUITestWithExtendedReporting : public SSLUITest {
+ public:
+ SSLUITestWithExtendedReporting() : SSLUITest() {}
+
+ void SetUpCommandLine(base::CommandLine* command_line) override {
+ // Enable a checkbox on SSL interstitials that allows users to opt
+ // in to reporting invalid certificate chains.
+ command_line->AppendSwitch(switches::kEnableInvalidCertCollection);
+ }
+};
+
// Visits a regular page over http.
IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTP) {
ASSERT_TRUE(test_server()->Start());
@@ -994,6 +1123,73 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, MAYBE_TestDisplaysInsecureContent) {
AuthState::DISPLAYED_INSECURE_CONTENT);
}
+// Test that when the checkbox is checked and the user proceeds through
+// the interstitial, the FraudulentCertificateReporter sees a request to
+// send a report.
+IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting,
+ TestBrokenHTTPSProceedWithReporting) {
+ TestBrokenHTTPSReporting(CertificateReporting::EXTENDED_REPORTING_OPT_IN,
+ CertificateReporting::SSL_INTERSTITIAL_PROCEED,
+ CertificateReporting::CERT_REPORT_EXPECTED,
+ browser());
+}
+
+// Test that when the checkbox is checked and the user goes back (does
+// not proceed through the interstitial), the
+// FraudulentCertificateReporter sees a request to send a report.
+IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting,
+ TestBrokenHTTPSGoBackWithReporting) {
+ TestBrokenHTTPSReporting(
+ CertificateReporting::EXTENDED_REPORTING_OPT_IN,
+ CertificateReporting::SSL_INTERSTITIAL_DO_NOT_PROCEED,
+ CertificateReporting::CERT_REPORT_EXPECTED, browser());
+}
+
+// Test that when the checkbox is not checked and the user proceeds
+// through the interstitial, the FraudulentCertificateReporter does not
+// see a request to send a report.
+IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting,
+ TestBrokenHTTPSProceedWithNoReporting) {
+ TestBrokenHTTPSReporting(
+ CertificateReporting::EXTENDED_REPORTING_DO_NOT_OPT_IN,
+ CertificateReporting::SSL_INTERSTITIAL_PROCEED,
+ CertificateReporting::CERT_REPORT_NOT_EXPECTED, browser());
+}
+
+// Test that when the checkbox is not checked and the user does not proceed
+// through the interstitial, the FraudulentCertificateReporter does not
+// see a request to send a report.
+IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting,
+ TestBrokenHTTPSGoBackWithNoReporting) {
+ TestBrokenHTTPSReporting(
+ CertificateReporting::EXTENDED_REPORTING_DO_NOT_OPT_IN,
+ CertificateReporting::SSL_INTERSTITIAL_DO_NOT_PROCEED,
+ CertificateReporting::CERT_REPORT_NOT_EXPECTED, browser());
+}
+
+// Test that when the command-line switch for reporting invalid cert
+// chains is not enabled, reports don't get sent, even if the opt-in
+// preference is set. (i.e. if a user enables invalid cert collection in
+// chrome://flags, checks the box on an interstitial, and then disables
+// the flag in chrome://flags, reports shouldn't be sent on the next
+// interstitial).
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestBrokenHTTPSNoReportingWithoutSwitch) {
+ TestBrokenHTTPSReporting(CertificateReporting::EXTENDED_REPORTING_OPT_IN,
+ CertificateReporting::SSL_INTERSTITIAL_PROCEED,
+ CertificateReporting::CERT_REPORT_NOT_EXPECTED,
+ browser());
+}
+
+// Test that reports don't get sent in incognito mode even if the opt-in
+// preference is set and the command-line switch is enabled.
+IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting,
+ TestBrokenHTTPSNoReportingInIncognito) {
+ TestBrokenHTTPSReporting(CertificateReporting::EXTENDED_REPORTING_OPT_IN,
+ CertificateReporting::SSL_INTERSTITIAL_PROCEED,
+ CertificateReporting::CERT_REPORT_NOT_EXPECTED,
+ CreateIncognitoBrowser());
+}
+
// Visits a page that runs insecure content and tries to suppress the insecure
// content warnings by randomizing location.hash.
// Based on http://crbug.com/8706
@@ -1944,7 +2140,7 @@ class SSLBlockingPageIDNTest : public SecurityInterstitialIDNTest {
request_url.host(), "CA", base::Time::Max(), base::Time::Max());
return new SSLBlockingPage(
contents, net::ERR_CERT_CONTAINS_ERRORS, ssl_info, request_url, 0,
- base::Time::NowFromSystemTime(), base::Callback<void(bool)>());
+ base::Time::NowFromSystemTime(), nullptr, base::Callback<void(bool)>());
}
};
« no previous file with comments | « chrome/browser/ssl/ssl_blocking_page.cc ('k') | chrome/browser/ssl/ssl_error_handler.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698