rswag / rswag

Seamlessly adds a Swagger to Rails-based API's
MIT License
1.93k stars 416 forks source link

[BUG] Duplicate CSP headers when Rails CSP is configured #744

Open jboler opened 2 months ago

jboler commented 2 months ago

Describe the bug

rswag-ui returns duplicate CSP headers when Rails has a CSP configured.

Steps to Test or Reproduce

Example repo: https://github.com/jboler/rswag-csp

The repo is an API only Rails 7.1 app with latest rswag.

To reproduce

bundle install
bin/rails s
curl -v localhost:3000/api-docs/index.html

Returns:

* Host localhost:3000 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:3000...
* Connected to localhost (::1) port 3000
> GET /api-docs/index.html HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/8.5.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Content-Type: text/html
< 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'; 
< content-security-policy: default-src 'self'
< etag: W/"113226cfb0f49141d46dc22487e30e76"
< cache-control: max-age=0, private, must-revalidate
< x-request-id: 3dfc9e63-3a84-4983-97f3-272c33b8c70a
< x-runtime: 0.003001
< server-timing: process_middleware.action_dispatch;dur=0.75
< Content-Length: 4173
< 
<!-- HTML for static distribution bundle build -->

Note the duplicate CSP headers.

Expected behavior

Ideally the rswag CSP header should override the rails CSP header.

I noticed that if I change the rswag-ui middleware source to use an all lower case content-security-policy header like rails then there is no duplicate header.

Dependency versions

The version of are you using for:

eric1234 commented 2 months ago

I believe your problem is this line in the demo app. That line is not in the stock template. You don't need to install the middleware yourself, Rails does it for you. By including it yourself it's probably being inserted in the wrong location in the middleware stack leading to the double header. Remove the line and your problem will probably go away.

jboler commented 2 months ago

I can still reproduce the issue. The reason for adding the Rails CSP middleware manually is because I have config.api_only = true in config/application.rb which reduces the Rails default middleware. I tested this and with config.api_only = true, just enabling the Rails CSP config doesn't add the header, the middleware must be manually added too (I added a line for this to application.rb and commented it out/in for the below tests).

I pushed a new branch which is cleaner. The first commit is just from rails new. The 2nd commit adds rswag (the Gemfile.lock change snuck into the 1st commit).

Test Results:

  1. api_only=false + Rails CSP enabled + rswag = only rails CSP header

    $ curl -v localhost:3000/api-docs/index.html
    * Host localhost:3000 was resolved.
    * IPv6: ::1
    * IPv4: 127.0.0.1
    *   Trying [::1]:3000...
    * Connected to localhost (::1) port 3000
    > GET /api-docs/index.html HTTP/1.1
    > Host: localhost:3000
    > User-Agent: curl/8.5.0
    > Accept: */*
    > 
    < HTTP/1.1 200 OK
    < content-type: text/html
    < content-security-policy: default-src 'self' https:
    < etag: W/"113226cfb0f49141d46dc22487e30e76"
    < cache-control: max-age=0, private, must-revalidate
    < x-request-id: 52970539-58e3-4696-9c9c-eb7e4a437fb1
    < x-runtime: 0.014395
    < server-timing: sql.active_record;dur=2.33, process_middleware.action_dispatch;dur=0.33
    < Content-Length: 4173
    < 
    <!-- HTML for static distribution bundle build -->
  2. api_only=false + Rails CSP disabled + rswag = only rswag CSP header

    $ curl -v localhost:3000/api-docs/index.html
    * Host localhost:3000 was resolved.
    * IPv6: ::1
    * IPv4: 127.0.0.1
    *   Trying [::1]:3000...
    * Connected to localhost (::1) port 3000
    > GET /api-docs/index.html HTTP/1.1
    > Host: localhost:3000
    > User-Agent: curl/8.5.0
    > Accept: */*
    > 
    < HTTP/1.1 200 OK
    < Content-Type: text/html
    < 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
    < x-request-id: 71d81806-2097-4a15-ab30-4e968fef0012
    < x-runtime: 0.012131
    < server-timing: sql.active_record;dur=1.77, process_middleware.action_dispatch;dur=0.37
    < Content-Length: 4173
    < 
    <!-- HTML for static distribution bundle build -->
  3. api_only=true + Rails CSP enabled + rswag = both rails + rswag CSP headers

    $ curl -v localhost:3000/api-docs/index.html
    * Host localhost:3000 was resolved.
    * IPv6: ::1
    * IPv4: 127.0.0.1
    *   Trying [::1]:3000...
    * Connected to localhost (::1) port 3000
    > GET /api-docs/index.html HTTP/1.1
    > Host: localhost:3000
    > User-Agent: curl/8.5.0
    > Accept: */*
    > 
    < HTTP/1.1 200 OK
    < Content-Type: text/html
    < 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'; 
    < content-security-policy: default-src 'self' https:
    < etag: W/"113226cfb0f49141d46dc22487e30e76"
    < cache-control: max-age=0, private, must-revalidate
    < x-request-id: 17201113-89a1-48fd-a31d-d9c5a50e3814
    < x-runtime: 0.019556
    < server-timing: sql.active_record;dur=16.43, process_middleware.action_dispatch;dur=0.37
    < Content-Length: 4173
    < 
    <!-- HTML for static distribution bundle build -->
  4. api_only=true + Rails CSP disabled + rswag = only rswag CSP headers

    $ curl -v localhost:3000/api-docs/index.html
    * Host localhost:3000 was resolved.
    * IPv6: ::1
    * IPv4: 127.0.0.1
    *   Trying [::1]:3000...
    * Connected to localhost (::1) port 3000
    > GET /api-docs/index.html HTTP/1.1
    > Host: localhost:3000
    > User-Agent: curl/8.5.0
    > Accept: */*
    > 
    < HTTP/1.1 200 OK
    < Content-Type: text/html
    < 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
    < x-request-id: ec56d2da-b0f0-4a7b-b94c-395b0e12a4e8
    < x-runtime: 0.010343
    < server-timing: sql.active_record;dur=1.98, process_middleware.action_dispatch;dur=0.33
    < Content-Length: 4173
    < 
    <!-- HTML for static distribution bundle build -->
