Add SSL error assistant component to dynamically update captive portal list

chrome/browser/ssl/ssl_browser_tests.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <utility>
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
@@ skipping 3911 matching lines @@
 }
 
 // Put captive portal related tests under a different namespace for nicer
 // pattern matching.
 using SSLUICaptivePortalListTest = SSLUITest;
 
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
 
 // Tests that the captive portal certificate list is not used when the feature
 // is disabled via Finch. The list is passed to SSLErrorHandler via a proto.
-IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListTest,
-                       CaptivePortalCertificateList_Disabled) {
+IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListTest, Disabled) {
   base::test::ScopedFeatureList scoped_feature_list;
   // Use InitFromCommandLine instead of InitAndDisableFeature to avoid making
   // the feature public in SSLErrorHandler header.
   scoped_feature_list.InitFromCommandLine(
       std::string() /* enabled */,
       "CaptivePortalCertificateList" /* disabled */);
 
   ASSERT_TRUE(https_server_mismatched_.Start());
   base::HistogramTester histograms;
 
   // Mark the server's cert as a captive portal cert.
   const net::HashValue server_spki_hash =
       GetSPKIHash(https_server_mismatched_.GetCertificate().get());
-  chrome_browser_ssl::SSLErrorAssistantConfig config_proto;
-  config_proto.add_captive_portal_cert()->set_sha256_hash(
+  auto config_proto =
+      base::MakeUnique<chrome_browser_ssl::SSLErrorAssistantConfig>();
+  config_proto->add_captive_portal_cert()->set_sha256_hash(
       server_spki_hash.ToString());
-  SSLErrorHandler::SetErrorAssistantProtoForTesting(config_proto);
+  SSLErrorHandler::SetErrorAssistantProto(std::move(config_proto));
 
   // Navigate to an unsafe page on the server. A normal SSL interstitial should
   // be displayed since CaptivePortalCertificateList feature is disabled.
   WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
   SSLInterstitialTimerObserver interstitial_timer_observer(tab);
   ui_test_utils::NavigateToURL(
       browser(), https_server_mismatched_.GetURL("/ssl/blank_page.html"));
   content::WaitForInterstitialAttach(tab);
 
   InterstitialPage* interstitial_page = tab->GetInterstitialPage();
   ASSERT_EQ(SSLBlockingPage::kTypeForTesting,
             interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
   EXPECT_TRUE(interstitial_timer_observer.timer_started());
 
   // Check that the histogram for the SSL interstitial was recorded.
   histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 2);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::HANDLE_ALL, 1);
   histograms.ExpectBucketCount(
       SSLErrorHandler::GetHistogramNameForTesting(),
       SSLErrorHandler::SHOW_SSL_INTERSTITIAL_OVERRIDABLE, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::CAPTIVE_PORTAL_CERT_FOUND, 0);
 }
 
 // Tests that the captive portal certificate list is used when the feature
 // is enabled via Finch. The list is passed to SSLErrorHandler via a proto.
-IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListTest,
-                       CaptivePortalCertificateList_Enabled_FromProto) {
+IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListTest, Enabled_FromProto) {
   base::test::ScopedFeatureList scoped_feature_list;
   scoped_feature_list.InitFromCommandLine(
       "CaptivePortalCertificateList" /* enabled */,
       std::string() /* disabled */);
 
   ASSERT_TRUE(https_server_mismatched_.Start());
   base::HistogramTester histograms;
 
   // Mark the server's cert as a captive portal cert.
   const net::HashValue server_spki_hash =
       GetSPKIHash(https_server_mismatched_.GetCertificate().get());
-  chrome_browser_ssl::SSLErrorAssistantConfig config_proto;
-  config_proto.add_captive_portal_cert()->set_sha256_hash(
+  auto config_proto =
+      base::MakeUnique<chrome_browser_ssl::SSLErrorAssistantConfig>();
+  config_proto->add_captive_portal_cert()->set_sha256_hash(
       server_spki_hash.ToString());
-  SSLErrorHandler::SetErrorAssistantProtoForTesting(config_proto);
+  SSLErrorHandler::SetErrorAssistantProto(std::move(config_proto));
 
   // Navigate to an unsafe page on the server. The captive portal interstitial
   // should be displayed since CaptivePortalCertificateList feature is enabled.
   WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
   SSLInterstitialTimerObserver interstitial_timer_observer(tab);
   ui_test_utils::NavigateToURL(
       browser(), https_server_mismatched_.GetURL("/ssl/blank_page.html"));
   content::WaitForInterstitialAttach(tab);
 
   InterstitialPage* interstitial_page = tab->GetInterstitialPage();
   ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting,
             interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
   EXPECT_FALSE(interstitial_timer_observer.timer_started());
 
   // Check that the histogram for the captive portal cert was recorded.
   histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 3);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::HANDLE_ALL, 1);
   histograms.ExpectBucketCount(
       SSLErrorHandler::GetHistogramNameForTesting(),
       SSLErrorHandler::SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::CAPTIVE_PORTAL_CERT_FOUND, 1);
 }
 
 namespace {
 
+const char kCaptivePortalSPKI[] =
+    "sha256/fjZPHewEHTrMDX3I1ecEIeoy3WFxHyGplOLv28kIbtI=";

[estark, 2017/02/08 20:31:17]

Magic value; I suggest adding a comment explaining from whence it came.

[meacer, 2017/02/08 22:34:32]

Done.

+
 // Test class that mimics a URL request with a certificate whose SPKI hash is in
 // ssl_error_assistant.asciipb resource. A better way of testing the SPKI hashes
 // inside the resource bundle would be to serve the actual certificate from the
 // embedded test server, but the test server can only serve a limited number of
 // predefined certificates.
 class SSLUICaptivePortalListResourceBundleTest
     : public CertVerifierBrowserTest {
  public:
   SSLUICaptivePortalListResourceBundleTest()
       : CertVerifierBrowserTest(),
         https_server_(net::EmbeddedTestServer::TYPE_HTTPS),
         https_server_mismatched_(net::EmbeddedTestServer::TYPE_HTTPS) {
     https_server_.ServeFilesFromSourceDirectory(base::FilePath(kDocRoot));
 
     https_server_mismatched_.SetSSLConfig(
         net::EmbeddedTestServer::CERT_MISMATCHED_NAME);
     https_server_mismatched_.AddDefaultHandlers(base::FilePath(kDocRoot));
   }
 
   void SetUp() override {
     CertVerifierBrowserTest::SetUp();
     SSLErrorHandler::ResetConfigForTesting();
-    SetUpCertVerifier(0, net::OK);
+    SetUpCertVerifier(0, net::OK, std::string());
   }
 
   void TearDown() override {
     SSLErrorHandler::ResetConfigForTesting();
     CertVerifierBrowserTest::TearDown();
   }
 
  protected:
-  void SetUpCertVerifier(net::CertStatus cert_status, int net_result) {
+  void SetUpCertVerifier(net::CertStatus cert_status,
+                         int net_result,
+                         const std::string& spki_hash) {
     scoped_refptr<net::X509Certificate> cert(https_server_.GetCertificate());
     net::CertVerifyResult verify_result;
     verify_result.is_issued_by_known_root =
         (net_result != net::ERR_CERT_AUTHORITY_INVALID);
     verify_result.verified_cert = cert;
     verify_result.cert_status = cert_status;
 
     // Set the SPKI hash to captive-portal.badssl.com leaf certificate. This

[estark, 2017/02/08 20:31:17]

Ah, I think this comment or something like it belongs on line 4022 now.

[meacer, 2017/02/08 22:34:32]

Done.

     // doesn't match the actual cert (ok_cert.pem) but is good enough for
     // testing.
-    net::HashValue hash;
-    ASSERT_TRUE(
-        hash.FromString("sha256/fjZPHewEHTrMDX3I1ecEIeoy3WFxHyGplOLv28kIbtI="));
-    verify_result.public_key_hashes.push_back(hash);
+    if (!spki_hash.empty()) {
+      net::HashValue hash;
+      ASSERT_TRUE(hash.FromString(spki_hash));
+      verify_result.public_key_hashes.push_back(hash);
+    }
     mock_cert_verifier()->AddResultForCert(cert, verify_result, net_result);
   }
 
   net::EmbeddedTestServer* https_server() { return &https_server_; }
   net::EmbeddedTestServer* https_server_mismatched() {
     return &https_server_mismatched_;
   }
 
  private:
   net::EmbeddedTestServer https_server_;
   net::EmbeddedTestServer https_server_mismatched_;
 };
 
 }  // namespace
 
 // Same as CaptivePortalCertificateList_Enabled_FromProto, but this time the
 // cert's SPKI hash is listed in ssl_error_assistant.asciipb.
-IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListResourceBundleTest,
-                       Enabled_FromResource) {
+IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListResourceBundleTest, Enabled) {
   base::test::ScopedFeatureList scoped_feature_list;
   scoped_feature_list.InitFromCommandLine(
       "CaptivePortalCertificateList" /* enabled */,
       std::string() /* disabled */);
   ASSERT_TRUE(https_server()->Start());
   base::HistogramTester histograms;
 
   // Mark the server's cert as a captive portal cert.
   SetUpCertVerifier(net::CERT_STATUS_COMMON_NAME_INVALID,
-                    net::ERR_CERT_COMMON_NAME_INVALID);
+                    net::ERR_CERT_COMMON_NAME_INVALID, kCaptivePortalSPKI);
 
   // Navigate to an unsafe page on the server. The captive portal interstitial
   // should be displayed since CaptivePortalCertificateList feature is enabled.
   WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
   SSLInterstitialTimerObserver interstitial_timer_observer(tab);
   ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/"));
   content::WaitForInterstitialAttach(tab);
 
   InterstitialPage* interstitial_page = tab->GetInterstitialPage();
   ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting,
             interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
   EXPECT_FALSE(interstitial_timer_observer.timer_started());
 
   // Check that the histogram for the captive portal cert was recorded.
   histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 3);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::HANDLE_ALL, 1);
   histograms.ExpectBucketCount(
       SSLErrorHandler::GetHistogramNameForTesting(),
       SSLErrorHandler::SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::CAPTIVE_PORTAL_CERT_FOUND, 1);
 }
 
