Move 'frame-src' CSP checks into FrameFetchContext.

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

Index: third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
index 6ee224534ab2ca6fe7960b45c00cf96f4f1bc161..55ff6dce44ecc81059c39894ab28f5c84190cb3f 100644
--- a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
@@ -514,18 +514,8 @@ ResourceRequestBlockedReason FrameFetchContext::canRequestInternal(Resource::Typ
         break;
     }
 
-    // FIXME: Convert this to check the isolated world's Content Security Policy once webkit.org/b/104520 is solved.
-    bool shouldBypassMainWorldCSP = frame()->script().shouldBypassMainWorldCSP() || options.contentSecurityPolicyOption == DoNotCheckContentSecurityPolicy;
-
-    // Don't send CSP messages for preloads, we might never actually display those items.
-    ContentSecurityPolicy::ReportingStatus cspReporting = forPreload ?
-        ContentSecurityPolicy::SuppressReport : ContentSecurityPolicy::SendReport;
-
-    if (m_document) {
-        DCHECK(m_document->contentSecurityPolicy());
-        if (!shouldBypassMainWorldCSP && !m_document->contentSecurityPolicy()->allowRequest(resourceRequest.requestContext(), url, redirectStatus, cspReporting))
-            return ResourceRequestBlockedReasonCSP;
-    }
+    if (contentSecurityPolicyBlocksRequest(type, resourceRequest, url, options, forPreload, redirectStatus))

[Mike West, 2016/06/02 13:39:30]

Does this extraction make you happier, Yoav? :)

[Yoav Weiss, 2016/06/02 14:13:37]

way happier :D

+        return ResourceRequestBlockedReasonCSP;
 
     if (type == Resource::Script || type == Resource::ImportResource) {
         ASSERT(frame());
@@ -569,6 +559,38 @@ ResourceRequestBlockedReason FrameFetchContext::canRequestInternal(Resource::Typ
     return ResourceRequestBlockedReasonNone;
 }
 
+bool FrameFetchContext::contentSecurityPolicyBlocksRequest(Resource::Type type, const ResourceRequest& resourceRequest, const KURL& url, const ResourceLoaderOptions& options, bool forPreload, ResourceRequest::RedirectStatus redirectStatus) const
+{
+    // FIXME: Convert this to check the isolated world's Content Security Policy once webkit.org/b/104520 is solved.
+    if (!frame()->script().shouldBypassMainWorldCSP() && options.contentSecurityPolicyOption == CheckContentSecurityPolicy) {
+        // Don't send CSP messages for preloads, we might never actually display those items.
+        ContentSecurityPolicy::ReportingStatus cspReporting = forPreload ?
+            ContentSecurityPolicy::SuppressReport : ContentSecurityPolicy::SendReport;
+        if (m_document) {
+            DCHECK(m_document->contentSecurityPolicy());
+            if (!m_document->contentSecurityPolicy()->allowRequest(resourceRequest.requestContext(), url, redirectStatus, cspReporting))
+                return true;
+        } else if (type == Resource::MainResource) {
+            // When loading the main document of an iframe, we won't have a document
+            // yet (so |csp| will be nullptr). We instead need to grab the frame's

[alexmos, 2016/06/02 22:21:07]

nit: I don't see |csp| defined anywhere in this function :)

[Mike West, 2016/06/06 08:40:10]

Done. Too much refactoring. :)

+            // parent's policy in order to perform 'frame-src' checks:
+            if (Frame* parentFrame = frame()->tree().parent()) {

[dcheng, 2016/06/02 21:48:56]

How does CSP inheritance work? Is it always strictly from the parent? Are there
any edge cases where it would not be inherited from the parent? I'm thinking of
something like https://crbug.com/583445, where document.open() changes the URL
to 'about:blank', yet the SecurityOrigin should not be inherited from the
parent.

[Mike West, 2016/06/06 08:40:10]

In this case, I think pulling the policy from the parent is fine. The goal of
`frame-src` is to allow a page to control the resources which it embeds. That
is, it controls the things which can be loaded into an `<iframe>`.

We only hit this case for requests which are navigating a frame, and since the
embedder's policy is the thing we need to look to when determining what it
allows itself to embed, I think we're correct in grabbing the parent frame's
policy if it exists.

+                if (!parentFrame->securityContext()->contentSecurityPolicy()->allowChildFrameFromSource(url, redirectStatus, cspReporting)) {
+                    // TODO(mkwst): If we cancel the request after a redirect, we never instantiate
+                    // a document, and therefore don't inherit the loader's sandbox flags, or trigger
+                    // a load event. This is strange.
+                    if (redirectStatus == ResourceRequest::RedirectStatus::FollowedRedirect) {
+                        frame()->document()->enforceSandboxFlags(SandboxOrigin);
+                        frame()->owner()->dispatchLoad();
+                    }
+                    return true;
+                }
+            }
+        }
+    }
+    return false;
+}
+
 bool FrameFetchContext::isControlledByServiceWorker() const
 {
     ASSERT(m_documentLoader || frame()->loader().documentLoader());