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

X-Forwarded-Host header is copied into the Host header #2463

Closed
octete opened this issue May 2, 2018 · 19 comments

Comments

@octete
Copy link

commented May 2, 2018

Is this a request for help?

Yes, and maybe a bug report. I wanted to explain the situation that I'm seeing, before filling a bug report, or asking for some configuration changes.

What keywords did you search in NGINX Ingress controller issues before filing this one? (If you have found any duplicates, you should instead reply there.):

I have found related entries:

#910
#911
#2030

These are either closed, or referring to the whole set of X-Forwarded-* headers, thus opening this one.


Is this a BUG REPORT or FEATURE REQUEST? (choose one):

Kind of a bug report.

NGINX Ingress controller version:
0.11.0

Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"8", GitVersion:"v1.8.6", GitCommit:"6260bb08c46c31eea6cb538b34a9ceb3e406689c", GitTreeState:"clean", BuildDate:"2017-12-21T06:34:11Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"7+", GitVersion:"v1.7.15-gke.0", GitCommit:"6601550c19b8dcd2643e82cbf6a25d2c4195775b", GitTreeState:"clean", BuildDate:"2018-03-20T20:21:03Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}

Environment:

  • GKE v1.7.15

What happened:

The nginx ingress controller is rewriting the X-Forwarded-Host header into the Host header. As per https://tools.ietf.org/html/rfc7239, that header seems to be an optional, informational header.

It can, however, have more than one host appended, like:

X-Forwarded-Host: host.com, host.com

In our setup, we are using Fastly as our frontend, which uses our main load balancers to route traffic between two different kubernetes clusters in GKE running an ingress controller. Fastly does set the X-Forwarded-Host header with multiple hosts appended to it, separate by commas, as per the example above. This happens, seemingly, when Fastly reroutes traffic between their datacentres. Our front end load balancers pass that header intact to the ingress controller. The ingress controller, however, copies it into the Host header, and this header then gets passed to the service running in k8s, which is Jetty based, as:

Host: host.com, host.com

At which point, Jetty returns with a 400 HTTP error, as it's an illegal Host header.

What you expected to happen:

Not rewrite X-Forwarded-Host when it has more than one value to the Host header.

How to reproduce it (as minimally and precisely as possible):

You can easily reproduce it with

curl -H 'X-Forwarded-Host: host.com, host.com' http://kubernetes-ingress-controller/

to the echoserver. You can also tcpdump on the receiving end. For a full test, you can hit Jetty too.

Anything else we need to know:

This is just an issue I wanted to raise, so my request is for help/solutions to this issues. It seems to me like a bug as you can have multiple hosts appended to X-Forwarded-Host, which is illegal for the Host header, but I might be missing something. Let me know what your thoughts are on this, please.

Thank you very much.

@aledbf

This comment has been minimized.

Copy link
Member

commented May 3, 2018

@octete please update to 0.14.0

@marklee77

This comment has been minimized.

Copy link

commented May 3, 2018

we still have this in the nginx.tmpl in 0.14.0:

    map $http_x_forwarded_host $best_http_host {
        default          $http_x_forwarded_host;
        ''               $this_host;
    }

@aledbf aledbf referenced this issue May 3, 2018
2 of 6 tasks complete
@atkaper

This comment has been minimized.

Copy link

commented May 31, 2018

We are also suffering from this "feature" (bug), we have an apache httpd server in front of the ingress nginx (due to legacy setup of environment), and it's mod_balancer does exactly this, it adds a comma separated list of host names to the x_forwarded_host header, breaking the upstream http host header.

@marklee77

This comment has been minimized.

Copy link

commented Jun 4, 2018

Just to reiterate here: https://httpd.apache.org/docs/2.2/mod/mod_proxy.html#x-headers

Be careful when using these headers on the origin server, since they will contain more than one (comma-separated) value if the original request already contained one of these headers. For example, you can use %{X-Forwarded-For}i in the log format string of the origin server to log the original clients IP address, but you may get more than one address if the request passes through several proxies.

@michelvocks

This comment has been minimized.

Copy link

commented Jun 12, 2018

We also faced the same issue. One service (using Zuul) proxy request to another service. It seems like Zuul (or our apache in front of it) sets a comma separated list to the x_forwarded_host header which returns a 400 (Bad Request) response.

