WICG / compression-dictionary-transport

Other
92 stars 8 forks source link

Supporting "no-cors" mode requests seems problematic. #39

Open horo-t opened 12 months ago

horo-t commented 12 months ago

When a browser fetches a cross-origin script (eg: <script src='https://static.example.com/script.js'> in https://www.example.com/index.html) , it sends a request with the mode set to no-cors and the credentials set to include.

The current explainer allows this type of request for both registering as a dictionary and using a registered dictionary for its decompression, as long as the response header contains a valid Access-Control-Allow-Origin header (* or https://www.example.com).

However, if we follow the CORS check step in the Fetch spec, the response header must also contain the Access-Control-Allow-Credentials: true header, and Access-Control-Allow-Origin header must be https://example.com. This means that the server must know the origin of the request, even though the request does not include an Origin header. (It may include a Referer header. But the Origin header and Referer header are conceptually different.)

For this reason, now I think supporting no-cors mode requests is problematic. Maybe we should support only navigate, same-origin, and cors mode requests?

@pmeenan @yoavweiss Do you have any thoughts?

yoavweiss commented 12 months ago

The current explainer allows this type of request for both registering as a dictionary and using a registered dictionary for its decompression, as long as the response header contains a valid Access-Control-Allow-Origin header (* or https://www.example.com).

That's not how I read the explainer (although I agree it can have tighter language). Specifically in static resource flow it says that "the response must also be non-opaque in order to be used as a dictionary"

horo-t commented 12 months ago

Ah, I may have misunderstood.

@pmeenan Do you think it is ok not to support no-cors cross origin requests?

Supporting no-cors requests is desirable for ease of deployment. But it sounds too risky from the security point of view.

If we don't support no-cors cross origin requests, cross origin script tags in the explainer's examples should have crossorigin attribute.

pmeenan commented 12 months ago

We definitely need to support no-cors cross origin requests assuming the set of headers would allow it for fetch, I just need to tighten up the language around it. Anything that is non-opaque to fetch should be allowed to be used for a dictionary and compressed payload.

I'll reach out to the security team to see if I can get some help crafting the exact combination of headers and language to spec it completely but for this case it sounds like Access-Control-Allow-Credentials: true needs to be added to the response.

There's a matrix of Sec-Fetch-Mode, Sec-Fetch-Site, Origin, Referer, Access-Control-Allow-Origin and Access-Control-Allow-Credentials that needs to be fleshed out better (and even in the case where an Origin request header isn't sent, if Access-Control-Allow-Origin allows for the actual origin it should still be allowed (I think).

I'm far from an expert in CORS though and the exact conditions for being non-opaque in all of the different fetch modes so I'll find someone to help clear up the specifics. There are a fair number of sites that have their static scripts and css on a separate subdomain even though they're "1st party" so we need to make sure it works for adoption.

pmeenan commented 12 months ago

There's a good chance I'm wrong though and no-cors is by-definition CORS-opaque in which case the example needs to be updated. Just need to check quickly with those that live and breathe this on a daily basis.

horo-t commented 11 months ago

How about same origin no-cors requests? For example, the request mode for <script src='./script.js'> is no-cors.

Supporting no-cors same origin requests would make the browser logic more complex. It needs to track redirects, and stop sending sec-available-dictionary header when cross origin redirect happens.

The request mode for <script src='./script.js' crossorigin> is cors. If we don't need to support no-cors same origin requests, the browser just need to check the mode only when starting the request.

pmeenan commented 11 months ago

It feels like that should be something we'd want to support but is still technically cors-opaque.

It might make sense to support no-cors requests that are cache-control: public with a non-zero ttl if a future fetch() would be able to pull the same response out of cache (need to do some testing).

Sec-fetch-site: could tell us same-site but that still doesn't guarantee cors-readability.

On Mon, Jul 10, 2023 at 9:24 PM Tsuyoshi Horo @.***> wrote:

How about same origin no-cors requests? For example, the request mode for <script src='./script.js'> is no-cors.

Supporting no-cors same origin requests would make the browser logic more complex. It needs to track redirects, and stop sending sec-available-dictionary header when cross origin redirect happens.

— Reply to this email directly, view it on GitHub https://github.com/WICG/compression-dictionary-transport/issues/39#issuecomment-1629953500, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADMOBNOFVLRXR3DQFG6O33XPSTLXANCNFSM6AAAAAA2EG5K5Q . You are receiving this because you were mentioned.Message ID: @.***>

yoavweiss commented 11 months ago

Supporting no-cors same origin requests would make the browser logic more complex. It needs to track redirects, and stop sending sec-available-dictionary header when cross origin redirect happens.

No-CORS same-origin requests are not opaque, and are something we should support. I'd imagine that the redirect tracking is something we're already doing (e.g. to enable such responses to be readable in fetch() calls.

pmeenan commented 11 months ago

Sorry for the delay, wanted to test the permutations to be sure. I used CSS because you can test for opaqueness by trying to access the cssRules from script. I set up a test page at https://test.patrickmeenan.com/opaque/ that loads a CSS from the same origin and from another subdomain under the same TLD.

The same-origin request is not opaque and is fetched with:

sec-fetch-mode: no-cors
sec-fetch-site: same-origin

The other request does not have sec-fetch-site.

I'm going to do a little more testing to verify that the sec-fetch-site state is updated as redirect chains are followed (I assume they are). If so, the solution is probably to always allow for a resource to be a dictionary as long as sec-fetch-site = same-origin.

Definitely need to get security peeps to weigh in though because sec-fetch-site is listed as a non-CORS header so we need to be sure that it can be used for this case.

pmeenan commented 11 months ago

Updated the test page to also try a same-origin request that redirects to cross-origin and the sec-fetch-site correctly gets reset on the redirect (and it is opaque from JS).

WebPageTest result with the headers is here.

I'll update the processing flow in the ID to have a first step in the processing that allows for dictionary use for sec-fetch-site: same-origin

pmeenan commented 11 months ago

I have an updated version of the processing in my github repo for the ID: https://pmeenan.github.io/i-d-compression-dictionary/draft-meenan-httpbis-compression-dictionary.html

horo-t commented 11 months ago

In the Fetch spec, a request has an associated response tainting flag, which is "basic", "cors", or "opaque". This is initially "basic" for same-origin request. And it is set to "opaque" when the request is redirected to a cross origin URL.

I think browsers should use a shared dictionary only when the response tainting is "basic" or "cors".

In Chromium, the response tainting is calculated in network::cors::CorsURLLoader when sending a request and handling a redirect. Unfortunately, the SharedDictionaryNetworkTransaction class that decodes shared dictionary compression is a bit far from network::cors::CorsURLLoader. I need to figure out how to pass the flag.