Add checkbox for reporting invalid TLS/SSL cert chains

chrome/browser/ssl/ssl_browser_tests.cc

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 2360705dbd73324ae3d23c14bd7bda92df460a2f..9e042a824b4f1e60c22839dc64ce09abeb4896fd 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -12,6 +12,7 @@
 #include "base/time/time.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/net/chrome_fraudulent_certificate_reporter.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/ssl_blocking_page.h"
 #include "chrome/browser/ui/browser.h"
@@ -45,6 +46,9 @@
 #include "net/base/test_data_directory.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
+#include "net/url_request/fraudulent_certificate_reporter.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
 
 #if defined(USE_NSS)
 #include "chrome/browser/net/nss_context.h"
@@ -53,11 +57,13 @@
 #endif  // defined(USE_NSS)
 
 using base::ASCIIToUTF16;
+using chrome_browser_net::ChromeFraudulentCertificateReporter;
 using content::InterstitialPage;
 using content::NavigationController;
 using content::NavigationEntry;
 using content::SSLStatus;
 using content::WebContents;
+using net::FraudulentCertificateReporter;
 using web_modal::WebContentsModalDialogManager;
 
 const base::FilePath::CharType kDocRoot[] =
@@ -169,6 +175,37 @@ void CheckSecurityState(WebContents* tab,
   AuthState::Check(*entry, expected_authentication_state);
 }
 
+// This class is used to test invalid certificate chain reporting when
+// the user opts in to do so on the interstitial.
+class MockReporter : public ChromeFraudulentCertificateReporter {
+ public:
+  explicit MockReporter(net::URLRequestContext* request_context)
+      : ChromeFraudulentCertificateReporter(request_context) {}
+
+  void SendReport(ReportType type,
+                  const std::string& hostname,
+                  const net::SSLInfo& ssl_info) override {
+    latest_hostname_reported_ = hostname;
+    EXPECT_EQ(type, REPORT_TYPE_EXTENDED_REPORTING);
+  }
+
+  const std::string& GetLatestHostnameReported() {
+    return latest_hostname_reported_;
+  }
+
+ private:
+  std::string latest_hostname_reported_;
+};
+
+// A helper function for using a MockReporter on a
+// URLRequestContext. Must be run on the IO thread.
+void SetUpMockReporter(
+    const scoped_refptr<net::URLRequestContextGetter> context_getter,
+    MockReporter* reporter) {
+  context_getter->GetURLRequestContext()->set_fraudulent_certificate_reporter(
+      reporter);
+}
+
 }  // namespace
 
 class SSLUITest : public InProcessBrowserTest {
@@ -349,6 +386,72 @@ class SSLUITest : public InProcessBrowserTest {
         page_with_unsafe_worker_path);
   }
 
+  void WaitForIOThread() {
+    base::RunLoop run_loop;
+    content::BrowserThread::PostTaskAndReply(
+        content::BrowserThread::IO, FROM_HERE, base::Bind(&base::DoNothing),
+        run_loop.QuitClosure());
+    run_loop.Run();
+    base::RunLoop().RunUntilIdle();
+  }
+
+  // Helper function for the testing invalid certificate chain reporting.
+  void TestBrokenHTTPSReporting(bool opt_in,
+                                bool proceed,
+                                bool switch_enabled,
+                                bool expect_report,
+                                Browser* browser) {
+    ASSERT_TRUE(https_server_expired_.Start());
+
+    // 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.)
+    MockReporter reporter(nullptr);
+    scoped_refptr<net::URLRequestContextGetter> context_getter =
+        browser->profile()->GetRequestContext();
+
+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO, FROM_HERE,
+        base::Bind(SetUpMockReporter, context_getter, &reporter));
+
+    // Wait until the mock reporter has been set up on the IO thread.
+    WaitForIOThread();
+
+    // Opt in to sending reports for invalid certificate chains.
+    browser->profile()->GetPrefs()->SetBoolean(
+        prefs::kSafeBrowsingExtendedReportingEnabled, opt_in);
+
+    EXPECT_EQ(reporter.GetLatestHostnameReported(), std::string());
+
+    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);
+
+    if (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.
+    WaitForIOThread();

[Bernhard Bauer, 2015/03/03 16:17:45]

This one is a bit more tricky... The problem with posting an empty task to the
IO thread is that you effectively rely on the fact that the interstitial posts
the send task to the IO thread and sending finishes once that tasks runs, which
is an implementation detail of the interstitial that is buried somewhere deep
down (it took me a couple of minutes to find it).

Given that the interstitial is otherwise completely on the UI thread, it might
make sense to add a callback to the interstitial that will be called on the UI
thread once the report has been sent (by doing the PostTaskAndReply thing in the
interstitial). For the sake of simplicity, I would suggest a method
SetReportSentCallbackForTesting or something.

[estark, 2015/03/03 18:06:20]

Makes sense! Done. I named it SetCertificateReportCallback since in some cases
it gets called to signal that the report has been not sent rather than sent
(e.g. incognito method). And I put it on SecurityInterstitialPage instead of
SSLBlockingPage, because we anticipate doing certificate reporting on
CaptivePortalBlockingPage in a follow-up CL.

+
+    if (expect_report) {
+      // Check that the mock reporter received a request to send a report.
+      EXPECT_EQ(reporter.GetLatestHostnameReported(),
+                https_server_expired_.GetURL("/").host());
+    } else {
+      EXPECT_EQ(reporter.GetLatestHostnameReported(), std::string());
+    }
+  }
+
   net::SpawnedTestServer https_server_;
   net::SpawnedTestServer https_server_expired_;
   net::SpawnedTestServer https_server_mismatched_;
@@ -390,6 +493,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());
@@ -446,6 +560,55 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestBrokenHTTPSWithInsecureContent) {
                                  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(true, true, true, true, 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(true, false, true, true, 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(false, true, true, false, 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(false, false, true, false, 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(true, true, false, false, 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(true, true, true, false, CreateIncognitoBrowser());
+}
+
 // http://crbug.com/91745
 #if defined(OS_CHROMEOS)
 #define MAYBE_TestOKHTTPS DISABLED_TestOKHTTPS