Change default value of csp.disableUnsafeEval to 'true'

[watson]

Today the unsafe-eval source expression is "on" by default in the Kibana Content Security Policy (CSP). Users can choose to set csp.disableUnsafeEval to true to remove unsafe-eval from the CSP.

Once we land support for inline partials in @kbn/handlebars (Update: Landed!), we can change the default behaviour so unsafe-eval is not present in our CSP unless users explicitly opt in. In this PR I do that by change the default value of csp.disableUnsafeEval from false to true.

Closes #150156

Release notes

The default value of csp.disableUnsafeEval is now true instead of false. This means that the unsafe-eval source expression isn't present by default in the Kibana Content Security Policy (CSP). If you depend on unsafe-eval, please set csp.disableUnsafeEval to false.

Questions to reviewers

1. Would it make sense to rename csp.disableUnsafeEval to csp.enableUnsafeEval now that the default value has changed from false to true? Obviously we'd still need keep support for csp.disableUnsafeEval around as a negated alias.
2. Can we land this change in a patch version, or should we only land it in a minor release?
3. I assume it's the right thing to do to remove the "experimental" label as I do in this PR?

Blocked by

• Support inline partials in @kbn/handlebars #139068

[github-actions]

Documentation preview:

• ✨ Changed pages

[elasticmachine]

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

[TinaHeiligers]

I left some comments and tried to answer the questions to the reviewers.

LGTM!

[jeramysoucy]

LGTM!

[pgayvallet]

Can we land this change in a patch version, or should we only land it in a minor release?

I was going to ask if that can even be done outside of the scope of a major. This seems like a breaking change to me (unless I misunderstand the potential implications for users)?

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 88b3218

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	48	49	+1
core	1029	1030	+1
total			+2

Unknown metric groups

API count

id	before	after	diff
@kbn/core-http-server-internal	54	55	+1
core	2845	2846	+1
total			+2

History

• 💚 Build #105286 succeeded 5574e63
• 💚 Build #105068 succeeded c74dcb6

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

cc @watson

[watson]

@pgayvallet technically it shouldn't have any impact on our users, unless they use a 3rd party plugin that replies on unsafe-eval being there. It should just be a transparent change. On top of that I'm pretty sure that our CSP's content isn't something we've committed to support and hence run through a deprecation cycle. @legrego or @azasypkin can keep me honest here

[pgayvallet]

Ok, thanks for elaborating. In that case, LGTM

[pgayvallet]

Would it make sense to rename csp.disableUnsafeEval to csp.enableUnsafeEval now that the default value has changed from false to true?

Doesn't feel mandatory, but it would probably be slightly better yeah. BWC can easily be done via a rename config deprecation

I assume it's the right thing to do to remove the "experimental" label as I do in this PR?

I think so yeah

[azasypkin]

@pgayvallet technically it shouldn't have any impact on our users, unless they use a 3rd party plugin that replies on unsafe-eval being there. It should just be a transparent change. On top of that I'm pretty sure that our CSP's content isn't something we've committed to support and hence run through a deprecation cycle. @legrego or @azasypkin can keep me honest here

Re: "changing default value of csp.disableUnsafeEval from false to true."

While technically it can be a breaking change for certain users, I feel that the benefits of the improved security outweigh potential risks here. Moreover, we have a well defined escape hatch\workaround for those users that might be affected.

Re: "Will be removed in 8.8.0."

I'm slightly nervous here, we recently explicitly called out csp.disableUnsafeEval in one of our very popular advisories and this setting might stick in customers' configs for awhile. If we remove it, Kibana will bootloop after upgrade to 8.8.0. I'd rather deprecate this setting and keep it spamming deprecation logs for quite some time until we feel more confident about dropping it (do we track this setting in out telemetry? If not, we might want to).

Alternatively, we can stop relying on it internally and mark it as unused starting from 8.{8|9|10}.0 .

Re: csp.disableUnsafeEval -> csp.enableUnsafeEval

As @pgayvallet mentioned, it's an easy\safe change with rename (and I'd vote for it in other circumstances), but since we want to eventually get rid of it I don't think it's worth renaming it now.

[watson]

@azasypkin I'll update the deprecation message saying it will be marked as unused in 8.9.0 (just picking the middle number because 🤷)

[azasypkin]

@azasypkin I'll update the deprecation message saying it will be marked as unused in 8.9.0 (just picking the middle number because shrug)

We also can just say "in future version" to not commit to any specific release and do it whenever we feel like it, but up to you.

[watson]

👍

Now the deprecation notice says:

Use csp.script_src: ['unsafe-eval'] instead if you wish to enable unsafe-eval. This config option will have no effect in a future version.