-
Notifications
You must be signed in to change notification settings - Fork 316
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
CacheStorage
& COEP:credentialless
#1592
Comments
I think an important thing to consider here is that typically these cache responses are returned to fetch, which will run its CORP check in step 7 of https://fetch.spec.whatwg.org/#concept-http-fetch. So I don't think 2 will work (at least not without adding further complexity elsewhere). Assuming we want to treat the policies in an equivalent manner (still a bit unclear), perhaps storing a bit on opaque responses as to whether they are "COEP friendly" is a way to go here. Not allowing opaque responses would be nicer still, but I'm not sure how feasible that is. |
+1 |
Thanks! |
Add tentative WPT tests about COEP and CacheStorage. Issues: - COEP:require-corp: w3c/ServiceWorker#1490 - COEP:credentialless: w3c/ServiceWorker#1592 Bug:1175099 Change-Id: I857adbd134443b17b9689c314307bb1e3888235b
Add tentative WPT tests about COEP and CacheStorage. Issues: - COEP:require-corp: w3c/ServiceWorker#1490 - COEP:credentialless: w3c/ServiceWorker#1592 Bug:1175099 Change-Id: I857adbd134443b17b9689c314307bb1e3888235b
Add tentative WPT tests about COEP and CacheStorage. Issues: - COEP:require-corp: w3c/ServiceWorker#1490 - COEP:credentialless: w3c/ServiceWorker#1592 Bug: 1175099 Change-Id: I857adbd134443b17b9689c314307bb1e3888235b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2876191 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#880447}
Add tentative WPT tests about COEP and CacheStorage. Issues: - COEP:require-corp: w3c/ServiceWorker#1490 - COEP:credentialless: w3c/ServiceWorker#1592 Bug: 1175099 Change-Id: I857adbd134443b17b9689c314307bb1e3888235b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2876191 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#880447}
Add tentative WPT tests about COEP and CacheStorage. Issues: - COEP:require-corp: w3c/ServiceWorker#1490 - COEP:credentialless: w3c/ServiceWorker#1592 Bug: 1175099 Change-Id: I857adbd134443b17b9689c314307bb1e3888235b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2876191 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#880447}
…a=testonly Automatic update from web-platform-tests [Credentialless]: WPT vs CacheStorage. Add tentative WPT tests about COEP and CacheStorage. Issues: - COEP:require-corp: w3c/ServiceWorker#1490 - COEP:credentialless: w3c/ServiceWorker#1592 Bug: 1175099 Change-Id: I857adbd134443b17b9689c314307bb1e3888235b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2876191 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#880447} -- wpt-commits: 6bf109f244a20f48bb8723663c384e1e2220f006 wpt-pr: 28891
We now have some tentative tests (49 cases) for Document, SharedWorker, ServiceWorker, and DedicatedWorker: I prototyped plumbing the
Then, we have two options:
(1) is simpler and stricter. I am not sure it really matter. CacheStorage is a per-origin API. I am expecting website to use a consistent COEP policy for the whole website & workers. I don't believe there will be a strong use case for using CacheStorage with different COEP policies. So I believe the strictest option (1) to be better. We can always make it more permissive later, if we feels this is useful. Note: Here are the expectations for both cases on the test
|
I was thinking about a different model, whereby we give an opaque response a bit (tentatively named "COEP safe") if it was fetched in the context of a COEP policy. And then if you obtain something from the cache in the context of a COEP policy and you get an opaque response, you'd error if the bit isn't set. Fetch would also have to be updated to check for this for responses from the service worker and to set this correctly when fetching something. I think this is equivalent to your less strict version. I have a slight preference for this as it seems like the logic would be simpler for anyone having to handle these kind of responses and it would be more consistent with how we treat the policies as interchangeable elsewhere (e.g., nested documents). |
Yes, "coep_safe" might work too. To clarify what you meant, what would give those cases?
(1) & (2) are just the base cases with
Indeed, that would be a nice thing to keep if can can. |
Thank you for writing that up, I see what I overlooked now. My approach would only work if the original client had COEP and so "COEP safe" is not realistic. I'm actually not sure what's ideal then, but storing whether it was obtained with credentials doesn't seem that bad. |
@mfalken and I caught up with this thread. Thanks for the detailed explanations. I wasn't really aware of I think our goal should be:
It feels like this could be achieved by the So, after step 4 of https://fetch.spec.whatwg.org/#cross-origin-resource-policy-internal-check:
Does that work? I guess request included credentials would be set around the same time as the range requested flag. |
@jakearchibald I am almost done with a prototype in Chrome and this looked reasonable. |
On w3c/ServiceWorker#1592 (comment) I mentionned 5 "interesting" cases about `COEP:credentialless` and CacheStorage. All are already tested, except the 4th. This patch introduces two new tests with request.credentials="omit", including the missing one above. R=antoniosartori@chromium.org CC:clamy@chromium.org CC:mkwst@chromium.org CC:lyf@chromium.org Bug: 1175099 Change-Id: If8ea3ebc9efd6622be0d8c4dae7eba1bb28ce3bc
On w3c/ServiceWorker#1592 (comment) I mentioned 5 "interesting" cases about `COEP:credentialless` and CacheStorage. All are already tested, except the 4th. This patch introduces two new tests with request.credentials="omit", including the missing one above. R=antoniosartori@chromium.org CC:clamy@chromium.org CC:mkwst@chromium.org CC:lyf@chromium.org Bug: 1175099 Change-Id: If8ea3ebc9efd6622be0d8c4dae7eba1bb28ce3bc
On w3c/ServiceWorker#1592 (comment) I mentioned 5 "interesting" cases about `COEP:credentialless` and CacheStorage. All are already tested, except the 4th. This patch introduces two new tests with request.credentials="omit", including the missing one above. R=antoniosartori@chromium.org CC: clamy@chromium.org CC: mkwst@chromium.org CC: lyf@chromium.org Bug: 1175099 Change-Id: If8ea3ebc9efd6622be0d8c4dae7eba1bb28ce3bc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2917020 Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#886703}
On w3c/ServiceWorker#1592 (comment) I mentioned 5 "interesting" cases about `COEP:credentialless` and CacheStorage. All are already tested, except the 4th. This patch introduces two new tests with request.credentials="omit", including the missing one above. R=antoniosartori@chromium.org CC: clamy@chromium.org CC: mkwst@chromium.org CC: lyf@chromium.org Bug: 1175099 Change-Id: If8ea3ebc9efd6622be0d8c4dae7eba1bb28ce3bc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2917020 Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#886703}
On w3c/ServiceWorker#1592 (comment) I mentioned 5 "interesting" cases about `COEP:credentialless` and CacheStorage. All are already tested, except the 4th. This patch introduces two new tests with request.credentials="omit", including the missing one above. R=antoniosartori@chromium.org CC: clamy@chromium.org CC: mkwst@chromium.org CC: lyf@chromium.org Bug: 1175099 Change-Id: If8ea3ebc9efd6622be0d8c4dae7eba1bb28ce3bc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2917020 Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#886703}
… cases about "omit"., a=testonly Automatic update from web-platform-tests [Credentialless]: WPT vs CacheStorage. 2 cases about "omit". On w3c/ServiceWorker#1592 (comment) I mentioned 5 "interesting" cases about `COEP:credentialless` and CacheStorage. All are already tested, except the 4th. This patch introduces two new tests with request.credentials="omit", including the missing one above. R=antoniosartori@chromium.org CC: clamy@chromium.org CC: mkwst@chromium.org CC: lyf@chromium.org Bug: 1175099 Change-Id: If8ea3ebc9efd6622be0d8c4dae7eba1bb28ce3bc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2917020 Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#886703} -- wpt-commits: b7a876caf70a8124d66f1a112d39c3712b2fa125 wpt-pr: 29102
… cases about "omit"., a=testonly Automatic update from web-platform-tests [Credentialless]: WPT vs CacheStorage. 2 cases about "omit". On w3c/ServiceWorker#1592 (comment) I mentioned 5 "interesting" cases about `COEP:credentialless` and CacheStorage. All are already tested, except the 4th. This patch introduces two new tests with request.credentials="omit", including the missing one above. R=antoniosartori@chromium.org CC: clamy@chromium.org CC: mkwst@chromium.org CC: lyf@chromium.org Bug: 1175099 Change-Id: If8ea3ebc9efd6622be0d8c4dae7eba1bb28ce3bc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2917020 Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#886703} -- wpt-commits: b7a876caf70a8124d66f1a112d39c3712b2fa125 wpt-pr: 29102
… cases about "omit"., a=testonly Automatic update from web-platform-tests [Credentialless]: WPT vs CacheStorage. 2 cases about "omit". On w3c/ServiceWorker#1592 (comment) I mentioned 5 "interesting" cases about `COEP:credentialless` and CacheStorage. All are already tested, except the 4th. This patch introduces two new tests with request.credentials="omit", including the missing one above. R=antoniosartori@chromium.org CC: clamy@chromium.org CC: mkwst@chromium.org CC: lyf@chromium.org Bug: 1175099 Change-Id: If8ea3ebc9efd6622be0d8c4dae7eba1bb28ce3bc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2917020 Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#886703} -- wpt-commits: b7a876caf70a8124d66f1a112d39c3712b2fa125 wpt-pr: 29102
Hi I am back from vacation. I prototyped what was discussed in Chrome and modified the fetch PR accordingly. The interesting part is the CORP check: From:
To:
This change passes all the test cases defined from the table above, and the corresponding WPT. I believe I am happy with that. What about you? |
Why is |
It isn't impacted. I wrote it the same way it is done in the fetch spec, where we assume there is an implicit break for every case of the switch statement. |
That's not how that construct works in my mind, though good to know it can be interpreted that way. As far as I know it's meant to follow |
You are right. I read to quickly the existing usage. I should insert a "Do nothing" instruction to remove the ambiguity. => Done. |
We don't need the latter half of (*) because it's already checked at the callers.
|
Yes that true. I got confused with the Chrome implementation (where response_type is not computed yet for fetch) and my understanding of opaque response was a bit different than the reality (I didn't know about tainted_origin for cross-origin redirect). Since the CORP check is not only run for CacheStorage, but also for normal fetch request, the current proposition would break normal fetch request. I should find a way to filter out:
|
I'm struggling to follow this a bit. I thought it was a good thing that the check included normal fetch requests. Can you walk me through a case where this breaks? |
For the network fetch nothing should be blocked because we omit the credentials. On the other hand, for responses coming from the service worker we need a similar check, so it may make sense to have the logic in the CORP check to not have duplicated logic. |
@jakearchibald, @yutakahirano Keeping the check in CORP (as Yutaka suggest) sounds good. And removing the unnecessary check about response_type = "opaque" is also good. This would lead to this change to the CORP check: |
Discussion here: w3c/ServiceWorker#1592 + Plumb response's |request_include_credentials| + Modify CORP check to prevent vulnerable credentialled opaque responses from entering COEP:credentialless document via CacheStorage. Change-Id: Ieaffd379da43904ba9b5dfe364489ef02ec20b2e Bug: 1175099 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2886899 Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#889719}
I feel we can close this issue now. It solves the issue with CacheStorage, but also for ServiceWorkers. I am happy with it. So for COEP:credentialless context, a no-cors subresource requires either a CORP header or to be requested without credentials. It means when a response is transferred from a COEP:unsafe-none context toward a COEP:credentialless, then it might be blocked. |
Add tentative WPT tests about COEP and CacheStorage. Issues: - COEP:require-corp: w3c/ServiceWorker#1490 - COEP:credentialless: w3c/ServiceWorker#1592 Bug: 1175099 Change-Id: I857adbd134443b17b9689c314307bb1e3888235b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2876191 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#880447} NOKEYCHECK=True GitOrigin-RevId: a07d25f92a3402c5d4b27014b353bff565873ddb
On w3c/ServiceWorker#1592 (comment) I mentioned 5 "interesting" cases about `COEP:credentialless` and CacheStorage. All are already tested, except the 4th. This patch introduces two new tests with request.credentials="omit", including the missing one above. R=antoniosartori@chromium.org CC: clamy@chromium.org CC: mkwst@chromium.org CC: lyf@chromium.org Bug: 1175099 Change-Id: If8ea3ebc9efd6622be0d8c4dae7eba1bb28ce3bc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2917020 Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#886703} NOKEYCHECK=True GitOrigin-RevId: ca89e2f8f1184dab4be6115e1efa09ff30688810
Discussion here: w3c/ServiceWorker#1592 + Plumb response's |request_include_credentials| + Modify CORP check to prevent vulnerable credentialled opaque responses from entering COEP:credentialless document via CacheStorage. Change-Id: Ieaffd379da43904ba9b5dfe364489ef02ec20b2e Bug: 1175099 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2886899 Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#889719} NOKEYCHECK=True GitOrigin-RevId: 22a2236d2969b1ff48ae3a642eaae241ffabdd28
The current behavior of CacheStorage with COEP is uniformly implemented across Firefox, Edge and Chrome:
https://wpt.fyi/results/html/cross-origin-embedder-policy?label=master&label=experimental&aligned
If the fetch client has
COEP:unsafe-none
and the response is fetched again via CacheStorage from a client withCOEP:require-corp
, we run again the CORP check on the cross-originno-cors
response and potentially block it. This avoids a cross-originno-cors
response from entering acrossOriginIsolated
process without an explicit opt-in from the server.The current spec is:
See:
We have the same question for
COEP:credentialless
(see whatwg/html#6637). How to prevent credentialled opaque responses from entering thecrossOriginIsolated
context?This is a bit different, because
COEP:credentialless
is about the request andCOEP:require-corp
about the response.Possibilities could be:
COEP:credentialless
context.COEP:require-corp
, run thecross-origin resource policy check
, but require corp.COEP:unsafe-none
intoCOEP:credentialless
. We can also potentially allow the one passingcross-origin resource policy check
with require-corp.@mikewest @annevk @yutakahirano @asutherland @camillelamy
The text was updated successfully, but these errors were encountered: