Changeset 207752 in webkit


Ignore:
Timestamp:
Oct 24, 2016 12:49:14 AM (8 years ago)
Author:
commit-queue@webkit.org
Message:

Redirections should be upgraded if CSP policy says so
​https://bugs.webkit.org/show_bug.cgi?id=163544

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

Source/WebCore:

Test: http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-after-redirect.https.html

Introducing CachedResourceLoader::updateRequestAfterRedirection to do the checks that CachedResourceLoader is doing
to the initial request, but for redirection requests.

Implemented URL upgrade according CSP policy, as specified by fetch algorithm.
Minor refactoring in CachedResourceRequest to share some code.
Fixing some constness issues.

  • loader/SubresourceLoader.cpp:

(WebCore::SubresourceLoader::willSendRequestInternal):

  • loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::allowedByContentSecurityPolicy):
(WebCore::CachedResourceLoader::canRequestAfterRedirection):
(WebCore::CachedResourceLoader::updateRequestAfterRedirection):

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

(WebCore::upgradeInsecureResourceRequestIfNeeded):
(WebCore::CachedResourceRequest::upgradeInsecureRequestIfNeeded):

  • loader/cache/CachedResourceRequest.h:

LayoutTests:

  • http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-after-redirect.https-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-after-redirect.https.html: Added.
  • http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-redirect-https-to-http-script-in-iframe-expected.txt:
  • http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-redirect-https-to-http-script-in-iframe.html:
  • platform/mac/TestExpectations:
Location:
trunk
Files:
2 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r207737 r207752  
     12016-10-24  Youenn Fablet  <youenn@apple.com>
     2
     3        Redirections should be upgraded if CSP policy says so
     4        https://bugs.webkit.org/show_bug.cgi?id=163544
     5
     6        Reviewed by Darin Adler.
     7
     8        * http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-after-redirect.https-expected.txt: Added.
     9        * http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-after-redirect.https.html: Added.
     10        * http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-redirect-https-to-http-script-in-iframe-expected.txt:
     11        * http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-redirect-https-to-http-script-in-iframe.html:
     12        * platform/mac/TestExpectations:
     13
    1142016-10-22  Sam Weinig  <sam@webkit.org>
    215

  • trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-redirect-https-to-http-script-in-iframe-expected.txt

    r201753 r207752  
    22main frame - didFinishDocumentLoadForFrame
    33frame "<!--framePath //<!--frame0-->-->" - didCommitLoadForFrame
    4 CONSOLE MESSAGE: [blocked] The page at https://127.0.0.1:8443/security/contentSecurityPolicy/upgrade-insecure-requests/resources/frame-with-redirect-https-to-http-script.html was not allowed to run insecure content from http://127.0.0.1:8080/security/mixedContent/resources/script.js.
    5 
    64frame "<!--framePath //<!--frame0-->-->" - didFinishDocumentLoadForFrame
    75frame "<!--framePath //<!--frame0-->-->" - didHandleOnloadEventsForFrame
    … …  
    97frame "<!--framePath //<!--frame0-->-->" - didFinishLoadForFrame
    108main frame - didFinishLoadForFrame
    11 This test loads a secure iframe that loads an insecure script (but with a tricky redirect). We should upgrade the relevant requests for the any top-level frames, but not sub-resources of those frames, triggering a mixed content callback.
     9This test loads a secure iframe that loads an insecure script (but with a tricky redirect). We should upgrade the relevant requests.
    1210
    1311

  • trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-redirect-https-to-http-script-in-iframe.html

    r201753 r207752  
    88}
    99</script>
    10 <p>This test loads a secure iframe that loads an insecure script (but with a
    11 tricky redirect).  We should upgrade the relevant requests for the any top-level
    12 frames, but not sub-resources of those frames, triggering a mixed content callback.</p>
     10<p>This test loads a secure iframe that loads an insecure script (but with a tricky redirect).  We should upgrade the relevant requests.</p>
    1311<iframe src="https://127.0.0.1:8443/security/contentSecurityPolicy/upgrade-insecure-requests/resources/frame-with-redirect-https-to-http-script.html"></iframe>
    1412</body>

  • trunk/LayoutTests/platform/mac/TestExpectations

    r207660 r207752  
    13321332webkit.org/b/155140 js/promises-tests/promises-tests-2-3-3.html [ Pass Failure ]
    13331333
     1334webkit.org/b/163544 http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-redirect-https-to-http-script-in-iframe.html [ Pass Timeout ]
     1335
    13341336# Content Security Policy for media redirects is not supported on some OSes.
    13351337[ Yosemite ElCapitan ] http/tests/security/contentSecurityPolicy/audio-redirect-blocked.html [ Failure ]

  • trunk/Source/WebCore/ChangeLog

    r207737 r207752  
     12016-10-24  Youenn Fablet  <youenn@apple.com>
     2
     3        Redirections should be upgraded if CSP policy says so
     4        https://bugs.webkit.org/show_bug.cgi?id=163544
     5
     6        Reviewed by Darin Adler.
     7
     8        Test: http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-after-redirect.https.html
     9
     10        Introducing CachedResourceLoader::updateRequestAfterRedirection to do the checks that CachedResourceLoader is doing
     11        to the initial request, but for redirection requests.
     12
     13        Implemented URL upgrade according CSP policy, as specified by fetch algorithm.
     14        Minor refactoring in CachedResourceRequest to share some code.
     15        Fixing some constness issues.
     16
     17        * loader/SubresourceLoader.cpp:
     18        (WebCore::SubresourceLoader::willSendRequestInternal):
     19        * loader/cache/CachedResourceLoader.cpp:
     20        (WebCore::CachedResourceLoader::allowedByContentSecurityPolicy):
     21        (WebCore::CachedResourceLoader::canRequestAfterRedirection):
     22        (WebCore::CachedResourceLoader::updateRequestAfterRedirection):
     23        * loader/cache/CachedResourceLoader.h:
     24        * loader/cache/CachedResourceRequest.cpp:
     25        (WebCore::upgradeInsecureResourceRequestIfNeeded):
     26        (WebCore::CachedResourceRequest::upgradeInsecureRequestIfNeeded):
     27        * loader/cache/CachedResourceRequest.h:
     28
    1292016-10-22  Sam Weinig  <sam@webkit.org>
    230

  • trunk/Source/WebCore/loader/SubresourceLoader.cpp

    r207388 r207752  
    204204        }
    205205
    206         if (!m_documentLoader->cachedResourceLoader().canRequestAfterRedirection(m_resource->type(), newRequest.url(), options())) {
     206        if (!m_documentLoader->cachedResourceLoader().updateRequestAfterRedirection(m_resource->type(), newRequest, options())) {
    207207            cancel();
    208208            return;

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

    r207459 r207752  
    381381}
    382382
    383 bool CachedResourceLoader::allowedByContentSecurityPolicy(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, ContentSecurityPolicy::RedirectResponseReceived redirectResponseReceived)
     383bool CachedResourceLoader::allowedByContentSecurityPolicy(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, ContentSecurityPolicy::RedirectResponseReceived redirectResponseReceived) const
    384384{
    385385    if (options.contentSecurityPolicyImposition == ContentSecurityPolicyImposition::SkipPolicyCheck)
    … …  
    471471
    472472// FIXME: Should we find a way to know whether the redirection is for a preload request like we do for CachedResourceLoader::canRequest?
    473 bool CachedResourceLoader::canRequestAfterRedirection(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options)
     473bool CachedResourceLoader::canRequestAfterRedirection(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options) const
    474474{
    475475    if (document() && !document()->securityOrigin()->canDisplay(url)) {
    … …  
    496496
    497497    return true;
     498}
     499
     500bool CachedResourceLoader::updateRequestAfterRedirection(CachedResource::Type type, ResourceRequest& request, const ResourceLoaderOptions& options)
     501{
     502    ASSERT(m_documentLoader);
     503    if (auto* document = m_documentLoader->cachedResourceLoader().document())
     504        upgradeInsecureResourceRequestIfNeeded(request, *document);
     505
     506    // FIXME: We might want to align the checks done here with the ones done in CachedResourceLoader::requestResource, content extensions blocking in particular.
     507
     508    return canRequestAfterRedirection(type, request.url(), options);
    498509}
    499510

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

    r207086 r207752  
    137137    void printPreloadStats();
    138138
    139     bool canRequestAfterRedirection(CachedResource::Type, const URL&, const ResourceLoaderOptions&);
     139    bool updateRequestAfterRedirection(CachedResource::Type, ResourceRequest&, const ResourceLoaderOptions&);
    140140
    141141    static const ResourceLoaderOptions& defaultCachedResourceOptions();
    … …  
    172172    bool shouldContinueAfterNotifyingLoadedFromMemoryCache(const CachedResourceRequest&, CachedResource*);
    173173    bool checkInsecureContent(CachedResource::Type, const URL&) const;
    174     bool allowedByContentSecurityPolicy(CachedResource::Type, const URL&, const ResourceLoaderOptions&, ContentSecurityPolicy::RedirectResponseReceived);
     174    bool allowedByContentSecurityPolicy(CachedResource::Type, const URL&, const ResourceLoaderOptions&, ContentSecurityPolicy::RedirectResponseReceived) const;
    175175
    176176    void performPostLoadActions();
    … …  
    179179    void reloadImagesIfNotDeferred();
    180180
     181    bool canRequestAfterRedirection(CachedResource::Type, const URL&, const ResourceLoaderOptions&) const;
    181182    bool canRequestInContentDispositionAttachmentSandbox(CachedResource::Type, const URL&) const;
    182    
     183
    183184    HashSet<String> m_validatedURLs;
    184185    mutable DocumentResourceMap m_documentResources;

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

    r207459 r207752  
    106106}
    107107
    108 void CachedResourceRequest::upgradeInsecureRequestIfNeeded(Document& document)
    109 {
    110     URL url = m_resourceRequest.url();
     108void upgradeInsecureResourceRequestIfNeeded(ResourceRequest& request, Document& document)
     109{
     110    URL url = request.url();
    111111
    112112    ASSERT(document.contentSecurityPolicy());
    113113    document.contentSecurityPolicy()->upgradeInsecureRequestIfNeeded(url, ContentSecurityPolicy::InsecureRequestType::Load);
    114114
    115     if (url == m_resourceRequest.url())
     115    if (url == request.url())
    116116        return;
    117117
    118     m_resourceRequest.setURL(url);
     118    request.setURL(url);
     119}
     120
     121void CachedResourceRequest::upgradeInsecureRequestIfNeeded(Document& document)
     122{
     123    upgradeInsecureResourceRequestIfNeeded(m_resourceRequest, document);
    119124}
    120125

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

    r207459 r207752  
    9595};
    9696
     97void upgradeInsecureResourceRequestIfNeeded(ResourceRequest&, Document&);
     98
    9799} // namespace WebCore
    98100
Note: See TracChangeset for help on using the changeset viewer.