diff --git a/.changelog/20417.txt b/.changelog/20417.txt new file mode 100644 index 0000000000..c55353348a --- /dev/null +++ b/.changelog/20417.txt @@ -0,0 +1,3 @@ +```release-note:bug +connect: Fix regression with SAN matching on terminating gateways [GH-20360](https://github.com/hashicorp/consul/issues/20360) +``` diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index d677d20607..df59604583 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -481,7 +481,7 @@ func makeMTLSTransportSocket(cfgSnap *proxycfg.ConfigSnapshot, uid proxycfg.Upst cfgSnap.RootPEMs(), makeTLSParametersFromProxyTLSConfig(cfgSnap.MeshConfigTLSOutgoing()), ) - err := injectSANMatcher(commonTLSContext, spiffeID.URI().String()) + err := injectSANMatcher(commonTLSContext, false, spiffeID.URI().String()) if err != nil { return nil, fmt.Errorf("failed to inject SAN matcher rules for cluster %q: %v", sni, err) } @@ -875,7 +875,7 @@ func (s *ResourceGenerator) injectGatewayServiceAddons(cfgSnap *proxycfg.ConfigS } if mapping.SNI != "" { tlsContext.Sni = mapping.SNI - if err := injectSANMatcher(tlsContext.CommonTlsContext, mapping.SNI); err != nil { + if err := injectSANMatcher(tlsContext.CommonTlsContext, true, mapping.SNI); err != nil { return fmt.Errorf("failed to inject SNI matcher into TLS context: %v", err) } } @@ -904,7 +904,7 @@ func (s *ResourceGenerator) injectGatewayDestinationAddons(cfgSnap *proxycfg.Con } if mapping.SNI != "" { tlsContext.Sni = mapping.SNI - if err := injectSANMatcher(tlsContext.CommonTlsContext, mapping.SNI); err != nil { + if err := injectSANMatcher(tlsContext.CommonTlsContext, true, mapping.SNI); err != nil { return fmt.Errorf("failed to inject SNI matcher into TLS context: %v", err) } } @@ -1226,7 +1226,7 @@ func (s *ResourceGenerator) makeUpstreamClusterForPeerService( rootPEMs, makeTLSParametersFromProxyTLSConfig(cfgSnap.MeshConfigTLSOutgoing()), ) - err = injectSANMatcher(commonTLSContext, peerMeta.SpiffeID...) + err = injectSANMatcher(commonTLSContext, false, peerMeta.SpiffeID...) if err != nil { return nil, fmt.Errorf("failed to inject SAN matcher rules for cluster %q: %v", clusterName, err) } @@ -1329,7 +1329,7 @@ func (s *ResourceGenerator) makeUpstreamClusterForPreparedQuery(upstream structs cfgSnap.RootPEMs(), makeTLSParametersFromProxyTLSConfig(cfgSnap.MeshConfigTLSOutgoing()), ) - err = injectSANMatcher(commonTLSContext, spiffeIDs...) + err = injectSANMatcher(commonTLSContext, false, spiffeIDs...) if err != nil { return nil, fmt.Errorf("failed to inject SAN matcher rules for cluster %q: %v", sni, err) } @@ -1609,7 +1609,7 @@ func (s *ResourceGenerator) makeExportedUpstreamClustersForMeshGateway(cfgSnap * } // injectSANMatcher updates a TLS context so that it verifies the upstream SAN. -func injectSANMatcher(tlsContext *envoy_tls_v3.CommonTlsContext, matchStrings ...string) error { +func injectSANMatcher(tlsContext *envoy_tls_v3.CommonTlsContext, terminatingEgress bool, matchStrings ...string) error { if tlsContext == nil { return fmt.Errorf("invalid type: expected CommonTlsContext_ValidationContext not to be nil") } @@ -1620,16 +1620,37 @@ func injectSANMatcher(tlsContext *envoy_tls_v3.CommonTlsContext, matchStrings .. tlsContext.ValidationContextType) } + // All mesh services should match by URI + types := []envoy_tls_v3.SubjectAltNameMatcher_SanType{ + envoy_tls_v3.SubjectAltNameMatcher_URI, + } + if terminatingEgress { + // Terminating gateways will need to match on many fields depending on user configuration, + // since they make egress calls outside of the cluster. Having more than one matcher behaves + // like an OR operation, where any match is sufficient to pass the certificate validation. + // To maintain backwards compatibility with the old untyped `match_subject_alt_names` behavior, + // we should match on all 4 enum types. + // https://github.com/hashicorp/consul/issues/20360 + // https://github.com/envoyproxy/envoy/pull/18628/files#diff-cf088136dc052ddf1762fb3c96c0e8de472f3031f288e7e300558e6e72c8e129R69-R75 + types = []envoy_tls_v3.SubjectAltNameMatcher_SanType{ + envoy_tls_v3.SubjectAltNameMatcher_URI, + envoy_tls_v3.SubjectAltNameMatcher_DNS, + envoy_tls_v3.SubjectAltNameMatcher_EMAIL, + envoy_tls_v3.SubjectAltNameMatcher_IP_ADDRESS, + } + } var matchers []*envoy_tls_v3.SubjectAltNameMatcher for _, m := range matchStrings { - matchers = append(matchers, &envoy_tls_v3.SubjectAltNameMatcher{ - SanType: envoy_tls_v3.SubjectAltNameMatcher_URI, - Matcher: &envoy_matcher_v3.StringMatcher{ - MatchPattern: &envoy_matcher_v3.StringMatcher_Exact{ - Exact: m, + for _, t := range types { + matchers = append(matchers, &envoy_tls_v3.SubjectAltNameMatcher{ + SanType: t, + Matcher: &envoy_matcher_v3.StringMatcher{ + MatchPattern: &envoy_matcher_v3.StringMatcher_Exact{ + Exact: m, + }, }, - }, - }) + }) + } } validationCtx.ValidationContext.MatchTypedSubjectAltNames = matchers diff --git a/agent/xds/failover_policy.go b/agent/xds/failover_policy.go index ab3e86f25d..fdd351670a 100644 --- a/agent/xds/failover_policy.go +++ b/agent/xds/failover_policy.go @@ -132,7 +132,7 @@ func (s *ResourceGenerator) mapDiscoChainTargets(cfgSnap *proxycfg.ConfigSnapsho makeTLSParametersFromProxyTLSConfig(cfgSnap.MeshConfigTLSOutgoing()), ) - err := injectSANMatcher(commonTLSContext, spiffeIDs...) + err := injectSANMatcher(commonTLSContext, false, spiffeIDs...) if err != nil { return failoverTargets, fmt.Errorf("failed to inject SAN matcher rules for cluster %q: %v", sni, err) } diff --git a/agent/xds/testdata/clusters/terminating-gateway-sni.latest.golden b/agent/xds/testdata/clusters/terminating-gateway-sni.latest.golden index 516b6d3915..190301470a 100644 --- a/agent/xds/testdata/clusters/terminating-gateway-sni.latest.golden +++ b/agent/xds/testdata/clusters/terminating-gateway-sni.latest.golden @@ -52,6 +52,24 @@ "exact": "bar.com" }, "sanType": "URI" + }, + { + "matcher": { + "exact": "bar.com" + }, + "sanType": "DNS" + }, + { + "matcher": { + "exact": "bar.com" + }, + "sanType": "EMAIL" + }, + { + "matcher": { + "exact": "bar.com" + }, + "sanType": "IP_ADDRESS" } ], "trustedCa": { @@ -148,6 +166,24 @@ "exact": "foo.com" }, "sanType": "URI" + }, + { + "matcher": { + "exact": "foo.com" + }, + "sanType": "DNS" + }, + { + "matcher": { + "exact": "foo.com" + }, + "sanType": "EMAIL" + }, + { + "matcher": { + "exact": "foo.com" + }, + "sanType": "IP_ADDRESS" } ], "trustedCa": { diff --git a/agent/xds/testdata/clusters/transparent-proxy-terminating-gateway-destinations-only.latest.golden b/agent/xds/testdata/clusters/transparent-proxy-terminating-gateway-destinations-only.latest.golden index 9f531929c7..ff66e6cc86 100644 --- a/agent/xds/testdata/clusters/transparent-proxy-terminating-gateway-destinations-only.latest.golden +++ b/agent/xds/testdata/clusters/transparent-proxy-terminating-gateway-destinations-only.latest.golden @@ -172,6 +172,24 @@ "exact": "api.test.com" }, "sanType": "URI" + }, + { + "matcher": { + "exact": "api.test.com" + }, + "sanType": "DNS" + }, + { + "matcher": { + "exact": "api.test.com" + }, + "sanType": "EMAIL" + }, + { + "matcher": { + "exact": "api.test.com" + }, + "sanType": "IP_ADDRESS" } ], "trustedCa": { diff --git a/agent/xds/testdata/rbac/v2-default-allow--httpfilter.golden b/agent/xds/testdata/rbac/v2-default-allow--httpfilter.golden index 0967ef424b..9e26dfeeb6 100644 --- a/agent/xds/testdata/rbac/v2-default-allow--httpfilter.golden +++ b/agent/xds/testdata/rbac/v2-default-allow--httpfilter.golden @@ -1 +1 @@ -{} +{} \ No newline at end of file diff --git a/agent/xds/testdata/rbac/v2-default-deny--httpfilter.golden b/agent/xds/testdata/rbac/v2-default-deny--httpfilter.golden index 290edfd0c5..a435b1f2b8 100644 --- a/agent/xds/testdata/rbac/v2-default-deny--httpfilter.golden +++ b/agent/xds/testdata/rbac/v2-default-deny--httpfilter.golden @@ -4,4 +4,4 @@ "@type": "type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC", "rules": {} } -} +} \ No newline at end of file diff --git a/agent/xds/testdata/rbac/v2-ignore-empty-permissions--httpfilter.golden b/agent/xds/testdata/rbac/v2-ignore-empty-permissions--httpfilter.golden index 536b0e04f7..f947a2d0be 100644 --- a/agent/xds/testdata/rbac/v2-ignore-empty-permissions--httpfilter.golden +++ b/agent/xds/testdata/rbac/v2-ignore-empty-permissions--httpfilter.golden @@ -17,4 +17,4 @@ } } ] -} +} \ No newline at end of file diff --git a/agent/xds/testdata/rbac/v2-kitchen-sink--httpfilter.golden b/agent/xds/testdata/rbac/v2-kitchen-sink--httpfilter.golden index a9c458e230..6bc1fb6e5f 100644 --- a/agent/xds/testdata/rbac/v2-kitchen-sink--httpfilter.golden +++ b/agent/xds/testdata/rbac/v2-kitchen-sink--httpfilter.golden @@ -111,4 +111,4 @@ } } ] -} +} \ No newline at end of file diff --git a/agent/xds/xds_protocol_helpers_test.go b/agent/xds/xds_protocol_helpers_test.go index 655e0458f4..4b7fb90f4b 100644 --- a/agent/xds/xds_protocol_helpers_test.go +++ b/agent/xds/xds_protocol_helpers_test.go @@ -38,7 +38,7 @@ import ( "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/xds/response" "github.com/hashicorp/consul/envoyextensions/xdscommon" - "github.com/hashicorp/consul/internal/mesh/proxy-snapshot" + proxysnapshot "github.com/hashicorp/consul/internal/mesh/proxy-snapshot" "github.com/hashicorp/consul/proto-public/pbresource" "github.com/hashicorp/consul/sdk/testutil" ) @@ -295,7 +295,7 @@ func xdsNewTransportSocket( }, } if len(spiffeID) > 0 { - require.NoError(t, injectSANMatcher(commonTLSContext, spiffeID...)) + require.NoError(t, injectSANMatcher(commonTLSContext, false, spiffeID...)) } var tlsContext proto.Message