Title: [239749] trunk
Revision
239749
Author
you...@apple.com
Date
2019-01-08 15:06:12 -0800 (Tue, 08 Jan 2019)

Log Message

service worker fetch handler results in bad referrer
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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (239748 => 239749)


--- trunk/LayoutTests/ChangeLog	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/LayoutTests/ChangeLog	2019-01-08 23:06:12 UTC (rev 239749)
@@ -1,5 +1,20 @@
 2019-01-08  Youenn Fablet  <you...@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  <you...@apple.com>
+
         IDB storage of Crypto keys does not work in private browsing mode
         https://bugs.webkit.org/show_bug.cgi?id=193219
 

Added: trunk/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade-expected.txt (0 => 239749)


--- trunk/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade-expected.txt	2019-01-08 23:06:12 UTC (rev 239749)
@@ -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
+

Added: trunk/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade.html (0 => 239749)


--- trunk/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade.html	2019-01-08 23:06:12 UTC (rev 239749)
@@ -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=""
+</body>
+</html>

Copied: trunk/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link-downgrade.html (from rev 239748, trunk/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link.html) (0 => 239749)


--- trunk/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link-downgrade.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link-downgrade.html	2019-01-08 23:06:12 UTC (rev 239749)
@@ -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="" rel="opener">If not running in DumpRenderTree, click this link</a>
+<div id="log"></div>
+</body>
+</html>

Modified: trunk/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link.html (239748 => 239749)


--- trunk/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link.html	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link.html	2019-01-08 23:06:12 UTC (rev 239749)
@@ -21,7 +21,7 @@
 </script>
 </head>
 <body>
-<a id="link" target="_blank" href="" rel="opener">If not running in DumpRenderTree, click this link</a>
+<a id="link" target="_blank" href="" rel="opener">If not running in DumpRenderTree, click this link</a>
 <div id="log"></div>
 </body>
 </html>

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (239748 => 239749)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2019-01-08 23:06:12 UTC (rev 239749)
@@ -1,3 +1,13 @@
+2019-01-08  Youenn Fablet  <you...@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  <d...@apple.com>
 
         Turn on Pointer Events by default for iOS

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt (239748 => 239749)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt	2019-01-08 23:06:12 UTC (rev 239749)
@@ -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 
 

Modified: trunk/LayoutTests/platform/ios-wk2/TestExpectations (239748 => 239749)


--- trunk/LayoutTests/platform/ios-wk2/TestExpectations	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/LayoutTests/platform/ios-wk2/TestExpectations	2019-01-08 23:06:12 UTC (rev 239749)
@@ -355,6 +355,7 @@
 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

Modified: trunk/Source/WebCore/ChangeLog (239748 => 239749)


--- trunk/Source/WebCore/ChangeLog	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/Source/WebCore/ChangeLog	2019-01-08 23:06:12 UTC (rev 239749)
@@ -1,5 +1,25 @@
 2019-01-08  Youenn Fablet  <you...@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  <you...@apple.com>
+
         IDB storage of Crypto keys does not work in private browsing mode
         https://bugs.webkit.org/show_bug.cgi?id=193219
 

Modified: trunk/Source/WebCore/loader/SubresourceLoader.cpp (239748 => 239749)


--- trunk/Source/WebCore/loader/SubresourceLoader.cpp	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/Source/WebCore/loader/SubresourceLoader.cpp	2019-01-08 23:06:12 UTC (rev 239749)
@@ -567,19 +567,18 @@
 
     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 @@
     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);
     }

Modified: trunk/Source/WebCore/platform/network/ResourceResponseBase.cpp (239748 => 239749)


--- trunk/Source/WebCore/platform/network/ResourceResponseBase.cpp	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/Source/WebCore/platform/network/ResourceResponseBase.cpp	2019-01-08 23:06:12 UTC (rev 239749)
@@ -401,6 +401,7 @@
         || 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 @@
         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,12 +452,7 @@
         m_httpHeaderFields = WTFMove(filteredHeaders);
         return;
     }
-    case ResourceResponse::Tainting::Opaqueredirect: {
-        auto location = httpHeaderField(HTTPHeaderName::Location);
-        m_httpHeaderFields.clear();
-        m_httpHeaderFields.add(HTTPHeaderName::Location, WTFMove(location));
     }
-    }
 }
 
 void ResourceResponseBase::sanitizeHTTPHeaderFields(SanitizationType type)

Modified: trunk/Source/WebKit/ChangeLog (239748 => 239749)


--- trunk/Source/WebKit/ChangeLog	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/Source/WebKit/ChangeLog	2019-01-08 23:06:12 UTC (rev 239749)
@@ -1,3 +1,22 @@
+2019-01-08  Youenn Fablet  <you...@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  <achristen...@webkit.org>
 
         Fix more assertions after r239710

Modified: trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h (239748 => 239749)


--- trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h	2019-01-08 23:06:12 UTC (rev 239749)
@@ -89,6 +89,8 @@
     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);

Modified: trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm (239748 => 239749)


--- trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm	2019-01-08 22:23:12 UTC (rev 239748)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm	2019-01-08 23:06:12 UTC (rev 239749)
@@ -197,8 +197,7 @@
 #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 @@
     }
 }
 
+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 @@
 #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 @@
     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({ });
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to