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

Only allow additive CSP configuration #94414

Closed
joshdover opened this issue Mar 11, 2021 · 4 comments · Fixed by #102059
Closed

Only allow additive CSP configuration #94414

joshdover opened this issue Mar 11, 2021 · 4 comments · Fixed by #102059
Assignees
Labels
Breaking Change Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@joshdover
Copy link
Contributor

joshdover commented Mar 11, 2021

Right now, we allow users to configure the entire Content Security Policy via the csp.rules configuration. The tricky thing about using this config is that you also need to know Kibana's default CSP in order to get this right and working. For example, if I wanted to add a single URL to the script-src directive, I would need dig into Kibana's source code to find the default CSP rules and add this to my config:

csp.rules:
  - script-src 'unsafe-eval' 'self' https://mycdn.com/script.js
  # another gotcha is that yaml will parse `worker-src blob:` as an object key unless you include quotes here
  - "worker-src blob: 'self'"
  - style-src 'unsafe-inline' 'self'

This is fragile because Kibana may change the default CSP it needs to run which may require a change to the user's configuration. In effect, we could introduce a breaking change on accident by changing our default csp.rules.

We could change this to only allow additive changes to the CSP by users. For instance, we could allow a configuration like:

csp:
  script_src:
    - https://mycdn.com/script.js
  worker_src:
    - https://mycdn.com/worker.js
  style_src:
    - https://mycdn.com/style.css

This would remove the ability for admins to further restrict Kibana's CSP, however we are already using the minimum CSP required for Kibana to work correctly, so this seems like a better default experience. If admins need, they can still restrict this CSP using a reverse-proxy to rewrite the response header to clients.

cc @elastic/kibana-security

Required for #101579

@joshdover joshdover added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Breaking Change labels Mar 11, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@pgayvallet
Copy link
Contributor

pgayvallet commented Mar 12, 2021

+1. This would also greatly ease the implementation of #93785 (and we may want to do both at the same time)

Should we add this to our list of 8.0 breaking changes meta issue?

@legrego
Copy link
Member

legrego commented Mar 15, 2021

I'm generally in favor of this change. It's certainly a better UX from an administrative perspective.

This would remove the ability for customers to further restrict Kibana's CSP, however we are already using the minimum CSP required for Kibana to work correctly, so this seems like a better default experience.

This is currently true, and I don't foresee this changing anytime soon. I could envision someone wanting to create a "hardened" installation, where they disable all of the "problematic" features that require such an open CSP. I don't expect that's possible today, but as we continue to harden Kibana, this might eventually be possible. It's a bit of a stretch though, so I don't think it's worth optimizing for this.

If customers need, they can still restrict this CSP using a reverse-proxy to rewrite the response header to clients.

I'm guilty of this as well, but we should be careful about this line of thinking. While this is true for on-prem deployments, this isn't really viable for ESS, unless you're able to instruct all of your users to always connect to the proxy, and never connect to the otherwise publicly accessible ESS link.

@pgayvallet
Copy link
Contributor

Are script-src, worker-src and style_src sufficient for the initial implementation, or do we foresee more CSP directive being necessary initially?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants