Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider redirects when defining same-site #1348

Merged
merged 8 commits into from
Feb 22, 2021

Conversation

englehardt
Copy link
Contributor

Addresses #889.

Note that this change also simplifies the definition by referencing the fetch definition of when two origins are considered same-site. The text is the current version is effectively a copy-paste of that definition.

@englehardt
Copy link
Contributor Author

PTAL @chlily1 @miketaylr @annevk

@chlily1
Copy link
Contributor

chlily1 commented Dec 23, 2020

Thanks, this LGTM!

@ioggstream ioggstream added the 6265bis samesite RFC6265bis's `SameSite` cookie attribute. label Dec 28, 2020
Copy link
Contributor

@reschke reschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems very misleading to me. If you add a date, you'll also need a URI that point to that very version.

@miketaylr
Copy link
Collaborator

If you add a date, you'll also need a URI that point to that very version.

You can grab a snapshot using this fancy button:

Screen Shot 2021-01-25 at 9 29 52 AM

@englehardt
Copy link
Contributor Author

If you add a date, you'll also need a URI that point to that very version.

You can grab a snapshot using this fancy button:

Screen Shot 2021-01-25 at 9 29 52 AM

I looked a bit more at the process here, and linking to a permalink in a spec is discouraged by HTML. See https://whatwg.org/working-mode#anchors. Specifically However, other standards organizations are discouraged from referencing these snapshots, as they generally contain contain known issues that have been fixed in the Living Standard, and can mislead implementers and web developers..

Following that process: I've filed whatwg/html#6329, and will reference that anchor using a non-snapshot link & date as a separate reference from the rest of these HTML terms the spec already references.

@miketaylr
Copy link
Collaborator

@englehardt it would be good to also add a changelog entry here.

@englehardt
Copy link
Contributor Author

@chlily1 Would you mind taking another look at this post-merge with #1384? I've simplified the language in the same-site section regarding how we calculate same-site for requests which are reload navigations resulting from UI. Previously you had:

whether that request is same-site or cross-site should be evaluated based on the client of the request that originally navigated to the reloaded page, rather than the client of the request itself.

But consider the example of a user that types in B.com in the address bar (i.e., no client), B.com HTTP redirects to A.com, and then the user manually refreshes A.com. The B.com to A.com redirect makes the initial load cross-site, and thus we'd expect the user-initiated reload to still be cross-site. But if we followed the quoted text above, we'd use the client of the initial load (i.e., null) and conclude the new request should be same-site. I've instead opted to use the criteria you added later in the draft:

Requests which are the result of a reload navigation triggered through a user interface element are same-site if the reloaded document was originally navigated to via a same-site request.

@englehardt
Copy link
Contributor Author

I've also made a terminology change, moving from target's URI's origin to request's current url's origin for the third criteria. This aligns with what I've added in the first criteria, and actually the original term target was never defined in the draft and isn't used elsewhere.

Base automatically changed from master to main February 15, 2021 06:30
@chlily1
Copy link
Contributor

chlily1 commented Feb 16, 2021

@chlily1 Would you mind taking another look at this post-merge with #1384? I've simplified the language in the same-site section regarding how we calculate same-site for requests which are reload navigations resulting from UI. Previously you had:

whether that request is same-site or cross-site should be evaluated based on the client of the request that originally navigated to the reloaded page, rather than the client of the request itself.

But consider the example of a user that types in B.com in the address bar (i.e., no client), B.com HTTP redirects to A.com, and then the user manually refreshes A.com. The B.com to A.com redirect makes the initial load cross-site, and thus we'd expect the user-initiated reload to still be cross-site. But if we followed the quoted text above, we'd use the client of the initial load (i.e., null) and conclude the new request should be same-site. I've instead opted to use the criteria you added later in the draft:

Requests which are the result of a reload navigation triggered through a user interface element are same-site if the reloaded document was originally navigated to via a same-site request.

Nice catch, thanks! Lgtm.

