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

[RFC6265bis WGLC] Lax-allowing-unsafe narrative #2791

Closed
bc-pi opened this issue May 30, 2024 · 7 comments · Fixed by #2812
Closed

[RFC6265bis WGLC] Lax-allowing-unsafe narrative #2791

bc-pi opened this issue May 30, 2024 · 7 comments · Fixed by #2812
Assignees
Labels
6265bis samesite RFC6265bis's `SameSite` cookie attribute. 6265bis

Comments

@bc-pi
Copy link
Contributor

bc-pi commented May 30, 2024

I don't believe this example given in 8.8.6. Top-level requests with "unsafe" methods particularity well reflects the situation that precipitated the behavior described in 5.5.7.2. "Lax-Allowing-Unsafe" enforcement:

For example, a login flow may involve a cross-site top-level POST request to an endpoint which expects a cookie with login information. For such a cookie, "Lax" enforcement is not appropriate, as it would cause the cookie to be excluded due to the unsafe HTTP request method. On the other hand, "None" enforcement would allow the cookie to be sent with all cross-site requests, which may not be desirable due to the cookie's sensitive contents.

It's only an explanatory example so arguably doesn't even really matter. But RFCs are forever and they are often cited so it'd be nice to have this bit of the historical narrative better reflect the realities at the time (I say this mostly from perspective of having experienced some of the difficulties caused by the introduction of SameSite cookies and their default enforcement mode and, to this day, continuing to have challenging conversations on the topic).

As best I can recall, login type flows were the main driver for the "Lax-allowing-unsafe" default mode but the need manifested itself more in the later part of the flow where the authenticating site (IDP) caused the browser to make a POST request callback to the site requesting authentication (RP). At the start of the flow, prior to sending the user to the IDP, the RP would typically set a cookie with or referencing some transaction specific state and use that info during processing of the callback for validation of the token provided by the IDP. Cookies with Lax as the default enforcement mode would not be sent with the callback POST request and the whole login flow would fail. The "Lax-allowing-unsafe" default mode (aka "Lax + POST mitigation") allowed those flows to not break for a while and gave RPs (and their vendors, frameworks, etc.) a little grace period to update to explicitly designate those cookies as SameSite=None.

Could we augment or replace the example described in 8.8.6. to account for all that?

The cookie age stuff in 5.5.7.2. , I think, makes more sense in the context of the above too.

I'm (obviously) having trouble articulating some of this so including some "helpful links" here too, which may or may not describe some of this better.

Chromium called it the "Lax + POST mitigation" where the cookie would have a CSRF token:
https://www.chromium.org/updates/same-site/faq/#q-what-is-the-lax-post-mitigation

This explainer by some Googlers describes some of this in Top-Level Cross-Site POST Requests:
https://github.com/explainers-by-googlers/standardizing-cross-site-cookie-semantics?tab=readme-ov-file#top-level-cross-site-post-requests

and links to this payment flow which is very similar to the login flow:
https://github.com/GoogleChromeLabs/samesite-examples/blob/3d-secure-impl/3d-secure.md

The question in this issue on kinda related stuff came back to my attention very recently, which is what led me to looking at these parts of RFC6265bis again. Which maybe explains some of why I'm putting in this annoying issue near the very end of WGLC. Or explains the timing anyway.
w3c-fedid/FedCM#212

@b---c b---c added 6265bis 6265bis samesite RFC6265bis's `SameSite` cookie attribute. labels May 30, 2024
@sbingler
Copy link
Collaborator

To clarify,

You'd like the example to be one which is closer to a real world case? Namely FedCM (guessing from your use of "RP", "IDP", and the issue on the repo). I'd like to avoid getting too specific and in the weeds but there's probably a way to generalize the FedCM case.

Contextualizing the cookie age also seems like a positive to me, its relevance isn't super clear otherwise.

@sbingler sbingler self-assigned this May 31, 2024
@bc-pi
Copy link
Contributor Author

bc-pi commented Jun 3, 2024

You'd like the example to be one which is closer to a real world case?

Yes. Apologies if that wasn't clear from my rambling above. And to try and be more clear, I'd like the example to be one which is closer to the real world cases that gave rise to the Lax-allowing-unsafe mode.

Namely FedCM (guessing from your use of "RP", "IDP", and the issue on the repo).

No, namely SAML with the authentication response delivered with the POST binding. Or OpenID Connect using the form post response mode where the 'nonce cookie' is needed to validate the returned ID token.

I'd like to avoid getting too specific and in the weeds but there's probably a way to generalize the FedCM case.

Agreed about not getting too specific and in the weeds but think the SAML/OIDC cases could be generalized too. Although I tried to do that in the issue description here and didn't seem to be too successful in conveying the concept.

Contextualizing the cookie age also seems like a positive to me, its relevance isn't super clear otherwise.

