github / github/secure_headers
Guide for transitioning from secure_headers to vanilla rails csp
- Dominant language
- Ruby
- Stars
- 3.2k
- Forks
- 253
- Avg merge
- 19h 11m
- Merged PRs (30d)
- 1
Description
This is more of a meta issue where I'm going to drop notes before actually coming up with a documented plan.
We intend to go through this process in the not so distant future and will have lessons to learn. GitHub uses the following features:
* Tons of dynamic modifications via 1st append_content_security_policy_directives`
* No less than three separate policies in one application - standard, error pages, some framed content
* The automatic handling of calling `append_content_security_policy_directives` on a value that is configured as `'none'`
https://bauland42.com/ruby-on-rails-content-security-policy-csp/ does a good job of explaining both APIs.
https://medium.com/just-tech/content-security-policy-ruby-on-rails-836d0c7ba098j is a story of someone doing exactly this (and calling out one important difference.
## base config
Creating your base config from the secure_headers config is mostly a find/replace. Instead of a hash, you're configuring an object with methods named after directives. They take varargs so the arrays need to be splatted.
```ruby
# TODO: provide find/replace regex for automating this
# secure_headers
SecureHeaders::Configuration.default do |config|
config.csp = {
directive_name: some_array
}
end
# rails
Rails.application.config.content_security_policy do |config|
config.directive_name *some_array
end
```
## dynamic modifications
As mentioned in https://bauland42.com/ruby-on-rails-content-security-policy-csp/, the rails API __overwrites__ config values. In secure_headers land, we generally append sources to an existing list rather than have to redefine the entire list upon use.
```ruby
def some_controller_action
the_value = request.content_security_policy.frame_src
is_now_nil =request.content_security_policy.frame_src
end
def some_controller_action_overwriting
# whatever was in frame_src was lost/overwritten and this only contains the new_sources
request.content_security_policy.frame_src new_sources
end
def some_controller_action_appending
current_config = request.content_security_policy.frame_src
request.content_security_policy.frame_src *(current_config + new_sources)
end
```
So the obvious workaround is to grab the current config and then re-set the value along with additions. This shim seems to generally solve the problem while backporting some functionality, notably:
* When calling `append_content_security_policy_directives` on a currently undefined directive, populate the newly defined directive with the current value of `default_src`
* When calling `append_content_security_policy_directives` on a directive with the value of `none`, discard the `none` value
```ruby
# this is just one option. We don't use the railtie so we're calling `SecureHeaders.append_content_security_policy_directives(request, additions)`. A simple multiline regex can convert these to `append_content_security_policy_directives(additions)`. I intend to write a rubocop for this so maybe I'll get the autocorrect working.
def append_content_security_policy_directives(additions)
# TODO: add allowlist for keys in the additions hash (re-use some constant)
if GitHub.flipper[:vanilla_csp].enabled?(current_user)
# current_content_security_policy gives us a safe, duplicate copy to use.
current_policy = current_content_security_policy
additions.each do |directive, source_values|
# TODO handle other unsupported values
next if directive == :preserve_schemes
if directive == :report_uri
request.content_security_policy.report_uri source_values
else
current_value = current_policy.send(directive)
if config.nil?
config = []
config = current_policy.default_src
end
if current_value == %w('none')
request.content_security_policy.send(directive, *source_values.compact.uniq)
else
request.content_security_policy.send(directive, *(current_value + source_values).compact.uniq)
end
end
end
else
SecureHeaders.append_content_security_policy_directives(request, additions)
end
end
def some_before_filter_on_every_request
if GitHub.flipper[:vanilla_csp].enabled?(current_user)
# we don't need to disable built-in rails because it exits if a policy is set
# depending on how middleware is loaded, we could end up generating a policy that is overwritten by secure_headers
SecureHeaders.opt_out_of_header(request, :csp)
end
end
```
## testing
By default, rails does not include middleware when running tests. If you're testing your CSP (hint: you should) then you've already done this in your test class:
```ruby
def app
@app ||= Rack::Builder.new do
use SecureHeaders::Middleware
use ActionDispatch::ContentSecurityPolicy::Middleware # this is the new addition
run GitHub::Application
end
end
end
```
## differing policies
If you're testing your policies (seriously: do it) then you won't get exact matches without some extra steps.
Rails won't dedup anything so you'll be calling `uniq` if you care. Rails doesn't like `nil`s in config values, `secure_headers` doesn't mind and will filter them. secure_headers tries to do fancy stuff around consolidating `*` or overlapping config values, rails is more literal. secure_headers omits schemes by default, rails does what you tell it to.
secure_headers generates a policy that is `default_src ... everything else ... report_uri` whereas rails doesn't enforce this order.
## config values that won't translate
The `preserve_schemes` / `disable_nonce_backwards_compatibility` concepts do not exist in rails.
## secure_headers with CSP disabled is somewhat of an edge case (for me)
I've never done it but while I was working on this, a test of ours that mucks with 1st X-Frame-Options raised an error about default_src not being configured 😆 thar be dragons.
## non-html resources don't get CSP
I'll add details, but there's a situation where you need to send csp on PDF resources and you may want to for e.g. api requests.
## And more
I'll keep updating this issue body as we roll out the change and I'd love to incorporate anything you've learned.
Contributor guide
Assessment
This issue has not been assessed yet.