From eccdf8197757da4ef766a9394a14a76a58078e69 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Mon, 22 May 2023 16:36:29 -0500 Subject: [PATCH] xds: generate listeners directly from API gateway snapshot (#17398) * API Gateway XDS Primitives, endpoints and clusters (#17002) * XDS primitive generation for endpoints and clusters Co-authored-by: Nathan Coleman * server_test * deleted extra file * add missing parents to test --------- Co-authored-by: Nathan Coleman * Routes for API Gateway (#17158) * XDS primitive generation for endpoints and clusters Co-authored-by: Nathan Coleman * server_test * deleted extra file * add missing parents to test * checkpoint * delete extra file * httproute flattening code * linting issue * so close on this, calling for tonight * unit test passing * add in header manip to virtual host * upstream rebuild commented out * Use consistent upstream name whether or not we're rebuilding * Start working through route naming logic * Fix typos in test descriptions * Simplify route naming logic * Simplify RebuildHTTPRouteUpstream * Merge additional compiled discovery chains instead of overwriting * Use correct chain for flattened route, clean up + add TODOs * Remove empty conditional branch * Restore previous variable declaration Limit the scope of this PR * Clean up, improve TODO * add logging, clean up todos * clean up function --------- Co-authored-by: Nathan Coleman * checkpoint, skeleton, tests not passing * checkpoint * endpoints xds cluster configuration * resources test fix * fix reversion in resources_test * checkpoint * Update agent/proxycfg/api_gateway.go Co-authored-by: John Maguire * unit tests passing * gofmt * add deterministic sorting to appease the unit test gods * remove panic * Find ready upstream matching listener instead of first in list * Clean up, improve TODO * Modify getReadyUpstreams to filter upstreams by listener (#17410) Each listener would previously have all upstreams from any route that bound to the listener. This is problematic when a route bound to one listener also binds to other listeners and so includes upstreams for multiple listeners. The list for a given listener would then wind up including upstreams for other listeners. * clean up todos, references to api gateway in listeners_ingress * merge in Nathan's fix * Update agent/consul/discoverychain/gateway.go * cleanup current todos, remove snapshot manipulation from generation code * Update agent/structs/config_entry_gateways.go Co-authored-by: Thomas Eckert * Update agent/consul/discoverychain/gateway.go Co-authored-by: Nathan Coleman * Update agent/consul/discoverychain/gateway.go Co-authored-by: Nathan Coleman * Update agent/proxycfg/snapshot.go Co-authored-by: Nathan Coleman * clarified header comment for FlattenHTTPRoute, changed RebuildHTTPRouteUpstream to BuildHTTPRouteUpstream * simplify cert logic * Delete scratch * revert route related changes in listener PR * Update agent/consul/discoverychain/gateway.go * Update agent/proxycfg/snapshot.go * clean up uneeded extra lines in endpoints --------- Co-authored-by: Nathan Coleman Co-authored-by: John Maguire Co-authored-by: Thomas Eckert --- agent/consul/discoverychain/gateway.go | 33 ++- agent/consul/server_test.go | 2 +- agent/proxycfg/api_gateway.go | 2 +- agent/proxycfg/snapshot.go | 3 +- agent/structs/config_entry_gateways.go | 5 + agent/xds/listeners.go | 9 +- agent/xds/listeners_apigateway.go | 385 +++++++++++++++++++++++++ agent/xds/listeners_ingress.go | 146 +--------- 8 files changed, 421 insertions(+), 164 deletions(-) create mode 100644 agent/xds/listeners_apigateway.go diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index c1c6a2b08..054e892e0 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -59,9 +59,13 @@ func (l *GatewayChainSynthesizer) SetHostname(hostname string) { // single hostname can be specified in multiple routes. Routing for a given // hostname must behave based on the aggregate of all rules that apply to it. func (l *GatewayChainSynthesizer) AddHTTPRoute(route structs.HTTPRouteConfigEntry) { - hostnames := route.FilteredHostnames(l.hostname) + l.matchesByHostname = getHostMatches(l.hostname, &route, l.matchesByHostname) +} + +func getHostMatches(hostname string, route *structs.HTTPRouteConfigEntry, currentMatches map[string][]hostnameMatch) map[string][]hostnameMatch { + hostnames := route.FilteredHostnames(hostname) for _, host := range hostnames { - matches, ok := l.matchesByHostname[host] + matches, ok := currentMatches[host] if !ok { matches = []hostnameMatch{} } @@ -90,8 +94,9 @@ func (l *GatewayChainSynthesizer) AddHTTPRoute(route structs.HTTPRouteConfigEntr } } - l.matchesByHostname[host] = matches + currentMatches[host] = matches } + return currentMatches } // Synthesize assembles a synthetic discovery chain from multiple other discovery chains @@ -116,6 +121,7 @@ func (l *GatewayChainSynthesizer) Synthesize(chains ...*structs.CompiledDiscover compiledChains := make([]*structs.CompiledDiscoveryChain, 0, len(set)) for i, service := range services { + entries := set[i] compiled, err := Compile(CompileRequest{ @@ -126,7 +132,6 @@ func (l *GatewayChainSynthesizer) Synthesize(chains ...*structs.CompiledDiscover EvaluateInTrustDomain: l.trustDomain, Entries: entries, }) - if err != nil { return nil, nil, err } @@ -188,17 +193,23 @@ func (l *GatewayChainSynthesizer) Synthesize(chains ...*structs.CompiledDiscover // consolidateHTTPRoutes combines all rules into the shortest possible list of routes // with one route per hostname containing all rules for that hostname. func (l *GatewayChainSynthesizer) consolidateHTTPRoutes() []structs.HTTPRouteConfigEntry { + return consolidateHTTPRoutes(l.matchesByHostname, l.suffix, l.gateway) +} + +// consolidateHTTPRoutes combines all rules into the shortest possible list of routes +// with one route per hostname containing all rules for that hostname. +func consolidateHTTPRoutes(matchesByHostname map[string][]hostnameMatch, suffix string, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { var routes []structs.HTTPRouteConfigEntry - for hostname, rules := range l.matchesByHostname { + for hostname, rules := range matchesByHostname { // Create route for this hostname route := structs.HTTPRouteConfigEntry{ Kind: structs.HTTPRoute, - Name: fmt.Sprintf("%s-%s-%s", l.gateway.Name, l.suffix, hostsKey(hostname)), + Name: fmt.Sprintf("%s-%s-%s", gateway.Name, suffix, hostsKey(hostname)), Hostnames: []string{hostname}, Rules: make([]structs.HTTPRouteRule, 0, len(rules)), - Meta: l.gateway.Meta, - EnterpriseMeta: l.gateway.EnterpriseMeta, + Meta: gateway.Meta, + EnterpriseMeta: gateway.EnterpriseMeta, } // Sort rules for this hostname in order of precedence @@ -258,12 +269,14 @@ func (l *GatewayChainSynthesizer) synthesizeEntries() ([]structs.IngressService, entries := []*configentry.DiscoveryChainSet{} for _, route := range l.consolidateHTTPRoutes() { - entrySet := configentry.NewDiscoveryChainSet() ingress, router, splitters, defaults := synthesizeHTTPRouteDiscoveryChain(route) + + services = append(services, ingress) + + entrySet := configentry.NewDiscoveryChainSet() entrySet.AddRouters(router) entrySet.AddSplitters(splitters...) entrySet.AddServices(defaults...) - services = append(services, ingress) entries = append(entries, entrySet) } diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index c246428b2..bd39e9676 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -1906,7 +1906,7 @@ func TestServer_ReloadConfig(t *testing.T) { defaults := DefaultConfig() got := s.raft.ReloadableConfig() require.Equal(t, uint64(4321), got.SnapshotThreshold, - "should have be reloaded to new value") + "should have been reloaded to new value") require.Equal(t, defaults.RaftConfig.SnapshotInterval, got.SnapshotInterval, "should have remained the default interval") require.Equal(t, defaults.RaftConfig.TrailingLogs, got.TrailingLogs, diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index a97312a7a..21631bd5f 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -6,7 +6,6 @@ package proxycfg import ( "context" "fmt" - "github.com/hashicorp/consul/acl" cachetype "github.com/hashicorp/consul/agent/cache-types" "github.com/hashicorp/consul/agent/proxycfg/internal/watch" @@ -445,6 +444,7 @@ func (h *handlerAPIGateway) recompileDiscoveryChains(snap *ConfigSnapshot) error for i, service := range services { id := NewUpstreamIDFromServiceName(structs.NewServiceName(service.Name, &service.EnterpriseMeta)) + if compiled[i].ServiceName != service.Name { return fmt.Errorf("Compiled Discovery chain for %s does not match service %s", compiled[i].ServiceName, id) } diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 49e3195bc..1baa155d5 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -736,6 +736,7 @@ type configSnapshotAPIGateway struct { // entry to save us trying to pass fields through Upstreams Listeners map[string]structs.APIGatewayListener // this acts as an intermediary for inlining certificates + // FUTURE(nathancoleman) Remove when ToIngress is removed ListenerCertificates map[IngressListenerKey][]structs.InlineCertificateConfigEntry BoundListeners map[string]structs.BoundAPIGatewayListener @@ -745,7 +746,7 @@ type configSnapshotAPIGateway struct { // This is temporary, for the sake of re-using existing codepaths when integrating // Consul API Gateway into Consul core. // -// FUTURE: Remove when API gateways have custom snapshot generation +// FUTURE(nathancoleman): Remove when API gateways have custom snapshot generation func (c *configSnapshotAPIGateway) ToIngress(datacenter string) (configSnapshotIngressGateway, error) { // Convert API Gateway Listeners to Ingress Listeners. ingressListeners := make(map[IngressListenerKey]structs.IngressListener, len(c.Listeners)) diff --git a/agent/structs/config_entry_gateways.go b/agent/structs/config_entry_gateways.go index cd66a0ae3..0be410d19 100644 --- a/agent/structs/config_entry_gateways.go +++ b/agent/structs/config_entry_gateways.go @@ -912,6 +912,11 @@ type APIGatewayTLSConfiguration struct { CipherSuites []types.TLSCipherSuite } +// IsEmpty returns true if all values in the struct are nil or empty. +func (a *APIGatewayTLSConfiguration) IsEmpty() bool { + return len(a.Certificates) == 0 && len(a.MaxVersion) == 0 && len(a.MinVersion) == 0 && len(a.CipherSuites) == 0 +} + // BoundAPIGatewayConfigEntry manages the configuration for a bound API // gateway with the given name. This type is never written from the client. // It is only written by the controller in order to represent an API gateway diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 645fe9594..2c8bb9715 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -860,16 +860,11 @@ func (s *ResourceGenerator) listenersFromSnapshotGateway(cfgSnap *proxycfg.Confi return nil, err } case structs.ServiceKindAPIGateway: - // TODO Find a cleaner solution, can't currently pass unexported property types - var err error - cfgSnap.IngressGateway, err = cfgSnap.APIGateway.ToIngress(cfgSnap.Datacenter) - if err != nil { - return nil, err - } - listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) + listeners, err := s.makeAPIGatewayListeners(a.Address, cfgSnap) if err != nil { return nil, err } + resources = append(resources, listeners...) case structs.ServiceKindIngressGateway: listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go new file mode 100644 index 000000000..ed6cc6a9e --- /dev/null +++ b/agent/xds/listeners_apigateway.go @@ -0,0 +1,385 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package xds + +import ( + "fmt" + envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + envoy_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" + envoy_tls_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" + + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/wrapperspb" + + "github.com/hashicorp/consul/agent/proxycfg" + "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/types" +) + +func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { + var resources []proto.Message + + readyUpstreamsList := getReadyUpstreams(cfgSnap) + + for _, readyUpstreams := range readyUpstreamsList { + listenerCfg := readyUpstreams.listenerCfg + listenerKey := readyUpstreams.listenerKey + boundListener := readyUpstreams.boundListenerCfg + + var certs []structs.InlineCertificateConfigEntry + for _, certRef := range boundListener.Certificates { + cert, ok := cfgSnap.APIGateway.Certificates.Get(certRef) + if !ok { + continue + } + certs = append(certs, *cert) + } + + isAPIGatewayWithTLS := len(boundListener.Certificates) > 0 + + tlsContext, err := makeDownstreamTLSContextFromSnapshotAPIListenerConfig(cfgSnap, listenerCfg) + if err != nil { + return nil, err + } + + if listenerKey.Protocol == "tcp" { + // Find the upstream matching this listener + + // We rely on the invariant of upstreams slice always having at least 1 + // member, because this key/value pair is created only when a + // GatewayService is returned in the RPC + u := readyUpstreams.upstreams[0] + uid := proxycfg.NewUpstreamID(&u) + + chain := cfgSnap.APIGateway.DiscoveryChain[uid] + if chain == nil { + // Wait until a chain is present in the snapshot. + continue + } + + cfg := s.getAndModifyUpstreamConfigForListener(uid, &u, chain) + useRDS := cfg.Protocol != "tcp" && !chain.Default + + var clusterName string + if !useRDS { + // When not using RDS we must generate a cluster name to attach to the filter chain. + // With RDS, cluster names get attached to the dynamic routes instead. + target, err := simpleChainTarget(chain) + if err != nil { + return nil, err + } + clusterName = CustomizeClusterName(target.Name, chain) + } + + filterName := fmt.Sprintf("%s.%s.%s.%s", chain.ServiceName, chain.Namespace, chain.Partition, chain.Datacenter) + + opts := makeListenerOpts{ + name: uid.EnvoyID(), + accessLogs: cfgSnap.Proxy.AccessLogs, + addr: address, + port: u.LocalBindPort, + direction: envoy_core_v3.TrafficDirection_OUTBOUND, + logger: s.Logger, + } + l := makeListener(opts) + + filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ + accessLogs: &cfgSnap.Proxy.AccessLogs, + routeName: uid.EnvoyID(), + useRDS: useRDS, + clusterName: clusterName, + filterName: filterName, + protocol: cfg.Protocol, + tlsContext: tlsContext, + }) + if err != nil { + return nil, err + } + l.FilterChains = []*envoy_listener_v3.FilterChain{ + filterChain, + } + + if isAPIGatewayWithTLS { + // construct SNI filter chains + l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.APIGateway.TLSConfig, listenerKey.Protocol, listenerFilterOpts{ + useRDS: useRDS, + protocol: listenerKey.Protocol, + routeName: listenerKey.RouteName(), + cluster: clusterName, + statPrefix: "ingress_upstream_", + accessLogs: &cfgSnap.Proxy.AccessLogs, + logger: s.Logger, + }, certs) + if err != nil { + return nil, err + } + + // add the tls inspector to do SNI introspection + tlsInspector, err := makeTLSInspectorListenerFilter() + if err != nil { + return nil, err + } + l.ListenerFilters = []*envoy_listener_v3.ListenerFilter{tlsInspector} + } + resources = append(resources, l) + + } else { + // If multiple upstreams share this port, make a special listener for the protocol. + listenerOpts := makeListenerOpts{ + name: listenerKey.Protocol, + accessLogs: cfgSnap.Proxy.AccessLogs, + addr: address, + port: listenerKey.Port, + direction: envoy_core_v3.TrafficDirection_OUTBOUND, + logger: s.Logger, + } + listener := makeListener(listenerOpts) + filterOpts := listenerFilterOpts{ + useRDS: true, + protocol: listenerKey.Protocol, + filterName: listenerKey.RouteName(), + routeName: listenerKey.RouteName(), + cluster: "", + statPrefix: "ingress_upstream_", + routePath: "", + httpAuthzFilters: nil, + accessLogs: &cfgSnap.Proxy.AccessLogs, + logger: s.Logger, + } + + // Generate any filter chains needed for services with custom TLS certs + // via SDS. + sniFilterChains := []*envoy_listener_v3.FilterChain{} + + if isAPIGatewayWithTLS { + sniFilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey.Protocol, filterOpts, certs) + if err != nil { + return nil, err + } + } + + // If there are any sni filter chains, we need a TLS inspector filter! + if len(sniFilterChains) > 0 { + tlsInspector, err := makeTLSInspectorListenerFilter() + if err != nil { + return nil, err + } + listener.ListenerFilters = []*envoy_listener_v3.ListenerFilter{tlsInspector} + } + + listener.FilterChains = sniFilterChains + + // See if there are other services that didn't have specific SNI-matching + // filter chains. If so add a default filterchain to serve them. + if len(sniFilterChains) < len(readyUpstreams.upstreams) && !isAPIGatewayWithTLS { + defaultFilter, err := makeListenerFilter(filterOpts) + if err != nil { + return nil, err + } + + transportSocket, err := makeDownstreamTLSTransportSocket(tlsContext) + if err != nil { + return nil, err + } + listener.FilterChains = append(listener.FilterChains, + &envoy_listener_v3.FilterChain{ + Filters: []*envoy_listener_v3.Filter{ + defaultFilter, + }, + TransportSocket: transportSocket, + }) + } + resources = append(resources, listener) + } + } + + return resources, nil +} + +func makeDownstreamTLSContextFromSnapshotAPIListenerConfig(cfgSnap *proxycfg.ConfigSnapshot, listenerCfg structs.APIGatewayListener) (*envoy_tls_v3.DownstreamTlsContext, error) { + var downstreamContext *envoy_tls_v3.DownstreamTlsContext + + tlsContext, err := makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap, listenerCfg) + if err != nil { + return nil, err + } + + if tlsContext != nil { + // Configure alpn protocols on TLSContext + tlsContext.AlpnProtocols = getAlpnProtocols(string(listenerCfg.Protocol)) + + downstreamContext = &envoy_tls_v3.DownstreamTlsContext{ + CommonTlsContext: tlsContext, + RequireClientCertificate: &wrapperspb.BoolValue{Value: false}, + } + } + + return downstreamContext, nil +} + +func makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap *proxycfg.ConfigSnapshot, listenerCfg structs.APIGatewayListener) (*envoy_tls_v3.CommonTlsContext, error) { + var tlsContext *envoy_tls_v3.CommonTlsContext + + //API Gateway TLS config is per listener + tlsCfg, err := resolveAPIListenerTLSConfig(listenerCfg.TLS) + if err != nil { + return nil, err + } + + connectTLSEnabled := (!listenerCfg.TLS.IsEmpty()) + + if tlsCfg.SDS != nil { + // Set up listener TLS from SDS + tlsContext = makeCommonTLSContextFromGatewayTLSConfig(*tlsCfg) + } else if connectTLSEnabled { + tlsContext = makeCommonTLSContext(cfgSnap.Leaf(), cfgSnap.RootPEMs(), makeTLSParametersFromGatewayTLSConfig(*tlsCfg)) + } + + return tlsContext, nil +} + +func resolveAPIListenerTLSConfig(listenerTLSCfg structs.APIGatewayTLSConfiguration) (*structs.GatewayTLSConfig, error) { + var mergedCfg structs.GatewayTLSConfig + + if !listenerTLSCfg.IsEmpty() { + if listenerTLSCfg.MinVersion != types.TLSVersionUnspecified { + mergedCfg.TLSMinVersion = listenerTLSCfg.MinVersion + } + if listenerTLSCfg.MaxVersion != types.TLSVersionUnspecified { + mergedCfg.TLSMaxVersion = listenerTLSCfg.MaxVersion + } + if len(listenerTLSCfg.CipherSuites) != 0 { + mergedCfg.CipherSuites = listenerTLSCfg.CipherSuites + } + } + + if err := validateListenerTLSConfig(mergedCfg.TLSMinVersion, mergedCfg.CipherSuites); err != nil { + return nil, err + } + + return &mergedCfg, nil +} + +func routeNameForAPIGatewayUpstream(l structs.IngressListener, s structs.IngressService) string { + key := proxycfg.IngressListenerKeyFromListener(l) + + // If the upstream service doesn't have any TLS overrides then it can just use + // the combined filterchain with all the merged routes. + if !ingressServiceHasSDSOverrides(s) { + return key.RouteName() + } + + // Return a specific route for this service as it needs a custom FilterChain + // to serve its custom cert so we should attach its routes to a separate Route + // too. We need this to be consistent between OSS and Enterprise to avoid xDS + // config golden files in tests conflicting so we can't use ServiceID.String() + // which normalizes to included all identifiers in Enterprise. + sn := s.ToServiceName() + svcIdentifier := sn.Name + if !sn.InDefaultPartition() || !sn.InDefaultNamespace() { + // Non-default partition/namespace, use a full identifier + svcIdentifier = sn.String() + } + return fmt.Sprintf("%s_%s", key.RouteName(), svcIdentifier) +} + +// when we have multiple certificates on a single listener, we need +// to duplicate the filter chains with multiple TLS contexts +func makeInlineOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, + tlsCfg structs.GatewayTLSConfig, + protocol string, + filterOpts listenerFilterOpts, + certs []structs.InlineCertificateConfigEntry) ([]*envoy_listener_v3.FilterChain, error) { + + var chains []*envoy_listener_v3.FilterChain + + constructChain := func(name string, hosts []string, tlsContext *envoy_tls_v3.CommonTlsContext) error { + filterOpts.filterName = name + filter, err := makeListenerFilter(filterOpts) + if err != nil { + return err + } + + // Configure alpn protocols on TLSContext + tlsContext.AlpnProtocols = getAlpnProtocols(protocol) + transportSocket, err := makeDownstreamTLSTransportSocket(&envoy_tls_v3.DownstreamTlsContext{ + CommonTlsContext: tlsContext, + RequireClientCertificate: &wrapperspb.BoolValue{Value: false}, + }) + if err != nil { + return err + } + + chains = append(chains, &envoy_listener_v3.FilterChain{ + FilterChainMatch: makeSNIFilterChainMatch(hosts...), + Filters: []*envoy_listener_v3.Filter{ + filter, + }, + TransportSocket: transportSocket, + }) + + return nil + } + + multipleCerts := len(certs) > 1 + + allCertHosts := map[string]struct{}{} + overlappingHosts := map[string]struct{}{} + + if multipleCerts { + // we only need to prune out overlapping hosts if we have more than + // one certificate + for _, cert := range certs { + hosts, err := cert.Hosts() + if err != nil { + return nil, fmt.Errorf("unable to parse hosts from x509 certificate: %v", hosts) + } + for _, host := range hosts { + if _, ok := allCertHosts[host]; ok { + overlappingHosts[host] = struct{}{} + } + allCertHosts[host] = struct{}{} + } + } + } + + for _, cert := range certs { + var hosts []string + + // if we only have one cert, we just use it for all ingress + if multipleCerts { + // otherwise, we need an SNI per cert and to fallback to our ingress + // gateway certificate signed by our Consul CA + certHosts, err := cert.Hosts() + if err != nil { + return nil, fmt.Errorf("unable to parse hosts from x509 certificate: %v", hosts) + } + // filter out any overlapping hosts so we don't have collisions in our filter chains + for _, host := range certHosts { + if _, ok := overlappingHosts[host]; !ok { + hosts = append(hosts, host) + } + } + + if len(hosts) == 0 { + // all of our hosts are overlapping, so we just skip this filter and it'll be + // handled by the default filter chain + continue + } + } + + if err := constructChain(cert.Name, hosts, makeInlineTLSContextFromGatewayTLSConfig(tlsCfg, cert)); err != nil { + return nil, err + } + } + + if multipleCerts { + // if we have more than one cert, add a default handler that uses the leaf cert from connect + if err := constructChain("default", nil, makeCommonTLSContext(cfgSnap.Leaf(), cfgSnap.RootPEMs(), makeTLSParametersFromGatewayTLSConfig(tlsCfg))); err != nil { + return nil, err + } + } + + return chains, nil +} diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index c16a1b74c..ea4c311cd 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -29,15 +29,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap return nil, fmt.Errorf("no listener config found for listener on proto/port %s/%d", listenerKey.Protocol, listenerKey.Port) } - var isAPIGatewayWithTLS bool - var certs []structs.InlineCertificateConfigEntry - if cfgSnap.APIGateway.ListenerCertificates != nil { - certs = cfgSnap.APIGateway.ListenerCertificates[listenerKey] - } - if certs != nil { - isAPIGatewayWithTLS = true - } - tlsContext, err := makeDownstreamTLSContextFromSnapshotListenerConfig(cfgSnap, listenerCfg) if err != nil { return nil, err @@ -102,28 +93,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap filterChain, } - if isAPIGatewayWithTLS { - // construct SNI filter chains - l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, listenerFilterOpts{ - useRDS: useRDS, - protocol: listenerKey.Protocol, - routeName: listenerKey.RouteName(), - cluster: clusterName, - statPrefix: "ingress_upstream_", - accessLogs: &cfgSnap.Proxy.AccessLogs, - logger: s.Logger, - }, certs) - if err != nil { - return nil, err - } - // add the tls inspector to do SNI introspection - tlsInspector, err := makeTLSInspectorListenerFilter() - if err != nil { - return nil, err - } - l.ListenerFilters = []*envoy_listener_v3.ListenerFilter{tlsInspector} - } - resources = append(resources, l) } else { // If multiple upstreams share this port, make a special listener for the protocol. @@ -135,6 +104,7 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap direction: envoy_core_v3.TrafficDirection_OUTBOUND, logger: s.Logger, } + listener := makeListener(listenerOpts) filterOpts := listenerFilterOpts{ @@ -157,13 +127,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap return nil, err } - if isAPIGatewayWithTLS { - sniFilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, filterOpts, certs) - if err != nil { - return nil, err - } - } - // If there are any sni filter chains, we need a TLS inspector filter! if len(sniFilterChains) > 0 { tlsInspector, err := makeTLSInspectorListenerFilter() @@ -177,7 +140,7 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap // See if there are other services that didn't have specific SNI-matching // filter chains. If so add a default filterchain to serve them. - if len(sniFilterChains) < len(upstreams) && !isAPIGatewayWithTLS { + if len(sniFilterChains) < len(upstreams) { defaultFilter, err := makeListenerFilter(filterOpts) if err != nil { return nil, err @@ -422,111 +385,6 @@ func makeSDSOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, return chains, nil } -// when we have multiple certificates on a single listener, we need -// to duplicate the filter chains with multiple TLS contexts -func makeInlineOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, - tlsCfg structs.GatewayTLSConfig, - listenerKey proxycfg.IngressListenerKey, - filterOpts listenerFilterOpts, - certs []structs.InlineCertificateConfigEntry) ([]*envoy_listener_v3.FilterChain, error) { - - listenerCfg, ok := cfgSnap.IngressGateway.Listeners[listenerKey] - if !ok { - return nil, fmt.Errorf("no listener config found for listener on port %d", listenerKey.Port) - } - - var chains []*envoy_listener_v3.FilterChain - - constructChain := func(name string, hosts []string, tlsContext *envoy_tls_v3.CommonTlsContext) error { - filterOpts.filterName = name - filter, err := makeListenerFilter(filterOpts) - if err != nil { - return err - } - - // Configure alpn protocols on TLSContext - tlsContext.AlpnProtocols = getAlpnProtocols(listenerCfg.Protocol) - transportSocket, err := makeDownstreamTLSTransportSocket(&envoy_tls_v3.DownstreamTlsContext{ - CommonTlsContext: tlsContext, - RequireClientCertificate: &wrapperspb.BoolValue{Value: false}, - }) - if err != nil { - return err - } - - chains = append(chains, &envoy_listener_v3.FilterChain{ - FilterChainMatch: makeSNIFilterChainMatch(hosts...), - Filters: []*envoy_listener_v3.Filter{ - filter, - }, - TransportSocket: transportSocket, - }) - - return nil - } - - multipleCerts := len(certs) > 1 - - allCertHosts := map[string]struct{}{} - overlappingHosts := map[string]struct{}{} - - if multipleCerts { - // we only need to prune out overlapping hosts if we have more than - // one certificate - for _, cert := range certs { - hosts, err := cert.Hosts() - if err != nil { - return nil, fmt.Errorf("unable to parse hosts from x509 certificate: %v", hosts) - } - for _, host := range hosts { - if _, ok := allCertHosts[host]; ok { - overlappingHosts[host] = struct{}{} - } - allCertHosts[host] = struct{}{} - } - } - } - - for _, cert := range certs { - var hosts []string - - // if we only have one cert, we just use it for all ingress - if multipleCerts { - // otherwise, we need an SNI per cert and to fallback to our ingress - // gateway certificate signed by our Consul CA - certHosts, err := cert.Hosts() - if err != nil { - return nil, fmt.Errorf("unable to parse hosts from x509 certificate: %v", hosts) - } - // filter out any overlapping hosts so we don't have collisions in our filter chains - for _, host := range certHosts { - if _, ok := overlappingHosts[host]; !ok { - hosts = append(hosts, host) - } - } - - if len(hosts) == 0 { - // all of our hosts are overlapping, so we just skip this filter and it'll be - // handled by the default filter chain - continue - } - } - - if err := constructChain(cert.Name, hosts, makeInlineTLSContextFromGatewayTLSConfig(tlsCfg, cert)); err != nil { - return nil, err - } - } - - if multipleCerts { - // if we have more than one cert, add a default handler that uses the leaf cert from connect - if err := constructChain("default", nil, makeCommonTLSContext(cfgSnap.Leaf(), cfgSnap.RootPEMs(), makeTLSParametersFromGatewayTLSConfig(tlsCfg))); err != nil { - return nil, err - } - } - - return chains, nil -} - func makeTLSParametersFromGatewayTLSConfig(tlsCfg structs.GatewayTLSConfig) *envoy_tls_v3.TlsParameters { return makeTLSParametersFromTLSConfig(tlsCfg.TLSMinVersion, tlsCfg.TLSMaxVersion, tlsCfg.CipherSuites) }