[WIP] Implement secure_headers gem #282

jgarber623-gov · 2018-01-10T15:23:17Z

Checklist

I have…

run the application locally (bin/rails server) and verified that my changes behave as expected.
run static code analysis (bin/rubocop) and vulnerability scan (bin/brakeman) against my changes.
run the test suite (bin/rake spec) and verified that all tests pass.
summarized below my changes and noted which issues (if any) this pull request fixes or addresses.
thoroughly outlined below the steps necessary to test my changes.

Summary of Changes

This pull request adds Twitter's secure_headers gem to the project and uses its features to implement more-secure HTTP response headers (e.g. X-Frame-Options, Content-Security-Policy) for routes served by the Rails app.

The move to do this was prompted by a conversation with @jobertabma at HackerOne. He noted the remote possibility of an attacker compromising a developer machine and taking advantage of our previous decision to offload header-setting to nginx instead of Rails/Action Dispatch.

Testing

To verify the changes proposed in this pull request…

clone this repo,
git checkout secure-headers,
set up development dependencies according to CONTRIBUTING.md,
run bin/rails server,
load up http://localhost:3000 and check your browser's console for the appropriate response headers.

jgarber623-gov · 2018-01-10T15:23:55Z

.ebextensions/nginx.config

-
-        # HSTS (HTTP Strict Transport Security)
-        # This header tells browsers to cache the certificate for six months and to connect exclusively via HTTPS.
-        add_header Strict-Transport-Security "max-age=15768000" always;


I'm not sure if we'd want to move some of these headers into the location blocks covering assets and/or error pages.

@hlieberman-gov What's your take?

jgarber623-gov · 2018-01-10T15:24:15Z

config/environments/production.rb


-  # Disable ActionDispatch's default headers (these are handled by nginx)
-  config.action_dispatch.default_headers = {}
-


No need for this anymore…

jgarber623-gov · 2018-01-10T15:25:26Z

config/initializers/action_dispatch.rb

+  config.action_dispatch.default_headers.merge!(
+    'X-UA-Compatible' => 'IE=Edge'
+  )
+end


secure_headers doesn't cover this IE-specific header, so adding this initializer here and merging it with Action Dispatch's default headers.

jgarber623-gov · 2018-01-10T15:26:31Z

config/initializers/secure_headers.rb

+  config.hsts = 'max-age=15768000'
+  config.referrer_policy = %w[origin-when-cross-origin strict-origin-when-cross-origin]
+  config.x_content_type_options = 'nosniff'
+  config.x_download_options = 'noopen'


This one's new to me, but interesting: https://msdn.microsoft.com/library/jj542450(v=vs.85).aspx

jgarber623-gov · 2018-01-10T15:26:58Z

config/initializers/secure_headers.rb

+  }
+
+  config.hsts = 'max-age=15768000'
+  config.referrer_policy = %w[origin-when-cross-origin strict-origin-when-cross-origin]


I wasn't familiar with the Referrer Policy spec, but I like it.

jgarber623-gov · 2018-01-10T15:27:39Z

config/initializers/secure_headers.rb

+  config.x_content_type_options = 'nosniff'
+  config.x_download_options = 'noopen'
+  config.x_frame_options = 'deny'
+  config.x_permitted_cross_domain_policies = 'none'


Flaaaaaaaaash.

hlieberman-gov · 2018-01-10T18:57:16Z

.ebextensions/nginx.config

-        # you can tell the browser that it can only download content from the domains you explicitly allow
-        # CSP can be quite difficult to configure, and cause real issues if you get it wrong
-        # There is website that helps you generate a policy here http://cspisawesome.com/
-        add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' https://dap.digitalgov.gov https://www.google-analytics.com; style-src 'self' 'unsafe-inline'; img-src 'self' data: https://www.google-analytics.com *.tile.openstreetmap.org; frame-ancestors 'none'; frame-src https://www.youtube.com;" always;


We only need to make sure this CSP header is delivered on root requests. For the error pages in particular, as well as any other directly delivered HTML, we should add one in.

hlieberman-gov · 2018-01-10T18:58:29Z

.ebextensions/nginx.config

-
-        # MIME type sniffing security protection
-        #	There are very few edge cases where you wouldn't want this enabled.
-        add_header X-Content-Type-Options nosniff always;


Yeah, we want this one delivered on static content.

hlieberman-gov · 2018-01-10T18:59:32Z

.ebextensions/nginx.config

-
-        # The X-Frame-Options header indicates whether a browser should be allowed
-        # to render a page within a frame or iframe.
-        add_header X-Frame-Options SAMEORIGIN always;


Do we ever iframe this page internally? Might be worth changing to DENY; either way, we should preserve this on static content.

hlieberman-gov · 2018-01-10T19:00:18Z

.ebextensions/nginx.config

-
-        # The X-XSS-Protection header is used by Internet Explorer version 8+
-        # The header instructs IE to enable its inbuilt anti-cross-site scripting filter.
-        add_header X-XSS-Protection "1; mode=block" always;


