Add DSL for configuring Content-Security-Policy header

[pixeltrix]

Pushing up to get feedback on implementation

Documentation
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy

Examples
Global config in an initializer:

# config/initializers/content_security_policy.rb
Rails.application.config.content_security_policy do
  p.default_src :self, :https
  p.font_src    :self, :https, :data
  p.img_src     :self, :https, :data
  p.object_src  :none
  p.script_src  :self, :https
  p.style_src   :self, :https, :unsafe_inline

  # Specify URI for violation reports
  # p.report_uri  "/csp-violation-report-endpoint"
end

# Report CSP violations to a specified URI
# For further information see the following documentation:
# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy-Report-Only
# Rails.application.config.content_security_policy_report_only = true

Per controller configuration:

# Override policy inline
class PostsController < ApplicationController
  content_security_policy do |p|
    p.upgrade_insecure_requests true
  end
end

# Using literal values
class PostsController < ApplicationController
  content_security_policy do |p|
    p.base_uri "https://www.example.com"
  end
end

# Using mixed static and dynamic values
class PostsController < ApplicationController
  content_security_policy do |p|
    p.base_uri :self, -> { "https://#{current_user.domain}.example.com" }
  end
end

[jeremy]

Lovely. Thanks for digging in to this. Added comments on 1. protect_content method naming; 2. declaring the base content security policy rather than using a boolean flag to indicate whether to use :application as the default policy; and 3. separating policy definitions from policy enforcement decisions, moving report-only out to the controller.

Also, food for thought: should the base policy be applied to all HTML responses in a Rack middleware? Catching other mounted apps, static pages, error pages, etc.

[jeremy]

Good spot to specify the app-wide policy. Since this is the base policy for the whole app, be definitive:

# Set the base policy
config.action_controller.content_security_policy = :application

# No app-wide policy
config.action_controller.content_security_policy = nil

[pixeltrix]

Agree with a single base policy so this becomes irrelevant.

[jeremy]

Dupes #fetch impl above.

[pixeltrix]

Irrelevant now that collection is going away

[jeremy]

What would the caller for this case look like?

[pixeltrix]

It's if you need to disable the value on a controller basis, e.g.

class LegacyPagesController < ApplicationController
  content_security_policy do |p|
    p.sandbox false
  end
end

[jeremy]

Would treat the policy itself and how it's enforced as separate concerns. Can hoist this (and header_name) out to the controller and handle in set_content_security_policy.

[jeremy]

directives.transform_values { |sources| sources.map(&:dup) }

[jeremy]

Can incorporate the existence check into the fetch:

MAPPINGS.fetch(source) do
  raise ArgumentError, "Unknown content security policy source mapping: #{source.inspect}"
end

[jeremy]

Appears that a directive sources will never be nil, should just be absent. Raise here?

[pixeltrix]

This is when a directive is set to false - there's a compact operation that removes these nil values in header_value:

def header_value(context = nil)
  build_directives(context).compact.join("; ") + ";"
end

[jeremy]

Maybe an edge case, but it could be sensible to set the header even if the policy is empty, making it clear that we've set an empty policy rather than omitting it.

[pixeltrix]

Yes, agreed.

[jeremy]

Do we foresee a controller action providing a custom policy that should preclude the declared controller policy for that action?

[pixeltrix]

The unless: :content_security_policy_present? was more about handling upgraded apps that may have set the header manually - didn't want to break them.

[jeremy]

Not sure about this policy collection API for central definition of multiple policies. I think in practice we'll have a single base policy that's extended in controller subclasses. If there's a set of controllers that use a stricter policy, I'd probably start by mixing in a concern that sets the strict policy.

[pixeltrix]

Agreed - a single base policy with overrides either in concern or a common parent controller class makes the implementation simpler and doesn't lose any flexibility.

[pixeltrix]

@jeremy thanks for the review - agree that we should generate the default policy for all HTML responses so will move the policy class out to Action Dispatch and then just have the override implementation in Action Controller.

[kaspth]

Minor nits, but overall on API 👍

[kaspth]

Could this be simplified with some kwarg help?

def content_security_policy_report_only(report_only = true, **options)
end

[kaspth]

