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

Add support for COOP navigation violation reporting #4133

Merged

Conversation

cdumez
Copy link
Contributor

@cdumez cdumez commented Sep 8, 2022

67d5c3c

Add support for COOP navigation violation reporting
https://bugs.webkit.org/show_bug.cgi?id=244936

Reviewed by Ryosuke Niwa and Brent Fulgham.

Add support for COOP navigation violation reporting:
- https://html.spec.whatwg.org/multipage/origin.html#coop-violation-navigation-to
- https://html.spec.whatwg.org/multipage/origin.html#coop-violation-navigation-from

* LayoutTests/TestExpectations:
Unskip navigation-reporting folder now that the feature is implemented.

* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/report-only-four-reports.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/report-only-from-unsafe-none.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/report-only-same-origin-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/report-only-same-origin.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-coop-navigated-opener.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-coop-navigated-opener.https.html:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-coop-navigated-popup.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-same-origin-allow-popups-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-same-origin-coep-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-same-origin-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-same-origin.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-unsafe-none-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-redirect-with-same-origin-allow-popups.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-redirect-with-same-origin-allow-popups.https.html:
Rebaseline tests that are now passing and merge the following upstream fix:
- web-platform-tests/wpt#35824

* Source/WebCore/Modules/reporting/ReportingClient.h:
* Source/WebCore/Modules/reporting/ViolationReportType.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::httpUserAgent const):
* Source/WebCore/dom/Document.h:
* Source/WebCore/loader/CrossOriginOpenerPolicy.cpp:
(WebCore::crossOriginOpenerPolicyValueToEffectivePolicyString):
(WebCore::createViolationReportObject):
(WebCore::sendViolationReportWhenNavigatingToCOOPResponse):
(WebCore::sendViolationReportWhenNavigatingAwayFromCOOPResponse):
(WebCore::enforceResponseCrossOriginOpenerPolicy):
(WebCore::doCrossOriginOpenerHandlingOfResponse):
* Source/WebCore/loader/CrossOriginOpenerPolicy.h:
* Source/WebCore/loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::doCrossOriginOpenerHandlingOfResponse):
* Source/WebCore/loader/PingLoader.cpp:
(WebCore::PingLoader::sendViolationReport):
(WebCore::PingLoader::startPingLoad):
* Source/WebCore/workers/WorkerGlobalScope.h:
* Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions):
(WebKit::NetworkResourceLoader::doCrossOriginOpenerHandlingOfResponse):
(WebKit::NetworkResourceLoader::didReceiveResponse):
(WebKit::NetworkResourceLoader::initializeReportingEndpoints):
(WebKit::NetworkResourceLoader::didRetrieveCacheEntry):
* Source/WebKit/NetworkProcess/NetworkResourceLoader.h:
* Source/WebKit/Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<RefPtr<WebCore::ReportBody>>::encode):
(IPC::ArgumentCoder<RefPtr<WebCore::ReportBody>>::decode):

Canonical link: https://commits.webkit.org/254291@main

4786e18

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe loading πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv   πŸ§ͺ mac-wk1
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch   πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch-sim βœ… πŸ§ͺ mac-wk2-stress

@cdumez cdumez requested a review from rniwa as a code owner September 8, 2022 16:19
@cdumez cdumez self-assigned this Sep 8, 2022
@cdumez cdumez added WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). WebKit Nightly Build labels Sep 8, 2022
@@ -1,4 +1,4 @@

FAIL coop reporting test A test with both COOP and COOP report only setup to CROSS_ORIGIN with same-origin-allow-popups; report-to="coop-popup-report-endpoint", require-corp, same-origin; report-to="coop-popup-report-only-endpoint", require-corp promise_test: Unhandled rejection with value: "No report matched the expected report for endpoint: coop-report-endpoint, expected report: {\"body\":{\"disposition\":\"enforce\",\"effectivePolicy\":\"same-origin-allow-popups\",\"nextResponseURL\":\"/uuid=(uuid)$/\",\"type\":\"navigation-from-response\"},\"url\":\"https://localhost:9443/html/cross-origin-opener-policy/reporting/navigation-reporting/report-only-four-reports.https.html\",\"type\":\"coop\"}, within available reports: []"
PASS coop reporting test A test with both COOP and COOP report only setup to CROSS_ORIGIN with same-origin-allow-popups; report-to="coop-popup-report-endpoint", require-corp, same-origin; report-to="coop-popup-report-only-endpoint", require-corp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-D

Copy link
Contributor

@brentfulgham brentfulgham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ryosuke already approved, but so do I!

