github / github/secure_headers

Guide for transitioning from secure_headers to vanilla rails csp

Aperta
#466 3 commenti 1 reazione 0 assegnatari Vedi su GitHub
Lingua principale
Ruby
Stelle
3.2k
Fork
253
Merge medio
19h 11m
PR unite (30g)
1

Descrizione

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.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.