CSP should be passed the referrer
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 May 2018 17:52:34 +0000 (17:52 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 May 2018 17:52:34 +0000 (17:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185367

Reviewed by Per Arne Vollan.

Source/WebCore:

As a step towards formalizing a CSP delegate object and removing the dependencies
on ScriptExecutionContext and Frame, we should pass the document's referrer directly
instead of indirectly obtaining it from the ScriptExecutionContext or Frame used
to instantiate the ContentSecurityPolicy object.

* dom/Document.cpp:
(WebCore::Document::processHttpEquiv): Pass the document's referrer.
(WebCore::Document::initSecurityContext): Ditto.
(WebCore::Document::applyQuickLookSandbox): Ditto.
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::responseReceived): Ditto.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::didBeginDocument): Ditto.
* page/csp/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::copyStateFrom): We pass a null string for the referrer
to didReceiveHeader() as a placeholder since it requires the referrer be given to it. We
fix up the referrer (m_referrer) after copying all the policy headers.
(WebCore::ContentSecurityPolicy::didReceiveHeaders): Ditto.
(WebCore::ContentSecurityPolicy::didReceiveHeader): Modified to take a referrer and WTFMove()s
it into an instance variable (m_referrer).
(WebCore::ContentSecurityPolicy::reportViolation const): Modified to use the stored referrer.
* page/csp/ContentSecurityPolicy.h:
* workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::applyContentSecurityPolicyResponseHeaders): Pass a null string
for the referrer as a worker does not have a referrer.

Source/WebKit:

Pass the referrer through NetworkLoadChecker so that it can pass it to the ContentSecurityPolicy
object it instantiates.

* NetworkProcess/NetworkLoadChecker.cpp:
(WebKit::NetworkLoadChecker::NetworkLoadChecker):
(WebKit::NetworkLoadChecker::contentSecurityPolicy const):
* NetworkProcess/NetworkLoadChecker.h:
(WebKit::NetworkLoadChecker::create):
* NetworkProcess/NetworkResourceLoader.cpp:
* NetworkProcess/PingLoad.cpp:
(WebKit::PingLoad::PingLoad):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231445 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/page/csp/ContentSecurityPolicy.cpp
Source/WebCore/page/csp/ContentSecurityPolicy.h
Source/WebCore/workers/WorkerGlobalScope.cpp
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
Source/WebKit/NetworkProcess/NetworkLoadChecker.h
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
Source/WebKit/NetworkProcess/PingLoad.cpp

index 3ce72cf..fc417a3 100644 (file)
@@ -1,5 +1,38 @@
 2018-05-07  Daniel Bates  <dabates@apple.com>
 
+        CSP should be passed the referrer
+        https://bugs.webkit.org/show_bug.cgi?id=185367
+
+        Reviewed by Per Arne Vollan.
+
+        As a step towards formalizing a CSP delegate object and removing the dependencies
+        on ScriptExecutionContext and Frame, we should pass the document's referrer directly
+        instead of indirectly obtaining it from the ScriptExecutionContext or Frame used
+        to instantiate the ContentSecurityPolicy object.
+
+        * dom/Document.cpp:
+        (WebCore::Document::processHttpEquiv): Pass the document's referrer.
+        (WebCore::Document::initSecurityContext): Ditto.
+        (WebCore::Document::applyQuickLookSandbox): Ditto.
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::responseReceived): Ditto.
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::didBeginDocument): Ditto.
+        * page/csp/ContentSecurityPolicy.cpp:
+        (WebCore::ContentSecurityPolicy::copyStateFrom): We pass a null string for the referrer
+        to didReceiveHeader() as a placeholder since it requires the referrer be given to it. We
+        fix up the referrer (m_referrer) after copying all the policy headers.
+        (WebCore::ContentSecurityPolicy::didReceiveHeaders): Ditto.
+        (WebCore::ContentSecurityPolicy::didReceiveHeader): Modified to take a referrer and WTFMove()s
+        it into an instance variable (m_referrer).
+        (WebCore::ContentSecurityPolicy::reportViolation const): Modified to use the stored referrer.
+        * page/csp/ContentSecurityPolicy.h:
+        * workers/WorkerGlobalScope.cpp:
+        (WebCore::WorkerGlobalScope::applyContentSecurityPolicyResponseHeaders): Pass a null string
+        for the referrer as a worker does not have a referrer.
+
+2018-05-07  Daniel Bates  <dabates@apple.com>
+
         CSP should only notify Inspector to pause the debugger on the first policy to violate a directive
         https://bugs.webkit.org/show_bug.cgi?id=185364
 
