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.

Sign up for GitHub

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

Jump to bottom

Permissions Policy Reporting #186892

Merged
merged 12 commits into from
Jul 4, 2024
Merged

Permissions Policy Reporting #186892

merged 12 commits into from
Jul 4, 2024

Conversation

elena-shostak
Copy link
Contributor

@elena-shostak elena-shostak commented Jun 25, 2024 •
edited
Loading

Summary

  1. Added top-level permissionsPolicy configuration setting.
  2. Added support for report_to directive.
  3. Added support for Permissions-Policy-Report-Only header to enable reporting mode.
  4. The spec mentions featureId in the reporting body, however the field is policyId in Chromium.

How to test

  • Add in your kibana.dev.yml.
server.customResponseHeaders.Reporting-Endpoints: violations-endpoint="https://localhost:5601/kibana/internal/security/analytics/_record_violations"
server.securityResponseHeaders.permissionsPolicy: 'microphone=()'
server.securityResponseHeaders.permissionsPolicyReportOnly: 'camera=()'
permissionsPolicy.report_to: [violations-endpoint]
  • Make sure you have dev tools configured for Reporting API.
  • In the browser console invoke navigator.mediaDevices.getUserMedia({ audio: true, video: true }).catch((e) => {});
  • Open Dev Tools -> Application -> Reporting API.
    You should see 2 reports for permissions violation, one with report disposition and another with enforce disposition.
Screenshot 2024-06-27 at 13 36 12

Checklist

  • Documentation was added for features that require explanation or tutorials
  • Unit or functional tests were updated or added to match the most common scenarios
  • If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list

For maintainers

Related Issue(s)

#175113, #184939

Release Note

Added support for Permissions Policy reporting.

@elena-shostak
Copy link
Contributor Author

/ci

5 similar comments
@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak elena-shostak added the ci:cloud-deploy Create or update a Cloud deployment label Jun 27, 2024
@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak elena-shostak added the ci:cloud-redeploy Always create a new Cloud deployment label Jun 27, 2024
@elena-shostak
Copy link
Contributor Author

/ci

1 similar comment
@elena-shostak
Copy link
Contributor Author

/ci

elena-shostak
elena-shostak commented Jun 27, 2024
View reviewed changes
x-pack/plugins/security/server/routes/analytics/record_violations.ts Outdated
@@ -91,7 +91,7 @@ export const permissionsPolicyViolationReportSchema = schema.object(
/**
* The string identifying the policy-controlled feature whose policy has been violated. This string can be used for grouping and counting related reports.
*/
featureId: schema.string(),
policyId: schema.string(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably @azasypkin could use your advice here, the spec mentions featureId, however the report that is sent from Chrome (my version is 126.0.6478.127) has policyId, not featureId. Should we support both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added support for both ✅

@elena-shostak
Copy link
Contributor Author

@elasticmachine merge upstream

@elena-shostak
Copy link
Contributor Author

/ci

1 similar comment
@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak elena-shostak added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Security/CSP Platform Security - Content Security Policy release_note:skip Skip the PR/issue when compiling release notes release_note:enhancement and removed ci:cloud-deploy Create or update a Cloud deployment ci:cloud-redeploy Always create a new Cloud deployment release_note:skip Skip the PR/issue when compiling release notes labels Jun 27, 2024
@kibana-ci
Copy link
Collaborator

kibana-ci commented Jun 27, 2024 •
edited
Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/core-http-server-internal 79 82 +3
Unknown metric groups

API count

id before after diff
@kbn/core-http-server-internal 92 95 +3

History

  • 💔 Build #218209 failed d9590276ebb168722e48bd04af6b342ca1c2bdd3
  • 💛 Build #218180 was flaky 61bb5d45950a1615b829fdecea959adc01fdc8c1
  • 💛 Build #217905 was flaky 40710144d3561e62fe25bda8513ef92ad512f977
  • 💔 Build #217844 failed f63a745edcd844e30dff600869055a3b347c8367
  • 💔 Build #217727 failed 82eb5ae91bec61d07539637c69711deb5c74deec

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@elena-shostak elena-shostak marked this pull request as ready for review June 28, 2024 09:00
@elena-shostak elena-shostak requested review from a team as code owners June 28, 2024 09:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

jbudz
jbudz approved these changes Jun 28, 2024
View reviewed changes
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

kibana-docker

pgayvallet
pgayvallet approved these changes Jul 2, 2024
View reviewed changes
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM

packages/core/http/core-http-server-internal/src/http_config.ts
Comment on lines 345 to +348
rawHttpConfig: HttpConfigType,
rawCspConfig: CspConfigType,
rawExternalUrlConfig: ExternalUrlConfig
rawExternalUrlConfig: ExternalUrlConfig,
rawPermissionsPolicyConfig: PermissionsPolicyConfigType
Copy link
Contributor

Choose a reason for hiding this comment

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

(thinking out loud, not directly related to this PR) I feel like this increasing accumulation of config segments we need to build the concrete http config is the sign of us forcing this artificial separation without good reasons. At some point we might want to regroup everything properly under the same server prefix (or, let's be crazy, a new http prefix) to break this.

@jeramysoucy jeramysoucy self-requested a review July 3, 2024 11:51
jeramysoucy
jeramysoucy approved these changes Jul 3, 2024
View reviewed changes
Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

LGTM! Just a question about the Permissions-Policy-Report-Only link in the settings documentation, as I didn't see this specific header mentioned.

docs/setup/settings.asciidoc
Comment on lines +431 to +432
experimental[] Controls whether the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Permissions-Policy[`Permissions-Policy-Report-Only`] header
is used in all responses to the client from the {kib} server, and specifies what value is used. Allowed values are any text value or `null`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't find any documentation specifically on the Permissions-Policy-Report-Only header. Is the link incorrect, or is this not explicitly documented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, it is not explicitly documented on mdn. I can link it to the spec https://w3c.github.io/webappsec-permissions-policy/#permissions-policy-report-only-http-header-field (but this is editor's draft)

@elena-shostak elena-shostak merged commit cc50c8d into elastic:main Jul 4, 2024
19 checks passed
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels Jul 4, 2024
@legrego legrego linked an issue Jul 29, 2024 that may be closed by this pull request
Enable reporting for Permissions Policy violations #184664
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pgayvallet pgayvallet pgayvallet approved these changes

@jbudz jbudz jbudz approved these changes

@jeramysoucy jeramysoucy jeramysoucy approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Security/CSP Platform Security - Content Security Policy release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v8.16.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable reporting for Permissions Policy violations
7 participants
@elena-shostak @kibana-ci @elasticmachine @pgayvallet @jbudz @jeramysoucy @kibanamachine