search

swagger-api / swagger-ui

Swagger UI is a collection of HTML, JavaScript, and CSS assets that dynamically generate beautiful documentation from a Swagger-compliant API.
https://swagger.io
Apache License 2.0
26.56k stars 8.95k forks source link

Swagger-ui requires 'unsafe-eval' in CSP Headers for SVG #7540

Open Gidgidonihah opened 3 years ago

Gidgidonihah commented 3 years ago

Q&A (please complete the following information)

Content & configuration

The default petstore example can be used.

Serve swagger-ui with a CSP that limits images, such as:

Content-Security-Policy: img-src 'self'

Describe the bug you're encountering

The swagger logo and icons are blocked when using a secure CSP. Swagger-ui should not use inline data images in the css for SVGs.

To reproduce...

Serve swagger-ui with a CSP that limits images, such as:

Content-Security-Policy: img-src 'self'

This will cause the browser to block images with a url starting with data:image/svg+xml;. The swagger-ui logo and expand/collapse icons are blocked.

Expected behavior

The swagger logo and icons should not be blocked when using a secure CSP.

Screenshots

Screen Shot 2021-10-05 at 12 15 41 PM

Additional context or thoughts

This can be worked around by using the CSP:

Content-Security-Policy: img-src 'self' data: 'unsafe-eval'

But, as is explicit in the header name, this is unsafe.

CSP in swagger-ui is a larger problem than this bug report

In this bug report, I kept it specific to the SVG images as I hadn't seen them mentioned in a bug report thus far. But CSP seems to be generally overlooked. I see no reason that swagger-ui should not work with a basic CSP:

Content-Security-Policy: default-src 'self'

As-is, I've had to build a very custom (see below) CSP that includes the hashes of various things to do with swagger-ui (across various versions) and also accept that the logo will look broken, and the expand/collaps icons are missing. This is not a great experience for the user.

Content-Security-Policy: default-src 'self'; script-src 'self' 'sha256-nFMTH3wGPLB0wJ/cmrR7Mkpqv/QqOOxvO9lDvelTy38=' 'sha256-HVWS/Zvb+/hKGrQbSIz41rEcAp5UwDhUBL8xoJ5Tdrw='; style-src 'self' 'sha256-pyVPiLlnqL9OWVoJPs/E6VVF5hBecRzM2gBiarnaqAo='; style-src-attr ; worker-src 'self'

See also: https://github.com/swagger-api/swagger-ui/issues/5817 https://github.com/swagger-api/swagger-ui/issues/3370 https://github.com/swagger-api/swagger-ui/issues?q=is%3Aissue+content-security-policy

silverwind commented 3 years ago

This is because SVGs are loaded as images which as per CSP is an "unsafe" source:

<img src="data:image/svg+xml;base64,..." alt="Swagger UI" height="40">

It should be replaced with inline SVG which has no such issues:

<svg xmlns="http://www.w3.org/2000/svg">...</svg>
char0n commented 3 years ago

Hello everybody,

Given @silverwind answer, this seems like something that can be fixed quite easily. Is anybody interested in issuing a PR?

mathis-m commented 3 years ago

@char0n probably this is due to the webpack pipeline. I think I had a look at it and the Image was imported via js import logo from 'some' and placed in the src={logo}. Maybe it could be change via webpack or it needs to be defined as svg inline.

silverwind commented 3 years ago

Probably need to remove .svg from url-loader here and move it to raw-loader and then load it via dangerouslySetInnerHTML.

rohitkadlag777 commented 2 years ago

When can this change is available in swashbuckle .net core package means no inline styles and javascript and image displayed from data source in swagger index.html page? So that we can set Content-Security-Policy:default-src 'self'

RobinGeffroy commented 2 years ago

Hello, even with the change available, it seems img-src 'self' data: is still needed. Will it be fixed as well ?

oleksiipiliugin commented 2 years ago

Hey, Same here. With the latest version of swagger-ui i still need to use the unsafe approach img-src 'self' data:. It would be great to get off images declaration as data:image/svg+xml;base64,... to make CSP happy.

cloudonshore commented 1 year ago

Any update on this? Still running into this issue on the latest version.

Codehardt commented 1 year ago

Same here. I am using the most-strict CSP default-src 'self'; frame-ancestors 'none'; that works for my whole application except for the API documentation page based on Swagger UI.

mmerezhko-hv commented 1 year ago

Do you plan to fix this in the near future?

