Track mixed content by origin, not by URL

When a page runs active mixed content (or an active resource loaded
with a certificate error), the origin's host is marked in
SSLHostStateDelegate. The SSLHostStateDelegate is then queried to set
mixed content flags on the NavigationEntry, which feeds into the
SecurityLevel of the page. Before this CL, the host of the
NavigationEntry's URL was being used to query SSLHostStateDelegate,
but this means that mixed content flags weren't being set correctly
for pages where the URL didn't match the actual origin (for example,
about:blank). This CL changes SSLManager to query SSLHostStateDelegate
with the committed origin of the root FrameNavigationEntry instead of
the URL.

As noted in the test in this CL, mixed content is still not being
tracked properly for the initial about:blank navigation in a window or
frame. That is tracked in https://crbug.com/1038765.

This change creates a slightly weird UI; when an about:blank page is
marked with active mixed content, the "about:" part is crossed-out (as
seen in https://crbug.com/526218). A follow-up CL will fix that by only
striking-through https:// schemes in the omnibox.

Bug: 609527
Change-Id: Ia0f13290a5769006edea3719642a11faefe9ae8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1986189
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728294}

chrome/browser/ssl/ssl_browsertest.cc

@@ -7747,6 +7747,62 @@ IN_PROC_BROWSER_TEST_F(RecurrentInterstitialBrowserTest,
       SSLErrorControllerClient::RecurrentErrorAction::kProceed, 1);
 }
 
+// Tests that mixed content is tracked by origin, not by URL. This is tested by
+// checking that mixed content flags are set appropriately for about:blank URLs
+// (who inherit the origin of their opener).
+//
+// Note: we test that mixed content flags are propagated from an opener page to
+// about:blank, but not the other way around. This is because there is no way
+// for a mixed content flag to propagate from about:blank to a different
+// tab. Passive mixed content flags are not propagated from one tab to another,
+// and for active mixed content, there's no way to bypass mixed content blocking
+// on about:blank pages, so there's no way that the origin would get flagged for
+// active mixed content from an about:blank page. (There's no way to bypass
+// mixed content blocking on about:blank pages because the bypass is implemented
+// as a content setting, which doesn't apply to about:blank.)
+IN_PROC_BROWSER_TEST_F(SSLUITest, ActiveMixedContentTrackedByOrigin) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  ASSERT_TRUE(https_server_.Start());
+
+  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+
+  std::string replacement_path = GetFilePathWithHostAndPortReplacement(
+      "/ssl/page_runs_insecure_content.html",
+      embedded_test_server()->host_port_pair());
+
+  // The insecure script is allowed to load because SSLUITestBase sets the
+  // --allow-running-insecure-content flag.
+  ui_test_utils::NavigateToURL(browser(),
+                               https_server_.GetURL(replacement_path));
+  ssl_test_util::CheckAuthenticationBrokenState(
+      tab, CertError::NONE, AuthState::RAN_INSECURE_CONTENT);
+
+  // Open a new tab from the current page. After an initial navigation,
+  // navigate it to about:blank and check that the about:blank page is
+  // downgraded, because it shares an origin with |tab| which ran mixed
+  // content.
+  //
+  // Note that the security indicator is not downgraded on the initial
+  // about:blank navigation in the new tab. Initial about:blank navigations
+  // don't have navigation entries (yet), so there is no way to track the mixed
+  // content state for these navigations. See https://crbug.com/1038765.
+  ui_test_utils::TabAddedWaiter tab_waiter(browser());
+  ASSERT_TRUE(content::ExecJs(tab, "w = window.open()"));
+  tab_waiter.Wait();
+  WebContents* opened_tab = browser()->tab_strip_model()->GetWebContentsAt(1);
+  content::TestNavigationObserver first_navigation(opened_tab);
+  ASSERT_TRUE(content::ExecJs(
+      tab, content::JsReplace("w.location.href = $1",
+                              embedded_test_server()->GetURL("/title1.html"))));
+  first_navigation.Wait();
+  ssl_test_util::CheckUnauthenticatedState(opened_tab, AuthState::NONE);
+  content::TestNavigationObserver about_blank_navigation(opened_tab);
+  ASSERT_TRUE(content::ExecJs(tab, "w.location.href = 'about:blank'"));
+  about_blank_navigation.Wait();
+  ssl_test_util::CheckAuthenticationBrokenState(
+      opened_tab, CertError::NONE, AuthState::RAN_INSECURE_CONTENT);
+}
+
 // TODO(jcampan): more tests to do below.
 
 // Visit a page over https that contains a frame with a redirect.

