luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

backport of commit 668dc5f7a767e85d62379e3e02405d2afa93f1db (#18448) 

Co-authored-by: James Rasell <jrasell@users.noreply.github.com>
This commit is contained in:
hc-github-team-nomad-core 2023-09-11 07:22:30 -05:00 committed by GitHub
parent a7f85c804f
commit 156db8d368
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 15 deletions
Show Stats 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) {