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

Service workers allow for more responses to be executed as script #1509

Open
annevk opened this issue Mar 31, 2020 · 31 comments
Open

Service workers allow for more responses to be executed as script #1509

annevk opened this issue Mar 31, 2020 · 31 comments
Labels
security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response.

Comments

@annevk
Copy link
Member

annevk commented Mar 31, 2020

Before service workers you could only execute as script what <script src> allows you to target. After service workers any response a service worker can obtain, can be given to a <script>. E.g., consider a response to a request whose method is POST or one that has a custom Accept header value.

Should we enshrine this as another weakening of the same-origin policy or do something about it?

Credit: 1lastBr3ath.

@annevk annevk added the security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. label Mar 31, 2020
@jakearchibald
Copy link
Contributor

Possible solutions:


Add the request method onto the response, then update the HTML spec so APIs that accept no-cors requests reject if the response is no-cors and the request method is not GET.


Limit cross origin no-cors requests to GET.

Which APIs can currently make no-cors non-navigation POST requests? sendBeacon is one. It isn't clear to me why that's no-cors. But, I don't know if we could change it now.

If that's the only one, we could make sendBeacon bypass the service worker. I think it already bypasses in Chrome fwiw.

@annevk
Copy link
Member Author

annevk commented Mar 31, 2020

fetch() can as well.

@jakearchibald
Copy link
Contributor

Right, but I'm saying we could stop that (if usage is low enough).

@jakearchibald
Copy link
Contributor

CSP's reporter is also POST and no-cors. I imagine other reporters are the same.

@jakearchibald
Copy link
Contributor

Although there are a few APIs that send no-cors POST requests, none of them seem to use the response body.

Around the same point in the spec as CORB (or even, part of CORB), if the request is no-cors & non-GET, the body of the response can be removed.

The fetch will still succeed, and specs will still be able to look at the status code, but <script> etc will be given an empty response.

@annevk
Copy link
Member Author

annevk commented Apr 7, 2020

What happens if such a POST gets a 301/302/303? Still blocked somehow?

This does seem quite nice on the face of it.

@jakearchibald
Copy link
Contributor

What happens if such a POST gets a 301/302/303? Still blocked somehow?

I guess you mean if the no-cors request leaves the origin but is redirected back to the origin for the final response?

@annevk
Copy link
Member Author

annevk commented Apr 14, 2020

That's a good twist, but a redirect to a POST with one of those response statuses will also change the request method to GET.

@arturjanc
Copy link

/cc @anforowicz @csreis in case they had considered something like this in the context of CORB.

@csreis
Copy link

csreis commented Apr 14, 2020

Our general goal with CORB was to only let cross-origin responses into the renderer process if they could be legitimately used as subresources (e.g., in script tags, image tags, etc), or if they were allowed via CORS. The blocking logic didn't care how the renderer process actually asked for the response (e.g., via ServiceWorker vs script tag), since a compromised renderer could claim to use whatever request type would let it get the most data. Thus, CORB blocks things like HTML, XML, and JSON whether a ServiceWorker is asking or not.

It does sound like this trick would let an origin try to run more things as script than usual, so I'm happy to see you all talking about ways to address it. I don't have strong feelings about how or where in the spec it goes, though. I only suggest that you avoid having the CORB part of the spec depend on how the content was requested (e.g., whether it was from a ServiceWorker), since that can be forged.

@jakearchibald
Copy link
Contributor

I only suggest that you avoid having the CORB part of the spec depend on how the content was requested (e.g., whether it was from a ServiceWorker)

Fwiw, that wouldn't be the condition here. The condition would be "is the response opaque, and was the request method not-GET?"

The condition wouldn't be dependent on the service worker at all. Although, I realise it's still checking aspects of the request.

It doesn't need to be in the same bit of the spec as CORB of course, but it would seem a shame to copy & paste a lot of the prose if it's 90% the same.

@anforowicz
Copy link

anforowicz commented Apr 15, 2020

So, how does the change in behavior look from the CORB perspective? Is it something like this?:

  • Before the proposed changes: Cross-origin, no-cors non-GET responses are already blocked by CORB if the response MIME type is CORB-eligible (html/xml/json/pdf/zip/etc)
  • After the proposed changes: Cross-origin, no-cors non-GET responses are always blocked by CORB regardless of the response MIME type (therefore the "after" behavior extends CORB protection to POST responses carrying things like image/png or application/javascript)

I think this should work. CORB only needs to allow legacy no-cors requests (images, scripts, stylesheets, etc.) and AFAIK these should always be GET requests. So, extending CORB heuristics to cover all non-GET requests makes sense from this perspective (no risk of breaking existing behavior + more requests protected by CORB = seems like a desirable change to me).

