diff --git a/.changelog/14982.txt b/.changelog/14982.txt new file mode 100644 index 000000000..b793df664 --- /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 c95629faa..ae906819b 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 42bddfc0d..e4ac8d460 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 db2cdf2f9..a4dad02d9 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 fdc174aa9..72c7c068a 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)