Changeset 206203 in webkit

Timestamp:

Sep 21, 2016 1:43:23 AM (8 years ago)

Author:

commit-queue@webkit.org

Message:

Refactor CachedResourceLoader::canRequest
​https://bugs.webkit.org/show_bug.cgi?id=162144

Patch by Youenn Fablet <​youenn@apple.com> on 2016-09-21
Reviewed by Darin Adler.

Covered by existing tests.

Simplifying CachedResourceLoader::canRequest by doing:

• CSP checks in another method
• Removing Same-Origin type-specific checks by setting FetchOptions::Mode appropriately in resource loader clients
• Moving script specific check in ScriptElement

Note that the last check may affect the loading behavior in the case scripts are enabled when starting the load
of a script, but gets disabled before receiving a redirection for the script load.

• dom/ProcessingInstruction.cpp:

(WebCore::ProcessingInstruction::checkStyleSheet): Setting XSLT stylesheet fetch mode to SameOrigin.

• dom/ScriptElement.cpp:

(WebCore::ScriptElement::requestScriptWithCache): Returning early if scripts are disabled.

• loader/CrossOriginPreflightChecker.cpp:

(WebCore::CrossOriginPreflightChecker::startPreflight): Bypassing CSP checks.

• loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::startLoadingMainResource): Bypassing CSP checks as CachedResourceLoader was not
checking them for MainResource.

• loader/DocumentThreadableLoader.cpp:

(WebCore::DocumentThreadableLoader::loadRequest): Ditto.

• loader/LinkLoader.cpp:

(WebCore::LinkLoader::preloadIfNeeded): Using new CachedResourceRequest constructor to enable moving the ResourceRequest.
(WebCore::LinkLoader::loadLink): Skipping CSP checks for link prefetch/subresources as CachedResourceLoader was
not checking them for Link Prefetch and Subresource types.

• loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::allowedByContentSecurityPolicy): Helper routine to check for CSP.
(WebCore::CachedResourceLoader::canRequest): Making use of introduced helper routine.
Simplified same origin check as all requests should have their options set.

• loader/cache/CachedResourceLoader.h:
• loader/cache/CachedResourceRequest.cpp:

(WebCore::CachedResourceRequest::CachedResourceRequest): More efficient constructor.

• loader/cache/CachedResourceRequest.h:
• loader/cache/CachedSVGDocumentReference.cpp:

(WebCore::CachedSVGDocumentReference::load): Setting fetch mode to SameOrigin.

• svg/SVGUseElement.cpp:

(WebCore::SVGUseElement::updateExternalDocument): Ditto.

• xml/XSLImportRule.cpp:

