From bcab7c1f0bf3dd5032860bb9ecdd93b0b9c4c822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pinho?= Date: Wed, 29 Nov 2023 08:23:55 +0000 Subject: [PATCH 1/7] adds the enable-real-ip-recursive allowing control over real_ip_recursive --- internal/ingress/controller/config/config.go | 6 ++++++ rootfs/etc/nginx/template/nginx.tmpl | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index bad82b8b0..85bd4bbad 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -556,6 +556,11 @@ type Configuration struct { // Sets whether to enable the real ip module EnableRealIP bool `json:"enable-real-ip"` + // Sets whether to use recursive search in the real ip module + // https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive + // Default: true + EnableRealIpRecursive bool `json:"enable-real-ip-recursive"` + // Sets the header field for identifying the originating IP address of a client // Default is X-Forwarded-For ForwardedForHeader string `json:"forwarded-for-header,omitempty"` @@ -790,6 +795,7 @@ func NewDefault() Configuration { ErrorLogLevel: errorLevel, UseForwardedHeaders: false, EnableRealIP: false, + EnableRealIPRecursive: true, ForwardedForHeader: "X-Forwarded-For", ComputeFullForwardedFor: false, ProxyAddOriginalURIHeader: false, diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 94dc12412..4c22cf62c 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -143,7 +143,12 @@ http { real_ip_header {{ $cfg.ForwardedForHeader }}; {{ end }} + {{ if $cfg.EnableRealIpRecursive }} real_ip_recursive on; + {{ else }} + real_ip_recursive off; + {{ end }} + {{ range $trusted_ip := $cfg.ProxyRealIPCIDR }} set_real_ip_from {{ $trusted_ip }}; {{ end }} From 8a268e46e23c3a5052bc0a76c695a31aff5d3023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pinho?= Date: Mon, 21 Mar 2022 18:57:39 +0000 Subject: [PATCH 2/7] documents new enable-real-ip-recursive configmap setting --- docs/user-guide/nginx-configuration/configmap.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 7038ca90d..4db186b17 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -929,6 +929,10 @@ If false, NGINX ignores incoming `X-Forwarded-*` headers, filling them with the `enable-real-ip` enables the configuration of [https://nginx.org/en/docs/http/ngx_http_realip_module.html](https://nginx.org/en/docs/http/ngx_http_realip_module.html). Specific attributes of the module can be configured further by using `forwarded-for-header` and `proxy-real-ip-cidr` settings. +## enable-real-ip-recursive + +`enable-real-ip-recursive` enables the configuration of [https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive](https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive). + ## forwarded-for-header Sets the header field for identifying the originating IP address of a client. _**default:**_ X-Forwarded-For From ae7fee940fa31c438d2d2c4b6e032ebcaebd7646 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pinho?= Date: Tue, 22 Mar 2022 14:46:54 +0000 Subject: [PATCH 3/7] add e2e test for enable-real-ip-recursive --- test/e2e/settings/enable_real_ip_recursive.go | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 test/e2e/settings/enable_real_ip_recursive.go diff --git a/test/e2e/settings/enable_real_ip_recursive.go b/test/e2e/settings/enable_real_ip_recursive.go new file mode 100644 index 000000000..46f7f65de --- /dev/null +++ b/test/e2e/settings/enable_real_ip_recursive.go @@ -0,0 +1,101 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package settings + +import ( + "fmt" + "net/http" + "strings" + + "github.com/onsi/ginkgo" + "github.com/stretchr/testify/assert" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.DescribeSetting("enable-real-ip-recursive", func() { + f := framework.NewDefaultFramework("enable-real-ip-recursive") + + setting := "enable-real-ip-recursive" + + ginkgo.BeforeEach(func() { + f.NewEchoDeployment() + + f.SetNginxConfigMapData(map[string]string{ + "log-format-escape-json": "true", + "log-format-upstream": "clientip=\"$remote_addr\"", + "use-forwarded-headers": "true", + setting: "false", + }) + }) + + ginkgo.It("should use the first IP in X-Forwarded-For header when setting is true", func() { + host := "forwarded-for-header" + + f.UpdateNginxConfigMapData(setting, "true") + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxConfiguration(func(conf string) bool { + return strings.Contains(conf, "real_ip_recursive on;") + }) + + ginkgo.By("ensuring single values are parsed correctly") + body := f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("X-Forwarded-For", "127.0.0.1, 1.2.3.4"). + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf("x-forwarded-for=127.0.0.1")) + + logs, err := f.NginxLogs() + assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs") + assert.Contains(ginkgo.GinkgoT(), logs, "clientip=\"127.0.0.1\"") + }) + + ginkgo.It("should use the last IP in X-Forwarded-For header when setting is false", func() { + host := "forwarded-for-header" + + f.UpdateNginxConfigMapData(setting, "false") + + f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil)) + + f.WaitForNginxConfiguration(func(conf string) bool { + return strings.Contains(conf, "real_ip_recursive off;") + }) + + body := f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("X-Forwarded-For", "127.0.0.1, 1.2.3.4"). + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf("x-forwarded-for=1.2.3.4")) + + logs, err := f.NginxLogs() + assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs") + assert.Contains(ginkgo.GinkgoT(), logs, "clientip=\"1.2.3.4\"") + }) +}) From 93f0afc60f1b2cbaf400d2198e3bc9794e68405b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pinho?= Date: Wed, 29 Nov 2023 08:26:40 +0000 Subject: [PATCH 4/7] capitalizes IP in EnableRealIPRecursive to match upstream changes --- internal/ingress/controller/config/config.go | 2 +- rootfs/etc/nginx/template/nginx.tmpl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 85bd4bbad..303afb563 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -559,7 +559,7 @@ type Configuration struct { // Sets whether to use recursive search in the real ip module // https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive // Default: true - EnableRealIpRecursive bool `json:"enable-real-ip-recursive"` + EnableRealIPRecursive bool `json:"enable-real-ip-recursive"` // Sets the header field for identifying the originating IP address of a client // Default is X-Forwarded-For diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 4c22cf62c..a393e1267 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -143,7 +143,7 @@ http { real_ip_header {{ $cfg.ForwardedForHeader }}; {{ end }} - {{ if $cfg.EnableRealIpRecursive }} + {{ if $cfg.EnableRealIPRecursive }} real_ip_recursive on; {{ else }} real_ip_recursive off; From fbb2c3bfe74258706aa5b88194b54971ed22f526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pinho?= Date: Wed, 29 Nov 2023 08:45:13 +0000 Subject: [PATCH 5/7] appease the linter by promoting re-used header name to const and removing unnessary Sprintfs --- test/e2e/settings/enable_real_ip_recursive.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/e2e/settings/enable_real_ip_recursive.go b/test/e2e/settings/enable_real_ip_recursive.go index 46f7f65de..d7af96090 100644 --- a/test/e2e/settings/enable_real_ip_recursive.go +++ b/test/e2e/settings/enable_real_ip_recursive.go @@ -17,7 +17,6 @@ limitations under the License. package settings import ( - "fmt" "net/http" "strings" @@ -27,6 +26,8 @@ import ( "k8s.io/ingress-nginx/test/e2e/framework" ) +const forwardedForHost = "forwarded-for-header" + var _ = framework.DescribeSetting("enable-real-ip-recursive", func() { f := framework.NewDefaultFramework("enable-real-ip-recursive") @@ -44,7 +45,7 @@ var _ = framework.DescribeSetting("enable-real-ip-recursive", func() { }) ginkgo.It("should use the first IP in X-Forwarded-For header when setting is true", func() { - host := "forwarded-for-header" + host := forwardedForHost f.UpdateNginxConfigMapData(setting, "true") @@ -65,7 +66,7 @@ var _ = framework.DescribeSetting("enable-real-ip-recursive", func() { Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf("x-forwarded-for=127.0.0.1")) + assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-for=127.0.0.1") logs, err := f.NginxLogs() assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs") @@ -73,7 +74,7 @@ var _ = framework.DescribeSetting("enable-real-ip-recursive", func() { }) ginkgo.It("should use the last IP in X-Forwarded-For header when setting is false", func() { - host := "forwarded-for-header" + host := forwardedForHost f.UpdateNginxConfigMapData(setting, "false") @@ -92,7 +93,7 @@ var _ = framework.DescribeSetting("enable-real-ip-recursive", func() { Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf("x-forwarded-for=1.2.3.4")) + assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-for=1.2.3.4") logs, err := f.NginxLogs() assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs") From 5b21caf09bf22cb8bb41ac6d92eb68fa7a534f75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pinho?= Date: Wed, 28 Feb 2024 11:49:05 +0000 Subject: [PATCH 6/7] nit: fix copyright year --- test/e2e/settings/enable_real_ip_recursive.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/settings/enable_real_ip_recursive.go b/test/e2e/settings/enable_real_ip_recursive.go index d7af96090..feff475e0 100644 --- a/test/e2e/settings/enable_real_ip_recursive.go +++ b/test/e2e/settings/enable_real_ip_recursive.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +Copyright 2024 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From 6d613e725bd247da9774ca52fcd6cc0abd3e6445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pinho?= Date: Wed, 28 Feb 2024 11:51:50 +0000 Subject: [PATCH 7/7] fix: make EnableRealIPRecursive default to false --- internal/ingress/controller/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 303afb563..bfe52ee12 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -795,7 +795,7 @@ func NewDefault() Configuration { ErrorLogLevel: errorLevel, UseForwardedHeaders: false, EnableRealIP: false, - EnableRealIPRecursive: true, + EnableRealIPRecursive: false, ForwardedForHeader: "X-Forwarded-For", ComputeFullForwardedFor: false, ProxyAddOriginalURIHeader: false,