Move 'frame-src' CSP checks into FrameFetchContext.

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

 /*
  * Copyright (C) 2013 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 496 matching lines @@
     case Resource::XSLStyleSheet:
         ASSERT(RuntimeEnabledFeatures::xsltEnabled());
     case Resource::SVGDocument:
         if (!securityOrigin->canRequest(url)) {
             printAccessDeniedMessage(url);
             return ResourceRequestBlockedReasonOrigin;
         }
         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());
         if (!frame()->loader().client()->allowScriptFromSource(!frame()->settings() || frame()->settings()->scriptEnabled(), url)) {
             frame()->loader().client()->didNotAllowScript();
             // TODO(estark): Use a different ResourceRequestBlockedReason
             // here, since this check has nothing to do with
             // CSP. https://crbug.com/600795
             return ResourceRequestBlockedReasonCSP;
         }
@@ skipping 23 matching lines @@
     // folks block mixed content with a CSP policy, they don't get a warning.
     // They'll still get a warning in the console about CSP blocking the load.
     MixedContentChecker::ReportingStatus mixedContentReporting = forPreload ?
         MixedContentChecker::SuppressReport : MixedContentChecker::SendReport;
     if (MixedContentChecker::shouldBlockFetch(frame(), resourceRequest, url, mixedContentReporting))
         return ResourceRequestBlockedReasonMixedContent;
 
     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());
     if (m_documentLoader)
         return frame()->loader().client()->isControlledByServiceWorker(*m_documentLoader);
     // m_documentLoader is null while loading resources from an HTML import.
     // In such cases whether the request is controlled by ServiceWorker or not
     // is determined by the document loader of the frame.
     return frame()->loader().client()->isControlledByServiceWorker(*frame()->loader().documentLoader());
 }
@@ skipping 219 matching lines @@
 }
 
 DEFINE_TRACE(FrameFetchContext)
 {
     visitor->trace(m_document);
     visitor->trace(m_documentLoader);
     FetchContext::trace(visitor);
 }
 
 } // namespace blink