Merge branch 'main' into renovate/google.golang.org-grpc-1.x

sjberman · web-flow · commit 97002a3ff6b1 · 2025-07-30T09:46:49.000-06:00
diff --git 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
diff --git 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
@@ -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
@@ -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
@@ -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/handler.go b/internal/controller/handler.go
@@ -208,7 +208,7 @@ func (h *eventHandlerImpl) sendNginxConfig(ctx context.Context, logger logr.Logg
 			panic("expected deployment, got nil")
 		}
 
-		cfg := dataplane.BuildConfiguration(ctx, gr, gw, h.cfg.serviceResolver, h.cfg.plus)
+		cfg := dataplane.BuildConfiguration(ctx, logger, gr, gw, h.cfg.serviceResolver, h.cfg.plus)
 		depCtx, getErr := h.getDeploymentContext(ctx)
 		if getErr != nil {
 			logger.Error(getErr, "error getting deployment context for usage reporting")
diff --git a/internal/controller/nginx/agent/agent.go b/internal/controller/nginx/agent/agent.go
@@ -90,6 +90,7 @@ func (n *NginxUpdaterImpl) UpdateConfig(
 ) {
 	msg := deployment.SetFiles(files)
 	if msg == nil {
+		n.logger.V(1).Info("No changes to nginx configuration files, not sending to agent")
 		return
 	}
 
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -7,6 +7,7 @@ import (
 	"slices"
 	"sort"
 
+	"github.com/go-logr/logr"
 	discoveryV1 "k8s.io/api/discovery/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -32,6 +33,7 @@ const (
 // BuildConfiguration builds the Configuration from the Graph.
 func BuildConfiguration(
 	ctx context.Context,
+	logger logr.Logger,
 	g *graph.Graph,
 	gateway *graph.Gateway,
 	serviceResolver resolver.ServiceResolver,
@@ -55,6 +57,7 @@ func BuildConfiguration(
 	backendGroups := buildBackendGroups(append(httpServers, sslServers...))
 	upstreams := buildUpstreams(
 		ctx,
+		logger,
 		gateway,
 		serviceResolver,
 		g.ReferencedServices,
@@ -71,9 +74,14 @@ func BuildConfiguration(
 		SSLServers:            sslServers,
 		TLSPassthroughServers: buildPassthroughServers(gateway),
 		Upstreams:             upstreams,
-		StreamUpstreams:       buildStreamUpstreams(ctx, gateway, serviceResolver, baseHTTPConfig.IPFamily),
-		BackendGroups:         backendGroups,
-		SSLKeyPairs:           buildSSLKeyPairs(g.ReferencedSecrets, gateway.Listeners),
+		StreamUpstreams: buildStreamUpstreams(
+			ctx,
+			logger,
+			gateway,
+			serviceResolver,
+			baseHTTPConfig.IPFamily),
+		BackendGroups: backendGroups,
+		SSLKeyPairs:   buildSSLKeyPairs(g.ReferencedSecrets, gateway.Listeners),
 		CertBundles: buildCertBundles(
 			buildRefCertificateBundles(g.ReferencedSecrets, g.ReferencedCaCertConfigMaps),
 			backendGroups,
@@ -163,6 +171,7 @@ func buildPassthroughServers(gateway *graph.Gateway) []Layer4VirtualServer {
 // buildStreamUpstreams builds all stream upstreams.
 func buildStreamUpstreams(
 	ctx context.Context,
+	logger logr.Logger,
 	gateway *graph.Gateway,
 	serviceResolver resolver.ServiceResolver,
 	ipFamily IPFamilyType,
@@ -202,7 +211,13 @@ func buildStreamUpstreams(
 
 			allowedAddressType := getAllowedAddressType(ipFamily)
 
-			eps, err := serviceResolver.Resolve(ctx, br.SvcNsName, br.ServicePort, allowedAddressType)
+			eps, err := serviceResolver.Resolve(
+				ctx,
+				logger,
+				br.SvcNsName,
+				br.ServicePort,
+				allowedAddressType,
+			)
 			if err != nil {
 				errMsg = err.Error()
 			}
@@ -670,6 +685,7 @@ func (hpr *hostPathRules) maxServerCount() int {
 
 func buildUpstreams(
 	ctx context.Context,
+	logger logr.Logger,
 	gateway *graph.Gateway,
 	svcResolver resolver.ServiceResolver,
 	referencedServices map[types.NamespacedName]*graph.ReferencedService,
@@ -701,6 +717,7 @@ func buildUpstreams(
 				for _, br := range rule.BackendRefs {
 					if upstream := buildUpstream(
 						ctx,
+						logger,
 						br,
 						gateway,
 						svcResolver,
@@ -735,6 +752,7 @@ func buildUpstreams(
 
 func buildUpstream(
 	ctx context.Context,
+	logger logr.Logger,
 	br graph.BackendRef,
 	gateway *graph.Gateway,
 	svcResolver resolver.ServiceResolver,
@@ -760,9 +778,10 @@ func buildUpstream(
 
 	var errMsg string
 
-	eps, err := svcResolver.Resolve(ctx, br.SvcNsName, br.ServicePort, allowedAddressType)
+	eps, err := svcResolver.Resolve(ctx, logger, br.SvcNsName, br.ServicePort, allowedAddressType)
 	if err != nil {
 		errMsg = err.Error()
+		logger.Error(err, "failed to resolve endpoints", "service", br.SvcNsName)
 	}
 
 	var upstreamPolicies []policies.Policy
@@ -991,6 +1010,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
@@ -7,6 +7,7 @@ import (
 	"sort"
 	"testing"
 
+	"github.com/go-logr/logr"
 	. "github.com/onsi/gomega"
 	apiv1 "k8s.io/api/core/v1"
 	discoveryV1 "k8s.io/api/discovery/v1"
@@ -714,6 +715,7 @@ func TestBuildConfiguration(t *testing.T) {
 
 	fakeResolver.ResolveStub = func(
 		_ context.Context,
+		_ logr.Logger,
 		nsName types.NamespacedName,
 		_ apiv1.ServicePort,
 		_ []discoveryV1.AddressType,
@@ -934,8 +936,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 +2237,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
 			}),
@@ -2533,7 +2537,8 @@ func TestBuildConfiguration(t *testing.T) {
 			g := NewWithT(t)
 
 			result := BuildConfiguration(
-				context.TODO(),
+				t.Context(),
+				logr.Discard(),
 				test.graph,
 				test.graph.Gateways[gatewayNsName],
 				fakeResolver,
@@ -2648,7 +2653,8 @@ func TestBuildConfiguration_Plus(t *testing.T) {
 			g := NewWithT(t)
 
 			result := BuildConfiguration(
-				context.TODO(),
+				t.Context(),
+				logr.Discard(),
 				test.graph,
 				test.graph.Gateways[gatewayNsName],
 				fakeResolver,
@@ -3372,6 +3378,7 @@ func TestBuildUpstreams(t *testing.T) {
 	fakeResolver := &resolverfakes.FakeServiceResolver{}
 	fakeResolver.ResolveCalls(func(
 		_ context.Context,
+		_ logr.Logger,
 		svcNsName types.NamespacedName,
 		_ apiv1.ServicePort,
 		_ []discoveryV1.AddressType,
@@ -3404,7 +3411,14 @@ func TestBuildUpstreams(t *testing.T) {
 
 	g := NewWithT(t)
 
-	upstreams := buildUpstreams(context.TODO(), gateway, fakeResolver, referencedServices, Dual)
+	upstreams := buildUpstreams(
+		t.Context(),
+		logr.Discard(),
+		gateway,
+		fakeResolver,
+		referencedServices,
+		Dual,
+	)
 	g.Expect(upstreams).To(ConsistOf(expUpstreams))
 }
 
@@ -4357,6 +4371,7 @@ func TestBuildStreamUpstreams(t *testing.T) {
 
 	fakeResolver.ResolveStub = func(
 		_ context.Context,
+		_ logr.Logger,
 		nsName types.NamespacedName,
 		_ apiv1.ServicePort,
 		_ []discoveryV1.AddressType,
@@ -4367,7 +4382,7 @@ func TestBuildStreamUpstreams(t *testing.T) {
 		return fakeEndpoints, nil
 	}
 
-	streamUpstreams := buildStreamUpstreams(context.Background(), gateway, &fakeResolver, Dual)
+	streamUpstreams := buildStreamUpstreams(t.Context(), logr.Discard(), gateway, &fakeResolver, Dual)
 
 	expectedStreamUpstreams := []Upstream{
 		{
diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go
diff --git a/internal/controller/state/resolver/resolver.go b/internal/controller/state/resolver/resolver.go
diff --git a/internal/controller/state/resolver/resolver_test.go b/internal/controller/state/resolver/resolver_test.go
diff --git a/internal/controller/state/resolver/resolverfakes/fake_service_resolver.go b/internal/controller/state/resolver/resolverfakes/fake_service_resolver.go
diff --git a/internal/controller/state/resolver/service_resolver_test.go b/internal/controller/state/resolver/service_resolver_test.go

Original file line number	Diff line number	Diff line change
`@@ -208,7 +208,7 @@ func (h *eventHandlerImpl) sendNginxConfig(ctx context.Context, logger logr.Logg`
`208`	`208`	`panic("expected deployment, got nil")`
`209`	`209`	`}`
`210`	`210`
`211`		`- cfg := dataplane.BuildConfiguration(ctx, gr, gw, h.cfg.serviceResolver, h.cfg.plus)`
	`211`	`+ cfg := dataplane.BuildConfiguration(ctx, logger, gr, gw, h.cfg.serviceResolver, h.cfg.plus)`
`212`	`212`	`depCtx, getErr := h.getDeploymentContext(ctx)`
`213`	`213`	`if getErr != nil {`
`214`	`214`	`logger.Error(getErr, "error getting deployment context for usage reporting")`
Original file line number	Diff line number	Diff line change
`@@ -90,6 +90,7 @@ func (n *NginxUpdaterImpl) UpdateConfig(`
`90`	`90`	`) {`
`91`	`91`	`msg := deployment.SetFiles(files)`
`92`	`92`	`if msg == nil {`
	`93`	`+ n.logger.V(1).Info("No changes to nginx configuration files, not sending to agent")`
`93`	`94`	`return`
`94`	`95`	`}`
`95`	`96`