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

content/browser/frame_host/mixed_content_navigation_throttle.cc

+// Copyright 2016 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 "base/strings/stringprintf.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/common/browser_side_navigation_policy.h"
+#include "content/public/common/content_client.h"
+#include "content/public/common/origin_util.h"
+#include "content/public/common/url_constants.h"
+#include "content/public/common/web_preferences.h"
+#include "net/base/url_util.h"
+#include "third_party/WebKit/public/platform/WebMixedContent.h"
+#include "url/gurl.h"
+#include "url/origin.h"
+#include "url/url_constants.h"
+
+namespace {
+
+using namespace content;
+
+// Should return the same value as SchemeRegistry::shouldTreatURLSchemeAsSecure.
+bool IsSecureScheme(const std::string& scheme) {
+  // TODO(carlosk): find out how to properly handle these secure schemes. Should
+  // statically defined ones in Chrome/embedder come from a shared file? Should
+  // dynamically defined ones from extensions register both with browser and
+  // renderer code? See https://crbug.com/627502.
+  bool result =
+      // Note: schemes statically defined in secureSchemes()
+      scheme == url::kHttpsScheme || scheme == url::kAboutScheme ||
+      scheme == url::kDataScheme || scheme == url::kWssScheme ||
+      // Note: schemes "dynamically" registered with
+      // SchemeRegistry::registerURLSchemeAsSecure.
+      scheme == kChromeUIScheme;
+  return result;
+}
+
+// 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
+// SchemeRegistry::shouldTreatURLSchemeAsLocal.
+bool HasLocalScheme(const GURL& url) {
+  // TODO(carlosk): Same registration problem as described in
+  // HasPotentiallySecureScheme applies here. See https://crbug.com/627502.
+  bool result =
+      // Note: schemes statically defined in schemesWithUniqueOrigins()
+      url.SchemeIs(url::kAboutScheme) || url.SchemeIs(url::kJavaScriptScheme) ||
+      url.SchemeIs(url::kDataScheme) ||
+      // Note: schemes "dynamically" registered with
+      // SchemeRegistry::registerURLSchemeAsNoAccess. There's no content/
+      // re-definition of kChromeNativeScheme.
+      url.SchemeIs("chrome-native");
+  return result;
+}
+
+// Should return the same value as SecurityOrigin::isSecure.
+bool SecurityOriginIsSecure(const GURL& url) {
+  return HasPotentiallySecureScheme(url) ||
+         (url.SchemeIsFileSystem() && url.inner_url() &&
+          HasPotentiallySecureScheme(*url.inner_url())) ||
+         (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.
+  if (url.SchemeIs(url::kBlobScheme) || url.SchemeIs(url::kFileSystemScheme))
+    is_secure |= true;
+
+  // Mimics SecurityOrigin::isPotentiallyTrustworthy without duplicating (much)
+  // of the checks already done previously.
+  if (HasLocalScheme(url) || 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)) {
+    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;
+}
+
+bool ShouldTreatURLSchemeAsCORSEnabled(const GURL& url) {
+  // TODO(carlosk): for CORS schemes we have the exact same issue as for the
+  // secure schemes above. See callers to and references in
+  // WebSecurityPolicy::registerURLSchemeAsCORSEnabled. See
+  // https://crbug.com/627502.
+  return
+      // Note: CORS schemes statically defined in CORSEnabledSchemes()
+      url.SchemeIsHTTPOrHTTPS() || url.SchemeIs(url::kDataScheme) ||
+      // Note: CORS schemes "dynamically" registered in
+      // RenderThreadImpl::RegisterSchemes.
+      url.SchemeIs(kChromeUIScheme);
+}
+
+const char* TypeNameFromContext(RequestContextType context) {
+  // Note: the static_cast below is guaranteed by the static asserts in
+  // content/child/web_url_request_util.cc.
+  return blink::WebMixedContent::requestContextName(

[jam, 2016/12/28 20:41:00]

similarly here, can we just send an IPC to the renderer to log this? It could
then call that wekbit method.

[carlosk, 2017/01/05 06:10:08]

Done.

I split FrameMsg_MixedContentUpdate IPC in two: one for Blink feature reporting
and one for when mixed content is found. As logging only happens when mixed
content is found the handling of the latter on the renderer now also does the
logging.

Beyond avoiding the Blink call from content/browser this was also an improvement
because the new IPCs are simpler and clearer, and because the log function is
not duplicated anymore.

+      static_cast<blink::WebURLRequest::RequestContext>(context));
+}
+
+blink::WebMixedContent::ContextType MixedContextTypeFromContext(
+    RequestContextType context,
+    bool block_mixed_plugin_content) {
+  // Note: the static_cast below is guaranteed by the static asserts in
+  // content/child/web_url_request_util.cc.
+  return blink::WebMixedContent::contextTypeFromRequestContext(

[jam, 2016/12/28 20:41:00]

we only include enums/POD from webkit in the browser process (see comment in
content/browser/DEPS)

Can we send the mixed context blocking type from the renderer along with the
other request data in the IPC? This would handle all the request types from the
renderer, other than the browser-initiated navigations with plznavigate
(WebURLRequest::RequestContextLocation I believe). For
WebURLRequest::RequestContextLocation we can just hard code that it's blockable.

[carlosk, 2017/01/05 06:10:08]

This proved a tad more complicated than I assumed. I'll try to explain it below
otherwise a VC might be in order.

The values of WebMixedContent::ContextType is derived from
WebURLRequest::RequestContext but it needs to be "fixed" for the plugin case
based on the setting for "strictMixedContentCheckingForPlugin", which is stored
at the render-view level.

I initially considered adding the new member to ResourceRequest and deriving its
value in WebURLLoaderImpl::Context::Start where
ResourceRequest::fetch_request_context_type is set but it doesn't seem like I
have access to the render-view there (it lives in content/child).

So I'm considering these alternatives:

a) Move the implementation of WebMixedContent::contextTypeFromRequestContext
into content/common (or content/public/common) and make the former call into
that. This would allow the browser to call it too.

b) Derive the WebMixedContent::ContextType value without the
strict-plugin-checking information in WebURLLoaderImpl::Context::Start and then
patch it when the mixed content check happens on the browser.

IMO a) seems better: clearer and keeps the "conversion" logic in one single
place. However I'm not sure of the best/correct way to make Blink call into a
global function defined in content/ (or maybe a static class function).

Comments on these suggestions as well as new ideas are both welcome.

[jam, 2017/01/05 17:21:53]

The plugin setting is available in the renderer via WebPreferences.
RenderFrameImpl::willSendRequest can store that boolean on
content::RequestExtraData which can then be read by WebURLLoaderImpl. WDYT?
But this is called by blink right? Blink can't call into content, since that's a
layering violation (i.e. it would be a circular dependency for content to call
blink and vice versa).

[carlosk, 2017/01/10 02:10:44]

Done. The content::RequestExtraData idea worked.

+      static_cast<blink::WebURLRequest::RequestContext>(context),
+      block_mixed_plugin_content);
+}
+
+// Based off of MixedContentChecker::logToConsoleAboutFetch.
+void LogToConsoleAboutNavigation(FrameTreeNode* mixed_content_node,
+                                 NavigationHandleImpl* handle_impl,
+                                 const GURL& url,
+                                 bool allowed) {
+  FrameTreeNode* tree_node = handle_impl->frame_tree_node();
+  // TODO(carlosk): we should only use the very tree_node here for the form
+  // submission case. (D)CHECK for that when we get there. For now just check
+  // this is not the root node.
+  DCHECK(tree_node->parent());
+  RequestContextType request_context_type = handle_impl->request_context_type();
+
+  std::string message = base::StringPrintf(
+      "Mixed Content: The page at '%s' was loaded over HTTPS, but requested an "
+      "insecure %s '%s'. %s",
+      mixed_content_node->current_url().spec().c_str(),
+      TypeNameFromContext(request_context_type), url.spec().c_str(),
+      allowed ? "This content should also be served over HTTPS."
+              : "This request has been blocked; the content must be served "
+                "over HTTPS.");
+  ConsoleMessageLevel messageLevel =
+      allowed ? ConsoleMessageLevel::CONSOLE_MESSAGE_LEVEL_WARNING
+              : ConsoleMessageLevel::CONSOLE_MESSAGE_LEVEL_ERROR;
+  tree_node->current_frame_host()->AddMessageToConsole(messageLevel, message);
+}
+
+}  // namespace
+
+namespace content {
+
+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 must check the final security level of
+  // the connection! Does it use an outdated protocol? See
+  // MixedContentChecker::handleCertificateError
+  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 with mixed content, if any.
+  FrameTreeNode* mixed_content_node =
+      InWhichFrameIsContentMixed(node, handle_impl->GetURL());
+  if (!mixed_content_node) {
+    MaybeSendRendererUpdate(node->current_frame_host(), for_redirect, false);
+    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(), 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 strictMode =
+      prefs.strict_mixed_content_checking || block_all_mixed_content;
+
+  blink::WebMixedContent::ContextType mixed_context_type =
+      MixedContextTypeFromContext(handle_impl->request_context_type(),
+                                  prefs.block_mixed_plugin_content);
+
+  if (!ShouldTreatURLSchemeAsCORSEnabled(handle_impl->GetURL())) {
+    mixed_context_type =
+        blink::WebMixedContent::ContextType::OptionallyBlockable;
+  }
+
+  bool allowed = false;
+  ContentBrowserClient* browser_client = GetContentClient()->browser();
+  RenderFrameHostDelegate* frame_host_delegate =
+      node->current_frame_host()->delegate();
+  switch (mixed_context_type) {
+    case blink::WebMixedContent::ContextType::OptionallyBlockable:
+      allowed = !strictMode;
+      if (allowed) {
+        browser_client->PassiveInsecureContentFound(handle_impl->GetURL());
+        frame_host_delegate->DidDisplayInsecureContent();
+      }
+      break;
+
+    case blink::WebMixedContent::ContextType::Blockable: {
+      // Note: from the renderer side implementation it doesn't seem like we
+      // need to care about the UseCounter reporting of
+      // BlockableMixedContentInSubframeBlocked because it is only triggered for
+      // sub-resources which are not handled in the browser.
+      bool shouldAskEmbedder =
+          !strictMode && (!prefs.strictly_block_blockable_mixed_content ||
+                          prefs.allow_running_insecure_content);
+      allowed = shouldAskEmbedder &&
+                browser_client->ShouldAllowRunningInsecureContent(
+                    prefs.allow_running_insecure_content,
+                    mixed_content_node->current_origin(), handle_impl->GetURL(),
+                    handle_impl->GetWebContents());
+      if (allowed) {
+        const GURL& origin_url = mixed_content_node->current_url().GetOrigin();
+        frame_host_delegate->DidRunInsecureContent(origin_url,
+                                                   handle_impl->GetURL());
+        browser_client->RecordURLMetric(
+            "ContentSettings.MixedScript.RanMixedScript", origin_url);
+        mixed_content_features_.insert(MixedContentBlockableAllowed);
+      }
+      break;
+    }
+
+    case blink::WebMixedContent::ContextType::ShouldBeBlockable:
+      allowed = !strictMode;
+      if (allowed)
+        frame_host_delegate->DidDisplayInsecureContent();
+      break;
+
+    case blink::WebMixedContent::ContextType::NotMixedContent:
+      NOTREACHED();
+      break;
+  };
+
+  LogToConsoleAboutNavigation(mixed_content_node, handle_impl,
+                              handle_impl->GetURL(), allowed);
+  MaybeSendRendererUpdate(node->current_frame_host(), for_redirect, true);
+
+  return !allowed;
+}
+
+FrameTreeNode* MixedContentNavigationThrottle::InWhichFrameIsContentMixed(
+    FrameTreeNode* node,
+    const GURL& url) {
+  // Main frame navigations cannot be mixed content.
+  // TODO(carlosk): except for form submissions which will be dealt with later.
+  if (node->IsMainFrame())
+    return nullptr;
+
+  // There's no mixed content if any of these are true:
+  // - The navigated URL is potentially secure.
+  // - The root nor parent frames' origins are secure.
+  FrameTreeNode* mixed_content_node = nullptr;
+  FrameTreeNode* root = node->frame_tree()->root();
+  FrameTreeNode* parent = node->parent();
+  if (!IsUrlPotentiallySecure(url)) {
+    // TODO(carlosk): don't we need to check more than just the immediate parent
+    // and the root? Is it always the case that these two are the only sources
+    // for obtaining the "origin of the security context"?
+    // See https://crbug.com/623486.
+
+    // Checks if the root or immediate parent frame's origin are secure.
+    if (DoesOriginSchemeRestricsMixedContent(root->current_origin()))
+      mixed_content_node = root;
+    else if (DoesOriginSchemeRestricsMixedContent(parent->current_origin()))
+      mixed_content_node = parent;
+  }
+
+  // Note: This code below should behave the same way as 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 occurance of non-webby mixed content to
+    // make sure we're not breaking the world without realizing it.
+    // Note: Based off of measureStricterVersionOfIsMixedContent in
+    // MixedContentChecker.cpp.
+    // TODO(carlosk): this will only ever work once we allow registration of new
+    // potentially secure schemes. crbug.com/627502
+    if (mixed_content_node->current_origin().scheme() != url::kHttpsScheme) {
+      mixed_content_features_.insert(
+          MixedContentInNonHTTPSFrameThatRestrictsMixedContent);
+    }
+  } else if (!SecurityOriginIsSecure(url) &&
+             (IsSecureScheme(root->current_origin().scheme()) ||
+              IsSecureScheme(parent->current_origin().scheme()))) {
+    mixed_content_features_.insert(
+        MixedContentInSecureFrameThatDoesNotRestrictMixedContent);
+  }
+  return mixed_content_node;
+}
+
+void MixedContentNavigationThrottle::MaybeSendRendererUpdate(
+    RenderFrameHost* rfh,
+    bool for_redirect,
+    bool has_mixed_content) {
+  if (!mixed_content_features_.empty() || has_mixed_content) {
+    rfh->Send(new FrameMsg_MixedContentUpdate(
+        rfh->GetRoutingID(), mixed_content_features_, has_mixed_content,
+        has_mixed_content ? navigation_handle()->GetURL() : GURL(),
+        for_redirect));
+    mixed_content_features_.clear();
+  }
+}
+
+// Based off of MixedContentChecker::count.
+void MixedContentNavigationThrottle::ReportBasicMixedContentFeatures(
+    RequestContextType context,
+    const WebPreferences& prefs) {
+  mixed_content_features_.insert(MixedContentPresent);
+
+  // Report any blockable content.
+  blink::WebMixedContent::ContextType mixed_context_type =
+      MixedContextTypeFromContext(context, prefs.block_mixed_plugin_content);
+  if (mixed_context_type == blink::WebMixedContent::ContextType::Blockable) {
+    mixed_content_features_.insert(MixedContentBlockable);
+    return;
+  }
+
+  // Note: as there's no mixed content checks for sub resources on the browser
+  // there should only be a subset |context| values that could ever be found
+  // here.
+  UseCounterFeature feature;
+  switch (context) {
+    case REQUEST_CONTEXT_TYPE_INTERNAL:
+      feature = MixedContentInternal;
+      break;
+    case REQUEST_CONTEXT_TYPE_PREFETCH:
+      feature = MixedContentPrefetch;
+      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 " << context
+                   << " and has WebMixedContent::ContextType of "
+                   << static_cast<int>(mixed_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