Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security headers only printed when ssl is enabled #162

Open
phillbaker opened this issue Apr 16, 2017 · 4 comments
Open

Security headers only printed when ssl is enabled #162

phillbaker opened this issue Apr 16, 2017 · 4 comments

Comments

@phillbaker
Copy link

This is true on the latest released version and I don't believe it's been fixed on master - a misplaced {% endif %} fixed this on the released version, however, I wanted to confirm that the behavior I was expecting was the intended behavior.

From 1f6484134cfc86d8bc1af33603c4175da7facb7c Mon Sep 17 00:00:00 2001
Date: Tue, 11 Apr 2017 13:01:46 -0400
Subject: [PATCH] nginx: Fix wrong location of endifs for security headers.

---
 .../templates/etc/nginx/sites-available/default.conf.j2   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/templates/etc/nginx/sites-available/default.conf.j2 b/templates/etc/nginx/sites-available/default.conf.j2
index 83a8620..371867c 100644
--- a/templates/etc/nginx/sites-available/default.conf.j2
+++ b/templates/etc/nginx/sites-available/default.conf.j2
@@ -1029,6 +1029,7 @@ server {
         resolver                  {{ (item.ocsp_resolvers | d(nginx_ocsp_resolvers)) | ipwrap | join(" ") }} valid=300s;
         resolver_timeout          5s;
 {% endif %}
+{% endif %}
 {% if item.hsts_enabled is undefined or (item.hsts_enabled is defined and item.hsts_enabled) %}
         add_header                Strict-Transport-Security "max-age={{ nginx_hsts_age | default('15768000') }}{% if nginx_hsts_subdomains|bool %}; includeSubDomains{% endif %}{% if (item.hsts_preload | d(nginx_hsts_preload)) | bool %}; preload{% endif %}";
 {% endif %}
@@ -1069,7 +1070,6 @@ server {
 
 {% endif %}
 {% endif %}
-{% endif %}
 {% if nginx_tpl_ssl | bool %}
 {{ print_root() }}
 {% if nginx_tpl_acme | bool %}

This patch would move the endif to applying to the if nginx_tpl_ssl up so that all the headers from hsts_enabled to permitted_cross_domain_policies are no longer dependent on nginx using ssl. What do you think?

@drybjed
Copy link
Member

drybjed commented Apr 17, 2017

I'm not sure if sending additional headers over HTTP apart from usual redirect to HTTPS is proper or not. It seems to me that you want to push as much over HTTPS as possible. @ypid, since you know more about HSTS than me, what's your thoughts about this?

@ypid
Copy link
Member

ypid commented Apr 17, 2017

I looked over the RFC again but did not find something immediately. So I checked someone who knows more then me as well 😉

curl http://ssllabs.com https://ssllabs.com -I
HTTP/1.1 302 Found
Date: Mon, 17 Apr 2017 10:45:56 GMT
Server: Apache
X-Content-Type-Options: nosniff
X-Xss-Protection: 1; mode=block
Location: https://www.ssllabs.com
Set-Cookie: JSESSIONID=82EE237E5D37A27097BC7F90AA46D1BD; Path=/; Secure; HttpOnly
X-Frame-Options: DENY
Content-Security-Policy: default-src 'self' ssllabs.com *.ssllabs.com;

HTTP/1.1 302 Found
Date: Mon, 17 Apr 2017 10:45:57 GMT
Server: Apache
X-Content-Type-Options: nosniff
X-Xss-Protection: 1; mode=block
Strict-Transport-Security: max-age=31536000
Location: https://www.ssllabs.com/
Set-Cookie: JSESSIONID=F047D96082C8AEBC94AC0E99E28586C3; Path=/; Secure; HttpOnly
X-Frame-Options: DENY
Content-Security-Policy: default-src 'self' ssllabs.com *.ssllabs.com;

I agree with @drybjed and ssllabs. No need to send those headers over plain legacy HTTP. User agents will remember HSTS for the time specified.

@ypid
Copy link
Member

ypid commented Apr 17, 2017

Note: The Strict-Transport-Security header is ignored by the browser when your site is accessed using HTTP; this is because an attacker may intercept HTTP connections and inject the header or remove it. When your site is accessed over HTTPS with no certificate errors, the browser knows your site is HTTPS capable and will honor the Strict-Transport-Security header.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security#Description

@phillbaker
Copy link
Author

Thanks for the digging @ypid and @drybjed - it seems like based on first example of curling http where X-Content-Type-Options, X-Xss-Protection, and Content-Security-Policy this change is desired - currently none of these headers would be returned if ssl is not turned on.

For the hsts header, the use case I see is when nginx is reverse proxying behind a load balancer which handles the ssl decryption/etc., in this case although the site is serving tls traffic, it's transparent to that instance of nginx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants