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

Unable to remove or update the NGINX added X-Forwarded-Proto header using charts/ingress-nginx #11040

Open
ashageetha opened this issue Feb 29, 2024 · 19 comments
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/support Categorizes issue or PR as a support question. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@ashageetha
Copy link

What happened:

Cannot remove or override the nginx ingress controller added X-Forwarded-Proto header value. Even though able to pass the customized proxy header using below values.yaml to helm install, but the URL still gets added with X-Forwarded-Proto: http header value.

controller:
  proxySetHeaders:
    X-Forwarded-Proto: ""

The nginx.conf inside the ingress controller pod shows the customized value is being added but the URLs still gets added with the first value and not the latest X-Forwarded-Proto value.

# kubectl  exec -it nginx-ingress-ingress-nginx-controller-799b9ccd64-kvxxf -n myns sh
/etc/nginx $ cat nginx.conf|grep X-Forwarded-Proto
                        proxy_set_header X-Forwarded-Proto      $pass_access_scheme;
                        proxy_set_header X-Forwarded-Proto                    "";
.
.
.
.

What you expected to happen:
Expect to have the support for removing the nginx controller added X-Forwarded-* headers from the URLs.

If we manually exec into the nginx controller pod and change the "$pass_access_scheme" to "" for X-Forwarded-Proto, works as expected.

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

/etc/nginx $ /nginx-ingress-controller --version
-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:       v1.9.6
  Build:         6a73aa3b05040a97ef8213675a16142a9c95952a
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.21.6

-------------------------------------------------------------------------------

/etc/nginx $

Kubernetes version (use kubectl version):

# kubectl version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.6", GitCommit:"741c8db18a52787d734cbe4795f0b4ad860906d6", GitTreeState:"clean", BuildDate:"2023-09-13T09:21:34Z", GoVersion:"go1.20.8", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v5.0.1
Server Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.8", GitCommit:"66fee42707cd7f5a89f1987f7cb81b02dd19161c", GitTreeState:"clean", BuildDate:"2023-11-15T16:50:09Z", GoVersion:"go1.20.11", Compiler:"gc", Platform:"linux/amd64"}

Environment:

  • Cloud provider or hardware configuration: Kubeadm based setup

  • OS (e.g. from /etc/os-release): Linux

  • Kernel (e.g. uname -a): Oracle Linux Server 7.8

  • Install tools: kubeadm

    • Please mention how/where was the cluster created like kubeadm/kops/minikube/kind etc.
  • Basic cluster related info:

    • kubectl version
    • kubectl get nodes -o wide
  • How was the ingress-nginx-controller installed:

    • If helm was used then please show output of helm ls -A | grep -i ingress
      # helm ls -A | grep -i ingress
      

