Provide a default Content Security Policy (CSP) that is lenient yet secure #24961
Conversation
Building on the concept of default headers, it would be great if CSP was applied by default. This is mostly geared towards new applications. Unfortunately, making this "report only" will generate a warning in the console since the policy, understandably, lacks a report-uri value. Adding a default report-uri realistically would require adding a CSP reporting endpoint to rails by default. Because of these factors, applying the potentially breaking header probably better suited as part of a generator script for new applications but I don't know the cleanest way of accomplishing this. In past lives (e.g. Twitter scala applications), applying CSP by default had amazing returns. The suggested policy should work for most applications and the browser console literally tells you what modifications need to be made. Adding code that violates this policy could be caught early on and people can choose to work within the suggested boundaries or they can decide to modify the policy. Naturally, there will be those who disable the policy entirely. That is entirely up to them but at least some thought will be put into it. This is a bit contentious, of course.
|
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @sgrif (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
@sgrif lol. I was about to assign to me. I like the idea and I agree we should not aim perfection. I'll review it. |
|
@rafaelfranca I'm in your head. ;) 👍 on this proposal from me FWIW |
|
Can you add some documentation explaining CSP? Also, we should probably have an option to disable this. |
|
@maclover7 glad to. However, I'd prefer to hold off until we know where the application will end up as there may be different context available. e.g. an explanation in the |
|
👍 My recommendation is that we stick the CSP config in |
|
@maclover7 this can be achieved by removing the header, by specifying a custom one on |
|
@vipulnsward I think making this a default header is probably a bad idea since it will break any application using risky behaviors. It should only be a default for newly generated applications. Although, introducing this as a breaking change for the next major version is an interesting idea... |
|
@oreoshake Unfortunately, the "breaking change boat" for 5.0 has left the dock, and we are in the feature freeze right for 5.0 (so no breaking changes 😢) @vipulnsward I was more asking about whether, for example, we could have a nice API for configuring the |
Yeah, that's what I figured. But rails 6.0 will be a thing! Maybe then it could be a |
|
@maclover7 @vipulnsward I recommend you to check #15777. |
|
@chancancode @kaspth @jeremy mind to take a look in the idea? I'm positive to it as new default in generated applications. |
|
I don't think this moves forward on @chancancode's objection to the previous CSP PR (which I agree with):
Also second @maclover7 in that this particular PR's approach would be far too difficult to modify or turn off. SpitballingIMO the ideal API looks more like how CSRF protection works today - it should live in ApplicationController, and have a scary-ish name that makes it clear that "if you comment this out, you are Doing Something Dangerous!" It should also use a Hash-based API for easy modification of just a few settings. class ApplicationController < ActionController::Base
protect_from_forgery with: :exception
private
# Rails takes care of transforming this into a CSP policy string
def content_security_policy
{
"default-src": %w[self https],
"font-src": %w[self https data],
"img-src": %w[self https data],
"object-src": %w[none],
"script-src": %w[self https],
"style-src": %w[self https unsafe-inline]
}
end
end
class MyInsecureController < ApplicationController
private
def content_security_policy
{
"default-src": %w[self]
}
end
end
class MyControllerWithAdditionalTrustedOrigin < ApplicationController
private
# Append a trusted origin to all directives, not sure if this actually works but you get the idea
def content_security_policy
policy = super
policy.each_with_object({}) do |memo, (k,ary)|
ary << "*.mytrustedorigin.com"
memo[k] = ary
end
end
end
|
|
I agree with the motivation and I'm sympathetic to the perfection-as-enemy-of-the-good perspective, but this is an arena that deserves a pretty high bar to introduce an important, far-reaching new concept to Rails users. The approach is clear: per-resource policy. We're blocked on implementation interest & effort. Really, the major push is documentation and communication. Per-controller declarative policy is straightforward to implement in comparison to writing up a fantastic CSP guide. #15777 has the essential implementation in place. It needs documentation and implementation polish, and default policy guidance for new apps. |
|
@jeremy I'd be happy to help out with the Content Security Policy guide if we're still willing to go forward with #15777. I've made a (admittedly non-comprehensive) CSP writeup when I pitched the addition of CSP headers for GitLab. While the initial attempt to add the headers ran into some roadblocks I still fully intend to ship it in the next few months. Between us and the fantastic information GitHub, Twitter, and Dropbox have published about their experiences with CSP I'm sure we can put something very useful together :) |
|
@connorshea we need someone to kickstart a guide to move forward with #15777 — you seem like just the guy to do it! I skimmed your referenced write up, it seems like a fine place to open a pull request and discuss further. Do take a stab at it! ❤️
Concurred 🤘 |
|
@connorshea happy to help with any revision/proofreading on that |
|
@connorshea let's try it separately. I'll petition the student who worked on the implementation to see if he wants to pick it up again. |
|
@connorshea @nateberkopec 👋, did any of you make some headway on a CSP guide? 😁 |
|
@kaspth unfortunately I haven't had time recently. |
|
@connorshea no worries 😉 |
|
In #27583 it was proposed to merge secureheaders into core to address this. A few technical details got in the way:
The first concern was addressed within the issue – @rafaelfranca suggested that the secureheaders gem become a part of the rails project, and generate the application with a call to that gem and a new initializer. @oreoshake was nice enough to do the legwork to get secureheaders re-licensed under MIT, addressing the second concern. As for the last concern, the effort in #15777 is not mutually exclusive with adding secureheaders to Rails. It may unstick this 2+ year process if the API (and docs) are developed alongside the other best-practices for security provided by secureheaders (HPKP, XFO, etc). As it stands, someone who wants to secure a new Rails app should use secureheaders today – so why not make that the default starting point? (I'd like to help with this if it seems like a reasonable approach.) |
|
Superseded by #31162 Thanks for the PR! ❤️ |
|
I have never been happier to see a PR of mine get closed unmerged 😄 |
Summary
This is not a new idea, "Add Content Security Policy(CSP)" has been proposed but seems to be stuck in analysis paralysis. Perfect being the enemy good, here's an alternate proposal solving a slightly different problem. Rather than a general solution, this PR proposes a generic CSP that may work for most applications.
Building on the concept of
default_headers, it would be great if CSP was applied by default. This is mostly geared towards new applications. Unfortunately, making this "report only" will generate a warning in the console since the policy, understandably, lacks a report-uri value. Adding a default report-uri realistically would require adding a CSP reporting endpoint to rails by default. Perhaps it could use the middleware @xuchu provided.Because of these factors, applying the potentially breaking header probably better suited as part of a generator script for new applications but I don't know the cleanest way of accomplishing this.
In past lives (e.g. Twitter scala applications), applying CSP by default had amazing returns. CSP adoption and awareness went from "wat"/"meh" to "this is useful". The suggested policy should work for most applications and the browser console literally tells you what modifications need to be made. Brand new applications do not produce any violations. Adding code that violates this policy could be caught early on and people can choose to work within the suggested boundaries or they can decide to modify the policy. Naturally, there will be those who disable the policy entirely. That is entirely up to them but at least some thought will be put into it.
This is a bit contentious, of course.
Other Information
Needs tests, but I wanted to wait for a better integration point before I write tests I know I'm going to throw away.
An Introduction to Content Security Policy
GitHub's CSP Journey
No, no, no. Thank you Rails!