Meh. If we can, great, otherwise, don't sweat it.

hlieberman-gov · 2018-01-10T19:00:51Z

.ebextensions/nginx.config

-
-        # HSTS (HTTP Strict Transport Security)
-        # This header tells browsers to cache the certificate for six months and to connect exclusively via HTTPS.
-        add_header Strict-Transport-Security "max-age=15768000" always;


HSTS we should pass on all requests, but it's not the end of the world if we don't do it for the static content, especially if we're planning on preloading.

hlieberman-gov · 2018-01-10T19:03:47Z

config/initializers/secure_headers.rb

+    style_src: %w['self' 'unsafe-inline']
+  }
+
+  config.hsts = 'max-age=15768000'


Should bump to max-age=63072000; includeSubDomains; preload

This reverts commit de091dc.

jgarber623-gov added the enhancement label Jan 10, 2018

jgarber623-gov self-assigned this Jan 10, 2018

jgarber623-gov requested review from aileendds, hlieberman-gov and jamesatheyDDS January 10, 2018 15:23

jgarber623-gov commented Jan 10, 2018

View reviewed changes

hlieberman-gov suggested changes Jan 10, 2018

View reviewed changes

jgarber623-gov added a commit that referenced this pull request Jan 11, 2018

Rework nginx configuration (#282 feedback)

9fa7215

jgarber623-gov force-pushed the secure-headers branch from 9fa7215 to 31250ff Compare January 11, 2018 19:17

jgarber623-gov added a commit that referenced this pull request Jan 11, 2018

Rework nginx configuration (#282 feedback)

31250ff

jgarber623-gov force-pushed the secure-headers branch from 31250ff to 6043a35 Compare January 11, 2018 19:37

jgarber623-gov added a commit that referenced this pull request Jan 11, 2018

Rework nginx configuration (#282 feedback)

6043a35

hlieberman-gov approved these changes Jan 12, 2018

View reviewed changes

jgarber623-gov force-pushed the secure-headers branch from a5e411a to f2b9a15 Compare January 12, 2018 20:14

jgarber623-gov added a commit that referenced this pull request Jan 12, 2018

Rework nginx configuration (#282 feedback)

75388db

jgarber623-gov force-pushed the secure-headers branch from f2b9a15 to d74bcf0 Compare February 1, 2018 14:28

jgarber623-gov added a commit that referenced this pull request Feb 1, 2018

Rework nginx configuration (#282 feedback)

c0d2c3d

jgarber623-gov force-pushed the secure-headers branch from d74bcf0 to 3c77beb Compare February 1, 2018 14:51

jgarber623-gov added a commit that referenced this pull request Feb 1, 2018

Rework nginx configuration (#282 feedback)

29cb5e8

jgarber623-gov force-pushed the secure-headers branch from 3c77beb to cf8b518 Compare February 1, 2018 14:57

jgarber623-gov added a commit that referenced this pull request Feb 1, 2018

Rework nginx configuration (#282 feedback)

366b74c

jgarber623-gov force-pushed the secure-headers branch from cf8b518 to 39a7f41 Compare February 1, 2018 15:10

jgarber623-gov added a commit that referenced this pull request Feb 1, 2018

Rework nginx configuration (#282 feedback)

f569f2b

jgarber623-gov force-pushed the secure-headers branch from 39a7f41 to 65c8c18 Compare February 27, 2018 15:48

jgarber623-gov added a commit that referenced this pull request Feb 27, 2018

Rework nginx configuration (#282 feedback)

65c8c18

jgarber623-gov force-pushed the secure-headers branch from 65c8c18 to f55eb67 Compare March 8, 2018 15:28

jgarber623-gov added a commit that referenced this pull request Mar 8, 2018

Rework nginx configuration (#282 feedback)

f55eb67

jgarber623-gov changed the title ~~Implement secure_headers gem~~ [WIP] Implement secure_headers gem Mar 9, 2018

jgarber623-gov added the in-progress label Mar 9, 2018

jgarber623-gov added 5 commits March 12, 2018 14:53

Revert "Disable default ActionDispatch headers in production"

295d16c

This reverts commit de091dc.

Add secure_headers gem and LICENSE info

a35db52

Implement secure_headers features

2c3600b

Change HSTS header max-age to 1 year

d13908d

Rework nginx configuration (#282 feedback)

ef8d8f9

jgarber623-gov force-pushed the secure-headers branch from f55eb67 to ef8d8f9 Compare March 12, 2018 18:53


		# Disable ActionDispatch's default headers (these are handled by nginx)
		config.action_dispatch.default_headers = {}

[WIP] Implement secure_headers gem #282

Are you sure you want to change the base?

[WIP] Implement secure_headers gem #282

Uh oh!

Conversation

jgarber623-gov commented Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Summary of Changes

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jgarber623-gov commented Jan 10, 2018 •

edited

Loading