@englehardt englehardt merged commit 519dd58 into httpwg:main Feb 22, 2021
@englehardt englehardt deleted the cross-site-redirects-cookies branch February 22, 2021 15:45
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 5, 2021
The cookie spec is being amended in
httpwg/http-extensions#1348
to consider the redirect chain when computing whether a request is
considered same-site.

This aligns with the new specification by considering a request cross-
site if any URL in the redirect chain was cross-site from the current
request URL.

Bug: 830101
Change-Id: I060026647ccea2a97267e865c8292ac64915e87b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 5, 2021
The cookie spec is being amended in
httpwg/http-extensions#1348
to consider the redirect chain when computing whether a request is
considered same-site.

This aligns with the new specification by considering a request cross-
site if any URL in the redirect chain was cross-site from the current
request URL.

Bug: 830101
Change-Id: I060026647ccea2a97267e865c8292ac64915e87b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 8, 2021
The cookie spec is being amended in
httpwg/http-extensions#1348
to consider the redirect chain when computing whether a request is
considered same-site.

This aligns with the new specification by considering a request cross-
site if any URL in the redirect chain was cross-site from the current
request URL.

Bug: 830101
Change-Id: I060026647ccea2a97267e865c8292ac64915e87b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 8, 2021
The cookie spec is being amended in
httpwg/http-extensions#1348
to consider the redirect chain when computing whether a request is
considered same-site.

This aligns with the new specification by considering a request cross-
site if any URL in the redirect chain was cross-site from the current
request URL.

Bug: 830101
Change-Id: I060026647ccea2a97267e865c8292ac64915e87b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2605504
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#860890}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 8, 2021
The cookie spec is being amended in
httpwg/http-extensions#1348
to consider the redirect chain when computing whether a request is
considered same-site.

This aligns with the new specification by considering a request cross-
site if any URL in the redirect chain was cross-site from the current
request URL.

Bug: 830101
Change-Id: I060026647ccea2a97267e865c8292ac64915e87b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2605504
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#860890}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Mar 9, 2021
The cookie spec is being amended in
httpwg/http-extensions#1348
to consider the redirect chain when computing whether a request is
considered same-site.

This aligns with the new specification by considering a request cross-
site if any URL in the redirect chain was cross-site from the current
request URL.

Bug: 830101
Change-Id: I060026647ccea2a97267e865c8292ac64915e87b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2605504
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#860890}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Mar 19, 2021
…n for same-site requests, a=testonly

Automatic update from web-platform-tests
SameSite cookies: Consider redirect chain for same-site requests

The cookie spec is being amended in
httpwg/http-extensions#1348
to consider the redirect chain when computing whether a request is
considered same-site.

This aligns with the new specification by considering a request cross-
site if any URL in the redirect chain was cross-site from the current
request URL.

Bug: 830101
Change-Id: I060026647ccea2a97267e865c8292ac64915e87b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2605504
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#860890}

--

wpt-commits: f5d7be1e657bdba8621715da535ba6058ca411a7
wpt-pr: 27902
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Jun 3, 2021
This adds a base::Feature (disabled by default) to control the redirect
chain checking for SameSite cookies.

When the feature is enabled, any cross-site redirect hop makes a request
cross-site. This is consistent with the behavior specified by RFC
6265bis as of httpwg/http-extensions#1348.

When the feature is disabled, only the site-for-cookies and
initiator are considered, relative to the request URL. This is
consistent with the behavior specified by previous versions of the spec.

The redirect chain checking behavior is being hidden behind a Feature
due to site breakage. We intend to re-enable it later.

