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

Plugins should be able to inject new CSP directives #101579

Open
joshdover opened this issue Jun 8, 2021 · 9 comments
Open

Plugins should be able to inject new CSP directives #101579

joshdover opened this issue Jun 8, 2021 · 9 comments
Labels
enhancement New value added to drive a business result Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@joshdover
Copy link
Member

We have some internal use cases for plugins that add tracking script tags to Kibana to be able to add additional script-src directives to Kibana's CSP so that these tracking scripts can be safely loaded by the client's browser.

This depends on #94414 which changes our CSP configuration to be additive rather than an open-ended box. Once that is implemented, we should be able to add a server-side Core API like:

core.http.csp.addDirective(directive: string);

// Or even more strict (to match config options in #94414)
core.http.csp.addScriptSrc(domain: string);
core.http.csp.addStyleSrc(domain: string);
core.http.csp.addWorkerSrc(domain: string);

cc @elastic/kibana-security for any suggestions or guardrails you'd like to see on this API

@joshdover joshdover added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc enhancement New value added to drive a business result labels Jun 8, 2021
@elasticmachine
Copy link
Contributor

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

@legrego
Copy link
Member

legrego commented Jun 8, 2021

I strongly prefer the more strict approach. I think we should only allow domain in your example above to be a <host-source> as defined by the spec:

<host-source>
Internet hosts by name or IP address, as well as an optional URL scheme and/or port number. The site's address may include an optional leading wildcard (the asterisk character, ''), and you may use a wildcard (again, '') as the port number, indicating that all legal ports are valid for the source.

Examples:

   http://*.example.com: Matches all attempts to load from any subdomain of example.com using the http: URL scheme.
   mail.example.com:443: Matches all attempts to access port 443 on mail.example.com.
   https://store.example.com: Matches all attempts to access store.example.com using https:.
   *.example.com: Matches all attempts to load from any subdomain of example.com using the current protocol.

I don't want to allow plugins to specify their own nonce or hash if we can help it, and I really want to make sure that we don't allow plugins to specify any of the unsafe directives, such as unsafe-eval. We are working really hard to eliminate unsafe-eval today, and I don't want to make it possible for plugins to weaken Kibana's entire CSP by re-declaring this directive, or another unsafe directive.

@pgayvallet
Copy link
Contributor

I think we should only allow domain in your example above to be a as defined by the spec:

Is there a regexp somewhere for the host-source allowed values? Or will we be forced to use a denylist of the 'special' values instead?

@legrego
Copy link
Member

legrego commented Jun 14, 2021

I think we should only allow domain in your example above to be a as defined by the spec:

Is there a regexp somewhere for the host-source allowed values? Or will we be forced to use a denylist of the 'special' values instead?

I'm not aware of a ready-to-go regex. I think we'd have to do our best to ensure that the input conforms to the host-source specification

@joshdover
Copy link
Member Author

This is no longer needed by any internal consumers. Closing, but feel free to reopen if a new use case comes up.

@jportner
Copy link
Contributor

jportner commented Mar 18, 2022

Kibana ships with a default Content Security Policy (CSP) that is as strict as possible without potentially breaking many operators' usage of Kibana.

Recently we've had users ask about configuring default-src on their deployments (is it safe to do?). We also have an open issue for setting default-src: 'self' by default here: #56996

Investigating this with @azasypkin for a couple hours, we've discovered that if default-src 'self' is configured, Kibana will break in several ways:

  1. FullStory (Cloud-only) needs to connect to https://rs.fullstory.com
  2. Newsfeed needs to connect to https://feeds.elastic.co (production) or https://feeds-staging.elastic.co (dev/staging)
  3. Maps needs to connect to https://tiles.maps.elastic.co and https://vector.maps.elastic.co (unless the operator is using a self-hosted EMS)
  4. Images in various places are loaded with data: and blob: URIs

This is not an exhaustive list, but it seems that we have enough justification to allow plugins to register specific CSP source values. We'd only want these source values to be applied to loosen the configured CSP to prevent Kibana from breaking, though. For example: we don't want to apply img-src: 'self' data: blob: by default because that is more strict than what Kibana has today, but if the operator configures default-src: 'self' and/or a custom img-src directive, then we do want to add the data: blob: source values to that directive.

@jportner jportner reopened this Mar 18, 2022
@jportner jportner added the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label Mar 18, 2022
@pgayvallet
Copy link
Contributor

For example: we don't want to apply img-src: 'self' data: blob: by default because that is more strict than what Kibana has today, but if the operator configures default-src: 'self' and/or a custom img-src directive, then we do want to add the data: blob: source values to that directive.

This feature is already available, we probably just need to improve it by adding the missing values necessary for Kibana to function. E.g we're not adding data: blob in any of those:

/**
* Per-directive rules that will be added when the configuration contains at least one value
* Main purpose is to add `self` value to some directives when the configuration specifies other values
*/
export const additionalRules: Partial<Record<CspDirectiveName, string[]>> = {
'connect-src': [`'self'`],
'default-src': [`'self'`],
'font-src': [`'self'`],
'img-src': [`'self'`],
'frame-ancestors': [`'self'`],
'frame-src': [`'self'`],
};

@jportner do you have an exhaustive list of what should be added?

@jportner
Copy link
Contributor

jportner commented Mar 23, 2022

This feature is already available, we probably just need to improve it by adding the missing values necessary for Kibana to function. E.g we're not adding data: blob in any of those:

It's partially available.

  1. This works, but it is only defined by Core (plugins can't dynamically register their own values to be added to this list). I'm envisioning the Cloud plugin dynamically adding values, for example, which wouldn't affect on-prem installations of Kibana because the Cloud plugin is disabled by default.
  2. This list doesn't factor in that the default-src directive is applied in the absence of many other directives. What I'd really like is for a consumer to be able to easily register a value that applies this logic:
if ( default-src is customized and img-src is NOT customized )
  add 'blob:' to default-src
else if ( img-src is customized )
  add 'blob:' to img-src
else
  do nothing

If we were to do this it would probably be prudent to log something on startup, like:

user defined csp: [default-src: https://example.com]
kibana changed this so it doesn't break stuff, final csp: [default-src: https://example.com 'self' blob:]

@jportner do you have an exhaustive list of what should be added?

I don't have an exhaustive list yet, I think what I specified in my comment is the majority of it right now. But I haven't fully tested every aspect of Kibana with such a minimally-defined CSP.

@pgayvallet
Copy link
Contributor

This works, but it is only defined by Core (plugins can't dynamically register their own values to be added to this list). I'm envisioning the Cloud plugin dynamically adding values

Ok, sorry I misunderstood the exact context / need here.

which wouldn't affect on-prem installations of Kibana because the Cloud plugin is disabled by default

FWIW, technically that's incorrect. the cloud plugin is shipped and enabled on every distribution AFAIK. The 'am I on cloud' logic is based on the presence of the cloud.id config property. But not a problem here, we can use this same logic to decide whether we should add the additional directives or not.

What I'd really like is for a consumer to be able to easily register a value that applies this logic

May be a bit tricky, because we don't know the order the plugins will be registering additional directives. So we would need to execute this logic as a post-process step, I think:

  • allow plugins to register directives during setup
  • apply this logic during the csp service (which is not a thing atm) start, as we know we won't be adding additional directive after that point.

But that doesn't seem too complex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
Development

No branches or pull requests

5 participants