+// Same as SSLUICaptivePortalListResourceBundleTest.Enabled, but this time the
+// proto is dynamically updated (e.g. by the component updater). The dynamic
+// update should always override the proto loaded from the resource bundle.
+IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListResourceBundleTest,
+                       Enabled_DynamicUpdate) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitFromCommandLine(
+      "CaptivePortalCertificateList" /* enabled */,
+      std::string() /* disabled */);
+  ASSERT_TRUE(https_server()->Start());
+
+  // Set interstitial delay to zero.

[estark, 2017/02/08 20:31:17]

nit: explain why this is needed

[meacer, 2017/02/08 22:34:32]

Not needed, removed.

+  SSLErrorHandler::SetInterstitialDelayForTesting(base::TimeDelta());
+  // Mark the server's cert as a captive portal cert.
+  SetUpCertVerifier(net::CERT_STATUS_COMMON_NAME_INVALID,
+                    net::ERR_CERT_COMMON_NAME_INVALID, kCaptivePortalSPKI);
+
+  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+
+  {
+    // Dynamically update the SSL error assistant config, do not include the
+    // captive portal SPKI hash.
+    auto config_proto =
+        base::MakeUnique<chrome_browser_ssl::SSLErrorAssistantConfig>();
+    config_proto->add_captive_portal_cert()->set_sha256_hash(
+        "sha256/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+    config_proto->add_captive_portal_cert()->set_sha256_hash(
+        "sha256/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
+    SSLErrorHandler::SetErrorAssistantProto(std::move(config_proto));
+
+    // Navigate to an unsafe page on the server. A generic SSL interstitial
+    // should be displayed because the dynamic update doesn't contain the hash
+    // of the captive portal certificate.
+    base::HistogramTester histograms;
+    SSLInterstitialTimerObserver interstitial_timer_observer(tab);
+    ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/"));
+    content::WaitForInterstitialAttach(tab);
+
+    InterstitialPage* interstitial_page = tab->GetInterstitialPage();
+    ASSERT_EQ(SSLBlockingPage::kTypeForTesting,
+              interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
+    EXPECT_TRUE(interstitial_timer_observer.timer_started());
+
+    // Check that the histogram was recorded for an SSL interstitial.
+    histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                                2);
+    histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                                 SSLErrorHandler::HANDLE_ALL, 1);
+    histograms.ExpectBucketCount(
+        SSLErrorHandler::GetHistogramNameForTesting(),
+        SSLErrorHandler::SHOW_SSL_INTERSTITIAL_OVERRIDABLE, 1);
+  }
+  {
+    // Dynamically update the error assistant config and add the captive portal
+    // SPKI hash.
+    auto config_proto =
+        base::MakeUnique<chrome_browser_ssl::SSLErrorAssistantConfig>();
+    config_proto->set_version_id(1);
+    config_proto->add_captive_portal_cert()->set_sha256_hash(
+        "sha256/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+    config_proto->add_captive_portal_cert()->set_sha256_hash(
+        "sha256/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
+    config_proto->add_captive_portal_cert()->set_sha256_hash(
+        kCaptivePortalSPKI);
+    SSLErrorHandler::SetErrorAssistantProto(std::move(config_proto));
+
+    // Navigate to the unsafe page again. This time, a captive portal
+    // interstitial should be displayed.
+    base::HistogramTester histograms;
+    SSLInterstitialTimerObserver interstitial_timer_observer(tab);
+    ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/"));
+    content::WaitForInterstitialAttach(tab);
+
+    InterstitialPage* interstitial_page = tab->GetInterstitialPage();
+    ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting,
+              interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
+    EXPECT_FALSE(interstitial_timer_observer.timer_started());
+
+    // Check that the histogram was recorded for a captive portal interstitial.
+    histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                                3);
+    histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                                 SSLErrorHandler::HANDLE_ALL, 1);
+    histograms.ExpectBucketCount(
+        SSLErrorHandler::GetHistogramNameForTesting(),
+        SSLErrorHandler::SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE, 1);
+    histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                                 SSLErrorHandler::CAPTIVE_PORTAL_CERT_FOUND, 1);
+  }
+  {
+    // Try dynamically updating the error assistant config with the an empty

[estark, 2017/02/08 20:31:17]

nit: extra "the"

[meacer, 2017/02/08 22:34:32]

Done.

+    // config with the same version number. The update should be ignored, and a
+    // captive portal interstitial should stil be displayed.
+    auto empty_config =
+        base::MakeUnique<chrome_browser_ssl::SSLErrorAssistantConfig>();
+    empty_config->set_version_id(1);
+    SSLErrorHandler::SetErrorAssistantProto(std::move(empty_config));
+
+    // Navigate to the unsafe page again. This time, a captive portal
+    // interstitial should be displayed.
+    base::HistogramTester histograms;
+    SSLInterstitialTimerObserver interstitial_timer_observer(tab);
+    ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/"));
+    content::WaitForInterstitialAttach(tab);
+
+    InterstitialPage* interstitial_page = tab->GetInterstitialPage();
+    ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting,
+              interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
+    EXPECT_FALSE(interstitial_timer_observer.timer_started());
+
+    // Check that the histogram was recorded for a captive portal interstitial.
+    histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                                3);
+    histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                                 SSLErrorHandler::HANDLE_ALL, 1);
+    histograms.ExpectBucketCount(
+        SSLErrorHandler::GetHistogramNameForTesting(),
+        SSLErrorHandler::SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE, 1);
+    histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                                 SSLErrorHandler::CAPTIVE_PORTAL_CERT_FOUND, 1);
+  }
+}
+
 // Same as SSLUICaptivePortalNameMismatchTest, but this time the error is
 // authority-invalid. Captive portal interstitial should not be shown.
 IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListResourceBundleTest,
