CH processing, cross-origin redirects and service workers

[yoavweiss]

During an IRC discussion about #773, @annevk brought up a few points we need to consider when it comes to CH processing:

• If we want to make sure that CH headers are not sent on cross-origin request headers, including cross-origin redirects, we need to have origin checks relatively low in the stack and only add those headers there.
• However, that would mean that the headers will not be exposed to service workers, which would limit some of their use-cases.
• An alternative would be to add the headers up the stack and remove them further down for cross-origin redirects, but that would mean we'd also remove them for user-added CH headers. It'd also mean that CH is the first feature introducing header removal.

So, we need to decide on the trade-off between privacy, usability and functionality here:

• How awful would it be to send CH to cross-origin redirects?
• If the answer to the above is "really awful", we need to find a way to avoid doing that without harming SW use of CH as well as user-added CH headers. Personally, I think the SW use case is more important than the user-added headers case.

@igrigorik @arturjanc - thoughts?

[igrigorik]

First, for context, per current plan we will use Feature Policy to enable CH delegation. Under that model, 1P will have to explicitly delegate delivery of CH to 3P origins; CH hints will not be sent to 3P origins by default.

So, above question should be (re)scoped to: if a CH-whitelisted origin redirects to a non-whitelisted origin, what's the behavior we expect — correct?

If so, I think the answer should be that CH headers are stripped on such redirects because the destination origin is not on the whitelist. Any reasons for why this shouldn't be the case?

---

I'm not sure I follow why SW wouldn't see these headers? We definitely do want SW to see them, as they can and should be used to do resource selection + caching when doing CH content negotiation.

[annevk]

@igrigorik I think what's missing in your analysis is that 1P is the attacker. So it safelisting 3P is immaterial.

[yoavweiss]

If 1P is the attacker, they can already leak that (not-really-private) information willingly to any 3P by many different means.

[annevk]

It's not about leaking, it's about attacking 3P through those headers. There's a reason we did #736.

[yoavweiss]

OK, so we're talking about different threat models. Is the threat model user-generated headers? Would limiting the headers' values similarly to #736 help?

That seems doable without sacrificing any of the use-cases (and regardless of FP).

[annevk]

It helps, but better would be to not extend that safelist.

[yoavweiss]

Not extending the safelist for UA generated values would add a significant cost to introducing any new request headers. That makes little sense. (For both CH as well as other UA generated headers. e.g. upgrade-insecure-requests).

Preflights don't come for free, and we should carefully consider the (infinitesimal IMO) risk of sending newly-introduced UA generated headers (with value restrictions) to servers vs. the certainty of inflicting performance pain on all/many users as a result.

/cc @mikewest

[annevk]

If Origin Policy is successful preflights do in fact come for "free". But also, routinely breaking the same-origin policy is a dangerous game with lots of potential bad consequences.

[yoavweiss]

@annevk just to make sure, was your main concern in the IRC discussion mentioned in the OP about CH being CORS safelisted? Are you also concerned about info leakage?

[annevk]

I posted my thoughts here: https://freenode.logbot.info/whatwg/20180824#c1678383. TL;DR is that the extending-SOP-exceptions concern can probably be ignored for now, though we should add tests to ensure values are adequately validated and preflighted as appropriate.

(SW wouldn't see these headers if we added them at a late point, avoiding the need to have to remove them. OP doesn't mention it, but removing request headers is somewhat unprecedented at the moment, though #609 will change that as well.)

Another concern not mentioned here is that we need to deal with the case where these headers were already set by the developer. If a developer sets DPR a browser would end up appending to it with the current wording. Is that desired? I'd argue it's better to avoid that and only set the header if it's not already set. (We need tests for this too.)

For the concerns mentioned in OP, it sounds like setting the headers early and potentially removing them upon redirects, even if they were developer-set, is the away to go.

Hope this helps.

[arturjanc]

I agree with @annevk's comment in #800 (comment).

When it comes to the level of badness of sending CH on cross-origin redirects, I'd rather this didn't happen, but I expect it's not a huge risk. The threat model in this case is accidentally disclosing CH information to third parties, and the 1p -> 3p redirect pattern doesn't seem very common, so the leakage potential is low. Still, I expect this to be moot because -- as Ilya said -- Feature Policy should disable the feature when it sees a redirect to a non-safelisted origin (similarly to how CSP rejects resource loads from outside of its safelist upon redirects).

[yoavweiss]

I posted my thoughts here: https://freenode.logbot.info/whatwg/20180824#c1678383. TL;DR is that the extending-SOP-exceptions concern can probably be ignored for now, though we should add tests to ensure values are adequately validated and preflighted as appropriate.

Thanks. Restricting the values SGTM.

Another concern not mentioned here is that we need to deal with the case where these headers were already set by the developer. If a developer sets DPR a browser would end up appending to it with the current wording. Is that desired? I'd argue it's better to avoid that and only set the header if it's not already set. (We need tests for this too.)

That makes sense. I can add that to #773

For the concerns mentioned in OP, it sounds like setting the headers early and potentially removing them upon redirects, even if they were developer-set, is the away to go.

OK, so a tentative design which may work can look something like:

• Perform cross-origin/Feature-Policy checks at step 1.7 of fetch, before adding any hints
• Perform cross-origin/Feature-Policy checks again in http redirect fetch and remove any of the hint headers there, if they shouldn't be sent.

That would indeed have the side effect of removing user-added CH headers from cross-origin redirects (unless they were opted-in using Feature-Policy). I think that's an edge case which we can live with.

@annevk - does that make sense?