Merge pull request #12672 from hashicorp/tgate-san-validation

Respect SNI with terminating gateways and log a warning if it isn't set alongside TLS

.changelog/12672.txt

@@ -0,0 +1,3 @@
+```release-note:security
+connect: Properly set SNI when configured for services behind a terminating gateway.
+```

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
 	}

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)

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 {

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

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

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,

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"
+}

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"
   }
 ]