(WebCore::XSLImportRule::loadSheet): Ditto.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ProcessingInstruction.cpp (modified) (1 diff)
• dom/ScriptElement.cpp (modified) (1 diff)
• loader/CrossOriginPreflightChecker.cpp (modified) (1 diff)
• loader/DocumentLoader.cpp (modified) (1 diff)
• loader/DocumentThreadableLoader.cpp (modified) (1 diff)
• loader/LinkLoader.cpp (modified) (2 diffs)
• loader/cache/CachedResourceLoader.cpp (modified) (2 diffs)
• loader/cache/CachedResourceLoader.h (modified) (1 diff)
• loader/cache/CachedResourceRequest.cpp (modified) (1 diff)
• loader/cache/CachedResourceRequest.h (modified) (1 diff)
• loader/cache/CachedSVGDocumentReference.cpp (modified) (1 diff)
• svg/SVGUseElement.cpp (modified) (1 diff)
• xml/XSLImportRule.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-09-21  Youenn Fablet  <youenn@apple.com>
+
+        Refactor CachedResourceLoader::canRequest
+        https://bugs.webkit.org/show_bug.cgi?id=162144
+
+        Reviewed by Darin Adler.
+
+        Covered by existing tests.
+
+        Simplifying CachedResourceLoader::canRequest by doing:
+        - CSP checks in another method
+        - Removing Same-Origin type-specific checks by setting FetchOptions::Mode appropriately in resource loader clients
+        - Moving script specific check in ScriptElement
+
+        Note that the last check may affect the loading behavior in the case scripts are enabled when starting the load
+        of a script, but gets disabled before receiving a redirection for the script load.
+
+        * dom/ProcessingInstruction.cpp:
+        (WebCore::ProcessingInstruction::checkStyleSheet): Setting XSLT stylesheet fetch mode to SameOrigin.
+        * dom/ScriptElement.cpp:
+        (WebCore::ScriptElement::requestScriptWithCache): Returning early if scripts are disabled.
+        * loader/CrossOriginPreflightChecker.cpp:
+        (WebCore::CrossOriginPreflightChecker::startPreflight): Bypassing CSP checks.
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::startLoadingMainResource): Bypassing CSP checks as CachedResourceLoader was not
+        checking them for MainResource.
+        * loader/DocumentThreadableLoader.cpp:
+        (WebCore::DocumentThreadableLoader::loadRequest): Ditto.
+        * loader/LinkLoader.cpp:
+        (WebCore::LinkLoader::preloadIfNeeded): Using new CachedResourceRequest constructor to enable moving the ResourceRequest.
+        (WebCore::LinkLoader::loadLink): Skipping CSP checks for link prefetch/subresources as CachedResourceLoader was
+        not checking them for Link Prefetch and Subresource types.
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::allowedByContentSecurityPolicy): Helper routine to check for CSP.
+        (WebCore::CachedResourceLoader::canRequest): Making use of introduced helper routine.
+        Simplified same origin check as all requests should have their options set.
+        * loader/cache/CachedResourceLoader.h:
+        * loader/cache/CachedResourceRequest.cpp:
+        (WebCore::CachedResourceRequest::CachedResourceRequest): More efficient constructor.
+        * loader/cache/CachedResourceRequest.h:
+        * loader/cache/CachedSVGDocumentReference.cpp:
+        (WebCore::CachedSVGDocumentReference::load): Setting fetch mode to SameOrigin.
+        * svg/SVGUseElement.cpp:
+        (WebCore::SVGUseElement::updateExternalDocument): Ditto.
+        * xml/XSLImportRule.cpp:
+        (WebCore::XSLImportRule::loadSheet): Ditto.
+
 2016-09-21  Miguel Gomez  <magomez@igalia.com>
 

trunk/Source/WebCore/dom/ProcessingInstruction.cpp

@@ -138 +138 @@
             document().authorStyleSheets().addPendingSheet();
 
