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

Impact on same-origin policy #100

Open
annevk opened this issue Nov 3, 2020 · 14 comments · May be fixed by whatwg/fetch#1434
Open

Impact on same-origin policy #100

annevk opened this issue Nov 3, 2020 · 14 comments · May be fixed by whatwg/fetch#1434
Assignees
Labels
bug Something isn't working

Comments

@annevk
Copy link

annevk commented Nov 3, 2020

These new headers increase the size of an HTTP request and coupled with attacker-controlled headers or header values could be used to carry out certain cookie-size sniffing attacks.

Privacy measures in browsers might invalidate some of these attacks, but the privacy boundary is typically not the origin, at least in today's implementations.

@yoavweiss
Copy link
Collaborator

Looking at table 2 in the paper quoted in that Bugzilla issue, it seems like the underlying concern is that we need to make sure that overall request header sizes should be below 8KB in order for them to be CORS safe.
The specific vector described to distinguish 200 from a 400 was since fixed, but maybe there are others.

This issue seems to be impacting all new request headers, not just UA-CH, or even CH in general. e.g. Sec-Fetch-*
/cc @mikewest

We cannot place a CORS cap on the overall request headers size, as this would create its own side channel. At the same time, we can (and looks like we have) add a cap on the overall size of safe-listed headers, as 1024.

So, if I'm reading this correctly, UA-CH and other safelisted headers don't impact same-origin policy beyond running a risk of triggering a preflight, if the list gets too long. Is that correct?

@annevk
Copy link
Author

annevk commented Nov 30, 2020

Well, it's unclear, because it's not defined how UA-CH et al work. (And there are no preflights for navigations, note.)

@yoavweiss
Copy link
Collaborator

We cannot place a CORS cap on the overall request headers size, as this would create its own side channel. At the same time, we can (and looks like we have) add a cap on the overall size of safe-listed headers, as 1024.

@annevk - reviving this part. Would it make sense to add a cap on the size of safelisted headers added per request? such that if someone is adding all the Client Hints, they run a risk of triggering preflights, but that won't be an issue in the typical case?

@annevk
Copy link
Author

annevk commented May 17, 2021

I think coupled with Sec-* that would indeed address the concern. It's a bit unclear to me how that would work in practice. And there's a problem with navigations as those don't do preflights at the moment.

@yoavweiss
Copy link
Collaborator

Should we move this to client-hints-infra? This doesn't seem specific to UA-CH

@annevk
Copy link
Author

annevk commented Mar 16, 2022

Sounds good. I think I raised this here because at the time this was about to ship in Chrome and might have been the only hints with these properties, but not sure.

@yoavweiss yoavweiss transferred this issue from WICG/ua-client-hints Mar 16, 2022
@arichiv
Copy link
Collaborator

arichiv commented Mar 16, 2022

Just to catch up on discussion, it sounds like the solution is a limit on the size of the Sec-CH-* headers in the HTTP request?

@annevk
Copy link
Author

annevk commented Mar 17, 2022

In particular, making them share a limit with CORS and Referer.

And given the multitude of headers we probably also need to take into account that currently the limit is for values only, but we probably want to account for the size of the header names as well.

(This concern might not be applicable when the server is actively requesting Client Hints. I saw a proposal of sorts for that come by at some point, but I don't know what the status is and I haven't tried to think it all through.)

@arichiv arichiv self-assigned this Mar 23, 2022
@arichiv arichiv added enhancement New feature or request and removed discussion labels Mar 23, 2022
@arichiv
Copy link
Collaborator

arichiv commented Apr 5, 2022

Notes from meeting with @annevk:

  • Issue is the overall size of headers sent via CORS
  • This is particularly an issue for Client Hints due to the large existing set and potential future growth
  • Current limits don't account for header name, which can be a significant part of the size if all CH headers are requested
  • The server server limit is 8k, and the referrer limit is 4k, but the length of cookies can be found by playing with the remaining space
  • CH devs should focus on a patch to Fetch, and the best way might be a cookie (and sensitive header) specific budget that other headers cannot impinge on

@annevk
Copy link
Author

annevk commented Apr 5, 2022

The concern also applies to "no-cors", you get some control over headers there as well. It essentially matters for all cross-origin requests, including "navigate", which I suppose is a novel angle that only CH headers touch.

@arichiv arichiv added bug Something isn't working and removed enhancement New feature or request labels Apr 12, 2022
@arichiv arichiv linked a pull request May 3, 2022 that will close this issue
3 tasks
@arichiv
Copy link
Collaborator

arichiv commented May 3, 2022

I could use some feedback on whatwg/fetch#1434 @annevk before adding more detail. I know the concept of 'cross-origin request' or 'serialized header size' aren't properly defined.

@annevk
Copy link
Author

annevk commented May 17, 2022

Hey @arichiv, I appreciate that you're tackling this, but unfortunately I don't have the bandwidth at the moment to take this on. I believe Google has a spec mentoring program that might be suitable here. Once things are more concrete and Chrome Security has reviewed the approach, I'd be happy to take a more detailed look.

@arichiv
Copy link
Collaborator

arichiv commented May 18, 2022

@yoavweiss would you be willing to review? I think you volunteered to help as a spec mentor :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants