service worker fetch handler results in bad referrer
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 23:06:12 +0000 (23:06 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 23:06:12 +0000 (23:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188248
<rdar://problem/47050478>

Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

* web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt:

Source/WebCore:

Response sanitization was removing the ReferrerPolicy header from opaque redirect responses.
Reduce sanitization of opaque redirect responses to opaque responses and allow Location header.
Make sure referrer policy is updated for all load redirections, not only CORS loads.

Test: http/tests/security/referrer-policy-redirect-link-downgrade.html

* loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::checkRedirectionCrossOriginAccessControl):
* platform/network/ResourceResponseBase.cpp:
(WebCore::isSafeCrossOriginResponseHeader):
(WebCore::ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting):

Source/WebKit:

NetworkDataTaskCocoa is sometimes updating the referrer on its own.
Instead of updating the referrer when sending the request to WebProcess for evaluation,
Update the referrer once the web process decides to follow the redirection.
This ensures that any referrer that the WebProcess will set will be updated by NetworkDataTaskCocoa.

* NetworkProcess/cocoa/NetworkDataTaskCocoa.h:
* NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
(WebKit::NetworkDataTaskCocoa::NetworkDataTaskCocoa):
(WebKit::NetworkDataTaskCocoa::restrictRequestReferrerToOriginIfNeeded):
(WebKit::NetworkDataTaskCocoa::willPerformHTTPRedirection):

LayoutTests:

* http/tests/security/referrer-policy-redirect-link-downgrade-expected.txt: Added.
* http/tests/security/referrer-policy-redirect-link-downgrade.html: Added.
* http/tests/security/resources/referrer-policy-redirect-link-downgrade.html: Added.
* http/tests/security/resources/referrer-policy-redirect-link.html:
* platform/ios-wk2/TestExpectations: Skip referrer-policy-redirect-link-downgrade.html
as it is very similar to already skipped referrer-policy-redirect-link.html.

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/referrer-policy-redirect-link-downgrade.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/referrer-policy-redirect-link.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt
LayoutTests/platform/ios-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/loader/SubresourceLoader.cpp
Source/WebCore/platform/network/ResourceResponseBase.cpp
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h
Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm

index fc27e31..9b6748a 100644 (file)
@@ -1,5 +1,20 @@
 2019-01-08  Youenn Fablet  <youenn@apple.com>
 
+        service worker fetch handler results in bad referrer
+        https://bugs.webkit.org/show_bug.cgi?id=188248
+        <rdar://problem/47050478>
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/security/referrer-policy-redirect-link-downgrade-expected.txt: Added.
+        * http/tests/security/referrer-policy-redirect-link-downgrade.html: Added.
+        * http/tests/security/resources/referrer-policy-redirect-link-downgrade.html: Added.
+        * http/tests/security/resources/referrer-policy-redirect-link.html:
+        * platform/ios-wk2/TestExpectations: Skip referrer-policy-redirect-link-downgrade.html
+        as it is very similar to already skipped referrer-policy-redirect-link.html.
+
+2019-01-08  Youenn Fablet  <youenn@apple.com>
+
         IDB storage of Crypto keys does not work in private browsing mode
         https://bugs.webkit.org/show_bug.cgi?id=193219
 
diff --git a/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade-expected.txt b/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade-expected.txt
new file mode 100644 (file)
index 0000000..9faf90e
--- /dev/null
@@ -0,0 +1,11 @@
+This test checks the referrer policy is obeyed along the redirect chain. The test passes if the referrer is empty as the redirect is going from HTTPS to HTTP.
+
+
+
+--------
+Frame: 'iframe'
+--------
+If not running in DumpRenderTree, click this link
+HTTP Referer header is empty
+Referrer is empty
+
diff --git a/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade.html b/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade.html
new file mode 100644 (file)
index 0000000..8f66e7e
--- /dev/null
@@ -0,0 +1,25 @@
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.dumpChildFramesAsText();
+    testRunner.waitUntilDone();
+    testRunner.setCanOpenWindows();
+    testRunner.setCloseRemainingWindowsWhenComplete(true);
+}
+
+function runTest() {
+    var iframe = document.getElementById("iframe");
+    iframe.contentWindow.postMessage({"action": "click", "offsetLeft": iframe.offsetLeft, "offsetTop": iframe.offsetTop}, "*");
+}
+</script>
+</head>
+<body>
+<p>
+This test checks the referrer policy is obeyed along the redirect chain.
+The test passes if the referrer is empty as the redirect is going from HTTPS to HTTP.
+</p>
+<iframe id="iframe" name="iframe" onload="runTest()" src="https://127.0.0.1:8443/security/resources/referrer-policy-redirect-link-downgrade.html"></iframe>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link-downgrade.html b/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link-downgrade.html
new file mode 100644 (file)
index 0000000..fb1bf33
--- /dev/null
@@ -0,0 +1,27 @@
+<html>
+<head>
+<meta name="referrer" content="origin" />
+<script>
+window.addEventListener("message", receiveMessage, false);
+
+function receiveMessage(evt) {
+    if (evt.data == "done") {
+        if (window.testRunner)
+            testRunner.notifyDone();
+    } else if (typeof(evt.data) == "object" && evt.data.action == "click")  {
+        var link = document.getElementById("link");
+        eventSender.mouseMoveTo(link.offsetLeft + evt.data.offsetLeft + 2,
+                                link.offsetTop + evt.data.offsetTop + 2);
+        eventSender.mouseDown();
+        eventSender.mouseUp();
+    } else {
+        document.getElementById("log").innerHTML += evt.data + "<br>";
+    }
+}
+</script>
+</head>
+<body>
+<a id="link" target="_blank" href="https://127.0.0.1:8443/resources/redirect.php?url=http://127.0.0.1:8000/security/resources/referrer-policy-postmessage.php" rel="opener">If not running in DumpRenderTree, click this link</a>
+<div id="log"></div>
+</body>
+</html>
index fb1bf33..23dd526 100644 (file)
@@ -21,7 +21,7 @@ function receiveMessage(evt) {
 </script>
 </head>
 <body>
-<a id="link" target="_blank" href="https://127.0.0.1:8443/resources/redirect.php?url=http://127.0.0.1:8000/security/resources/referrer-policy-postmessage.php" rel="opener">If not running in DumpRenderTree, click this link</a>
+<a id="link" target="_blank" href="https://127.0.0.1:8443/resources/redirect.php?url=https://127.0.0.1:8443/security/resources/referrer-policy-postmessage.php" rel="opener">If not running in DumpRenderTree, click this link</a>
 <div id="log"></div>
 </body>
 </html>
index 0a501ef..ed3de28 100644 (file)
@@ -1,3 +1,13 @@
+2019-01-08  Youenn Fablet  <youenn@apple.com>
+
+        service worker fetch handler results in bad referrer
+        https://bugs.webkit.org/show_bug.cgi?id=188248
+        <rdar://problem/47050478>
+
+        Reviewed by Alex Christensen.
+
+        * web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt:
+
 2019-01-07  Dean Jackson  <dino@apple.com>
 
         Turn on Pointer Events by default for iOS
index 2ea4ade..a4f6929 100644 (file)
@@ -1,7 +1,6 @@
 
-
 PASS Initialize global state (service worker registration) 
-FAIL Referrer for a main resource redirected with referrer-policy (origin) should only have origin. assert_equals: expected "https://localhost:9443/" but got "https://localhost:9443/service-workers/service-worker/referrer-policy-header.https.html"
+PASS Referrer for a main resource redirected with referrer-policy (origin) should only have origin. 
 PASS Referrer for fetch requests initiated from a service worker with referrer-policy (origin) should only have origin. 
 PASS Remove registration as a cleanup 
 
index c36183d..08370fe 100644 (file)
@@ -355,6 +355,7 @@ http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-window-open.html
 http/tests/security/mixedContent/about-blank-iframe-in-main-frame.html
 http/tests/security/mixedContent/redirect-http-to-https-iframe-in-main-frame.html
 http/tests/security/referrer-policy-redirect-link.html
+http/tests/security/referrer-policy-redirect-link-downgrade.html
 http/tests/security/referrer-policy-rel-noreferrer.html
 http/tests/security/video-cross-origin-accessfailure.html
 http/tests/security/video-cross-origin-accesssameorigin.html
index 24e5cde..aa617d8 100644 (file)
@@ -1,5 +1,25 @@
 2019-01-08  Youenn Fablet  <youenn@apple.com>
 
+        service worker fetch handler results in bad referrer
+        https://bugs.webkit.org/show_bug.cgi?id=188248
+        <rdar://problem/47050478>
+
+        Reviewed by Alex Christensen.
+
+        Response sanitization was removing the ReferrerPolicy header from opaque redirect responses.
+        Reduce sanitization of opaque redirect responses to opaque responses and allow Location header.
+        Make sure referrer policy is updated for all load redirections, not only CORS loads.
+
+        Test: http/tests/security/referrer-policy-redirect-link-downgrade.html
+
+        * loader/SubresourceLoader.cpp:
+        (WebCore::SubresourceLoader::checkRedirectionCrossOriginAccessControl):
+        * platform/network/ResourceResponseBase.cpp:
+        (WebCore::isSafeCrossOriginResponseHeader):
+        (WebCore::ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting):
+
+2019-01-08  Youenn Fablet  <youenn@apple.com>
+
         IDB storage of Crypto keys does not work in private browsing mode
         https://bugs.webkit.org/show_bug.cgi?id=193219
 
index b301bb9..483ecd3 100644 (file)
@@ -567,19 +567,18 @@ bool SubresourceLoader::checkRedirectionCrossOriginAccessControl(const ResourceR
 
     ASSERT(options().mode != FetchOptions::Mode::SameOrigin || !m_resource->isCrossOrigin());
 
-    if (options().mode != FetchOptions::Mode::Cors)
-        return true;
+    // Implementing https://fetch.spec.whatwg.org/#concept-http-redirect-fetch step 7 & 8.
+    if (options().mode == FetchOptions::Mode::Cors) {
+        if (m_resource->isCrossOrigin() && !isValidCrossOriginRedirectionURL(newRequest.url())) {
+            errorMessage = "URL is either a non-HTTP URL or contains credentials."_s;
+            return false;
+        }
 
-    // Implementing https://fetch.spec.whatwg.org/#concept-http-redirect-fetch step 8 & 9.
-    if (m_resource->isCrossOrigin() && !isValidCrossOriginRedirectionURL(newRequest.url())) {
-        errorMessage = "URL is either a non-HTTP URL or contains credentials."_s;
-        return false;
+        ASSERT(m_origin);
+        if (crossOriginFlag && !passesAccessControlCheck(redirectResponse, options().storedCredentialsPolicy, *m_origin, errorMessage))
+            return false;
     }
 
-    ASSERT(m_origin);
-    if (crossOriginFlag && !passesAccessControlCheck(redirectResponse, options().storedCredentialsPolicy, *m_origin, errorMessage))
-        return false;
-
     bool redirectingToNewOrigin = false;
     if (m_resource->isCrossOrigin()) {
         if (!crossOriginFlag && isNextRequestCrossOrigin)
@@ -592,9 +591,10 @@ bool SubresourceLoader::checkRedirectionCrossOriginAccessControl(const ResourceR
     if (crossOriginFlag && redirectingToNewOrigin)
         m_origin = SecurityOrigin::createUnique();
 
+    // Implementing https://fetch.spec.whatwg.org/#concept-http-redirect-fetch step 14.
     updateReferrerPolicy(redirectResponse.httpHeaderField(HTTPHeaderName::ReferrerPolicy));
     
-    if (redirectingToNewOrigin) {
+    if (options().mode == FetchOptions::Mode::Cors && redirectingToNewOrigin) {
         cleanHTTPRequestHeadersForAccessControl(newRequest, options().httpHeadersToKeep);
         updateRequestForAccessControl(newRequest, *m_origin, options().storedCredentialsPolicy);
     }
index c890975..20ac64d 100644 (file)
@@ -401,6 +401,7 @@ static bool isSafeCrossOriginResponseHeader(HTTPHeaderName name)
         || name == HTTPHeaderName::LastEventID
         || name == HTTPHeaderName::LastModified
         || name == HTTPHeaderName::Link
+        || name == HTTPHeaderName::Location
         || name == HTTPHeaderName::Pragma
         || name == HTTPHeaderName::Range
         || name == HTTPHeaderName::ReferrerPolicy
@@ -441,7 +442,8 @@ void ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting()
         m_httpHeaderFields = WTFMove(filteredHeaders);
         return;
     }
-    case ResourceResponse::Tainting::Opaque: {
+    case ResourceResponse::Tainting::Opaque:
+    case ResourceResponse::Tainting::Opaqueredirect: {
         HTTPHeaderMap filteredHeaders;
         for (auto& header : m_httpHeaderFields.commonHeaders()) {
             if (isSafeCrossOriginResponseHeader(header.key))
@@ -450,11 +452,6 @@ void ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting()
         m_httpHeaderFields = WTFMove(filteredHeaders);
         return;
     }
-    case ResourceResponse::Tainting::Opaqueredirect: {
-        auto location = httpHeaderField(HTTPHeaderName::Location);
-        m_httpHeaderFields.clear();
-        m_httpHeaderFields.add(HTTPHeaderName::Location, WTFMove(location));
-    }
     }
 }
 
index 353d602..f6c2a34 100644 (file)
@@ -1,3 +1,22 @@
+2019-01-08  Youenn Fablet  <youenn@apple.com>
+
+        service worker fetch handler results in bad referrer
+        https://bugs.webkit.org/show_bug.cgi?id=188248
+        <rdar://problem/47050478>
+
+        Reviewed by Alex Christensen.
+
+        NetworkDataTaskCocoa is sometimes updating the referrer on its own.
+        Instead of updating the referrer when sending the request to WebProcess for evaluation,
+        Update the referrer once the web process decides to follow the redirection.
+        This ensures that any referrer that the WebProcess will set will be updated by NetworkDataTaskCocoa.
+
+        * NetworkProcess/cocoa/NetworkDataTaskCocoa.h:
+        * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
+        (WebKit::NetworkDataTaskCocoa::NetworkDataTaskCocoa):
+        (WebKit::NetworkDataTaskCocoa::restrictRequestReferrerToOriginIfNeeded):
+        (WebKit::NetworkDataTaskCocoa::willPerformHTTPRedirection):
+
 2019-01-08  Alex Christensen  <achristensen@webkit.org>
 
         Fix more assertions after r239710
index 8424258..503f445 100644 (file)
@@ -89,6 +89,8 @@ private:
     bool tryPasswordBasedAuthentication(const WebCore::AuthenticationChallenge&, ChallengeCompletionHandler&);
     void applySniffingPoliciesAndBindRequestToInferfaceIfNeeded(__strong NSURLRequest*&, bool shouldContentSniff, bool shouldContentEncodingSniff);
 
+    void restrictRequestReferrerToOriginIfNeeded(WebCore::ResourceRequest&, bool shouldBlockCookies);
+
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
     static NSHTTPCookieStorage *statelessCookieStorage();
     void applyCookieBlockingPolicy(bool shouldBlock);
index 51c96fa..a8b113b 100644 (file)
@@ -197,8 +197,7 @@ NetworkDataTaskCocoa::NetworkDataTaskCocoa(NetworkSession& session, NetworkDataT
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
     shouldBlockCookies = session.networkStorageSession().shouldBlockCookies(request, frameID, pageID);
 #endif
-    if (shouldBlockCookies || (m_session->sessionID().isEphemeral() && isThirdPartyRequest(request)))
-        request.setExistingHTTPReferrerToOriginString();
+    restrictRequestReferrerToOriginIfNeeded(request, shouldBlockCookies);
 
     NSURLRequest *nsRequest = request.nsURLRequest(WebCore::HTTPBodyUpdatePolicy::UpdateHTTPBody);
     applySniffingPoliciesAndBindRequestToInferfaceIfNeeded(nsRequest, shouldContentSniff == WebCore::ContentSniffingPolicy::SniffContent && !url.isLocalFile(), shouldContentEncodingSniff == WebCore::ContentEncodingSniffingPolicy::Sniff);
@@ -265,6 +264,12 @@ NetworkDataTaskCocoa::~NetworkDataTaskCocoa()
     }
 }
 
+void NetworkDataTaskCocoa::restrictRequestReferrerToOriginIfNeeded(ResourceRequest& request, bool shouldBlockCookies)
+{
+    if (shouldBlockCookies || (m_session->sessionID().isEphemeral() && isThirdPartyRequest(request)))
+        request.setExistingHTTPReferrerToOriginString();
+}
+
 void NetworkDataTaskCocoa::didSendData(uint64_t totalBytesSent, uint64_t totalBytesExpectedToSend)
 {
     if (m_client)
@@ -355,9 +360,6 @@ void NetworkDataTaskCocoa::willPerformHTTPRedirection(WebCore::ResourceResponse&
 #endif
 #endif
 
-    if (shouldBlockCookies || (m_session->sessionID().isEphemeral() && isThirdPartyRequest(request)))
-        request.setExistingHTTPReferrerToOriginString();
-
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
     // Always apply the policy since blocking may need to be turned on or off in a redirect.
     applyCookieBlockingPolicy(shouldBlockCookies);
@@ -375,7 +377,13 @@ void NetworkDataTaskCocoa::willPerformHTTPRedirection(WebCore::ResourceResponse&
     updateTaskWithFirstPartyForSameSiteCookies(m_task.get(), request);
 
     if (m_client)
-        m_client->willPerformHTTPRedirection(WTFMove(redirectResponse), WTFMove(request), WTFMove(completionHandler));
+        m_client->willPerformHTTPRedirection(WTFMove(redirectResponse), WTFMove(request), [completionHandler = WTFMove(completionHandler), this, protectedThis = makeRef(*this)] (auto&& request) mutable {
+            if (!request.isNull()) {
+                bool shouldBlockCookies = m_session->networkStorageSession().shouldBlockCookies(request, m_frameID, m_pageID);
+                restrictRequestReferrerToOriginIfNeeded(request, shouldBlockCookies);
+            }
+            completionHandler(WTFMove(request));
+        });
     else {
         ASSERT_NOT_REACHED();
         completionHandler({ });