components/security_state/core/security_state.cc

@@ -173,6 +173,14 @@ SecurityLevel GetSecurityLevel(
     return NONE;
   }
 
+  // Downgrade the security level for active insecure subresources. This comes
+  // before handling non-cryptographic schemes below, because secure pages with
+  // non-cryptographic schemes (e.g., about:blank) can still have mixed content.
+  if (visible_security_state.ran_mixed_content ||
+      visible_security_state.ran_content_with_cert_errors) {
+    return kRanInsecureContentLevel;
+  }
+
   // Choose the appropriate security level for requests to HTTP and remaining
   // pseudo URLs (blob:, filesystem:). filesystem: is a standard scheme so does
   // not need to be explicitly listed here.
@@ -192,12 +200,6 @@ SecurityLevel GetSecurityLevel(
     return NONE;
   }
 
-  // Downgrade the security level for active insecure subresources.
-  if (visible_security_state.ran_mixed_content ||
-      visible_security_state.ran_content_with_cert_errors) {
-    return kRanInsecureContentLevel;
-  }
-
   // Downgrade the security level for pages loaded over legacy TLS versions.
   if (base::FeatureList::IsEnabled(
           security_state::features::kLegacyTLSWarnings) &&

content/browser/ssl/ssl_manager.cc

@@ -377,20 +377,28 @@ bool SSLManager::UpdateEntry(NavigationEntryImpl* entry,
   // necessarily have site instances.  Without a process, the entry can't
   // possibly have insecure content.  See bug https://crbug.com/12423.
   if (site_instance && ssl_host_state_delegate_) {
-    std::string host = entry->GetURL().host();
-    int process_id = site_instance->GetProcess()->GetID();
-    if (ssl_host_state_delegate_->DidHostRunInsecureContent(
-            host, process_id, SSLHostStateDelegate::MIXED_CONTENT)) {
-      entry->GetSSL().content_status |= SSLStatus::RAN_INSECURE_CONTENT;
-    }
+    const base::Optional<url::Origin>& entry_origin =
+        entry->root_node()->frame_entry->committed_origin();
+    // In some cases (e.g., unreachable URLs), navigation entries might not have
+    // origins attached to them. We don't care about tracking mixed content for
+    // those cases.
+    if (entry_origin.has_value()) {
+      const std::string& host = entry_origin->host();
+      int process_id = site_instance->GetProcess()->GetID();
+      if (ssl_host_state_delegate_->DidHostRunInsecureContent(
+              host, process_id, SSLHostStateDelegate::MIXED_CONTENT)) {
+        entry->GetSSL().content_status |= SSLStatus::RAN_INSECURE_CONTENT;
+      }
 
-    // Only record information about subresources with cert errors if the
-    // main page is HTTPS with a certificate.
-    if (entry->GetURL().SchemeIsCryptographic() &&
-        entry->GetSSL().certificate &&
-        ssl_host_state_delegate_->DidHostRunInsecureContent(
-            host, process_id, SSLHostStateDelegate::CERT_ERRORS_CONTENT)) {
-      entry->GetSSL().content_status |= SSLStatus::RAN_CONTENT_WITH_CERT_ERRORS;
+      // Only record information about subresources with cert errors if the
+      // main page is HTTPS with a certificate.
+      if (entry->GetURL().SchemeIsCryptographic() &&
+          entry->GetSSL().certificate &&
+          ssl_host_state_delegate_->DidHostRunInsecureContent(
+              host, process_id, SSLHostStateDelegate::CERT_ERRORS_CONTENT)) {
+        entry->GetSSL().content_status |=
+            SSLStatus::RAN_CONTENT_WITH_CERT_ERRORS;
+      }
     }
   }