Modify CORP for COEP reporting #9

yutakahirano · 2020-02-19T11:19:38Z

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

yutakahirano · 2020-02-19T11:19:51Z

@mikewest @annevk

yutakahirano · 2020-02-24T12:45:46Z

ping

yutakahirano · 2020-02-27T05:39:59Z

ping

yutakahirano · 2020-02-27T12:38:27Z

I started implementation, and https://chromium-review.googlesource.com/c/chromium/src/+/2076223 has web platform tests.

1: https://crrev.com/c/2074177 2: https://crrev.com/c/2075002 3: [this] This series of CLs implements WICG/cross-origin-embedder-policy#9. We introduce network::mojom::CrossOriginEmbedderPolicyReporter and its implementation content::CrossOriginEmbedderPolicyReporter, implement the reporting logic in content::CrossOriginEmbedderPolicyReporter and the CORP check, and plumb the mojo interface. This CL creates CrossOriginEmbedderPolicyReport during the frame navigation and dedicated worker initialization, and give it to the network service so that (possibly potential) CORP blocks are reported. Bug: 1052764 Change-Id: Ia39ff8277eb23d96025f5e6fba4e5a4fa6ffde70

mikewest

First pass. Sorry this has taken so long. I'll try to get to the others today...

index.bs

mikewest · 2020-03-03T08:14:57Z

index.bs

+
+    1. Let |blocked url| be the first URL of |request|'s [=request/URL list=].
+
+    2. Set |blocked url|'s [=url/username=] to the empty string, and its [=url/password=] to `null`.


Nit: I wonder if we should add some mechanism to the URL serializer to exclude credentials, as we do this kind of thing in a few places. Not for this PR, obviously...

index.bs

mikewest · 2020-03-03T08:19:14Z

index.bs

+
+    4. Let |body| be a new object containing the following properties with keys:
+
+        * key: "`blocked`", value: |serialized blocked url|.


Is this enough context? It's not clear to me what @arturjanc and co. actually need (nor do I recall off the top of my head what the Reporting API provides on its own... Is https://w3c.github.io/reporting/#try-delivery up to date?).

It looks like the default url value provided by the Reporting API and the blocked URL here would be sufficient for developers to debug CORP violations.

BTW, I'm not sure if it's intentional but CSP uses blocked-uri as the key for resources that didn't load (as opposed to blocked here), so maybe there's some value in making this consistent?

Another value that could potentially be useful is Request.destination (so the developer would know what kind of resource didn't load, e.g. they could prioritize fixing scripts) but this is a more of a nice-to-have than a necessity for debugging.

Nit: blocked-uri is deprecated. blocked-url (with an L, not an I) is preferred. :)

(I suspect @domenic would prefer that we follow the suggestions in https://w3ctag.github.io/design-principles/#casing-rules, using _ instead of -. Unfortunately, CSP is old and crusty and predates those good ideas.)

index.bs

mikewest

LGTM % the notes here. I think we can land this without waiting for @arturjanc, as we can always add bits and pieces later, and getting the infrastructure in place is more pressing.

index.bs

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

mikewest

LGTM. Thanks!

I'd suggest that you just merge this, and discuss additional changes to the value of the report with @arturjanc, et al. in a separate PR that also introduces the COEPReportBody interface.

index.bs

mikewest · 2020-03-04T08:33:35Z

index.bs

+
+    4. Let |body| be a new object containing the following properties with keys:
+
+        * key: "`blocked`", value: |serialized blocked url|.


Nit: blocked-uri is deprecated. blocked-url (with an L, not an I) is preferred. :)