jboler commented 2 months ago

If I edit this line to use just lowercase content-security-policy then the rswag CSP header always overwrites the Rails CSP header for requests to the rswag routes. Maybe this is a good way for a quick fix?

eric1234 commented 2 months ago

I'm confused. If it's a API only application why even have a CSP? What's the point?

If you really want to add a CSP to an API only application then my guess would be that appending the middleware like that in the CSP configuration is inserting it in the wrong location on the middleware stack. Take a look what rails middleware outputs with a normal Rails app, then what it looks like in a API only app. Use insert_before or insert_after to insert it to a similar place rather than just using use to put it on the bottom of the stack. See the docs for more info on modifying the middleware stack.

jboler commented 2 months ago

We have a CSP on our API because of some non-sensical Pentest requirements so I agree that it's an unusual case for api_only=true. There's still a bug here though - for a normal Rails app (api_only=false), when the Rails CSP config is enabled, it overwrites the rswag CSP header so rswag won't work. In my test I'm using the default Rails CSP middleware insertion, not manual insertion.

eric1234 commented 2 months ago

K, got around to digging into this and looks like you are correct, there is a bug here. It's not the location in the middleware stack like I was assuming. It is due to the switch from Rack 2 to Rack 3. Rails will check if there is already a CSP and if there is one it will not add one. So when rswag adds one Rails should not add it's own.

The problem as you noted is the casing of the header. When running under Rack 2 (which was what everyone was using back when I added the CSP to rswag) rails was using the header Content-Security-Policy therefore that is what I used in rswag. But Rack switched to all lowercase in Rack 3 to comply with the HTTP/2 spec. Rails therefore uses the old casing if running under Rack 2 and the new casing if running under Rack 3.

rswag has a few choices:

Personally I like the third option best as I prefer to not have the code duplication. But per the discussion on #428 there is some suggestion of trying to get rswag running under non-Rails apps. If that is a goal rswag wishes to pursue then probably option 2 is the best. And it's not really that much duplication really. Probably just define a constant that is conditionally decided when loaded then update the code to use that constant instead of the hard-coded string I currently have.

CONTENT_SECURITY_POLICY_HEADER = if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
  'Content-Security-Policy'
else
  'content-security-policy'
end

....within the middleware response...

return [ 200, { 'Content-Type' => 'text/html', CONTENT_SECURITY_POLICY_HEADER => csp }, [ render_template ] ]

Maybe if the maintainer of rswag can weigh in on option 2 vs option 3 one of us can put up a quick fix.

justwiebe commented 1 month ago

I would love to get a fix for this. Our app doesn't appear to be using api_only but I'm getting the exact same behaviour. This monkeypatch fixed it:

module Rswag::Ui::CSP
  def call(env)
    if base_path?(env)
      redirect_uri = env['SCRIPT_NAME'].chomp('/') + '/index.html'
      return [ 301, { 'Location' => redirect_uri }, [ ] ]
    end

    if index_path?(env)
      return [ 200, { 'Content-Type' => 'text/html', 'content-security-policy' => csp }, [ render_template ] ]
    end

    super
  end
end

Rswag::Ui::Middleware.prepend Rswag::Ui::CSP