WebKit Bugzilla
Attachment 362618 Details for
Bug 194906
: Same Site Lax cookies are not sent with cross-site redirect from client-initiated load
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch and layout test
bug-194906-20190221103156.patch (text/plain), 8.27 KB, created by
Daniel Bates
on 2019-02-21 10:31:59 PST
(
hide
)
Description:
Patch and layout test
Filename:
MIME Type:
Creator:
Daniel Bates
Created:
2019-02-21 10:31:59 PST
Size:
8.27 KB
patch
obsolete
>Subversion Revision: 241860 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 26277ea5e6422ed0ab34dfff421fa7cbc3266123..d6657dcb35346b6b1ba53b5c4b0e41867076f0b3 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,39 @@ >+2019-02-21 Daniel Bates <dabates@apple.com> >+ >+ Same Site Lax cookies are not sent with cross-site redirect from client-initiated load >+ https://bugs.webkit.org/show_bug.cgi?id=194906 >+ <rdar://problem/44305947> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Ensure that a request for a top-level navigation is annotated as such regardless of whether >+ the request has a computed Same Site policy. >+ >+ "New loads" initiated by a the client (Safari) either by API or a human either explicitly >+ typing a URL in the address bar or Command + clicking a hyperlink to open it in a new window/tab >+ are always considered Same Site. This is by definition from the spec. [1] as we aren't navigating >+ from an existing page. (Command + click should be thought of as a convenience to the user from >+ having to copy the hyperlink's URL, create a new window, and paste the URL into the address bar). >+ Currently the frame loader marks a request as a top-level navigation if and only if the request >+ does not have a pre-computed Same Site policy. However, "New loads" have a pre-computed Same Site >+ policy. So, these loads would never be marked as a top-level navigation by the frame loading code. >+ Therefore, if the "new load" turned out to be a cross-site redirect then WebKit would incorrectly >+ tell the networking stack that the load was a cross-site, non-top-level navigation, and per the >+ Same Site spec [2], the networking stack would not send Same Site Lax cookies. Instead, >+ WebKit should unconditionally ensure that requests are marked as a top-level navigation, if applicable. >+ >+ [1] See Note for (1) in <https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-02#section-5.2> >+ [2] <https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-02#section-5.3.7.1> >+ >+ Test: http/tests/cookies/same-site/user-load-cross-site-redirect.php >+ >+ * loader/FrameLoader.cpp: >+ (WebCore::FrameLoader::addExtraFieldsToRequest): Unconditionally update the request's top- >+ level navigation bit. >+ * platform/network/ResourceRequestBase.cpp: >+ (WebCore::ResourceRequestBase::setAsIsolatedCopy): Unconditionally copy a request's top- >+ level navigation bit. >+ > 2019-02-20 Simon Fraser <simon.fraser@apple.com> > > REGRESSION (240698): Fixed position banners flicker and move when scrolling on iOS >diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp >index 109b7d355d142531c42506ee71f5fd620cace02b..a611dc7437380220d933b554d7963cfcf5152cfa 100644 >--- a/Source/WebCore/loader/FrameLoader.cpp >+++ b/Source/WebCore/loader/FrameLoader.cpp >@@ -2885,8 +2885,8 @@ void FrameLoader::addExtraFieldsToRequest(ResourceRequest& request, FrameLoadTyp > ASSERT(ownerFrame || m_frame.isMainFrame()); > } > addSameSiteInfoToRequestIfNeeded(request, initiator); >- request.setIsTopSite(isMainResource && m_frame.isMainFrame()); > } >+ request.setIsTopSite(isMainResource && m_frame.isMainFrame()); > > Page* page = frame().page(); > bool hasSpecificCachePolicy = request.cachePolicy() != ResourceRequestCachePolicy::UseProtocolCachePolicy; >diff --git a/Source/WebCore/platform/network/ResourceRequestBase.cpp b/Source/WebCore/platform/network/ResourceRequestBase.cpp >index 625ac51f7e39c19e32fbcb52d62a74ffecc1b273..d540f44c3bd2944408e67dc5227a47ec341c0527 100644 >--- a/Source/WebCore/platform/network/ResourceRequestBase.cpp >+++ b/Source/WebCore/platform/network/ResourceRequestBase.cpp >@@ -70,10 +70,9 @@ void ResourceRequestBase::setAsIsolatedCopy(const ResourceRequest& other) > if (auto inspectorInitiatorNodeIdentifier = other.inspectorInitiatorNodeIdentifier()) > setInspectorInitiatorNodeIdentifier(*inspectorInitiatorNodeIdentifier); > >- if (!other.isSameSiteUnspecified()) { >+ if (!other.isSameSiteUnspecified()) > setIsSameSite(other.isSameSite()); >- setIsTopSite(other.isTopSite()); >- } >+ setIsTopSite(other.isTopSite()); > > updateResourceRequest(); > m_httpHeaderFields = other.httpHeaderFields().isolatedCopy(); >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index f344270aabe0148ae93eef4e820766545e8b4ee9..82163afd343f35c7240ed2558ae2cb663e9a250f 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,17 @@ >+2019-02-21 Daniel Bates <dabates@apple.com> >+ >+ Same Site Lax cookies are not sent with cross-site redirect from client-initiated load >+ https://bugs.webkit.org/show_bug.cgi?id=194906 >+ <rdar://problem/44305947> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Add a test that is representative of a user loading a cross-site page that redirects >+ to a page that expects Same Site Lax cookies. >+ >+ * http/tests/cookies/same-site/user-load-cross-site-redirect-expected.txt: Added. >+ * http/tests/cookies/same-site/user-load-cross-site-redirect.php: Added. >+ > 2019-02-20 Antti Koivisto <antti@apple.com> > > Make programmatic frame scrolling work on iOS >diff --git a/LayoutTests/http/tests/cookies/same-site/user-load-cross-site-redirect-expected.txt b/LayoutTests/http/tests/cookies/same-site/user-load-cross-site-redirect-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..b0f29fd449f22bc5def7f34ec77d765f5144daea >--- /dev/null >+++ b/LayoutTests/http/tests/cookies/same-site/user-load-cross-site-redirect-expected.txt >@@ -0,0 +1,18 @@ >+This test is representative of a user that loads a site, via the address bar or Command + clicking a hyperlink, that redirects to a cross-site page that expects its SameSite Lax cookies. >+ >+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". >+ >+ >+Cookies sent with HTTP request: >+PASS Do not have cookie "strict". >+PASS Has cookie "lax" with value 27. >+PASS Has cookie "normal" with value 27. >+ >+Cookies visible in DOM: >+PASS Do not have DOM cookie "strict". >+PASS Has DOM cookie "lax" with value 27. >+PASS Has DOM cookie "normal" with value 27. >+PASS successfullyParsed is true >+ >+TEST COMPLETE >+ >diff --git a/LayoutTests/http/tests/cookies/same-site/user-load-cross-site-redirect.php b/LayoutTests/http/tests/cookies/same-site/user-load-cross-site-redirect.php >new file mode 100644 >index 0000000000000000000000000000000000000000..ca1ffa7b7bd5a8fd7114283756c951b4b0baee56 >--- /dev/null >+++ b/LayoutTests/http/tests/cookies/same-site/user-load-cross-site-redirect.php >@@ -0,0 +1,45 @@ >+<?php >+ include_once("../resources/cookie-utilities.php"); >+ >+ if (hostnameIsEqualToString("127.0.0.1") && empty($_SERVER["QUERY_STRING"])) { >+ wkSetCookie("strict", "27", Array("SameSite" => "Strict", "Max-Age" => 100, "path" => "/")); >+ wkSetCookie("lax", "27", Array("SameSite" => "Lax", "Max-Age" => 100, "path" => "/")); >+ wkSetCookie("normal", "27", Array("Max-Age" => 100, "path" => "/")); >+ header("Location: http://localhost:8000/resources/redirect.php?url=" . urlencode("http://127.0.0.1:8000" . $_SERVER["REQUEST_URI"] . "?check-cookies")); >+ exit(0); >+ } >+?> >+<!DOCTYPE html> >+<html> >+<head> >+<script src="/js-test-resources/js-test.js"></script> >+<script src="../resources/cookie-utilities.js"></script> >+<script>_setCachedCookiesJSON('<?php echo json_encode($_COOKIE); ?>')</script> >+</head> >+<body> >+<script> >+window.jsTestIsAsync = true; >+ >+description("This test is representative of a user that loads a site, via the address bar or Command + clicking a hyperlink, that redirects to a cross-site page that expects its SameSite Lax cookies."); >+ >+async function checkResult() >+{ >+ debug("Cookies sent with HTTP request:"); >+ await shouldNotHaveCookie("strict"); >+ await shouldHaveCookieWithValue("lax", "27"); >+ await shouldHaveCookieWithValue("normal", "27"); >+ >+ debug("<br>Cookies visible in DOM:"); >+ shouldNotHaveDOMCookie("strict"); >+ shouldHaveDOMCookieWithValue("lax", "27"); >+ shouldHaveDOMCookieWithValue("normal", "27"); >+ >+ await resetCookies(); >+ finishJSTest(); >+} >+ >+checkResult(); >+</script> >+</body> >+</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
Flags:
bfulgham
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194906
: 362618