(I suspect @domenic would prefer that we follow the suggestions in https://w3ctag.github.io/design-principles/#casing-rules, using _ instead of -. Unfortunately, CSP is old and crusty and predates those good ideas.)

Co-Authored-By: Mike West <mike@mikewest.org>

yutakahirano · 2020-03-04T10:31:32Z

Done. Replaced "blocked" with "blocked-url", and added request's destination to the report.

yutakahirano · 2020-03-04T10:34:14Z

Oh sorry I overlooked your last comment. Removed the destinaion from the report.

I'd suggest that you just merge this

I think I don't have a commit access.

mikewest · 2020-03-04T10:39:19Z

I'd suggest that you just merge this

I think I don't have a commit access.

Ah. Let's fix that. Invite's waiting in your inbox!

yutakahirano · 2020-03-04T10:49:05Z

Thank you! Merged.

1: https://crrev.com/c/2074177 2: https://crrev.com/c/2075002 3: [this] This series of CLs implements WICG/cross-origin-embedder-policy#9. We introduce network::mojom::CrossOriginEmbedderPolicyReporter and its implementation content::CrossOriginEmbedderPolicyReporter, implement the reporting logic in content::CrossOriginEmbedderPolicyReporter and the CORP check, and plumb the mojo interface. This CL creates CrossOriginEmbedderPolicyReport during the frame navigation and dedicated worker initialization, and give it to the network service so that (possibly potential) CORP blocks are reported. Bug: 1052764 Change-Id: Ia39ff8277eb23d96025f5e6fba4e5a4fa6ffde70

1: https://crrev.com/c/2074177 2: https://crrev.com/c/2075002 3: [this] This series of CLs implements WICG/cross-origin-embedder-policy#9. We introduce network::mojom::CrossOriginEmbedderPolicyReporter and its implementation content::CrossOriginEmbedderPolicyReporter, implement the reporting logic in content::CrossOriginEmbedderPolicyReporter and the CORP check, and plumb the mojo interface. This CL creates CrossOriginEmbedderPolicyReport during the frame navigation and dedicated worker initialization, and give it to the network service so that (possibly potential) CORP blocks are reported. Bug: 1052764 Change-Id: Ia39ff8277eb23d96025f5e6fba4e5a4fa6ffde70 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076223 Commit-Queue: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#747176}

1: [this] 2: https://crrev.com/c/2075002 3: https://crrev.com/c/2076223 This series of CLs implements WICG/cross-origin-embedder-policy#9. We introduce network::mojom::CrossOriginEmbedderPolicyReporter and its implementation content::CrossOriginEmbedderPolicyReporter, implement the reporting logic in content::CrossOriginEmbedderPolicyReporter and the CORP check, and plumb the mojo interface. This CL introduces network::mojom::CrossOriginEmbedderPolicyReporter and its implementation content::CrossOriginEmbedderPolicyReporter. Bug: 1052764 Change-Id: I7ccce3e39c760393bf2d1b73786cf2a7ae838fde Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2074177 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/heads/master@{#747170}

1: https://crrev.com/c/2074177 2: [this] 3: https://crrev.com/c/2076223 This series of CLs implements WICG/cross-origin-embedder-policy#9. We introduce network::mojom::CrossOriginEmbedderPolicyReporter and its implementation content::CrossOriginEmbedderPolicyReporter, implement the reporting logic in content::CrossOriginEmbedderPolicyReporter and the CORP check, and plumb the mojo interface. This CL implements the reporting logic in the CORP check. Bug: 1052764 Change-Id: Ia3c4d3aec886c76be6dd32083809e9e447ce2a4f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2075002 Commit-Queue: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#747172}

1: https://crrev.com/c/2074177 2: https://crrev.com/c/2075002 3: [this] This series of CLs implements WICG/cross-origin-embedder-policy#9. We introduce network::mojom::CrossOriginEmbedderPolicyReporter and its implementation content::CrossOriginEmbedderPolicyReporter, implement the reporting logic in content::CrossOriginEmbedderPolicyReporter and the CORP check, and plumb the mojo interface. This CL creates CrossOriginEmbedderPolicyReport during the frame navigation and dedicated worker initialization, and give it to the network service so that (possibly potential) CORP blocks are reported. Bug: 1052764 Change-Id: Ia39ff8277eb23d96025f5e6fba4e5a4fa6ffde70 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076223 Commit-Queue: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#747176}

… a=testonly Automatic update from web-platform-tests Introduce COEP reporting for CORP (3/3) 1: https://crrev.com/c/2074177 2: https://crrev.com/c/2075002 3: [this] This series of CLs implements WICG/cross-origin-embedder-policy#9. We introduce network::mojom::CrossOriginEmbedderPolicyReporter and its implementation content::CrossOriginEmbedderPolicyReporter, implement the reporting logic in content::CrossOriginEmbedderPolicyReporter and the CORP check, and plumb the mojo interface. This CL creates CrossOriginEmbedderPolicyReport during the frame navigation and dedicated worker initialization, and give it to the network service so that (possibly potential) CORP blocks are reported. Bug: 1052764 Change-Id: Ia39ff8277eb23d96025f5e6fba4e5a4fa6ffde70 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076223 Commit-Queue: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#747176} -- wpt-commits: 0d183bf3945eda537d23f8d37afbde1dad982544 wpt-pr: 22005

… a=testonly Automatic update from web-platform-tests Introduce COEP reporting for CORP (3/3) 1: https://crrev.com/c/2074177 2: https://crrev.com/c/2075002 3: [this] This series of CLs implements WICG/cross-origin-embedder-policy#9. We introduce network::mojom::CrossOriginEmbedderPolicyReporter and its implementation content::CrossOriginEmbedderPolicyReporter, implement the reporting logic in content::CrossOriginEmbedderPolicyReporter and the CORP check, and plumb the mojo interface. This CL creates CrossOriginEmbedderPolicyReport during the frame navigation and dedicated worker initialization, and give it to the network service so that (possibly potential) CORP blocks are reported. Bug: 1052764 Change-Id: Ia39ff8277eb23d96025f5e6fba4e5a4fa6ffde70 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076223 Commit-Queue: Yutaka Hirano <yhiranochromium.org> Reviewed-by: Kinuko Yasuda <kinukochromium.org> Cr-Commit-Position: refs/heads/master{#747176} -- wpt-commits: 0d183bf3945eda537d23f8d37afbde1dad982544 wpt-pr: 22005 UltraBlame original commit: 928df8a4bbe0707f3da7ee6ebb8516f2eab4b142

yutakahirano mentioned this pull request Feb 21, 2020

Run the CORP check while processing navigation response #11

Merged

chromium-wpt-export-bot mentioned this pull request Feb 27, 2020

Introduce COEP reporting for CORP (3/3) web-platform-tests/wpt#22005

Merged

mikewest reviewed Mar 3, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

yutakahirano mentioned this pull request Mar 3, 2020

COEP: introduce IDL interfaces for reporting body whatwg/html#5328

Open

yutakahirano force-pushed the yhirano/report-to-corp branch from bf3484e to 873301b Compare March 3, 2020 12:01

Modify CORP for COEP reporting

9f178a7

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

yutakahirano force-pushed the yhirano/report-to-corp branch from 56dfab6 to 9f178a7 Compare March 3, 2020 12:57

mikewest approved these changes Mar 4, 2020

View reviewed changes

yutakahirano and others added 2 commits March 4, 2020 19:26

Update index.bs

bb2a635

Co-Authored-By: Mike West <mike@mikewest.org>

fix

4fc787a

fix

7989233

yutakahirano merged commit 73865bf into WICG:master Mar 4, 2020

yutakahirano deleted the yhirano/report-to-corp branch March 4, 2020 10:49

yutakahirano mentioned this pull request Mar 24, 2020

Add more information to COEP reporting whatwg/html#5391

Closed

domenic mentioned this pull request May 29, 2020

[COOP Reporting] Questions about the report body camillelamy/explainers#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify CORP for COEP reporting #9

Modify CORP for COEP reporting #9

yutakahirano commented Feb 19, 2020

yutakahirano commented Feb 19, 2020

yutakahirano commented Feb 24, 2020

yutakahirano commented Feb 27, 2020

yutakahirano commented Feb 27, 2020

mikewest left a comment

mikewest Mar 3, 2020

yutakahirano Mar 3, 2020

mikewest Mar 3, 2020

mikewest Mar 3, 2020

arturjanc Mar 3, 2020

arturjanc Mar 3, 2020

mikewest Mar 4, 2020

mikewest left a comment

mikewest left a comment

mikewest Mar 4, 2020

yutakahirano commented Mar 4, 2020

yutakahirano commented Mar 4, 2020

mikewest commented Mar 4, 2020

yutakahirano commented Mar 4, 2020


		1. Let \|blocked url\| be the first URL of \|request\|'s [=request/URL list=].

		2. Set \|blocked url\|'s [=url/username=] to the empty string, and its [=url/password=] to `null`.


		4. Let \|body\| be a new object containing the following properties with keys:

		* key: "`blocked`", value: \|serialized blocked url\|.

Modify CORP for COEP reporting #9

Modify CORP for COEP reporting #9

Conversation

yutakahirano commented Feb 19, 2020

yutakahirano commented Feb 19, 2020

yutakahirano commented Feb 24, 2020

yutakahirano commented Feb 27, 2020

yutakahirano commented Feb 27, 2020

mikewest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikewest left a comment

Choose a reason for hiding this comment

mikewest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yutakahirano commented Mar 4, 2020

yutakahirano commented Mar 4, 2020

mikewest commented Mar 4, 2020

yutakahirano commented Mar 4, 2020