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

Specify exception to redirect-tainting: Upgrade-Insecure-Requests (UIR) and HSTS scheme upgrades #1551

Open
Rob--W opened this issue Nov 26, 2022 · 5 comments

Comments

@Rob--W
Copy link

Rob--W commented Nov 26, 2022

Originally filed at w3c/webappsec-upgrade-insecure-requests#32

UIR (and HSTS) are commonly implemented as an internal redirect from http to https (at least in Firefox and Chrome, AFAIK). This has implications for CORS, and the last thread I found on this was unresolved (#324).

A central concept in CORS is "Origin", visible as a request header, defined at https://fetch.spec.whatwg.org/#serializing-a-request-origin as:

Serializing a request origin, given a request request, is to run these steps:

  1. If request has a redirect-tainted origin, then return "null".
  2. Return request’s origin, serialized.

The request is considered to have a "redirect-tainted origin" when any of the origins in the redirect chain are same-origin.

When UIR is implemented as a redirect, a cross-origin fetch to http://example.com is immediately followed by a (cross-origin) redirect to https://example.com. According to the above spec, the origin would therefore be null.
An argument can however be made for not tainting the Origin when a request is upgraded through UIR/HSTS.

Test case: https://jsfiddle.net/0kq28zgj/1/

In practice, current versions of Chrome (107) and Safari (15.6) send the original Origin.
Firefox (108) currently sends Origin: null, but internally uses the original Origin (and breaks CORS when the server mirrors Origin in Access-Control-Allow-Origin). I believe that this is a bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1800990), so once fixed, it looks like all major browsers will treat UIR/HSTS/extension-initiated scheme upgrades as a non-tainting redirect.

In short: it looks the the major browsers are (or will be) consistently not converting the Origin to null after a scheme upgrade, and it would be useful to update the specification to codify this.

@evilpie
Copy link
Contributor

evilpie commented Nov 28, 2022

I need to check again, but I was under the impression that UIR and HSTS aren't redirects in the fetch spec and thus should not trigger redirect tainting. (So this would just be an implementation bug in Firefox)

@Rob--W
Copy link
Author

Rob--W commented Nov 28, 2022

I need to check again, but I was under the impression that UIR and HSTS aren't redirects in the fetch spec and thus should not trigger redirect tainting. (So this would just be an implementation bug in Firefox)

I haven't tried HSTS, but UIR - see test case at https://bugzilla.mozilla.org/show_bug.cgi?id=1800990

But regardless of implementation, the current spec forces a tainted state when the origin changes, even if it is just a scheme change.

@annevk
Copy link
Member

annevk commented Nov 28, 2022

@Rob--W are you sure? We do report (though not enforce) CSP violations before, but step 5 of main fetch currently does UIR-related upgrades of the input and that's way ahead of any tainting that might occur.

HSTS happens after mixed content blocking, but I think that's still correct, although Mixed Content Level 2 might have changed a few things there?

@Rob--W
Copy link
Author

Rob--W commented Nov 28, 2022

That covers the initial request (including the minimal test case from my report in Firefox).

In case of repeated redirects, the same-origin check (step 9 and step 13) and redirect chain construction (step 18) happen before the "main fetch" step:
https://fetch.spec.whatwg.org/#http-redirect-fetch

Consequently, despite the requests being always over https, some parts of the specified algorithm treat the request as http.

@annevk
Copy link
Member

annevk commented Nov 28, 2022

Thanks, that's bad.

I guess what needs to happen is that a bunch of the policy decisions are moved from HTTP-redirect fetch to main fetch. This would actually work I think for all origin-related checks.

What cannot move is manipulation due to the redirect status. That doesn't involve origin so it should work, but also doesn't seem great.


Then separately there's the desire to do some of this as internal redirects (#1426) mainly because we might have to hit the network before we know whether we can do an upgrade. How exactly that should interleave with all the policies is a bit unclear to me at this point though.

cc @davidben

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

No branches or pull requests

3 participants