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 adding note that CORS preflight check excludes headers added by the browser or extensions #1005

Open
wanderview opened this issue Mar 5, 2020 · 19 comments

Comments

@wanderview
Copy link
Member

The CORS protocol currently requires sending a preflight request if there is an unsafe header on the request. Step 4.1 here:

https://fetch.spec.whatwg.org/#http-fetch

However, in practice browsers only examine headers set by content for this check. They exclude headers set by the browser itself or extensions.

For example, in firefox the CORS implementation looks at a header list:

https://searchfox.org/mozilla-central/rev/13b081a62d3f3e3e3120f95564529257b0bf451c/netwerk/protocol/http/nsCORSListenerProxy.cpp#983

Set explicitly in the child process close to the content script:

https://searchfox.org/mozilla-central/rev/13b081a62d3f3e3e3120f95564529257b0bf451c/dom/fetch/FetchDriver.cpp#673
https://searchfox.org/mozilla-central/rev/13b081a62d3f3e3e3120f95564529257b0bf451c/dom/xhr/XMLHttpRequestMainThread.cpp#2657

Until recently chromium also worked the same way by virtue of performing its entire CORS operation in the renderer process close to content.

Chromium recently implemented an out-of-renderer CORS (OOR-CORS) feature that moves the CORS operation into the network stack. This caused the preflight logic to begin examining headers set by browser internals and extensions. This has caused a great deal of web compat problems for the OOR-CORS rollout. There has been hesitance to match the old behavior because the spec does not allow it.

I am unsure what webkit does here.

Given the firefox behavior and legacy chromium behavior would it be reasonable to add a non-normative note indicating headers set by browser internals and extensions are excluded from the CORS preflight check?

@annevk
Copy link
Member

annevk commented Mar 6, 2020

I think generally we try not to say anything about extensions as standards cannot govern their scope. It's up to each browser to decide what kind of exemptions they grant extensions from the standard model.

@wanderview
Copy link
Member Author

Not even in a non-normative note? Maybe just saying that browsers can choose to allow extensions to add headers that bypass this check?

The reality currently is that browsers do make this exemption for both extensions and browser added headers, but there is no indication in the spec at all.

@annevk
Copy link
Member

annevk commented Mar 7, 2020

Extensions aren't really a concept that standards acknowledge at all. Maybe we could describe them in Infra as part of defining user agent and say that they can generally make the user agent do whatever.

@wanderview
Copy link
Member Author

To me, though, extensions are just one aspect here. Browsers can add other headers for different reasons, like enterprise policies, etc. What I really want to say is something like:

"Note: Browsers may internally add headers for various reasons such as enterprise policies, extensions, etc. Current implementations typically do not include these internally added headers when determining if a CORS preflight is required. The CORS preflight check typically only looks at headers added by content script."

@sleevi
Copy link

sleevi commented Mar 9, 2020

@wanderview Would it make sense framing that the restrictions apply to requests that can be controlled by Web content, whether directly and indirectly, and these restrictions do not necessarily apply to requests initiated or modified by a user agent outside of the control of the web content?

That is, rather than trying to capture what's not in scope, you instead anchor on capturing what is in scope, and leaving the rest in the nether and implementation-defined (which seems to be your goal)

@wanderview
Copy link
Member Author

Yes, that would work for me as well.

@youennf
Copy link
Collaborator

youennf commented Mar 9, 2020

I am unsure what webkit does here.

WebKit does CORS checks out-of-process and sends the list of headers set by the web application. Service workers do face a similar issue, since they are usually out of process.

The webRequest API allows modifying requests on the fly. It would be nice if it was easy for them to specify whether CORS/service worker should take these headers into account.

@domenic
Copy link
Member

domenic commented Mar 10, 2020

+1 for adding a note at least. I think it could avoid significant confusion, both on the implementation side (it's not clear currently from the spec that browsers are allowed to add additional CORS preflight-bypassing headers) and from the server operator side (it's not clear currently that your server could get hit by non-CORS-safelisted headers set by the browser).

