Split CSP into pre- and post-upgrade checks

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

Index: third_party/WebKit/Source/core/loader/FrameLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
index 8cd77648a4dd0116286c5cf0df7be66d44d27041..79b097f9281ca8c90899efa8dee67898f64e100a 100644
--- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -139,6 +139,63 @@ static void CheckForLegacyProtocolInSubresource(
       document, UseCounter::kLegacyProtocolEmbeddedAsSubresource);
 }
 
+static NavigationPolicy MaybeCheckCSP(
+    const ResourceRequest& request,
+    NavigationType type,
+    LocalFrame* frame,
+    NavigationPolicy policy,
+    bool should_check_main_world_content_security_policy,
+    bool browser_side_navigation_enabled,
+    ContentSecurityPolicy::CheckHeaderType check_header_type) {
+  // If we're loading content into |frame| (NavigationPolicyCurrentTab), check
+  // against the parent's Content Security Policy and kill the load if that
+  // check fails, unless we should bypass the main world's CSP.
+  if (policy == kNavigationPolicyCurrentTab &&
+      should_check_main_world_content_security_policy &&
+      // TODO(arthursonzogni): 'frame-src' check is disabled on the
+      // renderer side with browser-side-navigation, but is enforced on the
+      // browser side. See http://crbug.com/692595 for understanding why it
+      // can't be enforced on both sides instead.
+      !browser_side_navigation_enabled) {
+    Frame* parent_frame = frame->Tree().Parent();
+    if (parent_frame) {
+      ContentSecurityPolicy* parent_policy =
+          parent_frame->GetSecurityContext()->GetContentSecurityPolicy();
+      if (!parent_policy->AllowFrameFromSource(
+              request.Url(), request.GetRedirectStatus(),
+              SecurityViolationReportingPolicy::kReport, check_header_type)) {
+        // Fire a load event, as timing attacks would otherwise reveal that the
+        // frame was blocked. This way, it looks like every other cross-origin
+        // page load.
+        frame->GetDocument()->EnforceSandboxFlags(kSandboxOrigin);
+        frame->Owner()->DispatchLoad();
+        return kNavigationPolicyIgnore;
+      }
+    }
+  }
+
+  bool is_form_submission = type == kNavigationTypeFormSubmitted ||
+                            type == kNavigationTypeFormResubmitted;
+  if (is_form_submission &&
+      // 'form-action' check in the frame that is navigating is disabled on the
+      // renderer side when PlzNavigate is enabled, but is enforced on the
+      // browser side instead.
+      // N.B. check in the frame that initiates the navigation stills occurs in
+      // blink and is not enforced on the browser-side.
+      // TODO(arthursonzogni) The 'form-action' check should be fully disabled
+      // in blink when browser side navigation is enabled, except when the form
+      // submission doesn't trigger a navigation(i.e. javascript urls). Please
+      // see https://crbug.com/701749
+      !browser_side_navigation_enabled &&
+      !frame->GetDocument()->GetContentSecurityPolicy()->AllowFormAction(
+          request.Url(), request.GetRedirectStatus(),
+          SecurityViolationReportingPolicy::kReport, check_header_type)) {
+    return kNavigationPolicyIgnore;
+  }
+
+  return policy;
+}
+
 ResourceRequest FrameLoader::ResourceRequestForReload(
     FrameLoadType frame_load_type,
     const KURL& override_url,
@@ -1370,51 +1427,12 @@ NavigationPolicy FrameLoader::ShouldContinueForNavigationPolicy(
     return kNavigationPolicyCurrentTab;
 
   Settings* settings = frame_->GetSettings();
-  bool browser_side_navigation_enabled =
-      settings && settings->GetBrowserSideNavigationEnabled();
-
-  // If we're loading content into |m_frame| (NavigationPolicyCurrentTab), check
-  // against the parent's Content Security Policy and kill the load if that
-  // check fails, unless we should bypass the main world's CSP.
-  if (policy == kNavigationPolicyCurrentTab &&
-      should_check_main_world_content_security_policy ==
-          kCheckContentSecurityPolicy &&
-      // TODO(arthursonzogni): 'frame-src' check is disabled on the
-      // renderer side with browser-side-navigation, but is enforced on the
-      // browser side. See http://crbug.com/692595 for understanding why it
-      // can't be enforced on both sides instead.
-      !browser_side_navigation_enabled) {
-    Frame* parent_frame = frame_->Tree().Parent();
-    if (parent_frame) {
-      ContentSecurityPolicy* parent_policy =
-          parent_frame->GetSecurityContext()->GetContentSecurityPolicy();
-      if (!parent_policy->AllowFrameFromSource(request.Url(),
-                                               request.GetRedirectStatus())) {
-        // Fire a load event, as timing attacks would otherwise reveal that the
-        // frame was blocked. This way, it looks like every other cross-origin
-        // page load.
-        frame_->GetDocument()->EnforceSandboxFlags(kSandboxOrigin);
-        frame_->Owner()->DispatchLoad();
-        return kNavigationPolicyIgnore;
-      }
-    }
-  }
-
-  bool is_form_submission = type == kNavigationTypeFormSubmitted ||
-                            type == kNavigationTypeFormResubmitted;
-  if (is_form_submission &&
-      // 'form-action' check in the frame that is navigating is disabled on the
-      // renderer side when PlzNavigate is enabled, but is enforced on the
-      // browser side instead.
-      // N.B. check in the frame that initiates the navigation stills occurs in
-      // blink and is not enforced on the browser-side.
-      // TODO(arthursonzogni) The 'form-action' check should be fully disabled
-      // in blink when browser side navigation is enabled, except when the form
-      // submission doesn't trigger a navigation(i.e. javascript urls). Please
-      // see https://crbug.com/701749
-      !browser_side_navigation_enabled &&
-      !frame_->GetDocument()->GetContentSecurityPolicy()->AllowFormAction(
-          request.Url(), request.GetRedirectStatus())) {
+  if (MaybeCheckCSP(request, type, frame_, policy,
+                    should_check_main_world_content_security_policy ==
+                        kCheckContentSecurityPolicy,
+                    settings && settings->GetBrowserSideNavigationEnabled(),
+                    ContentSecurityPolicy::CheckHeaderType::kCheckEnforce) ==
+      kNavigationPolicyIgnore) {
     return kNavigationPolicyIgnore;
   }
 
@@ -1439,6 +1457,31 @@ NavigationPolicy FrameLoader::ShouldContinueForNavigationPolicy(
   return kNavigationPolicyIgnore;
 }
 
+NavigationPolicy FrameLoader::ShouldContinueForRedirectNavigationPolicy(
+    const ResourceRequest& request,
+    const SubstituteData& substitute_data,
+    DocumentLoader* loader,
+    ContentSecurityPolicyDisposition
+        should_check_main_world_content_security_policy,
+    NavigationType type,
+    NavigationPolicy policy,
+    FrameLoadType frame_load_type,
+    bool is_client_redirect,
+    HTMLFormElement* form) {
+  Settings* settings = frame_->GetSettings();
+  // Check report-only CSP policies, which are not checked by
+  // ShouldContinueForNavigationPolicy.
+  MaybeCheckCSP(request, type, frame_, policy,
+                should_check_main_world_content_security_policy ==
+                    kCheckContentSecurityPolicy,
+                settings && settings->GetBrowserSideNavigationEnabled(),
+                ContentSecurityPolicy::CheckHeaderType::kCheckReportOnly);
+  return ShouldContinueForNavigationPolicy(
+      request, substitute_data, loader,
+      should_check_main_world_content_security_policy, type, policy,
+      frame_load_type, is_client_redirect, form);
+}
+
 NavigationPolicy FrameLoader::CheckLoadCanStart(
     FrameLoadRequest& frame_load_request,
     FrameLoadType type,
@@ -1453,6 +1496,20 @@ NavigationPolicy FrameLoader::CheckLoadCanStart(
   // request.
   ResourceRequest& resource_request = frame_load_request.GetResourceRequest();
   RecordLatestRequiredCSP();
+  // Before modifying the request, check report-only CSP headers to give the
+  // site owner a chance to learn about requests that need to be modified.
+  //
+  // TODO(estark): this doesn't work with --enable-browser-side-navigation,
+  // wherein 'frame-src' is checked in the browser process. Figure out what to
+  // do; maybe with browser-side navigation the upgrade should be happening in
+  // the browser process too. See also https://crbug.com/692595

[Mike West, 2017/04/19 08:42:13]

Ugh. Yes. This is a mess.

Given infinite time, we should stop doing one set of checks in the browser, and
another in Blink. Given the world we live in, let's punt it.

+  Settings* settings = frame_->GetSettings();
+  MaybeCheckCSP(
+      resource_request, navigation_type, frame_, navigation_policy,
+      frame_load_request.ShouldCheckMainWorldContentSecurityPolicy() ==
+          kCheckContentSecurityPolicy,
+      settings && settings->GetBrowserSideNavigationEnabled(),
+      ContentSecurityPolicy::CheckHeaderType::kCheckReportOnly);
   ModifyRequestForCSP(resource_request, nullptr);
 
   return ShouldContinueForNavigationPolicy(