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

"Site for cookies" handling for service workers #1288

Closed
annevk opened this issue Oct 13, 2020 · 11 comments · Fixed by #2217
Closed

"Site for cookies" handling for service workers #1288

annevk opened this issue Oct 13, 2020 · 11 comments · Fixed by #2217
Assignees
Labels

Comments

@annevk
Copy link

annevk commented Oct 13, 2020

I'm a bit surprised by https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-06#section-5.2.2.2.

As I understand it the SameSite definition cookies use has a recursive parent check to prevent A2 in A1->B->A2 from getting cookies that A1 does get, as B might try to trick it. However, if A2 is entirely generated by a service worker, the above rules would mean it would still get spoofed.

cc @krgovind @jkarlin

@jkarlin
Copy link

jkarlin commented Oct 13, 2020

@mikewest

@davidben
Copy link
Contributor

The recursiveness check is useful for CSRF defenses, but I agree Service Workers (as usual...) kinda mess it up. @jkarlin, this reminds me of the conversations around partitioning schemes (double- vs triple-keying) for storage partitioning.

If we could align the partitioning rules with the recursive check, that would be great for XS-Search attacks and the HTTP cache, and would probably solve the Service Worker problem automatically. But the recursive check doesn't naturally lend itself to a partition key. The other direction would be to add some extra things to fetch requests so the document-side decision is preserved through SW, which also sounds messy.

@jkarlin
Copy link

jkarlin commented Oct 13, 2020

Short of including the full chain in the partitioning key, I think we'd need to do something like consider the partition to be opaque if the current frame site matches the top frame site but there is a separate site inbetween. I don't know how often this happens in practice, though it's something we could measure.

@davidben
Copy link
Contributor

It occurs to me that simply using SameSite-consistent partitioning rules probably wouldn't work anyway. For top-level requests, there are variations in SameSite behavior:

  • SameSite=Lax blocks the cookie on cross-site top-level POST but allows it on cross-site top-level GET
  • SameSite=Strict blocks the cookie on cross-site top-level navigations of both methods.
  • Funny LaxPlusPOST behavior of the default thing.

I dunno if SW/Fetch currently captures that behavior. If not, we should fix that too. If so, we can maybe mirror the solution for that. And then the question of whether we can harden XS-Search defenses with an ancestor-aware partitioning scheme can maybe be separate?

@annevk
Copy link
Author

annevk commented Oct 14, 2020

Currently cookie behavior is largely defined in https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis though I would prefer it if more of the logic moved into Fetch.

https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-06#section-5.5 seems to capture what you state though it talks about safe methods instead. However, in this specific case that's probably equivalent as navigation can only ever use GET or POST.

However, I'm not entirely persuaded that all this complexity is worth it for documents that allow themselves to be framed across the origin boundary as these are not the only attack vectors.

(Also, service workers are a very different case from the HTTP cache, they're in the same boat as localStorage.)

@davidben
Copy link
Contributor

davidben commented Oct 14, 2020

Yeah, safe method is the correct criteria. I used GET vs. POST as examples just to simplify things. My point is that SameSite already has different behaviors depending on value, which means we won't find a single Service Worker partitioning scheme that fits them all. That suggests instead we need to make the behaviors pass through SW correctly.

As for why those cases, cross-site top-level POSTs are the classic CSRF attack vector, so any CSRF story had better cover those. On the ancestor chaining case, whether the document allows itself to be framed cross-origin isn't the point. By the time the document has sent X-Frame-Options/frame-ancestors, the cookied request has already been sent and the site needs to worry about CSRF. Any CSRF story needs to kick in at the request. (Imagine if origin A embeds origin B, which tries to attack A by embedding some URL on A.)

Either way, SameSite=Lax and SameSite=Strict have existing semantics and are shipped across browsers already. Loosening their semantics now could introduce security problems in sites that were designed against those semantics.

@annevk
Copy link
Author

annevk commented Oct 15, 2020

Those sites would also introduce security problems by adopting service workers. You make a good point regarding CSRF, sites would want w3c/webappsec-fetch-metadata#56.

(As for "safe method", I generally don't think that's a good primitive as its meaning changes over time. This is somewhat problematic for APIs that allow arbitrary methods. But again, it's not a problem here as navigation doesn't do that.)

@MattMenke2
Copy link

We've started thinking about using the full chain in the partitioning key. This would allow behavior be aligned with SameSite cookies, and give a more consistent story when partitioning cookies. This also solves the X iframes Y iframes X ServiceWorker story.

However, there are still some issues:

  1. Top-level ServiceWorkers still have access to SameSite=Strict cookies on cross-site navigations.
  2. If X iframes Y and Y redirects to X, that redirect has SameSite=Strict cookies and X's first party ServiceWorker, cache, and other storage (other storage is less of a concern here, I believe).

SameSite=Strict could be modified to be stricter about cross-site redirects (which would doubtless break a bunch of sites), and the cache can be mitigated a bit by using a separate key for iframe requests, but that still leaves ServiceWorker.

@annevk
Copy link
Author

annevk commented Nov 4, 2020

It seems to me that if consistency is warranted, it's between "other" storage and cookies. Those types of storage are directly observable by developers and they might even have some relation between them. And by not fully considering service workers when standardizing SameSite, SameSite is not that useful once you use service workers. Which makes me question holding onto SameSite as the correct model, as service workers are not deprecated and are a technology we want to be able to recommend.

@miketaylr
Copy link
Collaborator

@mikewest to send in a PR.

@annevk
Copy link
Author

annevk commented Oct 1, 2021

Note that we have an ad hoc service worker meeting scheduled for next week where we might be able to make some progress on this: w3c/ServiceWorker#1604.

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

Successfully merging a pull request may close this issue.

8 participants