From fbe9f590489e291b3abcfb0fb1548a9f435a5e2d Mon Sep 17 00:00:00 2001
From: James Rasell <jrasell@users.noreply.github.com>
Date: Fri, 21 Oct 2022 09:05:17 +0200
Subject: [PATCH] acl: allow tokens to read policies linked via roles to the
token. (#14982)
ACL tokens are granted permissions either by direct policy links
or via ACL role links. Callers should therefore be able to read
policies directly assigned to the caller token or indirectly by
ACL role links.
---
.changelog/14982.txt | 3 +
nomad/acl_endpoint.go | 106 +++++++++++++++++++++++++++-----
nomad/acl_endpoint_test.go | 112 ++++++++++++++++++++++++++++++++++
nomad/structs/structs.go | 18 ------
nomad/structs/structs_test.go | 27 --------
5 files changed, 205 insertions(+), 61 deletions(-)
create mode 100644 .changelog/14982.txt
diff --git a/.changelog/14982.txt b/.changelog/14982.txt
new file mode 100644
index 0000000000..b793df6645
--- /dev/null
+++ b/.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
+```
diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go
index c95629faa1..ae906819bb 100644
--- a/nomad/acl_endpoint.go
+++ b/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
+}
diff --git a/nomad/acl_endpoint_test.go b/nomad/acl_endpoint_test.go
index 42bddfc0d3..e4ac8d460c 100644
--- a/nomad/acl_endpoint_test.go
+++ b/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
diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
index db2cdf2f96..a4dad02d97 100644
--- a/nomad/structs/structs.go
+++ b/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
diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go
index fdc174aa96..72c7c068aa 100644
--- a/nomad/structs/structs_test.go
+++ b/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)
--
GitLab