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

Guide for transitioning from secure_headers to vanilla rails csp #466

Open
oreoshake opened this issue Feb 12, 2021 · 3 comments
Open

Guide for transitioning from secure_headers to vanilla rails csp #466

oreoshake opened this issue Feb 12, 2021 · 3 comments

Comments

@oreoshake
Copy link
Contributor

oreoshake commented Feb 12, 2021

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.

# 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.

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
  # 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:

  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 nils 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.

@agrobbin
Copy link

@oreoshake I thought it worth mentioning that I just opened rails/rails#46859, which allows modification of Rails' global CSP. It is a much simpler implementation than what you describe in your comment above, but it's what I've been using in some projects via a monkey patch, and has worked reasonably well!

@oreoshake
Copy link
Contributor Author

That’s wonderful @agrobbin!

@zendesk-yianna
Copy link

This is really good work! Thank you :)

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

No branches or pull requests

3 participants