Could do:

MAPPINGS.fetch(source) do
  raise ArgumentError, …
end

[kaspth]

Extra space after p.report_uri.

[kaspth]

Should this config be commented out for upgrading apps? Or should it be loose enough that apps can upgrade seamlessly and get more security?

[pixeltrix]

Do initializers get generated for upgrading apps automatically?

[kaspth]

IIRC, yes.

[pixeltrix]

Okay, will investigate what the preferred solution is to be once the beta ships and it's easier to test against upgrading real world apps.

[dwightwatson]

Is there a guide or any documentation on how to use this in development? For example, JavaScripts served through the asset pipeline or Webpacker (either precompiled or with the hot-reloading development server) are all HTTP and therefore left broken with the default configuration.

[pixeltrix]

@dwightwatson I thought that 'self' would allow HTTP assets served from the same domain and https: allows third party assets from HTTPS sources? I took the defaults from #24961 which is described as 'lenient'.

[dwightwatson]

@pixeltrix That's what I thought as well, and after digging further into it that appears to be correct.

The issue I'm seeing is related but has to do with unsafe-eval. Looks like Webpacker bundles up modules using the eval method and this default CSP configuration doesn't allow for that.

Should I open another ticket to follow this issue instead? It might require the CSP configuration to be altered to allow for Webpacker assets or Webpacker might need to be configured to bundle another way.

Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' https:".

[pixeltrix]

@dwightwatson is it the webpacker gem that uses eval or webpack itself?

[pixeltrix]

@dwightwatson I wonder whether rails/webpacker#1011 is related? Can you change config.devtool to cheap-module-source-map and see whether that fixes your problem.

[dwightwatson]

I was just reading the same thing - yeah it looks like that configuration change fixes the issue, all working locally now. Is making that configuration the new default something that could be reconsidered?

[pixeltrix]

Is making that configuration the new default something that could be considered?

Possibly - will raise it in the Campfire channel.

[ryenski]

I'm also having this issue, using rack-livereload:

Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'self' https:". Either the 'unsafe-inline' keyword, a hash ('sha256-rlZ58lcfumEQqoV6vyIbnvH6c7q0w93savs/e1DR0TA='), or a nonce ('nonce-...') is required to enable inline execution.

Refused to load the script 'http://localhost:35729/livereload.js?' because it violates the following Content Security Policy directive: "script-src 'self' https:".

[pixeltrix]

@crispinheneise the quick fix is to add :unsafe_inline to the p.script_src line in your content_security_policy.rb initializer, however you lose some of the security that the CSP gives you in preventing XSS attacks. Ideally the rack-livereload middleware would add a nonce to the script tags it injects and then modify the CSP header to add nonce-<randomvalue>. That would allow you to use live reload in development without affecting your production environment.

[ryenski]

This may be an edge case, or it might belong to rack-livereload - if so please ignore.

In order to get CSP to work with rack-livereload I had to add unsafe-inline as @pixeltrix mentioned. Plus, I also had to add a script_src for port 35729 and a connect_src directive for the web socket. This is only needed in development, so this is what I did in content_security_policy.rb:

  if Rails.env.development?
    p.script_src  :self, :https, :unsafe_inline, 'http://localhost:35729'
    p.connect_src 'http://localhost:35729', 'ws://localhost:35729'
  else
    p.script_src  :self, :https
  end

[pixeltrix]

@crispinheneise only issue is that change is that you'll only find out about your own app's use of inline scripts when you deploy to production 😟

[nsingh]

Hi @pixeltrix ,

Thanks for implementing CSP in Rails! We are still on Rails 5.0.6 so we had to backport your code to get things in motion.

I did have one question regarding the ability to set both headers:

POLICY = "Content-Security-Policy".freeze
POLICY_REPORT_ONLY = "Content-Security-Policy-Report-Only".freeze

Currently it seems like if Report-Only option is true, it takes precedence:-

def header_name(request)
  if request.content_security_policy_report_only
    POLICY_REPORT_ONLY
  else
    POLICY
  end
end

The CSP documentation says that both headers can be present in the same response and could have different policies.

I am wondering if this was deliberate and whether Rails has plans to honor both headers in the near future?