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

COEP and report-only mode #5100

Closed
yutakahirano opened this issue Nov 19, 2019 · 22 comments
Closed

COEP and report-only mode #5100

yutakahirano opened this issue Nov 19, 2019 · 22 comments
Labels
topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal.

Comments

@yutakahirano
Copy link
Member

Our partner says they want "report-only" policy for COEP too. I think that's a reasonable feature request. What do you think?

@annevk @arturjanc @mikewest @domenic

@yutakahirano
Copy link
Member Author

@wanderview

@annevk annevk added the topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal. label Nov 19, 2019
@annevk
Copy link
Member

annevk commented Nov 19, 2019

For the kind of things resulting from COEP standalone (i.e., resources failing to load, potentially due to lack of CORP), probably yes. For how COEP modifies COOP, probably not.

@yutakahirano
Copy link
Member Author

yutakahirano commented Dec 12, 2019

It's super rough, but how does https://gist.github.com/yutakahirano/f14f15bd1595e1e913b0870649000470 look?

It is going to be a patch to https://mikewest.github.io/corpp

@annevk
Copy link
Member

annevk commented Dec 12, 2019

  1. I think we should use the Reporting API infrastructure, though the amended Reporting API as agreed upon at the last TPAC. Not sure what the status of that is. @clelland @yoavweiss?
  2. I'm not sure I understand why or how respondWith() would be blocked. (I've seen the discussion for match()/matchAll() at cache.match() and COEP w3c/ServiceWorker#1490.)
  3. If you point to a same-origin resource that redirects to a cross-origin resource without CORP, what URL ends up being reported? (Note https://fetch.spec.whatwg.org/#atomic-http-redirect-handling.)

@yutakahirano
Copy link
Member Author

Thank you!

I'm not sure I understand why or how respondWith() would be blocked. (I've seen the discussion for match()/matchAll() at w3c/ServiceWorker#1490.)

This is the third item of https://mikewest.github.io/corpp/#integration-fetch.

If you point to a same-origin resource that redirects to a cross-origin resource without CORP, what URL ends up being reported? (Note https://fetch.spec.whatwg.org/#atomic-http-redirect-handling.)

Good point. I'm fine with the original URL.

@annevk
Copy link
Member

annevk commented Dec 12, 2019

Okay, so you want to distinguish between the service worker handing you a faulty response and the server handing you a faulty response, but no changes to respondWith() itself are planned? I guess that would help debugging, though architecturally it doesn't seem great to expose the existence of service workers.

cc @jakearchibald

@yutakahirano
Copy link
Member Author

Okay, so you want to distinguish between the service worker handing you a faulty response and the server handing you a faulty response, but no changes to respondWith() itself are planned? I guess that would help debugging, though architecturally it doesn't seem great to expose the existence of service workers.

I'm not sure I understand the comment. This happens when service worker has COEP: none and the page has COEP: require-corp, and it is tested in https://github.com/web-platform-tests/wpt/blob/master/html/cross-origin-embedder-policy/none-sw-from-require-corp.https.html.

@annevk
Copy link
Member

annevk commented Dec 12, 2019

To restate, I'm wondering why we would want to treat that differently from how we report subresource or nested navigation failures?

@yutakahirano
Copy link
Member Author

There are some cases where the network is not directly related. For example, a COEP: none service worker can get an opaque (CORP: none) response via the cache API and pass it to a COEP: require-corp page.

@annevk
Copy link
Member

annevk commented Dec 12, 2019

Understood, but why treat them differently from a network failure? To fetch it's all the same after all.

@yutakahirano
Copy link
Member Author

yutakahirano commented Dec 12, 2019

Ah I see you are talking about having the logic in the Main fetch, right?

I would like to place "would be blocked due to COEP violation" logic as close as possible from a would-be blocking logic (the CORP check for subresource loading, for example), because otherwise "action would be blocked if global's embedder policy were "require-corp" would be difficult to evaluate.

Does it make sense?

@annevk
Copy link
Member

annevk commented Dec 12, 2019

Agreed that they should be around the same place. I guess the thing that's unclear at the moment is where we will call https://fetch.spec.whatwg.org/#cross-origin-resource-policy-check from to account for service workers mattering due to the new cross-origin value. If it'll be called from two places or one.

@clelland
Copy link
Contributor

  1. I think we should use the Reporting API infrastructure, though the amended Reporting API as agreed upon at the last TPAC. Not sure what the status of that is. @clelland @yoavweiss?

I'm just working this week on splitting out all of the non-core bits from the spec. Spec updates should land in GitHub next week some time, I'd expect.

@yutakahirano
Copy link
Member Author

I updated the gist. PTAL.

@annevk
Copy link
Member

annevk commented Jan 9, 2020

As I understand it that doesn't match all the use cases we have. For COEP I think there's effectively four states, not three:

  1. No COEP.
  2. COEP.
  3. No COEP, but reporting when COEP is bypassed.
  4. COEP and reporting when COEP is bypassed.

At least @arturjanc and colleagues will always kindly remind me that reporting is useful both to find code that needs updating before the change is made (3) and monitor things afterwards (4).

So I think what you need to do is store the potential return value in step 7 of https://mikewest.github.io/corpp/#corp-check, introduce a step 8 to do reporting and potentially return allowed early if there's no COEP, and a step 9 to return the actual value.

Also, since Mike went with the design of including navigation in the CORP check algorithm, shouldn't we handle reporting for it there as well?

@yutakahirano
Copy link
Member Author

Ah, thank you.

Also, since Mike went with the design of including navigation in the CORP check algorithm, shouldn't we handle reporting for it there as well?

Sorry I was not aware of that. Can you point me the spec or the discussion?

@annevk
Copy link
Member

annevk commented Jan 9, 2020

I'm not sure there was discussion, but https://mikewest.github.io/corpp/#corp-check deals with mode "navigate" whereas https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header does not.

@yutakahirano
Copy link
Member Author

Thank you, updated. How does it look like?

@annevk
Copy link
Member

annevk commented Jan 10, 2020

I'm still not entirely sure why there's a separate algorithm for HTML. I was hoping we could enforce it solely in Fetch. Other than that I think it's reasonable.

@yutakahirano
Copy link
Member Author

@mikewest, can you take a look too? If you're fine with the general direction I'll make PRs, so at this moment no detailed review is needed.

Also, do you have any thoughts on @annevk's comment?

I'm still not entirely sure why there's a separate algorithm for HTML. I was hoping we could enforce it solely in Fetch.

yutakahirano added a commit to yutakahirano/corpp that referenced this issue Jan 23, 2020
This change introduces "reporting endpoint" concept which will be used as an
reporting endpoint for potential COEP violations.

Reporting behavior will be specified in future changes.

Discussed at whatwg/html#5100.
@yutakahirano
Copy link
Member Author

I made a PR for introducing the concept and parting the header. I'll make another PR for the reporting behaviors.

WICG/cross-origin-embedder-policy#8

yutakahirano added a commit to yutakahirano/corpp that referenced this issue Jan 24, 2020
Queue a report when CORP see potential failures due to COEP.
Discussed at whatwg/html#5100.
mikewest pushed a commit to WICG/cross-origin-embedder-policy that referenced this issue Feb 19, 2020
This change introduces "reporting endpoint" concept which will be used as an
reporting endpoint for potential COEP violations.

Reporting behavior will be specified in future changes.

Discussed at whatwg/html#5100.
yutakahirano added a commit to yutakahirano/corpp that referenced this issue Feb 19, 2020
Queue a report when CORP see potential failures due to COEP.
Discussed at whatwg/html#5100.
yutakahirano added a commit to yutakahirano/corpp that referenced this issue Feb 19, 2020
Queue a report when CORP see potential failures due to COEP.
Discussed at whatwg/html#5100.
yutakahirano added a commit to yutakahirano/corpp that referenced this issue Mar 3, 2020
Queue a report when CORP see potential failures due to COEP.
Discussed at whatwg/html#5100.
yutakahirano added a commit to yutakahirano/corpp that referenced this issue Mar 3, 2020
Queue a report when CORP see potential failures due to COEP.
Discussed at whatwg/html#5100.
yutakahirano added a commit to WICG/cross-origin-embedder-policy that referenced this issue Mar 4, 2020
* Modify CORP for COEP reporting

Queue a report when CORP see potential failures due to COEP.
Discussed at whatwg/html#5100.

Co-Authored-By: Mike West <mike@mikewest.org>
@yutakahirano
Copy link
Member Author

Let me close this - we may introduce some more features but I think they can be discussed separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal.
Development

No branches or pull requests

3 participants