keycloak / keycloak

Open Source Identity and Access Management For Modern Applications and Services
https://www.keycloak.org
Apache License 2.0
22.24k stars 6.61k forks source link

Review Content Security Policy (CSP) #16277

Open stianst opened 1 year ago

stianst commented 1 year ago

Description

The default Content Security Policy (CSP) used by Keycloak is not locked down enough, and should be improved as it adds a lot of additional protection against XSS attacks.

We need to investigate what would be the best header. In addition it will need to be dynamic/smart as different parts of Keycloak requires different CSP headers.

In addition there may be a need to for users to be able to configure some aspects of the CSP header for some situations. Currently this is done by completely overriding the header, but this is not great as it results in a less secure CSP header as the same header is applied to everything Keycloak does.

Discussion

No response

Motivation

No response

Details

See #16759, #15874, #14078, #9553

### Tasks
- [ ] #29782
- [ ] #21728
- [ ] #32079
- [ ] #32078
- [ ] https://github.com/keycloak/keycloak/issues/32901
ChristianCiach commented 1 year ago

Some professional security advisers asked us to set the CSP header to default-src 'self'; frame-ancestors 'self'; object-src 'none', but this is currently not possible. These are the issues I've encountered:

This is true for Keycloak 20.0.1 when using the default theme. This list is surely not complete. I just wanted to contribute my own findings to this issue.

So, my complete CSP header currently looks like this:

frame-ancestors 'self'; default-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline'; img-src 'self' data:;

Especially the unsafe-inline ones are undesirable, but the embedded images (using the data: scheme) should also be extracted as proper image files if possible.

jonkoops commented 1 year ago

The admin console uses inline-style in addition to inline-script, so I had to add style-src 'self' 'unsafe-inline';.

Where is this script located? The only one I am aware of is of a type that should not be executable.

ChristianCiach commented 1 year ago

@jonkoops I am not sure if this is what you are asking, but when I open the admin console at https://my-keycloak.local/auth/admin/master/console/#/, this is the html source:

Expand for HTML

```html Keycloak Administration Console

Loading the admin console

```

As you can see, the whole page is essentially a single div. The rest is mostly inline-style and inline-script. If I forbid the loading of the inlined script by omitting script-src 'unsafe-inline' from the CSP, then the admin console won't load (showing a loading throbber forever).

jonkoops commented 1 year ago

@ChristianCiach the only script I am seeing here is non-executable. script-src 'unsafe-inline' blocks that as well?

ChristianCiach commented 1 year ago

@jonkoops I am not a CSP oder Javascript expert, but it looks like it does. Chrome clearly indicates in the developer console that this line is being blocked due to CSP.

ChristianCiach commented 1 year ago

@jonkoops It looks like I was wrong. Sorry about that! Chrome actually complains about these two things:

At https://my-keycloak.local/auth/admin/master/console/:10:

Refused to apply inline style because it violates the following Content Security Policy directive: "default-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-Nx73PRBbtDNJn7vKyE+HPicspzFRfNSmMMyBYXQtT9E='), or a nonce ('nonce-...') is required to enable inline execution. Note also that 'style-src' was not explicitly set, so 'default-src' is used as a fallback.`

At https://my-keycloak.local/auth/realms/master/protocol/openid-connect/3p-cookies/step1.html:7:

Refused to execute inline script because it violates the following Content Security Policy directive: "default-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-ojdN+QBxqZx2irtk+NdVEpnJ80rZvUusqKdiGBaJ2wY='), or a nonce ('nonce-...') is required to enable inline execution. Note also that 'script-src' was not explicitly set, so 'default-src' is used as a fallback.

step1.html seems to be an iframe, with this inline script:

<script>
      if ("hasStorageAccess" in document) {
        checkStorageAccess();
      } else {
        placeTestCookie();
      }

      function checkStorageAccess() {
        document.hasStorageAccess().then(function (hasAccess) {
          window.parent.postMessage(
            hasAccess ? "supported" : "unsupported",
            "*"
          );
        });
      }

      function placeTestCookie() {
        document.cookie =
          "KEYCLOAK_3P_COOKIE_SAMESITE=supported; Max-Age=60; SameSite=None; Secure";
        document.cookie = "KEYCLOAK_3P_COOKIE=supported; Max-Age=60";
        window.location = "step2.html";
      }
    </script>

This is pretty much everything that I can see. I am not a JavaScript developer and my skills using the browser console are very limited.

jonkoops commented 1 year ago

