Skip to content

HSTS header is sent multiple times #86

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

Closed
thetechnick opened this issue Nov 29, 2016 · 7 comments
Closed

HSTS header is sent multiple times #86

thetechnick opened this issue Nov 29, 2016 · 7 comments
Assignees
Milestone

Comments

@thetechnick
Copy link
Contributor

The current implementation of hsts may sent the header multiple times, if the backend application is already adding it to the http response.
This should not break clients as stated in #67, which is true for browsers.
But if the site is tested using https://www.ssllabs.com for example a error message is shown:
"Server sent invalid HSTS policy. See below for further information."
"Strict Transport Security (HSTS) Invalid Server provided more than one HSTS header "

Maybe we can find a way to add the header only if it does not exist already?

@pleshakov
Copy link
Contributor

@thetechnick

Maybe we can find a way to add the header only if it does not exist already?

There is a way to do that.

In the http context add the following map:

map $upstream_http_strict_transport_security $hsts {
  "" "value from config map or annotation";
  default $upstream_http_strict_transport_security;
}

$hsts will get either the value of the hsts header if it's present in the response from the upstream or the "value from config map or annotation" value if the header isn't present.

In the server context:

proxy_hide_header Strict-Transport-Security; # this will remove the hsts header if it's present in the response
add_header Strict-Transport-Security $hsts;

Not sure if it's necessary though as duplicated headers are allowed in the RFC.

@thetechnick
Copy link
Contributor Author

@pleshakov
Using the map will not work, because the user can choose different HSTS settings for each ingress object using the annotations. What do you think about adding proxy_hide_header as a new annotation/configmap option?
This way you can use the backends HSTS header if you set nginx.org/hsts: "False" and use only the header of the proxy if you use nginx.org/proxy-hide-header: Strict-TransportSecurity

@pleshakov
Copy link
Contributor

@thetechnick

What do you think about adding proxy_hide_header as a new annotation/configmap option?

It's valuable feature to have regardless of the hsts feature. To make it complete, it's better to add support for http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_pass_header directive.

This way you can use the backends HSTS header if you set nginx.org/hsts: "False" and use only the header of the proxy if you use nginx.org/proxy-hide-header: Strict-TransportSecurity

This way you can use the backends HSTS header if you set nginx.org/hsts: "False" and use only the header of the proxy if you use nginx.org/proxy-hide-header: Strict-TransportSecurity

What do you think about replacing of the original header by default? If the user doesn't want it, the user must turn off the hsts feature.

@thetechnick
Copy link
Contributor Author

It's valuable feature to have regardless of the hsts feature. To make it complete, it's better to add support for http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_pass_header directive.

Ok, I will add this to #88

What do you think about replacing of the original header by default? If the user doesn't want it, the user must turn off the hsts feature.

I would like this, but the solution with the map directive would prevent users from changing the hsts settings on a single ingress object.

@pleshakov
Copy link
Contributor

I would like this, but the solution with the map directive would prevent users from changing the hsts settings on a single ingress object.

without the map, just replacing altogether

proxy_hide_header Strict-Transport-Security;
add_header Strict-Transport-Security "something" always;

@thetechnick
Copy link
Contributor Author

thetechnick commented Nov 30, 2016

@pleshakov
Ok, sounds good
Can you add this?
I have enough open PRs to care about right now :)

@pleshakov
Copy link
Contributor

@thetechnick
great!
sure
thanks for your contributions!

@pleshakov pleshakov added this to the v0.7.0 milestone Nov 30, 2016
@pleshakov pleshakov self-assigned this Nov 30, 2016
@pleshakov pleshakov reopened this Dec 1, 2016
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

2 participants