[keycloak-dev] Improved CSP support

[Johannes Knutsen]

Actually, your suggestion to inject a cspNonce value through the
FreeMarkerLoginProvder/AccountProvider is the exact solution I chose
to implement in the example implementation,
https://github.com/keycloak/keycloak/compare/master...knutz3n:feature/support-stricter-csp-headers.

CSP support in the admin GUI, seems to be a hard problem. I did some
work to get this working and most issues were resolveable. However,
the ACE editor has an open issue,
https://github.com/ajaxorg/ace/issues/3260, regarding CSP and there
was also a minor change required to ng-file-upload.

Our first priority is anyways to be able to disable inline scripts on
our customer's authentication realm and currently we get a CSP error
when setting a script-src 'self' rule:
"Refused to execute inline script because it violates the following
Content Security Policy directive: "script-src 'self'". Either the
'unsafe-inline' keyword, a hash ('sha256-h/...'), or a nonce
('nonce-...') is required to enable inline execution."
This is due to the inline script injected by BrowserHistoryHelper.java
and adding a CSP nonce solves this.

I don't think CSP should replace using a sanitizer, since CSP support
is varying between browsers. However, if you would consider merging
the addition of adding a CSP nonce value to the Freemarker context as
shown in the diff above, I would be happy to add some tests. At least
it would allow template developers to also use inline scripts, by
using the CSP nonce value as you mentioned.

- Johannes

On Mon, Jul 30, 2018 at 7:48 PM, Stian Thorgersen <sthorger at redhat.com> wrote:
> Not sure if it's an option to remove all inline scripts/styles. Maybe it is.
>
> With regards to the other options. SHA hash seems rather brittle and hard to
> maintain, so a nonce would probably be the better option. I haven't looked
> to much at the details on how that's handled, but I assume it's a
> per-request so you can easily generate one in
> FreeMarkerLoginProvider/AccountProvider, add it to the header, then
> templates can themselves add the nonce using something like
> none="${cspNonce}".
>
> Seems like an elegant solution to something we're working on at the moment.
> Currently most fields are escaped so it's not possible to for example inject
> scripts, but there are some that are not as they need to output HTML tags
> (realm display name for instance). We've ended up with using OWASP HTML
> sanitizer library to escape unsafe elements. It's a bit messy code + we need
> to productize the OWASP library. It sounds like this could remove the need
> to do that altogether.
>
> On Wed, 20 Jun 2018 at 00:30, Johannes Knutsen <johannes at kodet.no> wrote:
>>
>> Hi,
>>
>> I am currently looking at improvements in the Content Security Policy
>> (CSP) support.
>>
>> In our deployment, we have security requirements stating that a CSP
>> header should be used and inline scripts, styles and resources should
>> be blocked. For example by setting a CSP value like default-src
>> 'self';.
>>
>> Such a policy breaks Keycloak's manipulation of the browser history
>> implemented in the BrowserHistoryHelper, since the
>> JavascriptHistoryReplace injects an inline JavaScript.
>>
>> The simplest workaround is to also inject a nonce value or SHA hash of
>> the script to the existing CSP header.
>>
>> However, while implementing this, I found that a CSP nonce in general
>> would be nice to have available in any template context. This will
>> also make it easier to migrate the default Keycloak theme to support
>> stricter security policies.
>>
>> An example implementation can be found here:
>>
>> https://github.com/knutz3n/keycloak/commit/c6cfb3efa2942d7569066c0e4bd90a2ed75a0005
>>
>> Would you be interested in merging a change like the one above? If
>> not, what is your view on how to allow stricter content security
>> policies?
>> Tests and documentation is currently missing, but I will add both if
>> this is something you would consider merging.
>>
>> As a note, I have also done some work on supporting a strict CSP value
>> for the default theme. But there are some issues with included 3rd
>> party scripts which must/should be resolved. Let me know if you want
>> more details regarding this.
>>
>> Best regards,
>> Johannes Knutsen
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev