Enable HSTS headers on all responses #895
Conversation
This syntax doesn't appear to be supported in an app.json file. It may be possible to use a github archive/*.tar.gz URL if we wish to keep the SHA references.
This consists of two configuration changes and will allow the Mozilla Observatory to see the HSTS header. Previously, if a client does not request an html content type then we return error JSON with a 404 on `/`. The first change is to add the `always` parameter which was added in nginx 1.7.5. This will include the header for all response status codes. The second change is to duplicate the add_header directive in both blocks. This surprised me, but the documentation states: "There could be several add_header directives. These directives are inherited from the previous level if and only if there are no add_header directives define on the current level."
|
I'm a bit confused about the Could someone confirm the buildpack configuration on staging and production? If so then I'll update this PR to match. I'm thinking app.json should only reference heroku-buildpack-multi so that all deploys (including new ones through the Deploy Button) will then read |
|
OMG!! Thank you for this investigation and documentation!!!! This saves me sooo much work I'm so happy!! 😻 😻 😻 😻
This is indeed the case, we are using heroku-buildpack-multi in staging and production.
I... can't remember why I did it this way, sooo... sounds good! |
This upgrades nginx and hard codes a SHA for this release of 1.13.3. This also sets up `app.json` for the Heroku Deploy Button to use heroku-buildpack-multi which will pull in other buildpacks from `.buildpacks`. [ci skip]
|
I've updated things as discussed. I also added the |
|
bors: r+ Let's give this a try! |
895: Enable HSTS headers on all responses r=carols10cents **Deployment Warning:** This pull request uses a configuration syntax added in nginx-1.7.5 and requires switching to a new buildpack. ~~If this pull request is merged the buildpack configuration on *staging* and *production* will need to be changed on Heroku.~~ (This is no longer a concern with the updated PR as this is specified in `.buildpacks`.) It is unclear if there are any existing mirror deployments on Heroku or otherwise depending on our `nginx.conf.erb` template. This pull request enables HSTS headers for all responses. My tests show an upgrade from an F to a D on the Mozilla Observatory. (Out of 100 points, this scores 35 instead of 15.) Please also see the discussion in #597 which is what prompted my review. This pull request also fixes the buildpack URLs defined in `app.json` so that the Heroku deploy button works. # Nginx buildpack upgrade The previous buildpack was based on nginx-1.5.7 which was released in November of 2013. Fortunately for us, it looks like travis-ci recently cloned this buildpack and upgraded it to the latest release of 1.13.3. Note that the odd minor version number represents that this (as well as our previous release) is from the development branch. The stable release of 1.12.1 may be preferable, but I expect that the travis-ci clone is our best bet for a responsive upstream at this time. # Nginx configuration change Two changes to the nginx configuration file where necessary. See http://nginx.org/en/docs/http/ngx_http_headers_module.html for more details on both of these. ## Use the always parameter The `always` parameter was introduced in nginx-1.7.5. The HSTS header is now included for all response codes. ## Duplicate header declaration in both blocks This surprised me, but the documentation states: > There could be several add_header directives. These directives are inherited from the previous level **if and only if** there are no add_header directives defined on the current level. I figured it was best to explicitly duplicate the declaration rather than rely on inheritance at all. # Fix the Deploy to Heroku button This pull request also fixes the Deploy to Heroku button in `docs/MIRROR.md` by updating the `app.json` file. ~~It appears that Heroku does not support URLs with the #SHA fragment. If we want to fix ourselves to specific commits for the buildpacks it may be possible to do so with `archive/*.tar.gz` links, but I suspect that we are currently targeting the master branches for these in production anyway.~~ The PR now sets up `app.json` to use heroku-buildpack-multi which will pull in the other buildpacks from `.buildpacks`. New mirror deployments should now match our staging and production environments.
Build succeeded |
Deployment Warning: This pull request uses a configuration syntax added in nginx-1.7.5 and requires switching to a new buildpack.
If this pull request is merged the buildpack configuration on staging and production will need to be changed on Heroku.(This is no longer a concern with the updated PR as this is specified in.buildpacks.) It is unclear if there are any existing mirror deployments on Heroku or otherwise depending on ournginx.conf.erbtemplate.This pull request enables HSTS headers for all responses. My tests show an upgrade from an F to a D on the Mozilla Observatory. (Out of 100 points, this scores 35 instead of 15.) Please also see the discussion in #597 which is what prompted my review.
This pull request also fixes the buildpack URLs defined in
app.jsonso that the Heroku deploy button works.Nginx buildpack upgrade
The previous buildpack was based on nginx-1.5.7 which was released in November of 2013. Fortunately for us, it looks like travis-ci recently cloned this buildpack and upgraded it to the latest release of 1.13.3.
Note that the odd minor version number represents that this (as well as our previous release) is from the development branch. The stable release of 1.12.1 may be preferable, but I expect that the travis-ci clone is our best bet for a responsive upstream at this time.
Nginx configuration change
Two changes to the nginx configuration file where necessary. See http://nginx.org/en/docs/http/ngx_http_headers_module.html for more details on both of these.
Use the always parameter
The
alwaysparameter was introduced in nginx-1.7.5. The HSTS header is now included for all response codes.Duplicate header declaration in both blocks
This surprised me, but the documentation states:
I figured it was best to explicitly duplicate the declaration rather than rely on inheritance at all.
Fix the Deploy to Heroku button
This pull request also fixes the Deploy to Heroku button in
docs/MIRROR.mdby updating theapp.jsonfile.It appears that Heroku does not support URLs with the #SHA fragment. If we want to fix ourselves to specific commits for the buildpacks it may be possible to do so withThe PR now sets uparchive/*.tar.gzlinks, but I suspect that we are currently targeting the master branches for these in production anyway.app.jsonto use heroku-buildpack-multi which will pull in the other buildpacks from.buildpacks. New mirror deployments should now match our staging and production environments.