NGINX: Correctly determine client IP when using PROXY protocol and multiple proxies are involved

Author: dmeremyanin

docs/user-guide/miscellaneous.md

@@ -10,7 +10,19 @@ If the ingress controller is running in AWS we need to use the VPC IPv4 CIDR.
 
 Another option is to enable the **PROXY protocol** using [`use-proxy-protocol: "true"`](https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#use-proxy-protocol).
 
-In this mode NGINX does not use the content of the header to get the source IP address of the connection.
+In this mode, NGINX uses the PROXY protocol TCP header to retrieve the source IP address of the connection.
+
+This works in most cases, but if you have a Layer 7 proxy (e.g., Cloudflare) in front of a TCP load balancer, it may not work correctly. The HTTP proxy IP address might appear as the client IP address. In this case, you should also enable the `use-forwarded-headers` setting in addition to enabling `use-proxy-protocol`, and properly configure `proxy-real-ip-cidr` to trust all intermediate proxies (both within the private network and any external proxies).
+
+Example configmap for setups with multiple proxies:
+
+```yaml
+use-proxy-protocol: "true"
+use-forwarded-headers: "true"
+proxy-real-ip-cidr: "10.0.0.0/8,131.0.72.0/22,172.64.0.0/13,104.24.0.0/14,104.16.0.0/13,162.158.0.0/15,198.41.128.0/17"
+```
+
+**Note:** Be sure to use real CIDRs that match your exact environment.
 
 ## Path types
 

docs/user-guide/nginx-configuration/configmap.md

@@ -131,6 +131,7 @@ The following table shows a configuration option's name, type, and the default v
 | [use-forwarded-headers](#use-forwarded-headers)                                 | bool         | "false"                                                                                                                                                                                                                                                                                                                                                      |                                                                                     |
 | [enable-real-ip](#enable-real-ip)                                               | bool         | "false"                                                                                                                                                                                                                                                                                                                                                      |                                                                                     |
 | [forwarded-for-header](#forwarded-for-header)                                   | string       | "X-Forwarded-For"                                                                                                                                                                                                                                                                                                                                            |                                                                                     |
+| [forwarded-for-proxy-protocol-header](#forwarded-for-proxy-protocol-header)     | string       | "X-Forwarded-For-Proxy-Protocol"                                                                                                                                                                                                                                                                                                                             |                                                                                     |
 | [compute-full-forwarded-for](#compute-full-forwarded-for)                       | bool         | "false"                                                                                                                                                                                                                                                                                                                                                      |                                                                                     |
 | [proxy-add-original-uri-header](#proxy-add-original-uri-header)                 | bool         | "false"                                                                                                                                                                                                                                                                                                                                                      |                                                                                     |
 | [generate-request-id](#generate-request-id)                                     | bool         | "true"                                                                                                                                                                                                                                                                                                                                                       |                                                                                     |
@@ -913,6 +914,10 @@ If false, NGINX ignores incoming `X-Forwarded-*` headers, filling them with the
 
 Sets the header field for identifying the originating IP address of a client. _**default:**_ X-Forwarded-For
 
+## forwarded-for-proxy-protocol-header
+
+Sets the name of the intermediate header used to determine the client's originating IP when both `use-proxy-protocol` and `use-forwarded-headers` are enabled. This doesn't impact functionality and should not typically be modified. _**default:**_ X-Forwarded-For-Proxy-Protocol
+
 ## compute-full-forwarded-for
 
 Append the remote address to the X-Forwarded-For header instead of replacing it. When this option is enabled, the upstream application is responsible for extracting the client IP based on its own list of trusted proxies.

internal/ingress/controller/config/config.go

@@ -569,6 +569,12 @@ type Configuration struct {
 	// Default is X-Forwarded-For
 	ForwardedForHeader string `json:"forwarded-for-header,omitempty"`
 
+	// Sets the name of the intermediate header used to determine the client's originating IP
+	// when both use-proxy-protocol and use-forwarded-headers are enabled. This doesn't impact
+	// functionality and should not typically be modified.
+	// Default is X-Forwarded-For-Proxy-Protocol
+	ForwardedForProxyProtocolHeader string `json:"forwarded-for-proxy-protocol-header,omitempty"`
+
 	// Append the remote address to the X-Forwarded-For header instead of replacing it
 	// Default: false
 	ComputeFullForwardedFor bool `json:"compute-full-forwarded-for,omitempty"`
@@ -780,6 +786,7 @@ func NewDefault() Configuration {
 		UseForwardedHeaders:              false,
 		EnableRealIP:                     false,
 		ForwardedForHeader:               "X-Forwarded-For",
+		ForwardedForProxyProtocolHeader:  "X-Forwarded-For-Proxy-Protocol",
 		ComputeFullForwardedFor:          false,
 		ProxyAddOriginalURIHeader:        false,
 		GenerateRequestID:                true,

rootfs/etc/nginx/template/nginx.tmpl

@@ -77,7 +77,9 @@ http {
     {{/* Enable the real_ip module only if we use either X-Forwarded headers or Proxy Protocol. */}}
     {{/* we use the value of the real IP for the geo_ip module */}}
     {{ if or (or $cfg.UseForwardedHeaders $cfg.UseProxyProtocol) $cfg.EnableRealIP }}
-    {{ if $cfg.UseProxyProtocol }}
+    {{ if and $cfg.UseForwardedHeaders $cfg.UseProxyProtocol }}
+    real_ip_header      {{ $cfg.ForwardedForProxyProtocolHeader }};
+    {{ else if $cfg.UseProxyProtocol }}
     real_ip_header      proxy_protocol;
     {{ else }}
     real_ip_header      {{ $cfg.ForwardedForHeader }};
@@ -409,6 +411,39 @@ http {
 
     {{ end }}
 
+    {{ if and $cfg.UseForwardedHeaders $cfg.UseProxyProtocol }}
+    # When the proxy protocol is enabled, we cannot rely solely on the proxy protocol address
+    # due to potential proxy chain issues. Multiple proxies may modify the client IP before
+    # it reaches the ingress controller. We use the proxy-real-ip-cidr list to trust specific proxy
+    # addresses and determine the correct client IP from the forwarded headers.
+    #
+    # --------     --------------             ---------------------                       -----------------
+    # | User | --> | HTTP Proxy | -- http --> | TCP Load Balancer | -- proxy protocol --> | Ingress Nginx |
+    # --------     --------------             ---------------------                       -----------------
+    #
+    # The algorithm for determining the header to be used with the real_ip_header:
+    # 1. Check if the $proxy_protocol_addr is trusted (i.e., whether it's in the proxy-real-ip-cidr list).
+    # 2. If trusted, use the configured forwarded-for header (X-Forwarded-For by default).
+    # 3. If not trusted, fall back to using the $proxy_protocol_addr.
+    #
+    geo $proxy_protocol_addr $proxy_protocol_addr_trusted {
+        default           0;
+        {{ range $trusted_ip := $cfg.ProxyRealIPCIDR }}
+        {{ $trusted_ip }} 1;
+        {{ end }}
+    }
+
+    map $proxy_protocol_addr_trusted $forwarded_for_proxy_protocol {
+        default   $proxy_protocol_addr;
+        1         {{ buildForwardedFor $cfg.ForwardedForHeader }};
+    }
+
+    # The realip module does not support variables for the real_ip_header directive
+    # so we need to define a custom header.
+    more_set_input_headers "{{ $cfg.ForwardedForProxyProtocolHeader }}: $forwarded_for_proxy_protocol";
+
+    {{ end }}
+
     # Create a variable that contains the literal $ character.
     # This works because the geo module will not resolve variables.
     geo $literal_dollar {

test/e2e/settings/proxy_protocol.go

@@ -44,6 +44,7 @@ var _ = framework.DescribeSetting("use-proxy-protocol", func() {
 		f.NewEchoDeployment()
 		f.UpdateNginxConfigMapData(setting, "false")
 	})
+
 	//nolint:dupl // Ignore dupl errors for similar test case
 	ginkgo.It("should respect port passed by the PROXY Protocol", func() {
 		host := proxyProtocol
@@ -227,4 +228,86 @@ var _ = framework.DescribeSetting("use-proxy-protocol", func() {
 		assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs")
 		assert.Contains(ginkgo.GinkgoT(), logs, `192.168.0.1`)
 	})
+
+	ginkgo.Context("when use-forwarded-headers setting is true", func() {
+		cmapData := map[string]string{}
+
+		cmapData[setting] = "true"
+		cmapData["use-forwarded-headers"] = "true"
+
+		ginkgo.It("should not trust X-Forwarded headers when the client IP address is not trusted", func() {
+			host := proxyProtocol
+
+			f.SetNginxConfigMapData(cmapData)
+
+			f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil))
+
+			f.WaitForNginxServer(host,
+				func(server string) bool {
+					return strings.Contains(server, "server_name proxy-protocol") &&
+						strings.Contains(server, "listen 80 proxy_protocol")
+				})
+
+			ip := f.GetNginxIP()
+
+			conn, err := net.Dial("tcp", net.JoinHostPort(ip, "80"))
+			assert.Nil(ginkgo.GinkgoT(), err, "unexpected error creating connection to %s:80", ip)
+			defer conn.Close()
+
+			header := "PROXY TCP4 192.168.0.1 192.168.0.11 56324 1234\r\n"
+			_, err = conn.Write([]byte(header))
+			assert.Nil(ginkgo.GinkgoT(), err, "unexpected error writing header")
+
+			_, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: proxy-protocol\r\nX-Forwarded-For: 192.168.0.111\r\n\r\n"))
+			assert.Nil(ginkgo.GinkgoT(), err, "unexpected error writing request")
+
+			data, err := io.ReadAll(conn)
+			assert.Nil(ginkgo.GinkgoT(), err, "unexpected error reading connection data")
+
+			body := string(data)
+			assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf("host=%v", proxyProtocol))
+			assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-port=1234")
+			assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-proto=http")
+			assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-for=192.168.0.1")
+		})
+
+		ginkgo.It("should trust X-Forwarded headers when the client IP address is trusted", func() {
+			host := proxyProtocol
+
+			// Trust IPs from the private network CIDR block and the client IP address in the proxy protocol header
+			cmapData["proxy-real-ip-cidr"] = "10.0.0.0/8,192.168.0.1/32"
+
+			f.SetNginxConfigMapData(cmapData)
+
+			f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil))
+
+			f.WaitForNginxServer(host,
+				func(server string) bool {
+					return strings.Contains(server, "server_name proxy-protocol") &&
+						strings.Contains(server, "listen 80 proxy_protocol")
+				})
+
+			ip := f.GetNginxIP()
+
+			conn, err := net.Dial("tcp", net.JoinHostPort(ip, "80"))
+			assert.Nil(ginkgo.GinkgoT(), err, "unexpected error creating connection to %s:80", ip)
+			defer conn.Close()
+
+			header := "PROXY TCP4 192.168.0.1 192.168.0.11 56324 1234\r\n"
+			_, err = conn.Write([]byte(header))
+			assert.Nil(ginkgo.GinkgoT(), err, "unexpected error writing header")
+
+			_, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: proxy-protocol\r\nX-Forwarded-For: 192.168.0.111\r\n\r\n"))
+			assert.Nil(ginkgo.GinkgoT(), err, "unexpected error writing request")
+
+			data, err := io.ReadAll(conn)
+			assert.Nil(ginkgo.GinkgoT(), err, "unexpected error reading connection data")
+
+			body := string(data)
+			assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf("host=%v", proxyProtocol))
+			assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-port=1234")
+			assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-proto=http")
+			assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-for=192.168.0.111")
+		})
+	})
 })