ingress-nginx icon indicating copy to clipboard operation
ingress-nginx copied to clipboard

Problem with HTTPS, CloudFlare and X-Forwarded-Port header

Open tpoindessous opened this issue 4 years ago • 40 comments

NGINX Ingress controller version: 0.40.2

Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.0", GitCommit:"9e991415386e4cf155a24b1da15becaa390438d8", GitTreeState:"clean", BuildDate:"2020-03-26T06:16:15Z", GoVersion:"go1.14", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"16+", GitVersion:"v1.16.13-gke.401", GitCommit:"eb94c181eea5290e9da1238db02cfef263542f5f", GitTreeState:"clean", BuildDate:"2020-09-09T00:57:35Z", GoVersion:"go1.13.9b4", Compiler:"gc", Platform:"linux/amd64"}

Environment: GKE

  • Cloud provider or hardware configuration: GKE
  • OS (e.g. from /etc/os-release): GKE
  • Kernel (e.g. uname -a): GKE
  • Install tools: Kustomize
  • Others: CloudFlare Proxy mode

What happened:

Hi, I have this setup

CloudFlare with HTTPS and Proxy mode => GKE nginx-ingress-controller in HTTP => myApp.

When I got to my website in HTTPS mode, myApp receives this request :

GET / HTTP/1.1
Host: myApp.mydomain.com
X-Request-ID: XXXXXXX
X-Real-IP: myIP
X-Forwarded-For: myIP
X-Forwarded-Host: myApp.mydomain.com
X-Forwarded-Port: 80
X-Forwarded-Proto: https
X-Scheme: https
X-Original-Forwarded-For: myIP

