Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom content security policy configurations #716

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

erimicel
Copy link

@erimicel erimicel commented Dec 20, 2023

Problem

Having forced content security policy blocks to use multiple server urls to be called from swagger UI

Example multiple server dropdowns in OpenAPI 3.0
https://swagger.io/docs/specification/api-host-and-base-path/

Current CSP:

default-src 'self';
img-src 'self' data: https://online.swagger.io;
font-src 'self' https://fonts.gstatic.com;
style-src 'self' 'unsafe-inline' https://fonts.googleapis.com;
script-src 'self' 'unsafe-inline';

We can expose these configurations to Rails app by configuration settings

I propose config options like this:
c.content_security_policy "default-src", "'self' https://example.com https:://anotherexample.com;"

This will override the default csp that in placed. Unless it is given gem will keep using the default csp.

Without any config this is string csp:

"default-src 'self'; img-src 'self' data: https://validator.swagger.io; font-src 'self' https://fonts.gstatic.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; script-src 'self' 'unsafe-inline';"

With changed config:

"default-src 'self' https://example.com https:://anotherexample.com; img-src 'self' data: https://validator.swagger.io; font-src 'self' https://fonts.gstatic.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; script-src 'self' 'unsafe-inline';"

This allow swagger to have 3 different server settings as dropdown without blocked by:
Content Security Policy directive: "default-src 'self'". Note that 'connect-src' was not explicitly set, so 'default-src' is used as a fallback.

servers: [
        {
          url: 'https://{defaultHost}',
          variables: {
            defaultHost: {
                enum: [www.example.com, www.anotherexample.com, v1.example.com],
                default: 'www.example.com'
            }
          }
        }
      ]

Related Issues

This related #174

Checklist

  • Added tests

@romanblanco romanblanco linked an issue Dec 20, 2023 that may be closed by this pull request
@eric-hemasystems
Copy link
Contributor

eric-hemasystems commented Dec 20, 2023

I wonder if it would be better to build on the CSP infrastructure in Rails to help build a custom CSP? So your configuration method might look more like:

def content_security_policy
  @config[:csp] = ActionDispatch::ContentSecurityPolicy.new do |policy|
    # default policy
    policy.default_src :self
    policy.img_src :self, :data, "https://validator.swagger.io"
    policy.font_src :self, "https://fonts.gstatic.com"
    policy.style_src :self, :unsafe_inline, "https://fonts.googleapis.com"
    policy.script_src :self, :unsafe_inline

    # app can override directives
    yield policy if block_given?
  end.build
end

Then the csp method in the middleware becomes:

def csp = @config.config_object[:csp] || Rswag::Ui.config.content_security_policy

If someone chose to make a custom CSP they are using the same DSL they are already using in Rails and it has all the same validations in place. The application would customize via:

Rswag::Ui.configure do |config|
  config.content_security_policy do |policy|
    policy.default_src :self, "https://example.com" "https://anotherexample.com"
  end
end

NOTE: I haven't tested any of the above. Just scratching out ideas. But the general concept should work.

Some alternatives to all this are:

  • Monkey-patch the csp method on the middleware object. Doesn't give you an official customization path but how common is this need?
  • Have it conditionally add the CSP depending on the env. That way a previous middleware could provide a CSP and this one would simply avoid overriding it.

Not saying those are better alternatives but just worth considering simpler solutions.

@erimicel
Copy link
Author

@eric-hemasystems I like the idea to use same DSL as rails, it definetly looks more clean and better as-well.

Thank you for detailed suggestion 💯 , I will give a try

@romanblanco
Copy link
Member

@erimicel, @eric-hemasystems Thank you for the help!

I wonder if it would be better to build on the CSP infrastructure in Rails to help build a custom CSP?

In consideration of #428, it would be nice to have the solution adaptable to other frameworks too.

@eric1234
Copy link

eric1234 commented Dec 21, 2023

