From fe80fe8abf2483acc4865f2a128844d48316e826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Nussbaumer?= Date: Wed, 19 Mar 2025 17:22:37 +0100 Subject: [PATCH 1/5] NGINX: add X-Original-Forwarded-Host header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit adds the equivalent of the existing X-Original-Forwarded-For header but for the X-Forwarded-Host instead. helps in the context of configuration snippet deprecations, as there is currently no safe way to add this header when it was already set upstream Signed-off-by: Clément Nussbaumer --- internal/ingress/controller/config/config.go | 5 +++++ rootfs/etc/nginx/template/nginx.tmpl | 3 ++- test/e2e/settings/forwarded_headers.go | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 2cbc10f6d9..4b076f1d34 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -567,6 +567,10 @@ type Configuration struct { // Default is X-Forwarded-For ForwardedForHeader string `json:"forwarded-for-header,omitempty"` + // Sets the header field for identifying the originating host of a client + // Default is X-Forwarded-Host + ForwardedHostHeader string `json:"forwarded-host-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"` @@ -778,6 +782,7 @@ func NewDefault() Configuration { UseForwardedHeaders: false, EnableRealIP: false, ForwardedForHeader: "X-Forwarded-For", + ForwardedHostHeader: "X-Forwarded-Host", ComputeFullForwardedFor: false, ProxyAddOriginalURIHeader: false, GenerateRequestID: true, diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 6b8e750b06..b73b9cb3f0 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -1279,7 +1279,8 @@ stream { {{ $proxySetHeader }} X-Scheme $pass_access_scheme; # Pass the original X-Forwarded-For - {{ $proxySetHeader }} X-Original-Forwarded-For {{ buildForwardedFor $all.Cfg.ForwardedForHeader }}; + {{ $proxySetHeader }} X-Original-Forwarded-For {{ buildForwardedFor $all.Cfg.ForwardedForHeader }}; + {{ $proxySetHeader }} X-Original-Forwarded-Host {{ buildForwardedFor $all.Cfg.ForwardedHostHeader }}; # mitigate HTTPoxy Vulnerability # https://www.nginx.com/blog/mitigating-the-httpoxy-vulnerability-with-nginx/ diff --git a/test/e2e/settings/forwarded_headers.go b/test/e2e/settings/forwarded_headers.go index 44460aca64..2e647d7dac 100644 --- a/test/e2e/settings/forwarded_headers.go +++ b/test/e2e/settings/forwarded_headers.go @@ -121,6 +121,7 @@ var _ = framework.DescribeSetting("use-forwarded-headers", func() { assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-proto=http") assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-scheme=http") assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-for=1.2.3.4") + assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-host=myhost") assert.NotContains(ginkgo.GinkgoT(), body, "host=myhost") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-proto=myproto") From 6f0114dfc813d16cbe35c8e95f4920e5d1c92b43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Nussbaumer?= Date: Wed, 19 Mar 2025 17:45:31 +0100 Subject: [PATCH 2/5] Test: fix X-Original-Forwarded-Host case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Clément Nussbaumer --- test/e2e/settings/enable_real_ip.go | 5 +++-- test/e2e/settings/forwarded_headers.go | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/test/e2e/settings/enable_real_ip.go b/test/e2e/settings/enable_real_ip.go index bf16e1ea04..efb25940ef 100644 --- a/test/e2e/settings/enable_real_ip.go +++ b/test/e2e/settings/enable_real_ip.go @@ -65,7 +65,7 @@ var _ = framework.DescribeSetting("enable-real-ip", func() { Body(). Raw() - assert.NotContains(ginkgo.GinkgoT(), body, "host=myhost") + assert.NotRegexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-proto=myproto") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-port=1234") @@ -105,7 +105,8 @@ var _ = framework.DescribeSetting("enable-real-ip", func() { assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-port=80") assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-proto=http") assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-for=1.2.3.4") - assert.NotContains(ginkgo.GinkgoT(), body, "host=myhost") + assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-host=myhost") + assert.NotRegexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-proto=myproto") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-port=1234") diff --git a/test/e2e/settings/forwarded_headers.go b/test/e2e/settings/forwarded_headers.go index 2e647d7dac..7706f3c0f2 100644 --- a/test/e2e/settings/forwarded_headers.go +++ b/test/e2e/settings/forwarded_headers.go @@ -66,7 +66,7 @@ var _ = framework.DescribeSetting("use-forwarded-headers", func() { Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, "host=myhost") + assert.Regexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost") assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-proto=myproto") assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-scheme=myproto") @@ -86,7 +86,7 @@ var _ = framework.DescribeSetting("use-forwarded-headers", func() { Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, "host=myhost.com") + assert.Regexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost.com") }) @@ -122,7 +122,7 @@ var _ = framework.DescribeSetting("use-forwarded-headers", func() { assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-scheme=http") assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-for=1.2.3.4") assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-host=myhost") - assert.NotContains(ginkgo.GinkgoT(), body, "host=myhost") + assert.NotRegexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-proto=myproto") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-scheme=myproto") From a8658eba4aaa6e4823759dc4c7bcb68a2f46a4d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Nussbaumer?= Date: Mon, 12 May 2025 08:38:10 +0200 Subject: [PATCH 3/5] Update internal/ingress/controller/config/config.go Co-authored-by: Marco Ebert --- 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 4b076f1d34..1a1c1098a2 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -567,7 +567,7 @@ type Configuration struct { // Default is X-Forwarded-For ForwardedForHeader string `json:"forwarded-for-header,omitempty"` - // Sets the header field for identifying the originating host of a client + // Sets the header field for identifying the originating Host header of a client // Default is X-Forwarded-Host ForwardedHostHeader string `json:"forwarded-host-header,omitempty"` From 0a68f5bbeac4e49e384fd8d598279b4ca7646645 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Nussbaumer?= Date: Mon, 12 May 2025 16:45:13 +0200 Subject: [PATCH 4/5] chore: add buildForwardedHost function used in order to pass the original `X-Forwarded-Host` header further to the client --- .../nginx-configuration/custom-template.md | 1 + .../ingress/controller/template/template.go | 13 +++++++++++++ .../controller/template/template_test.go | 18 ++++++++++++++++++ rootfs/etc/nginx/template/nginx.tmpl | 3 ++- 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/docs/user-guide/nginx-configuration/custom-template.md b/docs/user-guide/nginx-configuration/custom-template.md index 211223025e..d00804c36b 100644 --- a/docs/user-guide/nginx-configuration/custom-template.md +++ b/docs/user-guide/nginx-configuration/custom-template.md @@ -44,6 +44,7 @@ TODO: - buildDenyVariable: - buildUpstreamName: - buildForwardedFor: +- buildForwardedHost: - buildAuthSignURL: - buildNextUpstream: - filterRateLimits: diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index ed052e4ecf..70680ddaca 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -314,6 +314,7 @@ var funcMap = text_template.FuncMap{ }, "isValidByteSize": isValidByteSize, "buildForwardedFor": buildForwardedFor, + "buildForwardedHost": buildForwardedHost, "buildAuthSignURL": buildAuthSignURL, "buildAuthSignURLLocation": buildAuthSignURLLocation, "buildOpentelemetry": buildOpentelemetry, @@ -1153,6 +1154,18 @@ func buildForwardedFor(input interface{}) string { return fmt.Sprintf("$http_%v", ffh) } +func buildForwardedHost(input interface{}) string { + s, ok := input.(string) + if !ok { + klog.Errorf("expected a 'string' type but %T was returned", input) + return "" + } + + fhh := strings.ReplaceAll(s, "-", "_") + fhh = strings.ToLower(fhh) + return fmt.Sprintf("$http_%v", fhh) +} + func buildAuthSignURL(authSignURL, authRedirectParam string) string { u, err := url.Parse(authSignURL) if err != nil { diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index 6553f5daf9..428bdb0bc9 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -857,6 +857,24 @@ func TestBuildForwardedFor(t *testing.T) { } } +func TestBuildForwardedHost(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildForwardedHost(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + inputStr := "X-Forwarded-Host" + expected = "$http_x_forwarded_host" + actual = buildForwardedHost(inputStr) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } +} + func TestBuildResolvers(t *testing.T) { ipOne := net.ParseIP("192.0.0.1") ipTwo := net.ParseIP("2001:db8:1234:0000:0000:0000:0000:0000") diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index b73b9cb3f0..536f0aa005 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -1280,7 +1280,8 @@ stream { # Pass the original X-Forwarded-For {{ $proxySetHeader }} X-Original-Forwarded-For {{ buildForwardedFor $all.Cfg.ForwardedForHeader }}; - {{ $proxySetHeader }} X-Original-Forwarded-Host {{ buildForwardedFor $all.Cfg.ForwardedHostHeader }}; + # Pass the original X-Forwarded-Host + {{ $proxySetHeader }} X-Original-Forwarded-Host {{ buildForwardedHost $all.Cfg.ForwardedHostHeader }}; # mitigate HTTPoxy Vulnerability # https://www.nginx.com/blog/mitigating-the-httpoxy-vulnerability-with-nginx/ From 070fdfc80ee9bdb039eae5f28c3f4edd6a2a42a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Nussbaumer?= Date: Mon, 12 May 2025 17:07:06 +0200 Subject: [PATCH 5/5] test: add comment on regexp necessity for details, see #12999 and https://go.dev/play/p/cgkKxJZtCCp --- test/e2e/settings/enable_real_ip.go | 2 ++ test/e2e/settings/forwarded_headers.go | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/e2e/settings/enable_real_ip.go b/test/e2e/settings/enable_real_ip.go index efb25940ef..c56a8aefa1 100644 --- a/test/e2e/settings/enable_real_ip.go +++ b/test/e2e/settings/enable_real_ip.go @@ -65,6 +65,7 @@ var _ = framework.DescribeSetting("enable-real-ip", func() { Body(). Raw() + // we use a regexp to prevent matching the expression in the middle of the x-original-forwarded-host header assert.NotRegexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-proto=myproto") @@ -106,6 +107,7 @@ var _ = framework.DescribeSetting("enable-real-ip", func() { assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-proto=http") assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-for=1.2.3.4") assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-host=myhost") + // we use a regexp to prevent matching the expression in the middle of the x-original-forwarded-host header assert.NotRegexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-proto=myproto") diff --git a/test/e2e/settings/forwarded_headers.go b/test/e2e/settings/forwarded_headers.go index 7706f3c0f2..d0c29ebaa1 100644 --- a/test/e2e/settings/forwarded_headers.go +++ b/test/e2e/settings/forwarded_headers.go @@ -66,6 +66,7 @@ var _ = framework.DescribeSetting("use-forwarded-headers", func() { Body(). Raw() + // we use a regexp to prevent matching the expression in the middle of the x-original-forwarded-host header assert.Regexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost") assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-proto=myproto") @@ -86,7 +87,8 @@ var _ = framework.DescribeSetting("use-forwarded-headers", func() { Body(). Raw() - assert.Regexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) + // we use a regexp to prevent matching the expression in the middle of the x-original-forwarded-host header + assert.Regexp(ginkgo.GinkgoT(), `(\s)host=myhost.com`, body) assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost.com") }) @@ -122,6 +124,7 @@ var _ = framework.DescribeSetting("use-forwarded-headers", func() { assert.Contains(ginkgo.GinkgoT(), body, "x-forwarded-scheme=http") assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-for=1.2.3.4") assert.Contains(ginkgo.GinkgoT(), body, "x-original-forwarded-host=myhost") + // we use a regexp to prevent matching the expression in the middle of the x-original-forwarded-host header assert.NotRegexp(ginkgo.GinkgoT(), `(\s)host=myhost`, body) assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-host=myhost") assert.NotContains(ginkgo.GinkgoT(), body, "x-forwarded-proto=myproto")