⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 281431 in webkit


Ignore:
Timestamp:
Aug 22, 2021, 7:24:23 PM (5 years ago)
Author:
Kate Cheney
Message:

Report correct blocked URI in CSP violation report
​https://bugs.webkit.org/show_bug.cgi?id=226316
<rdar://problem/78552912>

Reviewed by Alex Christensen.

Source/WebCore:

Tests: http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect.html

http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects.html

Currently for a blocked redirection we report the blocked URI as the
target URL. This is not up to spec and we should actually report the
requested URL.

  • loader/DocumentThreadableLoader.cpp:

(WebCore::DocumentThreadableLoader::redirectReceived):
(WebCore::DocumentThreadableLoader::isAllowedByContentSecurityPolicy):

  • loader/DocumentThreadableLoader.h:
  • page/csp/ContentSecurityPolicy.cpp:

(WebCore::ContentSecurityPolicy::allowConnectToSource const):
(WebCore::ContentSecurityPolicy::reportViolation const):

  • page/csp/ContentSecurityPolicy.h:

Source/WebKit:

Currently for a blocked redirection we report the blocked URI as the
target URL. This is not up to spec and we should actually report the
requested URL.

  • NetworkProcess/NetworkLoadChecker.cpp:

(WebKit::NetworkLoadChecker::check):
(WebKit::NetworkLoadChecker::checkRedirection):
(WebKit::NetworkLoadChecker::checkRequest):
(WebKit::NetworkLoadChecker::isAllowedByContentSecurityPolicy):

  • NetworkProcess/NetworkLoadChecker.h:

LayoutTests:

  • http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect.html: Added.
  • http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects.html: Added.
  • platform/mac-wk1/http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects-expected.txt: Added.
  • platform/mac-wk1/http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect-expected.txt: Added.
  • platform/win/http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect-expected.txt: Added.
  • platform/win/http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects-expected.txt: Added.

WebKitLegacy and Win have different console logging.

