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

NGINX: Correctly determine client IP. #12768

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion docs/user-guide/miscellaneous.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions docs/user-guide/nginx-configuration/configmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -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" | |
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions internal/ingress/controller/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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,
Expand Down
37 changes: 36 additions & 1 deletion rootfs/etc/nginx/template/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }};
Expand Down Expand Up @@ -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 {
Expand Down
83 changes: 83 additions & 0 deletions test/e2e/settings/proxy_protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
})
})
})