char0n commented 1 year ago

We need to use https://www.npmjs.com/package/@svgr/webpack avoid the CSP issue.

char0n commented 1 year ago

Addressed the SVG assets in https://github.com/swagger-api/swagger-ui/pull/9209

There are additional tree SVG embedded in SASS files. These needs to be handled as well.

char0n commented 1 year ago

Well with SVG assets embedded in CSS we have following options

1. extract as external SVG

This is possible to do but the build will have to have additional assets directory. The issue will also be with build fragments distributed via different package systems. There were just two assets before - CSS and JavaScript file. This will introduce additional build fragments and will break established bundler configurations.

2. refactor actual React components

Instead of 

        <label htmlFor="servers">
          <select onChange={ this.onServerChange } value={currentServer}>
            { servers.valueSeq().map(
              ( server ) =>
              <option
                value={ server.get("url") }
                key={ server.get("url") }>
                { server.get("url") }
                { server.get("description") && ` - ${server.get("description")}` }
              </option>
            ).toArray()}
          </select>
        </label>

We have to do:

        <label htmlFor="servers">
          <ArrowDownIcon />
          <select onChange={ this.onServerChange } value={currentServer}>
            { servers.valueSeq().map(
              ( server ) =>
              <option
                value={ server.get("url") }
                key={ server.get("url") }>
                { server.get("url") }
                { server.get("description") && ` - ${server.get("description")}` }
              </option>
            ).toArray()}
          </select>
        </label>

This requires a lot of changes to existing React JSX.

I don't really like any of those options and I'm inclined to keep the status quo. Anybody has any other idea?

char0n commented 1 year ago

All right, I've merged https://github.com/swagger-api/swagger-ui/pull/9209. It helps the situation a bit, but doesn't resolve the issue completely. We still need to resolve https://github.com/swagger-api/swagger-ui/issues/7540#issuecomment-1718984278

silverwind commented 1 year ago

I'd say data: protocol is fine and should be part of any reasonable CSP because a lot of frontend js/css uses data: uris for many purposes and they are kind of a self source but were excluded from self for whatever reason. If it works with below CSP, I'd say the issue is solved.

Content-Security-Policy "default-src 'self' data:"
silverwind commented 1 year ago

Actually I think the original issue is invalid and unsafe-eval was never needed, only data: which is generally considered a "safe" source. I think the behaviour of unsafe-eval for img-src is not even defined in the spec and it's likely not matching on anything. Only these things classify for unsafe-eval for style-src and script-src:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#unsafe_eval_expressions https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src#unsafe_style_expressions

Codehardt commented 1 year ago

If you decide to improve CSP compatibility, I would suggest doing it right, which would include avoiding data:.

silverwind commented 1 year ago

Imho, SVGs as static hardcoded data: in CSS is perfectly fine and safe on it's own but of course, inline SVG in HTML would be ideal, which I guess means you have to touch all the JSX to extract to <svg>components.

char0n commented 1 year ago

We'll most likely not be addressing the inline svg images in CSS. It's a big time investment not worth the effort.

I'll do another analysis and the goal is to fit SwaggerUI into Content-Security-Policy "default-src 'self' data:". (which I think we already fit after last PR).

If you decide to improve CSP compatibility, I would suggest doing it right, which would include avoiding data:.

We did enhance the situation, but complete remediation just for the sake of purity is currently out of question. We invite you to issue a PR though.

Imho, SVGs as static hardcoded data: in CSS is perfectly fine and safe on it's own but of course, inline SVG in HTML would be ideal

Agreed, It's not ideal but it's more than acceptable.

I'll document the recommended CSP depending on the the plugin composition (with or without Standalone plugin).

RobinGeffroy commented 1 year ago

Using the data: keyword in CSP seems to be a security issue. For instance, it's an old thread but still valid: https://security.stackexchange.com/questions/94993/is-including-the-data-scheme-in-your-content-security-policy-safe Moreover, some security scans are currently detecting the usage of data: as a security vulnerability as well.

Do you have more informations about this ? I'm curious, and currently trying to get rid of this CSP directive.

char0n commented 1 year ago

I'm open to suggestions how to resolve this. I've already elaborated in https://github.com/swagger-api/swagger-ui/issues/7540#issuecomment-1718984278 what can be done. Both options have pros and cons.

IMHO ideal is to go for option 2 - refactor actual React components.

Are there any other options you can think of?

