cache.match() and COEP

[wanderview]

What should we do if cache.match() is called in a context with Cross-Origin-Embedder-Policy: require-corp and the Response to be returned does not have a Cross-Origin-Resource-Policy header?

I would like to advocate that we reject the match() since it seems possible there could be information stored in the headers that should not be exposed to spectre attacks. Also, it seems you can have a CORS response without a CORP header that would fail the COEP check and we would not want to expose the body in that case.

@mikewest @annevk @yutakahirano @makotoshimazu

[ArthurSonzogni]

@ArthurSonzogni

[annevk]

There shouldn't be a COEP/CORP check for non-opaque responses (such as a CORS response).

Our thinking about this so far has been that if the opaque response only holds a handle to the actual secret and there's no way for the secret to enter the content process there's no problem and no need for API changes.

But I don't care strongly.

cc @asutherland @perryjiang

[ArthurSonzogni]

@annevk, If I fully understand what you said:

From a document with Cross-Origin-Embedder-Policy: require-corp
If a CORS request is made and the response contains:Access-Control-Allow-Origin: * (for instance)
Then the response is non-opaque and the document can access it. Even if the response do NOT have: Cross-Origin-Resource-Policy: cross-origin
Is it what you said?

[annevk]

Yeah, https://mikewest.github.io/corpp/#abstract-opdef-cross-origin-resource-policy-check exits early with allowed for such a case, no?

[wanderview]

Ok, ignore the comment about CORS responses then. Sorry for my confusion.

It still feels more clear to me to reject the match() call instead of returning an opaque response that may blow up when passed to respondWith() further along. And may leak headers via spectre.

[annevk]

If it's not just a handle things will leak, agreed. And it might therefore be worthwhile to block earlier on. Firefox will implement COEP service/shared workers later on, but I suspect similar considerations apply and unless Andrew/Perry feel otherwise I think we'd be okay with blocking early.

[wanderview]

I'm fairly confident both chrome/firefox store and retrieve the full internal headers for opaque responses. In addition, both browsers do some amount of eager reading of the body data under different circumstances.

It would be possible to fix all that for COEP, but its much safer to never allow these Response objects to enter the child/renderer process at all.

[asutherland]

Yes, @wanderview is right that Firefox currently sends all the data about opaque responses to the content processes and only creates the opaque wrapped response when processing the received IPC data, which is too late. I think bug 1565199 covers the general scenario for Firefox.

The most recent thoughts @perryjiang and I had on this were that we would run the same CORP check on cache.match() responses and at least, for now, send a pre-opaqued response that can't be used for anything. (That is, there's just a opaque filtered response and any attempt to consume the underlying response will fail with a corp-check (network) error.)

API-wise, it seems like there are 5 options:

1. Partition storage of COEP-affected storage by the COEP bits. A COEP-safe storage cannot store something that would fail a CORP-check. All storages opened by COEP contexts set this bit on creation and fail to open a storage if the bit is not set. A non-COEP context can opt in to this constraint when creating a storage. Maybe whole storage buckets can be marked COEP-safe. This may or may not be something that would have synergies about the proposals to give WASM memory-mapped block storage APIs so lucene and SQLite can be compiled into WASM and be fast.
2. Hide keys and Responses when the Response would fail a CORP check. This seems untenably confusing.
3. Ben's proposed match-rejects when a matched Response fails a CORP check. This would cause the most immediate/obvious breakage as matchAll() calls that might not look at the CORP-failing Response might now explode, although that use case seems wacky. This approach becomes more problematic if we allow Responses to be stored in IndexedDB. Application logic that invokes getAll is a thing that happens, and poisoned values would be a real problem in that case.
4. Responses become unusable even though technically if they were just a handle and you were respondWith()ing to a non-COEP context, it could work. (Firefox's current short term plan.) Nothing breaks until you try and consume it.
5. Responses are just a handle and can transit a COEP-context into a non-COEP-context and it should work just like the COEP-context isn't involved. This is least breakage prone but the most complex.

The Responses-get-nulled situation is similar to Blob edge-cases where a <input type="file"> sourced-Blob is invalidated by changes on disk. (Or when Clear-Site-Data/privacy API's clear an origin's data and there are still (cross-origin) Blob references that only held a reference to on-disk data and didn't copy the contents out.) It's an allegedly immutable thing that can turn out not to be there.

The Responses-are-handles situation is sorta analogous to the current MessagePort.postMessage of a SharedArrayBuffer. Because you can never reason about what global the serialized payload will be deserialized in because the ports can always be shipped, there are cases where it's safe to ship the data itself, and there are times when you have to be shipping around a handle. However, in that case we fail to deserialize the entire serialized object due to the SAB, not creating a nulled SAB reference.

I don't have any specific proposal other than banding together to build a gateway to a parallel universe without SPECTRE attacks.

[wanderview]

Ben's proposed match-rejects when a matched Response fails a CORP check. This would cause the most immediate/obvious breakage as matchAll() calls that might not look at the CORP-failing Response might now explode, although that use case seems wacky. This approach becomes more problematic if we allow Responses to be stored in IndexedDB. Application logic that invokes getAll is a thing that happens, and poisoned values would be a real problem in that case.

I think these issues are only problematic for sites that want some contexts COEP protected and some other contexts non-COEP protected. This seems like an unusual use case.

Also, IDB support for responses does not exist today and I haven't seen any movement towards making it happen. Its easier to relax the rejecting behavior if we need to support something like that in the future than it is to claw back the returned Response.

I feel pretty strongly we should start by rejecting cache.match() and cache.matchAll().

[asutherland]

I feel pretty strongly we should start by rejecting cache.match() and cache.matchAll().

It's definitely easy to implement and easy to relax. I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1603168 for the specific implementation in Firefox and we'll likely implement it in the near future.

[ArthurSonzogni]

We also need to define what error type must be returned when cache.match is blocked by CORP checks. Do you have an opinion?

[annevk]

I think I'd prefer a TypeError, similar to what fetch() would return for a network error.

[annevk]

@yutakahirano it doesn't seem this change is captured by https://wicg.github.io/cross-origin-embedder-policy/ at the moment. Where is this captured in specification text?

[yutakahirano]

I'm making a service worker spec PR which includes the change.

[yutakahirano]

#1516