WebKit Bugzilla
Attachment 358452 Details for
Bug 188248
: service worker fetch handler results in bad referrer
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-188248-20190105190657.patch (text/plain), 18.66 KB, created by
youenn fablet
on 2019-01-05 19:06:58 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
youenn fablet
Created:
2019-01-05 19:06:58 PST
Size:
18.66 KB
patch
obsolete
>Subversion Revision: 239660 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 55b71dc9deb21074a32e10417dfd9dbcea258561..81a0a1d578c7c312229f59a16d70986d06067efa 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,23 @@ >+2019-01-05 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 NOBODY (OOPS!). >+ >+ 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-05 Youenn Fablet <youenn@apple.com> > > Service Worker fetch should obey its referrer policy >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 0335e45c08e7d7881ce142c900c30d6847633919..f0458f361c98a846de88c0bb34bc4aef39c04688 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,22 @@ >+2019-01-05 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 NOBODY (OOPS!). >+ >+ 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-04 Joseph Pecoraro <pecoraro@apple.com> > > Web Inspector: Use save sheet instead of dialog where possible >diff --git a/Source/WebCore/loader/SubresourceLoader.cpp b/Source/WebCore/loader/SubresourceLoader.cpp >index b301bb9bf6f7ade6fcdb68219e804f32e0c76b8e..483ecd3c13d61e69a358eb81cca6e364cc452009 100644 >--- a/Source/WebCore/loader/SubresourceLoader.cpp >+++ b/Source/WebCore/loader/SubresourceLoader.cpp >@@ -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); > } >diff --git a/Source/WebCore/platform/network/ResourceResponseBase.cpp b/Source/WebCore/platform/network/ResourceResponseBase.cpp >index c8909755e55ec48f11eed63c8a4e218fa2d01492..20ac64d57034c69ea92bc7a43536f16d795ffa29 100644 >--- a/Source/WebCore/platform/network/ResourceResponseBase.cpp >+++ b/Source/WebCore/platform/network/ResourceResponseBase.cpp >@@ -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)); >- } > } > } > >diff --git a/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h b/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h >index 84242585719eb1add3af8c34d5bd3cac26ab7195..503f44501a001e741ca0ab2f8af825181f2f6525 100644 >--- a/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h >+++ b/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h >@@ -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); >diff --git a/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm b/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm >index 58e10384885954c8f1566459754a45000413f0a7..8bf1f7dbbbfc956827a6ef31beb5aa288ffc5c47 100644 >--- a/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm >+++ b/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm >@@ -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({ }); >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 1ce9b3abfe75aca76d0c1be9a9caec47c2092082..144549179235f2b2a95a3306d0fcd51751657023 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,18 @@ >+2019-01-05 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 NOBODY (OOPS!). >+ >+ * 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 equivalent to previously skipped referrer-policy-redirect-link.html. >+ > 2019-01-05 Commit Queue <commit-queue@webkit.org> > > Unreviewed, rolling out r239607. >diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog >index 60ce6517d77823c4bbca6d055fad9a3c78e9e667..b6d8407610d29411711a0679c9086b8af9108db1 100644 >--- a/LayoutTests/imported/w3c/ChangeLog >+++ b/LayoutTests/imported/w3c/ChangeLog >@@ -1,3 +1,13 @@ >+2019-01-05 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 NOBODY (OOPS!). >+ >+ * web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt: >+ > 2019-01-05 Youenn Fablet <youenn@apple.com> > > Service Worker fetch should obey its referrer policy >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 >index 0000000000000000000000000000000000000000..9faf90e78eb6ca6fe4a9bd6beb5bea6f47d76162 >--- /dev/null >+++ b/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade-expected.txt >@@ -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 >index 0000000000000000000000000000000000000000..8f66e7ec9c34e931f13a4bef91ab1d74749f98d4 >--- /dev/null >+++ b/LayoutTests/http/tests/security/referrer-policy-redirect-link-downgrade.html >@@ -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 >index 0000000000000000000000000000000000000000..fb1bf3368f509cc3ce7a48983e45d86f0293347c >--- /dev/null >+++ b/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link-downgrade.html >@@ -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> >diff --git a/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link.html b/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link.html >index fb1bf3368f509cc3ce7a48983e45d86f0293347c..23dd52624a69b7fac4a28c404ce1dc9365bf7b44 100644 >--- a/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link.html >+++ b/LayoutTests/http/tests/security/resources/referrer-policy-redirect-link.html >@@ -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> >diff --git a/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt >index 2ea4aded04f513ea08ac60820891f62bf0390210..a4f692904444ee0a7555f83d516ce48adaad1031 100644 >--- a/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt >+++ b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt >@@ -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 > >diff --git a/LayoutTests/platform/ios-wk2/TestExpectations b/LayoutTests/platform/ios-wk2/TestExpectations >index c36183dda7a10cd08b0519fd31dabc5bc8df56bf..5b06c3c501af9b98091f33cba6f85e0f3bbabac7 100644 >--- a/LayoutTests/platform/ios-wk2/TestExpectations >+++ b/LayoutTests/platform/ios-wk2/TestExpectations >@@ -354,7 +354,7 @@ http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-window-open.ht > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 188248
:
358364
|
358386
|
358395
|
358402
|
358410
|
358416
|
358419
|
358424
|
358428
|
358429
|
358452
|
358454