From 8835f3ad2a6d8c85d273ebbc5fa66ba49e998c42 Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 23 Jul 2024 15:34:51 -0600 Subject: [PATCH] add support for preserving clientIP --- apis/v1alpha1/nginxproxy_types.go | 59 ++++++++ apis/v1alpha1/zz_generated.deepcopy.go | 35 +++++ charts/nginx-gateway-fabric/values.yaml | 4 + .../bases/gateway.nginx.org_nginxproxies.yaml | 38 +++++ deploy/azure/deploy.yaml | 20 +++ deploy/crds.yaml | 38 +++++ deploy/default/deploy.yaml | 20 +++ deploy/experimental/deploy.yaml | 20 +++ deploy/nodeport/deploy.yaml | 20 +++ .../mode/static/nginx/config/http/config.go | 15 +- internal/mode/static/nginx/config/servers.go | 22 ++- .../static/nginx/config/servers_template.go | 56 ++++++-- .../mode/static/nginx/config/servers_test.go | 36 ++++- .../static/state/dataplane/configuration.go | 28 ++++ .../state/dataplane/configuration_test.go | 91 ++++++++++++ internal/mode/static/state/dataplane/types.go | 26 +++- .../mode/static/state/graph/nginxproxy.go | 48 +++++++ .../static/state/graph/nginxproxy_test.go | 125 +++++++++++++--- .../how-to/monitoring/troubleshooting.md | 14 ++ site/content/reference/api.md | 136 ++++++++++++++++++ 20 files changed, 813 insertions(+), 38 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 018911da7e..5142f3a250 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -53,6 +53,12 @@ type NginxProxySpec struct { // // +optional Telemetry *Telemetry `json:"telemetry,omitempty"` + // RewriteClientIP defines configuration for rewriting the client IP to the original client's IP. + // +kubebuilder:validation:XValidation:message="if mode is set, trustedAddresses is a required field",rule="!(has(self.mode) && !has(self.trustedAddresses))" + // + // +optional + //nolint:lll + RewriteClientIP *RewriteClientIP `json:"rewriteClientIP,omitempty"` // DisableHTTP2 defines if http2 should be disabled for all servers. // Default is false, meaning http2 will be enabled for all servers. // @@ -114,3 +120,56 @@ type TelemetryExporter struct { // +kubebuilder:validation:Pattern=`^(?:http?:\/\/)?[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*(?::\d{1,5})?$` Endpoint string `json:"endpoint"` } + +// RewriteClientIP specifies the configuration for rewriting the client's IP address. +type RewriteClientIP struct { + // Mode defines how NGINX will rewrite the client's IP address. + // Possible modes: ProxyProtocol, XForwardedFor. + // + // +optional + Mode *RewriteClientIPModeType `json:"mode,omitempty"` + + // SetIPRecursively configures whether recursive search is used for selecting client's + // address from the X-Forwarded-For header and used in conjunction with TrustedAddresses. + // If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of + // array to start of array and select the first untrusted IP. + // + // +optional + SetIPRecursively *bool `json:"setIPRecursively,omitempty"` + + // TrustedAddresses specifies the addresses that are trusted to send correct client IP information. + // If a request comes from a trusted address, NGINX will rewrite the client IP information, + // and forward it to the backend in the X-Forwarded-For* and X-Real-IP headers. + // This field is required if mode is set. + // +kubebuilder:validation:MaxItems=16 + // +listType=atomic + // + // + // +optional + TrustedAddresses []TrustedAddress `json:"trustedAddresses,omitempty"` +} + +// RewriteClientIPModeType defines how NGINX Gateway Fabric will determine the client's original IP address. +// +kubebuilder:validation:Enum=ProxyProtocol;XForwardedFor +type RewriteClientIPModeType string + +const ( + // RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and, + // set the client's IP address to the IP address in the PROXY protocol header. + // Sets the proxy_protocol parameter to the listen directive on all servers, and sets real_ip_header + // to proxy_protocol: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header. + RewriteClientIPModeProxyProtocol RewriteClientIPModeType = "ProxyProtocol" + + // RewriteClientIPModeXForwardedFor configures NGINX to set the client's IP address to the + // IP address in the X-Forwarded-For HTTP header. + // https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header. + RewriteClientIPModeXForwardedFor RewriteClientIPModeType = "XForwardedFor" +) + +// TrustedAddress is a string value representing a CIDR block. +// Examples: 0.0.0.0/0 +// +// +kubebuilder:validation:Pattern=`^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$` +// +//nolint:lll +type TrustedAddress string diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 90cdca7a41..42410e127b 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -367,6 +367,11 @@ func (in *NginxProxySpec) DeepCopyInto(out *NginxProxySpec) { *out = new(Telemetry) (*in).DeepCopyInto(*out) } + if in.RewriteClientIP != nil { + in, out := &in.RewriteClientIP, &out.RewriteClientIP + *out = new(RewriteClientIP) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxySpec. @@ -463,6 +468,36 @@ func (in *ObservabilityPolicySpec) DeepCopy() *ObservabilityPolicySpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RewriteClientIP) DeepCopyInto(out *RewriteClientIP) { + *out = *in + if in.Mode != nil { + in, out := &in.Mode, &out.Mode + *out = new(RewriteClientIPModeType) + **out = **in + } + if in.SetIPRecursively != nil { + in, out := &in.SetIPRecursively, &out.SetIPRecursively + *out = new(bool) + **out = **in + } + if in.TrustedAddresses != nil { + in, out := &in.TrustedAddresses, &out.TrustedAddresses + *out = make([]TrustedAddress, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RewriteClientIP. +func (in *RewriteClientIP) DeepCopy() *RewriteClientIP { + if in == nil { + return nil + } + out := new(RewriteClientIP) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SpanAttribute) DeepCopyInto(out *SpanAttribute) { *out = *in diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 9cfc2064b2..6260c1dbb2 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -93,6 +93,10 @@ nginx: {} # disableHTTP2: false # ipFamily: dual + # rewriteClientIP: + # mode: "ProxyProtocol" + # trustedAddresses: ["0.0.0.0/0"] + # setIPRecursively: true # telemetry: # exporter: # endpoint: otel-collector.default.svc:4317 diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 73acae5e84..f22bac037c 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -62,6 +62,44 @@ spec: - ipv4 - ipv6 type: string + rewriteClientIP: + description: RewriteClientIP defines configuration for rewriting the + client IP to the original client's IP. + properties: + mode: + description: |- + Mode defines how NGINX will rewrite the client's IP address. + Possible modes: ProxyProtocol, XForwardedFor. + enum: + - ProxyProtocol + - XForwardedFor + type: string + setIPRecursively: + description: |- + SetIPRecursively configures whether recursive search is used for selecting client's + address from the X-Forwarded-For header and used in conjunction with TrustedAddresses. + If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of + array to start of array and select the first untrusted IP. + type: boolean + trustedAddresses: + description: |- + TrustedAddresses specifies the addresses that are trusted to send correct client IP information. + If a request comes from a trusted address, NGINX will rewrite the client IP information, + and forward it to the backend in the X-Forwarded-For* and X-Real-IP headers. + This field is required if mode is set. + items: + description: |- + TrustedAddress is a string value representing a CIDR block. + Examples: 0.0.0.0/0 + pattern: ^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$ + type: string + maxItems: 16 + type: array + x-kubernetes-list-type: atomic + type: object + x-kubernetes-validations: + - message: if mode is set, trustedAddresses is a required field + rule: '!(has(self.mode) && !has(self.trustedAddresses))' telemetry: description: Telemetry specifies the OpenTelemetry configuration. properties: diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index 968c1a2926..d633e1fe22 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -321,6 +321,10 @@ metadata: name: nginx spec: controllerName: gateway.nginx.org/nginx-gateway-controller + parametersRef: + group: gateway.nginx.org + kind: NginxProxy + name: nginx-gateway-proxy-config --- apiVersion: gateway.nginx.org/v1alpha1 kind: NginxGateway @@ -334,3 +338,19 @@ metadata: spec: logging: level: info +--- +# Source: nginx-gateway-fabric/templates/nginxproxy.yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxProxy +metadata: + name: nginx-gateway-proxy-config + labels: + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/version: "edge" +spec: + rewriteClientIP: + mode: ProxyProtocol + setIPRecursively: true + trustedAddresses: + - 0.0.0.0/0 diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 547c912748..d47a365d62 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -707,6 +707,44 @@ spec: - ipv4 - ipv6 type: string + rewriteClientIP: + description: RewriteClientIP defines configuration for rewriting the + client IP to the original client's IP. + properties: + mode: + description: |- + Mode defines how NGINX will rewrite the client's IP address. + Possible modes: ProxyProtocol, XForwardedFor. + enum: + - ProxyProtocol + - XForwardedFor + type: string + setIPRecursively: + description: |- + SetIPRecursively configures whether recursive search is used for selecting client's + address from the X-Forwarded-For header and used in conjunction with TrustedAddresses. + If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of + array to start of array and select the first untrusted IP. + type: boolean + trustedAddresses: + description: |- + TrustedAddresses specifies the addresses that are trusted to send correct client IP information. + If a request comes from a trusted address, NGINX will rewrite the client IP information, + and forward it to the backend in the X-Forwarded-For* and X-Real-IP headers. + This field is required if mode is set. + items: + description: |- + TrustedAddress is a string value representing a CIDR block. + Examples: 0.0.0.0/0 + pattern: ^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$ + type: string + maxItems: 16 + type: array + x-kubernetes-list-type: atomic + type: object + x-kubernetes-validations: + - message: if mode is set, trustedAddresses is a required field + rule: '!(has(self.mode) && !has(self.trustedAddresses))' telemetry: description: Telemetry specifies the OpenTelemetry configuration. properties: diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index 6245a2bbc7..e5f7bbe7c9 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -319,6 +319,10 @@ metadata: name: nginx spec: controllerName: gateway.nginx.org/nginx-gateway-controller + parametersRef: + group: gateway.nginx.org + kind: NginxProxy + name: nginx-gateway-proxy-config --- apiVersion: gateway.nginx.org/v1alpha1 kind: NginxGateway @@ -332,3 +336,19 @@ metadata: spec: logging: level: info +--- +# Source: nginx-gateway-fabric/templates/nginxproxy.yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxProxy +metadata: + name: nginx-gateway-proxy-config + labels: + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/version: "edge" +spec: + rewriteClientIP: + mode: ProxyProtocol + setIPRecursively: true + trustedAddresses: + - 0.0.0.0/0 diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index 28cc7b6d19..f36903769e 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -325,6 +325,10 @@ metadata: name: nginx spec: controllerName: gateway.nginx.org/nginx-gateway-controller + parametersRef: + group: gateway.nginx.org + kind: NginxProxy + name: nginx-gateway-proxy-config --- apiVersion: gateway.nginx.org/v1alpha1 kind: NginxGateway @@ -338,3 +342,19 @@ metadata: spec: logging: level: info +--- +# Source: nginx-gateway-fabric/templates/nginxproxy.yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxProxy +metadata: + name: nginx-gateway-proxy-config + labels: + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/version: "edge" +spec: + rewriteClientIP: + mode: ProxyProtocol + setIPRecursively: true + trustedAddresses: + - 0.0.0.0/0 diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index db81fdf259..f1b2d41f28 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -319,6 +319,10 @@ metadata: name: nginx spec: controllerName: gateway.nginx.org/nginx-gateway-controller + parametersRef: + group: gateway.nginx.org + kind: NginxProxy + name: nginx-gateway-proxy-config --- apiVersion: gateway.nginx.org/v1alpha1 kind: NginxGateway @@ -332,3 +336,19 @@ metadata: spec: logging: level: info +--- +# Source: nginx-gateway-fabric/templates/nginxproxy.yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxProxy +metadata: + name: nginx-gateway-proxy-config + labels: + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/version: "edge" +spec: + rewriteClientIP: + mode: ProxyProtocol + setIPRecursively: true + trustedAddresses: + - 0.0.0.0/0 diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 4e26604196..854ac109a2 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -109,9 +109,10 @@ type ProxySSLVerify struct { // ServerConfig holds configuration for an HTTP server and IP family to be used by NGINX. type ServerConfig struct { - Servers []Server - IPFamily shared.IPFamily - Plus bool + Servers []Server + RewriteClientIP RewriteClientIPSettings + IPFamily shared.IPFamily + Plus bool } // Include defines a file that's included via the include directive. @@ -119,3 +120,11 @@ type Include struct { Name string Content []byte } + +// RewriteClientIP holds the configuration for the rewrite client IP settings. +type RewriteClientIPSettings struct { + RealIPHeader string + RealIPFrom []string + Recursive bool + ProxyProtocol bool +} diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 4ced814bd0..c3b6e6e299 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -69,9 +69,10 @@ func (g GeneratorImpl) executeServers(conf dataplane.Configuration, generator po servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, conf.TLSPassthroughServers, generator) serverConfig := http.ServerConfig{ - Servers: servers, - IPFamily: getIPFamily(conf.BaseHTTPConfig), - Plus: g.plus, + Servers: servers, + IPFamily: getIPFamily(conf.BaseHTTPConfig), + Plus: g.plus, + RewriteClientIP: getRewriteClientIPSettings(conf.BaseHTTPConfig.RewriteClientIPSettings), } serverResult := executeResult{ @@ -874,3 +875,18 @@ func createDefaultRootLocation() http.Location { func isNonSlashedPrefixPath(pathType dataplane.PathType, path string) bool { return pathType == dataplane.PathTypePrefix && !strings.HasSuffix(path, "/") } + +// getRewriteClientIPSettings returns the configuration for the rewriting client IP settings. +func getRewriteClientIPSettings(rewriteIP dataplane.RewriteClientIPSettings) http.RewriteClientIPSettings { + var proxyProtocol bool + if rewriteIP.Mode == dataplane.RewriteIPModeProxyProtocol { + proxyProtocol = true + } + + return http.RewriteClientIPSettings{ + Recursive: rewriteIP.IPRecursive, + ProxyProtocol: proxyProtocol, + RealIPFrom: rewriteIP.TrustedCIDRs, + RealIPHeader: string(rewriteIP.Mode), + } +} diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 02b8fae97f..6249046622 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -2,27 +2,46 @@ package config const serversTemplateText = ` js_preload_object matches from /etc/nginx/conf.d/matches.json; -{{ range $s := .Servers -}} +{{ $proxyProtocol := "" }} +{{ if $.RewriteClientIP.ProxyProtocol }}{{ $proxyProtocol = " proxy_protocol" }}{{ end }} + +{{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { - {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} - listen {{ $s.Listen }} ssl default_server; + {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} + listen {{ $s.Listen }} ssl default_server{{ $proxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} - listen [::]:{{ $s.Listen }} ssl default_server; + listen [::]:{{ $s.Listen }} ssl default_server{{ $proxyProtocol }}; {{- end }} - ssl_reject_handshake on; + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $cidr }}; + {{- end}} + {{- if $.RewriteClientIP.RealIPHeader}} + real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; + {{- end}} + {{- if $.RewriteClientIP.Recursive }} + real_ip_recursive on; + {{ end }} } {{- else if $s.IsDefaultHTTP }} server { {{- if $.IPFamily.IPv4 }} - listen {{ $s.Listen }} default_server; + listen {{ $s.Listen }} default_server{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Listen }} default_server; + listen [::]:{{ $s.Listen }} default_server{{ $proxyProtocol }}; {{- end }} - + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $cidr }}; + {{- end}} + {{- if $.RewriteClientIP.RealIPHeader}} + real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; + {{- end}} + {{- if $.RewriteClientIP.Recursive }} + real_ip_recursive on; + {{ end }} default_type text/html; return 404; } @@ -30,10 +49,10 @@ server { server { {{- if $s.SSL }} {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} - listen {{ $s.Listen }} ssl; + listen {{ $s.Listen }} ssl{{ $proxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} - listen [::]:{{ $s.Listen }} ssl; + listen [::]:{{ $s.Listen }} ssl{{ $proxyProtocol }}; {{- end }} ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; @@ -43,10 +62,10 @@ server { } {{- else }} {{- if $.IPFamily.IPv4 }} - listen {{ $s.Listen }}; + listen {{ $s.Listen }}{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Listen }}; + listen [::]:{{ $s.Listen }}{{ $proxyProtocol }}; {{- end }} {{- end }} @@ -59,6 +78,19 @@ server { {{- range $i := $s.Includes }} include {{ $i.Name }}; {{- end }} + {{- range $i := $s.Includes }} + include {{ $i }}; + {{ end -}} + + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $cidr }}; + {{- end}} + {{- if $.RewriteClientIP.RealIPHeader}} + real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; + {{- end}} + {{- if $.RewriteClientIP.Recursive }} + real_ip_recursive on; + {{ end }} {{ range $l := $s.Locations }} location {{ $l.Path }} { diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index a912e6f2bc..d3ab248f75 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -144,7 +144,7 @@ func TestExecuteServers(t *testing.T) { } } -func TestExecuteServers_IPFamily(t *testing.T) { +func TestExecuteServerConfig(t *testing.T) { httpServers := []dataplane.VirtualServer{ { IsDefault: true, @@ -259,6 +259,40 @@ func TestExecuteServers_IPFamily(t *testing.T) { "listen [::]:8443 ssl default_server;": 1, "listen [::]:8443 ssl;": 1, "status_zone": 0, + "real_ip_header proxy-protocol;": 0, + "real_ip_recursive on;": 0, + }, + }, + { + msg: "http and ssl servers with rewrite client IP settings", + config: dataplane.Configuration{ + HTTPServers: httpServers, + SSLServers: sslServers, + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + IPFamily: dataplane.Dual, + RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ + Mode: dataplane.RewriteIPModeProxyProtocol, + TrustedCIDRs: []string{"0.0.0.0/0"}, + IPRecursive: true, + }, + }, + }, + expectedHTTPConfig: map[string]int{ + "set_real_ip_from 0.0.0.0/0;": 4, + "real_ip_header proxy-protocol;": 4, + "real_ip_recursive on;": 4, + "listen 8080 default_server proxy_protocol;": 1, + "listen 8080 proxy_protocol;": 1, + "listen 8443 ssl default_server proxy_protocol;": 1, + "listen 8443 ssl proxy_protocol;": 1, + "server_name example.com;": 2, + "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 1, + "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 1, + "ssl_reject_handshake on;": 1, + "listen [::]:8080 default_server proxy_protocol;": 1, + "listen [::]:8080 proxy_protocol;": 1, + "listen [::]:8443 ssl default_server proxy_protocol;": 1, + "listen [::]:8443 ssl proxy_protocol;": 1, }, }, } diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index ebaf15bf0b..54e40232ee 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -852,6 +852,26 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { } } + if g.NginxProxy.Source.Spec.RewriteClientIP != nil { + if g.NginxProxy.Source.Spec.RewriteClientIP.Mode != nil { + switch *g.NginxProxy.Source.Spec.RewriteClientIP.Mode { + case ngfAPI.RewriteClientIPModeProxyProtocol: + baseConfig.RewriteClientIPSettings.Mode = RewriteIPModeProxyProtocol + case ngfAPI.RewriteClientIPModeXForwardedFor: + baseConfig.RewriteClientIPSettings.Mode = RewriteIPModeXForwardedFor + } + } + + if len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses) > 0 { + trustedAddresses := convertTrustedCIDRs(g) + baseConfig.RewriteClientIPSettings.TrustedCIDRs = trustedAddresses + } + + if g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively != nil { + baseConfig.RewriteClientIPSettings.IPRecursive = *g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively + } + } + return baseConfig } @@ -872,3 +892,11 @@ func buildPolicies(graphPolicies []*graph.Policy) []policies.Policy { return finalPolicies } + +func convertTrustedCIDRs(g *graph.Graph) []string { + trustedAddresses := make([]string, len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses)) + for i, addr := range g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses { + trustedAddresses[i] = string(addr) + } + return trustedAddresses +} diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 0510a3719b..e303b9ed50 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -2180,6 +2180,48 @@ func TestBuildConfiguration(t *testing.T) { }), msg: "NginxProxy with IPv6 IPFamily and no routes", }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + g.NginxProxy = &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.TrustedAddress{"0.0.0.0/0"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.BaseHTTPConfig = BaseHTTPConfig{ + HTTP2: true, + IPFamily: Dual, + RewriteClientIPSettings: RewriteClientIPSettings{ + IPRecursive: true, + TrustedCIDRs: []string{"0.0.0.0/0"}, + Mode: RewriteIPModeProxyProtocol, + }, + } + return conf + }), + msg: "NginxProxy with rewriteClientIP details set", + }, } for _, test := range tests { @@ -3552,3 +3594,52 @@ func TestBuildStreamUpstreams(t *testing.T) { g.Expect(streamUpstreams).To(ConsistOf(expectedStreamUpstreams)) } + +func TestBuildRewriteIPSettings(t *testing.T) { + tests := []struct { + msg string + g *graph.Graph + expRewriteIPSettings RewriteClientIPSettings + }{ + { + msg: "no rewrite IP settings configured", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{}, + }, + }, + expRewriteIPSettings: RewriteClientIPSettings{}, + }, + { + msg: "rewrite IP settings configured", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + TrustedAddresses: []ngfAPI.TrustedAddress{"0.0.0.0/0"}, + SetIPRecursively: helpers.GetPointer(true), + }, + }, + }, + }, + }, + expRewriteIPSettings: RewriteClientIPSettings{ + Mode: RewriteIPModeProxyProtocol, + TrustedCIDRs: []string{"0.0.0.0/0"}, + IPRecursive: true, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.msg, func(t *testing.T) { + g := NewWithT(t) + baseConfig := buildBaseHTTPConfig(tc.g) + g.Expect(baseConfig.RewriteClientIPSettings).To(Equal(tc.expRewriteIPSettings)) + }) + } +} diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 3741f131c8..185c18c7f2 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -38,10 +38,10 @@ type Configuration struct { StreamUpstreams []Upstream // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup - // BaseHTTPConfig holds the configuration options at the http context. - BaseHTTPConfig BaseHTTPConfig // Telemetry holds the Otel configuration. Telemetry Telemetry + // BaseHTTPConfig holds the configuration options at the http context. + BaseHTTPConfig BaseHTTPConfig // Version represents the version of the generated configuration. Version int } @@ -309,10 +309,32 @@ type SpanAttribute struct { type BaseHTTPConfig struct { // IPFamily specifies the IP family for all servers. IPFamily IPFamilyType + // RewriteIPSettings defines configuration for rewriting the client IP to the original client's IP. + RewriteClientIPSettings RewriteClientIPSettings // HTTP2 specifies whether http2 should be enabled for all servers. HTTP2 bool } +// RewriteIPSettings defines configuration for rewriting the client IP to the original client's IP. +type RewriteClientIPSettings struct { + // Mode specifies the mode for rewriting the client IP. + Mode RewriteIPModeType + // TrustedCIDRs specifies the CIDRs that are trusted to provide the client IP. + TrustedCIDRs []string + // IPRecursive specifies whether a recursive search is used when selecting the client IP. + IPRecursive bool +} + +// RewriteIPModeType specifies the mode for rewriting the client IP. +type RewriteIPModeType string + +const ( + // RewriteIPModeProxyProtocol specifies that client IP will be rewrritten using the Proxy-Protocol header. + RewriteIPModeProxyProtocol RewriteIPModeType = "proxy-protocol" + // RewriteIPModeXForwardedFor specifies that client IP will be rewrritten using the X-Forwarded-For header. + RewriteIPModeXForwardedFor RewriteIPModeType = "X-Forwarded-For" +) + // IPFamilyType specifies the IP family to be used by NGINX. type IPFamilyType string diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index d36133308b..bd3b824b53 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -2,6 +2,7 @@ package graph import ( "k8s.io/apimachinery/pkg/types" + k8svalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" v1 "sigs.k8s.io/gateway-api/apis/v1" @@ -126,5 +127,52 @@ func validateNginxProxy( npCfg.Spec.IPFamily = helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual) } + allErrs = append(allErrs, validateRewriteClientIP(npCfg)...) + + return allErrs +} + +func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { + var allErrs field.ErrorList + spec := field.NewPath("spec") + + if npCfg.Spec.RewriteClientIP != nil { + rewriteClientIP := npCfg.Spec.RewriteClientIP + rewriteClientIPPath := spec.Child("rewriteClientIP") + trustedAddressesPath := rewriteClientIPPath.Child("trustedAddresses") + + if rewriteClientIP.Mode != nil { + mode := *rewriteClientIP.Mode + if len(rewriteClientIP.TrustedAddresses) == 0 { + allErrs = append(allErrs, + field.Required(rewriteClientIPPath, "trustedAddresses field required when mode is set")) + } + + switch mode { + case ngfAPI.RewriteClientIPModeProxyProtocol, ngfAPI.RewriteClientIPModeXForwardedFor: + default: + allErrs = append( + allErrs, + field.NotSupported( + rewriteClientIPPath.Child("mode"), + mode, + []string{string(ngfAPI.RewriteClientIPModeProxyProtocol), string(ngfAPI.RewriteClientIPModeXForwardedFor)}), + ) + } + } + + if len(rewriteClientIP.TrustedAddresses) > 16 { + allErrs = append( + allErrs, + field.TooLongMaxLength(trustedAddressesPath, rewriteClientIP.TrustedAddresses, 16)) + } + + for _, addr := range rewriteClientIP.TrustedAddresses { + if err := k8svalidation.IsValidCIDR(trustedAddressesPath, string(addr)); err != nil { + allErrs = append(allErrs, field.Invalid(trustedAddressesPath.Child(string(addr)), addr, err.ToAggregate().Error())) + } + } + } + return allErrs } diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index efec06e865..c521a0b784 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -222,27 +222,27 @@ func TestGCReferencesAnyNginxProxy(t *testing.T) { } } -func TestValidateNginxProxy(t *testing.T) { - createValidValidator := func() *validationfakes.FakeGenericValidator { - v := &validationfakes.FakeGenericValidator{} - v.ValidateEscapedStringNoVarExpansionReturns(nil) - v.ValidateEndpointReturns(nil) - v.ValidateServiceNameReturns(nil) - v.ValidateNginxDurationReturns(nil) +func createValidValidator() *validationfakes.FakeGenericValidator { + v := &validationfakes.FakeGenericValidator{} + v.ValidateEscapedStringNoVarExpansionReturns(nil) + v.ValidateEndpointReturns(nil) + v.ValidateServiceNameReturns(nil) + v.ValidateNginxDurationReturns(nil) - return v - } + return v +} - createInvalidValidator := func() *validationfakes.FakeGenericValidator { - v := &validationfakes.FakeGenericValidator{} - v.ValidateEscapedStringNoVarExpansionReturns(errors.New("error")) - v.ValidateEndpointReturns(errors.New("error")) - v.ValidateServiceNameReturns(errors.New("error")) - v.ValidateNginxDurationReturns(errors.New("error")) +func createInvalidValidator() *validationfakes.FakeGenericValidator { + v := &validationfakes.FakeGenericValidator{} + v.ValidateEscapedStringNoVarExpansionReturns(errors.New("error")) + v.ValidateEndpointReturns(errors.New("error")) + v.ValidateServiceNameReturns(errors.New("error")) + v.ValidateNginxDurationReturns(errors.New("error")) - return v - } + return v +} +func TestValidateNginxProxy(t *testing.T) { tests := []struct { np *ngfAPI.NginxProxy validator *validationfakes.FakeGenericValidator @@ -266,6 +266,11 @@ func TestValidateNginxProxy(t *testing.T) { }, }, IPFamily: helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual), + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "0.0.0.0/0"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, }, }, expectErrCount: 0, @@ -356,3 +361,89 @@ func TestValidateNginxProxy(t *testing.T) { }) } } + +func TestValidateRewriteClientIP(t *testing.T) { + tests := []struct { + np *ngfAPI.NginxProxy + validator *validationfakes.FakeGenericValidator + name string + expErrSubstring string + expectErrCount int + }{ + { + name: "valid rewriteClientIP", + validator: createValidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "0.0.0.0/0"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + expectErrCount: 0, + }, + { + name: "invalid CIDR in trustedAddresses", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + expectErrCount: 1, + expErrSubstring: "spec.rewriteClientIP.trustedAddresses", + }, + { + name: "invalid when mode is set and trustedAddresses is empty", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + expectErrCount: 1, + expErrSubstring: "trustedAddresses field required when mode is set", + }, + { + name: "invalid when trustedAddresses is greater in length than 16", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + TrustedAddresses: []ngfAPI.TrustedAddress{ + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + }, + }, + }, + }, + expectErrCount: 1, + expErrSubstring: "Too long: may not be longer than 16", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + allErrs := validateRewriteClientIP(test.np) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + if len(allErrs) > 0 { + g.Expect(allErrs.ToAggregate().Error()).To(ContainSubstring(test.expErrSubstring)) + } + }) + } +} diff --git a/site/content/how-to/monitoring/troubleshooting.md b/site/content/how-to/monitoring/troubleshooting.md index efb2457011..cdff635482 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -436,6 +436,20 @@ To **resolve** this, you can do one of the following: - Adjust the IPFamily of your Service to match that of the NginxProxy configuration. +##### Broken Header Error + +If you check your _nginx_ container logs and see the following error: + + ```text + 2024/07/25 00:50:45 [error] 211#211: *22 broken header: "GET /coffee HTTP/1.1" while reading PROXY protocol, client: 127.0.0.1, server: 0.0.0.0:80 + ``` + +It indicates that `proxy_protocol` is enabled for the gateway listeners but the request sent to the application endpoint does not contain proxy information.To **resolve** this, you can do one of the following: + +- Update the NginxProxy configuration with [`enableProxyProtocol`](({{< relref "reference/api.md" >}})) disabled. + +- Send valid proxy information with requests being handled by your application. + ### Further reading You can view the [Kubernetes Troubleshooting Guide](https://kubernetes.io/docs/tasks/debug/debug-application/) for more debugging guidance. diff --git a/site/content/reference/api.md b/site/content/reference/api.md index d9dd3d2de2..e7105983bd 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -329,6 +329,20 @@ Telemetry +rewriteClientIP
+ + +RewriteClientIP + + + + +(Optional) +

RewriteClientIP defines configuration for rewriting the client IP to the original client’s IP.

+ + + + disableHTTP2
bool @@ -951,6 +965,20 @@ Telemetry +rewriteClientIP
+ + +RewriteClientIP + + + + +(Optional) +

RewriteClientIP defines configuration for rewriting the client IP to the original client’s IP.

+ + + + disableHTTP2
bool @@ -1013,6 +1041,103 @@ Support: HTTPRoute, GRPCRoute.

+

RewriteClientIP + +

+

+(Appears on: +NginxProxySpec) +

+

+

RewriteClientIP specifies the configuration for rewriting the client’s IP address.

+

+ + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+mode
+ + +RewriteClientIPModeType + + +		 +(Optional) +

Mode defines how NGINX will rewrite the client’s IP address. +Possible modes: ProxyProtocol, XForwardedFor.

+
+setIPRecursively
+ +bool + +		 +(Optional) +

SetIPRecursively configures whether recursive search is used for selecting client’s +address from the X-Forwarded-For header and used in conjunction with TrustedAddresses. +If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of +array to start of array and select the first untrusted IP.

+
+trustedAddresses
+ + +[]TrustedAddress + + +		 +(Optional) +

TrustedAddresses specifies the addresses that are trusted to send correct client IP information. +If a request comes from a trusted address, NGINX will rewrite the client IP information, +and forward it to the backend in the X-Forwarded-For* and X-Real-IP headers. +This field is required if mode is set.

+
+

RewriteClientIPModeType +(string alias)

+

+

+(Appears on: +RewriteClientIP) +

+

+

RewriteClientIPModeType defines how NGINX Gateway Fabric will determine the client’s original IP address.

+

+ + + + + + + + + + + + +
ValueDescription

"ProxyProtocol"

RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and, +set the client’s IP address to the IP address in the PROXY protocol header. +Sets the proxy_protocol parameter to the listen directive on all servers, and sets real_ip_header +to proxy_protocol: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header.

+

"XForwardedFor"

RewriteClientIPModeXForwardedFor configures NGINX to set the client’s IP address to the +IP address in the X-Forwarded-For HTTP header. +https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header.

+

Size (string alias)

@@ -1353,6 +1478,17 @@ Examples of invalid names: some-$value, quoted-“value”-name, unescap +

TrustedAddress +(string alias)

+

+

+(Appears on: +RewriteClientIP) +

+

+

TrustedAddress is a string value representing a CIDR block. +Examples: 0.0.0.0/0

+

Generated with gen-crd-api-reference-docs