diff --git a/.changelog/12672.txt b/.changelog/12672.txt new file mode 100644 index 0000000000..449890850a --- /dev/null +++ b/.changelog/12672.txt @@ -0,0 +1,3 @@ +```release-note:security +connect: Properly set SNI when configured for services behind a terminating gateway. +``` diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index e87e9eceda..2e72f992ee 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -89,6 +89,14 @@ func (c *ConfigEntry) Apply(args *structs.ConfigEntryRequest, reply *bool) error return err } + // Log any applicable warnings about the contents of the config entry. + if warnEntry, ok := args.Entry.(structs.WarningConfigEntry); ok { + warnings := warnEntry.Warnings() + for _, warning := range warnings { + c.logger.Warn(warning) + } + } + if err := args.Entry.CanWrite(authz); err != nil { return err } diff --git a/agent/proxycfg/testing_terminating_gateway.go b/agent/proxycfg/testing_terminating_gateway.go index e66ef33996..60f27ca843 100644 --- a/agent/proxycfg/testing_terminating_gateway.go +++ b/agent/proxycfg/testing_terminating_gateway.go @@ -526,6 +526,30 @@ func testConfigSnapshotTerminatingGatewayLBConfig(t testing.T, variant string) * }) } +func TestConfigSnapshotTerminatingGatewaySNI(t testing.T) *ConfigSnapshot { + return TestConfigSnapshotTerminatingGateway(t, true, nil, []cache.UpdateEvent{ + { + CorrelationID: "gateway-services", + Result: &structs.IndexedGatewayServices{ + Services: []*structs.GatewayService{ + { + Service: structs.NewServiceName("web", nil), + CAFile: "ca.cert.pem", + SNI: "foo.com", + }, + { + Service: structs.NewServiceName("api", nil), + CAFile: "ca.cert.pem", + CertFile: "api.cert.pem", + KeyFile: "api.key.pem", + SNI: "bar.com", + }, + }, + }, + }, + }) +} + func TestConfigSnapshotTerminatingGatewayHostnameSubsets(t testing.T) *ConfigSnapshot { var ( api = structs.NewServiceName("api", nil) diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 7222a1ec61..09e05fa4ca 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -82,6 +82,14 @@ type UpdatableConfigEntry interface { ConfigEntry } +// WarningConfigEntry is an optional interface implemented by a ConfigEntry +// if it wants to be able to emit warnings when it is being upserted. +type WarningConfigEntry interface { + Warnings() []string + + ConfigEntry +} + // ServiceConfiguration is the top-level struct for the configuration of a service // across the entire cluster. type ServiceConfigEntry struct { diff --git a/agent/structs/config_entry_gateways.go b/agent/structs/config_entry_gateways.go index 94014230d5..fc9c840a06 100644 --- a/agent/structs/config_entry_gateways.go +++ b/agent/structs/config_entry_gateways.go @@ -570,6 +570,22 @@ func (e *TerminatingGatewayConfigEntry) GetEnterpriseMeta() *EnterpriseMeta { return &e.EnterpriseMeta } +func (e *TerminatingGatewayConfigEntry) Warnings() []string { + if e == nil { + return nil + } + + warnings := make([]string, 0) + for _, svc := range e.Services { + if (svc.CAFile != "" || svc.CertFile != "" || svc.KeyFile != "") && svc.SNI == "" { + warning := fmt.Sprintf("TLS is configured but SNI is not set for service %q. Enabling SNI is strongly recommended when using TLS.", svc.Name) + warnings = append(warnings, warning) + } + } + + return warnings +} + // GatewayService is used to associate gateways with their linked services. type GatewayService struct { Gateway ServiceName diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 4c98550367..63442eb516 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -390,6 +390,9 @@ func (s *ResourceGenerator) injectGatewayServiceAddons(cfgSnap *proxycfg.ConfigS } if mapping.SNI != "" { tlsContext.Sni = mapping.SNI + if err := injectRawSANMatcher(tlsContext.CommonTlsContext, []string{mapping.SNI}); err != nil { + return fmt.Errorf("failed to inject SNI matcher into TLS context: %v", err) + } } transportSocket, err := makeUpstreamTLSTransportSocket(tlsContext) @@ -803,6 +806,15 @@ func (s *ResourceGenerator) makeUpstreamClustersForDiscoveryChain( // injectSANMatcher updates a TLS context so that it verifies the upstream SAN. func injectSANMatcher(tlsContext *envoy_tls_v3.CommonTlsContext, spiffeIDs ...connect.SpiffeIDService) error { + var matchStrings []string + for _, id := range spiffeIDs { + matchStrings = append(matchStrings, id.URI().String()) + } + + return injectRawSANMatcher(tlsContext, matchStrings) +} + +func injectRawSANMatcher(tlsContext *envoy_tls_v3.CommonTlsContext, matchStrings []string) error { validationCtx, ok := tlsContext.ValidationContextType.(*envoy_tls_v3.CommonTlsContext_ValidationContext) if !ok { return fmt.Errorf("invalid type: expected CommonTlsContext_ValidationContext, got %T", @@ -810,10 +822,10 @@ func injectSANMatcher(tlsContext *envoy_tls_v3.CommonTlsContext, spiffeIDs ...co } var matchers []*envoy_matcher_v3.StringMatcher - for _, id := range spiffeIDs { + for _, m := range matchStrings { matchers = append(matchers, &envoy_matcher_v3.StringMatcher{ MatchPattern: &envoy_matcher_v3.StringMatcher_Exact{ - Exact: id.URI().String(), + Exact: m, }, }) } diff --git a/agent/xds/clusters_test.go b/agent/xds/clusters_test.go index c8a1e98e5d..8f4156aed7 100644 --- a/agent/xds/clusters_test.go +++ b/agent/xds/clusters_test.go @@ -581,6 +581,10 @@ func TestClustersFromSnapshot(t *testing.T) { name: "terminating-gateway-hostname-service-subsets", create: proxycfg.TestConfigSnapshotTerminatingGatewayHostnameSubsets, }, + { + name: "terminating-gateway-sni", + create: proxycfg.TestConfigSnapshotTerminatingGatewaySNI, + }, { name: "terminating-gateway-ignore-extra-resolvers", create: proxycfg.TestConfigSnapshotTerminatingGatewayIgnoreExtraResolvers, diff --git a/agent/xds/testdata/clusters/terminating-gateway-sni.envoy-1-20-x.golden b/agent/xds/testdata/clusters/terminating-gateway-sni.envoy-1-20-x.golden new file mode 100644 index 0000000000..5b784eabc7 --- /dev/null +++ b/agent/xds/testdata/clusters/terminating-gateway-sni.envoy-1-20-x.golden @@ -0,0 +1,174 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "name": "api.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "LOGICAL_DNS", + "connectTimeout": "5s", + "loadAssignment": { + "clusterName": "api.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "api.altdomain", + "portValue": 8081 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, + "dnsRefreshRate": "10s", + "dnsLookupFamily": "V4_ONLY", + "outlierDetection": { + + }, + "transportSocket": { + "name": "tls", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext", + "commonTlsContext": { + "tlsParams": { + + }, + "tlsCertificates": [ + { + "certificateChain": { + "filename": "api.cert.pem" + }, + "privateKey": { + "filename": "api.key.pem" + } + } + ], + "validationContext": { + "trustedCa": { + "filename": "ca.cert.pem" + }, + "matchSubjectAltNames": [ + { + "exact": "bar.com" + } + ] + } + }, + "sni": "bar.com" + } + } + }, + { + "@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "name": "cache.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "LOGICAL_DNS", + "connectTimeout": "5s", + "loadAssignment": { + "clusterName": "cache.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "cache.mydomain", + "portValue": 8081 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, + "dnsRefreshRate": "10s", + "dnsLookupFamily": "V4_ONLY", + "outlierDetection": { + + } + }, + { + "@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "name": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "LOGICAL_DNS", + "connectTimeout": "5s", + "loadAssignment": { + "clusterName": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "db.mydomain", + "portValue": 8081 + } + } + }, + "healthStatus": "UNHEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, + "dnsRefreshRate": "10s", + "dnsLookupFamily": "V4_ONLY", + "outlierDetection": { + + } + }, + { + "@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "name": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "EDS", + "edsClusterConfig": { + "edsConfig": { + "ads": { + + }, + "resourceApiVersion": "V3" + } + }, + "connectTimeout": "5s", + "outlierDetection": { + + }, + "transportSocket": { + "name": "tls", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext", + "commonTlsContext": { + "tlsParams": { + + }, + "validationContext": { + "trustedCa": { + "filename": "ca.cert.pem" + }, + "matchSubjectAltNames": [ + { + "exact": "foo.com" + } + ] + } + }, + "sni": "foo.com" + } + } + } + ], + "typeUrl": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "nonce": "00000001" +} \ No newline at end of file diff --git a/website/content/docs/connect/config-entries/terminating-gateway.mdx b/website/content/docs/connect/config-entries/terminating-gateway.mdx index 8da3b20e6c..80966017a4 100644 --- a/website/content/docs/connect/config-entries/terminating-gateway.mdx +++ b/website/content/docs/connect/config-entries/terminating-gateway.mdx @@ -30,6 +30,9 @@ from the terminating gateway will be encrypted using one-way TLS authentication. and [private key](/docs/connect/config-entries/terminating-gateway#keyfile) are also specified connections from the terminating gateway will be encrypted using mutual TLS authentication. +~> Setting the `SNI` field is strongly recommended when enabling TLS to a service. If this field is not set, +Consul will not attempt to verify the Subject Alternative Name fields in the service's certificate. + If none of these are provided, Consul will **only** encrypt connections to the gateway and not from the gateway to the destination service. @@ -163,6 +166,7 @@ Services = [ { Name = "billing" CAFile = "/etc/certs/ca-chain.cert.pem" + SNI = "billing.service.com" } ] ``` @@ -176,6 +180,7 @@ spec: services: - name: billing caFile: /etc/certs/ca-chain.cert.pem + sni: billing.service.com ``` ```json @@ -186,6 +191,7 @@ spec: { "Name": "billing", "CAFile": "/etc/certs/ca-chain.cert.pem" + "SNI": "billing.service.com" } ] } @@ -214,6 +220,7 @@ Services = [ Namespace = "finance" Name = "billing" CAFile = "/etc/certs/ca-chain.cert.pem" + SNI = "billing.service.com" } ] ``` @@ -228,6 +235,7 @@ spec: - name: billing namespace: finance caFile: /etc/certs/ca-chain.cert.pem + sni: billing.service.com ``` ```json @@ -239,7 +247,8 @@ spec: { "Namespace": "finance", "Name": "billing", - "CAFile": "/etc/certs/ca-chain.cert.pem" + "CAFile": "/etc/certs/ca-chain.cert.pem", + "SNI": "billing.service.com" } ] } @@ -273,6 +282,7 @@ Services = [ CAFile = "/etc/certs/ca-chain.cert.pem" KeyFile = "/etc/certs/gateway.key.pem" CertFile = "/etc/certs/gateway.cert.pem" + SNI = "billing.service.com" } ] ``` @@ -288,6 +298,7 @@ spec: caFile: /etc/certs/ca-chain.cert.pem keyFile: /etc/certs/gateway.key.pem certFile: /etc/certs/gateway.cert.pem + sni: billing.service.com ``` ```json @@ -299,7 +310,8 @@ spec: "Name": "billing", "CAFile": "/etc/certs/ca-chain.cert.pem", "KeyFile": "/etc/certs/gateway.key.pem", - "CertFile": "/etc/certs/gateway.cert.pem" + "CertFile": "/etc/certs/gateway.cert.pem", + "SNI": "billing.service.com" } ] } @@ -330,6 +342,7 @@ Services = [ CAFile = "/etc/certs/ca-chain.cert.pem" KeyFile = "/etc/certs/gateway.key.pem" CertFile = "/etc/certs/gateway.cert.pem" + SNI = "billing.service.com" } ] ``` @@ -346,6 +359,7 @@ spec: caFile: /etc/certs/ca-chain.cert.pem keyFile: /etc/certs/gateway.key.pem certFile: /etc/certs/gateway.cert.pem + sni: billing.service.com ``` ```json @@ -359,7 +373,8 @@ spec: "Name": "billing", "CAFile": "/etc/certs/ca-chain.cert.pem", "KeyFile": "/etc/certs/gateway.key.pem", - "CertFile": "/etc/certs/gateway.cert.pem" + "CertFile": "/etc/certs/gateway.cert.pem", + "SNI": "billing.service.com" } ] } @@ -396,7 +411,7 @@ Services = [ }, { Name = "billing" - CAFile = "/etc/billing-ca/ca-chain.cert.pem", + CAFile = "/etc/billing-ca/ca-chain.cert.pem" SNI = "billing.service.com" } ]