From fe7963ec3c5c94d281c99240a1c026ac60596c73 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Fri, 25 Jul 2025 13:33:55 +0100 Subject: [PATCH] Add disable SNI host validation field --- apis/v1alpha2/nginxproxy_types.go | 9 +++++ apis/v1alpha2/zz_generated.deepcopy.go | 5 +++ .../nginx-gateway-fabric/values.schema.json | 5 +++ charts/nginx-gateway-fabric/values.yaml | 3 ++ .../bases/gateway.nginx.org_nginxproxies.yaml | 9 +++++ deploy/crds.yaml | 9 +++++ .../controller/nginx/config/http/config.go | 9 +++-- internal/controller/nginx/config/servers.go | 9 +++-- .../nginx/config/servers_template.go | 2 + .../controller/nginx/config/servers_test.go | 38 +++++++++++++++++++ .../state/dataplane/configuration.go | 4 ++ .../state/dataplane/configuration_test.go | 12 +++--- internal/controller/state/dataplane/types.go | 6 ++- 13 files changed, 105 insertions(+), 15 deletions(-) diff --git a/apis/v1alpha2/nginxproxy_types.go b/apis/v1alpha2/nginxproxy_types.go index a1dfd1632a..fccdb92c25 100644 --- a/apis/v1alpha2/nginxproxy_types.go +++ b/apis/v1alpha2/nginxproxy_types.go @@ -73,6 +73,15 @@ type NginxProxySpec struct { // // +optional DisableHTTP2 *bool `json:"disableHTTP2,omitempty"` + // DisableSNIHostValidation disables the validation that ensures the SNI hostname + // matches the Host header in HTTPS requests. When disabled, HTTPS connections can + // be reused for requests to different hostnames covered by the same certificate. + // This resolves HTTP/2 connection coalescing issues with wildcard certificates but + // introduces security risks as described in Gateway API GEP-3567. + // If not specified, defaults to false (validation enabled). + // + // +optional + DisableSNIHostValidation *bool `json:"disableSNIHostValidation,omitempty"` // Kubernetes contains the configuration for the NGINX Deployment and Service Kubernetes objects. // // +optional diff --git a/apis/v1alpha2/zz_generated.deepcopy.go b/apis/v1alpha2/zz_generated.deepcopy.go index c220477cbf..cda55fce63 100644 --- a/apis/v1alpha2/zz_generated.deepcopy.go +++ b/apis/v1alpha2/zz_generated.deepcopy.go @@ -373,6 +373,11 @@ func (in *NginxProxySpec) DeepCopyInto(out *NginxProxySpec) { *out = new(bool) **out = **in } + if in.DisableSNIHostValidation != nil { + in, out := &in.DisableSNIHostValidation, &out.DisableSNIHostValidation + *out = new(bool) + **out = **in + } if in.Kubernetes != nil { in, out := &in.Kubernetes, &out.Kubernetes *out = new(KubernetesSpec) diff --git a/charts/nginx-gateway-fabric/values.schema.json b/charts/nginx-gateway-fabric/values.schema.json index d750c0a42a..88edebfdbf 100644 --- a/charts/nginx-gateway-fabric/values.schema.json +++ b/charts/nginx-gateway-fabric/values.schema.json @@ -106,6 +106,11 @@ "required": [], "type": "boolean" }, + "disableSNIHostValidation": { + "description": "DisableSNIHostValidation disables the validation that ensures the SNI hostname matches the Host header in HTTPS requests. This resolves HTTP/2 connection coalescing issues with wildcard certificates but introduces security risks as described in Gateway API GEP-3567.", + "required": [], + "type": "boolean" + }, "ipFamily": { "description": "IPFamily specifies the IP family to be used by the NGINX.", "enum": [ diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index e9765ea511..361a507616 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -251,6 +251,9 @@ nginx: # disableHTTP2: # description: DisableHTTP2 defines if http2 should be disabled for all servers. # type: boolean + # disableSNIHostValidation: + # description: DisableSNIHostValidation disables the validation that ensures the SNI hostname matches the Host header in HTTPS requests. This resolves HTTP/2 connection coalescing issues with wildcard certificates but introduces security risks as described in Gateway API GEP-3567. + # type: boolean # ipFamily: # description: IPFamily specifies the IP family to be used by the NGINX. # type: string diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 14def1da6d..c88c0b0534 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -56,6 +56,15 @@ spec: DisableHTTP2 defines if http2 should be disabled for all servers. If not specified, or set to false, http2 will be enabled for all servers. type: boolean + disableSNIHostValidation: + description: |- + DisableSNIHostValidation disables the validation that ensures the SNI hostname + matches the Host header in HTTPS requests. When disabled, HTTPS connections can + be reused for requests to different hostnames covered by the same certificate. + This resolves HTTP/2 connection coalescing issues with wildcard certificates but + introduces security risks as described in Gateway API GEP-3567. + If not specified, defaults to false (validation enabled). + type: boolean ipFamily: default: dual description: |- diff --git a/deploy/crds.yaml b/deploy/crds.yaml index db7e84980a..fb122d2733 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -641,6 +641,15 @@ spec: DisableHTTP2 defines if http2 should be disabled for all servers. If not specified, or set to false, http2 will be enabled for all servers. type: boolean + disableSNIHostValidation: + description: |- + DisableSNIHostValidation disables the validation that ensures the SNI hostname + matches the Host header in HTTPS requests. When disabled, HTTPS connections can + be reused for requests to different hostnames covered by the same certificate. + This resolves HTTP/2 connection coalescing issues with wildcard certificates but + introduces security risks as described in Gateway API GEP-3567. + If not specified, defaults to false (validation enabled). + type: boolean ipFamily: default: dual description: |- diff --git a/internal/controller/nginx/config/http/config.go b/internal/controller/nginx/config/http/config.go index 8f06dee04d..29c7fb6373 100644 --- a/internal/controller/nginx/config/http/config.go +++ b/internal/controller/nginx/config/http/config.go @@ -127,8 +127,9 @@ type ProxySSLVerify struct { // ServerConfig holds configuration for an HTTP server and IP family to be used by NGINX. type ServerConfig struct { - Servers []Server - RewriteClientIP shared.RewriteClientIPSettings - IPFamily shared.IPFamily - Plus bool + Servers []Server + RewriteClientIP shared.RewriteClientIPSettings + IPFamily shared.IPFamily + Plus bool + DisableSNIHostValidation bool } diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 0ae71691a1..88ba4fa8ea 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -61,10 +61,11 @@ func (g GeneratorImpl) executeServers( servers, httpMatchPairs := createServers(conf, generator, keepAliveCheck) serverConfig := http.ServerConfig{ - Servers: servers, - IPFamily: getIPFamily(conf.BaseHTTPConfig), - Plus: g.plus, - RewriteClientIP: getRewriteClientIPSettings(conf.BaseHTTPConfig.RewriteClientIPSettings), + Servers: servers, + IPFamily: getIPFamily(conf.BaseHTTPConfig), + Plus: g.plus, + RewriteClientIP: getRewriteClientIPSettings(conf.BaseHTTPConfig.RewriteClientIPSettings), + DisableSNIHostValidation: conf.BaseHTTPConfig.DisableSNIHostValidation, } serverResult := executeResult{ diff --git a/internal/controller/nginx/config/servers_template.go b/internal/controller/nginx/config/servers_template.go index b49884a94b..ac7e4ae5bf 100644 --- a/internal/controller/nginx/config/servers_template.go +++ b/internal/controller/nginx/config/servers_template.go @@ -55,9 +55,11 @@ server { ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; + {{- if not $.DisableSNIHostValidation }} if ($ssl_server_name != $host) { return 421; } + {{- end }} {{- else }} {{- if $.IPFamily.IPv4 }} listen {{ $s.Listen }}{{ $.RewriteClientIP.ProxyProtocol }}; diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 6cb90a9415..6b604d7bec 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -4024,3 +4024,41 @@ func TestGetIPFamily(t *testing.T) { }) } } + +func TestExecuteServers_DisableSNIHostValidation(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + sslServer := dataplane.VirtualServer{ + Hostname: "example.com", + SSL: &dataplane.SSL{ + KeyPairID: "test-keypair", + }, + Port: 8443, + } + gen := GeneratorImpl{} + + // Case 1: DisableSNIHostValidation = false (default) + confWithValidation := dataplane.Configuration{ + SSLServers: []dataplane.VirtualServer{sslServer}, + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + DisableSNIHostValidation: false, + }, + } + results := gen.executeServers(confWithValidation, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker) + serverConf := string(results[0].data) + g.Expect(serverConf).To(ContainSubstring("if ($ssl_server_name != $host)"), + "Expected SNI host validation block to be present when DisableSNIHostValidation is false") + + // Case 2: DisableSNIHostValidation = true + confWithoutValidation := dataplane.Configuration{ + SSLServers: []dataplane.VirtualServer{sslServer}, + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + DisableSNIHostValidation: true, + }, + } + results = gen.executeServers(confWithoutValidation, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker) + serverConf = string(results[0].data) + g.Expect(serverConf).NotTo(ContainSubstring("if ($ssl_server_name != $host)"), + "Expected SNI host validation block to be absent when DisableSNIHostValidation is true") +} diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index b2f24e2aac..a63694df93 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -991,6 +991,10 @@ func buildBaseHTTPConfig( baseConfig.HTTP2 = false } + if np.DisableSNIHostValidation != nil && *np.DisableSNIHostValidation { + baseConfig.DisableSNIHostValidation = true + } + if np.IPFamily != nil { switch *np.IPFamily { case ngfAPIv1alpha2.IPv4: diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index a95ef8f515..3554e502d1 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -934,8 +934,9 @@ func TestBuildConfiguration(t *testing.T) { }, ServiceName: helpers.GetPointer("my-svc"), }, - DisableHTTP2: helpers.GetPointer(true), - IPFamily: helpers.GetPointer(ngfAPIv1alpha2.Dual), + DisableHTTP2: helpers.GetPointer(true), + IPFamily: helpers.GetPointer(ngfAPIv1alpha2.Dual), + DisableSNIHostValidation: helpers.GetPointer(true), } nginxProxyIPv4 := &graph.EffectiveNginxProxy{ @@ -2234,9 +2235,10 @@ func TestBuildConfiguration(t *testing.T) { SpanAttributes: []SpanAttribute{}, } conf.BaseHTTPConfig = BaseHTTPConfig{ - HTTP2: false, - IPFamily: Dual, - NginxReadinessProbePort: DefaultNginxReadinessProbePort, + HTTP2: false, + IPFamily: Dual, + NginxReadinessProbePort: DefaultNginxReadinessProbePort, + DisableSNIHostValidation: true, } return conf }), diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 79b3f42d6f..ac8e263050 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -372,10 +372,12 @@ type BaseHTTPConfig struct { Snippets []Snippet // 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 // NginxReadinessProbePort is the port on which the health check endpoint for NGINX is exposed. NginxReadinessProbePort int32 + // HTTP2 specifies whether http2 should be enabled for all servers. + HTTP2 bool + // DisableSNIHostValidation specifies if the SNI host validation should be disabled. + DisableSNIHostValidation bool } // Snippet is a snippet of configuration.