Merge pull request #12747 from hashicorp/tgate-san-validate-1.10.x

backport/1.10.x: Use the GatewayService SNI field for upstream SAN validation
hashicorp · Apr 11, 2022 · 4657782 · 4657782
2 parents 5a60bd1 + 36e7920
commit 4657782
Show file tree

Hide file tree

Showing 9 changed files with 435 additions and 6 deletions.
diff --git a/.changelog/12672.txt 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
@@ -84,6 +84,13 @@ 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 authz != nil && !args.Entry.CanWrite(authz) {
 		return acl.ErrPermissionDenied
 	}

diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go
@@ -79,6 +79,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
@@ -425,6 +425,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
@@ -401,6 +401,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)
@@ -794,17 +797,26 @@ 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",
 			tlsContext.ValidationContextType)
 	}
 
 	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
@@ -563,6 +563,26 @@ func TestClustersFromSnapshot(t *testing.T) {
 				}
 			},
 		},
+		{
+			name:   "terminating-gateway-sni",
+			create: proxycfg.TestConfigSnapshotTerminatingGateway,
+			setup: func(snap *proxycfg.ConfigSnapshot) {
+				snap.TerminatingGateway.GatewayServices = map[structs.ServiceName]structs.GatewayService{
+					structs.NewServiceName("web", nil): {
+						Service: structs.NewServiceName("web", nil),
+						CAFile:  "ca.cert.pem",
+						SNI:     "foo.com",
+					},
+					structs.NewServiceName("api", nil): {
+						Service:  structs.NewServiceName("api", nil),
+						CAFile:   "ca.cert.pem",
+						CertFile: "api.cert.pem",
+						KeyFile:  "api.key.pem",
+						SNI:      "bar.com",
+					},
+				}
+			},
+		},
 		{
 			name:   "terminating-gateway-ignore-extra-resolvers",
 			create: proxycfg.TestConfigSnapshotTerminatingGateway,

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