So, because of X-Forwarded-Port: 80, myApp thinks that original request was in HTTPS BUT on port 80 :( and then, links generated on the response are not OK.

What you expected to happen:

I would like to receive a X-Forwarded-Port: 443 header.

How to reproduce it: My conf :

  body-size: "20m"
  # Cloudflare IP ranges which you can find online
  proxy-real-ip-cidr: "173.245.48.0/20,103.21.244.0/22,103.22.200.0/22,103.31.4.0/22,141.101.64.0/18,108.162.192.0/18,190.93.240.0/20,188.114.96.0/20,197.234.240.0/22,198.41.128.0/17,162.158.0.0/15,104.16.0.0/12,172.64.0.0/13,131.0.72.0/22,2400:cb00::/32,2606:4700::/32,2803:f800::/32,2405:b500::/32,2405:8100::/32,2a06:98c0::/29,2c0f:f248::/32"
  use-forwarded-headers: "true"
  forwarded-for-header: "CF-Connecting-IP"

Anything else we need to know:

I can't verify the original request (and so, headers added by CloudFlare) which arrives in nginx-ingress-controller, as I don't have tcpdump installed on this container. If you know how to dump the original request , I could dump it.

/kind bug

tpoindessous avatar Oct 21 '20 10:10 tpoindessous

Hi !

I found a way to do a tcpdump in nginx-ingress-controllers.

CloudFlare doesn't send a X-Forwarded-Port Header.

I think this is related to this : https://github.com/kubernetes/ingress-nginx/blob/fb6a03ffb42cfbf682c3c7f400f46fb4802caa1c/rootfs/etc/nginx/template/nginx.tmpl#L1123

This variable is set to the server port.

More critical is that I think this variable is not overwritable in general server-snippet nor in annotation in the Ingress.

for example, nginx.conf when I added this variable to annotation :

        server {
                server_name myApp.mydomain.com ;
                listen 80;
                listen [::]:80;
                set $proxy_upstream_name "-";
                proxy_set_header X-Forwarded-Port "443";
                location / {
                        proxy_set_header X-Forwarded-Port       $pass_port;

tpoindessous avatar Oct 23 '20 11:10 tpoindessous

I tried adding these on Ingress object but didn't change the header at all either. (Based on the "fix" in https://github.com/kubernetes/ingress-nginx/issues/3481#issuecomment-569683634)

annotations:
  nginx.ingress.kubernetes.io/configuration-snippet: |
    more_set_input_headers "X-Forwarded-Port: 443"

and

annotations:
  nginx.ingress.kubernetes.io/configuration-snippet: |
    more_clear_input_headers "X-Forwarded-Port"

I verified they showed up in nginx.conf but had no effect.

kolorful avatar Oct 27 '20 17:10 kolorful

Seems like this has somewhat been discussed in terms of the we can't override this header. https://github.com/kubernetes/ingress-nginx/issues/3481#issuecomment-443311017

@aledbf do you think it would make sense to add an config to let user override "X-Forwarded-Port" header, or do you know why more_set_input_headers would not work as claimed in https://github.com/kubernetes/ingress-nginx/issues/3481#issuecomment-569683634 thanks.

kolorful avatar Oct 28 '20 22:10 kolorful

Did you configure CloudFlare with the Full(strict) mode ? In this mode CloudFlare should talk to Ingress Nginx through port 443 which I think would cause Ingress Nginx to set the X-Forwarded-Port header properly.

Dohbedoh avatar Oct 30 '20 02:10 Dohbedoh

Did you configure CloudFlare with the Full(strict) mode ? In this mode CloudFlare should talk to Ingress Nginx through port 443 which I think would cause Ingress Nginx to set the X-Forwarded-Port header properly.

This.

Please verify that CloudFlare it set to Full or Full (strict) mode. Found under SSL-TLS => Overview: image

toredash avatar Nov 03 '20 14:11 toredash

Hi !

@Dohbedoh @toredash , no I didn't set Full mode, so my nginx-ingress-controller is listening in HTTP mode.

This is why nginx set X-Forwarded-Port to port 80 and this is why I want to override this (to delete this header).

Thanks !

tpoindessous avatar Nov 03 '20 16:11 tpoindessous

I think there is an issue with the way the Lua code sets pass_server_port (which pass_port is later derived from): https://github.com/kubernetes/ingress-nginx/blob/6c729e9cc76ca33ecd1b33c36b931c0aa27aa34f/rootfs/etc/nginx/lua/lua_ingress.lua#L147-L149

Ideally if ngx.var.http_x_forwarded_port is not set but ngx.var.http_x_forwarded_proto is then it would fall back to choosing a port based on ngx.var.http_x_forwarded_proto like this:

    if ngx.var.http_x_forwarded_port then
      ngx.var.pass_server_port = ngx.var.http_x_forwarded_port
    elseif ngx.var.http_x_forwarded_proto == "http" then
      ngx.var.pass_server_port = 80
    elseif ngx.var.http_x_forwarded_proto == "https" then
      ngx.var.pass_server_port = 443
    end

ailurarctos avatar Nov 04 '20 06:11 ailurarctos

@ailurarctos this would solves the problem but it is not clear to me if this is an acceptable / standard behavior ? It seems to me that CloudFlare SSL should set X-Forwarded-Port header accordingly and for some reason they don't. But I might be wrong ? Maybe we should add more flexibility to users to allow overriding / setting those headers to solve such particular scenarios. WDYT ?

Dohbedoh avatar Nov 04 '20 07:11 Dohbedoh

CloudFlare does not set X-Forwarded-Port according to their docs: https://support.cloudflare.com/hc/en-us/articles/200170986-How-does-Cloudflare-handle-HTTP-Request-headers-

@tpoindessous I think you need to set a location based snippet, as such:

nginx.ingress.kubernetes.io/configuration-snippet: |
  proxy_set_header X-Forwarded-Port 443;

I assume you are not using a HTTPS Load balancer in GKE ?

toredash avatar Nov 04 '20 07:11 toredash

@toredash if you use a snippet to set the X-Forwarded-Port header it ends up appending instead of replacing.

Ideally if the upstream loadbalancer is not sending an X-Forwarded-Port header and ingress-nginx is configured to use-forwarded-headers then either it would 1) not send an X-Forwarded-Port at all or 2) derive the X-Forwarded-Port from the X-Forwarded-Proto. Right now it is sending an X-Forwarded-Port that is the port NGINX itself is listening on, which means the resulting X-Forwarded-* headers are a mixture of NGINX and the upstream loadbalancer.

ailurarctos avatar Nov 04 '20 08:11 ailurarctos

Maybe we should add more flexibility to users to allow overriding / setting those headers to solve such particular scenarios. WDYT ?

@Dohbedoh if there is a way to override or set those headers that would work for me.

ailurarctos avatar Nov 04 '20 08:11 ailurarctos

Thanks, I will try.

No, I'm not using a HTTPS Load Balancer in GKE, I'm using a nginx-ingress-controller (associated automatically with a External TCP Load Balancer).

For the record : GCP HTTP Load Balancer in GKE, in NEG configuration, has the same issue : it's listening in HTTP/ port 80, and it sets a X-Forwarded-Port: 80, even if it is placed behind a HTTPS Cloudflare configuration.

thanks !

Le mer. 4 nov. 2020 à 08:53, Tore [email protected] a écrit :

CloudFlare does not set X-Forwarded-Port according to their docs: https://support.cloudflare.com/hc/en-us/articles/200170986-How-does-Cloudflare-handle-HTTP-Request-headers-

@tpoindessous https://github.com/tpoindessous I think you need to set a location based snippet, as such:

nginx.ingress.kubernetes.io/configuration-snippet: | proxy_set_header X-Forwarded-Port 443;

I assume you are not using a HTTPS Load balancer in GKE ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes/ingress-nginx/issues/6358#issuecomment-721568991, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXJYEGJPZAPS7XVZILOHU3SOEBLXANCNFSM4SZTQMVA .

tpoindessous avatar Nov 04 '20 09:11 tpoindessous

@tpoindessous I think you need to set a location based snippet, as such:

nginx.ingress.kubernetes.io/configuration-snippet: |
  proxy_set_header X-Forwarded-Port 443;

This does not work as expected because the value is appended, resulting in header:

X-Forwarded-Port: 80,443

Edit: We are attempting to use

nginx.ingress.kubernetes.io/configuration-snippet: |
  more_set_input_headers 'X-Forwarded-Port 443';

but it does not overwrite the Header.

hobti01 avatar Nov 05 '20 11:11 hobti01

but it does not overwrite the Header.

This doesn't work because the template contains proxy_set_header X-Forwarded-Port and the directive more_set_input_headers is executed in the same nginx phase.

This does not work as expected because the value is appended, resulting in header:

this works as expected for the same reason. Multiple proxy_set_header for the same header appends the values

aledbf avatar Nov 05 '20 11:11 aledbf

Changing the value of the header X-Forwarded-Port without using a custom template can be done with a plugin. https://github.com/kubernetes/ingress-nginx/tree/master/rootfs/etc/nginx/lua/plugins

The content of such a thing is trivial:

local ngx = ngx

local _M = {}

function _M.rewrite()
  if ngx.var.http_cf_connecting_ip then
    ngx.log(ngx.ERR, "Changing x-forwarded-port to 443")
    ngx.var.pass_port = 443
  end
end

return _M
curl localhost -H 'CF-Connecting-IP: 1.1.1'


Hostname: http-svc-6b7fcd49cc-jb27g

Pod Information:
	node name:	kind-control-plane
	pod name:	http-svc-6b7fcd49cc-jb27g
	pod namespace:	default
	pod IP:	10.244.0.11

Server values:
	server_version=nginx: 1.12.2 - lua: 10010

Request Information:
	client_address=10.244.0.7
	method=GET
	real path=/
	query=
	request_version=1.1
	request_scheme=http
	request_uri=http://localhost:8080/

Request Headers:
	accept=*/*
	cf-connecting-ip=1.1.1
	host=localhost
	user-agent=curl/7.68.0
	x-forwarded-for=172.18.0.1
	x-forwarded-host=localhost
	x-forwarded-port=443
	x-forwarded-proto=http
	x-real-ip=172.18.0.1
	x-request-id=dc00f8bf88bc383b786a227b7d2657e4
	x-scheme=http

Request Body:
	-no body in request-

The condition to change the variable can check for any other header (like limiting the change to a particular host) Using a configmap is possible to mount the plugin as a file https://github.com/kubernetes/ingress-nginx/blob/master/charts/ingress-nginx/values.yaml#L396-L404

aledbf avatar Nov 05 '20 12:11 aledbf

Thanks @aledbf !

tpoindessous avatar Nov 05 '20 13:11 tpoindessous

This problem is driving me nuts. Any solution yet?

Uysim avatar Jan 14 '21 10:01 Uysim

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-contributor-experience at kubernetes/community. /lifecycle stale

fejta-bot avatar Apr 14 '21 11:04 fejta-bot

/remove-lifecycle stale

tpoindessous avatar Apr 14 '21 13:04 tpoindessous

Hi @tpoindessous @kolorful @Uysim, can you guys can confirm that the issue still exists? Also with newer versions of ingress-nginx?

iamNoah1 avatar Jun 28 '21 08:06 iamNoah1

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Sep 26 '21 09:09 k8s-triage-robot

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot avatar Oct 26 '21 10:10 k8s-triage-robot

Hi @tpoindessous @kolorful @Uysim, can you guys can confirm that the issue still exists? Also with newer versions of ingress-nginx?

I can confirm this still happens on nginx 1.0.1

clglavan avatar Oct 26 '21 15:10 clglavan

@clglavan I don't have info in my hand. My application has been moved.

Uysim avatar Oct 27 '21 12:10 Uysim

/lifecycle active

iamNoah1 avatar Nov 02 '21 10:11 iamNoah1

/triage accepted /priority important-longterm /help

iamNoah1 avatar Nov 02 '21 10:11 iamNoah1

@iamNoah1: 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:

/triage accepted /priority important-longterm /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 avatar Nov 02 '21 10:11 k8s-ci-robot

I stumbled into this issue while troubleshooting the closely related problem. We use custom SSL port 5443 for ingress-controller instead of standard 443, but ingress-nginx always sends X-Forwarded-Port: 443 to our backend. Sad thing is that even sending this header from the client to nginx doesn't help, it always returns a constant 443 value, regardless of $server_port. I believe the problem is in the following part of Lua code: https://github.com/kubernetes/ingress-nginx/blob/6c729e9cc76ca33ecd1b33c36b931c0aa27aa34f/rootfs/etc/nginx/lua/lua_ingress.lua#L168-L169

I tried to comprehend this code but I really don't understand why the value is set to 443 and not to config.listen_ports.https (or left as is, because it is already set to the proper value at line 163).

When I tried to play with nginx.conf template on a live server, I figured out these things:

Variable Value OK?
$server_port 5443 OK :heavy_check_mark:
$pass_server_port 5443 OK :heavy_check_mark:
$pass_port 443 KO :heavy_multiplication_x:

As others already mentioned there's no way how to override this value by configuration.

Any help would be appreciated.

mouchar avatar Dec 06 '21 21:12 mouchar

Hi @tpoindessous @kolorful @Uysim, can you guys can confirm that the issue still exists? Also with newer versions of ingress-nginx?

Still exists as of most recent chart/controller (chart 4.0.13). It is virtually impossible to set X-Forwarded-Proto for upstream apps that need to know the original client made an HTTPS request.

brsolomon-deloitte avatar Jan 07 '22 21:01 brsolomon-deloitte

@aledbf

Changing the value of the header X-Forwarded-Port without using a custom template can be done with a plugin. https://github.com/kubernetes/ingress-nginx/tree/master/rootfs/etc/nginx/lua/plugins

The content of such a thing is trivial:

Slapping the word 'trivial' on something doesn't make it trivial. Asking people to write a Lua plugin is not trivial nor should it be necessary to set a single proxy header for an upstream app.

brsolomon-deloitte avatar Jan 07 '22 21:01 brsolomon-deloitte