-            CachedResourceRequest request(ResourceRequest(document().completeURL(href)));
-#if ENABLE(XSLT)
-            if (m_isXSL)
-                m_cachedSheet = document().cachedResourceLoader().requestXSLStyleSheet(WTFMove(request));
-            else
+#if ENABLE(XSLT)
+            if (m_isXSL) {
+                auto options = CachedResourceLoader::defaultCachedResourceOptions();
+                options.mode = FetchOptions::Mode::SameOrigin;
+                m_cachedSheet = document().cachedResourceLoader().requestXSLStyleSheet({ResourceRequest(document().completeURL(href)), options});
+            } else
 #endif
             {
+                CachedResourceRequest request(ResourceRequest(document().completeURL(href)));
                 String charset = attrs.get("charset");
                 if (charset.isEmpty())

trunk/Source/WebCore/dom/ScriptElement.cpp

@@ -282 +282 @@
 CachedResourceHandle<CachedScript> ScriptElement::requestScriptWithCache(const URL& sourceURL, const String& nonceAttribute)
 {
-    bool hasKnownNonce = m_element.document().contentSecurityPolicy()->allowScriptWithNonce(nonceAttribute, m_element.isInUserAgentShadowTree());
+    Document& document = m_element.document();
+    auto* settings = document.settings();
+    if (settings && !settings->isScriptEnabled())
+        return nullptr;
+
+    ASSERT(document.contentSecurityPolicy());
+    bool hasKnownNonce = document.contentSecurityPolicy()->allowScriptWithNonce(nonceAttribute, m_element.isInUserAgentShadowTree());
     ResourceLoaderOptions options = CachedResourceLoader::defaultCachedResourceOptions();
     options.contentSecurityPolicyImposition = hasKnownNonce ? ContentSecurityPolicyImposition::SkipPolicyCheck : ContentSecurityPolicyImposition::DoPolicyCheck;
 
     CachedResourceRequest request(ResourceRequest(sourceURL), options);
-    request.setAsPotentiallyCrossOrigin(m_element.attributeWithoutSynchronization(HTMLNames::crossoriginAttr), m_element.document());
-
-    m_element.document().contentSecurityPolicy()->upgradeInsecureRequestIfNeeded(request.mutableResourceRequest(), ContentSecurityPolicy::InsecureRequestType::Load);
+    request.setAsPotentiallyCrossOrigin(m_element.attributeWithoutSynchronization(HTMLNames::crossoriginAttr), document);
+
+    document.contentSecurityPolicy()->upgradeInsecureRequestIfNeeded(request.mutableResourceRequest(), ContentSecurityPolicy::InsecureRequestType::Load);
 
     request.setCharset(scriptCharset());
     request.setInitiator(&element());
 
-    return m_element.document().cachedResourceLoader().requestScript(WTFMove(request));
+    return document.cachedResourceLoader().requestScript(WTFMove(request));
 }
 

trunk/Source/WebCore/loader/CrossOriginPreflightChecker.cpp

@@ -104 +104 @@
     options.referrerPolicy = m_loader.options().referrerPolicy;
     options.redirect = FetchOptions::Redirect::Manual;
+    options.contentSecurityPolicyImposition = ContentSecurityPolicyImposition::SkipPolicyCheck;
 
     CachedResourceRequest preflightRequest(createAccessControlPreflightRequest(m_request, m_loader.securityOrigin()), options);

trunk/Source/WebCore/loader/DocumentLoader.cpp

@@ -1519 +1519 @@
     RELEASE_LOG_IF_ALLOWED("startLoadingMainResource: Starting load (frame = %p, main = %d)", m_frame, m_frame->isMainFrame());
 
-    static NeverDestroyed<ResourceLoaderOptions> mainResourceLoadOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials, ClientCredentialPolicy::MayAskClientForCredentials, FetchOptions::Credentials::Include, SkipSecurityCheck, FetchOptions::Mode::NoCors, IncludeCertificateInfo, ContentSecurityPolicyImposition::DoPolicyCheck, DefersLoadingPolicy::AllowDefersLoading, CachingPolicy::AllowCaching);
+    static NeverDestroyed<ResourceLoaderOptions> mainResourceLoadOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials, ClientCredentialPolicy::MayAskClientForCredentials, FetchOptions::Credentials::Include, SkipSecurityCheck, FetchOptions::Mode::NoCors, IncludeCertificateInfo, ContentSecurityPolicyImposition::SkipPolicyCheck, DefersLoadingPolicy::AllowDefersLoading, CachingPolicy::AllowCaching);
     m_mainResource = m_cachedResourceLoader->requestMainResource(CachedResourceRequest(ResourceRequest(request), mainResourceLoadOptions));
 

trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp

@@ -364 +364 @@
 
     if (m_async) {
-        ThreadableLoaderOptions options = m_options;
+        ResourceLoaderOptions options = m_options;
         options.clientCredentialPolicy = m_sameOriginRequest ? ClientCredentialPolicy::MayAskClientForCredentials : ClientCredentialPolicy::CannotAskClientForCredentials;
+        options.contentSecurityPolicyImposition = ContentSecurityPolicyImposition::SkipPolicyCheck;
 
         CachedResourceRequest newRequest(WTFMove(request), options);

trunk/Source/WebCore/loader/LinkLoader.cpp

@@ -159 +159 @@
     ResourceRequest resourceRequest(document.completeURL(href));
     resourceRequest.setIgnoreForRequestCount(true);
-    CachedResourceRequest linkRequest(resourceRequest, CachedResource::defaultPriorityForResourceType(type.value()));
+    CachedResourceRequest linkRequest(WTFMove(resourceRequest), CachedResourceLoader::defaultCachedResourceOptions(), CachedResource::defaultPriorityForResourceType(type.value()));
     linkRequest.setInitiator("link");
 
@@ -201 +201 @@
             m_cachedLinkResource = nullptr;
         }
