acl: allow tokens to read policies linked via roles to the token.

.changelog/14982.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+acl: Callers should be able to read policies linked via roles to the token used
+```

nomad/acl_endpoint.go

@@ -139,7 +139,7 @@ func (a *ACL) ListPolicies(args *structs.ACLPolicyListRequest, reply *structs.AC
 
 	// If it is not a management token determine the policies that may be listed
 	mgt := acl.IsManagement()
-	var policies map[string]struct{}
+	tokenPolicyNames := set.New[string](0)
 	if !mgt {
 		token, err := a.requestACLToken(args.AuthToken)
 		if err != nil {
@@ -149,10 +149,15 @@ func (a *ACL) ListPolicies(args *structs.ACLPolicyListRequest, reply *structs.AC
 			return structs.ErrTokenNotFound
 		}
 
-		policies = make(map[string]struct{}, len(token.Policies))
-		for _, p := range token.Policies {
-			policies[p] = struct{}{}
+		// Generate a set of policy names. This is initially generated from the
+		// ACL role links.
+		tokenPolicyNames, err = a.policyNamesFromRoleLinks(token.Roles)
+		if err != nil {
+			return err
 		}
+
+		// Add the token policies which are directly referenced into the set.
+		tokenPolicyNames.InsertAll(token.Policies)
 	}
 
 	// Setup the blocking query
@@ -179,9 +184,9 @@ func (a *ACL) ListPolicies(args *structs.ACLPolicyListRequest, reply *structs.AC
 				if raw == nil {
 					break
 				}
-				policy := raw.(*structs.ACLPolicy)
-				if _, ok := policies[policy.Name]; ok || mgt {
-					reply.Policies = append(reply.Policies, policy.Stub())
+				realPolicy := raw.(*structs.ACLPolicy)
+				if mgt || tokenPolicyNames.Contains(realPolicy.Name) {
+					reply.Policies = append(reply.Policies, realPolicy.Stub())
 				}
 			}
 
@@ -233,15 +238,17 @@ func (a *ACL) GetPolicy(args *structs.ACLPolicySpecificRequest, reply *structs.S
 			return structs.ErrTokenNotFound
 		}
 
-		found := false
-		for _, p := range token.Policies {
-			if p == args.Name {
-				found = true
-				break
-			}
+		// Generate a set of policy names. This is initially generated from the
+		// ACL role links.
+		tokenPolicyNames, err := a.policyNamesFromRoleLinks(token.Roles)
+		if err != nil {
+			return err
 		}
 
-		if !found {
+		// Add the token policies which are directly referenced into the set.
+		tokenPolicyNames.InsertAll(token.Policies)
+
+		if !tokenPolicyNames.Contains(args.Name) {
 			return structs.ErrPermissionDenied
 		}
 	}
@@ -310,11 +317,22 @@ func (a *ACL) GetPolicies(args *structs.ACLPolicySetRequest, reply *structs.ACLP
 	if err != nil {
 		return err
 	}
-
 	if token == nil {
 		return structs.ErrTokenNotFound
 	}
-	if token.Type != structs.ACLManagementToken && !token.PolicySubset(args.Names) {
+
+	// Generate a set of policy names. This is initially generated from the
+	// ACL role links.
+	tokenPolicyNames, err := a.policyNamesFromRoleLinks(token.Roles)
+	if err != nil {
+		return err
+	}
+
+	// Add the token policies which are directly referenced into the set.
+	tokenPolicyNames.InsertAll(token.Policies)
+
+	// Ensure the token has enough permissions to query the named policies.
+	if token.Type != structs.ACLManagementToken && !tokenPolicyNames.ContainsAll(args.Names) {
 		return structs.ErrPermissionDenied
 	}
 
@@ -1593,3 +1611,59 @@ func (a *ACL) GetRoleByName(
 		},
 	})
 }
+
+// policyNamesFromRoleLinks resolves the policy names which are linked via the
+// passed role links. This is useful when you need to understand what polices
+// an ACL token has access to and need to include role links. The function will
+// not return a nil set object, so callers can use this without having to check
+// this.
+func (a *ACL) policyNamesFromRoleLinks(roleLinks []*structs.ACLTokenRoleLink) (*set.Set[string], error) {
+
+	numRoles := len(roleLinks)
+	policyNameSet := set.New[string](numRoles)
+
+	if numRoles < 1 {
+		return policyNameSet, nil
+	}
+
+	stateSnapshot, err := a.srv.State().Snapshot()
+	if err != nil {
+		return policyNameSet, err
+	}
+
+	// Iterate all the token role links, so we can unpack these and identify
+	// the ACL policies.
+	for _, roleLink := range roleLinks {
+
+		// Any error reading the role means we cannot move forward. We just
+		// ignore any roles that have been detailed but are not within our
+		// state.
+		role, err := stateSnapshot.GetACLRoleByID(nil, roleLink.ID)
+		if err != nil {
+			return policyNameSet, err
+		}
+		if role == nil {
+			continue
+		}
+
+		// Unpack the policies held within the ACL role to form a single list
+		// of ACL policies that this token has available.
+		for _, policyLink := range role.Policies {
+			policyByName, err := stateSnapshot.ACLPolicyByName(nil, policyLink.Name)
+			if err != nil {
+				return policyNameSet, err
+			}
+
+			// Ignore policies that don't exist, since they don't grant any
+			// more privilege.
+			if policyByName == nil {
+				continue
+			}
+
+			// Add the policy to the tracking array.
+			policyNameSet.Insert(policyByName.Name)
+		}
+	}
+
+	return policyNameSet, nil
+}

nomad/acl_endpoint_test.go

@@ -104,6 +104,35 @@ func TestACLEndpoint_GetPolicy(t *testing.T) {
 	err := msgpackrpc.CallWithCodec(codec, "ACL.GetPolicy", get, &resp4)
 	require.Error(t, err)
 	require.Contains(t, err.Error(), structs.ErrPermissionDenied.Error())
+
+	// Generate and upsert an ACL role which links to the previously created
+	// policy.
+	mockACLRole := mock.ACLRole()
+	mockACLRole.Policies = []*structs.ACLRolePolicyLink{{Name: policy.Name}}
+	must.NoError(t, s1.fsm.State().UpsertACLRoles(
+		structs.MsgTypeTestSetup, 1010, []*structs.ACLRole{mockACLRole}, false))
+
+	// Generate and upsert an ACL token which only has ACL role links.
+	mockTokenWithRole := mock.ACLToken()
+	mockTokenWithRole.Policies = []string{}
+	mockTokenWithRole.Roles = []*structs.ACLTokenRoleLink{{ID: mockACLRole.ID}}
+	must.NoError(t, s1.fsm.State().UpsertACLTokens(
+		structs.MsgTypeTestSetup, 1020, []*structs.ACLToken{mockTokenWithRole}))
+
+	// Use the newly created token to attempt to read the policy which is
+	// linked via a role, and not directly referenced within the policy array.
+	req5 := &structs.ACLPolicySpecificRequest{
+		Name: policy.Name,
+		QueryOptions: structs.QueryOptions{
+			Region:    "global",
+			AuthToken: mockTokenWithRole.SecretID,
+		},
+	}
+
+	var resp5 structs.SingleACLPolicyResponse
+	must.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.GetPolicy", req5, &resp5))
+	must.Eq(t, 1000, resp5.Index)
+	must.Eq(t, policy, resp5.Policy)
 }
 
 func TestACLEndpoint_GetPolicy_Blocking(t *testing.T) {
@@ -265,6 +294,59 @@ func TestACLEndpoint_GetPolicies_TokenSubset(t *testing.T) {
 	if err := msgpackrpc.CallWithCodec(codec, "ACL.GetPolicies", get, &resp); err == nil {
 		t.Fatalf("expected error")
 	}
+
+	// Generate and upsert an ACL role which links to the previously created
+	// policy.
+	mockACLRole := mock.ACLRole()
+	mockACLRole.Policies = []*structs.ACLRolePolicyLink{{Name: policy.Name}}
+	must.NoError(t, s1.fsm.State().UpsertACLRoles(
+		structs.MsgTypeTestSetup, 1010, []*structs.ACLRole{mockACLRole}, false))
+
+	// Generate and upsert an ACL token which only has ACL role links.
+	mockTokenWithRole := mock.ACLToken()
+	mockTokenWithRole.Policies = []string{}
+	mockTokenWithRole.Roles = []*structs.ACLTokenRoleLink{{ID: mockACLRole.ID}}
+	must.NoError(t, s1.fsm.State().UpsertACLTokens(
+		structs.MsgTypeTestSetup, 1020, []*structs.ACLToken{mockTokenWithRole}))
+
+	// Use the newly created token to attempt to read the policy which is
+	// linked via a role, and not directly referenced within the policy array.
+	req1 := &structs.ACLPolicySetRequest{
+		Names: []string{policy.Name},
+		QueryOptions: structs.QueryOptions{
+			Region:    "global",
+			AuthToken: mockTokenWithRole.SecretID,
+		},
+	}
+
+	var resp1 structs.ACLPolicySetResponse
+	must.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.GetPolicies", req1, &resp1))
+	must.Eq(t, 1000, resp1.Index)
+	must.Eq(t, 1, len(resp1.Policies))
+	must.Eq(t, policy, resp1.Policies[policy.Name])
+
+	// Generate and upsert an ACL token which only has both direct policy links
+	// and ACL role links.
+	mockTokenWithRolePolicy := mock.ACLToken()
+	mockTokenWithRolePolicy.Policies = []string{policy2.Name}
+	mockTokenWithRolePolicy.Roles = []*structs.ACLTokenRoleLink{{ID: mockACLRole.ID}}
+	must.NoError(t, s1.fsm.State().UpsertACLTokens(
+		structs.MsgTypeTestSetup, 1030, []*structs.ACLToken{mockTokenWithRolePolicy}))
+
+	// Use the newly created token to attempt to read the policies which are
+	// linked directly, and by ACL roles.
+	req2 := &structs.ACLPolicySetRequest{
+		Names: []string{policy.Name, policy2.Name},
+		QueryOptions: structs.QueryOptions{
+			Region:    "global",
+			AuthToken: mockTokenWithRolePolicy.SecretID,
+		},
+	}
+
+	var resp2 structs.ACLPolicySetResponse
+	must.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.GetPolicies", req2, &resp2))
+	must.Eq(t, 1000, resp2.Index)
+	must.Eq(t, 2, len(resp2.Policies))
 }
 
 func TestACLEndpoint_GetPolicies_Blocking(t *testing.T) {
@@ -413,6 +495,36 @@ func TestACLEndpoint_ListPolicies(t *testing.T) {
 	if assert.Len(resp3.Policies, 1) {
 		assert.Equal(resp3.Policies[0].Name, p1.Name)
 	}
+
+	// Generate and upsert an ACL role which links to the previously created
+	// policy.
+	mockACLRole := mock.ACLRole()
+	mockACLRole.Policies = []*structs.ACLRolePolicyLink{{Name: p1.Name}}
+	must.NoError(t, s1.fsm.State().UpsertACLRoles(
+		structs.MsgTypeTestSetup, 1010, []*structs.ACLRole{mockACLRole}, false))
+
+	// Generate and upsert an ACL token which only has ACL role links.
+	mockTokenWithRole := mock.ACLToken()
+	mockTokenWithRole.Policies = []string{}
+	mockTokenWithRole.Roles = []*structs.ACLTokenRoleLink{{ID: mockACLRole.ID}}
+	must.NoError(t, s1.fsm.State().UpsertACLTokens(
+		structs.MsgTypeTestSetup, 1020, []*structs.ACLToken{mockTokenWithRole}))
+
+	// Use the newly created token to attempt to list the policies. We should
+	// get the single policy linked by the ACL role.
+	req4 := &structs.ACLPolicyListRequest{
+		QueryOptions: structs.QueryOptions{
+			Region:    "global",
+			AuthToken: mockTokenWithRole.SecretID,
+		},
+	}
+
+	var resp4 structs.ACLPolicyListResponse
+	must.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.ListPolicies", req4, &resp4))
+	must.Eq(t, 1000, resp4.Index)
+	must.Len(t, 1, resp4.Policies)
+	must.Eq(t, p1.Name, resp4.Policies[0].Name)
+	must.Eq(t, p1.Hash, resp4.Policies[0].Hash)
 }
 
 // TestACLEndpoint_ListPolicies_Unauthenticated asserts that

nomad/structs/structs.go

@@ -12111,24 +12111,6 @@ func (a *ACLToken) Stub() *ACLTokenListStub {
 	}
 }
 
-// PolicySubset checks if a given set of policies is a subset of the token
-func (a *ACLToken) PolicySubset(policies []string) bool {
-	// Hot-path the management tokens, superset of all policies.
-	if a.Type == ACLManagementToken {
-		return true
-	}
-	associatedPolicies := make(map[string]struct{}, len(a.Policies))
-	for _, policy := range a.Policies {
-		associatedPolicies[policy] = struct{}{}
-	}
-	for _, policy := range policies {
-		if _, ok := associatedPolicies[policy]; !ok {
-			return false
-		}
-	}
-	return true
-}
-
 // ACLTokenListRequest is used to request a list of tokens
 type ACLTokenListRequest struct {
 	GlobalOnly bool

nomad/structs/structs_test.go

@@ -6114,33 +6114,6 @@ func TestIsRecoverable(t *testing.T) {
 	}
 }
 
-func TestACLTokenPolicySubset(t *testing.T) {
-	ci.Parallel(t)
-
-	tk := &ACLToken{
-		Type:     ACLClientToken,
-		Policies: []string{"foo", "bar", "baz"},
-	}
-
-	assert.Equal(t, true, tk.PolicySubset([]string{"foo", "bar", "baz"}))
-	assert.Equal(t, true, tk.PolicySubset([]string{"foo", "bar"}))
-	assert.Equal(t, true, tk.PolicySubset([]string{"foo"}))
-	assert.Equal(t, true, tk.PolicySubset([]string{}))
-	assert.Equal(t, false, tk.PolicySubset([]string{"foo", "bar", "new"}))
-	assert.Equal(t, false, tk.PolicySubset([]string{"new"}))
-
-	tk = &ACLToken{
-		Type: ACLManagementToken,
-	}
-
-	assert.Equal(t, true, tk.PolicySubset([]string{"foo", "bar", "baz"}))
-	assert.Equal(t, true, tk.PolicySubset([]string{"foo", "bar"}))
-	assert.Equal(t, true, tk.PolicySubset([]string{"foo"}))
-	assert.Equal(t, true, tk.PolicySubset([]string{}))
-	assert.Equal(t, true, tk.PolicySubset([]string{"foo", "bar", "new"}))
-	assert.Equal(t, true, tk.PolicySubset([]string{"new"}))
-}
-
 func TestACLTokenSetHash(t *testing.T) {
 	ci.Parallel(t)