kemko/nomad
1
0
Fork 0
mirror of https://github.com/kemko/nomad.git synced 2026-01-01 16:05:42 +03:00
Code Issues Packages Projects Releases Wiki Activity

client: fix role permission issue with duplicate policies. (#18419)

Browse Source 
This change deduplicates the ACL policy list generated from ACL
roles referenced within an ACL token on the client.

Previously the list could contain duplicates, which would cause
erronous permission denied errors when calling client related RPC/
HTTP API endpoints. This is because the client calls the ACL get
policies endpoint which subsequently ensures the caller has
permission to view the ACL policies. This check is performed by
comparing the requested list args with the policies referenced by
the caller ACL token. When a duplicate is present, this check
fails, as the check must ensure the slices match exactly.
This commit is contained in:
James Rasell
2023-09-11 12:52:08 +01:00
committed by GitHub
parent ef24e40b39
commit 668dc5f7a7
3 changed files with 39 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/18419.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:bug
acl: Fixed a bug where ACL tokens linked to ACL roles containing duplicate policies would cause erronous permission denined responses
```

23
client/acl.go
View File

@@ -7,6 +7,7 @@ import (
"time"
"github.com/armon/go-metrics"
"github.com/hashicorp/go-set"
"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/nomad/structs"
)
@@ -252,11 +253,6 @@ func (c *Client) resolvePolicies(secretID string, policies []string) ([]*structs
func (c *Client) resolveTokenACLRoles(secretID string, roleLinks []*structs.ACLTokenRoleLink) ([]string, error) {
var (
// policyNames tracks the resolved ACL policies which are linked to the
// role. This is the output object and represents the authorisation
// this role provides token bearers.
policyNames []string
// missingRoleIDs are the roles linked which are not found within our
// cache. These must be looked up from the server via and RPC, so we
// can correctly identify the policy links.
@@ -268,6 +264,11 @@ func (c *Client) resolveTokenACLRoles(secretID string, roleLinks []*structs.ACLT
expiredRoleIDs []string
)
// policyNames tracks the resolved ACL policies which are linked to the
// role as a deduplicated list. This is the output object and represents
// the authorisation this role provides token bearers.
policyNames := set.New[string](0)
for _, roleLink := range roleLinks {
// Look within the cache to see if the role is already present. If we
@@ -284,7 +285,7 @@ func (c *Client) resolveTokenACLRoles(secretID string, roleLinks []*structs.ACLT
// each policy name to our return object tracking.
if entry.Age() <= c.GetConfig().ACLRoleTTL {
for _, policyLink := range entry.Get().Policies {
policyNames = append(policyNames, policyLink.Name)
policyNames.Insert(policyLink.Name)
}
} else {
expiredRoleIDs = append(expiredRoleIDs, entry.Get().ID)
@@ -295,7 +296,7 @@ func (c *Client) resolveTokenACLRoles(secretID string, roleLinks []*structs.ACLT
// generate a list of linked policy names. Therefore, we can avoid making
// any RPC calls.
if len(missingRoleIDs)+len(expiredRoleIDs) == 0 {
return policyNames, nil
return policyNames.Slice(), nil
}
// Created a combined list of role IDs that we need to lookup from server
@@ -325,10 +326,10 @@ func (c *Client) resolveTokenACLRoles(secretID string, roleLinks []*structs.ACLT
c.logger.Warn("failed to resolve ACL roles, using expired cached value", "error", err)
for _, aclRole := range roleByIDResp.ACLRoles {
for _, rolePolicyLink := range aclRole.Policies {
policyNames = append(policyNames, rolePolicyLink.Name)
policyNames.Insert(rolePolicyLink.Name)
}
}
return policyNames, nil
return policyNames.Slice(), nil
}
return nil, err
}
@@ -345,9 +346,9 @@ func (c *Client) resolveTokenACLRoles(secretID string, roleLinks []*structs.ACLT
// Iterate the role policy links, extracting the name and adding this
// to our return response tracking.
for _, rolePolicyLink := range aclRole.Policies {
policyNames = append(policyNames, rolePolicyLink.Name)
policyNames.Insert(rolePolicyLink.Name)
}
}
return policyNames, nil
return policyNames.Slice(), nil
}

28
client/acl_test.go
View File

@@ -131,13 +131,13 @@ func TestClient_resolveTokenACLRoles(t *testing.T) {
defer cleanup()
// Create an ACL Role and a client token which is linked to this.
mockACLRole := mock.ACLRole()
mockACLRole1 := mock.ACLRole()
mockACLToken := mock.ACLToken()
mockACLToken.Policies = []string{}
mockACLToken.Roles = []*structs.ACLTokenRoleLink{{ID: mockACLRole.ID}}
mockACLToken.Roles = []*structs.ACLTokenRoleLink{{ID: mockACLRole1.ID}}
err := testServer.State().UpsertACLRoles(structs.MsgTypeTestSetup, 10, []*structs.ACLRole{mockACLRole}, true)
err := testServer.State().UpsertACLRoles(structs.MsgTypeTestSetup, 10, []*structs.ACLRole{mockACLRole1}, true)
must.NoError(t, err)
err = testServer.State().UpsertACLTokens(structs.MsgTypeTestSetup, 20, []*structs.ACLToken{mockACLToken})
must.NoError(t, err)
@@ -150,12 +150,32 @@ func TestClient_resolveTokenACLRoles(t *testing.T) {
// Test the cache directly and check that the ACL role previously queried
// is now cached.
must.Eq(t, 1, testClient.roleCache.Len())
must.True(t, testClient.roleCache.Contains(mockACLRole.ID))
must.True(t, testClient.roleCache.Contains(mockACLRole1.ID))
// Resolve the roles again to check we get the same results.
resolvedRoles2, err := testClient.resolveTokenACLRoles(rootACLToken.SecretID, mockACLToken.Roles)
must.NoError(t, err)
must.SliceContainsAll(t, resolvedRoles1, resolvedRoles2)
// Create another ACL role which will have the same ACL policy links as the
// previous
mockACLRole2 := mock.ACLRole()
must.NoError(t,
testServer.State().UpsertACLRoles(
structs.MsgTypeTestSetup, 30, []*structs.ACLRole{mockACLRole2}, true))
// Update the ACL token so that it links to two ACL roles, which include
// duplicate ACL policies.
mockACLToken.Roles = append(mockACLToken.Roles, &structs.ACLTokenRoleLink{ID: mockACLRole2.ID})
must.NoError(t,
testServer.State().UpsertACLTokens(
structs.MsgTypeTestSetup, 40, []*structs.ACLToken{mockACLToken}))
// Ensure when resolving the ACL token, we are returned a deduplicated list
// of ACL policy names.
resolvedRoles3, err := testClient.resolveTokenACLRoles(rootACLToken.SecretID, mockACLToken.Roles)
must.NoError(t, err)
must.SliceContainsAll(t, []string{"mocked-test-policy-1", "mocked-test-policy-2"}, resolvedRoles3)
}
func TestClient_ACL_ResolveToken_Disabled(t *testing.T) {