-        m_cachedLinkResource = document.cachedResourceLoader().requestLinkResource(type, CachedResourceRequest(ResourceRequest(document.completeURL(href)), priority));
+        ResourceLoaderOptions options = CachedResourceLoader::defaultCachedResourceOptions();
+        options.contentSecurityPolicyImposition = ContentSecurityPolicyImposition::SkipPolicyCheck;
+        m_cachedLinkResource = document.cachedResourceLoader().requestLinkResource(type, CachedResourceRequest(ResourceRequest(document.completeURL(href)), options, priority));
         if (m_cachedLinkResource)
             m_cachedLinkResource->addClient(this);

trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

@@ -385 +385 @@
 }
 
+bool CachedResourceLoader::allowedByContentSecurityPolicy(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, bool didReceiveRedirectResponse)
+{
+    if (options.contentSecurityPolicyImposition == ContentSecurityPolicyImposition::SkipPolicyCheck)
+        return true;
+
+    ASSERT(m_document);
+    ASSERT(m_document->contentSecurityPolicy());
+
+    auto redirectResponseReceived = didReceiveRedirectResponse ? ContentSecurityPolicy::RedirectResponseReceived::Yes : ContentSecurityPolicy::RedirectResponseReceived::No;
+
+    switch (type) {
+#if ENABLE(XSLT)
+    case CachedResource::XSLStyleSheet:
+#endif
+    case CachedResource::Script:
+        if (!m_document->contentSecurityPolicy()->allowScriptFromSource(url, false, redirectResponseReceived))
+            return false;
+        break;
+    case CachedResource::CSSStyleSheet:
+        if (!m_document->contentSecurityPolicy()->allowStyleFromSource(url, false, redirectResponseReceived))
+            return false;
+        break;
+    case CachedResource::SVGDocumentResource:
+    case CachedResource::ImageResource:
+        if (!m_document->contentSecurityPolicy()->allowImageFromSource(url, false, redirectResponseReceived))
+            return false;
+        break;
+#if ENABLE(SVG_FONTS)
+    case CachedResource::SVGFontResource:
+#endif
+    case CachedResource::FontResource:
+        if (!m_document->contentSecurityPolicy()->allowFontFromSource(url, false, redirectResponseReceived))
+            return false;
+        break;
+    case CachedResource::MediaResource:
+#if ENABLE(VIDEO_TRACK)
+    case CachedResource::TextTrackResource:
+#endif
+        if (!m_document->contentSecurityPolicy()->allowMediaFromSource(url, false, redirectResponseReceived))
+            return false;
+        break;
+    case CachedResource::RawResource:
+        return true;
+    default:
+        ASSERT_NOT_REACHED();
+    }
+    return true;
+}
+
 bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, bool forPreload, bool didReceiveRedirectResponse)
 {
@@ -394 +443 @@
     }
 
