fix panic when Connect mesh gateway doesn't have a proxy block (#11257)

Co-authored-by: Michael Schurter <mschurter@hashicorp.com>
hashicorp · Oct 4, 2021 · 3c1aaf9 · 3c1aaf9
1 parent cbb7bfd
commit 3c1aaf9
Show file tree

Hide file tree

Showing 4 changed files with 139 additions and 84 deletions.
diff --git a/.changelog/11257.txt b/.changelog/11257.txt
@@ -0,0 +1,3 @@
+```release-note:security
+consul/connect: Fixed a bug causing the Nomad agent to panic if a mesh gateway was registered without a `proxy` block.
+```
diff --git a/command/agent/consul/service_client.go b/command/agent/consul/service_client.go
@@ -1100,26 +1100,32 @@ func (c *ServiceClient) serviceRegs(ops *operations, service *structs.Service, w
 		kind = api.ServiceKindIngressGateway
 	case service.Connect.IsTerminating():
 		kind = api.ServiceKindTerminatingGateway
-		// set the default port if bridge / default listener set
-		if defaultBind, exists := service.Connect.Gateway.Proxy.EnvoyGatewayBindAddresses["default"]; exists {
-			portLabel := envoy.PortLabel(structs.ConnectTerminatingPrefix, service.Name, "")
-			if dynPort, ok := workload.Ports.Get(portLabel); ok {
-				defaultBind.Port = dynPort.Value
+
+		if proxy := service.Connect.Gateway.Proxy; proxy != nil {
+			// set the default port if bridge / default listener set
+			if defaultBind, exists := proxy.EnvoyGatewayBindAddresses["default"]; exists {
+				portLabel := envoy.PortLabel(structs.ConnectTerminatingPrefix, service.Name, "")
+				if dynPort, ok := workload.Ports.Get(portLabel); ok {
+					defaultBind.Port = dynPort.Value
+				}
 			}
 		}
 	case service.Connect.IsMesh():
 		kind = api.ServiceKindMeshGateway
-		// wan uses the service port label, which is typically on a discrete host_network
-		if wanBind, exists := service.Connect.Gateway.Proxy.EnvoyGatewayBindAddresses["wan"]; exists {
-			if wanPort, ok := workload.Ports.Get(service.PortLabel); ok {
-				wanBind.Port = wanPort.Value
+
+		if proxy := service.Connect.Gateway.Proxy; proxy != nil {
+			// wan uses the service port label, which is typically on a discrete host_network
+			if wanBind, exists := proxy.EnvoyGatewayBindAddresses["wan"]; exists {
+				if wanPort, ok := workload.Ports.Get(service.PortLabel); ok {
+					wanBind.Port = wanPort.Value
+				}
 			}
-		}
-		// lan uses a nomad generated dynamic port on the default network
-		if lanBind, exists := service.Connect.Gateway.Proxy.EnvoyGatewayBindAddresses["lan"]; exists {
-			portLabel := envoy.PortLabel(structs.ConnectMeshPrefix, service.Name, "lan")
-			if dynPort, ok := workload.Ports.Get(portLabel); ok {
-				lanBind.Port = dynPort.Value
+			// lan uses a nomad generated dynamic port on the default network
+			if lanBind, exists := proxy.EnvoyGatewayBindAddresses["lan"]; exists {
+				portLabel := envoy.PortLabel(structs.ConnectMeshPrefix, service.Name, "lan")
+				if dynPort, ok := workload.Ports.Get(portLabel); ok {
+					lanBind.Port = dynPort.Value
+				}
 			}
 		}
 	}

diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go
@@ -279,16 +279,10 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error {
 			// a name of an injected gateway task
 			service.Name = env.ReplaceEnv(service.Name)
 
-			// detect whether the group is in host networking mode, which will
-			// require tweaking the default gateway task config
-			netHost := g.Networks[0].Mode == "host"
-
-			if !netHost && service.Connect.IsGateway() {
-				// Modify the gateway proxy service configuration to automatically
-				// do the correct envoy bind address plumbing when inside a net
-				// namespace, but only if things are not explicitly configured.
-				service.Connect.Gateway.Proxy = gatewayProxyForBridge(service.Connect.Gateway)
-			}
+			// Generate a proxy configuration, if one is not provided, that is
+			// most appropriate for the network mode being used.
+			netMode := g.Networks[0].Mode
+			service.Connect.Gateway.Proxy = gatewayProxy(service.Connect.Gateway, netMode)
 
 			// Inject a port whether bridge or host network (if not already set).
 			// This port is accessed by the magic of Connect plumbing so it seems
@@ -317,6 +311,10 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error {
 			// inject the gateway task only if it does not yet already exist
 			if !hasGatewayTaskForService(g, service.Name) {
 				prefix := service.Connect.Gateway.Prefix()
+
+				// detect whether the group is in host networking mode, which will
+				// require tweaking the default gateway task config
+				netHost := netMode == "host"
 				task := newConnectGatewayTask(prefix, service.Name, netHost)
 				g.Tasks = append(g.Tasks, task)
 
@@ -356,10 +354,10 @@ func gatewayProxyIsDefault(proxy *structs.ConsulGatewayProxy) bool {
 	return false
 }
 
-// gatewayProxyForBridge scans an existing gateway proxy configuration and tweaks
-// it given an associated configuration entry so that it works as intended from
-// inside a network namespace.
-func gatewayProxyForBridge(gateway *structs.ConsulGateway) *structs.ConsulGatewayProxy {
+// gatewayProxy scans an existing gateway proxy configuration and tweaks it
+// given an associated configuration entry so that it works as intended with
+// the network mode specified.
+func gatewayProxy(gateway *structs.ConsulGateway, mode string) *structs.ConsulGatewayProxy {
 	if gateway == nil {
 		return nil
 	}
@@ -383,40 +381,42 @@ func gatewayProxyForBridge(gateway *structs.ConsulGateway) *structs.ConsulGatewa
 		proxy.ConnectTimeout = helper.TimeToPtr(defaultConnectTimeout)
 	}
 
-	// magically configure bind address(es) for bridge networking, per gateway type
-	// non-default configuration is gated above
-	switch {
-	case gateway.Ingress != nil:
-		proxy.EnvoyGatewayNoDefaultBind = true
-		proxy.EnvoyGatewayBindTaggedAddresses = false
-		proxy.EnvoyGatewayBindAddresses = gatewayBindAddressesIngress(gateway.Ingress)
-	case gateway.Terminating != nil:
-		proxy.EnvoyGatewayNoDefaultBind = true
-		proxy.EnvoyGatewayBindTaggedAddresses = false
-		proxy.EnvoyGatewayBindAddresses = map[string]*structs.ConsulGatewayBindAddress{
-			"default": {
-				Address: "0.0.0.0",
-				Port:    -1, // filled in later with dynamic port
-			}}
-	case gateway.Mesh != nil:
-		proxy.EnvoyGatewayNoDefaultBind = true
-		proxy.EnvoyGatewayBindTaggedAddresses = false
-		proxy.EnvoyGatewayBindAddresses = map[string]*structs.ConsulGatewayBindAddress{
-			"wan": {
-				Address: "0.0.0.0",
-				Port:    -1, // filled in later with configured port
-			},
-			"lan": {
-				Address: "0.0.0.0",
-				Port:    -1, // filled in later with generated port
-			},
+	if mode == "bridge" {
+		// magically configure bind address(es) for bridge networking, per gateway type
+		// non-default configuration is gated above
+		switch {
+		case gateway.Ingress != nil:
+			proxy.EnvoyGatewayNoDefaultBind = true
+			proxy.EnvoyGatewayBindTaggedAddresses = false
+			proxy.EnvoyGatewayBindAddresses = gatewayBindAddressesIngressForBridge(gateway.Ingress)
+		case gateway.Terminating != nil:
+			proxy.EnvoyGatewayNoDefaultBind = true
+			proxy.EnvoyGatewayBindTaggedAddresses = false
+			proxy.EnvoyGatewayBindAddresses = map[string]*structs.ConsulGatewayBindAddress{
+				"default": {
+					Address: "0.0.0.0",
+					Port:    -1, // filled in later with dynamic port
+				}}
+		case gateway.Mesh != nil:
+			proxy.EnvoyGatewayNoDefaultBind = true
+			proxy.EnvoyGatewayBindTaggedAddresses = false
+			proxy.EnvoyGatewayBindAddresses = map[string]*structs.ConsulGatewayBindAddress{
+				"wan": {
+					Address: "0.0.0.0",
+					Port:    -1, // filled in later with configured port
+				},
+				"lan": {
+					Address: "0.0.0.0",
+					Port:    -1, // filled in later with generated port
+				},
+			}
 		}
 	}
 
 	return proxy
 }
 
-func gatewayBindAddressesIngress(ingress *structs.ConsulIngressConfigEntry) map[string]*structs.ConsulGatewayBindAddress {
+func gatewayBindAddressesIngressForBridge(ingress *structs.ConsulIngressConfigEntry) map[string]*structs.ConsulGatewayBindAddress {
 	if ingress == nil || len(ingress.Listeners) == 0 {
 		return make(map[string]*structs.ConsulGatewayBindAddress)
 	}