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

Setting samesite cookies when not same site context #594

Closed
aliams opened this issue Apr 25, 2018 · 9 comments
Closed

Setting samesite cookies when not same site context #594

aliams opened this issue Apr 25, 2018 · 9 comments
Labels

Comments

@aliams
Copy link

aliams commented Apr 25, 2018

What happens when a page tries to set a samesite cookie when we are not in a samesite context? For instance, a top-level a.com has an iframe to b.com and b.com tries to set a cookie with the samesite attribute. It sounds like we would want the cookie to not be writeable since it's not readable in that case.

cc @mikewest

@fmarier
Copy link

fmarier commented Apr 26, 2018

@mnot mnot added the 6265bis label Apr 30, 2018
@johnwilander
Copy link
Contributor

johnwilander commented Dec 11, 2018

I vaguely remember discussions on this, i.e. whether the whole cookie should be dropped or just the SameSite attribute. Anyone on this thread who remembers? (Part of what I remember is that Mozilla changed their mind.)

@ericlaw1979
Copy link

Blocking SameSite cookie creation in cross-origin POST contexts means that an OAuth cookie that is set as a result from a POST from the auth provider site will fail. This seems undesirable.

@mikewest
Copy link
Member

I think it's pretty clear that we ought to be blocking the creation of SameSite={Lax,Strict} cookies in cross-site subresource requests. Firefox does that today, and Chrome just landed a patch to do the same.

Navigations are more interesting. We send SameSite=Lax cookies along with "safe" HTTP methods (e.g. GET, but not POST). Chrome's implementation applies the same restriction to SameSite=Lax creation. I assumed that Firefox's implementation did the same, but @ericlaw1979 suggests in https://bugs.chromium.org/p/chromium/issues/detail?id=837412#c18 that it doesn't. That seems strange to me, though perhaps justifiable given the use case Eric's pointing to.

I think I could live with a world in which top-level navigations (regardless of method) are empowered to create SameSite cookies (but keep the rules around sending them as they are). Would that address your concerns, Eric? Does it open us up to attacks I'm not thinking about?

/cc @mozmark @morlovich

@ericlaw1979
Copy link

ericlaw1979 commented Apr 30, 2019

I've built a reduced repro for this scenario at https://webdbg.com/test/cookie/samesite/post.aspx .

Firefox Nightly ignores the attempt to set the SameSite cookie for the cross-origin image, but permits the cookie to be set during the form post.

I believe that the proposed fix (matching Firefox) unblocks the OAuth scenario regression we've found, although I do wonder about the notion that a context shouldn't be able to set cookies that it, itself, cannot read (e.g. a page can currently set secure or path= to which the context itself does not belong).

@mikewest
Copy link
Member

mikewest commented May 1, 2019

Firefox Nightly ignores the attempt to set the SameSite cookie for the cross-origin image, but permits the cookie to be set during the form post.

Yeah, that does seem to be Firefox's behavior. I'm curious about how they landed on that, and how exactly they're determining whether to allow the cookie to be set. Are they just ignoring the "safe method" bit? Do nested navigations work as well? @mozmark, WDYT?

I do wonder about the notion that a context shouldn't be able to set cookies that it, itself, cannot read (e.g. a page can currently set secure or path= to which the context itself does not belong).

You're right about path, but a page cannot set secure unless it's capable of reading secure cookies by virtue of being delivered over secure transport.

@ericlaw1979
Copy link

a page cannot set secure unless it's capable of reading secure cookies by virtue of
being delivered over secure transport.

Ah, I'd forgotten that this was added in leave-secure-cookies-alone and further confused myself by running my test case in a Edge Spartan window (IE and old Edge never implemented that update).

@mikewest
Copy link
Member

mikewest commented May 2, 2019

Assuming @mozmark doesn't object in the near future, I'd suggest running with the proposal to remove the "safe" method check for storage. I'll put up a PR at some point soonish.

mikewest added a commit that referenced this issue May 2, 2019
…gation

Based on the discussion in #594, this patch aligns
the specification with Firefox's status quo behavior: allowing
`SameSite=*` cookies to be set from all top-level navigations, not just
navigations for which `SameSite=*` cookies would also be sent.
@mikewest
Copy link
Member

mikewest commented May 2, 2019

PR in #800.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

6 participants