Set X-Frame-Options to DENY when proxying to S3 via Nginx

[floehopper]

When requesting an asset from assets-origin.dev.gov.uk with the proxy_to_s3_via_nginx option set, we don't see the X-Frame-Options: DENY response header that we do see when the option is not set. See the output from curl in this comment.

I think header is being added by this add_header statement.

Depending on whether this header is actually important, we might either need to duplicate this add_header statement in the "proxying to S3" location block or somehow copy the header set by in the outer block.

---

Todo

• [ ] Test the change on integration

[chrisroos]

From Mozilla's page on X-Frame-Options:

The X-Frame-Options HTTP response header can be used to indicate whether or not a browser should be allowed to render a page in a <frame>, <iframe> or <object>. Sites can use this to avoid clickjacking attacks, by ensuring that their content is not embedded into other sites.

I've tested the effect of this header by creating the following html page containing two iframes: one that embeds an asset served from the filesystem and the other that embeds an asset served from S3.

<h2>Iframe with asset from GOV.UK NFS</h2>
<iframe src="https://assets-origin.integration.publishing.service.gov.uk/media/59a82951e5274a05f52732a2/2017-07-22-richmond-half-marathon-in-google-maps.png"></iframe>

<h2>Iframe with asset from GOV.UK S3</h2>
<iframe src="https://assets-origin.integration.publishing.service.gov.uk/media/59a82951e5274a05f52732a2/2017-07-22-richmond-half-marathon-in-google-maps.png?proxy_to_s3_via_nginx=true"></iframe>

The screenshot below shows that the asset served from the filesystem isn't displayed, while the one from S3 is:

This gives me confidence that we should be setting the header when proxying asset requests to S3.

[chrisroos]

I've opened https://github.com/alphagov/govuk-puppet/pull/6385 to address this issue.

[chrisroos]

I've just updated https://github.com/alphagov/govuk-puppet/pull/6385 and asked James to take another look.

[chrisroos]

I've merged https://github.com/alphagov/govuk-puppet/pull/6385 and plan to test the effect of this change on integration once the Puppet changes have been deployed.

[chrisroos]

I've tested this in integration by uploading an asset and requesting it from NFS and from S3:

$ curl https://assets-origin.integration.publishing.service.gov.uk/media/59ae7ce4e5274a5d5d9c81bf/2017-07-22-richmond-half-marathon-in-google-maps.png -I -v -s > /dev/null 
< HTTP/1.1 200 OK
< X-Frame-Options: DENY

$ curl https://assets-origin.integration.publishing.service.gov.uk/media/59ae7ce4e5274a5d5d9c81bf/2017-07-22-richmond-half-marathon-in-google-maps.png?proxy_to_s3_via_nginx=true  -v -s > /dev/null 
< HTTP/1.1 200 OK
< X-Frame-Options: DENY

I'm happy that this is working as expected so I'm closing the issue.

[floehopper]

It looks as if we broke this for all assets served from NFS (vs S3) in alphagov/govuk-puppet#6410, specifically by adding add_header GOVUK-Asset-Manager-File-Store NFS;. See the following output:

With call to add_header

$ curl -v -s http://assets-origin.dev.gov.uk/media/59ca79e5759b744666dc7219/blue-960x640.gif >/dev/null
*   Trying 10.1.1.254...
* TCP_NODELAY set
* Connected to assets-origin.dev.gov.uk (10.1.1.254) port 80 (#0)
> GET /media/59ca79e5759b744666dc7219/blue-960x640.gif HTTP/1.1
> Host: assets-origin.dev.gov.uk
> User-Agent: curl/7.54.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Server: nginx
< Date: Fri, 29 Sep 2017 14:28:26 GMT
< Content-Type: image/gif
< Content-Length: 1323
< Connection: close
< Last-Modified: Tue, 26 Sep 2017 16:01:41 GMT
< Content-Disposition: inline; filename="blue-960x640.gif"
< Cache-Control: private
< ETag: "59ca79e5-52b"
< GOVUK-Asset-Manager-File-Store: NFS
< Accept-Ranges: bytes
< Strict-Transport-Security: max-age=31536000
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Methods: GET, OPTIONS
< Access-Control-Allow-Headers: origin, authorization
< 
{ [1323 bytes data]
* Closing connection 0

Note: There is no X-Frame-Options in the response.

Without call to add_header

$ curl -v -s http://assets-origin.dev.gov.uk/media/59ca79e5759b744666dc7219/blue-960x640.gif >/dev/null
*   Trying 10.1.1.254...
* TCP_NODELAY set
* Connected to assets-origin.dev.gov.uk (10.1.1.254) port 80 (#0)
> GET /media/59ca79e5759b744666dc7219/blue-960x640.gif HTTP/1.1
> Host: assets-origin.dev.gov.uk
> User-Agent: curl/7.54.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Server: nginx
< Date: Fri, 29 Sep 2017 14:27:35 GMT
< Content-Type: image/gif
< Content-Length: 1323
< Connection: close
< Last-Modified: Tue, 26 Sep 2017 16:01:41 GMT
< Content-Disposition: inline; filename="blue-960x640.gif"
< Cache-Control: private
< ETag: "59ca79e5-52b"
< X-Frame-Options: DENY
< Accept-Ranges: bytes
< Strict-Transport-Security: max-age=31536000
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Methods: GET, OPTIONS
< Access-Control-Allow-Headers: origin, authorization
{ [1323 bytes data]
* Closing connection 0

Note: There is no X-Frame-Options in the response.

The Nginx documentation says:

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 think the clause in italics is what's causing the problem.

Fortunately, we're not serving any assets from NFS in production, so this is not currently a problem. However, we probably need to sort it out if we want to serve Whitehall assets from NFS. #235 is closely related to this.