(Sorry, posting from personal account due to holidays but I'm the same person as eric-hemasystems)

@romanblanco - That's an excellent point. Since rswag is already tied so much to Rails I sort of assumed it was ok to add another tie but if there is a goal of removing those ties then perhaps that's not the right solution.

I'm sort of two minds. On one hand, lets not add yet another tie to Rails if the goal is to make this framework independent. OTOH, everyone currently using the library is using Rails. And even if all the ties are removed probably 99% of the users will use Rails. It sort of sucks to not take advantage of the ergonomics that Rails provides for the 99% just to give a path for the 1%.

Given that both sides appeal to me, perhaps we should go about this a different way. My original goal of #174 was not to lock down Swagger but actually to open it up. My app did not allow unsafe-inline but Swagger needed that. Therefore I was providing a more open CSP than my app. While doing that it seemed most prudent to still make that CSP as tight as possible but maybe it's worth evaluating if a more open CSP could be shipped and therefore no config is needed. Seems the need here is the connect-src contacting other sites perhaps we just make the default:

default-src 'self';
connect-src 'self' https:;
img-src 'self' data: https://online.swagger.io;
font-src 'self' https://fonts.gstatic.com;
style-src 'self' 'unsafe-inline' https://fonts.googleapis.com;
script-src 'self' 'unsafe-inline';

This way self (which is probably still useful for dev in case dev is not on HTTPS) and any TLS-enabled site will work. This does allow documentation from any URL to display, but I'm not sure that risk is really that huge.

@erimicel
Copy link
Author

erimicel commented Dec 22, 2023

@romanblanco @eric-hemasystems Good point!,

Seems the need here is the connect-src contacting other sites perhaps we just make the default:

When setting this as the default, it might seem like we're imposing a fixed approach on developers rather than allowing flexibility, especially concerning the connect-src policy in CSP. However, I do see your point that if we enable it for HTTPS by default, additional configurations might not be necessary. Currently, this pull request employs the default setting if no configuration is specified by the developer, ensuring functionality in this scenario. If a developer wishes to modify the CSP, they still have the option to do so.

We can strip down the configuration for Rails csp and use in our case with similiar DSL in ruby file.
https://github.com/rails/rails/blob/6b93fff8af32ef5e91f4ec3cfffb081d0553faf0/actionpack/lib/action_dispatch/http/content_security_policy.rb

/rswag/ui/content_security_policy.rb

Otherwise to change current config developer has to monkey patch the gem middleware.

It sounds like suggesting the addition of connect-src 'self' https:; could be a straightforward and tidy solution for now, serving as a temporary fix until version 3.0 is merged and thoroughly considered. This quick adjustment might provide a simple yet effective way to address the issue at hand. @eric-hemasystems

@AlexanderFisenko
Copy link

Hello. Any activity on this? Do you have plans to merge it?

@erimicel
Copy link
Author

Hello. Any activity on this? Do you have plans to merge it?

I am happy to continue and merge this as it is, at least it gives flexibility until we have better solution

@eric-hemasystems
Copy link
Contributor

Personally, I would still lean towards just opening up the default CSP more to address the specific concern needed (making connect-src 'self' https:;). If we make it a bit more open it's not clear if there is a need for an ability to customize it. The default CSP we have was just my first stab at it based on my needs and is certainly still open to further refinement.

Once you make an API for customizing the CSP it seems you are somewhat stuck with it if that isn't the approach you want to take later.

@jboler
Copy link

jboler commented Apr 17, 2024

My Rails 7.1.2 API app has a basic default CSP set:

Rails.application.configure do
  config.content_security_policy do |policy|
    policy.default_src :self, :https
  end
end

I'm finding that the request to my Rswag::Ui::Engine route path returns 2 Content-Security-Policy: headers, first the hard coded rswag-ui policy and then the rails policy, so the rswag policy get overwritten by the rails policy and rswag-ui cannot load:

$ curl -v https://private-app-name.url/api-docs/index.html
...
> GET /api-docs/index.html HTTP/2
> Host: private-app-name.url
> User-Agent: curl/8.4.0
> Accept: */*
>
< HTTP/2 200
< date: Wed, 17 Apr 2024 19:12:44 GMT
< content-type: text/html
< content-length: 4173
< vary: Accept-Encoding
< content-security-policy: default-src 'self'; img-src 'self' data: https://validator.swagger.io; font-src 'self' https://fonts.gstatic.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; script-src 'self' 'unsafe-inline';
< etag: W/"113226cfb0f49141d46dc22487e30e76"
< cache-control: max-age=0, private, must-revalidate
< content-security-policy: default-src 'self' https:
< x-request-id: 6febab0f-9dba-419d-839d-91d3db9591f3
< strict-transport-security: max-age=31536000; includeSubDomains
...

It would be ideal if rswag overided the Rails policy for just the rswag routes so that this double header issue doesn't occur.

@eric-hemasystems
Copy link
Contributor

It should not be adding two headers (and does not for me). There must be something else going on. How are you mounting the UI in your app? Is your CSP being set by the normal Rails mechanism?

I would suggest you open a new ticket though to discuss your issue since it's sort of a tangent from this ticket.

@jboler
Copy link

jboler commented Apr 17, 2024

It should not be adding two headers (and does not for me). There must be something else going on. How are you mounting the UI in your app? Is your CSP being set by the normal Rails mechanism?

I would suggest you open a new ticket though to discuss your issue since it's sort of a tangent from this ticket.

New ticket opened with minimal repo: #744

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow api-docs CSP to be supplied
6 participants