diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index a8d075bee..2230f80f7 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -352,6 +352,9 @@ metadata: name: custom-headers-configmap ``` +!!! attention + First define the allowed response headers in [global-allowed-response-headers](https://github.com/kubernetes/ingress-nginx/blob/main/docs/user-guide/nginx-configuration/configmap.md#global-allowed-response-headers). + ### Default Backend This annotation is of the form `nginx.ingress.kubernetes.io/default-backend: ` to specify a custom default backend. This `` is a reference to a service inside of the same namespace in which you are applying this annotation. This annotation overrides the global default backend. In case the service has [multiple ports](https://kubernetes.io/docs/concepts/services-networking/service/#multi-port-services), the first one is the one which will receive the backend traffic. diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 89a35b07b..52c54c4fe 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -209,6 +209,7 @@ The following table shows a configuration option's name, type, and the default v |[syslog-host](#syslog-host)| string | "" || |[syslog-port](#syslog-port)| int | 514 || |[no-tls-redirect-locations](#no-tls-redirect-locations)| string | "/.well-known/acme-challenge" || +|[global-allowed-response-headers](#global-allowed-response-headers)|string|""|| |[global-auth-url](#global-auth-url)| string | "" || |[global-auth-method](#global-auth-method)| string | "" || |[global-auth-signin](#global-auth-signin)| string | "" || @@ -1285,6 +1286,10 @@ Sets the port of syslog server. _**default:**_ 514 A comma-separated list of locations on which http requests will never get redirected to their https counterpart. _**default:**_ "/.well-known/acme-challenge" +## global-allowed-response-headers + +A comma-separated list of allowed response headers inside the [custom headers annotations](https://github.com/kubernetes/ingress-nginx/blob/main/docs/user-guide/nginx-configuration/annotations.md#custom-headers) + ## global-auth-url A url to an existing service that provides authentication for all the locations. diff --git a/internal/ingress/annotations/customheaders/main.go b/internal/ingress/annotations/customheaders/main.go index f612992e4..12428a1be 100644 --- a/internal/ingress/annotations/customheaders/main.go +++ b/internal/ingress/annotations/customheaders/main.go @@ -24,6 +24,7 @@ import ( networking "k8s.io/api/networking/v1" + "golang.org/x/exp/slices" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ing_errors "k8s.io/ingress-nginx/internal/ingress/errors" "k8s.io/ingress-nginx/internal/ingress/resolver" @@ -61,6 +62,7 @@ func (a customHeaders) Parse(ing *networking.Ingress) (interface{}, error) { } var headers map[string]string + defBackend := a.r.GetDefaultBackend() if clientHeadersConfigMapName != "" { clientHeadersMapContents, err := a.r.GetConfigMap(clientHeadersConfigMapName) @@ -72,6 +74,9 @@ func (a customHeaders) Parse(ing *networking.Ingress) (interface{}, error) { if !ValidHeader(header) { return nil, ing_errors.NewLocationDenied("invalid client-headers in configmap") } + if !slices.Contains(defBackend.AllowedResponseHeaders, header) { + return nil, ing_errors.NewLocationDenied(fmt.Sprintf("header %s is not allowed, defined allowed headers inside global-allowed-response-headers %v", header, defBackend.AllowedResponseHeaders)) + } } headers = clientHeadersMapContents.Data diff --git a/internal/ingress/annotations/customheaders/main_test.go b/internal/ingress/annotations/customheaders/main_test.go index c090f8697..150fd0720 100644 --- a/internal/ingress/annotations/customheaders/main_test.go +++ b/internal/ingress/annotations/customheaders/main_test.go @@ -24,6 +24,7 @@ import ( networking "k8s.io/api/networking/v1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -46,11 +47,21 @@ func buildIngress() *networking.Ingress { } } +type mockBackend struct { + resolver.Mock +} + +// GetDefaultBackend returns the backend that must be used as default +func (m mockBackend) GetDefaultBackend() defaults.Backend { + return defaults.Backend{ + AllowedResponseHeaders: []string{"Content-Type", "Access-Control-Max-Age"}, + } +} + func TestCustomHeadersParseInvalidAnnotations(t *testing.T) { ing := buildIngress() - configMapResolver := &resolver.Mock{ - ConfigMaps: map[string]*api.ConfigMap{}, - } + configMapResolver := mockBackend{} + configMapResolver.ConfigMaps = map[string]*api.ConfigMap{} _, err := NewParser(configMapResolver).Parse(ing) if err != nil { @@ -76,15 +87,14 @@ func TestCustomHeadersParseAnnotations(t *testing.T) { data[parser.GetAnnotationWithPrefix("custom-headers")] = "custom-headers-configmap" ing.SetAnnotations(data) - configMapResolver := &resolver.Mock{ - ConfigMaps: map[string]*api.ConfigMap{}, - } + configMapResolver := mockBackend{} + configMapResolver.ConfigMaps = map[string]*api.ConfigMap{} configMapResolver.ConfigMaps["custom-headers-configmap"] = &api.ConfigMap{Data: map[string]string{"Content-Type": "application/json", "Access-Control-Max-Age": "600"}} i, err := NewParser(configMapResolver).Parse(ing) if err != nil { - t.Errorf("unexpected error parsing ingress with custom-response-headers") + t.Errorf("unexpected error parsing ingress with custom-response-headers: %s", err) } val, ok := i.(*Config) if !ok { diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 371f3069c..47f2120f1 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -888,6 +888,7 @@ func NewDefault() Configuration { ProxyHTTPVersion: "1.1", ProxyMaxTempFileSize: "1024m", ServiceUpstream: false, + AllowedResponseHeaders: []string{}, }, UpstreamKeepaliveConnections: 320, UpstreamKeepaliveTime: "1h", diff --git a/internal/ingress/controller/template/configmap.go b/internal/ingress/controller/template/configmap.go index 9dc019bcc..25827a08e 100644 --- a/internal/ingress/controller/template/configmap.go +++ b/internal/ingress/controller/template/configmap.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/ingress-nginx/internal/ingress/annotations/authreq" + "k8s.io/ingress-nginx/internal/ingress/annotations/customheaders" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/controller/config" ing_net "k8s.io/ingress-nginx/internal/net" @@ -54,6 +55,7 @@ const ( nginxStatusIpv6Whitelist = "nginx-status-ipv6-whitelist" proxyHeaderTimeout = "proxy-protocol-header-timeout" workerProcesses = "worker-processes" + globalAllowedResponseHeaders = "global-allowed-response-headers" globalAuthURL = "global-auth-url" globalAuthMethod = "global-auth-method" globalAuthSignin = "global-auth-signin" @@ -115,6 +117,7 @@ func ReadConfig(src map[string]string) config.Configuration { blockUserAgentList := make([]string, 0) blockRefererList := make([]string, 0) responseHeaders := make([]string, 0) + allowedResponseHeaders := make([]string, 0) luaSharedDicts := make(map[string]int) debugConnectionsList := make([]string, 0) @@ -248,6 +251,22 @@ func ReadConfig(src map[string]string) config.Configuration { } } + // Verify that the configured global external authorization response headers are valid. if not, set the default value + if val, ok := conf[globalAllowedResponseHeaders]; ok { + delete(conf, globalAllowedResponseHeaders) + + if len(val) != 0 { + harr := splitAndTrimSpace(val, ",") + for _, header := range harr { + if !customheaders.ValidHeader(header) { + klog.Warningf("Global allowed response headers denied - %s.", header) + } else { + allowedResponseHeaders = append(allowedResponseHeaders, header) + } + } + } + } + // Verify that the configured global external authorization method is a valid HTTP method. if not, set the default value if val, ok := conf[globalAuthMethod]; ok { delete(conf, globalAuthMethod) @@ -422,6 +441,7 @@ func ReadConfig(src map[string]string) config.Configuration { to.ProxyStreamResponses = streamResponses to.DisableIpv6DNS = !ing_net.IsIPv6Enabled() to.LuaSharedDicts = luaSharedDicts + to.Backend.AllowedResponseHeaders = allowedResponseHeaders decoderConfig := &mapstructure.DecoderConfig{ Metadata: nil, diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go index 2bb58c858..cfad388ef 100644 --- a/internal/ingress/defaults/main.go +++ b/internal/ingress/defaults/main.go @@ -176,6 +176,9 @@ type Backend struct { // By default, the NGINX ingress controller uses a list of all endpoints (Pod IP/port) in the NGINX upstream configuration. // It disables that behavior and instead uses a single upstream in NGINX, the service's Cluster IP and port. ServiceUpstream bool `json:"service-upstream"` + + // AllowedResponseHeaders allows to define allow response headers for custom header annotation + AllowedResponseHeaders []string `json:"global-allowed-response-headers"` } type SecurityConfiguration struct { diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index e87c836d3..4c0da2eb9 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -1492,7 +1492,7 @@ stream { {{ if $location.CustomHeaders }} # Custom Response Headers {{ range $k, $v := $location.CustomHeaders.Headers }} - more_set_headers {{ printf "%s: %s" $k $v | quote }}; + more_set_headers {{ printf "%s: %s" $k $v | escapeLiteralDollar | quote }}; {{ end }} {{ end }} diff --git a/test/e2e/annotations/customheaders.go b/test/e2e/annotations/customheaders.go index 4dedb904c..529d6b31b 100644 --- a/test/e2e/annotations/customheaders.go +++ b/test/e2e/annotations/customheaders.go @@ -82,8 +82,10 @@ var _ = framework.DescribeAnnotation("custom-headers-*", func() { } f.CreateConfigMap("custom-headers", map[string]string{ - "My-Custom-Header": "42", + "My-Custom-Header": "42", + "My-Custom-Header-Dollar": "$remote_addr", }) + f.UpdateNginxConfigMapData("global-allowed-response-headers", "My-Custom-Header,My-Custom-Header-Dollar") ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations) f.EnsureIngress(ing) @@ -99,5 +101,11 @@ var _ = framework.DescribeAnnotation("custom-headers-*", func() { Expect(). Status(http.StatusOK). Header("My-Custom-Header").Contains("42") + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + Expect(). + Status(http.StatusOK). + Header("My-Custom-Header-Dollar").Contains("$remote_addr") }) })