-    bool skipContentSecurityPolicyCheck = options.contentSecurityPolicyImposition == ContentSecurityPolicyImposition::SkipPolicyCheck;
-    ContentSecurityPolicy::RedirectResponseReceived redirectResponseReceived = didReceiveRedirectResponse ? ContentSecurityPolicy::RedirectResponseReceived::Yes : ContentSecurityPolicy::RedirectResponseReceived::No;
-
-    // Some types of resources can be loaded only from the same origin. Other types of resources, like Images, Scripts, and CSS, can be loaded from any URL.
-    // FIXME: We should remove that check and handle it by setting the correct ResourceLoaderOptions::mode.
-    switch (type) {
-    case CachedResource::MainResource:
-    case CachedResource::ImageResource:
-    case CachedResource::CSSStyleSheet:
-    case CachedResource::Script:
-#if ENABLE(SVG_FONTS)
-    case CachedResource::SVGFontResource:
-#endif
-    case CachedResource::MediaResource:
-    case CachedResource::FontResource:
-    case CachedResource::RawResource:
-#if ENABLE(LINK_PREFETCH)
-    case CachedResource::LinkPrefetch:
-    case CachedResource::LinkSubresource:
-#endif
-#if ENABLE(VIDEO_TRACK)
-    case CachedResource::TextTrackResource:
-#endif
-        if (options.mode == FetchOptions::Mode::SameOrigin && !isSameOriginDataURL(url, options, didReceiveRedirectResponse) &&!m_document->securityOrigin()->canRequest(url)) {
-            printAccessDeniedMessage(url);
-            return false;
-        }
-        break;
-    case CachedResource::SVGDocumentResource:
-#if ENABLE(XSLT)
-    case CachedResource::XSLStyleSheet:
-        if (!m_document->securityOrigin()->canRequest(url)) {
-            printAccessDeniedMessage(url);
-            return false;
-        }
-#endif
-        break;
-    }
-
-    switch (type) {
-#if ENABLE(XSLT)
-    case CachedResource::XSLStyleSheet:
-        if (!m_document->contentSecurityPolicy()->allowScriptFromSource(url, skipContentSecurityPolicyCheck, redirectResponseReceived))
-            return false;
-        break;
-#endif
-    case CachedResource::Script:
-        if (!m_document->contentSecurityPolicy()->allowScriptFromSource(url, skipContentSecurityPolicyCheck, redirectResponseReceived))
-            return false;
-        if (frame() && !frame()->settings().isScriptEnabled())
-            return false;
-        break;
-    case CachedResource::CSSStyleSheet:
-        if (!m_document->contentSecurityPolicy()->allowStyleFromSource(url, skipContentSecurityPolicyCheck, redirectResponseReceived))
-            return false;
-        break;
-    case CachedResource::SVGDocumentResource:
-    case CachedResource::ImageResource:
-        if (!m_document->contentSecurityPolicy()->allowImageFromSource(url, skipContentSecurityPolicyCheck, redirectResponseReceived))
-            return false;
-        break;
-#if ENABLE(SVG_FONTS)
-    case CachedResource::SVGFontResource:
-#endif
-    case CachedResource::FontResource: {
-        if (!m_document->contentSecurityPolicy()->allowFontFromSource(url, skipContentSecurityPolicyCheck, redirectResponseReceived))
-            return false;
-        break;
-    }
-    case CachedResource::MainResource:
-    case CachedResource::RawResource:
-#if ENABLE(LINK_PREFETCH)
-    case CachedResource::LinkPrefetch:
-    case CachedResource::LinkSubresource:
-#endif
-        break;
-    case CachedResource::MediaResource:
-#if ENABLE(VIDEO_TRACK)
-    case CachedResource::TextTrackResource:
-#endif
-        if (!m_document->contentSecurityPolicy()->allowMediaFromSource(url, skipContentSecurityPolicyCheck, redirectResponseReceived))
-            return false;
-        break;
-    }
+    // FIXME: Remove same-origin data URL flag since it was removed from fetch spec (see https://github.com/whatwg/fetch/issues/381).
+    if (options.mode == FetchOptions::Mode::SameOrigin && !isSameOriginDataURL(url, options, didReceiveRedirectResponse) && !m_document->securityOrigin()->canRequest(url)) {
+        printAccessDeniedMessage(url);
+        return false;
+    }
+
+    if (!allowedByContentSecurityPolicy(type, url, options, didReceiveRedirectResponse))
+        return false;
 
     // SVG Images have unique security rules that prevent all subresource requests except for data urls.

trunk/Source/WebCore/loader/cache/CachedResourceLoader.h

