Cross origin opener policy reporting #5518

camillelamy · 2020-05-07T12:39:07Z

Adds the notion of reporting and report-only mode to cross-origin opener policy.

At least two implementers are interested (and none opposed):
- Chrome
- Firefox
Tests are written and can be reviewed and commented upon at:
- https://github.com/web-platform-tests/wpt/tree/master/html/cross-origin-opener-policy/reporting
Implementation bugs are filed:
- Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=1059303
- Firefox: …
- Safari: …

/browsers.html ( diff )
/browsing-the-web.html ( diff )
/dom.html ( diff )
/iana.html ( diff )
/index.html ( diff )
/origin.html ( diff )
/window-object.html ( diff )

camillelamy · 2020-06-17T16:36:50Z

@annevk and @domenic: could you take a look at this spec PR? This is a tentative to spec the first parts of COOP reporting policy. I have been working on an explainer that details what we have in mind (see https://github.com/camillelamy/explainers/blob/master/coop_reporting.md). This spec PR adds:

the header definition
the modification of the COOP definition to include reporting
the reporting of browsing context group switches

What it doesn't add is:

the reporting of blocked accesses to/from the COOP page

This is because we are still discussing the feasibility of reporting the accesses in enforcement mode. We're reasonably convinced we have a way forward in report-only mode though, so I can add report of the accesses from the page in report only mode to this PR if you want (I though it looked a bit weird to have it in report-only mode only though, so this is why I didn't add them initially).

domenic · 2020-06-19T17:52:09Z

At a quick glance this looks reasonable, but it is a bit hard to review since it also contains most of the commits of #5334. I can do a more in-depth review after #5334 lands, and ideally also #5454 since that contains some reporting stuff.

camillelamy · 2020-06-25T09:55:02Z

@domenic Thanks! Now that COOP has been merged, this should be more readable.

domenic · 2020-06-25T20:42:04Z

Great! Would you mind rebasing one more time, on top of the recently-merged COEP work?

camillelamy · 2020-06-26T13:05:44Z

@domenic I have rebased on the COEP work. Note that this PR does not cover the access reporting. Do you want me to put this in this PR or do a follow-up?

domenic · 2020-06-26T20:40:27Z

I'm OK either way; please do whichever is more convenient for you. (I can see separate PRs being easier to focus on and being able to land more independently, but also can see the dependency causing problems.)

domenic

I did a mostly-editorial pass, but overall it's looking quite solid.

source

All of this is put behind a flag disabled by default. This is mostly based on the initial prototype: https://chromium-review.googlesource.com/c/chromium/src/+/2223934/24 This patches list all the potential accesses to be checked and reported. The CrossOriginOpenerPolicyReporter is preparing itself to install all the CoopAccessMonitor to the renderer(s), but It doesn't send them for now. To write a meaningful patch, this will need accesses to COOP-Report-Only headers and the virtual browsing context group. Explainer [WIP]: https://github.com/camillelamy/explainers/blob/master/coop_reporting.md Specification [WIP]: whatwg/html#5518 Tests [WIP]: https://wpt.fyi/results/html/cross-origin-opener-policy/access-reporting Doc [WIP]: https://docs.google.com/document/d/1H8Be0w27fKPXKqyuJj9oEqIJEjB9Rw5AP3x-w-Fx2Zg/edit?usp=sharing Bug: chromium:1090273 Change-Id: I5d4c613a671f99ba15b4b174431d8d3ddeaa44c6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264294 Reviewed-by: Camille Lamy <[email protected]> Reviewed-by: Pâris Meuleman <[email protected]> Commit-Queue: Arthur Sonzogni <[email protected]> Cr-Commit-Position: refs/heads/master@{#783972}

camillelamy

Thanks!

source

domenic

Alright; I did another editorial pass. I think I understand at least everything in the "Cross-origin opener policies" section now, and it looks good.

The rest of it appears to be mostly plumbing through the navigate algorithm. (I'm sorry that's such a pain; we should really wrap all that stuff up into a single struct one day...) I haven't checked it in detail yet, but can later; I know it's easy to mismatch arguments accidentally.

Otherwise, everything looks right. Two potentially-substantive suggestions:

