PlzNavigate: Move navigation-level mixed content checks to the browser.

content/browser/frame_host/mixed_content_navigation_throttle.cc

Index: content/browser/frame_host/mixed_content_navigation_throttle.cc
diff --git a/content/browser/frame_host/mixed_content_navigation_throttle.cc b/content/browser/frame_host/mixed_content_navigation_throttle.cc
new file mode 100644
index 0000000000000000000000000000000000000000..866b07f137cf276416ba1089d8822deda6261d8e
--- /dev/null
+++ b/content/browser/frame_host/mixed_content_navigation_throttle.cc
@@ -0,0 +1,385 @@
+// Copyright 2017 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 "content/browser/frame_host/mixed_content_navigation_throttle.h"
+
+#include <algorithm>
+
+#include "base/memory/ptr_util.h"
+#include "content/browser/frame_host/frame_tree.h"
+#include "content/browser/frame_host/frame_tree_node.h"
+#include "content/browser/frame_host/navigation_handle_impl.h"
+#include "content/browser/frame_host/render_frame_host_delegate.h"
+#include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/common/frame_messages.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/common/browser_side_navigation_policy.h"
+#include "content/public/common/content_client.h"
+#include "content/public/common/origin_util.h"
+#include "content/public/common/web_preferences.h"
+#include "net/base/url_util.h"
+#include "url/gurl.h"
+#include "url/origin.h"
+#include "url/url_constants.h"
+#include "url/url_util.h"
+
+namespace {
+
+using namespace content;
+
+// Should return the same value as SchemeRegistry::shouldTreatURLSchemeAsSecure.
+bool IsSecureScheme(const std::string& scheme) {
+  const std::vector<std::string>& secure_schemes = url::GetSecureSchemes();
+  return std::find(secure_schemes.begin(), secure_schemes.end(), scheme) !=
+         secure_schemes.end();
+}
+
+// Should return the same value as SchemeRegistry::shouldTreatURLSchemeAsSecure.
+bool HasPotentiallySecureScheme(const GURL& url) {
+  return IsSecureScheme(url.scheme());
+}
+
+// Should return the same value as SecurityOrigin::isLocal (and consequentially
+// SchemeRegistry::shouldTreatURLSchemeAsLocal).
+bool HasLocalScheme(const GURL& url) {
+  const std::vector<std::string>& local_schemes = url::GetLocalSchemes();
+  return std::find(local_schemes.begin(), local_schemes.end(), url.scheme()) !=
+         local_schemes.end();
+}
+
+// This reflects the result expected from SecurityOrigin::isUnique (not its
+// logic). It takes into account how SecurityOrigin::create might return unique
+// origins for URLS that cause SchemeRegistry::shouldTreatURLSchemeAsNoAccess to
+// return true.
+bool IsUniqueScheme(const GURL& url) {
+  const std::vector<std::string>& no_access_schemes = url::GetNoAccessSchemes();
+  return std::find(no_access_schemes.begin(), no_access_schemes.end(),
+                   url.scheme()) != no_access_schemes.end();
+}
+
+// Should return the same value as SecurityOrigin::isLocal and
+// SchemeRegistry::shouldTreatURLSchemeAsCORSEnabled.
+bool ShouldTreatURLSchemeAsCORSEnabled(const GURL& url) {
+  const std::vector<std::string>& cors_schemes = url::GetCORSEnabledSchemes();
+  return std::find(cors_schemes.begin(), cors_schemes.end(), url.scheme()) !=
+         cors_schemes.end();
+}
+
+// Should return the same value as SecurityOrigin::isSecure.
+bool SecurityOriginIsSecure(const GURL& url) {

[nasko, 2017/01/23 22:32:38]

nit: IsOriginSecure? We don't have "SecurityOrigin" concept in browser code,
which makes reading the name of this method a bit awkward.

[carlosk, 2017/02/08 02:59:03]

Done.

+  return HasPotentiallySecureScheme(url) ||
+         (url.SchemeIsFileSystem() && url.inner_url() &&
+          HasPotentiallySecureScheme(*url.inner_url())) ||

[nasko, 2017/01/23 22:32:38]

Let's use a different code pattern for extracting the data from Filesystem and
Blob URLs, so we are consistent across the codebase. Once we have one of these,
we can construct an url::Origin from the URL and check its scheme. An example
can be seen in ExtensionNavigationThrottle::WillStartRequest.

[carlosk, 2017/02/08 02:59:03]

Done.

But looking at this again made me concerned about the SecurityOrigin
"uniqueness" aspect that is queried in the Blink implementation when it calls to
SecurityPolicy::isOriginWhiteListedTrustworthy. I did a lot of changes and I
think it is now properly done in the browser too.

+         (url.SchemeIsBlob() &&
+          HasPotentiallySecureScheme(GURL(url.GetContent()))) ||
+         IsOriginWhiteListedTrustworthy(url);
+}
+
+// Should return the same value as the resource URL checks made inside
+// MixedContentChecker::isMixedContent.
+bool IsUrlPotentiallySecure(const GURL& url) {
+  // TODO(carlosk): secure origin checks don't match between content and Blink
+  // hence this implementation here instead of a direct call to IsOriginSecure
+  // (in origin_util.cc). See https://crbug.com/629059.
+
+  bool is_secure = SecurityOriginIsSecure(url);
+
+  // blob: and filesystem: URLs never hit the network, and access is restricted
+  // to same-origin contexts, so they are not blocked either.

[nasko, 2017/01/23 22:32:38]

nit: no need for " either" as the rest of the comment from the Blink code is not
here and the "either" doesn't make much sense.

[carlosk, 2017/02/08 02:59:03]

Done.

+  if (url.SchemeIs(url::kBlobScheme) || url.SchemeIs(url::kFileSystemScheme))
+    is_secure |= true;
+
+  // These next checks mimics the behavior of
+  // SecurityOrigin::isPotentiallyTrustworthy without duplicating much of the

[nasko, 2017/01/23 22:32:38]

It is a bit sad to have this code and
https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_re....
Any chance we can combine the two somehow? Possibly in a separate CL?

[carlosk, 2017/02/08 02:59:03]

Done and this method changed quite considerably because of that.

+  // checks already performed previously (hence this not being enclosed in
+  // another method). The logic here will consider a unique scheme to be secure
+  // as done when SecurityOrigin::m_isUniqueOriginPotentiallyTrustworthy is
+  // true.
+  if (IsUniqueScheme(url) || HasLocalScheme(url) ||

[nasko, 2017/01/23 22:32:38]

This doesn't match what isPotentiallyTrustworthy does, as it checks the
uniqueness of the origin, which is not what this code does. You could very well
have an http(s) based document that is in a unique origin, through iframe
sandboxing. Would this work correctly in those cases?

[carlosk, 2017/02/08 02:59:03]

With my latest patch I tried to mirror the uniqueness check too.

I'm not sure of what would happen in the case you described. Should I add a new
to-test item to my ongoing list?

+      net::IsLocalhost(url.HostNoBrackets()))
+    is_secure |= true;
+
+  // TODO(mkwst): Remove this once 'localhost' is no longer considered
+  // potentially trustworthy.
+  if (is_secure && url.SchemeIs(url::kHttpScheme) &&
+      net::IsLocalHostname(url.HostNoBrackets(), nullptr)) {

[nasko, 2017/01/23 22:32:38]

Why IsLocalHostname vs IsLocalhost above? 

[carlosk, 2017/02/08 02:59:03]

This is due to a CL by mkwst@ that I had to mirror and caused the need to use of
both:
https://chromium.googlesource.com/chromium/src/+/130ee686fa00b617bfc001ceb3bb...

Note that the IsLocalhost call moved into IsPotentiallyTrustworthyOrigin.

+    is_secure = false;
+  }
+
+  return is_secure;
+}
+
+// This method should return the same results as
+// SchemeRegistry::shouldTreatURLSchemeAsRestrictingMixedContent.
+bool DoesOriginSchemeRestricsMixedContent(const url::Origin& origin) {
+  return origin.scheme() == url::kHttpsScheme;
+}
+
+void UpdateRendererOnMixedContentFound(NavigationHandleImpl* handle_impl,
+                                       const GURL& mixed_content_url,
+                                       bool was_allowed,
+                                       bool for_redirect) {
+  // TODO(carlosk): the root node should never be considered as being/having
+  // mixed content for now. Once/if the browser should also check form submits
+  // for mixed content than this will be allowed to happen and this DCHECK
+  // should be updated.
+  DCHECK(handle_impl->frame_tree_node()->parent());
+  RenderFrameHost* rfh = handle_impl->frame_tree_node()->current_frame_host();
+  rfh->Send(new FrameMsg_MixedContentFound(
+      rfh->GetRoutingID(), mixed_content_url, handle_impl->GetURL(),
+      handle_impl->request_context_type(), was_allowed, for_redirect));
+}
+
+}  // namespace
+
+namespace content {
+
+// static
+std::unique_ptr<NavigationThrottle>
+MixedContentNavigationThrottle::CreateThrottleForNavigation(
+    NavigationHandle* navigation_handle) {
+  if (IsBrowserSideNavigationEnabled())
+    return base::WrapUnique(
+        new MixedContentNavigationThrottle(navigation_handle));
+  return nullptr;
+}
+
+MixedContentNavigationThrottle::MixedContentNavigationThrottle(
+    NavigationHandle* navigation_handle)
+    : NavigationThrottle(navigation_handle) {
+  DCHECK(IsBrowserSideNavigationEnabled());
+}
+
+MixedContentNavigationThrottle::~MixedContentNavigationThrottle() {}
+
+ThrottleCheckResult MixedContentNavigationThrottle::WillStartRequest() {
+  bool should_block = ShouldBlockNavigation(false);
+  return should_block ? ThrottleCheckResult::CANCEL
+                      : ThrottleCheckResult::PROCEED;
+}
+
+ThrottleCheckResult MixedContentNavigationThrottle::WillRedirectRequest() {
+  // Upon redirects the same checks are to be executed as for requests.
+  bool should_block = ShouldBlockNavigation(true);
+  return should_block ? ThrottleCheckResult::CANCEL
+                      : ThrottleCheckResult::PROCEED;
+}
+
+ThrottleCheckResult MixedContentNavigationThrottle::WillProcessResponse() {
+  // TODO(carlosk): At this point we are about to process the request response.
+  // So if we ever need to, here/now it is a good moment to check for the final
+  // attained security level of the connection. For instance, does it use an
+  // outdated protocol? The implementation should be based off
+  // MixedContentChecker::handleCertificateError. See https://crbug.com/576270.
+  return ThrottleCheckResult::PROCEED;
+}
+
+// Based off of MixedContentChecker::shouldBlockFetch.
+bool MixedContentNavigationThrottle::ShouldBlockNavigation(bool for_redirect) {
+  NavigationHandleImpl* handle_impl =
+      static_cast<NavigationHandleImpl*>(navigation_handle());
+  FrameTreeNode* node = handle_impl->frame_tree_node();
+
+  // Find the parent node where mixed content is characterized, if any.
+  FrameTreeNode* mixed_content_node =
+      InWhichFrameIsContentMixed(node, handle_impl->GetURL());
+  if (!mixed_content_node) {
+    MaybeSendBlinkFeatureUsageReport();
+    return false;
+  }
+
+  // From this point on we know this is not a main frame navigation and that
+  // there is mixed content. Now let's decide if it's OK to proceed with it.
+
+  const WebPreferences& prefs = mixed_content_node->current_frame_host()
+                                    ->render_view_host()
+                                    ->GetWebkitPreferences();
+
+  ReportBasicMixedContentFeatures(handle_impl->request_context_type(),
+                                  handle_impl->mixed_content_context_type(),
+                                  prefs);
+
+  // If we're in strict mode, we'll automagically fail everything, and
+  // intentionally skip the client/embedder checks in order to prevent degrading
+  // the site's security UI.
+  bool block_all_mixed_content = !!(
+      mixed_content_node->current_replication_state().insecure_request_policy &
+      blink::kBlockAllMixedContent);
+  bool strict_mode =
+      prefs.strict_mixed_content_checking || block_all_mixed_content;
+
+  blink::WebMixedContentContextType mixed_context_type =
+      handle_impl->mixed_content_context_type();
+
+  if (!ShouldTreatURLSchemeAsCORSEnabled(handle_impl->GetURL())) {

[nasko, 2017/01/23 22:32:38]

nit: no need of {} for one line if statements.

[carlosk, 2017/02/08 02:59:03]

Done.

+    mixed_context_type = blink::WebMixedContentContextType::OptionallyBlockable;
+  }
+
+  bool allowed = false;
+  RenderFrameHostDelegate* frame_host_delegate =
+      node->current_frame_host()->delegate();
+  switch (mixed_context_type) {
+    case blink::WebMixedContentContextType::OptionallyBlockable:
+      allowed = !strict_mode;
+      if (allowed) {
+        frame_host_delegate->PassiveInsecureContentFound(handle_impl->GetURL());
+        frame_host_delegate->DidDisplayInsecureContent();
+      }
+      break;
+
+    case blink::WebMixedContentContextType::Blockable: {
+      // Note: from the renderer side implementation it seems like we don't need
+      // to care about reporting
+      // blink::UseCounter::BlockableMixedContentInSubframeBlocked because it is
+      // only triggered for sub-resources which are not checked for in the
+      // browser.
+      bool should_ask_embedder =
+          !strict_mode && (!prefs.strictly_block_blockable_mixed_content ||
+                           prefs.allow_running_insecure_content);
+      allowed =
+          should_ask_embedder &&
+          frame_host_delegate->ShouldAllowRunningInsecureContent(
+              handle_impl->GetWebContents(),
+              prefs.allow_running_insecure_content,
+              mixed_content_node->current_origin(), handle_impl->GetURL());
+      if (allowed) {
+        const GURL& origin_url = mixed_content_node->current_url().GetOrigin();
+        frame_host_delegate->DidRunInsecureContent(origin_url,
+                                                   handle_impl->GetURL());
+        GetContentClient()->browser()->RecordURLMetric(
+            "ContentSettings.MixedScript.RanMixedScript", origin_url);
+        mixed_content_features_.insert(MIXED_CONTENT_BLOCKABLE_ALLOWED);
+      }
+      break;
+    }
+
+    case blink::WebMixedContentContextType::ShouldBeBlockable:
+      allowed = !strict_mode;
+      if (allowed)
+        frame_host_delegate->DidDisplayInsecureContent();
+      break;
+
+    case blink::WebMixedContentContextType::NotMixedContent:
+      NOTREACHED();
+      break;
+  };
+
+  UpdateRendererOnMixedContentFound(
+      handle_impl, mixed_content_node->current_url(), allowed, for_redirect);
+  MaybeSendBlinkFeatureUsageReport();
+
+  return !allowed;
+}
+
+FrameTreeNode* MixedContentNavigationThrottle::InWhichFrameIsContentMixed(
+    FrameTreeNode* node,
+    const GURL& url) {
+  // Main frame navigations cannot be mixed content.
+  // TODO(carlosk): except for form submissions which might be supported in the
+  // future.
+  if (node->IsMainFrame())
+    return nullptr;
+
+  // There's no mixed content if any of these are true:
+  // - The navigated URL is potentially secure.
+  // - Neither the root nor parent frames have secure origins.
+  FrameTreeNode* mixed_content_node = nullptr;
+  FrameTreeNode* root = node->frame_tree()->root();
+  FrameTreeNode* parent = node->parent();
+  if (!IsUrlPotentiallySecure(url)) {
+    // TODO(carlosk): we might need to check more than just the immediate parent
+    // and the root. See https://crbug.com/623486.
+
+    // Checks if the root and then the immediate parent frames' origins are
+    // secure.
+    if (DoesOriginSchemeRestricsMixedContent(root->current_origin()))
+      mixed_content_node = root;
+    else if (DoesOriginSchemeRestricsMixedContent(parent->current_origin()))
+      mixed_content_node = parent;
+  }
+
+  // Note: The code below should behave the same way as the two calls to
+  // measureStricterVersionOfIsMixedContent from inside
+  // MixedContentChecker::inWhichFrameIs.
+  if (mixed_content_node) {
+    // We're currently only checking for mixed content in `https://*` contexts.
+    // What about other "secure" contexts the SchemeRegistry knows about? We'll
+    // use this method to measure the occurrence of non-webby mixed content to
+    // make sure we're not breaking the world without realizing it.
+    if (mixed_content_node->current_origin().scheme() != url::kHttpsScheme) {
+      mixed_content_features_.insert(
+          MIXED_CONTENT_IN_NON_HTTPS_FRAME_THAT_RESTRICTS_MIXED_CONTENT);
+    }
+  } else if (!SecurityOriginIsSecure(url) &&
+             (IsSecureScheme(root->current_origin().scheme()) ||
+              IsSecureScheme(parent->current_origin().scheme()))) {
+    mixed_content_features_.insert(
+        MIXED_CONTENT_IN_SECURE_FRAME_THAT_DOES_NOT_RESTRICT_MIXED_CONTENT);
+  }
+  return mixed_content_node;
+}
+
+void MixedContentNavigationThrottle::MaybeSendBlinkFeatureUsageReport() {
+  if (!mixed_content_features_.empty()) {
+    NavigationHandleImpl* handle_impl =
+        static_cast<NavigationHandleImpl*>(navigation_handle());
+    RenderFrameHost* rfh = handle_impl->frame_tree_node()->current_frame_host();
+    rfh->Send(new FrameMsg_BlinkFeatureUsageReport(rfh->GetRoutingID(),
+                                                   mixed_content_features_));
+    mixed_content_features_.clear();
+  }
+}
+
+// Based off of MixedContentChecker::count.
+void MixedContentNavigationThrottle::ReportBasicMixedContentFeatures(
+    RequestContextType request_context_type,
+    blink::WebMixedContentContextType mixed_content_context_type,
+    const WebPreferences& prefs) {
+  mixed_content_features_.insert(MIXED_CONTENT_PRESENT);
+
+  // Report any blockable content.
+  if (mixed_content_context_type ==
+      blink::WebMixedContentContextType::Blockable) {
+    mixed_content_features_.insert(MIXED_CONTENT_BLOCKABLE);
+    return;
+  }
+
+  // Note: as there's no mixed content checks for sub-resources on the browser
+  // side there should only be a subset of RequestContextType values that could
+  // ever be found here.
+  UseCounterFeature feature;
+  switch (request_context_type) {
+    case REQUEST_CONTEXT_TYPE_INTERNAL:
+      feature = MIXED_CONTENT_INTERNAL;
+      break;
+    case REQUEST_CONTEXT_TYPE_PREFETCH:
+      feature = MIXED_CONTENT_PREFETCH;
+      break;
+
+    case REQUEST_CONTEXT_TYPE_AUDIO:
+    case REQUEST_CONTEXT_TYPE_DOWNLOAD:
+    case REQUEST_CONTEXT_TYPE_FAVICON:
+    case REQUEST_CONTEXT_TYPE_IMAGE:
+    case REQUEST_CONTEXT_TYPE_PLUGIN:
+    case REQUEST_CONTEXT_TYPE_VIDEO:
+    default:
+      NOTREACHED() << "RequestContextType has value " << request_context_type
+                   << " and has WebMixedContentContextType of "
+                   << static_cast<int>(mixed_content_context_type);
+      return;
+  }
+  mixed_content_features_.insert(feature);
+}
+
+// static
+bool MixedContentNavigationThrottle::IsMixedContentForTesting(
+    const GURL& origin_url,
+    const GURL& url) {
+  const url::Origin origin(origin_url);
+  return !IsUrlPotentiallySecure(url) &&
+         DoesOriginSchemeRestricsMixedContent(origin);
+}
+
+}  // namespace content