From a6d3fe90b173575da5da6c63ba59f95c3fa0ac0f Mon Sep 17 00:00:00 2001 From: freddygv Date: Mon, 28 Jun 2021 19:58:12 -0600 Subject: [PATCH] Validate Subject Alternative Name for upstreams These changes ensure that the identity of services dialed is cryptographically verified. For all upstreams we validate against SPIFFE IDs in the format used by Consul's service mesh: spiffe:///ns//dc//svc/ --- agent/xds/clusters.go | 55 ++++++++++++++++++++++++-- agent/xds/xds_protocol_helpers_test.go | 51 +++++++++++++++--------- 2 files changed, 84 insertions(+), 22 deletions(-) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 3bcb1a76f1..26209d9cf4 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -9,6 +9,7 @@ import ( envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" envoy_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3" envoy_tls_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" + envoy_matcher_v3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" envoy_type_v3 "github.com/envoyproxy/go-control-plane/envoy/type/v3" "github.com/golang/protobuf/jsonpb" @@ -528,9 +529,22 @@ func (s *ResourceGenerator) makeUpstreamClusterForPreparedQuery(upstream structs } } + spiffeID := connect.SpiffeIDService{ + Host: cfgSnap.Roots.TrustDomain, + Namespace: upstream.DestinationNamespace, + Datacenter: dc, + Service: upstream.DestinationName, + } + // Enable TLS upstream with the configured client certificate. + commonTLSContext := makeCommonTLSContextFromLeaf(cfgSnap, cfgSnap.Leaf()) + err = injectSANMatcher(commonTLSContext, spiffeID.URI().String()) + if err != nil { + return nil, fmt.Errorf("failed to inject SAN matcher rules for cluster %q: %v", sni, err) + } + tlsContext := &envoy_tls_v3.UpstreamTlsContext{ - CommonTlsContext: makeCommonTLSContextFromLeaf(cfgSnap, cfgSnap.Leaf()), + CommonTlsContext: commonTLSContext, Sni: sni, } @@ -598,6 +612,13 @@ func (s *ResourceGenerator) makeUpstreamClustersForDiscoveryChain( sni := target.SNI clusterName := CustomizeClusterName(target.Name, chain) + spiffeID := connect.SpiffeIDService{ + Host: cfgSnap.Roots.TrustDomain, + Namespace: target.Namespace, + Datacenter: target.Datacenter, + Service: target.Service, + } + if failoverThroughMeshGateway { actualTargetID := firstHealthyTarget( chain.Targets, @@ -609,6 +630,13 @@ func (s *ResourceGenerator) makeUpstreamClustersForDiscoveryChain( if actualTargetID != targetID { actualTarget := chain.Targets[actualTargetID] sni = actualTarget.SNI + + spiffeID = connect.SpiffeIDService{ + Host: cfgSnap.Roots.TrustDomain, + Namespace: actualTarget.Namespace, + Datacenter: actualTarget.Datacenter, + Service: actualTarget.Service, + } } } @@ -658,9 +686,14 @@ func (s *ResourceGenerator) makeUpstreamClustersForDiscoveryChain( c.Http2ProtocolOptions = &envoy_core_v3.Http2ProtocolOptions{} } - // Enable TLS upstream with the configured client certificate. + commonTLSContext := makeCommonTLSContextFromLeaf(cfgSnap, cfgSnap.Leaf()) + err = injectSANMatcher(commonTLSContext, spiffeID.URI().String()) + if err != nil { + return nil, fmt.Errorf("failed to inject SAN matcher rules for cluster %q: %v", sni, err) + } + tlsContext := &envoy_tls_v3.UpstreamTlsContext{ - CommonTlsContext: makeCommonTLSContextFromLeaf(cfgSnap, cfgSnap.Leaf()), + CommonTlsContext: commonTLSContext, Sni: sni, } @@ -688,6 +721,22 @@ func (s *ResourceGenerator) makeUpstreamClustersForDiscoveryChain( return out, nil } +// injectSANMatcher updates a TLS context so that it verifies the upstream SAN. +func injectSANMatcher(tlsContext *envoy_tls_v3.CommonTlsContext, uri string) error { + validationCtx, ok := tlsContext.ValidationContextType.(*envoy_tls_v3.CommonTlsContext_ValidationContext) + if !ok { + return fmt.Errorf("invalid type: expected CommonTlsContext_ValidationContext, got %T", + tlsContext.ValidationContextType) + } + + validationCtx.ValidationContext.MatchSubjectAltNames = []*envoy_matcher_v3.StringMatcher{ + { + MatchPattern: &envoy_matcher_v3.StringMatcher_Exact{Exact: uri}, + }, + } + return nil +} + // makeClusterFromUserConfig returns the listener config decoded from an // arbitrary proto3 json format string or an error if it's invalid. // diff --git a/agent/xds/xds_protocol_helpers_test.go b/agent/xds/xds_protocol_helpers_test.go index 3837c930b7..734f3e36b9 100644 --- a/agent/xds/xds_protocol_helpers_test.go +++ b/agent/xds/xds_protocol_helpers_test.go @@ -242,15 +242,16 @@ func xdsNewPublicTransportSocket( t *testing.T, snap *proxycfg.ConfigSnapshot, ) *envoy_core_v3.TransportSocket { - return xdsNewTransportSocket(t, snap, true, true, "") + return xdsNewTransportSocket(t, snap, true, true, "", "") } func xdsNewUpstreamTransportSocket( t *testing.T, snap *proxycfg.ConfigSnapshot, sni string, + uri string, ) *envoy_core_v3.TransportSocket { - return xdsNewTransportSocket(t, snap, false, false, sni) + return xdsNewTransportSocket(t, snap, false, false, sni, uri) } func xdsNewTransportSocket( @@ -259,11 +260,12 @@ func xdsNewTransportSocket( downstream bool, requireClientCert bool, sni string, + uri string, ) *envoy_core_v3.TransportSocket { // Assume just one root for now, can get fancier later if needed. caPEM := snap.Roots.Roots[0].RootCert - commonTlsContext := &envoy_tls_v3.CommonTlsContext{ + commonTLSContext := &envoy_tls_v3.CommonTlsContext{ TlsParams: &envoy_tls_v3.TlsParameters{}, TlsCertificates: []*envoy_tls_v3.TlsCertificate{{ CertificateChain: xdsNewInlineString(snap.Leaf().CertPEM), @@ -275,6 +277,9 @@ func xdsNewTransportSocket( }, }, } + if uri != "" { + require.NoError(t, injectSANMatcher(commonTLSContext, uri)) + } var tlsContext proto.Message if downstream { @@ -284,12 +289,12 @@ func xdsNewTransportSocket( } tlsContext = &envoy_tls_v3.DownstreamTlsContext{ - CommonTlsContext: commonTlsContext, + CommonTlsContext: commonTLSContext, RequireClientCertificate: requireClientCertPB, } } else { tlsContext = &envoy_tls_v3.UpstreamTlsContext{ - CommonTlsContext: commonTlsContext, + CommonTlsContext: commonTLSContext, Sni: sni, } } @@ -356,6 +361,14 @@ func makeTestResource(t *testing.T, raw interface{}) *envoy_discovery_v3.Resourc } func makeTestCluster(t *testing.T, snap *proxycfg.ConfigSnapshot, fixtureName string) *envoy_cluster_v3.Cluster { + var ( + dbSNI = "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + dbURI = "spiffe://11111111-2222-3333-4444-555555555555.consul/ns/default/dc/dc1/svc/db" + + geocacheSNI = "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul" + geocacheURI = "spiffe://11111111-2222-3333-4444-555555555555.consul/ns/default/dc/dc1/svc/geo-cache" + ) + switch fixtureName { case "tcp:local_app": return &envoy_cluster_v3.Cluster{ @@ -375,7 +388,7 @@ func makeTestCluster(t *testing.T, snap *proxycfg.ConfigSnapshot, fixtureName st } case "tcp:db": return &envoy_cluster_v3.Cluster{ - Name: "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + Name: dbSNI, ClusterDiscoveryType: &envoy_cluster_v3.Cluster_Type{ Type: envoy_cluster_v3.Cluster_EDS, }, @@ -384,16 +397,16 @@ func makeTestCluster(t *testing.T, snap *proxycfg.ConfigSnapshot, fixtureName st }, CircuitBreakers: &envoy_cluster_v3.CircuitBreakers{}, OutlierDetection: &envoy_cluster_v3.OutlierDetection{}, - AltStatName: "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + AltStatName: dbSNI, CommonLbConfig: &envoy_cluster_v3.Cluster_CommonLbConfig{ HealthyPanicThreshold: &envoy_type_v3.Percent{Value: 0}, }, ConnectTimeout: ptypes.DurationProto(5 * time.Second), - TransportSocket: xdsNewUpstreamTransportSocket(t, snap, "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"), + TransportSocket: xdsNewUpstreamTransportSocket(t, snap, dbSNI, dbURI), } case "tcp:db:timeout": return &envoy_cluster_v3.Cluster{ - Name: "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + Name: dbSNI, ClusterDiscoveryType: &envoy_cluster_v3.Cluster_Type{ Type: envoy_cluster_v3.Cluster_EDS, }, @@ -402,16 +415,16 @@ func makeTestCluster(t *testing.T, snap *proxycfg.ConfigSnapshot, fixtureName st }, CircuitBreakers: &envoy_cluster_v3.CircuitBreakers{}, OutlierDetection: &envoy_cluster_v3.OutlierDetection{}, - AltStatName: "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + AltStatName: dbSNI, CommonLbConfig: &envoy_cluster_v3.Cluster_CommonLbConfig{ HealthyPanicThreshold: &envoy_type_v3.Percent{Value: 0}, }, ConnectTimeout: ptypes.DurationProto(1337 * time.Second), - TransportSocket: xdsNewUpstreamTransportSocket(t, snap, "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"), + TransportSocket: xdsNewUpstreamTransportSocket(t, snap, dbSNI, dbURI), } case "http2:db": return &envoy_cluster_v3.Cluster{ - Name: "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + Name: dbSNI, ClusterDiscoveryType: &envoy_cluster_v3.Cluster_Type{ Type: envoy_cluster_v3.Cluster_EDS, }, @@ -420,17 +433,17 @@ func makeTestCluster(t *testing.T, snap *proxycfg.ConfigSnapshot, fixtureName st }, CircuitBreakers: &envoy_cluster_v3.CircuitBreakers{}, OutlierDetection: &envoy_cluster_v3.OutlierDetection{}, - AltStatName: "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + AltStatName: dbSNI, CommonLbConfig: &envoy_cluster_v3.Cluster_CommonLbConfig{ HealthyPanicThreshold: &envoy_type_v3.Percent{Value: 0}, }, ConnectTimeout: ptypes.DurationProto(5 * time.Second), - TransportSocket: xdsNewUpstreamTransportSocket(t, snap, "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"), + TransportSocket: xdsNewUpstreamTransportSocket(t, snap, dbSNI, dbURI), Http2ProtocolOptions: &envoy_core_v3.Http2ProtocolOptions{}, } case "http:db": return &envoy_cluster_v3.Cluster{ - Name: "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + Name: dbSNI, ClusterDiscoveryType: &envoy_cluster_v3.Cluster_Type{ Type: envoy_cluster_v3.Cluster_EDS, }, @@ -439,17 +452,17 @@ func makeTestCluster(t *testing.T, snap *proxycfg.ConfigSnapshot, fixtureName st }, CircuitBreakers: &envoy_cluster_v3.CircuitBreakers{}, OutlierDetection: &envoy_cluster_v3.OutlierDetection{}, - AltStatName: "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + AltStatName: dbSNI, CommonLbConfig: &envoy_cluster_v3.Cluster_CommonLbConfig{ HealthyPanicThreshold: &envoy_type_v3.Percent{Value: 0}, }, ConnectTimeout: ptypes.DurationProto(5 * time.Second), - TransportSocket: xdsNewUpstreamTransportSocket(t, snap, "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"), + TransportSocket: xdsNewUpstreamTransportSocket(t, snap, dbSNI, dbURI), // HttpProtocolOptions: &envoy_core_v3.Http1ProtocolOptions{}, } case "tcp:geo-cache": return &envoy_cluster_v3.Cluster{ - Name: "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul", + Name: geocacheSNI, ClusterDiscoveryType: &envoy_cluster_v3.Cluster_Type{ Type: envoy_cluster_v3.Cluster_EDS, }, @@ -459,7 +472,7 @@ func makeTestCluster(t *testing.T, snap *proxycfg.ConfigSnapshot, fixtureName st CircuitBreakers: &envoy_cluster_v3.CircuitBreakers{}, OutlierDetection: &envoy_cluster_v3.OutlierDetection{}, ConnectTimeout: ptypes.DurationProto(5 * time.Second), - TransportSocket: xdsNewUpstreamTransportSocket(t, snap, "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul"), + TransportSocket: xdsNewUpstreamTransportSocket(t, snap, geocacheSNI, geocacheURI), } default: t.Fatalf("unexpected fixture name: %s", fixtureName)