In my mind it makes a lot more sense in the context of a short term cookie used for transaction specific state and validation. And doesn't make a lot of sense in the context of "cookie[s] with login information". I guess my hope is that it would just be better contextualized by way of using an example that's more reflective of the historic occurrence. But more contextualizing, if needed/appropriate, would certainly be okay by me too.

@miketaylr
Copy link
Collaborator

@bc-pi could you propose a diff to the paragraph:

For example, a login flow may involve a cross-site top-level POST request to an endpoint which expects a cookie with login information. For such a cookie, "Lax" enforcement is not appropriate, as it would cause the cookie to be excluded due to the unsafe HTTP request method. On the other hand, "None" enforcement would allow the cookie to be sent with all cross-site requests, which may not be desirable due to the cookie's sensitive contents.

Or is it sufficient to just tweak the first sentence of that paragraph?

For someone who is not super familiar with SAML authentication flows (👋 , it's me), what exists in the draft text today feels like a fuzzier higher-level description than:

he later part of the flow where the authenticating site (IDP) caused the browser to make a POST request callback to the site requesting authentication (RP). At the start of the flow, prior to sending the user to the IDP, the RP would typically set a cookie with or referencing some transaction specific state and use that info during processing of the callback for validation of the token provided by the IDP. Cookies with Lax as the default enforcement mode would not be sent with the callback POST request and the whole login flow would fail.

It's entirely possible that's the only pattern that motivated Lax-allowing-unsafe, but maybe not - the web is messy. Maybe a compromise here is to tweak the text to cover your example generally (as well as other examples sitauations to ~POST w/ a cookie expecting some login information), and go super precise in the commit message. Just a thought.

@bc-pi
Copy link
Contributor Author

bc-pi commented Jun 6, 2024

@bc-pi could you propose a diff to the paragraph:

For example [...] cookie's sensitive contents.

Or is it sufficient to just tweak the first sentence of that paragraph?

It'd be a bit more involved than just the first sentence but I'm happy to take a stab at reworking that paragraph and maybe a few other very minor related things, if the authors/WG are amenable to the idea?

For someone who is not super familiar with SAML authentication flows (👋 , it's me),

Not to be expected :) The OpenID Connect flow is probably more salient but familiarity there isn't expected either.

what exists in the draft text today feels like a fuzzier higher-level description than:

he later part of the [...] whole login flow would fail.

The important distinction (to me anyway) is where the cookie in question would be sent (or not sent), the associated failure mode, and the effectiveness of the mitigating default mode.

With what exists in the draft text today, the cookie is holding/referencing login state that's sent to the site that will authenticate the user (maybe based on an existing session identified by that cookie). If the cookie isn't sent (due to default of Lax and a x-site POST style redirect), the failure mode will usually be the user needing to enter their login credentials again. Which is annoying but recoverable. Also the Lax-allowing-unsafe applicability to "cookies which were created recently [~2 mins]" doesn't help in many many cases for cookie that's used for login state due to such cookies being relatively long lived (not created recently) and/or the user not having even visiting the site recently.

With the pattern I'm (trying) to describe, the cookie is holding transactional state that's sent back to the site requesting authentication after user authentication. If the cookie isn't sent (due to default of Lax and a x-site POST style redirect back to the originating site), the failure mode will usually be an unrecoverable error for the whole login flow. The Lax-allowing-unsafe applicability to "cookies which were created recently [~2 mins]" is quite helpful in this case because that transactional state specific cookie will typically be set right at the beginning of the whole login flow and only needs to live as long as it takes for the user to complete the login journey.

It's entirely possible that's the only pattern that motivated Lax-allowing-unsafe, but maybe not - the web is messy.

Oh yes, of course, it's very messy. I'm sure it wasn't the only pattern that motivated Lax-allowing-unsafe but I'm almost as sure that it was the primary pattern that motivated it.

Maybe a compromise here is to tweak the text to cover your example generally (as well as other examples sitauations to ~POST w/ a cookie expecting some login information), and go super precise in the commit message. Just a thought.

Let me see if I can tweak the text to cover things generally (without screwing it up) and the commit message can reference this issue to link back for more info?

@sbingler
Copy link
Collaborator

Hi @bc-pi,

Have you had any progress?

@bc-pi
Copy link
Contributor Author

bc-pi commented Jun 27, 2024

I'm sorry @sbingler, probably my fault in the way I left the prior comment but I wasn't sure if I was supposed to be working on a PR or was waiting for an okay from you/WG/etc to do so. And in the intervening time my attention for it was sort of overtaken by events. Apologies again and I'll try and do something soon.

@bc-pi
Copy link
Contributor Author

bc-pi commented Jun 27, 2024

#2812 is a minimalist attempt at some adjusted text for this

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. 6265bis
Development

Successfully merging a pull request may close this issue.

4 participants