consider failing same-origin fetch requests that get a cross-origin cors Response synthesized by a service worker

[wanderview]

At the service worker f2f at TPAC today we discussed the issue I raised here:

#146 (comment)
https://bugzilla.mozilla.org/show_bug.cgi?id=1222008#c39

To summarize, consider this use case:

// main window
let outerResponse = await fetch(url, { mode: 'same-origin' });

// controlling service worker
addEventListener('fetch', evt => {
  evt.respondWith(async function() {
    let innerResponse = await fetch(crossOriginURL, { mode: 'cors' });
    return innerResponse;
  }());
});

For a long time we have allowed the service worker to synthesize the cross-origin CORS response on the same-origin FetchEvent.request. The rationale was that the body could be read from the CORS response and used to create a completely synthetic Response, so it would be a toothless restriction.

At this point in history, though, the original request URL was maintained. So outerResponse.url === url and innerResponse.url === crossOriginURL.

PR #146, however, then added a change that exposes the Response.url if its present instead of using the Request.url. After this the example should result in values such that outerResponse.url === innerResponse.url === crossOriginURL.

Now, this seems somewhat innocuous for this fetch() example. Consider, though, what this means for other same-origin network requests.

A top level worker script is required to be same origin. If we allow a cross-origin CORS response to be synthesized for these scripts and expose the URL then we will result in a self.location.href that is of a different origin than self.origin. Also, all importScripts() will relative to the other origin.

Its not clear that there is an attack vector here, but it is clearly unprecedented and quite weird.

Also, consider that the original argument for allowing cross-origin CORS responses for same-origin requests is no longer quite valid. If script created a new synthetic Response, the URL ends up as empty string and the Request URL is used instead. The direct passing of the CORS response is not equivalent any more.

Also, passing the cross-origin CORS response for same-origin requests is inconsistent with other restrictions we have put in place. Consider a manual redirect Request. We fail these requests if you pass them a Responsed with .redirected true. In general this is easily worked around by creating a new synthetic Response with the same body which will not get the redirected flag set. We still enforce the .redirected restriction, however, even though its just as toothless as the same-origin restriction we originally decided not to enforce.

Given all of this, during the face-to-face we came to the conclusion that it would be desirable to enforce the same-origin restriction. If a service worker synthesizes a cross-origin CORS response on a same-origin Request then the fetch would be rejected with a NetworkError.

There is some question if this is too breaking or not at this point. We need to collect some data to that end.

If its too breaking, though, I think we should consider using the Request.url if the Response.url is cross-origin and the Request.mode is same-origin. This might be something we would consider doing in gecko in the short term while we collect usage data.

@annevk, what do you think?

[annevk]

Consider a manual redirect Request.

We enforce the restriction here because the caller (well, at least one of them: navigation) cannot deal with a different URL.

I think we should consider using the Request.url if the Response.url is cross-origin and the Request.mode is same-origin.

That seems rather dangerous if the implementation is indeed still making authority decisions based on the request URL rather than the response concept. It would mean that certain headers normally not exposed through CORS might inadvertently end up being exposed.

[annevk]

cc @whatwg/security

[wanderview]

That seems rather dangerous if the implementation is indeed still making authority decisions based on the request URL rather than the response concept. It would mean that certain headers normally not exposed through CORS might inadvertently end up being exposed.

Its exactly our universal CORS checking which caught this problem. Doing what the spec currently says requires poking holes through these security checks. I have a real problem with doing this.

Also, I would appreciate you addressing my other point that this:

evt.respondWith(fetch(crossOriginURL, { mode: 'cors' }));

And this:

evt.respondWith(async function() {
  let corsResponse = await fetch(crossOriginURL, { mode: 'cors' });
  let syntheticResponse = new Response(corsResponse.body);
  return syntheticResponse;
}());

Are no longer equivalent. They used to be when we originally allowed CORS responses to bypass the same-origin Response, but stopped being the same when we chose to expose Response.url.

Also, please address the concern about self.origin being a different origin from self.location is extremely unusual ergonomics and currently unprecedented for non-local URLs.

[wanderview]

That seems rather dangerous if the implementation is indeed still making authority decisions
based on the request URL rather than the response concept. It would mean that certain headers
normally not exposed through CORS might inadvertently end up being exposed.

Its exactly our universal CORS checking which caught this problem. Doing what the spec currently > says requires poking holes through these security checks. I have a real problem with doing this.

In other words, we don't use the Request.url. We use the Response.url. But if we expose a cross-origin Response.url for a network request that has a same-origin restriction our security restrictions rightfully want us to fail the request.

