Make Origin header on cross-origin redirects consistent between CORS and non-CORS |
||||||||||
Issue descriptionThis is split off from issue #760487 , most context in comment #8 there. The Origin header is used as both a part of CORS and as a CSRF protection mechanism. The code is currently awkwardly split where the former is in Blink and the latter is partially in //net (can we move it all to Blink?). In issue #465517 , the latter use case was made to null out the origin on cross-origin redirects. CORS has similar logic, but it's slightly different: If A fetches A which redirects to B, CORS will not clear the origin due the "CORS flag" in Fetch, while the non-CORS //net code will. The origin is still unambiguously A at this point and this behavior was confusing in issue #760487 . Instead, we could do: if newURL.origin != oldURL.origin && request.origin != oldURL.origin { request.origin = null } (Right now we don't have the second check.) This should allow the same behavior between CORS and non-CORS. Still implemented twice (I'll let Blink folks comment on how we might fold it together), but at least the behavior will be unified. See Fetch issue/PR: https://github.com/whatwg/fetch/issues/593 https://github.com/whatwg/fetch/pull/594 mkwst: does this seem reasonable from the CSRF side of things?
,
Sep 14 2017
Sorry for the late response. > The code is currently awkwardly split where the former is in Blink and the latter is partially in //net (can we move it all to Blink?). We're moving CORS code out of blink (so called CORS service). I'm wondering if customers other than blink needs this (i.e. "the latter") implementation. If the answer is no, we can move the implementation to the CORS service.
,
Sep 14 2017
Related bug: - bug 665766 (Credentials mode change propagation) - bug 736308 (Out of Blink CORS)
,
Sep 14 2017
I'm going to strip Internals>Network from this bug to get it off the triager's radar screen since it doesn't seem like a general networking issue and the Blink>SecurityFeature>CORS seems like it categorized the bug pretty completely. Feel free to re-add if you disagree (setting it to Available would also get it off the triager's radar screen).
,
Sep 28 2017
Tentatively assigning to tyoshino@ based on relationship with Out of Blink CORS.
,
Oct 3 2017
mkwst: Friendly ping on initial question. This affects a CSRF protection feature, so your input would be appreciated.
,
Oct 4 2017
,
Oct 4 2017
For the initial question, I think your proposal is pretty reasonable. It preserves some CSRF mitigation for B, and I can't think of any negative side-effects. Thanks for the ping!
,
Oct 4 2017
Thanks! tyoshino: in advance of Out of Blink CORS work, do you think it's worth patching up the behavior in //net now? Just to get things consistent and shake out any problems earlier.
,
Oct 4 2017
(Also +mmenke FYI since Internals>Network was removed and we were talking about this logic recently.)
,
Nov 8 2017
We have this test/staging environment whose XHR are blocked in the current version of Chrome 61 but not in Canary or other browsers. They are not blocked in Canary because Canary sends the Origin header. Should the server check for the referrer Header in case the Origin Header is not present? Steps to reproduce 1. Open mbeta.housing.com/in/buy/mumbai/powai 2. Click on the first property. If you are stuck with skeleton stubs then CORS errors have occurred. This may be a bug from my side, please let me know.
,
Nov 10 2017
,
Feb 18 2018
,
Feb 26 2018
Reassigning to toyoshim@.
,
Feb 26 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by davidben@chromium.org
, Sep 5 2017