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

Prevent cross-origin sensitive header probing #1434

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

arichiv
Copy link

@arichiv arichiv commented May 3, 2022

The goal of this algorithm is to prevent cross-origin requests from probing the size of sensitive headers (Authorization or Cookie) by adding headers to cross-site requests until the total size of all HTTP request headers exceeds the server side limit. In order for this approach to succeed, servers should not set a HTTP request headers size limit below 8KB.

closes WICG/client-hints-infrastructure#100

  • At least two implementers are interested (and none opposed):
  • Tests are written and can be reviewed and commented upon at:
  • Implementation bugs are filed:
    • Chrome: …
    • Firefox: …
    • Safari: …
    • Deno (not for CORS changes): …

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

@yoavweiss yoavweiss changed the title Prevent cross-origin sensitive header probind Prevent cross-origin sensitive header probing May 3, 2022
Copy link
Collaborator

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be good to discuss the overall approach, before diving into the details (which need some work):

  • What headers do we want to limit? All Sec- headers? More?
  • What size do we want to limit them to? IIUC from the issue comments we want them to share the limit with Referer. Is that correct?
  • What request modes and credentials modes are impacted?
  • What happens if the limits are passed? Do we want to preflight? kill the request? Drop some headers?

fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
@arichiv
Copy link
Author

arichiv commented May 31, 2022

@yoavweiss ready for another look

fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
Copy link
Collaborator

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-authoritative LGTM % comments

fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
@arichiv
Copy link
Author

arichiv commented Jun 29, 2022

@annevk for further review

@arichiv
Copy link
Author

arichiv commented Aug 17, 2022

@annevk Have any time to take a look?

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this is only meant to impact CORS, but wouldn't some of these client hints be added to navigations and such? Some of these headers are added quite late in the game too (e.g., Cookie and Authorization) and I'm not sure how that would work given the envisioned setup.

fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Sep 26, 2022

Also somehow the Build bot hasn't run for your latest commit. Perhaps rebasing will help with that?

@arichiv
Copy link
Author

arichiv commented Oct 5, 2022

Also somehow the Build bot hasn't run for your latest commit. Perhaps rebasing will help with that?

"First-time contributors need a maintainer to approve running workflows."

@arichiv
Copy link
Author

arichiv commented Oct 5, 2022

It seems this is only meant to impact CORS, but wouldn't some of these client hints be added to navigations and such? Some of these headers are added quite late in the game too (e.g., Cookie and Authorization) and I'm not sure how that would work given the envisioned setup.

Ah, does this require integration in the HTTP spec as well?

@davidben
Copy link
Collaborator

davidben commented Oct 5, 2022

Is the proposal to make CORS depend on Cookie and Authorization header? Did you have an implementation in mind? I also don't see how that could work in, say, Chromium. Authorization headers are especially fun because HTTP auth can cause a single high-level request to actually contact the server multiple times. (Some auth methods may require several requests.) And then the HTTP stack might itself add others headers like If-None-Match for caching, etc. Caching, for that matter, can also require multiple requests in some cases.

I suspect limits for headers applied deep in HTTP would need to be applied separately, and you wouldn't be able to use preflights as an escape hatch. I think they'd have to be hard limits. And then the value servers need to set would be the sum of every layer's limits.

@annevk
Copy link
Member

annevk commented Oct 7, 2022

Ah, does this require integration in the HTTP spec as well?

No, Fetch defines most of the networking architecture of a browser, including navigations ("navigate") and "no-cors" requests.

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

Successfully merging this pull request may close these issues.

Impact on same-origin policy
4 participants