update ACLs for cluster peering (#15317)

* update ACLs for cluster peering

* add changelog

* Update .changelog/15317.txt

Co-authored-by: Eric Haberkorn <erichaberkorn@gmail.com>

Co-authored-by: Eric Haberkorn <erichaberkorn@gmail.com>

.changelog/15317.txt

@@ -0,0 +1,3 @@
+```release-note:improvements
+acl: Allow reading imported services and nodes from cluster peers with read all permissions
+```

acl/acl.go

@@ -20,6 +20,10 @@ type ExportFetcher interface {
 }
 
 type ExportedServices struct {
+	// Data is a map of [namespace] -> [service] -> [list of partitions the service is exported to]
+	// This includes both the names of typical service instances and their corresponding sidecar proxy
+	// instance names. Meaning that if "web" is exported, "web-sidecar-proxy" instances will also be
+	// shown as exported.
 	Data map[string]map[string][]string
 }
 

agent/consul/prepared_query_endpoint_test.go

@@ -1578,7 +1578,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 	execNoNodesToken := createTokenWithPolicyName(t, codec1, "no-nodes", `service_prefix "foo" { policy = "read" }`, "root")
 	rules := `
-		service_prefix "foo" { policy = "read" }
+		service_prefix "" { policy = "read" }
 		node_prefix "" { policy = "read" }
 	`
 	execToken := createTokenWithPolicyName(t, codec1, "with-read", rules, "root")

agent/structs/aclfilter/filter.go

@@ -142,6 +142,9 @@ func (f *Filter) Filter(subject any) {
 		if f.filterGatewayServices(&v.Gateways) {
 			v.QueryMeta.ResultsFilteredByACLs = true
 		}
+		if f.filterCheckServiceNodes(&v.ImportedNodes) {
+			v.QueryMeta.ResultsFilteredByACLs = true
+		}
 
 	default:
 		panic(fmt.Errorf("Unhandled type passed to ACL filter: %T %#v", subject, subject))
@@ -319,13 +322,11 @@ func (f *Filter) filterNodeServiceList(services *structs.NodeServiceList) bool {
 // true if any elements were removed.
 func (f *Filter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) bool {
 	csn := *nodes
-	var authzContext acl.AuthorizerContext
 	var removed bool
 
 	for i := 0; i < len(csn); i++ {
 		node := csn[i]
-		node.Service.FillAuthzContext(&authzContext)
+		if node.CanRead(f.authorizer) == acl.Allow {
-		if f.allowNode(node.Node.Node, &authzContext) && f.allowService(node.Service.Service, &authzContext) {
 			continue
 		}
 		f.logger.Debug("dropping node from result due to ACLs", "node", structs.NodeNameString(node.Node.Node, node.Node.GetEnterpriseMeta()))

agent/structs/aclfilter/filter_test.go

@@ -1107,12 +1107,51 @@ func TestACL_filterIndexedNodesWithGateways(t *testing.T) {
 				{Service: structs.ServiceNameFromString("foo")},
 				{Service: structs.ServiceNameFromString("bar")},
 			},
+			ImportedNodes: structs.CheckServiceNodes{
+				{
+					Node: &structs.Node{
+						Node:     "imported-node",
+						PeerName: "cluster-02",
+					},
+					Service: &structs.NodeService{
+						ID:       "zip",
+						Service:  "zip",
+						PeerName: "cluster-02",
+					},
+					Checks: structs.HealthChecks{
+						{
+							Node:        "node1",
+							CheckID:     "check1",
+							ServiceName: "zip",
+							PeerName:    "cluster-02",
+						},
+					},
+				},
+			},
 		}
 	}
 
-	t.Run("allowed", func(t *testing.T) {
+	type testCase struct {
+		authzFn func() acl.Authorizer
+		expect  *structs.IndexedNodesWithGateways
+	}
 
+	run := func(t *testing.T, tc testCase) {
+		authz := tc.authzFn()
+
+		list := makeList()
+		New(authz, logger).Filter(list)
+
+		require.Equal(t, tc.expect, list)
+	}
+
+	tt := map[string]testCase{
+		"not filtered": {
+			authzFn: func() acl.Authorizer {
 				policy, err := acl.NewPolicyFromSource(`
+			service "baz" {
+				policy = "write"
+			}
 			service "foo" {
 			  policy = "read"
 			}
@@ -1127,17 +1166,58 @@ func TestACL_filterIndexedNodesWithGateways(t *testing.T) {
 
 				authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
 				require.NoError(t, err)
-
+				return authz
-		list := makeList()
+			},
-		New(authz, logger).Filter(list)
+			expect: &structs.IndexedNodesWithGateways{
-
+				Nodes: structs.CheckServiceNodes{
-		require.Len(t, list.Nodes, 1)
+					{
-		require.Len(t, list.Gateways, 2)
+						Node: &structs.Node{
-		require.False(t, list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+							Node: "node1",
-	})
+						},
-
+						Service: &structs.NodeService{
-	t.Run("not allowed to read the node", func(t *testing.T) {
+							ID:      "foo",
-
+							Service: "foo",
+						},
+						Checks: structs.HealthChecks{
+							{
+								Node:        "node1",
+								CheckID:     "check1",
+								ServiceName: "foo",
+							},
+						},
+					},
+				},
+				Gateways: structs.GatewayServices{
+					{Service: structs.ServiceNameFromString("foo")},
+					{Service: structs.ServiceNameFromString("bar")},
+				},
+				// Service write to "bar" allows reading all imported services
+				ImportedNodes: structs.CheckServiceNodes{
+					{
+						Node: &structs.Node{
+							Node:     "imported-node",
+							PeerName: "cluster-02",
+						},
+						Service: &structs.NodeService{
+							ID:       "zip",
+							Service:  "zip",
+							PeerName: "cluster-02",
+						},
+						Checks: structs.HealthChecks{
+							{
+								Node:        "node1",
+								CheckID:     "check1",
+								ServiceName: "zip",
+								PeerName:    "cluster-02",
+							},
+						},
+					},
+				},
+				QueryMeta: structs.QueryMeta{ResultsFilteredByACLs: false},
+			},
+		},
+		"not allowed to read the node": {
+			authzFn: func() acl.Authorizer {
 				policy, err := acl.NewPolicyFromSource(`
 			service "foo" {
 			  policy = "read"
@@ -1150,17 +1230,20 @@ func TestACL_filterIndexedNodesWithGateways(t *testing.T) {
 
 				authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
 				require.NoError(t, err)
-
+				return authz
-		list := makeList()
+			},
-		New(authz, logger).Filter(list)
+			expect: &structs.IndexedNodesWithGateways{
-
+				Nodes: structs.CheckServiceNodes{},
-		require.Empty(t, list.Nodes)
+				Gateways: structs.GatewayServices{
-		require.Len(t, list.Gateways, 2)
+					{Service: structs.ServiceNameFromString("foo")},
-		require.True(t, list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+					{Service: structs.ServiceNameFromString("bar")},
-	})
+				},
-
+				ImportedNodes: structs.CheckServiceNodes{},
-	t.Run("allowed to read the node, but not the service", func(t *testing.T) {
+				QueryMeta:     structs.QueryMeta{ResultsFilteredByACLs: true},
-
+			},
+		},
+		"not allowed to read the service": {
+			authzFn: func() acl.Authorizer {
 				policy, err := acl.NewPolicyFromSource(`
 			node "node1" {
 			  policy = "read"
@@ -1173,17 +1256,19 @@ func TestACL_filterIndexedNodesWithGateways(t *testing.T) {
 
 				authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
 				require.NoError(t, err)
-
+				return authz
-		list := makeList()
+			},
-		New(authz, logger).Filter(list)
+			expect: &structs.IndexedNodesWithGateways{
-
+				Nodes: structs.CheckServiceNodes{},
-		require.Empty(t, list.Nodes)
+				Gateways: structs.GatewayServices{
-		require.Len(t, list.Gateways, 1)
+					{Service: structs.ServiceNameFromString("bar")},
-		require.True(t, list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+				},
-	})
+				ImportedNodes: structs.CheckServiceNodes{},
-
+				QueryMeta:     structs.QueryMeta{ResultsFilteredByACLs: true},
-	t.Run("not allowed to read the other gatway service", func(t *testing.T) {
+			},
-
+		},
+		"not allowed to read the other gateway service": {
+			authzFn: func() acl.Authorizer {
 				policy, err := acl.NewPolicyFromSource(`
 			service "foo" {
 			  policy = "read"
@@ -1196,24 +1281,50 @@ func TestACL_filterIndexedNodesWithGateways(t *testing.T) {
 
 				authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
 				require.NoError(t, err)
+				return authz
+			},
+			expect: &structs.IndexedNodesWithGateways{
+				Nodes: structs.CheckServiceNodes{
+					{
+						Node: &structs.Node{
+							Node: "node1",
+						},
+						Service: &structs.NodeService{
+							ID:      "foo",
+							Service: "foo",
+						},
+						Checks: structs.HealthChecks{
+							{
+								Node:        "node1",
+								CheckID:     "check1",
+								ServiceName: "foo",
+							},
+						},
+					},
+				},
+				Gateways: structs.GatewayServices{
+					{Service: structs.ServiceNameFromString("foo")},
+				},
+				ImportedNodes: structs.CheckServiceNodes{},
+				QueryMeta:     structs.QueryMeta{ResultsFilteredByACLs: true},
+			},
+		},
+		"denied": {
+			authzFn: acl.DenyAll,
+			expect: &structs.IndexedNodesWithGateways{
+				Nodes:         structs.CheckServiceNodes{},
+				Gateways:      structs.GatewayServices{},
+				ImportedNodes: structs.CheckServiceNodes{},
+				QueryMeta:     structs.QueryMeta{ResultsFilteredByACLs: true},
+			},
+		},
+	}
 
-		list := makeList()
+	for name, tc := range tt {
-		New(authz, logger).Filter(list)
+		t.Run(name, func(t *testing.T) {
-
+			run(t, tc)
-		require.Len(t, list.Nodes, 1)
-		require.Len(t, list.Gateways, 1)
-		require.True(t, list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
-	})
-
-	t.Run("denied", func(t *testing.T) {
-
-		list := makeList()
-		New(acl.DenyAll(), logger).Filter(list)
-
-		require.Empty(t, list.Nodes)
-		require.Empty(t, list.Gateways)
-		require.True(t, list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 		})
+	}
 }
 
 func TestACL_filterIndexedServiceDump(t *testing.T) {

agent/structs/structs.go

@@ -1998,6 +1998,15 @@ func (csn *CheckServiceNode) CanRead(authz acl.Authorizer) acl.EnforcementDecisi
 	authzContext := new(acl.AuthorizerContext)
 	csn.Service.EnterpriseMeta.FillAuthzContext(authzContext)
 
+	if csn.Node.PeerName != "" || csn.Service.PeerName != "" {
+		if authz.ServiceReadAll(authzContext) == acl.Allow ||
+			authz.ServiceWriteAny(authzContext) == acl.Allow {
+
+			return acl.Allow
+		}
+		return acl.Deny
+	}
+
 	if authz.NodeRead(csn.Node.Node, authzContext) != acl.Allow {
 		return acl.Deny
 	}

agent/structs/structs_test.go

@@ -1759,6 +1759,33 @@ func TestCheckServiceNode_CanRead(t *testing.T) {
 			authz:    acl.AllowAll(),
 			expected: acl.Allow,
 		},
+		{
+			name: "can read imported csn if can read all",
+			csn: CheckServiceNode{
+				Node:    &Node{Node: "name", PeerName: "cluster-2"},
+				Service: &NodeService{Service: "service-name", PeerName: "cluster-2"},
+			},
+			authz:    aclAuthorizerCheckServiceNode{allowReadAllServices: true},
+			expected: acl.Allow,
+		},
+		{
+			name: "can read imported csn if can write any",
+			csn: CheckServiceNode{
+				Node:    &Node{Node: "name", PeerName: "cluster-2"},
+				Service: &NodeService{Service: "service-name", PeerName: "cluster-2"},
+			},
+			authz:    aclAuthorizerCheckServiceNode{allowServiceWrite: true},
+			expected: acl.Allow,
+		},
+		{
+			name: "can't read imported csn with authz for local services and nodes",
+			csn: CheckServiceNode{
+				Node:    &Node{Node: "name", PeerName: "cluster-2"},
+				Service: &NodeService{Service: "service-name", PeerName: "cluster-2"},
+			},
+			authz:    aclAuthorizerCheckServiceNode{allowService: true, allowNode: true},
+			expected: acl.Deny,
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -1771,6 +1798,8 @@ type aclAuthorizerCheckServiceNode struct {
 	acl.Authorizer
 	allowNode            bool
 	allowService         bool
+	allowServiceWrite    bool
+	allowReadAllServices bool
 }
 
 func (a aclAuthorizerCheckServiceNode) ServiceRead(string, *acl.AuthorizerContext) acl.EnforcementDecision {
@@ -1787,6 +1816,20 @@ func (a aclAuthorizerCheckServiceNode) NodeRead(string, *acl.AuthorizerContext)
 	return acl.Deny
 }
 
+func (a aclAuthorizerCheckServiceNode) ServiceReadAll(*acl.AuthorizerContext) acl.EnforcementDecision {
+	if a.allowReadAllServices {
+		return acl.Allow
+	}
+	return acl.Deny
+}
+
+func (a aclAuthorizerCheckServiceNode) ServiceWriteAny(*acl.AuthorizerContext) acl.EnforcementDecision {
+	if a.allowServiceWrite {
+		return acl.Allow
+	}
+	return acl.Deny
+}
+
 func TestStructs_DirEntry_Clone(t *testing.T) {
 	e := &DirEntry{
 		LockIndex: 5,