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

HSTS does not work #417

Closed
istr opened this issue Sep 15, 2015 · 4 comments
Closed

HSTS does not work #417

istr opened this issue Sep 15, 2015 · 4 comments
Labels

Comments

@istr
Copy link

istr commented Sep 15, 2015

If you set up a vanilla docker-gitlab with a real certificate and test using SSL Labs you will find that HSTS does not work.
There are multiple issues; those already outlined in #138 and #206.
Apart from that:

  • the HSTS configuration should be in place for every request (including redirects)
  • currently no response by nginx will contain a Strict-Transport-Security header, ever (this is due to a defect in the simple add_header mechanism that could be replaced with the more_set_headers module)

I went through the intricacy of setting up HSTS with nginx reliably in a different context (including upstream services and nginx as SSL offloader), so maybe will try to fork and propose a PR.

@sameersbn
Copy link
Owner

you mean add_header Strict-Transport-Security should be present in the http headers when using a load balancer with SSL?

@istr
Copy link
Author

istr commented Oct 18, 2015

No, I mean that I want the setting GITLAB_HTTPS_HSTS_ENABLED to have the advertised effect.

The point is that the current setup simply is broken. So "We are correctly configuring HSTS in the nginx configs as user specifies them in the configs." still is not true, because
a) the settings do not have the wanted effect at all -- the directive in nginx exists, but the header is simply not added (which could be verified easily using curl or SSL Labs tool).
Logged into the running gitlab docker container and verified:

root@4b8f0031c341:/home/git/gitlab# grep Strict /etc/nginx/sites-enabled/gitlab 
add_header Strict-Transport-Security max-age=31536000;

Accessed the running instance using curl (real host hidden here):

curl -D- https://{...}.com/users/sign_in 2>/dev/null | head -n 20
HTTP/1.1 200 OK
Server: nginx
Date: Sun, 18 Oct 2015 13:28:13 GMT
Content-Type: text/html; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
Status: 200 OK
X-Frame-Options: DENY
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-UA-Compatible: IE=edge
ETag: "aad47721a290c9749d3f86c1d7a44383"
Cache-Control: max-age=0, private, must-revalidate
Set-Cookie: _gitlab_session=b42f9c8949802de8f56a0696254c66a1; path=/; expires=Sun, 25 Oct 2015 13:28:13 -0000; secure; HttpOnly
X-Request-Id: 68a53101-ddb6-45e1-8ff4-b82496563d81
X-Runtime: 0.024318
X-Accel-Buffering: no

<!DOCTYPE html>
<html lang='en'>

b) you miss to set the headers for all HTTPS responses (e.g. 302, 404)

I had similar issues in a different nginx setup -- for me the solution was to replace the
add_header directive with the more_set_headers 'Strict-Transport-Security: max-age=31536000'; directive.

@sameersbn
Copy link
Owner

@istr sorry for the delay. I have been spending time investigating the issue where the HSTS headers are missing from the response. It appears that the reason why the header is missing is that some of the location blocks also have add_header directives. It seems that these will override ant add_header directive in the main server block.

For example, in the following block, the add_header X-Accel-Buffering off; from the location / block overrides add_header Strict-Transport-Security "max-age=31536000';"; from the server block and as a result nginx does not output the HSTS header.

server {
  ...
  add_header Strict-Transport-Security "max-age=31536000';";
  ...

 location / {
   ...
   add_header X-Accel-Buffering off;
   ...
 }
}

This is the exact issue that is occuring. To avoid spewing the add_header Strict-Transport-Security directive all over the place, I am just going to move the add_header X-Accel-Buffering directive to the server block as well, like so:

server {
  ...
  add_header Strict-Transport-Security "max-age=31536000';";
  add_header X-Accel-Buffering off;
  ...

 location / {
   ...
 }
}

What do you think?

sameersbn pushed a commit that referenced this issue Dec 13, 2015
@istr
Copy link
Author

istr commented Dec 16, 2015

Great! That should solve the issue. Thanks.

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

No branches or pull requests

2 participants