Support --allow-insecure-localhost in content_shell.

This is a content switch, so it makes sense that content would respect
it by default, rather than just chrome. This also happens to make this
feature easier to test.

Bug: 1220829
Change-Id: Ibcd3bc4315c10f9cabb956a37a04bd09181ad762
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2966998
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900147}

chrome/browser/ssl/stateful_ssl_host_state_delegate_test.cc

@@ -737,32 +737,3 @@ IN_PROC_BROWSER_TEST_F(StatefulSSLHostStateDelegateExtensionTest,
                                net::ERR_CERT_DATE_INVALID, tab));
   EXPECT_FALSE(state->HasAllowException(kWWWGoogleHost, tab));
 }
-
-// When the flag is set, requests to localhost with invalid certificates
-// should be allowed.
-class AllowLocalhostErrorsSSLHostStateDelegateTest
-    : public StatefulSSLHostStateDelegateTest {
- protected:
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    StatefulSSLHostStateDelegateTest::SetUpCommandLine(command_line);
-    command_line->AppendSwitch(switches::kAllowInsecureLocalhost);
-  }
-};
-
-IN_PROC_BROWSER_TEST_F(AllowLocalhostErrorsSSLHostStateDelegateTest,
-                       LocalhostErrorWithFlag) {
-  // Serve the Google cert for localhost to generate an error.
-  scoped_refptr<net::X509Certificate> cert = GetOkCert();
-  content::WebContents* tab =
-      browser()->tab_strip_model()->GetActiveWebContents();
-  Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
-  content::SSLHostStateDelegate* state = profile->GetSSLHostStateDelegate();
-
-  EXPECT_EQ(content::SSLHostStateDelegate::ALLOWED,
-            state->QueryPolicy("localhost", *cert,
-                               net::ERR_CERT_COMMON_NAME_INVALID, tab));
-
-  EXPECT_EQ(content::SSLHostStateDelegate::ALLOWED,
-            state->QueryPolicy("127.0.0.1", *cert,
-                               net::ERR_CERT_COMMON_NAME_INVALID, tab));
-}

components/security_interstitials/content/stateful_ssl_host_state_delegate.cc

@@ -284,15 +284,6 @@ StatefulSSLHostStateDelegate::QueryPolicy(const std::string& host,
                                           content::WebContents* web_contents) {
   DCHECK(web_contents);
 
-  // If the appropriate flag is set, let requests on localhost go
-  // through even if there are certificate errors. Errors on localhost
-  // are unlikely to indicate actual security problems.
-  GURL url = GetSecureGURLForHost(host);
-  bool allow_localhost = base::CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kAllowInsecureLocalhost);
-  if (allow_localhost && net::IsLocalhost(url))
-    return ALLOWED;
-
   content::StoragePartition* storage_partition =
       browser_context_->GetStoragePartition(
           web_contents->GetMainFrame()->GetSiteInstance(),
@@ -312,6 +303,7 @@ StatefulSSLHostStateDelegate::QueryPolicy(const std::string& host,
     return DENIED;
   }
 
+  GURL url = GetSecureGURLForHost(host);
   std::unique_ptr<base::Value> value(
       host_content_settings_map_->GetWebsiteSetting(
           url, url, ContentSettingsType::SSL_CERT_DECISIONS, nullptr));

content/browser/ssl/ssl_manager.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
@@ -28,6 +29,8 @@
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/ssl_host_state_delegate.h"
 #include "content/public/common/content_client.h"
+#include "content/public/common/content_switches.h"
+#include "net/base/url_util.h"
 #include "net/cert/cert_status_flags.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
@@ -310,12 +313,22 @@ void SSLManager::DidRunContentWithCertErrors(const GURL& security_origin) {
 void SSLManager::OnCertError(std::unique_ptr<SSLErrorHandler> handler) {
   // First we check if we know the policy for this error.
   DCHECK(handler->ssl_info().is_valid());
-  SSLHostStateDelegate::CertJudgment judgment =
-      ssl_host_state_delegate_
-          ? ssl_host_state_delegate_->QueryPolicy(
-                handler->request_url().host(), *handler->ssl_info().cert.get(),
-                handler->cert_error(), handler->web_contents())
-          : SSLHostStateDelegate::DENIED;
+
+  SSLHostStateDelegate::CertJudgment judgment;
+  if (net::IsLocalhost(handler->request_url()) &&
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kAllowInsecureLocalhost)) {
+    // If the appropriate flag is set, let requests on localhost go
+    // through even if there are certificate errors. Errors on localhost
+    // are unlikely to indicate actual security problems.
+    judgment = SSLHostStateDelegate::ALLOWED;
+  } else if (ssl_host_state_delegate_) {
+    judgment = ssl_host_state_delegate_->QueryPolicy(
+        handler->request_url().host(), *handler->ssl_info().cert.get(),
+        handler->cert_error(), handler->web_contents());
+  } else {
+    judgment = SSLHostStateDelegate::DENIED;
+  }
 
   if (judgment == SSLHostStateDelegate::ALLOWED) {
     handler->ContinueRequest();

content/browser/web_contents/web_contents_impl_browsertest.cc

@@ -4864,4 +4864,56 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
   EXPECT_FALSE(shell()->web_contents()->IsCrashed());
 }
 
+class WebContentsImplInsecureLocalhostBrowserTest
+    : public WebContentsImplBrowserTest {
+ protected:
+  void SetUpOnMainThread() override {
+    WebContentsImplBrowserTest::SetUpOnMainThread();
+    https_server_.AddDefaultHandlers(GetTestDataFilePath());
+  }
+
+  net::EmbeddedTestServer& https_server() { return https_server_; }
+
+ private:
+  net::EmbeddedTestServer https_server_{net::EmbeddedTestServer::TYPE_HTTPS};
+};
+
+IN_PROC_BROWSER_TEST_F(WebContentsImplInsecureLocalhostBrowserTest,
+                       BlocksByDefault) {
+  https_server().SetSSLConfig(net::EmbeddedTestServer::CERT_EXPIRED);
+  ASSERT_TRUE(https_server().Start());
+  GURL url = https_server().GetURL("/title1.html");
+
+  NavigateToURLBlockUntilNavigationsComplete(shell(), url, 1);
+  EXPECT_TRUE(
+      IsLastCommittedEntryOfPageType(shell()->web_contents(), PAGE_TYPE_ERROR));
+}
+
+class WebContentsImplAllowInsecureLocalhostBrowserTest
+    : public WebContentsImplInsecureLocalhostBrowserTest {
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(switches::kAllowInsecureLocalhost);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(WebContentsImplAllowInsecureLocalhostBrowserTest,
+                       WarnsWithSwitch) {
+  https_server().SetSSLConfig(net::EmbeddedTestServer::CERT_EXPIRED);
+  ASSERT_TRUE(https_server().Start());
+  GURL url = https_server().GetURL("/title1.html");
+
+  WebContentsConsoleObserver observer(shell()->web_contents());
+  observer.SetFilter(base::BindRepeating(
+      [](const GURL& expected_url,
+         const WebContentsConsoleObserver::Message& message) {
+        return message.source_frame->GetLastCommittedURL() == expected_url;
+      },
+      url));
+  observer.SetPattern("*SSL certificate*");
+
+  ASSERT_TRUE(NavigateToURL(shell(), url));
+  observer.Wait();
+}
+
 }  // namespace content