index 7dc8907..9c61a95 100644 (file)
@@ -3384,12 +3384,12 @@ void Document::processHttpEquiv(const String& equiv, const String& content, bool
 
     case HTTPHeaderName::ContentSecurityPolicy:
         if (isInDocumentHead)
-            contentSecurityPolicy()->didReceiveHeader(content, ContentSecurityPolicyHeaderType::Enforce, ContentSecurityPolicy::PolicyFrom::HTTPEquivMeta);
+            contentSecurityPolicy()->didReceiveHeader(content, ContentSecurityPolicyHeaderType::Enforce, ContentSecurityPolicy::PolicyFrom::HTTPEquivMeta, referrer());
         break;
 
     case HTTPHeaderName::XWebKitCSP:
         if (isInDocumentHead)
-            contentSecurityPolicy()->didReceiveHeader(content, ContentSecurityPolicyHeaderType::PrefixedEnforce, ContentSecurityPolicy::PolicyFrom::HTTPEquivMeta);
+            contentSecurityPolicy()->didReceiveHeader(content, ContentSecurityPolicyHeaderType::PrefixedEnforce, ContentSecurityPolicy::PolicyFrom::HTTPEquivMeta, referrer());
         break;
 
     default:
@@ -5522,7 +5522,7 @@ void Document::initSecurityContext()
 
     String overrideContentSecurityPolicy = m_frame->loader().client().overrideContentSecurityPolicy();
     if (!overrideContentSecurityPolicy.isNull())
-        contentSecurityPolicy()->didReceiveHeader(overrideContentSecurityPolicy, ContentSecurityPolicyHeaderType::Enforce, ContentSecurityPolicy::PolicyFrom::API);
+        contentSecurityPolicy()->didReceiveHeader(overrideContentSecurityPolicy, ContentSecurityPolicyHeaderType::Enforce, ContentSecurityPolicy::PolicyFrom::API, referrer());
 
 #if USE(QUICK_LOOK)
     if (shouldEnforceQuickLookSandbox())
@@ -7277,7 +7277,7 @@ void Document::applyQuickLookSandbox()
     static NeverDestroyed<String> quickLookCSP = makeString("default-src ", QLPreviewProtocol(), ": 'unsafe-inline'; base-uri 'none'; sandbox allow-same-origin allow-scripts");
     RELEASE_ASSERT(contentSecurityPolicy());
     // The sandbox directive is only allowed if the policy is from an HTTP header.
-    contentSecurityPolicy()->didReceiveHeader(quickLookCSP, ContentSecurityPolicyHeaderType::Enforce, ContentSecurityPolicy::PolicyFrom::HTTPHeader);
+    contentSecurityPolicy()->didReceiveHeader(quickLookCSP, ContentSecurityPolicyHeaderType::Enforce, ContentSecurityPolicy::PolicyFrom::HTTPHeader, referrer());
 
     disableSandboxFlags(SandboxNavigation);
 
index 4cc1b18..8f5eec2 100644 (file)
@@ -768,7 +768,7 @@ void DocumentLoader::responseReceived(const ResourceResponse& response, Completi
     auto url = response.url();
 
     ContentSecurityPolicy contentSecurityPolicy(SecurityOrigin::create(url), m_frame);
-    contentSecurityPolicy.didReceiveHeaders(ContentSecurityPolicyResponseHeaders(response));
+    contentSecurityPolicy.didReceiveHeaders(ContentSecurityPolicyResponseHeaders(response), m_frame->loader().referrer());
     if (!contentSecurityPolicy.allowFrameAncestors(*m_frame, url)) {
         stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied(identifier, response);
         return;
index 606a0f1..8ef68f2 100644 (file)
@@ -732,7 +732,7 @@ void FrameLoader::didBeginDocument(bool dispatch)
         if (!dnsPrefetchControl.isEmpty())
             m_frame.document()->parseDNSPrefetchControlHeader(dnsPrefetchControl);
 
-        m_frame.document()->contentSecurityPolicy()->didReceiveHeaders(ContentSecurityPolicyResponseHeaders(m_documentLoader->response()), ContentSecurityPolicy::ReportParsingErrors::No);
+        m_frame.document()->contentSecurityPolicy()->didReceiveHeaders(ContentSecurityPolicyResponseHeaders(m_documentLoader->response()), referrer(), ContentSecurityPolicy::ReportParsingErrors::No);
 
         String headerContentLanguage = m_documentLoader->response().httpHeaderField(HTTPHeaderName::ContentLanguage);
         if (!headerContentLanguage.isEmpty()) {
index b579e96..b03c2eb 100644 (file)
@@ -111,7 +111,8 @@ void ContentSecurityPolicy::copyStateFrom(const ContentSecurityPolicy* other)
         return;
     ASSERT(m_policies.isEmpty());
     for (auto& policy : other->m_policies)
-        didReceiveHeader(policy->header(), policy->headerType(), ContentSecurityPolicy::PolicyFrom::Inherited);
+        didReceiveHeader(policy->header(), policy->headerType(), ContentSecurityPolicy::PolicyFrom::Inherited, String { });
+    m_referrer = other->m_referrer;
 }
 
 void ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom(const ContentSecurityPolicy& other)
@@ -171,18 +172,21 @@ ContentSecurityPolicyResponseHeaders ContentSecurityPolicy::responseHeaders() co
     return *m_cachedResponseHeaders;
 }
 
-void ContentSecurityPolicy::didReceiveHeaders(const ContentSecurityPolicyResponseHeaders& headers, ReportParsingErrors reportParsingErrors)
+void ContentSecurityPolicy::didReceiveHeaders(const ContentSecurityPolicyResponseHeaders& headers, String&& referrer, ReportParsingErrors reportParsingErrors)
 {
     SetForScope<bool> isReportingEnabled(m_isReportingEnabled, reportParsingErrors == ReportParsingErrors::Yes);
     for (auto& header : headers.m_headers)
-        didReceiveHeader(header.first, header.second, ContentSecurityPolicy::PolicyFrom::HTTPHeader);
+        didReceiveHeader(header.first, header.second, ContentSecurityPolicy::PolicyFrom::HTTPHeader, String { });
+    m_referrer = WTFMove(referrer);
 }
 
-void ContentSecurityPolicy::didReceiveHeader(const String& header, ContentSecurityPolicyHeaderType type, ContentSecurityPolicy::PolicyFrom policyFrom)
+void ContentSecurityPolicy::didReceiveHeader(const String& header, ContentSecurityPolicyHeaderType type, ContentSecurityPolicy::PolicyFrom policyFrom, String&& referrer)
 {
     if (m_hasAPIPolicy)
         return;
 
+    m_referrer = WTFMove(referrer);
+
     if (policyFrom == PolicyFrom::API) {
         ASSERT(m_policies.isEmpty());
         m_hasAPIPolicy = true;
@@ -664,7 +668,6 @@ void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirec
     }
     String violatedDirectiveText = violatedDirective;
     String originalPolicy = violatedDirectiveList.header();
-    String referrer = document.referrer();
     ASSERT(document.loader());
     // FIXME: Is it policy to not use the status code for HTTPS, or is that a bug?
     unsigned short statusCode = document.url().protocolIs("http") && document.loader() ? document.loader()->response().httpStatusCode() : 0;
@@ -683,7 +686,7 @@ void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirec
     // 1. Dispatch violation event.
     bool canBubble = false;
     bool cancelable = false;
-    document.enqueueDocumentEvent(SecurityPolicyViolationEvent::create(eventNames().securitypolicyviolationEvent, canBubble, cancelable, documentURI, referrer, blockedURI, violatedDirectiveText, effectiveViolatedDirective, originalPolicy, sourceFile, statusCode, lineNumber, columnNumber));
+    document.enqueueDocumentEvent(SecurityPolicyViolationEvent::create(eventNames().securitypolicyviolationEvent, canBubble, cancelable, documentURI, m_referrer, blockedURI, violatedDirectiveText, effectiveViolatedDirective, originalPolicy, sourceFile, statusCode, lineNumber, columnNumber));
 
     // 2. Send violation report (if applicable).
     auto& reportURIs = violatedDirectiveList.reportURIs();
@@ -702,7 +705,7 @@ void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirec
 
     auto cspReport = JSON::Object::create();
     cspReport->setString(ASCIILiteral("document-uri"), documentURI);
-    cspReport->setString(ASCIILiteral("referrer"), referrer);
+    cspReport->setString(ASCIILiteral("referrer"), m_referrer);
     cspReport->setString(ASCIILiteral("violated-directive"), violatedDirectiveText);
     cspReport->setString(ASCIILiteral("effective-directive"), effectiveViolatedDirective);
     cspReport->setString(ASCIILiteral("original-policy"), originalPolicy);
index 293d756..dcf484f 100644 (file)
@@ -81,8 +81,8 @@ public:
     };
     WEBCORE_EXPORT ContentSecurityPolicyResponseHeaders responseHeaders() const;
     enum ReportParsingErrors { No, Yes };
-    WEBCORE_EXPORT void didReceiveHeaders(const ContentSecurityPolicyResponseHeaders&, ReportParsingErrors = ReportParsingErrors::Yes);
-    void didReceiveHeader(const String&, ContentSecurityPolicyHeaderType, ContentSecurityPolicy::PolicyFrom);
+    WEBCORE_EXPORT void didReceiveHeaders(const ContentSecurityPolicyResponseHeaders&, String&& referrer, ReportParsingErrors = ReportParsingErrors::Yes);
+    void didReceiveHeader(const String&, ContentSecurityPolicyHeaderType, ContentSecurityPolicy::PolicyFrom, String&& referrer);
 
     bool allowScriptWithNonce(const String& nonce, bool overrideContentSecurityPolicy = false) const;
     bool allowStyleWithNonce(const String& nonce, bool overrideContentSecurityPolicy = false) const;
@@ -211,6 +211,7 @@ private:
     CSPDirectiveListVector m_policies;
     String m_lastPolicyEvalDisabledErrorMessage;
     String m_lastPolicyWebAssemblyDisabledErrorMessage;
+    String m_referrer;
     SandboxFlags m_sandboxFlags;
     bool m_overrideInlineStyleAllowed { false };
     bool m_isReportingEnabled { true };
index ce125b7..eb2a7b6 100644 (file)
@@ -118,7 +118,7 @@ bool WorkerGlobalScope::isSecureContext() const
 
 void WorkerGlobalScope::applyContentSecurityPolicyResponseHeaders(const ContentSecurityPolicyResponseHeaders& contentSecurityPolicyResponseHeaders)
 {
-    contentSecurityPolicy()->didReceiveHeaders(contentSecurityPolicyResponseHeaders);
+    contentSecurityPolicy()->didReceiveHeaders(contentSecurityPolicyResponseHeaders, String { });
 }
 
 URL WorkerGlobalScope::completeURL(const String& url) const
index b920026..8c7c390 100644 (file)
@@ -1,5 +1,24 @@
 2018-05-07  Daniel Bates  <dabates@apple.com>
 
+        CSP should be passed the referrer
+        https://bugs.webkit.org/show_bug.cgi?id=185367
+
+        Reviewed by Per Arne Vollan.
+
+        Pass the referrer through NetworkLoadChecker so that it can pass it to the ContentSecurityPolicy
+        object it instantiates.
+
+        * NetworkProcess/NetworkLoadChecker.cpp:
+        (WebKit::NetworkLoadChecker::NetworkLoadChecker):
+        (WebKit::NetworkLoadChecker::contentSecurityPolicy const):
+        * NetworkProcess/NetworkLoadChecker.h:
+        (WebKit::NetworkLoadChecker::create):
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        * NetworkProcess/PingLoad.cpp:
+        (WebKit::PingLoad::PingLoad):
+
+2018-05-07  Daniel Bates  <dabates@apple.com>
+
         Substitute CrossOriginPreflightResultCache::clear() for CrossOriginPreflightResultCache::empty()
         https://bugs.webkit.org/show_bug.cgi?id=185170
 
index e93eb7b..0190cd2 100644 (file)
@@ -48,13 +48,14 @@ static inline bool isSameOrigin(const URL& url, const SecurityOrigin* origin)
     return url.protocolIsData() || url.protocolIsBlob() || !origin || origin->canRequest(url);
 }
 
-NetworkLoadChecker::NetworkLoadChecker(FetchOptions&& options, PAL::SessionID sessionID, HTTPHeaderMap&& originalRequestHeaders, URL&& url, RefPtr<SecurityOrigin>&& sourceOrigin, PreflightPolicy preflightPolicy)
+NetworkLoadChecker::NetworkLoadChecker(FetchOptions&& options, PAL::SessionID sessionID, HTTPHeaderMap&& originalRequestHeaders, URL&& url, RefPtr<SecurityOrigin>&& sourceOrigin, PreflightPolicy preflightPolicy, String&& referrer)
     : m_options(WTFMove(options))
     , m_sessionID(sessionID)
     , m_originalRequestHeaders(WTFMove(originalRequestHeaders))
     , m_url(WTFMove(url))
     , m_origin(WTFMove(sourceOrigin))
     , m_preflightPolicy(preflightPolicy)
+    , m_referrer(WTFMove(referrer))
 {
     m_isSameOriginRequest = isSameOrigin(m_url, m_origin.get());
     switch (options.credentials) {
@@ -314,7 +315,7 @@ ContentSecurityPolicy* NetworkLoadChecker::contentSecurityPolicy() const
 {
     if (!m_contentSecurityPolicy && m_cspResponseHeaders) {
         m_contentSecurityPolicy = std::make_unique<ContentSecurityPolicy>(*m_origin);
-        m_contentSecurityPolicy->didReceiveHeaders(*m_cspResponseHeaders, ContentSecurityPolicy::ReportParsingErrors::No);
+        m_contentSecurityPolicy->didReceiveHeaders(*m_cspResponseHeaders, String { m_referrer }, ContentSecurityPolicy::ReportParsingErrors::No);
     }
     return m_contentSecurityPolicy.get();
 }
index 654a38f..910cf98 100644 (file)
@@ -42,9 +42,9 @@ class NetworkCORSPreflightChecker;
 
 class NetworkLoadChecker : public RefCounted<NetworkLoadChecker> {
 public:
-    static Ref<NetworkLoadChecker> create(WebCore::FetchOptions&& options, PAL::SessionID sessionID, WebCore::HTTPHeaderMap&& originalHeaders, WebCore::URL&& url, RefPtr<WebCore::SecurityOrigin>&& sourceOrigin, WebCore::PreflightPolicy preflightPolicy)
+    static Ref<NetworkLoadChecker> create(WebCore::FetchOptions&& options, PAL::SessionID sessionID, WebCore::HTTPHeaderMap&& originalHeaders, WebCore::URL&& url, RefPtr<WebCore::SecurityOrigin>&& sourceOrigin, WebCore::PreflightPolicy preflightPolicy, String&& referrer)
     {
-        return adoptRef(*new NetworkLoadChecker { WTFMove(options), sessionID, WTFMove(originalHeaders), WTFMove(url), WTFMove(sourceOrigin), preflightPolicy });
+        return adoptRef(*new NetworkLoadChecker { WTFMove(options), sessionID, WTFMove(originalHeaders), WTFMove(url), WTFMove(sourceOrigin), preflightPolicy, WTFMove(referrer) });
     }
     ~NetworkLoadChecker();
 
@@ -69,7 +69,7 @@ public:
     WebCore::StoredCredentialsPolicy storedCredentialsPolicy() const { return m_storedCredentialsPolicy; }
 
 private:
-    NetworkLoadChecker(WebCore::FetchOptions&&, PAL::SessionID, WebCore::HTTPHeaderMap&&, WebCore::URL&&, RefPtr<WebCore::SecurityOrigin>&&, WebCore::PreflightPolicy);
+    NetworkLoadChecker(WebCore::FetchOptions&&, PAL::SessionID, WebCore::HTTPHeaderMap&&, WebCore::URL&&, RefPtr<WebCore::SecurityOrigin>&&, WebCore::PreflightPolicy, String&& referrer);
 
     WebCore::ContentSecurityPolicy* contentSecurityPolicy() const;
     bool isChecking() const { return !!m_corsPreflightChecker; }
@@ -111,6 +111,7 @@ private:
     WebCore::URL m_previousURL;
     WebCore::PreflightPolicy m_preflightPolicy;
     String m_dntHeaderValue;
+    String m_referrer;
 };
 
 }
index 9d78a44..1f78403 100644 (file)
@@ -111,7 +111,7 @@ NetworkResourceLoader::NetworkResourceLoader(NetworkResourceLoadParameters&& par
     }
 
     if (synchronousReply || parameters.shouldRestrictHTTPResponseAccess) {
-        m_networkLoadChecker = NetworkLoadChecker::create(FetchOptions { m_parameters.options }, m_parameters.sessionID, HTTPHeaderMap { m_parameters.originalRequestHeaders }, URL { m_parameters.request.url() }, m_parameters.sourceOrigin.copyRef(), m_parameters.preflightPolicy);
+        m_networkLoadChecker = NetworkLoadChecker::create(FetchOptions { m_parameters.options }, m_parameters.sessionID, HTTPHeaderMap { m_parameters.originalRequestHeaders }, URL { m_parameters.request.url() }, m_parameters.sourceOrigin.copyRef(), m_parameters.preflightPolicy, originalRequest().httpReferrer());
         if (m_parameters.cspResponseHeaders)
             m_networkLoadChecker->setCSPResponseHeaders(ContentSecurityPolicyResponseHeaders { m_parameters.cspResponseHeaders.value() });
 #if ENABLE(CONTENT_EXTENSIONS)
index cd86401..35c7c03 100644 (file)
@@ -42,7 +42,7 @@ PingLoad::PingLoad(NetworkResourceLoadParameters&& parameters, WTF::CompletionHa
     : m_parameters(WTFMove(parameters))
     , m_completionHandler(WTFMove(completionHandler))
     , m_timeoutTimer(*this, &PingLoad::timeoutTimerFired)
-    , m_networkLoadChecker(NetworkLoadChecker::create(FetchOptions { m_parameters.options}, m_parameters.sessionID, WTFMove(m_parameters.originalRequestHeaders), URL { m_parameters.request.url() }, m_parameters.sourceOrigin.copyRef(), m_parameters.preflightPolicy))
+    , m_networkLoadChecker(NetworkLoadChecker::create(FetchOptions { m_parameters.options}, m_parameters.sessionID, WTFMove(m_parameters.originalRequestHeaders), URL { m_parameters.request.url() }, m_parameters.sourceOrigin.copyRef(), m_parameters.preflightPolicy, m_parameters.request.httpReferrer()))
 {
 
     if (m_parameters.cspResponseHeaders)