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

docs(security): clarify CSP requirements and provide example TALISMAN_CONFIG #22711

Merged

Conversation

reidab
Copy link
Contributor

@reidab reidab commented Jan 13, 2023

SUMMARY

This is an attempt to provide some more clarity around the CSP requirements and to add a basic example TALISMAN_CONFIG.

I've been submitting docs updates for issues I hit during an initial deployment of Superset on Kubernetes. Upon deployment, I started seeing CSP warnings from the CONTENT_SECURITY_POLICY_WARNING setting in my logs, but the docs did not provide much information about what sort of CSP Superset requires in order to function.

I first tried a basic strict CSP of default-src: 'self'; object-src: 'none'. This resulted Superset's UI failing to run with a number of errors related to unsafe-inline and unsafe-eval. Adding those keywords to the basic policy allowed Superset's UI to operate in our deployment.

  • Does this capture all the necessary requirements for Superset's CSP? Are there other considerations that should be highlighted?
  • Is there any reason to break out individual CSP sections like script-src and style-src vs using default-src in the example? I'd favor simplicity as a starting point if there's no compelling reason to break them out.

TESTING INSTRUCTIONS

Read the new docs.

  • Does this seem like a reasonable description of the CSP requirements?
  • Is the example given a reasonable default configuration?

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@michael-s-molina
Copy link
Member

michael-s-molina commented Jan 13, 2023

Thank you for the PR @reidab!

Does this capture all the necessary requirements for Superset's CSP? Are there other considerations that should be highlighted?

Depending on the features one will use, there may be additional configurations:

  • Some dashboards use the data protocol to load images and therefore need "img-src": "'self' data:"
  • MapBox chart requires: "worker-src": "'self' blob:" and "connect-src": "'self' https://api.mapbox.com https://events.mapbox.com"

Is there any reason to break out individual CSP sections like script-src and style-src vs using default-src in the example? I'd favor simplicity as a starting point if there's no compelling reason to break them out.

You should break only if you need to (see examples above).

@rusackas rusackas requested a review from dpgaspar January 13, 2023 16:41
@reidab reidab force-pushed the docs-security-clarify-csp-requirements branch from 2bfea7b to 71a593a Compare January 13, 2023 18:47
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jan 13, 2023
@reidab
Copy link
Contributor Author

reidab commented Jan 13, 2023

@michael-s-molina thanks for the clarification and extra info! I've updated the branch to include all of these requirements.

Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

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

Thanks @reidab for the additional documentation!

@michael-s-molina michael-s-molina merged commit f9972ad into apache:master Jan 13, 2023
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants