script hash rails integration PoC #67

oreoshake · 2013-09-04T07:43:10Z

Application demonstrating this: https://github.com/oreoshake/script_hash_test
Video demo: https://www.youtube.com/embed/Bc2hvziTRxg

Two pieces:

Rake task to find all inline script, compute the script-hash value, store in config/script_hash.yml

AND

Contains new before_filter to subscribe to ActiveSupport::Notifications events (rendering partials/templates)
All rendered templates write to an env value that contains all hashes for each rendered template/partial
Middleware then (stupidly) does a find/replace or adds a directive containing the script hash values

Based on "simple" script hash proposal.
Need to verify hashed values with various methods.

This makes me think the before_filters should create a ContentSecurityPolicy object and build the entire header in the middleware.

oreoshake · 2013-09-04T07:44:58Z

Super hacky, no tests, no haml support(?), only works for rails 3.1+. Not going to supply a Capistrano task for now... suggesting guard-rake because if somehow you forgot to update your hashes before deploy... clearly you didn't run/have integration tests.

oreoshake · 2013-09-04T08:58:56Z

Only works if entire tag is on one line lol. Don't use scan.

oreoshake · 2013-09-06T20:47:29Z

lib/secure_headers/script_hash.rb

+        # jank to make sure we're messing w/ the right header
+        header_name = ContentSecurityPolicy.new(nil, :ua => env["HTTP_USER_AGENT"]).name
+
+        csp = headers[header_name]


Error if no CSP policy set

oreoshake · 2013-09-06T22:10:53Z

Writeup of the why and how of this branch: http://nmatatal.blogspot.com/2013/09/how-my-script-hash-poc-works.html

oreoshake · 2013-09-12T05:59:11Z

JS equivalent hash computation with jQuery and cryptojs

$.each($('script'), function(index, x) { console.log(CryptoJS.SHA256(x.innerHTML).toString(CryptoJS.enc.Base64));});

oreoshake · 2013-10-15T16:06:05Z

This PoC has served it's purpose

oreoshake · 2014-01-17T22:19:05Z

Reopening because someone else might be motivated to make this a thing before I get to it.

Some people would rather support dynamic hashing (in development only) rather than have an external process (guard) grep/populate hashes.

I still disagree. Whatevs.

spikebrehm · 2014-08-21T18:12:21Z

Sweet, thanks for this PoC. What do you think about a more simple approach of using a specific Rails helper, say, inline_script_with hash:

<%= inline_script_with_hash do %>
  var timestamp = Date.now();
<% end %>

which explicitly computes a hash, and saves it to a list for later (controller instance variable, request.env, whatever), and the SecureHeaders gem utilizes the list when building the header?

Or is this the approach you mention here:

Some people would rather support dynamic hashing (in development only) rather than have an external process (guard) grep/populate hashes.

spikebrehm · 2014-08-21T19:29:49Z

FWIW I got this working by copying over your SecureHeaders::ScriptHash middleware into our app, and with this helper:

module InlineScriptHelper
  def inline_script_with_hash(&block)
    content = "\n#{capture(&block)}"
    (request.env['script_hashes'] ||= []) << compute_inline_script_hash(content)
    content_tag :script, content
  end

  def compute_inline_script_hash(content)
    Digest::SHA256.base64digest(content)
  end
end

oreoshake · 2014-08-23T01:39:28Z

@spikebrehm I like it. A lot. I still have to support mustache but that shouldn't be too hard.

spikebrehm · 2014-08-25T21:42:08Z

Are there plans to change secureheaders to support computing the header in a middleware, in a less-hacky way? I notice this is a year old. If not, I suppose we'll fork this to support it, but I'd rather not fork if we can help it.

oreoshake · 2014-08-25T21:58:53Z

This branch was just a PoC. I welcome any pull requests on this! Don't fork!

oreoshake · 2014-08-25T22:08:59Z

One issue with this dynamic hashing is that it doesn't buy you anything. I'd want the hash values to be static or otherwise limited in production

spikebrehm · 2014-08-25T22:16:49Z

Sure, I can agree that hashing of dynamic script content may be a bad idea, and that the helper approach makes it easy to do that.