The current spec (which I'm asking to change) requires us to either poke a hole through that restriction or avoid using the Response.url in that case. I don't want to do either one of these. I want to reject which I think is the safest.

[wanderview]

Also I would point out the part that is appealing to security team here is the exact behavior we do for this case in SW:

evt.respondWith(async function() {
  let corsResponse = await fetch(crossOriginURL, { mode: 'cors' });
  let syntheticResponse = new Response(corsResponse.body);
  return syntheticResponse;
}());

A cross-origin body is returned but the outer Response ends up with the Request.url. Are you really arguing that is unsafe @annevk?

[wanderview]

Sorry, talking on IRC I realize I was not specific enough in my previous posts here.

When I saw "same-origin Request" above, I mean Request.mode === 'same-origin'; not just a same-origin URL. In other words, the Request has a policy to reject cross-origin URLs.

[annevk]

I think you're reading way more into the couple points I made and copying folks interested in security than was meant. https://github.com/whatwg/meta/blob/master/GITHUB-TEAMS.md might help.

I think that changing the URL of a resource is problematic, more so than changing its origin (can also be done through sandboxing), as it ends up breaking relative URLs. If you do this explicitly through a synthetic response it seems less problematic, as presumably in that case you know it to not break.

I also think that a security model that puts the sole authority with the request is wrong in the world of service workers.

I would be happy with rejecting CORS responses when request's mode is "same-origin". That seems better than introducing a local quirk that does not normally apply. I call it a quirk as it gives the request a strange new primitive we don't otherwise expose.

[wanderview]

I would be happy with rejecting CORS responses when request's mode is "same-origin".

Thanks for clarifying. Sorry for my earlier confusion.

Just to clarify, do you mean all CORS responses or only cross-origin CORS responses?

I think @jakearchibald and I were thinking we would only reject on cross-origin CORS responses.

Assuming we are on the same page there, the question is how to get there now. The current situation is:

• Firefox does not propagate Response.url on interception in any case. We also don't reject cross-origin CORS Responses when intercepting a same-origin mode Request.
• Chrome does propagate Response.url on interception for at least some cases, like fetch API. Its hard to tell if it applies to all cases, though, and google implementors don't think it does. And chrome does not reject cross-origin CORS responsesfor same-origin mode Requests either.

Since chrome has shipped propagating the Response.url in some fairly visible cases I'd like to also move forward in firefox. I don't want to wait weeks or months to collect data on the cross-origin CORS for same-origin mode Request case.

I can see two ways to move forward:

1. Implement Response.url propagation and immediately implement rejecting cross-origin CORS responses for same-origin mode Requests. Then see if anyone complains.
2. Implement Response.url propagation. For cross-origin CORS responses for same-origin mode Requests do the following:
   a. Log telemetry to collect data we need to eventually reject.
   b. Log a deprecation warning to console
   c. Add a quirk to do the equivalent of new Response(crossOriginResponse.body, crossOriginResponse).
   d. Once we have enough data remove the quirk and reject.

I personally feel like (2) would give us the best immediate interop between chrome and firefox. It would also give us a path to rejecting. If chrome is willing to follow suite quickly with rejecting, then I would be open to implementing (1).

Thoughts @annevk @jakearchibald @mattto?

[annevk]

How can a CORS response be same-origin? After a cross-origin redirect? I think those should be rejected too.

[wanderview]

Ok, fair enough. I was confusing a mode==cors Request. I thought that was a CORS Response with a same-origin URL, but you are correct it should return a basic Response. A CORS Response should only be returned if something cross-origin happened. Sorry for my confusion.

[wanderview]

Just to clarify another question:

If a CORS response is provided by the service worker we would still accept that for a mode==='navigate' Request, correct?

The rationale being that the Response.url is not normally exposed there and the manual redirect mode already blocks Response.redirect being passed through as anything other than false.

This doesn't match gecko internal very well, but we can make it work.

[jakearchibald]

I've filed https://bugs.chromium.org/p/chromium/issues/detail?id=784018 to discover if we can start rejecting cross-origin responses to same-origin requests without breaking the web.

[wanderview]

We are also going to add a telemetry probe for this:

https://bugzilla.mozilla.org/show_bug.cgi?id=1416629

I'm also asking Tom to work on the quirk controlled by a preference. This will allow us to disable the quirk quickly once we get the data saying its ok or multiple browsers agree we should break compat anyways.

[wanderview]

@mattto, I know its early, but do you have any initial data yet?

Our probe has only been in nightly a few days, but so far we have ~2M synthesized responses and 0 cors-response-for-same-origin-request. Again, this is only nightly and just a few days of data.

[wanderview]

I guess I don't see anything for RespondToSameOriginRequestWithCrossOriginResponse here:

https://www.chromestatus.com/metrics/feature/popularity

[mfalken]

Same, it looks like we have never logged a RespondToSameOriginRequestWithCrossOriginResponse event and this code should be live in Dev/Canary.

[wanderview]

We're probably going to land the rejecting behavior in firefox nightly 59. We'll look at adding a fallback mechanism before 59 merges to beta based on how the data comes in.

[wanderview]

FWIW the telemetry still does not show any hits on CORS responses synthesized on same-origin mode requests. So we're going to go ahead with the rejection in this case.

We are looking at updating the tests in:

https://bugzilla.mozilla.org/show_bug.cgi?id=1427978

I think the spec change here probably just involves adding a bullet to step 3.2.3 here:

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

We implemented like:

• request's mode is "same-origin" and response's type is "cors".

[annevk]

@mattto are you all comfortable making that change as well per above? We can land tests then (and I guess I'll update the specification).

[wanderview]

Also @hober, does webkit have any objections here? My impression was we were all relatively in agreement at the f2f.

I would like to ask Microsoft as well, but I don't have a github handle for Ali. I'll email him.

[annevk]

Pretty sure that's @aliams. Would be great to get feedback from all, indeed.

[aliams]

This sounds fair to me and we'd be comfortable making the change.

[youennf]

Change looks good to me too. I don’t think WebKit has any compat concern here.

[jakearchibald]

Based on the results, I'm happy with this change

[mfalken]

Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=800234

[wanderview]

FYI, we expect to ship this in FF59 in mid-March time frame.