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

third_party/WebKit/Source/core/loader/MixedContentChecker.cpp

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 125 matching lines @@
   NOTREACHED();
   return "resource";
 }
 
 }  // namespace
 
 static void measureStricterVersionOfIsMixedContent(Frame* frame,
                                                    const KURL& url) {
   // 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.
+  // use this method to measure the occurrence of non-webby mixed content to
+  // make sure we're not breaking the world without realizing it.
   SecurityOrigin* origin = frame->securityContext()->getSecurityOrigin();
   if (MixedContentChecker::isMixedContent(origin, url)) {
     if (origin->protocol() != "https") {
       UseCounter::count(
           frame,
           UseCounter::MixedContentInNonHTTPSFrameThatRestrictsMixedContent);
     }
   } else if (!SecurityOrigin::isSecure(url) &&
              SchemeRegistry::shouldTreatURLSchemeAsSecure(origin->protocol())) {
     UseCounter::count(
@@ skipping 129 matching lines @@
 }
 
 // static
 bool MixedContentChecker::shouldBlockFetch(
     LocalFrame* frame,
     WebURLRequest::RequestContext requestContext,
     WebURLRequest::FrameType frameType,
     ResourceRequest::RedirectStatus redirectStatus,
     const KURL& url,
     MixedContentChecker::ReportingStatus reportingStatus) {
+  // Frame-level loads are checked by the browser if PlzNavigate is enabled. No
+  // need to check them again here.
+  if (frame->settings()->getBrowserSideNavigationEnabled() &&
+      frameType != WebURLRequest::FrameTypeNone) {
+    return false;

[Mike West, 2017/01/13 13:20:00]

Does this change the timing of MIX and HSTS? That is, we currently check mixed
content first, then apply HSTS. It seems like this would change that, as we'd
send the request, upgrade it via HSTS, and only then kick into the throttle.
While that's the right thing to do in the long run, we shouldn't do it without
also changing the platform as per https://wicg.github.io/hsts-priming/.

Instead of returning false, why not just let the checks run through? Blink will
handle the initial request, and the browser will handle redirects. Seems like
the best of both worlds.

[carlosk, 2017/01/21 02:54:59]

I wasn't aware of HSTS until you mentioned it here so I don't know how these
changes would affect it. Are there tests I can run to verify that?

But your proposed solution of allowing checks to happen renderer-side as well
causes problems while running layout tests: in all cases where mixed content is
found but allowed the warning message end up being logged twice, once for each
detection in browser and renderer. Specific mixed content failures:

Regressions: Unexpected text-only failures (6)
  http/tests/security/mixedContent/insecure-iframe-in-iframe.html [ Failure ]
  http/tests/security/mixedContent/insecure-iframe-in-main-frame-allowed.html [
Failure ]
  http/tests/security/mixedContent/insecure-iframe-in-main-frame.html [ Failure
]
 
virtual/mojo-loading/http/tests/security/mixedContent/insecure-iframe-in-iframe.html
[ Failure ]
 
virtual/mojo-loading/http/tests/security/mixedContent/insecure-iframe-in-main-frame-allowed.html
[ Failure ]
 
virtual/mojo-loading/http/tests/security/mixedContent/insecure-iframe-in-main-frame.html
[ Failure ]

[Mike West, 2017/01/25 11:37:30]

I would hope that we have tests, but I wouldn't be at all surprised to find that
we don't. :) As a manual test, add `<iframe src="http://youtube.com/"></iframe>`
to `https://example.com/`. It shouldn't load. As a less-manual test, we'd
probably need to teach Blink's layout test engine how to register specific
`*.test` subdomains as HSTSized.
That surprises me, as we should be blocking the load before it hits the browser
if we consider it mixed content. Ah, looking at the tests, they all include
`testRunner.overridePreference("WebKitAllowRunningInsecureContent", true);`,
which explains things. Reworking those tests to treat frames as blockable
content (which is the behavior we ship) should resolve the double-warning.

It wouldn't resolve doubled error messages for folks who click through the
mixed-content shield, but since we're planning to remove that shield at some
point in the somewhat near future, I'm not terribly concerned about that case.

WDYT?

+  }
+
   Frame* effectiveFrame = effectiveFrameForFrameType(frame, frameType);
   Frame* mixedFrame =
       inWhichFrameIsContentMixed(effectiveFrame, frameType, url);
   if (!mixedFrame)
     return false;
 
   MixedContentChecker::count(mixedFrame, requestContext);
   if (ContentSecurityPolicy* policy =
           frame->securityContext()->contentSecurityPolicy())
     policy->reportMixedContent(url, redirectStatus);
@@ skipping 249 matching lines @@
   if (contextType == WebMixedContentContextType::Blockable) {
     client->didRunContentWithCertificateErrors(response.url());
   } else {
     // contextTypeFromRequestContext() never returns NotMixedContent (it
     // computes the type of mixed content, given that the content is mixed).
     DCHECK_NE(contextType, WebMixedContentContextType::NotMixedContent);
     client->didDisplayContentWithCertificateErrors(response.url());
   }
 }
 
+// static
+void MixedContentChecker::mixedContentFoundByTheBrowser(
+    LocalFrame* frame,
+    const KURL& mainResourceUrl,
+    const KURL& mixedContentUrl,
+    WebURLRequest::RequestContext requestContext,
+    bool wasAllowed,
+    bool hadRedirect) {
+  // Logs to the frame console.
+  logToConsoleAboutFetch(frame, mainResourceUrl, mixedContentUrl,
+                         requestContext, wasAllowed);
+  // Reports to the CSP policy.
+  ContentSecurityPolicy* policy =
+      frame->securityContext()->contentSecurityPolicy();
+  if (policy) {
+    policy->reportMixedContent(
+        mixedContentUrl, hadRedirect
+                             ? ResourceRequest::RedirectStatus::FollowedRedirect
+                             : ResourceRequest::RedirectStatus::NoRedirect);
+  }
+}
+
 WebMixedContentContextType MixedContentChecker::contextTypeForInspector(
     LocalFrame* frame,
     const ResourceRequest& request) {
   Frame* effectiveFrame =
       effectiveFrameForFrameType(frame, request.frameType());
 
   Frame* mixedFrame = inWhichFrameIsContentMixed(
       effectiveFrame, request.frameType(), request.url());
   if (!mixedFrame)
     return WebMixedContentContextType::NotMixedContent;
 
   // See comment in shouldBlockFetch() about loading the main resource of a
   // subframe.
   if (request.frameType() == WebURLRequest::FrameTypeNested &&
       !SchemeRegistry::shouldTreatURLSchemeAsCORSEnabled(
           request.url().protocol())) {
     return WebMixedContentContextType::OptionallyBlockable;
   }
 
   bool strictMixedContentCheckingForPlugin =
       mixedFrame->settings() &&
       mixedFrame->settings()->getStrictMixedContentCheckingForPlugin();
   return WebMixedContent::contextTypeFromRequestContext(
       request.requestContext(), strictMixedContentCheckingForPlugin);
 }
 
 }  // namespace blink