The new base::Feature is added to the set of features enabled by
--enable-experimental-web-platform-features, so that web tests can run
with the feature enabled (so as to match Firefox's behavior).

The new base::Feature is also added to the set of features enabled by
--enable-experimental-cookie-features.

Bug: 1215167
Change-Id: Ie9a637f8ab0921f13559254caba93772ba8990fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2930367
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#888696}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 26, 2021
These tests were temporarily disabled due to an incompatibility between
Firefox and Chromium. Now that this has been resolved by a spec PR
(httpwg/http-extensions#1428), the tests can be
re-enabled.

Additionally, one of the tests was adjusted to reflect the behavior for
redirects specified in
httpwg/http-extensions#1348, which is enabled as
part of --enable-experimental-web-platform-features.

Bug: 1074441
Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 26, 2021
These tests were temporarily disabled due to an incompatibility between
Firefox and Chromium. Now that this has been resolved by a spec PR
(httpwg/http-extensions#1428), the tests can be
re-enabled.

Additionally, one of the tests was adjusted to reflect the behavior for
redirects specified in
httpwg/http-extensions#1348, which is enabled as
part of --enable-experimental-web-platform-features.

Bug: 1074441
Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294
Reviewed-by: Steven Bingler <bingler@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905421}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 26, 2021
These tests were temporarily disabled due to an incompatibility between
Firefox and Chromium. Now that this has been resolved by a spec PR
(httpwg/http-extensions#1428), the tests can be
re-enabled.

Additionally, one of the tests was adjusted to reflect the behavior for
redirects specified in
httpwg/http-extensions#1348, which is enabled as
part of --enable-experimental-web-platform-features.

Bug: 1074441
Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294
Reviewed-by: Steven Bingler <bingler@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905421}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Jul 27, 2021
These tests were temporarily disabled due to an incompatibility between
Firefox and Chromium. Now that this has been resolved by a spec PR
(httpwg/http-extensions#1428), the tests can be
re-enabled.

Additionally, one of the tests was adjusted to reflect the behavior for
redirects specified in
httpwg/http-extensions#1348, which is enabled as
part of --enable-experimental-web-platform-features.

Bug: 1074441
Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294
Reviewed-by: Steven Bingler <bingler@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905421}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 30, 2021
…=testonly

Automatic update from web-platform-tests
Re-enable SameSite cookie iframe WPTs

These tests were temporarily disabled due to an incompatibility between
Firefox and Chromium. Now that this has been resolved by a spec PR
(httpwg/http-extensions#1428), the tests can be
re-enabled.

Additionally, one of the tests was adjusted to reflect the behavior for
redirects specified in
httpwg/http-extensions#1348, which is enabled as
part of --enable-experimental-web-platform-features.

Bug: 1074441
Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294
Reviewed-by: Steven Bingler <bingler@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905421}

--

wpt-commits: d4deee9e5303f3c09f65bc0d40783cc18a30606e
wpt-pr: 29792
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 31, 2021
…=testonly

Automatic update from web-platform-tests
Re-enable SameSite cookie iframe WPTs

These tests were temporarily disabled due to an incompatibility between
Firefox and Chromium. Now that this has been resolved by a spec PR
(httpwg/http-extensions#1428), the tests can be
re-enabled.

Additionally, one of the tests was adjusted to reflect the behavior for
redirects specified in
httpwg/http-extensions#1348, which is enabled as
part of --enable-experimental-web-platform-features.

Bug: 1074441
Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294
Reviewed-by: Steven Bingler <bingler@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905421}

--

wpt-commits: d4deee9e5303f3c09f65bc0d40783cc18a30606e
wpt-pr: 29792
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Aug 4, 2021
…=testonly

Automatic update from web-platform-tests
Re-enable SameSite cookie iframe WPTs

These tests were temporarily disabled due to an incompatibility between
Firefox and Chromium. Now that this has been resolved by a spec PR
(httpwg/http-extensions#1428), the tests can be
re-enabled.

Additionally, one of the tests was adjusted to reflect the behavior for
redirects specified in
httpwg/http-extensions#1348, which is enabled as
part of --enable-experimental-web-platform-features.

Bug: 1074441
Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294
Reviewed-by: Steven Bingler <bingler@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905421}

--

wpt-commits: d4deee9e5303f3c09f65bc0d40783cc18a30606e
wpt-pr: 29792
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Aug 4, 2021
…=testonly

Automatic update from web-platform-tests
Re-enable SameSite cookie iframe WPTs

These tests were temporarily disabled due to an incompatibility between
Firefox and Chromium. Now that this has been resolved by a spec PR
(httpwg/http-extensions#1428), the tests can be
re-enabled.

Additionally, one of the tests was adjusted to reflect the behavior for
redirects specified in
httpwg/http-extensions#1348, which is enabled as
part of --enable-experimental-web-platform-features.

Bug: 1074441
Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294
Reviewed-by: Steven Bingler <bingler@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905421}

--

wpt-commits: d4deee9e5303f3c09f65bc0d40783cc18a30606e
wpt-pr: 29792
GrumpyOldTroll pushed a commit to GrumpyOldTroll/chromium that referenced this pull request Sep 4, 2021
[Merge to M93 branch]

This adds histograms and UseCounter/UKM to measure the impact of the
cookie spec change in
httpwg/http-extensions#1348, which incorporates
the redirect chain into the computation of a request's same-site or
cross-site status.

The histograms measure, on a per-cookie level, how many cookies are
affected by the change (i.e. how many cookies have their ultimate
inclusion changed by considering a cross-site redirect to make the
request cross-site) as well as the SameSite attributes of those affected
cookies.

The UseCounter/UKM measures how many pageloads have impacted cookies.

(cherry picked from commit 2db3a42)

Bug: 1221316
Change-Id: I018f110466aa08e304c46f6de26fb1cc18facf33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3024588
Reviewed-by: Steven Bingler <bingler@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#903573}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3049050
Auto-Submit: Lily Chen <chlily@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Prudhvi Kumar Bommana <pbommana@google.com>
Owners-Override: Prudhvi Kumar Bommana <pbommana@google.com>
Cr-Commit-Position: refs/branch-heads/4577@{chromium#109}
Cr-Branched-From: 761ddde-refs/heads/master@{#902210}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
The cookie spec is being amended in
httpwg/http-extensions#1348
to consider the redirect chain when computing whether a request is
considered same-site.

This aligns with the new specification by considering a request cross-
site if any URL in the redirect chain was cross-site from the current
request URL.

Bug: 830101
Change-Id: I060026647ccea2a97267e865c8292ac64915e87b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2605504
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#860890}
GitOrigin-RevId: 306b8fba167a809c5389a58d65bee438ca3bd15d
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This adds histograms and UseCounter/UKM to measure the impact of the
cookie spec change in
httpwg/http-extensions#1348, which incorporates
the redirect chain into the computation of a request's same-site or
cross-site status.

The histograms measure, on a per-cookie level, how many cookies are
affected by the change (i.e. how many cookies have their ultimate
inclusion changed by considering a cross-site redirect to make the
request cross-site) as well as the SameSite attributes of those affected
cookies.

The UseCounter/UKM measures how many pageloads have impacted cookies.

Bug: 1221316
Change-Id: I018f110466aa08e304c46f6de26fb1cc18facf33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3024588
Reviewed-by: Steven Bingler <bingler@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#903573}
NOKEYCHECK=True
GitOrigin-RevId: 2db3a4287cc5ebbd7b1189509380cfc5647715ef
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
These tests were temporarily disabled due to an incompatibility between
Firefox and Chromium. Now that this has been resolved by a spec PR
(httpwg/http-extensions#1428), the tests can be
re-enabled.

Additionally, one of the tests was adjusted to reflect the behavior for
redirects specified in
httpwg/http-extensions#1348, which is enabled as
part of --enable-experimental-web-platform-features.

Bug: 1074441
Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294
Reviewed-by: Steven Bingler <bingler@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905421}
NOKEYCHECK=True
GitOrigin-RevId: 41ff2ed8246af31d078a263d228de4533b5991a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6265bis samesite RFC6265bis's `SameSite` cookie attribute.
Development

Successfully merging this pull request may close these issues.

None yet

6 participants