status-im
/
consul
mirror of https://github.com/status-im/consul.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix SAN matching on terminating gateways (#20417) 

Fixes issue: hashicorp/consul#20360

A regression was introduced in hashicorp/consul#19954 where the SAN validation
matching was reduced from 4 potential types down to just the URI.

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` Envoy behavior, we should match on all 4
enum types.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#enum-extensions-transport-sockets-tls-v3-subjectaltnamematcher-santype
This commit is contained in:
Derek Menteer 2024-01-31 12:17:45 -06:00 committed by GitHub
parent 890332cacb
commit 3e8ec8d18e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 98 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/20417.txt Normal file
View File

@ -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)
```

47
agent/xds/clusters.go
View File

@ -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

2
agent/xds/failover_policy.go
View File

@ -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)
}

36
agent/xds/testdata/clusters/terminating-gateway-sni.latest.golden vendored
View File

@ -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": {

18
agent/xds/testdata/clusters/transparent-proxy-terminating-gateway-destinations-only.latest.golden vendored
View File

@ -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": {

2
agent/xds/testdata/rbac/v2-default-allow--httpfilter.golden vendored
View File

@ -1 +1 @@
{}
{}

2
agent/xds/testdata/rbac/v2-default-deny--httpfilter.golden vendored
View File

@ -4,4 +4,4 @@
"@type": "type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC",
"rules": {}
}
}
}

2
agent/xds/testdata/rbac/v2-ignore-empty-permissions--httpfilter.golden vendored
View File

@ -17,4 +17,4 @@
}
}
]
}
}

2
agent/xds/testdata/rbac/v2-kitchen-sink--httpfilter.golden vendored
View File

@ -111,4 +111,4 @@
}
}
]
}
}

4
agent/xds/xds_protocol_helpers_test.go
View File

@ -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