I probed 2. refactor actual React components and determined that it is quite significant effort. If there are any volunteers, that would be great.

silverwind commented 1 year ago

Imho, refactor to react components, but make it generic, e.g. <Icon name="arrow-down"/>.

tafaust commented 1 year ago

Can we also include the script integrity for oauth2-redirect.html here https://github.com/swagger-api/swagger-ui/blob/e6837593bdeb6d489b6c8ffe288538c801aaf5a7/dist/oauth2-redirect.html#L7 ?

Quick google search gave me openssl dgst -sha384 -binary FILENAME.js | openssl base64 -A on how to compute the checksum.

Alex-ley-scrub commented 6 months ago

The most strict CSP I have found that still enables the site to function fully is:

Content-Security-Policy: default-src 'self'; img-src 'self' data: https://validator.swagger.io/; script-src 'self' 'sha256-5Fyhk/sAyk1LqSOCABSorU3lSp7aso7Uf5/J5HBwgDg='

worth noting these comments (but you should verify this is tolerable for your use case yourself):

# "data:" is generally considered insecure, but limited to "img-src" it seems tolerable:
# see: https://security.stackexchange.com/a/167244/271464

In flask I implement this with Talisman:

import os
from flask import Flask
from flask_talisman import Talisman
from flask_swagger_ui import get_swaggerui_blueprint

ROOT_PATH: str = os.path.dirname(os.path.abspath(__file__))
SWAGGER_URL = "/openapi/swagger"  # URL for exposing Swagger UI (without trailing '/')
API_URL = "/openapi.yaml"  # Our API url (can be a local resource or url)

app = Flask(__name__, static_folder="", root_path=ROOT_PATH)

swaggerui_blueprint = get_swaggerui_blueprint(
    SWAGGER_URL,  # Swagger UI static files will be served at {SWAGGER_URL}/dist/
    API_URL,
    # config={}, # Swagger UI config overrides
    # oauth_config={},
)
app.register_blueprint(swaggerui_blueprint)

# we need this CSP for talisman for swagger_ui to load:
# see: https://github.com/swagger-api/swagger-ui/issues/7540#issuecomment-1719302413
# and: https://github.com/swagger-api/swagger-ui/issues/7540#issue-1016654521
# and: https://github.com/GoogleCloudPlatform/flask-talisman?tab=readme-ov-file#content-security-policy
CSP = {
    "default-src": ["'self'"],
    # "data:" is generally considered insecure, but limited to "img-src" it seems tolerable:
    # see: https://security.stackexchange.com/a/167244/271464
    "img-src": ["'self'", "data:", "https://validator.swagger.io/"],
    "script-src": ["'self'", "'sha256-5Fyhk/sAyk1LqSOCABSorU3lSp7aso7Uf5/J5HBwgDg='"],
}

talisman = Talisman(
    app, strict_transport_security_preload=True, content_security_policy=CSP
)
onkobu commented 5 months ago

Content-Security-Policy: default-src 'self'; img-src 'self' data: https://validator.swagger.io/; script-src 'self' 'sha256-5Fyhk/sAyk1LqSOCABSorU3lSp7aso7Uf5/J5HBwgDg=' Does not work, presumably the hash already changed.

The checkboxes in the Authorize-overlay cannot be checked anymore or at least the check marks don't appear since the resource is blocked. Finally the redirect from an OAuth-provider is blocked because of the inline handler. With A proper CSP swagger-ui is not usable with (OAuth) authorization. (And it is tedious to obtain and copy bearer tokens every couple minutes to a simple bearer input field)

Regarding the redirect there are also #5720 and #8330

phatcher commented 3 months ago

Sorry if the question is dumb, but is it easy to compute the sha in the build pipeline and include in the nuget, or can we add something in our CI pipeline to compute it?

onkobu commented 3 months ago

I don't see the reason for JavaScript-quirks to simply have an input of type checkbox checked (that are to be transferred as part of a form submit). But I also tolerate people using pliers to pull on trousers.

To answer the checksum-pipeline-question: as long as a checksum like in the comment above can be put into the appropriate resource. But with a CSP for a HTTP-server out of your control the answer cannot be other than »fix on customer side«. I can compute a checksum according to my needs which is tedious and breaks from time to time (with auto update tools for dependencies like Renovate). And others will return with the same error causing noise.

All the means of security can either be made right (avoid the unnecessary extensions/ reduce), undermined (by relaxing if possible) or left aside (making it unusable for contexts with strict security policies).