[keycloak-dev] Improved CSP support

[Stian Thorgersen]

Forgot about this one, sorry.

Any updates from your end Johannes?

On Tue, 31 Jul 2018 at 20:39, Stian Thorgersen <sthorger at redhat.com> wrote:

>
>
> On Mon, 30 Jul 2018 at 20:19, Johannes Knutsen <johannes at kodet.no> wrote:
>
>> 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
>> .
>>
>
> Sounds good, I'm leaving for holiday today so won't have time to take a
> look at it before I'm back in a week or so.
>
>
>>
>> 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.
>>
>
> Is it only the ACE editor that is an issue? If that remains an issue we
> could consider removing it. It's only used when editing js authenticators
> and policies, but I don't think it's all that useful afaik.
>
>
>>
>> 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.
>>
>
> Do you generate the nonce on demand in your branch? As it's only needed
> for very few requests we should generate one in the FreeMarker providers
> unless it's actually needed.
>
>
>>
>> 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.
>>
>
> We'll keep the sanitizer as well. Always good to have more layers of
> defence than less ;)
>
> Would be ideal to have a complete solution. Nonce available, updated
> default value for CSP header for realms, including migrating existing
> realms when the value hasn't changed.
>
>
>>
>> - 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
>>
>