nginx-ingress myns 1 2024-02-28 22:00:50.671827003 -0800 PST deployed ingress-nginx-4.9.1 1.9.6
```

  • If helm was used then please show output of helm -n <ingresscontrollernamespace> get values <helmreleasename>

    # helm get values -n myns nginx-ingress
    USER-SUPPLIED VALUES:
    controller:
      admissionWebhooks:
        enabled: false
      allowSnippetAnnotation: true
      proxySetHeaders:
        X-Forwarded-Proto: ""
    
  • If helm was not used, then copy/paste the complete precise command used to install the controller, along with the flags and options used

  • if you have more than one instance of the ingress-nginx-controller installed in the same cluster, please provide details for all the instances

  • Current State of the controller:

    • kubectl describe ingressclasses
       # kubectl describe ingressclasses
        Name:         nginx
       Labels:       app.kubernetes.io/component=controller
                 app.kubernetes.io/instance=nginx-ingress
                 app.kubernetes.io/managed-by=Helm
                 app.kubernetes.io/name=ingress-nginx
                 app.kubernetes.io/part-of=ingress-nginx
                 app.kubernetes.io/version=1.9.6
                 helm.sh/chart=ingress-nginx-4.9.1
       Annotations:  meta.helm.sh/release-name: nginx-ingress
                     meta.helm.sh/release-namespace: myns
       Controller:   k8s.io/ingress-nginx
       Events:       <none>
      
    • kubectl -n <ingresscontrollernamespace> get all -A -o wide
    • kubectl -n <ingresscontrollernamespace> describe po <ingresscontrollerpodname>
    • kubectl -n <ingresscontrollernamespace> describe svc <ingresscontrollerservicename>
  • Current state of ingress object, if applicable:

    • kubectl -n <appnamespace> get all,ing -o wide
    • kubectl -n <appnamespace> describe ing <ingressname>
    • If applicable, then, your complete and exact curl/grpcurl command (redacted if required) and the reponse to the curl/grpcurl command with the -v flag
  • Others:

    • Any other related information like ;
      • copy/paste of the snippet (if applicable)
      • kubectl describe ... of any custom configmap(s) created and in use
      • Any other related information that may help

How to reproduce this issue:

Anything else we need to know:

@ashageetha ashageetha added the kind/bug Categorizes issue or PR as related to a bug. label Feb 29, 2024
@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Feb 29, 2024
@longwuyuan
Copy link
Contributor

/remove-kind bug

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Feb 29, 2024
@ashageetha
Copy link
Author

# kubectl  -n myns get cm nginx-ingress-ingress-nginx-controller -o yaml
apiVersion: v1
data:
  allow-snippet-annotations: "false"
  proxy-set-headers: myns/nginx-ingress-ingress-nginx-custom-proxy-headers
kind: ConfigMap
metadata:
  annotations:
    meta.helm.sh/release-name: nginx-ingress
    meta.helm.sh/release-namespace: myns
  creationTimestamp: "2024-02-29T06:00:51Z"
  labels:
    app.kubernetes.io/component: controller
    app.kubernetes.io/instance: nginx-ingress
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: ingress-nginx
    app.kubernetes.io/part-of: ingress-nginx
    app.kubernetes.io/version: 1.9.6
    helm.sh/chart: ingress-nginx-4.9.1
  name: nginx-ingress-ingress-nginx-controller
  namespace: myns
  resourceVersion: "11272346"
  uid: bae7c8c0-ac7f-474f-9dff-dce93af6661a
  
  
# kubectl -n myns get cm nginx-ingress-ingress-nginx-custom-proxy-headers -o yaml
apiVersion: v1
data:
  X-Forwarded-Proto: ""
kind: ConfigMap
metadata:
  annotations:
    meta.helm.sh/release-name: nginx-ingress
    meta.helm.sh/release-namespace: myns
  creationTimestamp: "2024-02-29T06:00:51Z"
  labels:
    app.kubernetes.io/component: controller
    app.kubernetes.io/instance: nginx-ingress
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: ingress-nginx
    app.kubernetes.io/part-of: ingress-nginx
    app.kubernetes.io/version: 1.9.6
    helm.sh/chart: ingress-nginx-4.9.1
  name: nginx-ingress-ingress-nginx-custom-proxy-headers
  namespace: myns
  resourceVersion: "11272345"
  uid: 1fe1c0e2-d431-4898-9055-e88575f50f78

@longwuyuan
Copy link
Contributor

What I tested and found out is that the configMap is for adding new and customized headers but not for changing the default headers.

@longwuyuan
Copy link
Contributor

longwuyuan commented Feb 29, 2024

I have very little know on this topic but I found that this header is actually a factual info that is sent to the upstream (backend pod) . https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/#using-the-forwarded-header

So in that context, why would you want to set this manually ? Its does not seem plausible that you make a HTTPS request to the cluster but you want to tell the backend pod that its GRPC or anything other than HTTPS !

To the layman reader here, please explain what is the real problem you want to solve by setting "X-Forwarded-Proto" header manually . Thanks

@longwuyuan
Copy link
Contributor

/triage needs-information

@k8s-ci-robot k8s-ci-robot added the triage/needs-information Indicates an issue needs more information in order to work on it. label Feb 29, 2024
@ashageetha
Copy link
Author

One of our product URL is behaving different and causing too many redirects (recursively redirecting) if X-Forwarded-Proto is available in the request URL. Hence, we tried to remove this header and not able to achieve with NGINX. But we could modify and set accordingly with Traefik based ingress controller using customRequestHeaders.

@longwuyuan
Copy link
Contributor

  • I think we need a nginx expert to comment
  • If you meant you tried with vanilla non-kubernetes nginx and it wa not possible, then highly likely that not possible in ingress-nginx controller as well
  • Will keep google searching if nginx vanilla non-kubernetes was done or documented by someone

@longwuyuan
Copy link
Contributor

After reading this https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto I am convinced you can not change it, in the context of saying your browser connects with HTTPS scheme but you change it manually to HTTP. If traefik changes it, I wonder how it impacts upstream getting false info. But like I said I am not a expert so wait for someone to comment

@longwuyuan
Copy link
Contributor

/remove-triage needs-information
/help

@k8s-ci-robot
Copy link
Contributor

@longwuyuan:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/remove-triage needs-information
/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed triage/needs-information Indicates an issue needs more information in order to work on it. labels Feb 29, 2024
@longwuyuan
Copy link
Contributor

/kind support
/triage accepted

@k8s-ci-robot k8s-ci-robot added kind/support Categorizes issue or PR as a support question. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 29, 2024
@longwuyuan
Copy link
Contributor

/assign

Copy link

github-actions bot commented Apr 2, 2024

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

@github-actions github-actions bot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Apr 2, 2024
@Mahoney
Copy link

Mahoney commented Oct 17, 2024

My problem is that I am using TLS termination at the NLB using https://github.com/kubernetes/ingress-nginx/blob/main/deploy/static/provider/aws/nlb-with-tls-termination/deploy.yaml .

Because an NLB is a layer 4 device it cannot add the X-Forwarded-Proto: https header when it proxies the unencrypted request to the nginx pod over HTTP (unlike a layer 7 device like an ALB, which can alter headers).

So when the request reaches nginx it is over HTTP and without any X-Forwarded-Proto header.

Consequently nginx proxies it to the actual application service without that header, so the actual application does not know that the original request was made over HTTPS.

It seems very odd to me that a manifest called nlb-with-tls-termination would not solve this problem - perhaps I am missing something?

@longwuyuan
Copy link
Contributor

@Mahoney

@dkarlinsky
Copy link

Hi,
Looks like we have just run into the exact same issue.
From what I managed to gather the issue is that here we have proxy_set_header X-Forwarded-Proto $pass_access_scheme; directive added unconditionally to the nginix.conf for each location.
So if we try to clear the header using X-Forwarded-Proto: "" or explicitly set it to https, like what we are trying to do, what we get is two directives proxy_set_header X-Forwarded-Proto ..., which apparently results in nginx setting the header twice.
This is what I saw using tcpdump:

X-Forwarded-Proto: http
...
X-Forwarded-Proto: https

(http in the first instance is due to $pass_access_scheme being resolved to http because NLB terminates SSL but doesn't set this header.)

And my guess is that most backend servers will just use the first instance of the header.

So possible fixes for this issue could be:

  1. Add a boolean config that allows to skip setting X-Forwarded-Proto (and X-Forwarded-Scheme?) to $pass_access_scheme
  2. More general solution: for each header set unconditionally check if they key is not in $all.ProxySetHeaders

@kendrickm
Copy link

I'm running into the same issue. In most places I have been able to remove the requirement to check for https(since the NLB is ensuring that) but Wordpress is proving to be a challenge.

@longwuyuan
Copy link
Contributor

show kubectl -n ingress-nginx get cm ingress-nginx-controller -o yaml

@kendrickm
Copy link

kendrickm commented Jan 24, 2025

I just added the use-forwarded-headers: false option after reading this thread but it didn't seem to change anything

data:
  allow-snippet-annotations: "true"
  annotations-risk-level: Critical
  use-forwarded-headers: "false"
kind: ConfigMap
metadata:
  annotations:
    meta.helm.sh/release-name: ingress-nginx
    meta.helm.sh/release-namespace: ingress-nginx
  creationTimestamp: "2025-01-09T18:33:44Z"
  labels:
    app.kubernetes.io/component: controller
    app.kubernetes.io/instance: ingress-nginx
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: ingress-nginx
    app.kubernetes.io/part-of: ingress-nginx
    app.kubernetes.io/version: 1.12.0
    helm.sh/chart: ingress-nginx-4.12.0
  name: ingress-nginx-controller
  namespace: ingress-nginx
  resourceVersion: "651370645"
  uid: a3957421-30a9-42be-94dc-034007a423b3```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/support Categorizes issue or PR as a support question. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Development

No branches or pull requests

6 participants