Thanks for verifying that, I think we can certainly look into adding a nonce or hash for these ones. Question is what the right direction is for these (see MDN docs).

ChristianCiach commented 1 year ago

Using hashes or nonces would make it very tough to continue to allow setting a custom CSP header as part of the realm settings. The only way I could think of to make this work would be some kind of variable that we could use inside the custom CSP header, like script-src 'self' 'nonce-{{ SCRIPT_NONCE }}';.

Of course, the nonce must be unique for every request, since it would be useless otherwise.

At this point it may be easier to "just" move all inline scripts to external files, but that may be easier said than done if the scripts are dynamically generated for every response.

jonkoops commented 1 year ago

At this point it may be easier to "just" move all inline scripts to external files

I mean yes, if it does have a security benefit. For example, that script in the iframe is already in a performance critical section and is not dynamically generated - so an unrealistic attack vector.

gracegyu commented 1 year ago

I'm having the same problem too. has it been resolved?

Adding "style-src 'self' 'unsafe-inline';" to Content-Security-Policy doesn't work.

Refused to frame 'http://localhost:8080/' because an ancestor violates the following Content Security Policy directive: "frame-ancestors 'self'".
epuronta commented 1 year ago

I'm interested in this as well. The issue came up with an automated security scanner, which (appropriately) flagged the permissive CSP policies. Applying more restrictive policies causes several issues with the default templates, making it impractical for us.

On a related topic, it seems that customizing the CSP headers on the welcome page is impossible as all the admin-configurable policies apply on a realm level. The common welcome page is obviously not part of any single realm. This is problematic for more primitive scanners that only scan the domain root = welcome page.

olbusat commented 1 year ago

Faced with the same problem. Subscribing for comments :)

m4rc77 commented 10 months ago

FYI: theme/keycloak.v2 requires 'unsafe-eval' in Content-Security-Policy (CSP) : https://github.com/keycloak/keycloak/issues/24802

shubhit7 commented 9 months ago

I'm having the same problem too. has it been resolved?

Adding "style-src 'self' 'unsafe-inline';" to Content-Security-Policy doesn't work.

Refused to frame 'http://localhost:8080/' because an ancestor violates the following Content Security Policy directive: "frame-ancestors 'self'".

I was facing similar error on browser console in my angular application. This was only happening in case of check-sso init options with silent redirect. Adding http://localhost:4200/* (default url for angular apps) in Valid redirect URIs in keycloak client fixed the issue.

zc-devs commented 6 months ago

On 23.0.6 version I had to develop CSP below in Traefik:

apiVersion: traefik.io/v1alpha1
kind: IngressRoute
metadata:
  name: keycloak
spec:
  routes:
    - kind: Rule
      match: Host(`keycloak.example.org`) && PathPrefix(`/admin`)
      priority: 3
      middlewares:
        - name: keycloak-admin-headers
    - kind: Rule
      match: Host(`keycloak.example.org`) && PathPrefix(`/realms/{realm:[a-z]+}/account`)
      priority: 2
      middlewares:
        - name: keycloak-account-headers
        - name: security-headers
    - kind: Rule
      match: Host(`keycloak.example.org`) && (PathPrefix(`/realms`,`/resources`,`/js`) || Path(`/robots.txt`))
      priority: 1
      middlewares:
        - name: keycloak-headers
        - name: security-headers
apiVersion: traefik.io/v1alpha1
kind: Middleware
metadata:
  name: keycloak-admin-headers
spec:
  headers:
    browserXssFilter: true # X-XSS-Protection: 1; mode=block
    contentTypeNosniff: true # X-Content-Type-Options: nosniff
    contentSecurityPolicy: >-
      upgrade-insecure-requests;
      base-uri 'self';
      default-src 'none';
      img-src 'self';
      font-src 'self';
      style-src 'self' 'unsafe-inline';
      script-src 'self' 'unsafe-inline';
      connect-src 'self';
      frame-src 'self';
      frame-ancestors 'self';
apiVersion: traefik.io/v1alpha1
kind: Middleware
metadata:
  name: keycloak-account-headers
spec:
  headers:
    contentSecurityPolicy: >-
      base-uri 'none';
      default-src 'none';
      img-src 'self' data:;
      font-src 'self';
      style-src 'self' 'unsafe-inline';
      script-src 'self' 'unsafe-inline' 'unsafe-eval';
      connect-src 'self';
      frame-src 'self';
      frame-ancestors 'self';
apiVersion: traefik.io/v1alpha1
kind: Middleware
metadata:
  name: keycloak-headers
