Security issue with 'unsafe-inline'

[RobertoLetica1]

Describe the bug

We used security scan to check for security issues. One of the issues was that we are using script-src 'unsafe-inline' and recommended solution for that issue was to remove 'unsafe-inline' from our CSP header and add hash values instead. So we would like to safely remove this 'unsafe-inline' element from CSP header and change that 'unsafe-inline' with hash values. We tried to do that, but on the login page of our web app we get this javascript snippet that causes problems. We have here lastExecutionUrl and that url is always different and because of that we always need to use different hash value so we can't remove this 'unsafe-inline' from our script elements and add hash values instead. Is there some other way to safely remove this 'unsafe-inline' element or is there a way we can avoid using this js snippet?

Version

12.0.4

Expected behavior

We can safely remove 'unsafe-inline' from our CSP.

Actual behavior

We can't remove it because of lastExecutionUrl and because of that we can't login to our web app.

How to Reproduce?

1. Remove 'unsafe-inline' from CSP
2. Try to login to some web app with user that's on Keycloak and Keycloak provides login for that web app
3. User can't login

Anything else?

No response

[steve192]

This is still a security relevant problem in the latest keycloak version.

For security reasons we also want to use a stricter CSP to prevent XSS attacks. For this we don't want to allow inline scripts. The problem is that some of the freemaker templates use inline scripts (https://github.com/keycloak/keycloak/search?l=FreeMarker&p=1&q=%22%3Cscript%3E%22).

We found that keycloak supports externalizing js scripts into seperate js files, which is a safer way and allows a more strict CSP. The problem was that in some of the former inline scripts freemarker template variables were used. These are no longer available in externalized scripts.

Our solution was to pass these variables as "parameters" to the scripts. E.g.

<script
  src="js/login.js"
  type="text/javascript"
  variable1="${msg("text1")}"
  variable2="${msg("text2")}"
  variable2="${msg("text3")}"/>

And in the script itself:

const variable1 = document.currentScript.getAttribute( 'variable1' );
const variable2 = document.currentScript.getAttribute( 'variable2' );
const variable3 = document.currentScript.getAttribute( 'variable3' );

...
script that useses the variables
...

The problem that remains is that a lot of freemarker templates need to be overwritten. Meaning if something changes in a new keycloak release, we have to adapt it.

My suggestion would be to include this "externalization technique" in the official keycloak source. I guess many people want to use a strict CSP in these days, especially for an authentication service.

I would gladly provide a PR for this.

[antoinechamot]

I don't see admin part in your PR ?

[mposolda]

This will be very nice to fix, so adding it to Keycloak 22. On the other hand, it is not a blocker for Keycloak 22, so it can be postponed.

@steve192 If you can provide PR with the approach you suggested, it would be welcome!

BTV. Keycloak uses inline javascript probably in few more places. For example I am aware that on the login page (in login.ftl), there is this snippet to prevent "double-submit" when user clicks twice on the button:

<form id="kc-form-login" onsubmit="login.disabled = true; return true;" action="${url.loginAction}" method="post">

So I guess this may also require externalize the snippet from onsubmit to some external JS file?

[ChristianCiach]

This issue is strongly related to https://github.com/keycloak/keycloak/issues/16277. (Just adding the link as a cross reference.)

[jonkoops]

Thanks for mentioning that @ChristianCiach, I've add a mention of this issue there.

[dbt-lucka]

Hello,

the fact that this issue is postponed milestone after milestone since 21 months now let's us believe that resolution of this security issue in a future release won't come as well.

Since we want to respect CSP on our website, without having smelly workarounds like unsafe-inline, we have to consider building the login form with authentication handling ourselves in a clean way, because we cannot really live with a nonce or hash alternative.

Every update of Keycloak may change the hash and thus, even a patch update is considered a potential breaking change then. Just wanted to point that out, that inline javascripts in Keycloak are that big of a problem for every web project respecting CSP in its designed fashion.

[jonkoops]

Feel free to open up a PR to improve this. We're currently at capacity so this is unlikely to get picked up anytime soon. Adding this to the backlog to reflect that.

[stianst]

Putting this back into triage as it's far from great

[jonkoops]

I think we can possibly mitigate this by ensuring the scripts we are embedding here are marked with a nonce-source or a hash-source (see MDN docs). My preference would be to use a hash-source, so we can can guarantee that the contents are unmodified from the source where it is generated, without needing to annotate the script tag further.

I'll do some investigation into this, but from what I am seeing so far we'll likely have to refactor a lot of code to ensure a secure CSP by default, so the work for this will likely have to be gated by an experimental feature flag until deemed stable enough. I'd be interested if there are participants in this issue that would be interested in test driving this.

[jonkoops]

I am closing this issue, as it is sufficiently covered by #16277. Any effort to improve the content security policy will have to be a project-wide effort anyways.