@@ -164 +164 @@
     bool shouldContinueAfterNotifyingLoadedFromMemoryCache(const CachedResourceRequest&, CachedResource*);
     bool checkInsecureContent(CachedResource::Type, const URL&) const;
+    bool allowedByContentSecurityPolicy(CachedResource::Type, const URL&, const ResourceLoaderOptions&, bool);
 
     void performPostLoadActions();

trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp

@@ -45 +45 @@
 }
 
-CachedResourceRequest::CachedResourceRequest(ResourceRequest&& resourceRequest, const ResourceLoaderOptions& options)
+CachedResourceRequest::CachedResourceRequest(ResourceRequest&& resourceRequest, const ResourceLoaderOptions& options, Optional<ResourceLoadPriority> priority)
     : m_resourceRequest(WTFMove(resourceRequest))
     , m_options(options)
-    , m_forPreload(false)
-    , m_defer(NoDefer)
-{
-}
-
-CachedResourceRequest::CachedResourceRequest(const ResourceRequest& resourceRequest, Optional<ResourceLoadPriority> priority)
-    : m_resourceRequest(resourceRequest)
-    , m_options(CachedResourceLoader::defaultCachedResourceOptions())
     , m_priority(priority)
     , m_forPreload(false)
     , m_defer(NoDefer)
-{
-}
-
-CachedResourceRequest::~CachedResourceRequest()
 {
 }

trunk/Source/WebCore/loader/cache/CachedResourceRequest.h

@@ -43 +43 @@
 
     explicit CachedResourceRequest(const ResourceRequest&, const String& charset = String(), Optional<ResourceLoadPriority> = Nullopt);
-    CachedResourceRequest(ResourceRequest&&, const ResourceLoaderOptions&);
-    CachedResourceRequest(const ResourceRequest&, Optional<ResourceLoadPriority>);
-    ~CachedResourceRequest();
+    CachedResourceRequest(ResourceRequest&&, const ResourceLoaderOptions&, Optional<ResourceLoadPriority> = Nullopt);
 
     ResourceRequest& mutableResourceRequest() { return m_resourceRequest; }

trunk/Source/WebCore/loader/cache/CachedSVGDocumentReference.cpp

@@ -53 +53 @@
         return;
 
-    CachedResourceRequest request(ResourceRequest(loader.document()->completeURL(m_url)), options);
+    auto fetchOptions = options;
+    fetchOptions.mode = FetchOptions::Mode::SameOrigin;
+    CachedResourceRequest request(ResourceRequest(loader.document()->completeURL(m_url)), fetchOptions);
     request.setInitiator(cachedResourceRequestInitiators().css);
     m_document = loader.requestSVGDocument(WTFMove(request));

trunk/Source/WebCore/svg/SVGUseElement.cpp

@@ -571 +571 @@
         ResourceLoaderOptions options = CachedResourceLoader::defaultCachedResourceOptions();
         options.contentSecurityPolicyImposition = isInUserAgentShadowTree() ? ContentSecurityPolicyImposition::SkipPolicyCheck : ContentSecurityPolicyImposition::DoPolicyCheck;
-
+        options.mode = FetchOptions::Mode::SameOrigin;
         CachedResourceRequest request { ResourceRequest { externalDocumentURL }, options };
         request.setInitiator(this);

trunk/Source/WebCore/xml/XSLImportRule.cpp

@@ -101 +101 @@
     if (m_cachedSheet)
         m_cachedSheet->removeClient(this);
-    m_cachedSheet = cachedResourceLoader->requestXSLStyleSheet(CachedResourceRequest(ResourceRequest(cachedResourceLoader->document()->completeURL(absHref))));
+
+    auto options = CachedResourceLoader::defaultCachedResourceOptions();
+    options.mode = FetchOptions::Mode::SameOrigin;
+    m_cachedSheet = cachedResourceLoader->requestXSLStyleSheet({ResourceRequest(cachedResourceLoader->document()->completeURL(absHref)), options});
 
     if (m_cachedSheet) {