-                       Enabled_FromResource_AuthorityInvalid) {
+                       Enabled_AuthorityInvalid) {
   base::test::ScopedFeatureList scoped_feature_list;
   scoped_feature_list.InitFromCommandLine(
       "CaptivePortalCertificateList" /* enabled */,
       std::string() /* disabled */);
   ASSERT_TRUE(https_server()->Start());
   base::HistogramTester histograms;
 
   // Set interstitial delay to zero.
   SSLErrorHandler::SetInterstitialDelayForTesting(base::TimeDelta());
   // Mark the server's cert as a captive portal cert, but with an
   // authority-invalid error.
   SetUpCertVerifier(net::CERT_STATUS_AUTHORITY_INVALID,
-                    net::ERR_CERT_AUTHORITY_INVALID);
+                    net::ERR_CERT_AUTHORITY_INVALID, kCaptivePortalSPKI);
 
   // Navigate to an unsafe page on the server. CaptivePortalCertificateList
   // feature is enabled but the error is not a name mismatch, so a generic SSL
   // interstitial should be displayed.
   WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
   SSLInterstitialTimerObserver interstitial_timer_observer(tab);
   ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/"));
   content::WaitForInterstitialAttach(tab);
 
   InterstitialPage* interstitial_page = tab->GetInterstitialPage();
@@ skipping 24 matching lines @@
 
   ASSERT_TRUE(https_server_mismatched_.Start());
   base::HistogramTester histograms;
 
   // Mark the server's cert as a captive portal cert.
   const net::HashValue server_spki_hash =
       GetSPKIHash(https_server_mismatched_.GetCertificate().get());
   chrome_browser_ssl::SSLErrorAssistantConfig config_proto;
   config_proto.add_captive_portal_cert()->set_sha256_hash(
       server_spki_hash.ToString());
-  SSLErrorHandler::SetErrorAssistantProtoForTesting(config_proto);
+  SSLErrorHandler::SetErrorAssistantProto(config_proto);
 
   // Navigate to an unsafe page on the server. The captive portal interstitial
   // should be displayed since CaptivePortalCertificateList feature is enabled.
   WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
   SSLInterstitialTimerObserver interstitial_timer_observer(tab);
   ui_test_utils::NavigateToURL(
       browser(), https_server_mismatched_.GetURL("/ssl/blank_page.html"));
   content::WaitForInterstitialAttach(tab);
 
   InterstitialPage* interstitial_page = tab->GetInterstitialPage();
@@ skipping 16 matching lines @@
 
 // Visit a page over https that contains a frame with a redirect.
 
 // XMLHttpRequest insecure content in synchronous mode.
 
 // XMLHttpRequest insecure content in asynchronous mode.
 
 // XMLHttpRequest over bad ssl in synchronous mode.
 
 // XMLHttpRequest over OK ssl in synchronous mode.