[keycloak-dev] Improved CSP support

Tue Jul 31 14:39:53 EDT 2018

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
>