Add SRI

[elenatanasoiu]

Trello card

Github issue

Based on a previous attempt to add SRI - see PR here

What this does

This PR enables govuk_template to use SRI for the css and js assets it is serving to other projects.

Background

What is SRI

SRI will add an integrity attribute on your script tags:

<script src="https://example.com/example.css" 
integrity_no="sha384oqVuAfXRKap7fdgcCY5uykM6+R9GqQ8K/uxy9rx7HNQlGYl1kPzQho1wx4JwY8w" 
crossorigin="anonymous"></script>

The example above is generated automatically by sprockets-rails in your project if you do this:
<%= stylesheet_script_tag 'example', integrity: true %>

The way the digest value for the integrity attribute is calculated is by applying SHA256 on the contents of the file. In that way, the browser can double check that the right file is being received and hasn’t been tampered with (which would mean the contents would have changed and the resulting digest would be incorrect).

The problem with using SRI in govuk_template

The css files provided by govuk_template (for example: to the static project) are still css.erb files. In which case the contents of those files will change when they are processed in static (specifically there’s multiple <%= asset_path(...)%>s in there).

So here’s the problem illustrated:

1. The govuk_template gem compiles a file called govuk_template.css.erb which contains multiple asset_path lines, e.g.
   background-image: url(<%= asset_path 'images/govuk-crest.png' %>);

2. static receives this file and replaces all the asset_path lines with it’s own path:
   background-image: url(**http://static.dev.gov.uk/static**/images/govuk-crest-2x.png);
   Notice that this url will vary depending on the app that uses the gem. As a result, the contents of the resulting govuk_template.css will vary depending on which app is using the gem so the digest will also vary.
   This means you can’t generate the digest inside the gem, and the calculation will have to be left to the apps to do individually.

3. You can't use sprockets-rails in govuk_template directly because the assets it serves are also compiled into django, play, liquid, mustache and other crazy formats.
   So you can’t do this:
   <%= stylesheet_link_tag ‘govuk-template-print’, integrity: true%>
   and then leave it up to sprockets-rails to calculate the digest for you, because this will not work for django and all the other formats. They don’t know what stylesheet_tag is.
   Instead, this is what govuk_template does so it can be compiled into multiple languages:
   <link href="<%= asset_path "govuk-template-print.css" %>" media="print" rel="stylesheet" />

4. You can't calculate the integrity digest by hand in the gem, because of point 2 -> the content of the file will change depending on which application is using it.

Proposed solution

We use stylesheet_link_tag and set integrity: true, but we leave it up to the apps using the gem to actually generate the integrity digest. For the other languages (django, jijna, liquid, mustache) we define a stylesheet_include_tag method in their respective processors that will translate that tag into something they can understand:
<link href="<%= asset_path "govuk-template-print.css" %>" media="print" rel="stylesheet" />

[h-lame]

I wonder if it would be worth adding a runtime dependency for this gem on "sprockets-rails > 3.2". All we have at the moment is "rails > 3.1" but with the integrity attribute we need sprockets-rails 3.2 or greater. I don't know if it'll break in earlier rails versions though, or worse it still works but produces html tags with integrity attributes reading "true" that would block them from being loaded in modern browsers. Worth checking I think.

[h-lame]

minor nitpick: do we need this conditional? if options is .empty? then the .each_key iterator won't do anything, so it should be safe to leave this method as it was.

[h-lame]

Because we rely on 'active_support/core_ext/hash' we can call .stringify_keys on the hash directly, so we don't need to implement this here. It's in rails 3.1 so doesn't break our dependency (see: http://guides.rubyonrails.org/v3.1/active_support_core_extensions.html#stringify_keys-and-stringify_keys).

[h-lame]

If we added a require for activesupport/core/ext/array as well as the hash one then we could also rely on Array#extract_options! and not need to implement it here. It's also in rails 3.1 and wouldn't break our dependency (see: http://guides.rubyonrails.org/v3.1/active_support_core_extensions.html#options-extraction)

[elenatanasoiu]

sprockets-rails was extracted from rails 4.0+, so unfortunately it won't be very helpful to see a dependency on sprockets-rails if your version of rails is still 3.1.

I've added details about how SRI can be used with govuk_template gem in the docs, under docs/usage-rails.md

[h-lame]

"should parse the hash" could be more descriptive. "removes integrity and crossorigin keys from the hash" perhaps?

[elenatanasoiu]

Renamed

[h-lame]

"should parse the hash" could be more descriptive. "flattens the hash into a string of quoted html attributes" perhaps?

[elenatanasoiu]

Renamed

[h-lame]

Super happy you've introduced tests for all the processors used in the compilation step. ➕ 💯

[fofr]

Do we need to back this out because of alphagov/government-frontend#368

If yes, do we also need to yank old versions?

cc @h-lame @theleebriggs

[h-lame]

@fofr maybe we should back this out and re-release, but I'd say we don't need to yank old versions as it's not broken or a security risk or anything like that. Before we do anything though we need to investigate the impact more; it might be we're comfortable with the % of users it would affect and want to remerge the SRI stuff on government frontend.

[fofr]

Bug affects users of Firefox versions less than 52, which is about 0.7% of all users in the last month, about 315k users. There's also a proportion of users stuck on version 45 because they can't upgrade.