search

kubernetes / ingress-nginx

Ingress-NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.2k stars 8.19k forks source link

hsts-preload does not work with www-redirect #11591

Open SvenKirschbaum opened 1 month ago

SvenKirschbaum commented 1 month ago

What happened:

Setting hsts-preload: true in the ingress-nginx controller configmap and creating an ingress with the nginx.ingress.kubernetes.io/from-to-www-redirect annotation for the www subdomain of a domain does no result in a hsts preload eligible configuration for the used domain.

The resulting redirects are:

http://domain.tld -> http://www.domain.tld -> https://www.domain.tld (with HSTS header)

and

https://domain.tld (without HSTS header) -> https://www.domain.tld (with HSTS header)

What you expected to happen:

I would expect this configuration to result in the domain being eligible for submission to chromes hsts preload lists. This would require the following redirects:

http://domain.tld -> https://domain.tld (with HSTS header)-> https://www.domain.tld (with HSTS header)

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

NGINX Ingress controller Release: v1.10.1 Build: 4fb5aac1dd3669daa3a14d9de3e3cdb371b4c518 Repository: https://github.com/kubernetes/ingress-nginx nginx version: nginx/1.25.3

Kubernetes version (use kubectl version): 1.29.6

Environment:

How to reproduce this issue:

$ curl -vk http://localhost 2>&1 |grep "Location\|strict"
< Location: http://www.localhost
$ curl -vk http://www.localhost 2>&1 |grep "Location\|strict"
< Location: https://www.localhost
$ curl -vk https://localhost 2>&1 |grep "Location\|strict"
< Location: https://www.localhost
$ curl -vk https://www.localhost 2>&1 |grep "Location\|strict"
< strict-transport-security: max-age=31536000; includeSubDomains; preload
k8s-ci-robot commented 1 month ago

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 1 month ago

@SvenKirschbaum what you posted is like a awareness but not really a report that can be analyzed.

The template of a new bug report asks questions and it hints at providing info like the kubectl describe output for the controller pod,svc,configmap and the app pod,svc,ingress. There is also questions for the exact curl command used as is and the logs of the controller pod. Please edit the issue description here and provide all that info to help readers.

I am not sure if the docs sets an expectation that redirection will sustain HSTS but maybe you can write a step by step guide that someone else can use, on a kind or minikube cluster, to copy/paste from your guide and reproduce the problem (maybe using self-signed certs if required or otherwise).

/remove-kind bug /kind support

Once the problem can be reproduced by others, then you can rea-apply the bug label here.

SvenKirschbaum commented 1 month ago

Thanks for your response @longwuyuan .

I didn't provide more information about my environment as i deemed it unnecessary, as the issue described is more of a conceptual nature, afaik it will arise in every ingress-nginx deployment when configuring a ingress in this way. However, I have just updated the reproduction steps in the issue so they are easier to follow.

I think it js debatable if this behavior should be categorized as a bug or rather as a feature request. I personally would have expected the configuration to result in the hsts header being applied to the pre-redirect domain too, and subsequently the domain being eligible for preloading, however it is also correct that this is not specified in the docs. I would however still vouch for changing the current behavior, as this would result in an overall more secure configuration.

I have reapplied the bug label, but please feel free to mark this as a feature request if you deem that more appropriate.

/remove-kind support /kind bug

chengjoey commented 3 weeks ago

@SvenKirschbaum , I reproduced this problem in kind using your example. I checked nginx.conf and found that redirect(from/to www) did not call the header method of ingress_lua. I think this is the reason why the strict-transport-security header is missing. After adding header_filter_by_lua_block to it, it works normally.

https://github.com/kubernetes/ingress-nginx/blob/b93ccdf7b600be4c40a874819c21f4e3f842cbea/rootfs/etc/nginx/lua/lua_ingress.lua#L170-L181

curl -vk https://localhost 2>&1 |grep "Location\|Strict"
< Location: https://www.localhost
< Strict-Transport-Security: max-age=31536000; includeSubDomains; preload
longwuyuan commented 2 weeks ago

I think this would be a nice to have.

But the idea of repeatedly changing the controller code to make all features work for every use-case has resulted in severe problems ranging from security to maintenance.

We already decided in the project that we will move towards maintaining the core ingress-api functionalities first. We also decided that we will deprecate and remove features that are not directly implementing the K8S ingress api functionalities.

In the above context, it will be nice to have a link posted here, that shows where in the K8S KEP for ingress-api,, there is a acceptable description of this from-to-www feature. That section will explain how to deal with HSTS inegration with the from-to-www feature. Can you post that link here @SvenKirschbaum @chengjoey