@annevk
Copy link
Member

annevk commented Mar 10, 2020

To phrase it differently, I see extensions exhibiting non-standard behavior similar to a user compiling their own browser with non-standard behavior from scratch. In theory extensions can also modify the way scripts execute, HTML parses, URLs are displayed, etc. Where do you draw the line on what non-standard behavior to call out?

@domenic
Copy link
Member

domenic commented Mar 10, 2020

I think a good way to draw the line is what confuses implementers.

@wanderview
Copy link
Member Author

Also, I thought whatwg was focused on describing what browsers implement instead of theoretical purity. The reality is that browsers have implemented the distinction between content-set headers and browser-set headers when it comes to CORS preflights for a long time. We ran into a lot of compat pain when we tried to remove the distinction in order to conform strictly to the current spec text.

@annevk
Copy link
Member

annevk commented Mar 10, 2020

The scope is limited to things observable from web content with unmodified browsers. Otherwise there would be many more things to document (or more likely, linger around in issues), some even contradictory.

@domenic
Copy link
Member

domenic commented Mar 10, 2020

The browsers in question here are unmodified. You keep referring to extensions, but browsers append headers even when extensions are not involved.

@wanderview
Copy link
Member Author

And the impact is observable to content because requests fail to load.

@wanderview
Copy link
Member Author

As sleevi@ pointed out we can write this in a way that does not mention extensions. For example:

"Note: Browsers typically only apply the CORS preflight check against headers that can be directly or indirectly set from web content. Browsers may add other headers internally that are not subjected to this check."

@annevk
Copy link
Member

annevk commented Mar 10, 2020

Is this a duplicate of #530 then?

@wanderview
Copy link
Member Author

At first glance they don't seem the same to me. Perhaps underlying issues related to internal browser behavior, not the same question about where that intersects with the spec. I could be wrong, though.
Maybe @sleevi can chime in.

@annevk
Copy link
Member

annevk commented Mar 10, 2020

That's about requests somewhat outside the sphere of "web content" but still done by the browser. If this is different, some concrete examples might help.

@sleevi
Copy link

sleevi commented Mar 10, 2020

I agree with @wanderview that this seems orthogonal. If anything, the closest similarity I can think to is the discussion about "internal" redirects in #576.

In that case, similar to here, browser implementation details can lead to inconsistent web-observable behaviours (in that case, the distinction of "internal" redirects; in this case, the distinction of internally-added headers)

Regardless of the spec purity angle (that is, that we can only spec behaviours for things that conform to specced behaviours), there does seem to be a set of implementation decisions that can either lead to web-observable behaviours or browser-interop/compat issues. When last looking at #869 (and the related Chrome bug), one of the challenges was trying to decide whether or not locally configured policy should be able to alter the CORS-preflight. An argument for compat (largely with Google's own internal servers, which I find a poor reason) would suggest the policy can/should affect preflights, while alignment to the resolution in #869 would suggest that it should not. Aligning with #869 would at least ensure Firefox could be used at Google, while if Chrome went its own way, I am confident it would make Firefox less likely to be able to used to access internal sites, and thus less likely to be used to test external sites.

Ultimately, while browsers all ideally try to align on the same spec, and specs are signs, not cops, there's also benefit in having some description for the "common" extensibility points or internal implementation decisions UAs have to make, and how and whether these are web-observable or not. My pragmatic view is that it's worth documenting these cases, because the more we leave them up in the air without clear guidance, the more we'll have organizations and developers coding to specific implementation-extension points, and that's worse for browser agility and interoperability.

If Edge and Chromium and Firefox all support a common extension API, but that API behaves differently in different UAs, then organizations that adopt/mandate the extension may preclude certain UAs based on their behaviours. Having seen that happen in Google, leading to less testing in other UAs, I'd like to avoid that.

Without wanting to put words in @youennf's mouth, that also seems aligned with the response in #1005 (comment)

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

5 participants