From 2a818c9d032f7019c91f48b9c2f5d312ae30a82c 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 | 49 +++++++ apis/v1alpha1/zz_generated.deepcopy.go | 35 +++++ charts/nginx-gateway-fabric/values.yaml | 5 + .../bases/gateway.nginx.org_nginxproxies.yaml | 33 +++++ deploy/crds.yaml | 33 +++++ .../mode/static/nginx/config/http/config.go | 13 +- internal/mode/static/nginx/config/servers.go | 20 ++- .../static/nginx/config/servers_template.go | 48 +++++-- .../mode/static/nginx/config/servers_test.go | 136 +++++++++++------- .../static/state/dataplane/configuration.go | 19 +++ .../state/dataplane/configuration_test.go | 78 ++++++++++ internal/mode/static/state/dataplane/types.go | 26 +++- .../mode/static/state/graph/nginxproxy.go | 49 ++++++- .../static/state/graph/nginxproxy_test.go | 125 +++++++++++++--- .../how-to/monitoring/troubleshooting.md | 14 ++ site/content/reference/api.md | 123 ++++++++++++++++ 16 files changed, 721 insertions(+), 85 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 018911da7e..913f4e9336 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,46 @@ 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 + // + // +optional + TrustedAddresses []string `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" +) diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 90cdca7a41..2cbfd84d16 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([]string, 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..066eb61c05 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -93,6 +93,11 @@ nginx: {} # disableHTTP2: false # ipFamily: dual + # enableProxyProtocol: true + # rewriteClientIP: + # mode: "ProxyProtocol" + # trustedAddresses: ["127.0.0.0/28", "::1/128"] + # 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..1c0edfca66 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -62,6 +62,39 @@ 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: + type: string + maxItems: 16 + type: array + 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/crds.yaml b/deploy/crds.yaml index 547c912748..15cde2f395 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -707,6 +707,39 @@ 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: + type: string + maxItems: 16 + type: array + 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/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 9326ebb439..2d3867e2cb 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -115,6 +115,15 @@ type ProxySSLVerify struct { // ServerConfig holds configuration for an HTTP server and IP family to be used by NGINX. type ServerConfig struct { - Servers []Server - IPFamily IPFamily + Servers []Server + RewriteClientIP RewriteClientIPSettings + IPFamily IPFamily +} + +// 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 3aeefa47c7..3d411bf973 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -61,8 +61,9 @@ func executeServers(conf dataplane.Configuration) []executeResult { servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers) serverConfig := http.ServerConfig{ - Servers: servers, - IPFamily: getIPFamily(conf.BaseHTTPConfig), + Servers: servers, + IPFamily: getIPFamily(conf.BaseHTTPConfig), + RewriteClientIP: getRewriteClientIPSettings(conf.BaseHTTPConfig.RewriteClientIPSettings), } serverResult := executeResult{ @@ -103,6 +104,21 @@ func getIPFamily(baseHTTPConfig dataplane.BaseHTTPConfig) http.IPFamily { return http.IPFamily{IPv4: true, IPv6: true} } +// 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), + } +} + func createAdditionFileResults(conf dataplane.Configuration) []executeResult { uniqueAdditions := make(map[string][]byte) diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 4d8196b180..3a56ab3ba5 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; +{{ $proxyProtocol := "" }} +{{ if $.RewriteClientIP.ProxyProtocol }}{{ $proxyProtocol = " proxy_protocol" }}{{ end }} + {{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { {{- if $.IPFamily.IPv4 }} - listen {{ $s.Port }} ssl default_server; + listen {{ $s.Port }} ssl default_server{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }} ssl default_server; + listen [::]:{{ $s.Port }} 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.Port }} default_server; + listen {{ $s.Port }} default_server{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }} default_server; + listen [::]:{{ $s.Port }} 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 $.IPFamily.IPv4 }} - listen {{ $s.Port }} ssl; + listen {{ $s.Port }} ssl{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }} ssl; + listen [::]:{{ $s.Port }} 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.Port }}; + listen {{ $s.Port }}{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }}; + listen [::]:{{ $s.Port }}{{ $proxyProtocol }}; {{- end }} {{- end }} @@ -55,6 +74,15 @@ server { {{- 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 effb0099ab..d8cb74fe56 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -137,7 +137,7 @@ func TestExecuteServers(t *testing.T) { } } -func TestExecuteServersForIPFamily(t *testing.T) { +func TestExecuteServerConfig(t *testing.T) { httpServers := []dataplane.VirtualServer{ { IsDefault: true, @@ -166,68 +166,102 @@ func TestExecuteServersForIPFamily(t *testing.T) { expectedHTTPConfig map[string]int config dataplane.Configuration }{ - { - msg: "http and ssl servers with IPv4 IP family", - config: dataplane.Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, - BaseHTTPConfig: dataplane.BaseHTTPConfig{ - IPFamily: dataplane.IPv4, - }, - }, - expectedHTTPConfig: map[string]int{ - "listen 8080 default_server;": 1, - "listen 8080;": 1, - "listen 8443 ssl default_server;": 1, - "listen 8443 ssl;": 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, - }, - }, - { - msg: "http and ssl servers with IPv6 IP family", - config: dataplane.Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, - BaseHTTPConfig: dataplane.BaseHTTPConfig{ - IPFamily: dataplane.IPv6, - }, - }, - expectedHTTPConfig: map[string]int{ - "listen [::]:8080 default_server;": 1, - "listen [::]:8080;": 1, - "listen [::]:8443 ssl default_server;": 1, - "listen [::]:8443 ssl;": 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, - }, - }, - { - msg: "http and ssl servers with Dual IP family", + // { + // msg: "http and ssl servers with IPv4 IP family", + // config: dataplane.Configuration{ + // HTTPServers: httpServers, + // SSLServers: sslServers, + // BaseHTTPConfig: dataplane.BaseHTTPConfig{ + // IPFamily: dataplane.IPv4, + // }, + // }, + // expectedHTTPConfig: map[string]int{ + // "listen 8080 default_server;": 1, + // "listen 8080;": 1, + // "listen 8443 ssl default_server;": 1, + // "listen 8443 ssl;": 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, + // }, + // }, + // { + // msg: "http and ssl servers with IPv6 IP family", + // config: dataplane.Configuration{ + // HTTPServers: httpServers, + // SSLServers: sslServers, + // BaseHTTPConfig: dataplane.BaseHTTPConfig{ + // IPFamily: dataplane.IPv6, + // }, + // }, + // expectedHTTPConfig: map[string]int{ + // "listen [::]:8080 default_server;": 1, + // "listen [::]:8080;": 1, + // "listen [::]:8443 ssl default_server;": 1, + // "listen [::]:8443 ssl;": 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, + // }, + // }, + // { + // msg: "http and ssl servers with Dual IP family", + // config: dataplane.Configuration{ + // HTTPServers: httpServers, + // SSLServers: sslServers, + // BaseHTTPConfig: dataplane.BaseHTTPConfig{ + // IPFamily: dataplane.Dual, + // }, + // }, + // expectedHTTPConfig: map[string]int{ + // "listen 8080 default_server;": 1, + // "listen 8080;": 1, + // "listen 8443 ssl default_server;": 1, + // "listen 8443 ssl;": 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;": 1, + // "listen [::]:8080;": 1, + // "listen [::]:8443 ssl default_server;": 1, + // "listen [::]:8443 ssl;": 1, + // "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{ - "listen 8080 default_server;": 1, - "listen 8080;": 1, - "listen 8443 ssl default_server;": 1, - "listen 8443 ssl;": 1, + "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;": 1, - "listen [::]:8080;": 1, - "listen [::]:8443 ssl default_server;": 1, - "listen [::]:8443 ssl;": 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 b81dbfe7a0..9bd2587d41 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -696,6 +696,25 @@ 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 { + baseConfig.RewriteClientIPSettings.TrustedCIDRs = g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses + } + + if g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively != nil { + baseConfig.RewriteClientIPSettings.IPRecursive = *g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively + } + } + return baseConfig } diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 83d5873ce0..811bc4e245 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -2089,6 +2089,34 @@ 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{}, + // }, + // } + // return g + // }), + // expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + // conf.SSLServers = []VirtualServer{} + // conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + // conf.BaseHTTPConfig = BaseHTTPConfig{HTTP2: true, IPFamily: Dual, ProxyProtocol: true} + // return conf + // }), + // msg: "NginxProxy with proxy protocol enabled", + // }, } for _, test := range tests { @@ -3215,3 +3243,53 @@ func TestGetAllowedAddressType(t *testing.T) { }) } } + +// TODO: add more tests. +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: []string{"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 d342ff3b0c..846b1a086f 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -33,10 +33,10 @@ type Configuration struct { Upstreams []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 } @@ -298,10 +298,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..f53ff34cb5 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" @@ -31,7 +32,6 @@ func buildNginxProxy( npCfg := nps[types.NamespacedName{Name: gc.Spec.ParametersRef.Name}] if npCfg != nil { errs := validateNginxProxy(validator, npCfg) - return &NginxProxy{ Source: npCfg, Valid: len(errs) == 0, @@ -126,5 +126,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 i, addr := range rewriteClientIP.TrustedAddresses { + if err := k8svalidation.IsValidCIDR(trustedAddressesPath, addr); err != nil { + allErrs = append(allErrs, field.Invalid(trustedAddressesPath.Index(i), 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..611145d108 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: []string{"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: []string{"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: []string{"2001:db8:a0b:12f0::1"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + expectErrCount: 1, + expErrSubstring: "spec.rewriteClientIP.trustedAddresses[0]", + }, + { + 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: []string{ + "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 e15bee6a1c..98ff2f8b05 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -439,6 +439,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 b597e1a39e..1e738de3d4 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -328,6 +328,20 @@ Telemetry +rewriteClientIP
+ + +RewriteClientIP + + + + +(Optional) +

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

+ + + + disableHTTP2
bool @@ -950,6 +964,20 @@ Telemetry +rewriteClientIP
+ + +RewriteClientIP + + + + +(Optional) +

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

+ + + + disableHTTP2
bool @@ -1012,6 +1040,101 @@ 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
+ +[]string + +		 +(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)