-
Notifications
You must be signed in to change notification settings - Fork 28
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
Introduce request
infrastructure for storage access
#214
base: main
Are you sure you want to change the base?
Conversation
1. If |request|'s [=request/client=] is null, return "<code>[=storage access eligibility/unset=]</code>". | ||
1. If |request|'s [=request/client=]'s [=environment/ancestry=] is not "<code>cross-site</code>", return "<code>[=storage access eligibility/unset=]</code>" | ||
1. If |request|'s [=request/client=]'s [=environment/has storage access=] is false, return "<code>[=storage access eligibility/ineligible=]</code>". | ||
1. If |request|'s [=url/origin=] is not [=/same site=] with |request|'s [=request/url=]'s [=url/origin=], return "<code>[=storage access eligibility/ineligible=]</code>". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I'm not sure I understand what purpose this step serves now, would you mind elaborating it for my benefit? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This checks that the request's initiator is same-site with the URL that we're considering sending cookies to. If they're cross-site, we shouldn't include cookies, because doing so would allow an attacker to manipulate an iframe into CSRF-ing itself, IIUC.
Note that this is one of the two things that change to be 'same-origin' in 6b6ccef, since the rest of the web platform uses origin as the security boundary, rather than site.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, okay, and SAH would override eligibility if supplied by the request then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, see step 8 of https://privacycg.github.io/storage-access-headers/#http-storage-access-retry-fetch.
I tweaked the field name (eligible to storage-access
-> storage access eligibility
) when I modified its type (bool -> tri-state enum) in this PR, but the SAH spec can be updated to use the new name once this PR lands.
This splits out the underlying infrastructure changes introduced in #213 so that #213 can just introduce the behavior change, and this PR can add the infrastructure needed to formally define the current behavior (in Chrome and Firefox; Safari TBD).
I also incorporated @annevk's request that
request/eligible for storage-access
be a tri-state enum, rather than a boolean.This PR relies on the
environment/ancestry
enum, similar to other cookie layering work (e.g.).Preview | Diff