Edit: For people facing the same issue but still want to use versions after 0.10.2:
You can work around the issue by unsetting the header "X-Forwarded-Host" in your proxy.
For apache it's "RequestHeader unset x-Forwarded-Host".

@marklee77

This comment has been minimized.

Copy link

commented Jun 12, 2018

We just force x-forwarded-host to be the appropriate value in our edge proxies, but it would be nice if this behavior could be fixed to be complaint with the header documentation. It's not even a real standard, so at the least the ingress needs to parse x-forwarded-host and only set host if the contents are valid.

@mentalblock

This comment has been minimized.

Copy link

commented Jun 21, 2018

I am facing a problem with this as well. I think the Host header should not be touched and leave the upstream to decide what to do with the x-forwarded-host header.

@fejta-bot

This comment has been minimized.

Copy link

commented Sep 19, 2018

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@benhamill

This comment has been minimized.

Copy link

commented Sep 19, 2018

Can anyone explain why the ingress should touch the Host header at all? Is there some benefit I'm not seeing? It seems like it is trying to make any proxies outside the ingress invisible to things inside the ingress, which seems really suspect to me.

The only modification strategy I could think of that would make sense to me would be if it prepended the Host header into X-Forwarded-Host (with a comma if it's not empty) and then set Host to the service name of whatever it ends up routing to. But I'm not sure of the utility of that over just leaving the headers untouched.

@carl-youngblood

This comment has been minimized.

Copy link

commented Sep 19, 2018

Per @benhamill's comment, this behavior makes the nginx ingress not transparent.

If I have an existing setup of:
my_proxy -> backend

And my backend is configured to use the Host header when making decisions, I am unable to transparently migrate my backend to Kubernetes.

After the migration with the following setup:
my_proxy -> ingress -> backend

The ingress has mutated the state of the request by reverting the Host header to the value of X-Forwarded-Host. The data that backend was using has been lost.

@carl-youngblood

This comment has been minimized.

Copy link

commented Sep 19, 2018

Another question, why does it make sense to set Host and X-Forwarded-Host to the same value?

proxy_set_header Host                   $best_http_host;
proxy_set_header X-Forwarded-Host       $best_http_host;
@carl-youngblood

This comment has been minimized.

Copy link

commented Sep 20, 2018

If you're experiencing this issue you may be able to work around it with an annotation on your ingress like

nginx.ingress.kubernetes.io/configuration-snippet: |
      more_clear_input_headers "Host" "X-Forwarded-Host";
      proxy_set_header Host $http_host;
      proxy_set_header X-Forwarded-Host $http_x_forwarded_host;
@aledbf

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

Can anyone explain why the ingress should touch the Host header at all?
Another question, why does it make sense to set Host and X-Forwarded-Host to the same value?

The reason for this is that right now we have only one template for all the scenarios where the ingress controller could be used. This means we need to consider all this logic https://github.com/kubernetes/ingress-nginx/blob/master/rootfs/etc/nginx/template/nginx.tmpl#L269 before the generation of the final nginx.conf file.

I want to split all this into several "profiles", having sane defaults (like when you enable proxy-protocol) and also allowing the user to define custom behaviors. That said, this is just an idea in my TODO list

@fejta-bot

This comment has been minimized.

Copy link

commented Oct 20, 2018

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@aiman-alsari

This comment has been minimized.

Copy link

commented Nov 8, 2018

Workaround as described by @carl-youngblood didn't seem to work for me, but adding the following to the nginx configmap did.

http-snippet: |
    map $host $best_http_host {
        default         $this_host;
    }
@fejta-bot

This comment has been minimized.

Copy link

commented Dec 8, 2018

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2018

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.

@paovitali

This comment has been minimized.

Copy link

commented Feb 20, 2019

Hi, I'm still facing the same issue with 0.22 version, once I set through CFM this value use-forwarded-headers: "false" in controller, I have the Host header populated with values coming from X-Forwarded-Host . This due to the $best_http_host settings in template. Host header should not be treated as an array of values when it should contain one and one only value (unlike the X-Forwarded-Host one)

/reopen

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

@paovitali: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

Hi, I'm still facing the same issue with 0.22 version, once I set through CFM this value use-forwarded-headers: "false" in controller, I have the Host header populated with values coming from X-Forwarded-Host . This due to the $best_http_host settings in template. Host header should not be treated as an array of values when it should contain one and one only value (unlike the X-Forwarded-Host one)

/reopen

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.