Source/WebCore/loader/PingLoader.cpp Show resolved Hide resolved
@@ -186,7 +191,7 @@ void PingLoader::sendViolationReport(Frame& frame, const URL& reportURL, Ref<For

HTTPHeaderMap originalRequestHeader = request.httpHeaderFields();

if (reportType != ViolationReportType::StandardReportingAPIViolation && reportType != ViolationReportType::Test)
if (reportType != ViolationReportType::StandardReportingAPIViolation && reportType != ViolationReportType::Test && reportType != ViolationReportType::CrossOriginOpenerPolicy)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should check check for equality with the CSP ViolationReportType, unless we have other types that need this behavior.

@@ -215,7 +220,7 @@ void PingLoader::startPingLoad(Frame& frame, ResourceRequest& request, HTTPHeade
options.cache = FetchOptions::Cache::NoCache;

// https://www.w3.org/TR/reporting/#try-delivery
if (violationReportType == ViolationReportType::StandardReportingAPIViolation || violationReportType == ViolationReportType::Test) {
if (violationReportType == ViolationReportType::StandardReportingAPIViolation || violationReportType == ViolationReportType::Test || violationReportType == ViolationReportType::CrossOriginOpenerPolicy) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@cdumez cdumez added the merge-queue Applied to send a pull request to merge-queue label Sep 8, 2022
@webkit-ews-buildbot webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Sep 8, 2022
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Sep 8, 2022
@cdumez cdumez added the merge-queue Applied to send a pull request to merge-queue label Sep 9, 2022
https://bugs.webkit.org/show_bug.cgi?id=244936

Reviewed by Ryosuke Niwa and Brent Fulgham.

Add support for COOP navigation violation reporting:
- https://html.spec.whatwg.org/multipage/origin.html#coop-violation-navigation-to
- https://html.spec.whatwg.org/multipage/origin.html#coop-violation-navigation-from

* LayoutTests/TestExpectations:
Unskip navigation-reporting folder now that the feature is implemented.

* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/report-only-four-reports.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/report-only-from-unsafe-none.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/report-only-same-origin-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/report-only-same-origin.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-coop-navigated-opener.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-coop-navigated-opener.https.html:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-coop-navigated-popup.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-same-origin-allow-popups-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-same-origin-coep-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-same-origin-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-same-origin.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-popup-unsafe-none-report-to.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-redirect-with-same-origin-allow-popups.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/reporting/navigation-reporting/reporting-redirect-with-same-origin-allow-popups.https.html:
Rebaseline tests that are now passing and merge the following upstream fix:
- web-platform-tests/wpt#35824

* Source/WebCore/Modules/reporting/ReportingClient.h:
* Source/WebCore/Modules/reporting/ViolationReportType.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::httpUserAgent const):
* Source/WebCore/dom/Document.h:
* Source/WebCore/loader/CrossOriginOpenerPolicy.cpp:
(WebCore::crossOriginOpenerPolicyValueToEffectivePolicyString):
(WebCore::createViolationReportObject):
(WebCore::sendViolationReportWhenNavigatingToCOOPResponse):
(WebCore::sendViolationReportWhenNavigatingAwayFromCOOPResponse):
(WebCore::enforceResponseCrossOriginOpenerPolicy):
(WebCore::doCrossOriginOpenerHandlingOfResponse):
* Source/WebCore/loader/CrossOriginOpenerPolicy.h:
* Source/WebCore/loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::doCrossOriginOpenerHandlingOfResponse):
* Source/WebCore/loader/PingLoader.cpp:
(WebCore::PingLoader::sendViolationReport):
(WebCore::PingLoader::startPingLoad):
* Source/WebCore/workers/WorkerGlobalScope.h:
* Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions):
(WebKit::NetworkResourceLoader::doCrossOriginOpenerHandlingOfResponse):
(WebKit::NetworkResourceLoader::didReceiveResponse):
(WebKit::NetworkResourceLoader::initializeReportingEndpoints):
(WebKit::NetworkResourceLoader::didRetrieveCacheEntry):
* Source/WebKit/NetworkProcess/NetworkResourceLoader.h:
* Source/WebKit/Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<RefPtr<WebCore::ReportBody>>::encode):
(IPC::ArgumentCoder<RefPtr<WebCore::ReportBody>>::decode):

Canonical link: https://commits.webkit.org/254291@main
@webkit-commit-queue
Copy link
Collaborator

Committed 254291@main (67d5c3c): https://commits.webkit.org/254291@main

Reviewed commits have been landed. Closing PR #4133 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 67d5c3c into WebKit:main Sep 9, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 9, 2022
@cdumez cdumez deleted the coop_reporting_support branch September 9, 2022 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
6 participants