Manual backport 1.15.x of Disable remote proxy patching except AWS Lambda

.changelog/17415.txt

@@ -0,0 +1,7 @@
+```release-note:security
+extensions: Disable remote downstream proxy patching by Envoy Extensions other than AWS Lambda. Previously, an operator with service:write ACL permissions for an upstream service could modify Envoy proxy config for downstream services without equivalent permissions for those services. This issue only impacts the Lua extension. [[CVE-2023-2816](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-2816)]
+```
+
+```release-note:breaking-change
+extensions: The Lua extension now targets local proxy listeners for the configured service's upstreams, rather than remote downstream listeners for the configured service, when ListenerType is set to outbound in extension configuration. See [CVE-2023-2816](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-2816) changelog entry for more details.
+```

agent/envoyextensions/builtin/aws-lambda/aws_lambda.go

@@ -39,7 +39,7 @@ func Constructor(ext api.EnvoyExtension) (extensioncommon.EnvoyExtender, error)
 	if err := a.fromArguments(ext.Arguments); err != nil {
 		return nil, err
 	}
-	return &extensioncommon.BasicEnvoyExtender{
+	return &extensioncommon.UpstreamEnvoyExtender{
 		Extension: &a,
 	}, nil
 }
@@ -62,7 +62,7 @@ func (a *awsLambda) validate() error {
 // CanApply returns true if the kind of the provided ExtensionConfiguration matches
 // the kind of the lambda configuration
 func (a *awsLambda) CanApply(config *extensioncommon.RuntimeConfig) bool {
-	return config.Kind == config.OutgoingProxyKind()
+	return config.Kind == config.UpstreamOutgoingProxyKind()
 }
 
 // PatchRoute modifies the routing configuration for a service of kind TerminatingGateway. If the kind is
@@ -72,6 +72,11 @@ func (a *awsLambda) PatchRoute(r *extensioncommon.RuntimeConfig, route *envoy_ro
 		return route, false, nil
 	}
 
+	// Only patch outbound routes.
+	if extensioncommon.IsRouteToLocalAppCluster(route) {
+		return route, false, nil
+	}
+
 	for _, virtualHost := range route.VirtualHosts {
 		for _, route := range virtualHost.Routes {
 			action, ok := route.Action.(*envoy_route_v3.Route_Route)
@@ -92,6 +97,11 @@ func (a *awsLambda) PatchRoute(r *extensioncommon.RuntimeConfig, route *envoy_ro
 
 // PatchCluster patches the provided envoy cluster with data required to support an AWS lambda function
 func (a *awsLambda) PatchCluster(_ *extensioncommon.RuntimeConfig, c *envoy_cluster_v3.Cluster) (*envoy_cluster_v3.Cluster, bool, error) {
+	// Only patch outbound clusters.
+	if extensioncommon.IsLocalAppCluster(c) {
+		return c, false, nil
+	}
+
 	transportSocket, err := makeUpstreamTLSTransportSocket(&envoy_tls_v3.UpstreamTlsContext{
 		Sni: "*.amazonaws.com",
 	})
@@ -153,7 +163,12 @@ func (a *awsLambda) PatchCluster(_ *extensioncommon.RuntimeConfig, c *envoy_clus
 
 // PatchFilter patches the provided envoy filter with an inserted lambda filter being careful not to
 // overwrite the http filters.
-func (a *awsLambda) PatchFilter(_ *extensioncommon.RuntimeConfig, filter *envoy_listener_v3.Filter) (*envoy_listener_v3.Filter, bool, error) {
+func (a *awsLambda) PatchFilter(_ *extensioncommon.RuntimeConfig, filter *envoy_listener_v3.Filter, isInboundListener bool) (*envoy_listener_v3.Filter, bool, error) {
+	// Only patch outbound filters.
+	if isInboundListener {
+		return filter, false, nil
+	}
+
 	if filter.Name != "envoy.filters.network.http_connection_manager" {
 		return filter, false, nil
 	}

agent/envoyextensions/builtin/aws-lambda/aws_lambda_test.go

@@ -83,7 +83,7 @@ func TestConstructor(t *testing.T) {
 
 			if tc.ok {
 				require.NoError(t, err)
-				require.Equal(t, &extensioncommon.BasicEnvoyExtender{Extension: &tc.expected}, e)
+				require.Equal(t, &extensioncommon.UpstreamEnvoyExtender{Extension: &tc.expected}, e)
 			} else {
 				require.Error(t, err)
 			}
@@ -320,10 +320,11 @@ func TestPatchFilter(t *testing.T) {
 		return v
 	}
 	tests := map[string]struct {
-		filter       *envoy_listener_v3.Filter
-		expectFilter *envoy_listener_v3.Filter
-		expectBool   bool
-		expectErr    string
+		filter          *envoy_listener_v3.Filter
+		isInboundFilter bool
+		expectFilter    *envoy_listener_v3.Filter
+		expectBool      bool
+		expectErr       string
 	}{
 		"invalid filter name is ignored": {
 			filter:       &envoy_listener_v3.Filter{Name: "something"},
@@ -413,6 +414,36 @@ func TestPatchFilter(t *testing.T) {
 			},
 			expectBool: true,
 		},
+		"inbound filter ignored": {
+			filter: &envoy_listener_v3.Filter{
+				Name: "envoy.filters.network.http_connection_manager",
+				ConfigType: &envoy_listener_v3.Filter_TypedConfig{
+					TypedConfig: makeAny(&envoy_http_v3.HttpConnectionManager{
+						HttpFilters: []*envoy_http_v3.HttpFilter{
+							{Name: "one"},
+							{Name: "two"},
+							{Name: "envoy.filters.http.router"},
+							{Name: "three"},
+						},
+					}),
+				},
+			},
+			expectFilter: &envoy_listener_v3.Filter{
+				Name: "envoy.filters.network.http_connection_manager",
+				ConfigType: &envoy_listener_v3.Filter_TypedConfig{
+					TypedConfig: makeAny(&envoy_http_v3.HttpConnectionManager{
+						HttpFilters: []*envoy_http_v3.HttpFilter{
+							{Name: "one"},
+							{Name: "two"},
+							{Name: "envoy.filters.http.router"},
+							{Name: "three"},
+						},
+					}),
+				},
+			},
+			isInboundFilter: true,
+			expectBool:      false,
+		},
 	}
 
 	for name, tc := range tests {
@@ -422,7 +453,7 @@ func TestPatchFilter(t *testing.T) {
 				PayloadPassthrough: true,
 				InvocationMode:     "asynchronous",
 			}
-			f, ok, err := l.PatchFilter(nil, tc.filter)
+			f, ok, err := l.PatchFilter(nil, tc.filter, tc.isInboundFilter)
 			require.Equal(t, tc.expectBool, ok)
 			if tc.expectErr == "" {
 				require.NoError(t, err)

agent/envoyextensions/builtin/lua/lua.go

@@ -61,11 +61,11 @@ func (l *lua) validate() error {
 
 // CanApply determines if the extension can apply to the given extension configuration.
 func (l *lua) CanApply(config *extensioncommon.RuntimeConfig) bool {
-	return string(config.Kind) == l.ProxyType && l.matchesListenerDirection(config)
+	return string(config.Kind) == l.ProxyType
 }
 
-func (l *lua) matchesListenerDirection(config *extensioncommon.RuntimeConfig) bool {
-	return (config.IsUpstream() && l.Listener == "outbound") || (!config.IsUpstream() && l.Listener == "inbound")
+func (l *lua) matchesListenerDirection(isInboundListener bool) bool {
+	return (!isInboundListener && l.Listener == "outbound") || (isInboundListener && l.Listener == "inbound")
 }
 
 // PatchRoute does nothing.
@@ -79,7 +79,12 @@ func (l *lua) PatchCluster(_ *extensioncommon.RuntimeConfig, c *envoy_cluster_v3
 }
 
 // PatchFilter inserts a lua filter directly prior to envoy.filters.http.router.
-func (l *lua) PatchFilter(_ *extensioncommon.RuntimeConfig, filter *envoy_listener_v3.Filter) (*envoy_listener_v3.Filter, bool, error) {
+func (l *lua) PatchFilter(_ *extensioncommon.RuntimeConfig, filter *envoy_listener_v3.Filter, isInboundListener bool) (*envoy_listener_v3.Filter, bool, error) {
+	// Make sure filter matches extension config.
+	if !l.matchesListenerDirection(isInboundListener) {
+		return filter, false, nil
+	}
+
 	if filter.Name != "envoy.filters.network.http_connection_manager" {
 		return filter, false, nil
 	}

agent/envoyextensions/registered_extensions_test.go

@@ -1,9 +1,11 @@
 package envoyextensions
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/envoyextensions/extensioncommon"
 	"github.com/stretchr/testify/require"
 )
 
@@ -58,3 +60,52 @@ func TestValidateExtensions(t *testing.T) {
 		})
 	}
 }
+
+// This test is included here so that we can test all registered extensions without creating a cyclic dependency between
+// envoyextensions and extensioncommon.
+func TestUpstreamExtenderLimitations(t *testing.T) {
+	type testCase struct {
+		config *extensioncommon.RuntimeConfig
+		ok     bool
+		errMsg string
+	}
+	unauthorizedExtensionCase := func(name string) testCase {
+		return testCase{
+			config: &extensioncommon.RuntimeConfig{
+				Kind:                  api.ServiceKindConnectProxy,
+				ServiceName:           api.CompoundServiceName{Name: "api"},
+				Upstreams:             map[api.CompoundServiceName]*extensioncommon.UpstreamData{},
+				IsSourcedFromUpstream: true,
+				EnvoyExtension: api.EnvoyExtension{
+					Name: name,
+				},
+			},
+			ok:     false,
+			errMsg: fmt.Sprintf("extension %q is not permitted to be applied via upstream service config", name),
+		}
+	}
+	cases := map[string]testCase{
+		// Make sure future extensions are theoretically covered, even if not registered in the same way.
+		"unknown extension": unauthorizedExtensionCase("someotherextension"),
+	}
+	for name := range extensionConstructors {
+		// AWS Lambda is the only extension permitted to modify downstream proxy resources.
+		if name == api.BuiltinAWSLambdaExtension {
+			continue
+		}
+		cases[name] = unauthorizedExtensionCase(name)
+	}
+
+	for n, tc := range cases {
+		t.Run(n, func(t *testing.T) {
+			extender := extensioncommon.UpstreamEnvoyExtender{}
+			_, err := extender.Extend(nil, tc.config)
+			if tc.ok {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				require.ErrorContains(t, err, tc.errMsg)
+			}
+		})
+	}
+}

agent/xds/delta_envoy_extender_oss_test.go

@@ -59,17 +59,16 @@ func TestEnvoyExtenderWithSnapshot(t *testing.T) {
 		}
 	}
 
-	makeLuaServiceDefaults := func(inbound bool) *structs.ServiceConfigEntry {
+	// Apply Lua extension to the local service and ensure http is used so the extension can be applied.
+	makeLuaNsFunc := func(inbound bool) func(ns *structs.NodeService) {
 		listener := "inbound"
 		if !inbound {
 			listener = "outbound"
 		}
 
-		return &structs.ServiceConfigEntry{
-			Kind:     structs.ServiceDefaults,
-			Name:     "db",
-			Protocol: "http",
-			EnvoyExtensions: []structs.EnvoyExtension{
+		return func(ns *structs.NodeService) {
+			ns.Proxy.Config["protocol"] = "http"
+			ns.Proxy.EnvoyExtensions = []structs.EnvoyExtension{
 				{
 					Name: api.BuiltinLuaExtension,
 					Arguments: map[string]interface{}{
@@ -81,7 +80,7 @@ function envoy_on_request(request_handle)
 end`,
 					},
 				},
-			},
+			}
 		}
 	}
 
@@ -127,57 +126,47 @@ end`,
 			create: proxycfg.TestConfigSnapshotTerminatingGatewayWithLambdaServiceAndServiceResolvers,
 		},
 		{
-			name: "lua-outbound-applies-to-upstreams",
+			name: "lua-outbound-applies-to-local-upstreams",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
-				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, nil, nil, makeLuaServiceDefaults(false))
+				// upstreams need to be http in order for lua to be applied to listeners.
+				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, makeLuaNsFunc(false), nil, &structs.ServiceConfigEntry{
+					Kind:     structs.ServiceDefaults,
+					Name:     "db",
+					Protocol: "http",
+				}, &structs.ServiceConfigEntry{
+					Kind:     structs.ServiceDefaults,
+					Name:     "geo-cache",
+					Protocol: "http",
+				})
 			},
 		},
 		{
-			name: "lua-inbound-doesnt-applies-to-upstreams",
+			// We expect an inbound public listener lua filter here because the extension targets inbound.
+			// The only difference between goldens for this and lua-inbound-applies-to-inbound
+			// should be that db has HTTP filters rather than TCP.
+			name: "lua-inbound-doesnt-apply-to-local-upstreams",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
-				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, nil, nil, makeLuaServiceDefaults(true))
+				// db is made an HTTP upstream so that the extension _could_ apply, but does not because
+				// the direction for the extension is inbound.
+				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, makeLuaNsFunc(true), nil, &structs.ServiceConfigEntry{
+					Kind:     structs.ServiceDefaults,
+					Name:     "db",
+					Protocol: "http",
+				})
 			},
 		},
 		{
 			name: "lua-inbound-applies-to-inbound",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
-				return proxycfg.TestConfigSnapshot(t, func(ns *structs.NodeService) {
-					ns.Proxy.Config["protocol"] = "http"
-					ns.Proxy.EnvoyExtensions = []structs.EnvoyExtension{
-						{
-							Name: api.BuiltinLuaExtension,
-							Arguments: map[string]interface{}{
-								"ProxyType": "connect-proxy",
-								"Listener":  "inbound",
-								"Script": `
-function envoy_on_request(request_handle)
-  request_handle:headers():add("test", "test")
-end`,
-							},
-						},
-					}
-				}, nil)
+				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, makeLuaNsFunc(true), nil)
 			},
 		},
 		{
+			// We expect _no_ lua filters here, because the extension targets outbound, but there are
+			// no upstream HTTP services. We also should not see public listener, which is HTTP, patched.
 			name: "lua-outbound-doesnt-apply-to-inbound",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
-				return proxycfg.TestConfigSnapshot(t, func(ns *structs.NodeService) {
-					ns.Proxy.Config["protocol"] = "http"
-					ns.Proxy.EnvoyExtensions = []structs.EnvoyExtension{
-						{
-							Name: api.BuiltinLuaExtension,
-							Arguments: map[string]interface{}{
-								"ProxyType": "connect-proxy",
-								"Listener":  "outbound",
-								"Script": `
-function envoy_on_request(request_handle)
-  request_handle:headers():add("test", "test")
-end`,
-							},
-						},
-					}
-				}, nil)
+				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, makeLuaNsFunc(false), nil)
 			},
 		},
 		{