Re: [keycloak-dev] Improved CSP support

On Mon, 30 Jul 2018 at 20:19, Johannes Knutsen <johannes(a)kodet.no&gt; 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/sup... > . > 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(a)redhat.com&gt; > 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(a)kodet.no&gt; > 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/c6cfb3efa2942d7569066c0e4bd90a... > >> > >> 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(a)lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/keycloak-dev >