Location:
trunk
Files:
8 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r281429 r281431  
     12021-08-22  Kate Cheney  <katherine_cheney@apple.com>
     2
     3        Report correct blocked URI in CSP violation report
     4        https://bugs.webkit.org/show_bug.cgi?id=226316
     5        <rdar://problem/78552912>
     6
     7        Reviewed by Alex Christensen.
     8
     9        * http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect-expected.txt: Added.
     10        * http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect.html: Added.
     11        * http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects-expected.txt: Added.
     12        * http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects.html: Added.
     13        * platform/mac-wk1/http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects-expected.txt: Added.
     14        * platform/mac-wk1/http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect-expected.txt: Added.
     15        * platform/win/http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect-expected.txt: Added.
     16        * platform/win/http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects-expected.txt: Added.
     17        WebKitLegacy and Win have different console logging.
     18
    1192021-08-22  Yusuke Suzuki  <ysuzuki@apple.com>
    220

  • trunk/Source/WebCore/ChangeLog

    r281426 r281431  
     12021-08-22  Kate Cheney  <katherine_cheney@apple.com>
     2
     3        Report correct blocked URI in CSP violation report
     4        https://bugs.webkit.org/show_bug.cgi?id=226316
     5        <rdar://problem/78552912>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Tests: http/tests/security/contentSecurityPolicy/report-blocked-uri-after-blocked-redirect.html
     10               http/tests/security/contentSecurityPolicy/report-blocked-uri-after-multiple-redirects.html
     11       
     12        Currently for a blocked redirection we report the blocked URI as the
     13        target URL. This is not up to spec and we should actually report the
     14        requested URL.
     15
     16        * loader/DocumentThreadableLoader.cpp:
     17        (WebCore::DocumentThreadableLoader::redirectReceived):
     18        (WebCore::DocumentThreadableLoader::isAllowedByContentSecurityPolicy):
     19        * loader/DocumentThreadableLoader.h:
     20        * page/csp/ContentSecurityPolicy.cpp:
     21        (WebCore::ContentSecurityPolicy::allowConnectToSource const):
     22        (WebCore::ContentSecurityPolicy::reportViolation const):
     23        * page/csp/ContentSecurityPolicy.h:
     24
    1252021-08-22  Myles C. Maxfield  <mmaxfield@apple.com>
    226

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

    r281097 r281431  
    329329    }
    330330
    331     if (!isAllowedByContentSecurityPolicy(request.url(), redirectResponse.isNull() ? ContentSecurityPolicy::RedirectResponseReceived::No : ContentSecurityPolicy::RedirectResponseReceived::Yes)) {
     331    if (!isAllowedByContentSecurityPolicy(request.url(), redirectResponse.isNull() ? ContentSecurityPolicy::RedirectResponseReceived::No : ContentSecurityPolicy::RedirectResponseReceived::Yes, redirectResponse.url())) {
    332332        reportContentSecurityPolicyError(redirectResponse.url());
    333333        clearResource();
    … …  
    676676}
    677677
    678 bool DocumentThreadableLoader::isAllowedByContentSecurityPolicy(const URL& url, ContentSecurityPolicy::RedirectResponseReceived redirectResponseReceived)
     678bool DocumentThreadableLoader::isAllowedByContentSecurityPolicy(const URL& url, ContentSecurityPolicy::RedirectResponseReceived redirectResponseReceived, const URL& preRedirectURL)
    679679{
    680680    switch (m_options.contentSecurityPolicyEnforcement) {
    … …  
    684684        return contentSecurityPolicy().allowChildContextFromSource(url, redirectResponseReceived);
    685685    case ContentSecurityPolicyEnforcement::EnforceConnectSrcDirective:
    686         return contentSecurityPolicy().allowConnectToSource(url, redirectResponseReceived);
     686        return contentSecurityPolicy().allowConnectToSource(url, redirectResponseReceived, preRedirectURL);
    687687    case ContentSecurityPolicyEnforcement::EnforceScriptSrcDirective:
    688688        return contentSecurityPolicy().allowScriptFromSource(url, redirectResponseReceived);

  • trunk/Source/WebCore/loader/DocumentThreadableLoader.h

    r280953 r281431  
    104104        void loadRequest(ResourceRequest&&, SecurityCheckPolicy);
    105105        bool isAllowedRedirect(const URL&);
    106         bool isAllowedByContentSecurityPolicy(const URL&, ContentSecurityPolicy::RedirectResponseReceived);
     106        bool isAllowedByContentSecurityPolicy(const URL&, ContentSecurityPolicy::RedirectResponseReceived, const URL& preRedirectURL = URL());
    107107
    108108        SecurityOrigin& securityOrigin() const;

  • trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp

    r280504 r281431  
    601601}
    602602
    603 bool ContentSecurityPolicy::allowConnectToSource(const URL& url, RedirectResponseReceived redirectResponseReceived) const
     603bool ContentSecurityPolicy::allowConnectToSource(const URL& url, RedirectResponseReceived redirectResponseReceived, const URL& preRedirectURL) const
    604604{
    605605    if (LegacySchemeRegistry::schemeShouldBypassContentSecurityPolicy(url.protocol().toStringWithoutCopying()))
    … …  
    609609    auto handleViolatedDirective = [&] (const ContentSecurityPolicyDirective& violatedDirective) {
    610610        String consoleMessage = consoleMessageForViolation(ContentSecurityPolicyDirectiveNames::connectSrc, violatedDirective, url, "Refused to connect to");
    611         reportViolation(ContentSecurityPolicyDirectiveNames::connectSrc, violatedDirective, url, consoleMessage, sourceURL, sourcePosition);
     611        reportViolation(ContentSecurityPolicyDirectiveNames::connectSrc, violatedDirective, url, consoleMessage, sourceURL, sourcePosition, preRedirectURL);
    612612    };
    613613    return allPoliciesAllow(WTFMove(handleViolatedDirective), &ContentSecurityPolicyDirectiveList::violatedDirectiveForConnectSource, url, redirectResponseReceived == RedirectResponseReceived::Yes);
    … …  
    660660}
    661661
    662 void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirective, const ContentSecurityPolicyDirective& violatedDirective, const URL& blockedURL, const String& consoleMessage, const String& sourceURL, const TextPosition& sourcePosition, JSC::JSGlobalObject* state) const
    663 {
    664     return reportViolation(effectiveViolatedDirective, violatedDirective.text(), violatedDirective.directiveList(), blockedURL, consoleMessage, sourceURL, sourcePosition, state);
    665 }
    666 
    667 void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirective, const String& violatedDirective, const ContentSecurityPolicyDirectiveList& violatedDirectiveList, const URL& blockedURL, const String& consoleMessage, const String& sourceURL, const TextPosition& sourcePosition, JSC::JSGlobalObject* state) const
     662void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirective, const ContentSecurityPolicyDirective& violatedDirective, const URL& blockedURL, const String& consoleMessage, const String& sourceURL, const TextPosition& sourcePosition, const URL& preRedirectURL, JSC::JSGlobalObject* state) const
     663{
     664    return reportViolation(effectiveViolatedDirective, violatedDirective.text(), violatedDirective.directiveList(), blockedURL, consoleMessage, sourceURL, sourcePosition, state, preRedirectURL);
     665}
     666
     667void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirective, const String& violatedDirective, const ContentSecurityPolicyDirectiveList& violatedDirectiveList, const URL& blockedURL, const String& consoleMessage, const String& sourceURL, const TextPosition& sourcePosition, JSC::JSGlobalObject* state, const URL& preRedirectURL) const
    668668{
    669669    logToConsole(consoleMessage, sourceURL, sourcePosition.m_line, sourcePosition.m_column, state);
    … …  
    700700    ASSERT(m_client || is<Document>(m_scriptExecutionContext));
    701701
    702     String blockedURI = deprecatedURLForReporting(blockedURL);
     702    String blockedURI;
     703    if (preRedirectURL.isNull())
     704        blockedURI = deprecatedURLForReporting(blockedURL);
     705    else
     706        blockedURI = deprecatedURLForReporting(preRedirectURL);
     707
    703708    // FIXME: Is it policy to not use the status code for HTTPS, or is that a bug?
    704709    unsigned short httpStatusCode = m_selfSourceProtocol == "http" ? m_httpStatusCode : 0;

  • trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h

    r278669 r281431  
    115115    bool allowChildFrameFromSource(const URL&, RedirectResponseReceived = RedirectResponseReceived::No) const;
    116116    WEBCORE_EXPORT bool allowChildContextFromSource(const URL&, RedirectResponseReceived = RedirectResponseReceived::No) const;
    117     WEBCORE_EXPORT bool allowConnectToSource(const URL&, RedirectResponseReceived = RedirectResponseReceived::No) const;
     117    WEBCORE_EXPORT bool allowConnectToSource(const URL&, RedirectResponseReceived = RedirectResponseReceived::No, const URL& requestedURL = URL()) const;
    118118    bool allowFormAction(const URL&, RedirectResponseReceived = RedirectResponseReceived::No) const;
    119119
    … …  
    211211    void reportViolation(const String& effectiveViolatedDirective, const ContentSecurityPolicyDirective& violatedDirective, const URL& blockedURL, const String& consoleMessage, JSC::JSGlobalObject*) const;
    212212    void reportViolation(const String& effectiveViolatedDirective, const String& violatedDirective, const ContentSecurityPolicyDirectiveList&, const URL& blockedURL, const String& consoleMessage, JSC::JSGlobalObject* = nullptr) const;
    213     void reportViolation(const String& effectiveViolatedDirective, const ContentSecurityPolicyDirective& violatedDirective, const URL& blockedURL, const String& consoleMessage, const String& sourceURL, const TextPosition& sourcePosition, JSC::JSGlobalObject* = nullptr) const;
    214     void reportViolation(const String& effectiveViolatedDirective, const String& violatedDirective, const ContentSecurityPolicyDirectiveList& violatedDirectiveList, const URL& blockedURL, const String& consoleMessage, const String& sourceURL, const TextPosition& sourcePosition, JSC::JSGlobalObject*) const;
     213    void reportViolation(const String& effectiveViolatedDirective, const ContentSecurityPolicyDirective& violatedDirective, const URL& blockedURL, const String& consoleMessage, const String& sourceURL, const TextPosition& sourcePosition, const URL& preRedirectURL = URL(), JSC::JSGlobalObject* = nullptr) const;
     214    void reportViolation(const String& effectiveViolatedDirective, const String& violatedDirective, const ContentSecurityPolicyDirectiveList& violatedDirectiveList, const URL& blockedURL, const String& consoleMessage, const String& sourceURL, const TextPosition& sourcePosition, JSC::JSGlobalObject*, const URL& preRedirectURL = URL()) const;
    215215    void reportBlockedScriptExecutionToInspector(const String& directiveText) const;
    216216

  • trunk/Source/WebKit/ChangeLog

    r281420 r281431  
     12021-08-22  Kate Cheney  <katherine_cheney@apple.com>
     2
     3        Report correct blocked URI in CSP violation report
     4        https://bugs.webkit.org/show_bug.cgi?id=226316
     5        <rdar://problem/78552912>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Currently for a blocked redirection we report the blocked URI as the
     10        target URL. This is not up to spec and we should actually report the
     11        requested URL.
     12
     13        * NetworkProcess/NetworkLoadChecker.cpp:
     14        (WebKit::NetworkLoadChecker::check):
     15        (WebKit::NetworkLoadChecker::checkRedirection):
     16        (WebKit::NetworkLoadChecker::checkRequest):
     17        (WebKit::NetworkLoadChecker::isAllowedByContentSecurityPolicy):
     18        * NetworkProcess/NetworkLoadChecker.h:
     19
    1202021-08-22  Wenson Hsieh  <wenson_hsieh@apple.com>
    221

  • trunk/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp

    r280953 r281431  
    9393
    9494    m_firstRequestHeaders = request.httpHeaderFields();
    95     checkRequest(WTFMove(request), client, WTFMove(handler));
     95    checkRequest(WTFMove(request), client, URL(), WTFMove(handler));
    9696}
    9797
    … …  
    143143    m_url = redirectRequest.url();
    144144
    145     checkRequest(WTFMove(redirectRequest), client, [handler = WTFMove(handler), request = WTFMove(request), redirectResponse = WTFMove(redirectResponse)](auto&& result) mutable {
     145    checkRequest(WTFMove(redirectRequest), client, redirectResponse.url(), [handler = WTFMove(handler), request = WTFMove(request), redirectResponse](auto&& result) mutable {
    146146        WTF::switchOn(result,
    147147            [&handler] (ResourceError& error) mutable {
    … …  
    208208}
    209209
    210 void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurityPolicyClient* client, ValidationHandler&& handler)
     210void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurityPolicyClient* client, const URL& preRedirectURL, ValidationHandler&& handler)
    211211{
    212212    ResourceRequest originalRequest = request;
    … …  
    217217            contentSecurityPolicy->upgradeInsecureRequestIfNeeded(request, type);
    218218        }
    219         if (!this->isAllowedByContentSecurityPolicy(request, client)) {
     219        if (!this->isAllowedByContentSecurityPolicy(request, client, preRedirectURL)) {
    220220            handler(this->accessControlErrorForValidationHandler("Blocked by Content Security Policy."_s));
    221221            return;
    … …  
    254254}
    255255
    256 bool NetworkLoadChecker::isAllowedByContentSecurityPolicy(const ResourceRequest& request, WebCore::ContentSecurityPolicyClient* client)
     256bool NetworkLoadChecker::isAllowedByContentSecurityPolicy(const ResourceRequest& request, WebCore::ContentSecurityPolicyClient* client, const URL& preRedirectURL)
    257257{
    258258    auto* contentSecurityPolicy = this->contentSecurityPolicy();
    … …  
    276276        return true;
    277277    case FetchOptions::Destination::EmptyString:
    278         return contentSecurityPolicy->allowConnectToSource(request.url(), redirectResponseReceived);
     278        return contentSecurityPolicy->allowConnectToSource(request.url(), redirectResponseReceived, preRedirectURL);
    279279    case FetchOptions::Destination::Audio:
    280280    case FetchOptions::Destination::Document:

  • trunk/Source/WebKit/NetworkProcess/NetworkLoadChecker.h

    r280953 r281431  
    107107    bool isRedirected() const { return m_redirectCount; }
    108108
    109     void checkRequest(WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, ValidationHandler&&);
     109    void checkRequest(WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, const URL&, ValidationHandler&&);
    110110
    111     bool isAllowedByContentSecurityPolicy(const WebCore::ResourceRequest&, WebCore::ContentSecurityPolicyClient*);
     111    bool isAllowedByContentSecurityPolicy(const WebCore::ResourceRequest&, WebCore::ContentSecurityPolicyClient*, const URL& preRedirectURL = URL());
    112112
    113113    void continueCheckingRequest(WebCore::ResourceRequest&&, ValidationHandler&&);
Note: See TracChangeset for help on using the changeset viewer.