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

Feature: Trusted Types support #64975

Merged
merged 44 commits into from
Apr 27, 2023
Merged

Feature: Trusted Types support #64975

merged 44 commits into from
Apr 27, 2023

Conversation

KristianGrafana
Copy link
Contributor

@KristianGrafana KristianGrafana commented Mar 17, 2023

To try this PR, enable the feature flag trustedTypes.

Trusted Types is security mechanism that locks down DOM API's that often provides sinks that are used when exploiting DOM XSS. By using trusted types you will force the string to be handled (preferably sanitized) before the string can be used by the API. Trusted types provides a robust defense against DOM XSS. Trusted types is currently only supported in Chrome.

This PR shows a minimal working PoC that uses createHTML, createScript and createScriptURL. Currently each one of them uses a different method; sanitizing via a library (DOMPurify), inline sanitizing and do nothing.

Grafana uses many third-party libraries that we can not control, but we do have some first party code that we can change to either return a trusted type object or refactor the code to not use unsafe DOM API's. These will be documented in this PR.

  • Choose if the code should be refactored, sanitized through the "default" policy or create a new policy
  • Find 1st party code that uses dangerous API's

Added things

  1. Enable CSP for e2e tests
    • Add meta tag with CSP content in order to make it compatible with Cypress
    • Export replace function from Go middleware
  2. Add trusted types default policy
    • Initiliazed before jQuery and Angular is loaded
    • Limited sanitization in order to make Angular work
  3. Add sanitization functions
    • Returns TrustedHTML, which is trusted types compatible
    • Extend DOMParser type to support TrustedHTML

How to use

Enable Trusted Types without implementation (to see errors)

  1. Enable content_security_policy_report_only = true
  2. Add require-trusted-types-for 'script'; to content_security_policy_report_only_template
  3. Set disable_sanitize_html = true in order to see errors

Test implementation

  1. Set feature flag trustedTypes = true
  2. See no errors and removed HTML

@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration .github/workflows/pr-codeql-analysis-javascript.yml:analyze. As part of the setup process, we have scanned this repository and found 30 existing alerts. Please check the repository Security tab to see all alerts.

@KristianGrafana
Copy link
Contributor Author

KristianGrafana commented Mar 20, 2023

@ryantxu I pushed a small change to feed.ts with a custom policy that does nothing just to suppress TT violation. It works, but I wonder if //@ts-ignore is OK to use here? TS throws exception when parseFromString process anything other than a string.

@ryantxu
Copy link
Member

ryantxu commented Mar 20, 2023

//@ts-ignore is fine -- before merge, we can evaluate which ones are actually required

@KristianGrafana
Copy link
Contributor Author

@ryantxu I've refactored the code so we can reach sanitizett() in the global window object which will sanitize using DOMPurify and return a TrustedHTML object. This seems to work fairly OK, but there are still issues with race condition which I need help with solving. So for context:

The default Trusted Type policy will be used for sinks when no policy has been defined. In this PR we can see that feed.ts has a policy (atom) defined, however, there is an issue where the default policy will be used instead of atom -- I don't understand why and someone with more insight into how Grafana does might know why :)

@tskarhed tskarhed mentioned this pull request Mar 30, 2023
7 tasks
@tskarhed tskarhed requested a review from chri2547 as a code owner April 18, 2023 15:09
@tskarhed
Copy link
Contributor

We might do the News/rss sanitization in another PR.

tskarhed and others added 3 commits April 19, 2023 11:21
Comment on lines 33 to 35
CSPContent string
CSPEnabled bool
TestModeEnabled bool
Copy link
Member

Choose a reason for hiding this comment

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

Do we need all three? Why send CSPContent to the frontend if it is not enabled?

Can we derive TrustedTypesEnabled from the CSP content? IIUC, it would be invalid we say trusted types are enabled, but the CSP content does not (and the reverse)

Copy link
Collaborator

@chri2547 chri2547 left a comment

Choose a reason for hiding this comment

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

lgtm

@tskarhed
Copy link
Contributor

  • Removed feature flag, and only enable trusted types default policy when it is the CSP content'
  • Add custom logging for report only mode
  • Only insert the CSP content in HTML if it is in development mode
  • Update docs

@chri2547 chri2547 removed their request for review April 27, 2023 15:48
@tskarhed tskarhed merged commit 3540714 into main Apr 27, 2023
@tskarhed tskarhed deleted the feat-trusted-types branch April 27, 2023 16:20
@zerok zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants