REGRESSION (r231107): CSP report-only policies are ignored for beacon, importScripts, fetch(), EventSource, and XHR
https://bugs.webkit.org/show_bug.cgi?id=185789
<rdar://problem/40380175>
Reviewed by Andy Estes.
Source/WebCore:
Fixes an issue where CSP report-only policies were ignored for DocumentThreadableLoader and
PingLoad initiated loads as a result of moving CSP processing to NetworkProcess.
Have NetworkLoadChecker implement the ContentSecurityPolicyClient interface and support logging
console messages, sending CSP reports, and dispatching SecurityPolicyViolation events. To support
the latter we introduce a new WebPage message, EnqueueSecurityPolicyViolationEvent, to enqueue
a SecurityPolicyViolationEvent created from an event init dictionary on the document's event
dispatch queue.
Additionally, shorten the description for a ResourceError caused by CSP to "Blocked by Content Security Policy"
because the CSP code run in NetworkProcess can now log its more detailed error description to
Web Inspector.
Tests: http/tests/security/contentSecurityPolicy/connect-src-beacon-allowed.html
http/tests/security/contentSecurityPolicy/connect-src-beacon-blocked.html
http/tests/security/contentSecurityPolicy/report-only-connect-src-beacon-redirect-blocked.php
http/tests/security/contentSecurityPolicy/report-only-connect-src-xmlhttprequest-redirect-to-blocked.php
* WebCore.xcodeproj/project.pbxproj: Change SecurityPolicyViolationEvent.h from a project header to
a private header so that we can include it in WebKit code.
* dom/Document.cpp:
(WebCore::Document::enqueueSecurityPolicyViolationEvent): Added.
* dom/Document.h:
* dom/EventInit.h:
(WebCore::EventInit::encode const):
(WebCore::EventInit::decode
* dom/SecurityPolicyViolationEvent.h:
(WebCore::SecurityPolicyViolationEvent::Init::encode const):
(WebCore::SecurityPolicyViolationEvent::Init::decode):
Support encoding and decoding for the event.
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::enqueueSecurityPolicyViolationEvent): Formerly named "dispatchSecurityPolicyViolationEvent".
(WebCore::DocumentLoader::dispatchSecurityPolicyViolationEvent): Deleted; renamed to "enqueueSecurityPolicyViolationEvent".
* loader/DocumentLoader.h:
* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::redirectReceived): While I am here, move the check for whether the loader
strategy took responsibility for performing security checks to be before we perform the CSP check to avoid doing
such CSP checks twice in the case that the loader strategy already did them.
(WebCore::DocumentThreadableLoader::didFail): Remove code that checked the CSP policy if the load failed. When
the loader strategy (NetworkProcess) is responsible for performing security checks then this code would never
be executed for a violation of a CSP report-only policy because the loader does not and should not fail the load
for a report-only violations. As the name implies, a report-only violation is only reported. That is, it is not
enforced such that the load is blocked; => fail the load.
(WebCore::DocumentThreadableLoader::reportContentSecurityPolicyError): Update the error description to more
accurately describe the error and be consistent with the error message used in NetworkProcess. This error
message is shown for a redirect blocked by CSP regardless of whether the redirect was to a same-origin or
cross-origin resource. I chose to make the error message more vague than necessary for simplicity because
the CSP code will log a more detailed message for this error than could ever be captured by error message
for the ResourceError. Also use ASCIILiteral to efficiently construct the String object for the error
message.
* page/csp/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::reportViolation const): Build up a SecurityPolicyViolationEvent::Init and
pass that to the delegate to dispatch.
* page/csp/ContentSecurityPolicy.h: Export allowScriptFromSource() and allowChildContextFromSource() so that
we can call them from WebKit.
* page/csp/ContentSecurityPolicyClient.h: Update for renaming.
* platform/network/ResourceRequestBase.h: Define a new requester type to be able to differentiate a request
initiated by importScripts() from other requests. We use this to perform the appropriate CSP checks in NetworkProcess.
* workers/WorkerScriptLoader.cpp:
(WebCore::WorkerScriptLoader::loadSynchronously): Set the requester on the ResourceRequest to ResourceRequest::Requester::ImportScripts
so that we can differentiate this request from other requests. See remark for file ResourceRequestBase.h for
more details.
Source/WebKit:
Have NetworkLoadChecker implement the ContentSecurityPolicyClient interface and support logging
console messages, sending CSP reports, and dispatching SecurityPolicyViolation events.
* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::loadPing):
* NetworkProcess/NetworkLoadChecker.cpp:
(WebKit::NetworkLoadChecker::NetworkLoadChecker): Modified to take a reference to the NetworkConnectionToWebProcess,
the web page ID, the web frame ID, and the resource load identifier. These details are necessary
in order to implement the ContentSecurityPolicyClient interface.
(WebKit::NetworkLoadChecker::isAllowedByContentSecurityPolicy): Added.
(WebKit::NetworkLoadChecker::continueCheckingRequest): Write in terms of isAllowedByContentSecurityPolicy().
(WebKit::NetworkLoadChecker::contentSecurityPolicy): Pass ourself as the client so that we receive
delegate callbacks.
(WebKit::NetworkLoadChecker::addConsoleMessage): Added.
(WebKit::NetworkLoadChecker::sendCSPViolationReport): Added.
(WebKit::NetworkLoadChecker::enqueueSecurityPolicyViolationEvent): Added.
* NetworkProcess/NetworkLoadChecker.h:
* NetworkProcess/NetworkResourceLoader.cpp:
(NetworkResourceLoader::enqueueSecurityPolicyViolationEvent): Added.
* NetworkProcess/NetworkResourceLoader.h:
* NetworkProcess/PingLoad.cpp:
(WebKit::PingLoad::PingLoad): Modified to take a reference to the NetworkConnectionToWebProcess and pass
this through to the NetworkLoadChecker along with the web page ID, web frame ID and resource load identifier.
* NetworkProcess/PingLoad.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::enqueueSecurityPolicyViolationEvent): Added.
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in: Add message EnqueueSecurityPolicyViolationEvent.
LayoutTests:
Add some new tests and updated the expected results of other tests.
The tests connect-src-beacon-{allowed, blocked} are derived from the Blink test:
<https://chromium.googlesource.com/chromium/src/+/5c265c1a56a60533a1957589d33eabc201e2e8b6/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/connect-src-beacon-allowed.html>
* http/tests/quicklook/same-origin-xmlhttprequest-allowed-expected.txt: Update expected result. Note that these results are
a continuation of a regression caused by r231107. See <https://bugs.webkit.org/show_bug.cgi?id=185807> for more details.
* http/tests/security/contentSecurityPolicy/1.1/child-src/worker-redirect-blocked-expected.txt:
* http/tests/security/contentSecurityPolicy/connect-src-beacon-allowed-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/connect-src-beacon-allowed.html: Added.
* http/tests/security/contentSecurityPolicy/connect-src-beacon-blocked-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/connect-src-beacon-blocked.html: Added.
* http/tests/security/contentSecurityPolicy/connect-src-eventsource-redirect-to-blocked-expected.txt:
* http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-redirect-to-blocked-expected.txt:
* http/tests/security/contentSecurityPolicy/report-only-connect-src-beacon-redirect-blocked-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/report-only-connect-src-beacon-redirect-blocked.php: Added.
* http/tests/security/contentSecurityPolicy/report-only-connect-src-xmlhttprequest-redirect-to-blocked-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/report-only-connect-src-xmlhttprequest-redirect-to-blocked.php: Added.
* http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked-expected.txt:
* http/tests/security/contentSecurityPolicy/worker-csp-blocks-xhr-redirect-cross-origin-expected.txt:
* http/tests/security/contentSecurityPolicy/worker-csp-importScripts-redirect-cross-origin-blocked-expected.txt:
* http/wpt/beacon/connect-src-beacon-redirect-blocked.sub-expected.txt:
* platform/mac-wk1/TestExpectations: Skip the beacon tests because we do not support beacon in WebKit1.
* platform/mac-wk1/http/tests/security/contentSecurityPolicy/connect-src-eventsource-redirect-to-blocked-expected.txt:
* platform/mac-wk1/http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-redirect-to-blocked-expected.txt:
* platform/mac-wk1/http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked-expected.txt: Added.
* platform/mac-wk1/http/tests/security/contentSecurityPolicy/worker-csp-importScripts-redirect-cross-origin-blocked-expected.txt: Added.
* platform/win/TestExpectations: Skip the beacon tests because we do not support beacon in WebKit1.
* platform/win/http/tests/security/contentSecurityPolicy/connect-src-eventsource-redirect-to-blocked-expected.txt:
* platform/win/http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-redirect-to-blocked-expected.txt:
* platform/win/http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked-expected.txt: Added.
* platform/win/http/tests/security/contentSecurityPolicy/worker-csp-importScripts-redirect-cross-origin-blocked-expected.txt: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@232032 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/loader/DocumentThreadableLoader.cpp b/Source/WebCore/loader/DocumentThreadableLoader.cpp
index 7e1b0d8..b145c17 100644
--- a/Source/WebCore/loader/DocumentThreadableLoader.cpp
+++ b/Source/WebCore/loader/DocumentThreadableLoader.cpp
@@ -283,17 +283,17 @@
return completionHandler(WTFMove(request));
}
+ if (platformStrategies()->loaderStrategy()->havePerformedSecurityChecks(redirectResponse)) {
+ completionHandler(WTFMove(request));
+ return;
+ }
+
if (!isAllowedByContentSecurityPolicy(request.url(), redirectResponse.isNull() ? ContentSecurityPolicy::RedirectResponseReceived::No : ContentSecurityPolicy::RedirectResponseReceived::Yes)) {
reportContentSecurityPolicyError(redirectResponse.url());
clearResource();
return completionHandler(WTFMove(request));
}
- if (platformStrategies()->loaderStrategy()->havePerformedSecurityChecks(redirectResponse)) {
- completionHandler(WTFMove(request));
- return;
- }
-
// Allow same origin requests to continue after allowing clients to audit the redirect.
if (isAllowedRedirect(request.url()))
return completionHandler(WTFMove(request));
@@ -461,14 +461,6 @@
}
#endif
- // NetworkProcess might return a CSP violation as an AccessControl error in case of redirection.
- // Let's recheck CSP to generate the report if needed.
- // FIXME: We should introduce an error dedicated to CSP violation.
- if (shouldPerformSecurityChecks() && error.isAccessControl() && error.failingURL().protocolIsInHTTPFamily() && !isAllowedByContentSecurityPolicy(error.failingURL(), ContentSecurityPolicy::RedirectResponseReceived::Yes)) {
- reportContentSecurityPolicyError(m_resource->resourceRequest().url());
- return;
- }
-
if (m_shouldLogError == ShouldLogError::Yes)
logError(m_document, error, m_options.initiator);
@@ -670,7 +662,7 @@
void DocumentThreadableLoader::reportContentSecurityPolicyError(const URL& url)
{
- logErrorAndFail(ResourceError(errorDomainWebKitInternal, 0, url, "Cross-origin redirection denied by Content Security Policy.", ResourceError::Type::AccessControl));
+ logErrorAndFail(ResourceError(errorDomainWebKitInternal, 0, url, ASCIILiteral { "Blocked by Content Security Policy." }, ResourceError::Type::AccessControl));
}
void DocumentThreadableLoader::reportCrossOriginResourceSharingError(const URL& url)