spec:
  headers:
    contentSecurityPolicy: >-
      base-uri 'none';
      default-src 'none';
      img-src 'self' data:;
      font-src 'self';
      style-src 'self';
      script-src 'self' 'unsafe-inline';
      connect-src 'self';
      frame-src 'self';
      frame-ancestors 'self';
apiVersion: traefik.io/v1alpha1
kind: Middleware
metadata:
  name: security-headers
spec:
  headers:
    frameDeny: true # X-Frame-Options: DENY
    browserXssFilter: true # X-XSS-Protection: 1; mode=block
    contentTypeNosniff: true # X-Content-Type-Options: nosniff

Keycloak account management page requires even unsafe-eval script policy because of lines like if (item.hidden && eval(item.hidden)) continue;.

Also there are warnings in Chrome:

An iframe which has both allow-scripts and allow-same-origin for its sandbox attribute can escape its sandboxing.
/realms/master/protocol/openid-connect/3p-cookies/step2.html:1 An iframe which has both allow-scripts and allow-same-origin for its sandbox attribute can escape its sandboxing.
login-status-iframe.html:1 An iframe which has both allow-scripts and allow-same-origin for its sandbox attribute can escape its sandboxing.
jonkoops commented 3 months ago

Just re-posting my comment from https://github.com/keycloak/keycloak/issues/9553#issuecomment-2114828798 here:

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 commented 2 months ago

We've converted this to an epic to determine what is needed to improve the content security policy experience in Keycloak. Feel free to suggest any issues that should be considered a part of this, and I'll see if they can be added.

dominiktopp commented 1 month ago

Another little feature request regarding CSP: We'd like to use CSP violation reports. So we add the directives report-to csp-endpoint; report-uri csp-endpoint to our CSP header and also need to set the header Reporting-Endpoints to 'csp-endpoint="/csp-report". Would be great if this would also be possible in Keycloak Admin UI.

jonkoops commented 1 month ago

@dominiktopp feel free to log this as a seperate feature request and ping me, I'll add it to this epic for evaluation.

dominiktopp commented 1 month ago

@jonkoops here it is: https://github.com/keycloak/keycloak/issues/32078

dominiktopp commented 1 month ago

I also created https://github.com/keycloak/keycloak/issues/32079 for using CSP nonce. At the moment we must rework our CSP and unsafe-inline is not permitted by our security department.

jamesshuriff commented 1 month ago

I have a patch that adds script-src support to ContentSecurityPolicyBuilder and the SecurityHeadersProvider. It can be used to inject hashes into the CSP header. The patch utilizes this to inject a hash for the BrowserHistoryHelper used for browser-based flows. It can be used for anything else on the server that generates on-the-fly JavaScript.

If this is desirable, I'll make a PR.

jonkoops commented 1 month ago

@jamesshuriff I would certainly welcome that, feel free to open up a PR for it. We'll have to do a close evaluation of our scripts, ideally all in-line scripts would be covered by this, but we can run it behind an experimental feature for a while to ensure folks can try it out.

jonkoops commented 1 month ago

Thanks @dominiktopp, I've linked them both here so they can be tracked under this epic.

jamesshuriff commented 1 month ago

@jonkoops I've opened #32125

jamesshuriff commented 1 month ago

@jonkoops as far as the inline scripts go, I'm writing a patch for the Freemarker provider to generate nonces that can be incorporated into templates so that the inline scripts don't have to be externalized. #32125 is more targeted towards on-the-fly JavaScript generated by the server.

jonkoops commented 1 month ago

I'm writing a patch for the Freemarker provider to generate nonces that can be incorporated into templates so that the inline scripts don't have to be externalized.

Awesome, that is what I had in mind for those as well. Great stuff :+1:

dominiktopp commented 1 month ago

I'm writing a patch for the Freemarker provider to generate nonces that can be incorporated into templates so that the inline scripts don't have to be externalized. #32125 is more targeted towards on-the-fly JavaScript generated by the server.

@jamesshuriff this sounds exactly like https://github.com/keycloak/keycloak/issues/32079. Thank you. Unfortuantely I don't have the time to support on implementing this

jamesshuriff commented 1 month ago

@dominiktopp I'm finishing it up, now. I'm in the same boat you are, with security teams giving me a hard time over CSP.

jamesshuriff commented 1 month ago

I've opened #32144 to implement a NonceBean for Freemarker templates. It covers script-src and style-src. It also includes the previously discussed enhancements.