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

Consider a Sec-CH- prefix for client hint headers #716

Closed
mikewest opened this issue Nov 6, 2018 · 23 comments
Closed

Consider a Sec-CH- prefix for client hint headers #716

mikewest opened this issue Nov 6, 2018 · 23 comments

Comments

@mikewest
Copy link
Member

mikewest commented Nov 6, 2018

In w3ctag/design-reviews#320, @annevk (re-)raised the question of Client Hints' integration with CORS. One suggestion in that thread is to prefix headers with Sec-CH-, which on the one hand makes them trivial to add en masse to the CORS-safelisted list, and on the other prevents JavaScript from setting their values (which in turn limits the risk associated with adding them to the CORS-safelisted list in the first place).

I have two concrete proposals for hints (UA-* and Lang) that adopt this pattern. Perhaps it's one that could be baked more deeply into the Client Hints infrastructure?

/cc @igrigorik @yoavweiss @arturjanc

@reschke
Copy link
Contributor

reschke commented Nov 6, 2018

Please no additional overloading of names.

@yoavweiss
Copy link
Contributor

Please no additional overloading of names.

OK. Can you expand on that?

@reschke
Copy link
Contributor

reschke commented Nov 6, 2018

Names are names. I realize that there's a precedent with "Sec-". But it really doesn't scale.

If we're serious about this, it needs to be done with a plan that explains how this would work with the next keyword being added. In particular, if the "next" extension is completely orthogonal to this one.

@mikewest
Copy link
Member Author

mikewest commented Nov 6, 2018

Sec- exists, and has meaning (at least in browsers). Are you objecting to that in itself? Or are you objecting to adding an additional CH-?

@reschke
Copy link
Contributor

reschke commented Nov 6, 2018

I object to this unless we have a documented and agreed-upon plan how this scheme would work with future keywords.

@mikewest
Copy link
Member Author

mikewest commented Nov 6, 2018

Sec-CH-Next-. :)

@reschke
Copy link
Contributor

reschke commented Nov 6, 2018

What if "Next" doesn't automatically always have the properties of "Sec"?

@yoavweiss
Copy link
Contributor

@reschke apologies, but at least to me, it's still not clear what you're objecting to.
Is it Sec- prefixes in general? Adding CH- prefixes to the Sec- prefix? If it's the latter, would a stand-alone CH- prefix (like we used to have) work better?

@reschke
Copy link
Contributor

reschke commented Nov 6, 2018

I'm objecting to the further abuse of overloading names with semantics without having a plan how this would work with future extensions.

We dropped "CH-" because we did not need it, right?

@yoavweiss
Copy link
Contributor

The reason we realize now that it is in fact needed is that we want client-hints to get a free-pass when it's comes to the CORS safelist. That is, we don't want to add every new header to the list.
Having a prefix will enable us to give all these headers the same treatment, but will also enable us to have a clearer mind when doing so, because the prefix reduces the probability that some existing server is already using those request header values in their logic in an unpredicted way.

@mikewest
Copy link
Member Author

mikewest commented Nov 6, 2018

we want client-hints to get a free-pass when it's comes to the CORS safelist

FWIW, I don't actually care about this. It's fine with me if Fetch just lists all the headers, and y'all have to send PRs to Fetch to add them. :)

because the prefix reduces the probability that some existing server is already using those request header values in their logic in an unpredicted way.

I do care about this. And I especially care that we significantly reduce the risk of the headers containing interesting payloads if we block JavaScript access by prefixing them with Sec-. The CH- bit is just defense in depth/obscurity from my perspective.

@mnot
Copy link
Member

mnot commented Nov 11, 2018

Right. I can understand the Sec- prefix because that's defined behaviour (but I note @mikewest's comment with interest; I've never really liked that). CH- is just sugar here AIUI.

You could document that Sec- is required and CH- is just convention that doesn't actually change recipient behaviour -- just as Allow- Accept- Content- etc. are.

@arturjanc
Copy link

There seems to be concrete value in adding the Sec- prefix for client hints, primarily for the reason Yoav mentioned: it reduces the compatibility concerns associated with sending a new request header. Otherwise, adding a hint header of Foo would break applications that currently set a custom header with the same name in a way that's difficult to predict in advance or detect.

When it comes to CH-, a common prefix that identifies hints also appeals to me. It would make it slightly easier to see which headers carry hints, helping developers discover their presence, but also allowing middleware to reliably strip them (e.g. in privacy extensions which aim to reduce the amount of information sent by the browser).