@anforowicz
Copy link

anforowicz commented Apr 15, 2020

Oh, one more question - will this proposal be testable via WPT? AFAIK there are no web APIs for reading the response body associated with beacon and/or reporting POST requests, right? If the proposal is non-testable via WPT, then should it be in a non-normative part of the spec?

@jakearchibald
Copy link
Contributor

So, how does the change in behavior look from the CORB perspective? Is it something like this?:

  • Before the proposed changes: Cross-origin, no-cors non-GET responses are already blocked by CORB if the response MIME type is CORB-eligible (html/xml/json/pdf/zip/etc)
  • After the proposed changes: Cross-origin, no-cors non-GET responses are always blocked by CORB regardless of the response MIME type (therefore the "after" behavior extends CORB protection to POST responses carrying things like image/png or application/javascript)

Exactly.

Oh, one more question - will this proposal be testable via WPT?

Yeah, you can make no-cors POST requests using fetch(). The test would be:

  1. <script src="whatever.js">.
  2. Intercept that with a service worker.
  3. Respond with the response from a cross origin no-cors POST request, where the response is window.bad = true;
  4. Once the script loads, window.bad must be undefined.

@annevk
Copy link
Member Author

annevk commented Apr 17, 2020

I would prefer we network error for this instead of using CORB. I'm not convinced CORB as-is is the right long term solution for opaque responses.

@jakearchibald
Copy link
Contributor

APIs are making no-cors POST requests. I'm worried about compatibility issues if those start failing. From what I've seen, they're fire-and-forget, but I don't know if they're all like that.

@mikewest
Copy link
Member

From what I've seen, they're fire-and-forget, but I don't know if they're all like that.

Perhaps adding a fire-and-forget request mode that always returned a network error would be better than treating the reporting bits and pieces discussed above as no-cors?

@annevk
Copy link
Member Author

annevk commented Apr 20, 2020

Unfortunately it affects fetch() itself and we probably cannot change its shape at this point.

@jakearchibald
Copy link
Contributor

I guess we could look at the numbers

@mikewest
Copy link
Member

Adding a metric for no-cors POST requests via fetch() seems like a reasonable thing to do. If we can get away with a network error, I agree that would be cleaner. In the short term, applying CORB seems like something we can do without waiting for numbers.

@jakearchibald
Copy link
Contributor

@annevk

That's a good twist, but a redirect to a POST with one of those response statuses will also change the request method to GET.

My memory is a bit flakey here so correct me if I'm wrong: if an opaque response redirects, the response is always opaque right?

CSP uses "error" for redirect. I guess we would enforce that for all non-GET no-cors requests.

If we're going the CORB-style route, I guess we could follow redirects by default, but create a new response tainting of "no-body", which fetch will use to discard response bodies before it re-enters the page.

@annevk
Copy link
Member Author

annevk commented Apr 20, 2020

Yeah, you're not supposed to be able to recover from opaque.

@jakearchibald
Copy link
Contributor

@annevk
Copy link
Member Author

annevk commented Apr 20, 2020

Yeah, thanks.

@wanderview
Copy link
Member

Where does this issue stand?

@annevk
Copy link
Member Author

annevk commented Nov 10, 2021

It seems that per https://chromestatus.com/metrics/feature/timeline/popularity/3316 there's an unusually high number of responses to which this is applicable. It's not clear to me however that making these a network error would end up breaking things as they might mostly be beacon requests of sorts.

I guess nobody did what @mikewest suggested in https://bugs.chromium.org/p/chromium/issues/detail?id=1072350#c27?

@wanderview
Copy link
Member

Instead of blocking the requests at the fetch() layer, can we instead have a check at respondWith() which rejects an opaque response that was originated with a non-GET method when the request destination is script? Like the other checks at step 5.5.5 here:

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

@annevk
Copy link
Member Author

annevk commented Nov 10, 2021

We could I suppose. That would require storing an additional bit on the response, right? How would that work with the Cache API?

@wanderview
Copy link
Member

The bit would have to be stored along with the response so it could be reproduced when read back out. In chromium we already store it for other reasons:

https://source.chromium.org/chromium/chromium/src/+/main:content/browser/cache_storage/cache_storage.proto;l=58;drc=22a2236d2969b1ff48ae3a642eaae241ffabdd28

@wanderview
Copy link
Member

I guess this doesn't help with the "custom Accept header" from the original post above, though?

@annevk
Copy link
Member Author

annevk commented Nov 10, 2021

Yeah, it wouldn't help with the couple of headers the attacker gets to control, but method is a lot more significant and it seems somewhat reasonable to declare the headers out-of-scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response.
Projects
None yet
Development

No branches or pull requests

7 participants