PlzNavigate: Enforce 'frame-src' CSP on the browser.

content/browser/frame_host/ancestor_throttle.cc

Index: content/browser/frame_host/ancestor_throttle.cc
diff --git a/content/browser/frame_host/ancestor_throttle.cc b/content/browser/frame_host/ancestor_throttle.cc
index ca87f0a221d797c54f9304669bd016aa961c9bcc..7513df9d6f07cff7c95d2a243f80ba99fc3104ad 100644
--- a/content/browser/frame_host/ancestor_throttle.cc
+++ b/content/browser/frame_host/ancestor_throttle.cc
@@ -11,9 +11,11 @@
 #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/navigation_request.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/navigation_throttle.h"
+#include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/console_message_level.h"
 #include "net/http/http_response_headers.h"
 #include "url/origin.h"
@@ -165,6 +167,54 @@ AncestorThrottle::WillProcessResponse() {
   return NavigationThrottle::BLOCK_RESPONSE;
 }
 
+NavigationThrottle::ThrottleCheckResult
+AncestorThrottle::CheckContentSecurityPolicyFrameSrc(bool is_redirect) {
+  // If PlzNavigate is not enabled, "frame-src" can not be enforced on the
+  // browser-side since a NavigationRequest is needed below. It doesn't matter
+  // because it is still enforced on the renderer-side.
+  if (!IsBrowserSideNavigationEnabled())
+    return NavigationThrottle::PROCEED;
+
+  NavigationHandleImpl* handle =
+      static_cast<NavigationHandleImpl*>(navigation_handle());
+
+  const GURL& url = navigation_handle()->GetURL();
+  if (url.SchemeIs(url::kAboutScheme))
+    return NavigationThrottle::PROCEED;
+
+  // Allow the request when it bypasses the CSP of the parent frame.
+  // Note: it is possible that there is no navigation_request associated with
+  // this navigation, but it only happens when the navigation_handle was created
+  // by CreateNavigationHandleForTesting().
+  if (handle->frame_tree_node()->navigation_request() &&
+      handle->frame_tree_node()
+          ->navigation_request()
+          ->common_params()
+          .should_bypass_main_world_CSP) {
+    return NavigationThrottle::PROCEED;
+  }
+
+  auto parent = handle->frame_tree_node()->parent();
+  DCHECK(parent);
+
+  CSPContext* csp_context = parent->ContentSecurityPolicyContext();
+  if (!csp_context->Allow(parent->ContentSecurityPolicies(),
+                          CSPDirective::FrameSrc, url, is_redirect)) {
+    return NavigationThrottle::BLOCK_REQUEST;

[alexmos, 2017/02/10 22:59:53]

Will this result in loading a regular error page?  I remember we discussed
whether to load a blank page with a unique origin (data:,) temporarily until we
fix the error pages, because otherwise we can get the problems with the CWS kill
and more down the road as we tighten up CanCommitURL for OOPIFs
(https://crbug.com/622385, https://crbug.com/689733).  Can you check if you get
the kill in 622385 with this approach, if a parent page declares a CSP that
disallows the CWS URL?  Would the frame-src checks happen before XFO blocks it
using BLOCK_RESPONSE, for which you used data:, when you moved XFO over to the
browser?

[arthursonzogni, 2017/02/13 16:33:20]

Yes you are right. I forgot this.
XFO checks happens after 'frame-src'.
I will have to create a new ThrottleCheckResult for this.

[arthursonzogni, 2017/02/15 17:02:15]

I am working on a solution. See https://codereview.chromium.org/2697713005/
Now, the browser will not ask the renderer to commit an error page with an URL
that would make the browser kills the renderer if it tries to commit it.
Unfortunately, it only works with PlzNavigate. 

+  }
+
+  return NavigationThrottle::PROCEED;
+}
+
+NavigationThrottle::ThrottleCheckResult AncestorThrottle::WillStartRequest() {
+  return CheckContentSecurityPolicyFrameSrc(false);
+}
+
+NavigationThrottle::ThrottleCheckResult
+AncestorThrottle::WillRedirectRequest() {
+  return CheckContentSecurityPolicyFrameSrc(true);
+}
+
 AncestorThrottle::AncestorThrottle(NavigationHandle* handle)
     : NavigationThrottle(handle) {}