@mnot
Copy link
Member

mnot commented Mar 25, 2019

Having thought about this a bit more - I'm not very happy with forcing Client Hints to be Sec-, because making them unavailable from JS limits the cases they can be used; while the current set of hints are (mostly) automatically set by browsers, it's not hard to imagine future hints where the client-side application might have something to say.

I could see an argument that each hint should consider whether it needs to be prefixed by Sec-, but the criteria needs to be what actual security properties that hint has, not "we can't be bothered sending PRs to Fetch."

BTW, this is why registries were invented...

@yoavweiss
Copy link
Contributor

I could see an argument that each hint should consider whether it needs to be prefixed by Sec-, but the criteria needs to be what actual security properties that hint has, not "we can't be bothered sending PRs to Fetch."

I may not expressed myself properly before, but the reasoning for Sec- is not simply "we can't be bothered to send Fetch PRs". The way I see it, the reasoning is:

  • We want to have confidence that those headers do not conflict with current server logic before declaring them CORS-safe. Adding Sec- prefix reduces the probability of such collision. The alternative is to add somewhat complex logic to the CORS safety logic, making sure that values that users are setting are "valid" ones.
  • Making sure these headers can only be set by the User Agent itself will add further confidence to Servers when processing those headers, as attackers won't be able to set them from JS with arbitrary values.
  • Preventing SW from adding those headers will also simplify the processing somewhat (although that part is still being discussed

Does that make our motivation for this clearer?

@mnot
Copy link
Member

mnot commented Mar 26, 2019

Hi Yoav,

I think I understood you pretty well. What I'm saying is that whether something is CORS-safe is separable from the original purpose of Sec-; you're conflating them (for what looks like convenience).

Some headers might need confidence that they can't be set from JS, while I strongly suspect some may not.

@annevk
Copy link

annevk commented Mar 26, 2019

It's not quite convenience. Sec- headers are an escape for clients to not have to violate the same-origin policy as arguably sending new request headers cross-origin violates the same-origin policy. This also helps servers in knowing what to expect and where the boundaries are. If clients create new headers that are same-origin policy exceptions and do not use Sec- this leaves servers on a slow update cycle exposed to some extent.

@arturjanc
Copy link

As a specific example of what @annevk is talking about, let's say I'm a server that puts a certain degree of trust in requests with a Width header because they can currently only be set same-origin (or via preflighted CORS). Once clients start sending Width hints anyone will be able to make a request on behalf of another user with the header set, potentially subverting my security logic.

We know that servers frequently use the presence of custom headers for security purposes (e.g. X-CSRF-Protection: 1 is fairly common). We can hope that hint names would not clash with such sensitive headers, but especially as the list of hints grows, it will be more difficult to guarantee this, and we will quietly make some applications insecure.

@reschke reschke changed the title Consider a Sec-CH- prefix for client hint headers. Consider a Sec-CH- prefix for client hint headers Jul 10, 2019
@yoavweiss
Copy link
Contributor

@reschke - can you elaborate on "I object to this unless we have a documented and agreed-upon plan how this scheme would work with future keywords."?

What would such agreed plan look like?

Let's discuss this more next week at the HTTPWG meeting! (or before, if we can)

@yoavweiss
Copy link
Contributor

@reschke - friendly ping :)

@reschke
Copy link
Contributor

reschke commented Jul 22, 2019

I looked at the comments in this issue, and it seems to me everything was already mentioned.

Overloading the name with semantics doesn't scale, unless there's a agreed-upon way how new prefixes would be added and combined.

Right now, we seem to conflate:

  • making things ok in CORS (no preflight)
  • making things always UA controlled

and also

  • adding some arbitrary prefix reduces the risk of stepping on somebody else's private use field name

I need we believe to discuss these issues separetely.

@yoavweiss
Copy link
Contributor

This was discussed at the HTTPWG meeting today. A few highlights:

  • If the "CH-" part does not impact processing, it should not be a prefix. Maybe a convention that implementing features could follow.
    • One part where a "CH-" convention may be helpful is in signaling CDNS that they may not want to log that information, as it may be privacy sensitive as a fingerprinting vector.
    • At the same time, CDNs are likely to use CH values to manipulate content, in which case they are likely to log it as something that impacts their processing.
  • Maybe we should consider making the Sec- prefix decision on a feature-by-feature basis. However, that may complicate the Fetch processing model and it's not clear it is required.

@yoavweiss
Copy link
Contributor

Closed by #776

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

No branches or pull requests

6 participants