From d77c692f3de1ff953fa7b557c09e42b9298c7bc1 Mon Sep 17 00:00:00 2001 From: zitudu Date: Wed, 16 Aug 2023 14:45:17 +0800 Subject: [PATCH 1/7] Add standard Forwarded header support --- .../nginx-configuration/configmap.md | 25 +++ internal/ingress/controller/config/config.go | 25 +++ .../ingress/controller/template/configmap.go | 50 +++++ .../controller/template/configmap_test.go | 86 ++++++++ .../ingress/controller/template/template.go | 14 ++ .../controller/template/template_test.go | 21 ++ rootfs/etc/nginx/template/nginx.tmpl | 139 +++++++++++++ test/e2e/settings/forwarded_rfc7239.go | 192 ++++++++++++++++++ 8 files changed, 552 insertions(+) create mode 100644 test/e2e/settings/forwarded_rfc7239.go diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 0a7e44dce..56132cdd9 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -130,6 +130,11 @@ The following table shows a configuration option's name, type, and the default v |[enable-real-ip](#enable-real-ip)|bool|"false"|| |[forwarded-for-header](#forwarded-for-header)|string|"X-Forwarded-For"|| |[compute-full-forwarded-for](#compute-full-forwarded-for)|bool|"false"|| +|[enable-forwarded-rfc7239](#enable-forwarded-rfc7239)|bool|"false"|| +|[forwarded-rfc7239-strip-incomming](#forwarded-rfc7239-strip-incomming)|bool|"false"|| +|[forwarded-rfc7239](#forwarded-rfc7239)|[]string|"for"|| +|[forwarded-rfc7239-for](#forwarded-rfc7239-for)|string|"ip"|| +|[forwarded-rfc7239-by](#forwarded-rfc7239-by)|string|"ip"|| |[proxy-add-original-uri-header](#proxy-add-original-uri-header)|bool|"false"|| |[generate-request-id](#generate-request-id)|bool|"true"|| |[enable-opentracing](#enable-opentracing)|bool|"false"|| @@ -922,6 +927,26 @@ Sets the header field for identifying the originating IP address of a client. _* Append the remote address to the X-Forwarded-For header instead of replacing it. When this option is enabled, the upstream application is responsible for extracting the client IP based on its own list of trusted proxies. +## enable-forwarded-rfc7239 + +Enable standard Forwarded header defined in RFC 7239, or the Forwared header will not be sent to upstream and the incoming Forwarded header will be discarded. The parameters can be configured using [forwarded-rfc7239](#forwarded-rfc7239). Transition between Forwarded header and X-Forwarded headers will not happen. _**default:** false + +## forwarded-rfc7239-strip-incomming + +Whether or not retain Forwarded header from downstream. _**default:**_ false + +## forwarded-rfc7239 + +Sets enabled parameters and their order. Supported parameters are "for", "by", "host" and, "proto". _**default:**_ for + +## forwarded-rfc7239-for + +Sets value of "for" parameter. It can be "ip" or static obfuscated string. If "ip" is given, the remote client address will be used. The static obfuscated string is selected by user and must start with a underscore "_" and consist of only digits, letters and characters ".", "_", and "-". _**default:**_ ip + +## forwarded-rfc7239-by + +Sets value of "by" parameter. It can be "ip" or static obfuscated string. If "ip" is given, the server's host-port pair that remote client connects to will be used. The static obfuscated string is selected by user and must start with a underscore "_" and consist of only digits, letters and characters ".", "_", and "-". _**default:**_ ip + ## proxy-add-original-uri-header Adds an X-Original-Uri header with the original request URI to the backend request diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 6e78964ed..222279845 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -558,6 +558,26 @@ type Configuration struct { // Default: false ComputeFullForwardedFor bool `json:"compute-full-forwarded-for,omitempty"` + // Enable standard forwarded header. + // Default: false + EnableForwardedRFC7239 bool `json:"enable-forwarded-rfc7239"` + + // Sets if strips incoming Forwarded header. + // Default: false + ForwardedRFC7239StripIncomming bool `json:"forwarded-rfc7239-strip-incomming"` + + // Sets Forwarded parameters and their order. Available options are "for", "by", "host", "proto". + // Default: "for" + ForwardedRFC7239 []string `json:"forwarded-rfc7239"` + + // Sets Forwarded "for" parameter node identifier, should be "ip" or a static string. + // Default: "ip" + ForwardedRFC7239For string `json:"forwarded-rfc7239-for,omitempty"` + + // Sets Forwarded "by" parameter node identifier, should be "ip" or a static string. + // Default: "ip" + ForwardedRFC7239By string `json:"forwarded-rfc7239-by,omitempty"` + // If the request does not have a request-id, should we generate a random value? // Default: true GenerateRequestID bool `json:"generate-request-id,omitempty"` @@ -889,6 +909,11 @@ func NewDefault() Configuration { EnableRealIp: false, ForwardedForHeader: "X-Forwarded-For", ComputeFullForwardedFor: false, + EnableForwardedRFC7239: false, + ForwardedRFC7239StripIncomming: false, + ForwardedRFC7239: []string{"for"}, + ForwardedRFC7239For: "ip", + ForwardedRFC7239By: "ip", ProxyAddOriginalURIHeader: false, GenerateRequestID: true, HTTP2MaxFieldSize: "", diff --git a/internal/ingress/controller/template/configmap.go b/internal/ingress/controller/template/configmap.go index c73f3b6c0..505b21997 100644 --- a/internal/ingress/controller/template/configmap.go +++ b/internal/ingress/controller/template/configmap.go @@ -53,6 +53,9 @@ const ( nginxStatusIpv4Whitelist = "nginx-status-ipv4-whitelist" nginxStatusIpv6Whitelist = "nginx-status-ipv6-whitelist" proxyHeaderTimeout = "proxy-protocol-header-timeout" + forwardedRFC7239 = "forwarded-rfc7239" + forwardedRFC7239For = "forwarded-rfc7239-for" + forwardedRFC7239By = "forwarded-rfc7239-by" workerProcesses = "worker-processes" globalAuthURL = "global-auth-url" globalAuthMethod = "global-auth-method" @@ -83,6 +86,7 @@ var ( "global_throttle_cache": 10240, } defaultGlobalAuthRedirectParam = "rd" + forwardedRFC7239ValidParams = [...]string{"for", "by", "host", "proto"} ) const ( @@ -346,6 +350,36 @@ func ReadConfig(src map[string]string) config.Configuration { } } + if val, ok := conf[forwardedRFC7239]; ok { + delete(conf, forwardedRFC7239) + params := []string{} // non-nil value + for _, param := range splitAndTrimSpace(val, ",") { + param = strings.ToLower(param) + if sliceContains(forwardedRFC7239ValidParams[:], param) { + if sliceContains(params, param) { + klog.Warningf("%v ignores duplicate parameter: %v", forwardedRFC7239, param) + continue + } + params = append(params, param) + } + } + to.ForwardedRFC7239 = params + } + + if val, ok := conf[forwardedRFC7239For]; ok { + if !isValidObfuscatedIdentifier(val) { + delete(conf, forwardedRFC7239For) + klog.Warningf("%v is not a valid obfuscated identifier", val) + } + } + + if val, ok := conf[forwardedRFC7239By]; ok { + if !isValidObfuscatedIdentifier(val) { + delete(conf, forwardedRFC7239By) + klog.Warningf("%v is not a valid obfuscated identifier", val) + } + } + streamResponses := 1 if val, ok := conf[proxyStreamResponses]; ok { delete(conf, proxyStreamResponses) @@ -487,3 +521,19 @@ func dictKbToStr(size int) string { } return fmt.Sprintf("%dK", size) } + +func sliceContains(slice []string, s string) bool { + for _, value := range slice { + if value == s { + return true + } + } + return false +} + +// isValidObfuscatedIdentifier tests if s is a valid obfuscated identifier +// as section 6.3 of rfc7239 defines. +func isValidObfuscatedIdentifier(s string) bool { + re := regexp.MustCompile("^_[0-9a-zA-Z._-]*$") + return re.MatchString(s) +} diff --git a/internal/ingress/controller/template/configmap_test.go b/internal/ingress/controller/template/configmap_test.go index dad841694..212ccf5e2 100644 --- a/internal/ingress/controller/template/configmap_test.go +++ b/internal/ingress/controller/template/configmap_test.go @@ -364,6 +364,38 @@ func TestGlobalExternalAuthCacheDurationParsing(t *testing.T) { } } +func TestForwardedRFC7239Parsing(t *testing.T) { + testCases := map[string]struct { + params string + expect []string + }{ + "nothing": {"", []string{}}, + "spaces": {" ", []string{}}, + "one param": {"for", []string{"for"}}, + "two params": {"by,for", []string{"by", "for"}}, + "case insensitive": {"for,HOST,By", []string{"for", "host", "by"}}, + "duplicate params": {"for,host,for,by", []string{"for", "host", "by"}}, + } + + { + name := "absent field" + expect := []string{"for"} // default + cfg := ReadConfig(map[string]string{}) + + if !reflect.DeepEqual(cfg.ForwardedRFC7239, expect) { + t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", name, expect, cfg.ForwardedRFC7239) + } + } + + for name, tc := range testCases { + cfg := ReadConfig(map[string]string{"forwarded-rfc7239": tc.params}) + + if !reflect.DeepEqual(cfg.ForwardedRFC7239, tc.expect) { + t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", name, tc.expect, cfg.ForwardedRFC7239) + } + } +} + func TestLuaSharedDictsParsing(t *testing.T) { testsCases := []struct { name string @@ -530,3 +562,57 @@ func TestDictKbToStr(t *testing.T) { } } } + +func TestSliceContains(t *testing.T) { + testCases := map[string]struct { + slice []string + find string + expect bool + }{ + "containing": { + slice: []string{"ip", "_obf"}, + find: "ip", + expect: true, + }, + "not containing": { + slice: []string{"ip", "_obf"}, + find: "obfuscated", + expect: false, + }, + } + + for name, tc := range testCases { + if b := sliceContains(tc.slice, tc.find); b != tc.expect { + t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", name, tc.expect, b) + } + } +} + +func TestIsValidObfuscatedIdentifier(t *testing.T) { + testCases := map[string]struct { + input string + expect bool + }{ + "rfc example _hidden": { + input: "_hidden", + expect: true, + }, + "rfc example _SEVKISEK": { + input: "_SEVKISEK", + expect: true, + }, + "non-leading underscore": { + input: "hidden", + expect: false, + }, + "containing invalid characters": { + input: "_hidden:", + expect: false, + }, + } + for name, tc := range testCases { + if isValid := isValidObfuscatedIdentifier(tc.input); isValid != tc.expect { + t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", name, tc.input, isValid) + } + } +} diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 147455771..29340a0cb 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -281,9 +281,14 @@ var ( "shouldLoadAuthDigestModule": shouldLoadAuthDigestModule, "buildServerName": buildServerName, "buildCorsOriginRegex": buildCorsOriginRegex, + "fieldValueComponet": fieldValueComponet, } ) +var ( + headerFieldValueTokenChars = regexp.MustCompile("^[!#$%&'*+.^_`|~0-9A-Za-z-]+$") +) + // escapeLiteralDollar will replace the $ character with ${literal_dollar} // which is made to work via the following configuration in the http section of // the template: @@ -1854,3 +1859,12 @@ func buildCorsOriginRegex(corsOrigins []string) string { originsRegex = originsRegex + ")$ ) { set $cors 'true'; }" return originsRegex } + +// Token containing character other than tchar must be quoted using +// double-quote marks (section 3.2.6 of RFC 7230). +func fieldValueComponet(s string) string { + if headerFieldValueTokenChars.MatchString(s) { + return s + } + return fmt.Sprintf("%q", s) +} diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index a2c3b8299..c8c6cb11d 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -2237,3 +2237,24 @@ func TestCleanConf(t *testing.T) { t.Errorf("cleanConf result don't match with expected: %s", diff) } } + +func TestFieldValueComponet(t *testing.T) { + testCases := map[string]struct { + input string + expect string + }{ + "empty": {"", `""`}, + "spaces": {" ", `" "`}, + "ipv4": {"1.2.3.4", `1.2.3.4`}, + "ipv6": {"[::1]", `"[::1]"`}, + "obfuscated identifier": {"_hidden", `_hidden`}, + "ipv4 port": {"1.2.3.4:80", `"1.2.3.4:80"`}, + "domain": {"example.com", `example.com`}, + "http schema": {"http", `http`}, + } + for name, tc := range testCases { + if comp := fieldValueComponet(tc.input); comp != tc.expect { + t.Errorf("%v: expected '%v' but returned '%v'", name, tc.expect, comp) + } + } +} diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 0a031442c..7c09d5123 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -453,6 +453,137 @@ http { {{ end }} + {{ if $cfg.EnableForwardedRFC7239 }} + # Enabled standard forwarded (RFC 7239). + + # Used to declare constant variables. + map $nginx_version $nginx_constant { + default ''; + } + + {{ if eq "ip" $cfg.ForwardedRFC7239For }} + # Set proxy_forwarded_rfc2379_for as IP of remote end. + # We use $remote_addr instead of $realip_remote_addr because we don't + # translate "x-" header to standard forwarded header. + map $remote_addr $proxy_forwarded_rfc2379_for { + # IPv4 addresses can be sent as-is + ~^[0-9.]+$ "for=$remote_addr"; + + # IPv6 addresses need to be bracketed and quoted + ~^[0-9A-Fa-f:.]+$ "for=\"[$remote_addr]\""; + + # Unix domain socket names cannot be represented in RFC 7239 syntax + default "for=unknown"; + } + {{ else }} + # Set proxy_forwarded_rfc2379_for as a constant string. + map $nginx_constant $proxy_forwarded_rfc2379_for { + default {{ $cfg.ForwardedRFC7239For | fieldValueComponet | quote }}; + } + {{ end }} + + {{ if eq "ip" $cfg.ForwardedRFC7239By }} + # Set proxy_forwarded_rfc2379_by as "IP:port" that the remote end connects to. + map $server_addr $proxy_forwarded_rfc2379_by { + # IPv4:port need to be quoted since ":" is not a valid + # header field value character. + ~^[0-9.]+$ "by=\"$server_addr:$server_port\""; + + # IPv6 addresses need to be bracketed and quoted with port joined by ":" + ~^[0-9A-Fa-f:.]+$ "by=\"[$server_addr]:$server_port\""; + + # Unix domain socket names cannot be represented in RFC 7239 syntax + default "by=unknown"; + } + {{ else }} + # Set proxy_forwarded_rfc2379_by as a constant string. + map $nginx_constant $proxy_forwarded_rfc2379_by { + default {{ $cfg.ForwardedRFC7239By | fieldValueComponet | quote }}; + } + {{ end }} + + # Set proxy_forwarded_rfc2379_host as the original HOST header and quote + # it if needed. Section 3.2.2 of RFC 3986 defines host syntax, and it must + # be one of ipv4, ipv6, and reg-name optionaly followed by a port after + # a colon. + map $http_host $proxy_forwarded_rfc2379_host { + # IPv4 and reg-name formed of valid field value characters can be sent + # as-is. The regular expression of IPv4 is subset of this one, and this + # expression also matches IPv4. + # Since characters "#", "^", "|", and "`" are invalid for reg-name, + # these four characters are removed from character set. + "~^[!$%&'*+._~0-9A-Za-z-]+$" "host=$http_host"; + + # reg-names are not matched last regular expression need to be quoted. + "~^[!$&'()*+,;=._~%0-9A-Za-z-]$" "host=\"$http_host\""; + + # IPv6 addresses (with port or not) need to quoted (it must be already + # bracketed). + "~^\[[^]]+\](:[0-9]+)*$" "host=\"$http_host\""; + + # IPv4:port and reg-name:port and need to be quoted. + "~^[!$&'()*+,;=._~%0-9A-Za-z-]+:[0-9]+$" "host=\"$http_host\""; + + # Otherwise it is invalid. + default "host=unknown"; + } + + # Section 3.1 of RFC3986 defines scheme syntax. + map $scheme $proxy_forwarded_rfc2379_proto { + ~^[a-zA-Z][0-9a-zA-Z.+-]+$ "proto=$scheme"; + default "proto=unknown"; + } + + # Add incoming Forwarded header if valid and not strip. + # Ref: https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/#how-to-use-it-in-nginx + map $http_forwarded $proxy_forwarded_rfc2379_incoming { + {{ if not $cfg.ForwardedRFC7239StripIncomming }} + # If the incoming Forwarded header is syntactically valid, use it. + "~^(,[ \\t]*)*([!#$%&'*+.^_`|~0-9A-Za-z-]+=([!#$%&'*+.^_`|~0-9A-Za-z-]+|\"([\\t \\x21\\x23-\\x5B\\x5D-\\x7E\\x80-\\xFF]|\\\\[\\t \\x21-\\x7E\\x80-\\xFF])*\"))?(;([!#$%&'*+.^_`|~0-9A-Za-z-]+=([!#$%&'*+.^_`|~0-9A-Za-z-]+|\"([\\t \\x21\\x23-\\x5B\\x5D-\\x7E\\x80-\\xFF]|\\\\[\\t \\x21-\\x7E\\x80-\\xFF])*\"))?)*([ \\t]*,([ \\t]*([!#$%&'*+.^_`|~0-9A-Za-z-]+=([!#$%&'*+.^_`|~0-9A-Za-z-]+|\"([\\t \\x21\\x23-\\x5B\\x5D-\\x7E\\x80-\\xFF]|\\\\[\\t \\x21-\\x7E\\x80-\\xFF])*\"))?(;([!#$%&'*+.^_`|~0-9A-Za-z-]+=([!#$%&'*+.^_`|~0-9A-Za-z-]+|\"([\\t \\x21\\x23-\\x5B\\x5D-\\x7E\\x80-\\xFF]|\\\\[\\t \\x21-\\x7E\\x80-\\xFF])*\"))?)*)?)*$" "$http_forwarded"; + + # Otherwise, discard it. + default ''; + {{ else }} + # Strip incomming forwarded header. + default ''; + {{ end }} + } + + # Add a comma separator when both the incoming part and the current node's part exist. + map $proxy_forwarded_rfc2379_incoming $proxy_forwarded_rfc2379_sep { + {{ if not $cfg.ForwardedRFC7239 }} + # Pass through the incoming forwarded header. + default ''; + {{ else }} + # Incoming forwarded header is empty. + '' ''; + # Both of two parts exist, so insert a comma. + default ', '; + {{ end }} + } + + # Append Forwarded field to the existing Forwarded header after a comma separator, + # or generate a new one if the incoming Forwarded header is invalid or empty. + map $proxy_forwarded_rfc2379_incoming $proxy_add_forwarded_rfc2379 { + default "$proxy_forwarded_rfc2379_incoming$proxy_forwarded_rfc2379_sep + {{- /* NOTE: We need to trip both leading and trailing spaces to ensure the format correct. */ -}} + {{- range $index, $param := $cfg.ForwardedRFC7239 -}} + {{- if gt $index 0 -}} ; {{- end -}} + {{- if eq "for" $param -}} + $proxy_forwarded_rfc2379_for + {{- else if eq "by" $param -}} + $proxy_forwarded_rfc2379_by + {{- else if eq "host" $param -}} + $proxy_forwarded_rfc2379_host + {{- else if eq "proto" $param -}} + $proxy_forwarded_rfc2379_proto + {{- end -}} + {{- end -}} + "; + } + + {{ end }}{{- /* if $cfg.EnableForwardedRFC7239 */ -}} + # Create a variable that contains the literal $ character. # This works because the geo module will not resolve variables. geo $literal_dollar { @@ -1153,6 +1284,10 @@ stream { proxy_set_header X-Forwarded-For $remote_addr; {{ end }} + {{ if $all.Cfg.EnableForwardedRFC7239 }} + proxy_set_header Forwarded $proxy_add_forwarded_rfc2379; + {{ end }} + {{ if $externalAuth.RequestRedirect }} proxy_set_header X-Auth-Request-Redirect {{ $externalAuth.RequestRedirect }}; {{ else }} @@ -1439,6 +1574,10 @@ stream { # Pass the original X-Forwarded-For {{ $proxySetHeader }} X-Original-Forwarded-For {{ buildForwardedFor $all.Cfg.ForwardedForHeader }}; + {{ if $all.Cfg.EnableForwardedRFC7239 }} + {{ $proxySetHeader }} Forwarded $proxy_add_forwarded_rfc2379; + {{ end }} + # mitigate HTTPoxy Vulnerability # https://www.nginx.com/blog/mitigating-the-httpoxy-vulnerability-with-nginx/ {{ $proxySetHeader }} Proxy ""; diff --git a/test/e2e/settings/forwarded_rfc7239.go b/test/e2e/settings/forwarded_rfc7239.go new file mode 100644 index 000000000..68b744599 --- /dev/null +++ b/test/e2e/settings/forwarded_rfc7239.go @@ -0,0 +1,192 @@ +/* +Copyright 2023 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 ( + "net/http" + "strings" + + "github.com/onsi/ginkgo/v2" + "github.com/stretchr/testify/assert" + "k8s.io/ingress-nginx/test/e2e/framework" +) + +const ( + enableForwardedRFC7239 = "enable-forwarded-rfc7239" + forwardedRFC7239StripIncomming = "forwarded-rfc7239-strip-incomming" + forwardedRFC7239 = "forwarded-rfc7239" + forwardedRFC7239By = "forwarded-rfc7239-by" + forwardedRFC7239For = "forwarded-rfc7239-for" +) + +var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { + f := framework.NewDefaultFramework("enable-forwarded-rfc7239") + + ginkgo.BeforeEach(func() { + f.NewEchoDeployment() + }) + + ginkgo.AfterEach(func() { + }) + + ginkgo.It("should not send forwarded header when disabled", func() { + host := "forwarded-rfc7239" + + f.UpdateNginxConfigMapData(enableForwardedRFC7239, "false") + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name forwarded-rfc7239") && + !strings.Contains(server, "proxy_set_header Forwarded $proxy_add_forwarded_rfc2379;") + }) + + body := f.HTTPTestClient(). + GET("/"). + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.NotContains(ginkgo.GinkgoT(), body, "forwarded=1.2.3.4") + }) + + ginkgo.It("should trust Forwarded header when striping-incoming is false", func() { + host := "forwarded-rfc7239" + + f.UpdateNginxConfigMapData(enableForwardedRFC7239, "true") + f.UpdateNginxConfigMapData(forwardedRFC7239StripIncomming, "false") + f.UpdateNginxConfigMapData(forwardedRFC7239For, "ip") + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name forwarded-rfc7239") && + strings.Contains(server, "proxy_set_header Forwarded $proxy_add_forwarded_rfc2379;") + }) + + ginkgo.By("ensuring valid headers are passed through correctly") + body := f.HTTPTestClient(). + GET("/"). + WithHeader("Forwarded", "for=1.1.1.1;secret=_5ecREy"). + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.Contains(ginkgo.GinkgoT(), body, "for=1.1.1.1;secret=_5e(REy, for=1.2.3.4") + + ginkgo.By("ensuring invalid headers are striped") + body = f.HTTPTestClient(). + GET("/"). + WithHeader("Forwarded", "for=1.1.1.1;secret=:x:"). // colon should be quoted + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.Contains(ginkgo.GinkgoT(), body, "for=1.2.3.4") + }) + + ginkgo.It("should contain parameters in order as setting forwarded-rfc7239 specified", func() { + host := "forwarded-rfc7239" + + f.UpdateNginxConfigMapData(enableForwardedRFC7239, "true") + f.UpdateNginxConfigMapData(forwardedRFC7239StripIncomming, "false") + f.UpdateNginxConfigMapData(forwardedRFC7239For, "ip") + f.UpdateNginxConfigMapData(forwardedRFC7239By, "ip") + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name forwarded-rfc7239") && + strings.Contains(server, "proxy_set_header Forwarded $proxy_add_forwarded_rfc2379;") + }) + + ginkgo.By("ensuring singly pass through incoming header when empty parameter list") + f.UpdateNginxConfigMapData(forwardedRFC7239, "") + body := f.HTTPTestClient(). + GET("/"). + WithHeader("Forwarded", "for=1.1.1.1"). + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.Contains(ginkgo.GinkgoT(), body, "for=1.1.1.1") + + ginkgo.By("ensuring any parameter combinations work") + f.UpdateNginxConfigMapData(forwardedRFC7239, "for,by,proto,host") + body = f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.Contains(ginkgo.GinkgoT(), body, `for=1.2.3.4;by="1.2.3.4:80";proto=http;host=forwarded-rfc7239`) + }) + + ginkgo.It("should config for and by parameters as constants", func() { + host := "forwarded-rfc7239" + + f.UpdateNginxConfigMapData(enableForwardedRFC7239, "true") + f.UpdateNginxConfigMapData(forwardedRFC7239, "for,by") + f.UpdateNginxConfigMapData(forwardedRFC7239For, "ip") + f.UpdateNginxConfigMapData(forwardedRFC7239By, "host2") + + f.UpdateNginxConfigMapData(forwardedRFC7239, "") + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name forwarded-rfc7239") && + strings.Contains(server, "proxy_set_header Forwarded $proxy_add_forwarded_rfc2379;") + }) + + ginkgo.By("ensuring by parameter is specified value") + body := f.HTTPTestClient(). + GET("/"). + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.Contains(ginkgo.GinkgoT(), body, "for=1.2.3.4;by=host2") + + ginkgo.By("ensuring for parameter is specified value") + f.UpdateNginxConfigMapData(forwardedRFC7239For, "a@cY") + f.UpdateNginxConfigMapData(forwardedRFC7239By, "ip") + body = f.HTTPTestClient(). + GET("/"). + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.Contains(ginkgo.GinkgoT(), body, `for="a@cY";by="1.2.3.4:80"`) + }) +}) From 485ad5a80d5e79ccb7e351207f6d1343b2489fe0 Mon Sep 17 00:00:00 2001 From: zitudu Date: Thu, 17 Aug 2023 11:57:56 +0800 Subject: [PATCH 2/7] Fix doc typo --- docs/user-guide/nginx-configuration/configmap.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 56132cdd9..485528422 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -929,7 +929,7 @@ Append the remote address to the X-Forwarded-For header instead of replacing it. ## enable-forwarded-rfc7239 -Enable standard Forwarded header defined in RFC 7239, or the Forwared header will not be sent to upstream and the incoming Forwarded header will be discarded. The parameters can be configured using [forwarded-rfc7239](#forwarded-rfc7239). Transition between Forwarded header and X-Forwarded headers will not happen. _**default:** false +Enable standard Forwarded header defined in RFC 7239, or the Forwared header will not be sent to upstream and the incoming Forwarded header will be discarded. The parameters can be configured using [forwarded-rfc7239](#forwarded-rfc7239). Transition between Forwarded header and X-Forwarded headers will not happen. _**default:**_ false ## forwarded-rfc7239-strip-incomming From 32e2111745891b0023e7c11735088d82c485a078 Mon Sep 17 00:00:00 2001 From: zitudu Date: Thu, 17 Aug 2023 11:58:47 +0800 Subject: [PATCH 3/7] Fix Forwarded host regular expression --- rootfs/etc/nginx/template/nginx.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 7c09d5123..367e6d031 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -519,7 +519,7 @@ http { # IPv6 addresses (with port or not) need to quoted (it must be already # bracketed). - "~^\[[^]]+\](:[0-9]+)*$" "host=\"$http_host\""; + "~^\[[0-9A-Fa-f:.]+\](:[0-9]+)*$" "host=\"$http_host\""; # IPv4:port and reg-name:port and need to be quoted. "~^[!$&'()*+,;=._~%0-9A-Za-z-]+:[0-9]+$" "host=\"$http_host\""; From 8ae1072d39df95db4f1e356babf6a6c374cb4c1c Mon Sep 17 00:00:00 2001 From: zitudu Date: Thu, 17 Aug 2023 12:01:22 +0800 Subject: [PATCH 4/7] Add e2e test invalid static obfuscated values are ingored --- test/e2e/settings/forwarded_rfc7239.go | 61 +++++++++++++++++--------- 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/test/e2e/settings/forwarded_rfc7239.go b/test/e2e/settings/forwarded_rfc7239.go index 68b744599..d514e68c1 100644 --- a/test/e2e/settings/forwarded_rfc7239.go +++ b/test/e2e/settings/forwarded_rfc7239.go @@ -70,9 +70,12 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { ginkgo.It("should trust Forwarded header when striping-incoming is false", func() { host := "forwarded-rfc7239" - f.UpdateNginxConfigMapData(enableForwardedRFC7239, "true") - f.UpdateNginxConfigMapData(forwardedRFC7239StripIncomming, "false") - f.UpdateNginxConfigMapData(forwardedRFC7239For, "ip") + config := map[string]string{} + config[enableForwardedRFC7239] = "true" + config[forwardedRFC7239StripIncomming] = "false" + config[forwardedRFC7239] = "for" + config[forwardedRFC7239For] = "ip" + f.SetNginxConfigMapData(config) ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil) f.EnsureIngress(ing) @@ -109,10 +112,13 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { ginkgo.It("should contain parameters in order as setting forwarded-rfc7239 specified", func() { host := "forwarded-rfc7239" - f.UpdateNginxConfigMapData(enableForwardedRFC7239, "true") - f.UpdateNginxConfigMapData(forwardedRFC7239StripIncomming, "false") - f.UpdateNginxConfigMapData(forwardedRFC7239For, "ip") - f.UpdateNginxConfigMapData(forwardedRFC7239By, "ip") + config := map[string]string{} + config[enableForwardedRFC7239] = "true" + config[forwardedRFC7239StripIncomming] = "false" + config[forwardedRFC7239] = "for" + config[forwardedRFC7239For] = "ip" + config[forwardedRFC7239By] = "ip" + f.SetNginxConfigMapData(config) ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil) f.EnsureIngress(ing) @@ -148,15 +154,16 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { assert.Contains(ginkgo.GinkgoT(), body, `for=1.2.3.4;by="1.2.3.4:80";proto=http;host=forwarded-rfc7239`) }) - ginkgo.It("should config for and by parameters as constants", func() { + ginkgo.It("should config for and by parameters as static obfuscated values", func() { host := "forwarded-rfc7239" - f.UpdateNginxConfigMapData(enableForwardedRFC7239, "true") - f.UpdateNginxConfigMapData(forwardedRFC7239, "for,by") - f.UpdateNginxConfigMapData(forwardedRFC7239For, "ip") - f.UpdateNginxConfigMapData(forwardedRFC7239By, "host2") - - f.UpdateNginxConfigMapData(forwardedRFC7239, "") + config := map[string]string{} + config[enableForwardedRFC7239] = "true" + config[forwardedRFC7239StripIncomming] = "false" + config[forwardedRFC7239] = "for,by" + config[forwardedRFC7239For] = "ip" + config[forwardedRFC7239By] = "_SERVER1" + f.SetNginxConfigMapData(config) ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil) f.EnsureIngress(ing) @@ -167,7 +174,7 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { strings.Contains(server, "proxy_set_header Forwarded $proxy_add_forwarded_rfc2379;") }) - ginkgo.By("ensuring by parameter is specified value") + ginkgo.By("ensuring \"by\" parameter is a static obfuscated value") body := f.HTTPTestClient(). GET("/"). Expect(). @@ -175,11 +182,12 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, "for=1.2.3.4;by=host2") + assert.Contains(ginkgo.GinkgoT(), body, "for=1.2.3.4;by=_SERVER1") - ginkgo.By("ensuring for parameter is specified value") - f.UpdateNginxConfigMapData(forwardedRFC7239For, "a@cY") - f.UpdateNginxConfigMapData(forwardedRFC7239By, "ip") + ginkgo.By("ensuring \"for\" parameter is a static obfuscated value") + config[forwardedRFC7239For] = "_HOST1" + config[forwardedRFC7239By] = "ip" + f.SetNginxConfigMapData(config) body = f.HTTPTestClient(). GET("/"). Expect(). @@ -187,6 +195,19 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, `for="a@cY";by="1.2.3.4:80"`) + assert.Contains(ginkgo.GinkgoT(), body, `for="_HOST1";by="1.2.3.4:80"`) + + ginkgo.By("ensuring invalid static obfuscated values are ingored") + config[forwardedRFC7239For] = "_HOST1" + config[forwardedRFC7239By] = "_%" + f.SetNginxConfigMapData(config) + body = f.HTTPTestClient(). + GET("/"). + Expect(). + Status(http.StatusOK). + Body(). + Raw() + + assert.Contains(ginkgo.GinkgoT(), body, `for=1.2.3.4;by="1.2.3.4:80"`) }) }) From b6f83d55b89d86b1d16a7c5a7a8005b5e9cde7a8 Mon Sep 17 00:00:00 2001 From: zitudu Date: Thu, 17 Aug 2023 15:23:40 +0800 Subject: [PATCH 5/7] Fix e2e tests --- rootfs/etc/nginx/template/nginx.tmpl | 4 +- test/e2e/settings/forwarded_rfc7239.go | 58 +++++++++++++++++++------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 367e6d031..df2746ad0 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -478,7 +478,7 @@ http { {{ else }} # Set proxy_forwarded_rfc2379_for as a constant string. map $nginx_constant $proxy_forwarded_rfc2379_for { - default {{ $cfg.ForwardedRFC7239For | fieldValueComponet | quote }}; + default {{ $cfg.ForwardedRFC7239For | fieldValueComponet | printf "for=%s" | quote }}; } {{ end }} @@ -498,7 +498,7 @@ http { {{ else }} # Set proxy_forwarded_rfc2379_by as a constant string. map $nginx_constant $proxy_forwarded_rfc2379_by { - default {{ $cfg.ForwardedRFC7239By | fieldValueComponet | quote }}; + default {{ $cfg.ForwardedRFC7239By | fieldValueComponet | printf "by=%s" | quote }}; } {{ end }} diff --git a/test/e2e/settings/forwarded_rfc7239.go b/test/e2e/settings/forwarded_rfc7239.go index d514e68c1..5ffd4be2d 100644 --- a/test/e2e/settings/forwarded_rfc7239.go +++ b/test/e2e/settings/forwarded_rfc7239.go @@ -17,6 +17,9 @@ limitations under the License. package settings import ( + "fmt" + "log" + "net" "net/http" "strings" @@ -59,12 +62,13 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { body := f.HTTPTestClient(). GET("/"). + WithHeader("Host", host). Expect(). Status(http.StatusOK). Body(). Raw() - assert.NotContains(ginkgo.GinkgoT(), body, "forwarded=1.2.3.4") + assert.NotContains(ginkgo.GinkgoT(), body, "forwarded=") }) ginkgo.It("should trust Forwarded header when striping-incoming is false", func() { @@ -86,27 +90,32 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { strings.Contains(server, "proxy_set_header Forwarded $proxy_add_forwarded_rfc2379;") }) + serverIP := f.GetNginxPodIP() + clientIP := getClientIP(serverIP) + ginkgo.By("ensuring valid headers are passed through correctly") body := f.HTTPTestClient(). GET("/"). - WithHeader("Forwarded", "for=1.1.1.1;secret=_5ecREy"). + WithHeader("Host", host). + WithHeader("Forwarded", "for=1.1.1.1;secret=_5ecREy, for=\"[2001:4860:4860::8888]\""). Expect(). Status(http.StatusOK). Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, "for=1.1.1.1;secret=_5e(REy, for=1.2.3.4") + assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf("forwarded=for=1.1.1.1;secret=_5ecREy, for=\"[2001:4860:4860::8888]\", for=%s", clientIP)) ginkgo.By("ensuring invalid headers are striped") body = f.HTTPTestClient(). GET("/"). - WithHeader("Forwarded", "for=1.1.1.1;secret=:x:"). // colon should be quoted + WithHeader("Host", host). + WithHeader("Forwarded", "for=2001:4860:4860::8888"). // invalid header, ipv6 should be bracked and quoted Expect(). Status(http.StatusOK). Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, "for=1.2.3.4") + assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf("forwarded=for=%s", clientIP)) }) ginkgo.It("should contain parameters in order as setting forwarded-rfc7239 specified", func() { @@ -129,17 +138,21 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { strings.Contains(server, "proxy_set_header Forwarded $proxy_add_forwarded_rfc2379;") }) + serverIP := f.GetNginxPodIP() + clientIP := getClientIP(serverIP) + ginkgo.By("ensuring singly pass through incoming header when empty parameter list") f.UpdateNginxConfigMapData(forwardedRFC7239, "") body := f.HTTPTestClient(). GET("/"). + WithHeader("Host", host). WithHeader("Forwarded", "for=1.1.1.1"). Expect(). Status(http.StatusOK). Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, "for=1.1.1.1") + assert.Contains(ginkgo.GinkgoT(), body, "forwarded=for=1.1.1.1") ginkgo.By("ensuring any parameter combinations work") f.UpdateNginxConfigMapData(forwardedRFC7239, "for,by,proto,host") @@ -151,10 +164,10 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, `for=1.2.3.4;by="1.2.3.4:80";proto=http;host=forwarded-rfc7239`) + assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf(`forwarded=for=%s;by="%s:80";proto=http;host=forwarded-rfc7239`, clientIP, serverIP)) }) - ginkgo.It("should config for and by parameters as static obfuscated values", func() { + ginkgo.It("should config \"for\" and \"by\" parameters as static obfuscated strings", func() { host := "forwarded-rfc7239" config := map[string]string{} @@ -174,40 +187,57 @@ var _ = framework.DescribeSetting("Configure Forwarded RFC7239", func() { strings.Contains(server, "proxy_set_header Forwarded $proxy_add_forwarded_rfc2379;") }) - ginkgo.By("ensuring \"by\" parameter is a static obfuscated value") + serverIP := f.GetNginxPodIP() + clientIP := getClientIP(serverIP) + + ginkgo.By("ensuring \"by\" parameter is a static obfuscated string") body := f.HTTPTestClient(). GET("/"). + WithHeader("Host", host). Expect(). Status(http.StatusOK). Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, "for=1.2.3.4;by=_SERVER1") + assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf("forwarded=for=%s;by=_SERVER1", clientIP)) - ginkgo.By("ensuring \"for\" parameter is a static obfuscated value") + ginkgo.By("ensuring \"for\" parameter is a static obfuscated string") config[forwardedRFC7239For] = "_HOST1" config[forwardedRFC7239By] = "ip" f.SetNginxConfigMapData(config) body = f.HTTPTestClient(). GET("/"). + WithHeader("Host", host). Expect(). Status(http.StatusOK). Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, `for="_HOST1";by="1.2.3.4:80"`) + assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf(`forwarded=for=_HOST1;by="%s:80"`, serverIP)) - ginkgo.By("ensuring invalid static obfuscated values are ingored") + ginkgo.By("ensuring invalid static obfuscated strings are ingored") config[forwardedRFC7239For] = "_HOST1" config[forwardedRFC7239By] = "_%" f.SetNginxConfigMapData(config) body = f.HTTPTestClient(). GET("/"). + WithHeader("Host", host). Expect(). Status(http.StatusOK). Body(). Raw() - assert.Contains(ginkgo.GinkgoT(), body, `for=1.2.3.4;by="1.2.3.4:80"`) + assert.Contains(ginkgo.GinkgoT(), body, fmt.Sprintf(`forwarded=for=_HOST1;by="%s:80"`, serverIP)) }) }) + +func getClientIP(serverIP string) net.IP { + conn, err := net.Dial("tcp", serverIP+":80") + if err != nil { + log.Fatal(err) + } + defer conn.Close() + + localAddr := conn.LocalAddr().(*net.TCPAddr) + return localAddr.IP +} From ee9d9d1b7ae50d6c32f12e7a45348f58f40e9ee9 Mon Sep 17 00:00:00 2001 From: zitudu Date: Thu, 17 Aug 2023 15:35:18 +0800 Subject: [PATCH 6/7] Remove unused helper fieldValueComponet --- .../ingress/controller/template/template.go | 14 ------------- .../controller/template/template_test.go | 21 ------------------- rootfs/etc/nginx/template/nginx.tmpl | 10 ++++----- 3 files changed, 5 insertions(+), 40 deletions(-) diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 29340a0cb..147455771 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -281,14 +281,9 @@ var ( "shouldLoadAuthDigestModule": shouldLoadAuthDigestModule, "buildServerName": buildServerName, "buildCorsOriginRegex": buildCorsOriginRegex, - "fieldValueComponet": fieldValueComponet, } ) -var ( - headerFieldValueTokenChars = regexp.MustCompile("^[!#$%&'*+.^_`|~0-9A-Za-z-]+$") -) - // escapeLiteralDollar will replace the $ character with ${literal_dollar} // which is made to work via the following configuration in the http section of // the template: @@ -1859,12 +1854,3 @@ func buildCorsOriginRegex(corsOrigins []string) string { originsRegex = originsRegex + ")$ ) { set $cors 'true'; }" return originsRegex } - -// Token containing character other than tchar must be quoted using -// double-quote marks (section 3.2.6 of RFC 7230). -func fieldValueComponet(s string) string { - if headerFieldValueTokenChars.MatchString(s) { - return s - } - return fmt.Sprintf("%q", s) -} diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index c8c6cb11d..a2c3b8299 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -2237,24 +2237,3 @@ func TestCleanConf(t *testing.T) { t.Errorf("cleanConf result don't match with expected: %s", diff) } } - -func TestFieldValueComponet(t *testing.T) { - testCases := map[string]struct { - input string - expect string - }{ - "empty": {"", `""`}, - "spaces": {" ", `" "`}, - "ipv4": {"1.2.3.4", `1.2.3.4`}, - "ipv6": {"[::1]", `"[::1]"`}, - "obfuscated identifier": {"_hidden", `_hidden`}, - "ipv4 port": {"1.2.3.4:80", `"1.2.3.4:80"`}, - "domain": {"example.com", `example.com`}, - "http schema": {"http", `http`}, - } - for name, tc := range testCases { - if comp := fieldValueComponet(tc.input); comp != tc.expect { - t.Errorf("%v: expected '%v' but returned '%v'", name, tc.expect, comp) - } - } -} diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index df2746ad0..b7f6a6a22 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -456,7 +456,7 @@ http { {{ if $cfg.EnableForwardedRFC7239 }} # Enabled standard forwarded (RFC 7239). - # Used to declare constant variables. + # Used to declare variables as constants. map $nginx_version $nginx_constant { default ''; } @@ -476,9 +476,9 @@ http { default "for=unknown"; } {{ else }} - # Set proxy_forwarded_rfc2379_for as a constant string. + # Set proxy_forwarded_rfc2379_for as a obfuscated string. map $nginx_constant $proxy_forwarded_rfc2379_for { - default {{ $cfg.ForwardedRFC7239For | fieldValueComponet | printf "for=%s" | quote }}; + default "for={{ $cfg.ForwardedRFC7239For }}"; } {{ end }} @@ -496,9 +496,9 @@ http { default "by=unknown"; } {{ else }} - # Set proxy_forwarded_rfc2379_by as a constant string. + # Set proxy_forwarded_rfc2379_by as a static obfuscated string. map $nginx_constant $proxy_forwarded_rfc2379_by { - default {{ $cfg.ForwardedRFC7239By | fieldValueComponet | printf "by=%s" | quote }}; + default "by={{ $cfg.ForwardedRFC7239By }}"; } {{ end }} From cee878be25363f83972b7451ea99734b903e4a89 Mon Sep 17 00:00:00 2001 From: zitudu Date: Thu, 17 Aug 2023 15:48:11 +0800 Subject: [PATCH 7/7] comment & lint --- internal/ingress/controller/config/config.go | 4 +-- rootfs/etc/nginx/template/nginx.tmpl | 38 ++++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 222279845..324fe8941 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -570,11 +570,11 @@ type Configuration struct { // Default: "for" ForwardedRFC7239 []string `json:"forwarded-rfc7239"` - // Sets Forwarded "for" parameter node identifier, should be "ip" or a static string. + // Sets Forwarded "for" parameter node identifier, should be "ip" or a static obfuscated string. // Default: "ip" ForwardedRFC7239For string `json:"forwarded-rfc7239-for,omitempty"` - // Sets Forwarded "by" parameter node identifier, should be "ip" or a static string. + // Sets Forwarded "by" parameter node identifier, should be "ip" or a static obfuscated string. // Default: "ip" ForwardedRFC7239By string `json:"forwarded-rfc7239-by,omitempty"` diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index b7f6a6a22..c710ef3ba 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -507,25 +507,25 @@ http { # be one of ipv4, ipv6, and reg-name optionaly followed by a port after # a colon. map $http_host $proxy_forwarded_rfc2379_host { - # IPv4 and reg-name formed of valid field value characters can be sent - # as-is. The regular expression of IPv4 is subset of this one, and this - # expression also matches IPv4. - # Since characters "#", "^", "|", and "`" are invalid for reg-name, - # these four characters are removed from character set. - "~^[!$%&'*+._~0-9A-Za-z-]+$" "host=$http_host"; - - # reg-names are not matched last regular expression need to be quoted. - "~^[!$&'()*+,;=._~%0-9A-Za-z-]$" "host=\"$http_host\""; - - # IPv6 addresses (with port or not) need to quoted (it must be already - # bracketed). - "~^\[[0-9A-Fa-f:.]+\](:[0-9]+)*$" "host=\"$http_host\""; - - # IPv4:port and reg-name:port and need to be quoted. - "~^[!$&'()*+,;=._~%0-9A-Za-z-]+:[0-9]+$" "host=\"$http_host\""; - - # Otherwise it is invalid. - default "host=unknown"; + # IPv4 and reg-name formed of valid field value characters can be sent + # as-is. The regular expression of IPv4 is subset of this one, and this + # expression also matches IPv4. + # Since characters "#", "^", "|", and "`" are invalid for reg-name, + # these four characters are removed from character set. + "~^[!$%&'*+._~0-9A-Za-z-]+$" "host=$http_host"; + + # reg-names are not matched last regular expression need to be quoted. + "~^[!$&'()*+,;=._~%0-9A-Za-z-]$" "host=\"$http_host\""; + + # IPv6 addresses (with port or not) need to quoted (it must be already + # bracketed). + "~^\[[0-9A-Fa-f:.]+\](:[0-9]+)*$" "host=\"$http_host\""; + + # IPv4:port and reg-name:port and need to be quoted. + "~^[!$&'()*+,;=._~%0-9A-Za-z-]+:[0-9]+$" "host=\"$http_host\""; + + # Otherwise it is invalid. + default "host=unknown"; } # Section 3.1 of RFC3986 defines scheme syntax.