It seems like "check if a response requires a browsing context group switch" could return a COOP enforcement result, instead of mutating one that was passed to it. This would be slightly easier to follow, as then it'd be clear that the enforcement result is a logical output of that algorithm, instead of an input. The navigation algorithm would still need to initialize it where it currently does (step 6-ish), but step 9.6.6.3 could overwrite coopEnforcementResult instead of passing it in.
I noticed that, unlike COEP, you don't pass an environment settings object when queuing a report. From what I can tell, the main impact of this is that these COOP reports wont work with ReportingObserver. I think that is intentional, based on the aside in https://github.com/camillelamy/explainers/blob/master/coop_reporting.md#emit-reports. However, it'd probably be good to add a note explaining why we don't support ReportingObserver for these cases... I have a guess, but having it spelled out in the spec would be helpful.

I think this is ready for a second set of eyes, so I'd love to get @annevk's review. (And perhaps he could investigate whether Mozilla's support of COOP generally extends to support for reporting?) It looks like of his concerns on https://github.com/camillelamy/explainers/issues, only camillelamy/explainers#6 is applicable to this set of reports, right?

This adds the support for report-only COOP triggered Browsing context group switches. During navigation, besides computing whether COOP triggers a BCG switch, this also computes if any of the report-only policies would also trigger a switch if they were effective, the resulting values are stored in booleans within |CrossOriginOpenerPolicyStatus|. This booleans are then used to trigger the navigation reports, and in a follow up to trigger access reports. Explainer [WIP]: https://github.com/camillelamy/explainers/blob/master/coop_reporting.md Specification [WIP]: whatwg/html#5518 Bug: 1099208 Change-Id: I2cb66ec5cdcd9d5b4658c28f0608bc6b52d0da6b

This adds the support for report-only COOP triggered Browsing context group switches. During navigation, besides computing whether COOP triggers a BCG switch, this also computes if any of the report-only policies would also trigger a switch if they were effective, the resulting values are stored in booleans within |CrossOriginOpenerPolicyStatus|. This booleans are then used to trigger the navigation reports, and in a follow up to trigger access reports. Explainer [WIP]: https://github.com/camillelamy/explainers/blob/master/coop_reporting.md Specification [WIP]: whatwg/html#5518 Bug: 1099208

…only Automatic update from web-platform-tests COOP: add reporting to redirects This CL allows reporting browsing context group switches triggered by redirects and updates the reports sent in this case to the latest version of the spec PR (whatwg/html#5518). Since the status of COOP was becoming hard to track, I moved it to its own class for better encapsulation. Bug: 1059303 Change-Id: Ifafb23073301bd05cd9ce83fdb0b748c28e8a51f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352880 Commit-Queue: Camille Lamy <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Reviewed-by: Mike West <[email protected]> Reviewed-by: Kinuko Yasuda <[email protected]> Cr-Commit-Position: refs/heads/master@{#799863} -- wpt-commits: 677c57c0e8816b0892cc3ae1c2772189b1bdcf65 wpt-pr: 25072

Fixup Thread through source Rework redirect handling Add access reporting Change report body names to camelCase

camillelamy · 2020-10-29T13:43:59Z

@domenic @annevk I have picked this up again after a few months of being unable to make progress. I have a new rebased version that integrates @domenic's comments. PTAL.

domenic

I pushed a commit with some typo and nit fixes. The only substantial question left is the use of null for the endpoints, and how that causes divergence (I think in behavior, as well as spec structure) from COEP.

source

camillelamy · 2020-11-04T17:10:14Z

@domenic PTAL. And also sorry, it seems I messed up with the github interface and my previous review comments were not sent.

domenic

Overall LGTM. What remains are the following nits (denoted above by unresolved comments, but centralized here in case it's helpful): two missing spaces, improving the precision for cross-origin accessible window property name, and changing the link for "group".

I believe there's been good progress on adding web platform tests for this feature as well, right? Can you update the OP to link to them, and comment on how complete you think the coverage is?

Finally, for multi-implementer interest, perhaps @annevk can comment for Firefox?

domenic · 2020-11-04T23:35:25Z

source

+       data-x="coop-enforcement-result">cross-origin opener policy enforcement result</span> whose
+       <span data-x="coop-enforcement-bcg-switch">needs a browsing context group switch</span> is
+       false, <span data-x="coop-enforcement-bcg-switch-report-only">would need a browsing context
+       group switch due to report-only</span> is false,<span


Missing space

domenic · 2020-11-04T23:35:29Z

source

+       data-x="coop-enforcement-result">cross-origin opener policy enforcement result</span> whose
+       <span data-x="coop-enforcement-bcg-switch">needs a browsing context group switch</span> is
+       false, <span data-x="coop-enforcement-bcg-switch-report-only">would need a browsing context
+       group switch due to report-only</span> is false,<span


Missing space

annevk · 2020-11-16T13:37:51Z

I think I said before that Firefox is supportive of reporting for COOP and COEP, though it's not a priority for us. (I also worry about the complexity this adds due to the unusual requirements of COOP reporting, but hopefully adequate test coverage, in particular for all the various origin checks, can mitigate some of that.)

camillelamy · 2020-11-19T13:45:12Z

Thanks Domenic! I should have fixed all remaining issues in the latest update, and updated the pull request. In terms of WPTs, I believe we have thorough testing for all base cases for access reporting, including the URLs to send in reports. For browsing context switch, we're at least missing tests for 2 base cases that I am trying to add right now. After that I think we should be good. We have generally tested each interesting part of the feature on its own, and haven't done combinatorial tests.

domenic · 2020-11-19T21:30:30Z

Awesome! I'll merge this momentarily, then. The test coverage sounds like it's definitely enough to merge the spec change, although of course more is always better.

sideshowbarker · 2020-12-08T05:43:22Z

cc @whatwg/documentation

This adds the support for report-only COOP triggered Browsing context group switches. During navigation, besides computing whether COOP triggers a BCG switch, this also computes if any of the report-only policies would also trigger a switch if they were effective, the resulting values are stored in booleans within |CrossOriginOpenerPolicyStatus|. This booleans are then used to trigger the navigation reports, and in a follow up to trigger access reports. Explainer [WIP]: https://github.com/camillelamy/explainers/blob/master/coop_reporting.md Specification [WIP]: whatwg/html#5518 Bug: 1099208 Change-Id: I2cb66ec5cdcd9d5b4658c28f0608bc6b52d0da6b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2241529 Commit-Queue: Pâris Meuleman <[email protected]> Reviewed-by: Camille Lamy <[email protected]> Reviewed-by: Arthur Hemery <[email protected]> Auto-Submit: Pâris Meuleman <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#790781} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 9c2ef27118d4009c5fa8659a04b74b48aa0ca574

This CL allows reporting browsing context group switches triggered by redirects and updates the reports sent in this case to the latest version of the spec PR (whatwg/html#5518). Since the status of COOP was becoming hard to track, I moved it to its own class for better encapsulation. Bug: 1059303 Change-Id: Ifafb23073301bd05cd9ce83fdb0b748c28e8a51f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352880 Commit-Queue: Camille Lamy <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Reviewed-by: Mike West <[email protected]> Reviewed-by: Kinuko Yasuda <[email protected]> Cr-Commit-Position: refs/heads/master@{#799863} GitOrigin-RevId: 9f52c7e47ed28cba6ba6a1fba302a9665f27fa36

camillelamy force-pushed the cross-origin-opener-policy-reporting branch 4 times, most recently from 34050e4 to d6a428d Compare May 7, 2020 13:56

annevk mentioned this pull request May 11, 2020

Cross-origin opener policy reporting #5517

Closed

3 tasks

annevk added the topic: cross-origin-opener-policy Issues and ideas around the new "inverse of rel=noopener" header label May 11, 2020

camillelamy force-pushed the cross-origin-opener-policy-reporting branch 3 times, most recently from 753cd49 to 25e8501 Compare June 16, 2020 15:19

camillelamy force-pushed the cross-origin-opener-policy-reporting branch from 25e8501 to a7e5435 Compare June 17, 2020 16:27

camillelamy marked this pull request as ready for review June 17, 2020 16:36

camillelamy force-pushed the cross-origin-opener-policy-reporting branch from a7e5435 to e38bc93 Compare June 25, 2020 09:53

camillelamy force-pushed the cross-origin-opener-policy-reporting branch from e38bc93 to 69d0e7f Compare June 26, 2020 13:04

domenic reviewed Jun 26, 2020

View reviewed changes

camillelamy force-pushed the cross-origin-opener-policy-reporting branch from 69d0e7f to 0f03172 Compare July 2, 2020 13:25

camillelamy commented Jul 2, 2020

View reviewed changes

domenic force-pushed the cross-origin-opener-policy-reporting branch from f64abe3 to 6cca0fc Compare July 6, 2020 20:29

domenic reviewed Jul 6, 2020

View reviewed changes

chromium-wpt-export-bot mentioned this pull request Jul 10, 2020

[COOP report] Add support for report-only web-platform-tests/wpt#24552

Merged

domenic mentioned this pull request Jul 10, 2020

Add some sort of navigation-params container to stop threading stuff through navigation-related algorithms #5727

Closed

clamy and others added 11 commits October 28, 2020 11:53

Add cross-origin opener policy reporting API

e33ea5e

Fixup Thread through source Rework redirect handling Add access reporting Change report body names to camelCase

Change to virtual browsing context group ID

135f904

Changes to browsing context initial URL

d755763

Change link destination

85f285c

Small changes in COOP definition

0e5310b

Change struct initialization

2959d45

Change queue calls

47a9653

Small changes

2893f75

Change access reporting

3dc1772

Add origin to navigation reports algorithm

3b9c0a1

Change way we get script, line and column

7c921fc

camillelamy force-pushed the cross-origin-opener-policy-reporting branch from a673e90 to 7c921fc Compare October 29, 2020 11:23

Typo and nit fixes

dd47f00

domenic reviewed Oct 30, 2020

View reviewed changes

Address Domenic's comments

23cac25

domenic approved these changes Nov 4, 2020

View reviewed changes

Address Domenic's comments

bb96255

Tweak to COA window property definition

c8265a7

domenic merged commit 0666f4e into whatwg:master Nov 19, 2020

sideshowbarker added the impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation label Dec 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cross origin opener policy reporting #5518

Cross origin opener policy reporting #5518

camillelamy commented May 7, 2020 •

edited by pr-preview bot

Loading

camillelamy commented Jun 17, 2020

domenic commented Jun 19, 2020

camillelamy commented Jun 25, 2020

domenic commented Jun 25, 2020

camillelamy commented Jun 26, 2020

domenic commented Jun 26, 2020

domenic left a comment

camillelamy left a comment

domenic left a comment

camillelamy commented Oct 29, 2020

domenic left a comment

camillelamy commented Nov 4, 2020

domenic left a comment

domenic Nov 4, 2020

camillelamy Nov 19, 2020

domenic Nov 4, 2020

camillelamy Nov 19, 2020

annevk commented Nov 16, 2020

camillelamy commented Nov 19, 2020

domenic commented Nov 19, 2020

sideshowbarker commented Dec 8, 2020

Cross origin opener policy reporting #5518

Cross origin opener policy reporting #5518

Conversation

camillelamy commented May 7, 2020 • edited by pr-preview bot Loading

camillelamy commented Jun 17, 2020

domenic commented Jun 19, 2020

camillelamy commented Jun 25, 2020

domenic commented Jun 25, 2020

camillelamy commented Jun 26, 2020

domenic commented Jun 26, 2020

domenic left a comment

Choose a reason for hiding this comment

camillelamy left a comment

Choose a reason for hiding this comment

domenic left a comment

Choose a reason for hiding this comment

camillelamy commented Oct 29, 2020

domenic left a comment

Choose a reason for hiding this comment

camillelamy commented Nov 4, 2020

domenic left a comment

Choose a reason for hiding this comment

domenic Nov 4, 2020

Choose a reason for hiding this comment

camillelamy Nov 19, 2020

Choose a reason for hiding this comment

domenic Nov 4, 2020

Choose a reason for hiding this comment

camillelamy Nov 19, 2020

Choose a reason for hiding this comment

annevk commented Nov 16, 2020

camillelamy commented Nov 19, 2020

domenic commented Nov 19, 2020

sideshowbarker commented Dec 8, 2020

camillelamy commented May 7, 2020 •

edited by pr-preview bot

Loading