However, I do think that the helper approach is simpler and easier to use than Rake task / YAML approach.

spikebrehm · 2014-09-02T23:15:47Z

lib/secure_headers/script_hash.rb

+
+      # need to settle on stuffs
+    def hash_source_expression(hashes, format = "sha256", delimeter = "-", hash_delimeter = " ", wrapper = "'")
+      wrapper + format + delimeter + hashes.join(hash_delimeter) + wrapper


This causes improperly formatted script-src when there's multiple script hashes. As is, the output for two hashes, abcde and fjhij is:

script-src 'sha256-abcde fghij'

This should be:

script-src 'sha256-abcde' 'sha256-fghij'

Here's the proper source code:

def hash_source_expression(hashes, format = "sha256", delimeter = "-", hash_delimeter = " ", wrapper = "'") hashes.map { |hash| wrapper + format + delimeter + hash + wrapper }.join(hash_delimeter) end

If you like I can make a PR to this branch that fixes.

It should also be sha1. Only sha1 is supported for now afaik.

Please do submit a PR if you have the time, including your other changes. Or submit a fresh PR :)

Here we go: #109

Should it really be sha1? sha256 works for me in Chrome 37.

Oh sweet. Didn't know that.

There exists a bug which causes ian improperly formatted `script-src` expression for multiple script hashes. As is, the output for two hash `abcde` and `fjhij` is: script-src 'sha256-abcde fghij' This should be: script-src 'sha256-abcde' 'sha256-fghij'

Conflicts: lib/secure_headers/railtie.rb spec/lib/secure_headers_spec.rb

Fix support for mutliple script hashes

oreoshake · 2014-09-03T02:31:14Z

However, I do think that the helper approach is simpler and easier to use than Rake task / YAML approach.

Totally. So how about the helper works in !production and then we just make the hash generation just another deploy step?

I'd love to iron all this out and ship something somewhat soon. I should have the cycles too.

spikebrehm · 2014-09-03T03:59:45Z

Totally. So how about the helper works in !production and then we just make the hash generation just another deploy step?

Today I've actually been implementing the YAML approach, and there does seem to be a benefit in the fact that it's literally impossible to whitelist dynamic scripts. The helper approach doesn't have that benefit, because you will always be whitelisting the dynamic value post-evaluation. That is certainly a nice development experience, but it may be too permissive given an organization's needs.

The downside of the YAML approach is that the developer would have to run the Rake task and restart the Rails server every time they add/update a <script> tag. That sounds like a PITA, but hopefully in practice app developers shouldn't actually be writing script tags; there should be patterns in place that make writing script tags a special case.

You're proposing a hybrid approach? What would that look like?

oreoshake · 2014-09-03T04:37:01Z

I received some feedback that a helper that dynamically computes the values during development is pretty much necessary.

One option is to always calculate hashes dynamically AND compare to a list of known hashes and respond to mismatches in a configurable way. i.e. "OH NOEZ raise exception" vs "yeah, we logged that out and we'll clean it up in the next release, don't blow up in the meantime"

csp = {
  ...,
  rescue_from_script_hash_mismatch: lambda {
    if RAILS.env.production?
      raise "go home, you're drunk"
    elsif RAILS.env.development?
      puts "oops, unexpected hash, run be rake secure_headers:generate_hashes"
    end
  }
}

Perhaps the lack of a rescue handler implies that the dynamic calculation should not even happen, and the inline script helper becomes a noop, falling back to the yaml-only behavior.

spikebrehm · 2014-09-03T21:12:56Z

Interesting, I like that. So the script-finding regex will also have to support finding the ERB statement containing the call to the helper.

spikebrehm · 2014-09-03T21:24:48Z

lib/secure_headers.rb

+    def script_hashes
+      if @script_hashes
+        @script_hashes
+      elsif superclass.respond_to?(:script_hashes) # stop at application_controller


This could be an issue if the superclass is itself a subclass of ApplicationController, yeah? i.e.:

MyController < AuthenticatedController < ApplicationController

Is it too hacky to recursively look up the superclass tree until it finds superclass.name == 'ApplicationController'?

Sounds about right, I think that's only the case when you override settings from one controller to another. I guess this has just never come up 🤷

oreoshake · 2014-09-03T23:05:58Z

Yeah, I just hope we don't run into whitespace issues.

oreoshake and others added 5 commits September 3, 2013 16:01

detect partials

bcf3418

middleware to gsub in the script_hashes

9982d8b

Rake task to generate script_hashes

8bcc240

mustache support + cleanup

b09c4aa

yaml is now a test dependency :(

128b39b

Neil Matatall added 5 commits September 5, 2013 16:58

smarter dynamic script detection

d034b14

1.8.7 doesn't support named captures

4e8dbae

Openssl parity

958bbbf

cleanup

4170cce

I don't understand rake parameters

5d7e7c9

oreoshake reviewed Sep 6, 2013
View reviewed changes

oreoshake closed this Oct 15, 2013

oreoshake reopened this Jan 17, 2014

spikebrehm reviewed Sep 2, 2014
View reviewed changes

spikebrehm and others added 5 commits September 2, 2014 17:29

Merge branch 'master' into script_hash

e76ca02

Conflicts: lib/secure_headers/railtie.rb spec/lib/secure_headers_spec.rb

Merge pull request #109 from spikebrehm/fix-multiple-script-hash

069f176

Fix support for mutliple script hashes

clean up some jank around passing hashes to the middleware

207a1e3

rip out debug stuff

18e8c7f

spikebrehm reviewed Sep 3, 2014
View reviewed changes

oreoshake closed this Nov 20, 2014

oreoshake deleted the script_hash branch December 6, 2014 01:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

script hash rails integration PoC #67

script hash rails integration PoC #67

oreoshake commented Sep 4, 2013

oreoshake commented Sep 4, 2013

oreoshake commented Sep 4, 2013

oreoshake Sep 6, 2013

oreoshake commented Sep 6, 2013

oreoshake commented Sep 12, 2013

oreoshake commented Oct 15, 2013

oreoshake commented Jan 17, 2014

spikebrehm commented Aug 21, 2014

spikebrehm commented Aug 21, 2014

oreoshake commented Aug 23, 2014

spikebrehm commented Aug 25, 2014

oreoshake commented Aug 25, 2014

oreoshake commented Aug 25, 2014

spikebrehm commented Aug 25, 2014

spikebrehm Sep 2, 2014

oreoshake Sep 2, 2014

spikebrehm Sep 3, 2014

oreoshake Sep 3, 2014

oreoshake commented Sep 3, 2014

spikebrehm commented Sep 3, 2014

oreoshake commented Sep 3, 2014

spikebrehm commented Sep 3, 2014

spikebrehm Sep 3, 2014

oreoshake Sep 3, 2014

oreoshake commented Sep 3, 2014

script hash rails integration PoC #67

script hash rails integration PoC #67

Conversation

oreoshake commented Sep 4, 2013

oreoshake commented Sep 4, 2013

oreoshake commented Sep 4, 2013

oreoshake Sep 6, 2013

Choose a reason for hiding this comment

oreoshake commented Sep 6, 2013

oreoshake commented Sep 12, 2013

oreoshake commented Oct 15, 2013

oreoshake commented Jan 17, 2014

spikebrehm commented Aug 21, 2014

spikebrehm commented Aug 21, 2014

oreoshake commented Aug 23, 2014

spikebrehm commented Aug 25, 2014

oreoshake commented Aug 25, 2014

oreoshake commented Aug 25, 2014

spikebrehm commented Aug 25, 2014

spikebrehm Sep 2, 2014

Choose a reason for hiding this comment

oreoshake Sep 2, 2014

Choose a reason for hiding this comment

spikebrehm Sep 3, 2014

Choose a reason for hiding this comment

oreoshake Sep 3, 2014

Choose a reason for hiding this comment

oreoshake commented Sep 3, 2014

spikebrehm commented Sep 3, 2014

oreoshake commented Sep 3, 2014

spikebrehm commented Sep 3, 2014

spikebrehm Sep 3, 2014

Choose a reason for hiding this comment

oreoshake Sep 3, 2014

Choose a reason for hiding this comment

oreoshake commented Sep 3, 2014