Setting a cookie doesn't respect schemeful SameSite
Categories
(Core :: Networking: Cookies, defect, P3)
Tracking
()
People
(Reporter: tschuster, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-triaged])
When setting a cookie we seem to use CookieService::CanSetCookie to verify if we can set a cookie. That also includes this check:
if ((effectiveSameSite != nsICookie::SAMESITE_NONE) &&
aIsForeignAndNotAddon)
That doesn't seem to account for schemeful-ness. We probably also need to account for the difference between Lax and Strict with top site navigation.
I am also not sure why CookieService::SetCookieStringFromDocument calls ShouldIncludeCrossSiteCookieForDocument, when it already uses CookieCommons::CreateCookieFromDocument, which calls the previously mentioned CanSetCookie.
|
Reporter | |
Updated•2 years ago
|
|
|
Reporter | |
Comment 1•2 years ago
|
||
So I am actually not sure if we need to do anything at all here. Do we really want to introduce a difference between Lax and Strict? I.e. not allow setting SameSite=Strict cookies on a non-toplevel navigation?
|
|
Reporter | |
Updated•1 year ago
|
|
||
Comment 2•8 months ago
|
||
(In reply to Tom Schuster (MoCo) from comment #1)
So I am actually not sure if we need to do anything at all here. Do we really want to introduce a difference between Lax and Strict? I.e. not allow setting SameSite=Strict cookies on a non-toplevel navigation?
According to the spec (step 18 of §5.6 Storage model) a cross-site navigation can only create a SameSite lax or strict cookie when it's top-level. Any kind of top-level navigation can create any kind of SameSite cookie. Lax and Strict are treated the same when setting a cookie; they only differ when the browser sends cookies on a top-level navigation.
Now that bug 1844827 is landed is there anything left to do in this bug?
|
|
Reporter | |
Comment 3•8 months ago
|
||
It's good to know that we have consistent behavior for Strict and Lax. However in comment 0 I seem to be talking about some other stuff related to schemeful, which might actually be something else. I am not quite sure anymore what this was about. Maybe Ed can give this quick look?
It appears that the essence of this bug is to ensure that cookie setting respects schemeful same-site.
The check for aIsForeignAndNotAddon in CanSetCookie derives it's value from our ThirdPartyUtil::IsThirdPartyChannel, which indeed does NOT do a scheme check of the between the channel and URL
If/When we proceed with schemeful samesite cookies, we probably want something like this. Which is used when SENDING a cookie and relaying necessary cookies to the necessary content processes, but not currently for SETTING (as far as I can tell). I would need to look at the callers of ThirdPartyUtil to decide where this check should go exactly, ideally it would go inside IsThirdPartyChannel, but worst-case, the CanSetCookie.
BTW the current spec makes no guarantees about isolation by scheme, so I think this is only relevant to schemeful samesite.
I don't know much about the original breakage of laxByDefault or schemeful-samesite, but this lack of scheme check appear to date back to the original patches and seems like it could be a significant point failure.
Description
•