From f4db3b64a2ee72d4a45c7fafae93dc83ce06b335 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Wed, 24 Aug 2022 13:51:51 +0200 Subject: [PATCH] acl: allow tokens to lookup linked roles. (#14227) When listing or reading an ACL role, roles linked to the ACL token used for authentication can be returned to the caller. --- command/agent/acl_endpoint_test.go | 5 +- go.mod | 4 +- go.sum | 8 +- nomad/acl_endpoint.go | 112 +++++++++++++++++++++++++--- nomad/acl_endpoint_test.go | 114 ++++++++++++++++++++++++++--- nomad/state/state_store.go | 16 +++- 6 files changed, 226 insertions(+), 33 deletions(-) diff --git a/command/agent/acl_endpoint_test.go b/command/agent/acl_endpoint_test.go index 7effd85111e..6cbc0766707 100644 --- a/command/agent/acl_endpoint_test.go +++ b/command/agent/acl_endpoint_test.go @@ -579,9 +579,8 @@ func TestHTTPServer_ACLRoleListRequest(t *testing.T) { // Send the HTTP request. obj, err := srv.Server.ACLRoleListRequest(respW, req) - require.Error(t, err) - require.ErrorContains(t, err, "Permission denied") - require.Nil(t, obj) + require.NoError(t, err) + require.Empty(t, obj) }, }, { diff --git a/go.mod b/go.mod index c1ddc2fd8cf..49e8c02185e 100644 --- a/go.mod +++ b/go.mod @@ -65,7 +65,7 @@ require ( github.com/hashicorp/go-plugin v1.4.3 github.com/hashicorp/go-secure-stdlib/listenerutil v0.1.4 github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 - github.com/hashicorp/go-set v0.1.2 + github.com/hashicorp/go-set v0.1.3 github.com/hashicorp/go-sockaddr v1.0.2 github.com/hashicorp/go-syslog v1.0.0 github.com/hashicorp/go-uuid v1.0.2 @@ -110,7 +110,7 @@ require ( github.com/ryanuber/go-glob v1.0.0 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529 github.com/shirou/gopsutil/v3 v3.21.12 - github.com/shoenig/test v0.3.0 + github.com/shoenig/test v0.3.1 github.com/skratchdot/open-golang v0.0.0-20160302144031-75fb7ed4208c github.com/stretchr/testify v1.8.0 github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 diff --git a/go.sum b/go.sum index 0dfc011d5b0..99906ce56a4 100644 --- a/go.sum +++ b/go.sum @@ -755,8 +755,8 @@ github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 h1:kes8mmyCpxJsI7FTwtzRqEy9 github.com/hashicorp/go-secure-stdlib/strutil v0.1.2/go.mod h1:Gou2R9+il93BqX25LAKCLuM+y9U2T4hlwvT1yprcna4= github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1 h1:Yc026VyMyIpq1UWRnakHRG01U8fJm+nEfEmjoAb00n8= github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1/go.mod h1:l8slYwnJA26yBz+ErHpp2IRCLr0vuOMGBORIz4rRiAs= -github.com/hashicorp/go-set v0.1.2 h1:WqFkeT32zKiD/l7zwO1RLF4YwctJwp6IByML0LLa0os= -github.com/hashicorp/go-set v0.1.2/go.mod h1:0jTQeDo6GKX0WMFUV4IicFkxXo9DuoRnUODngpsoYCk= +github.com/hashicorp/go-set v0.1.3 h1:1fyYno7QjlfAaMp1rdkMtMorFgSC5Te2TV+V60OD/cI= +github.com/hashicorp/go-set v0.1.3/go.mod h1:XFMEKCP3rGoZUBvdYwC9k2YVDj8PsMU/B0ITuYkl8IA= github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU= github.com/hashicorp/go-sockaddr v1.0.2 h1:ztczhD1jLxIRjVejw8gFomI1BQZOe2WoVOu0SyteCQc= github.com/hashicorp/go-sockaddr v1.0.2/go.mod h1:rB4wwRAUzs07qva3c5SdrY/NEtAUjGlgmH/UkBUC97A= @@ -1178,8 +1178,8 @@ github.com/shirou/gopsutil v0.0.0-20181107111621-48177ef5f880/go.mod h1:5b4v6he4 github.com/shirou/gopsutil/v3 v3.21.12 h1:VoGxEW2hpmz0Vt3wUvHIl9fquzYLNpVpgNNB7pGJimA= github.com/shirou/gopsutil/v3 v3.21.12/go.mod h1:BToYZVTlSVlfazpDDYFnsVZLaoRG+g8ufT6fPQLdJzA= github.com/shirou/w32 v0.0.0-20160930032740-bb4de0191aa4/go.mod h1:qsXQc7+bwAM3Q1u/4XEfrquwF8Lw7D7y5cD8CuHnfIc= -github.com/shoenig/test v0.3.0 h1:H6tfSvgLrPHRR5NH9S40+lOfoyeH2PbswBr4twgn9Po= -github.com/shoenig/test v0.3.0/go.mod h1:xYtyGBC5Q3kzCNyJg/SjgNpfAa2kvmgA0i5+lQso8x0= +github.com/shoenig/test v0.3.1 h1:dhGZztS6nQuvJ0o0RtUiQHaEO4hhArh/WmWwik3Ols0= +github.com/shoenig/test v0.3.1/go.mod h1:xYtyGBC5Q3kzCNyJg/SjgNpfAa2kvmgA0i5+lQso8x0= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/sirupsen/logrus v1.0.4-0.20170822132746-89742aefa4b2/go.mod h1:pMByvHTf9Beacp5x1UXfOR9xyW/9antXMhjMPG0dEzc= github.com/sirupsen/logrus v1.0.6/go.mod h1:pMByvHTf9Beacp5x1UXfOR9xyW/9antXMhjMPG0dEzc= diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go index 5c06abe2759..f735cb56c92 100644 --- a/nomad/acl_endpoint.go +++ b/nomad/acl_endpoint.go @@ -12,6 +12,7 @@ import ( metrics "github.com/armon/go-metrics" log "github.com/hashicorp/go-hclog" memdb "github.com/hashicorp/go-memdb" + "github.com/hashicorp/go-set" policy "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/uuid" @@ -1264,13 +1265,35 @@ func (a *ACL) ListRoles( } defer metrics.MeasureSince([]string{"nomad", "acl", "list_roles"}, time.Now()) - // TODO (jrasell) allow callers to list role associated to their token. - if acl, err := a.srv.ResolveToken(args.AuthToken); err != nil { + // Resolve the token and ensure it has some form of permissions. + acl, err := a.srv.ResolveToken(args.AuthToken) + if err != nil { return err - } else if acl == nil || !acl.IsManagement() { + } else if acl == nil { return structs.ErrPermissionDenied } + // If the token is a management token, they can list all tokens. If not, + // the role set tracks which role links the token has and therefore which + // ones the caller can list. + isManagement := acl.IsManagement() + roleSet := &set.Set[string]{} + + // If the token is not a management token, we determine which roles are + // linked to the token and therefore can be listed by the caller. + if !isManagement { + token, err := a.requestACLToken(args.AuthToken) + if err != nil { + return err + } + if token == nil { + return structs.ErrTokenNotFound + } + + // Generate a set of Role IDs from the token role links. + roleSet = set.FromFunc(token.Roles, func(roleLink *structs.ACLTokenRoleLink) string { return roleLink.ID }) + } + // Set up and return the blocking query. return a.srv.blockingRPC(&blockingOptions{ queryOpts: &args.QueryOptions, @@ -1300,9 +1323,16 @@ func (a *ACL) ListRoles( return err } - // Iterate all the results and add these to our reply object. + // Iterate all the results and add these to our reply object. Check + // before appending to the reply that the caller is allowed to view + // the role. for raw := iter.Next(); raw != nil; raw = iter.Next() { - reply.ACLRoles = append(reply.ACLRoles, raw.(*structs.ACLRole).Stub()) + + role := raw.(*structs.ACLRole) + + if roleSet.Contains(role.ID) || isManagement { + reply.ACLRoles = append(reply.ACLRoles, role.Stub()) + } } // Use the index table to populate the query meta as we have no way @@ -1380,13 +1410,43 @@ func (a *ACL) GetRoleByID( } defer metrics.MeasureSince([]string{"nomad", "acl", "get_role_id"}, time.Now()) - // TODO (jrasell) allow callers to detail a role associated to their token. - if acl, err := a.srv.ResolveToken(args.AuthToken); err != nil { + // Resolve the token and ensure it has some form of permissions. + acl, err := a.srv.ResolveToken(args.AuthToken) + if err != nil { return err - } else if acl == nil || !acl.IsManagement() { + } else if acl == nil { return structs.ErrPermissionDenied } + // If the token is a management token, they can detail any token they so + // desire. + isManagement := acl.IsManagement() + + // If the token is not a management token, we determine if the caller wants + // to detail a role linked to their token. + if !isManagement { + aclToken, err := a.requestACLToken(args.AuthToken) + if err != nil { + return err + } + if aclToken == nil { + return structs.ErrTokenNotFound + } + + found := false + + for _, roleLink := range aclToken.Roles { + if roleLink.ID == args.RoleID { + found = true + break + } + } + + if !found { + return structs.ErrPermissionDenied + } + } + // Set up and return the blocking query. return a.srv.blockingRPC(&blockingOptions{ queryOpts: &args.QueryOptions, @@ -1435,13 +1495,43 @@ func (a *ACL) GetRoleByName( } defer metrics.MeasureSince([]string{"nomad", "acl", "get_role_name"}, time.Now()) - // TODO (jrasell) allow callers to detail a role associated to their token. - if acl, err := a.srv.ResolveToken(args.AuthToken); err != nil { + // Resolve the token and ensure it has some form of permissions. + acl, err := a.srv.ResolveToken(args.AuthToken) + if err != nil { return err - } else if acl == nil || !acl.IsManagement() { + } else if acl == nil { return structs.ErrPermissionDenied } + // If the token is a management token, they can detail any token they so + // desire. + isManagement := acl.IsManagement() + + // If the token is not a management token, we determine if the caller wants + // to detail a role linked to their token. + if !isManagement { + aclToken, err := a.requestACLToken(args.AuthToken) + if err != nil { + return err + } + if aclToken == nil { + return structs.ErrTokenNotFound + } + + found := false + + for _, roleLink := range aclToken.Roles { + if roleLink.Name == args.RoleName { + found = true + break + } + } + + if !found { + return structs.ErrPermissionDenied + } + } + // Set up and return the blocking query. return a.srv.blockingRPC(&blockingOptions{ queryOpts: &args.QueryOptions, diff --git a/nomad/acl_endpoint_test.go b/nomad/acl_endpoint_test.go index 1e01eb1f244..b134e390213 100644 --- a/nomad/acl_endpoint_test.go +++ b/nomad/acl_endpoint_test.go @@ -2099,12 +2099,13 @@ func TestACL_ListRoles(t *testing.T) { // Try listing roles without a valid ACL token. aclRoleReq1 := &structs.ACLRolesListRequest{ QueryOptions: structs.QueryOptions{ - Region: DefaultRegion, + Region: DefaultRegion, + AuthToken: uuid.Generate(), }, } var aclRoleResp1 structs.ACLRolesListResponse err := msgpackrpc.CallWithCodec(codec, structs.ACLListRolesRPCMethod, aclRoleReq1, &aclRoleResp1) - require.ErrorContains(t, err, "Permission denied") + require.ErrorContains(t, err, "ACL token not found") // Try listing roles with a valid ACL token. aclRoleReq2 := &structs.ACLRolesListRequest{ @@ -2146,6 +2147,28 @@ func TestACL_ListRoles(t *testing.T) { require.NoError(t, err) require.Len(t, aclRoleResp4.ACLRoles, 2) + // Generate and upsert an ACL Token which links to only one of the two + // roles within state. + aclToken := mock.ACLToken() + aclToken.Policies = nil + aclToken.Roles = []*structs.ACLTokenRoleLink{{ID: aclRoles[1].ID}} + + err = testServer.fsm.State().UpsertACLTokens(structs.MsgTypeTestSetup, 20, []*structs.ACLToken{aclToken}) + require.NoError(t, err) + + aclRoleReq5 := &structs.ACLRolesListRequest{ + QueryOptions: structs.QueryOptions{ + Region: DefaultRegion, + AuthToken: aclToken.SecretID, + }, + } + var aclRoleResp5 structs.ACLRolesListResponse + err = msgpackrpc.CallWithCodec(codec, structs.ACLListRolesRPCMethod, aclRoleReq5, &aclRoleResp5) + require.NoError(t, err) + require.Len(t, aclRoleResp5.ACLRoles, 1) + require.Equal(t, aclRoleResp5.ACLRoles[0].ID, aclRoles[1].ID) + require.Equal(t, aclRoleResp5.ACLRoles[0].Name, aclRoles[1].Name) + // Now test a blocking query, where we wait for an update to the list which // is triggered by a deletion. type res struct { @@ -2155,7 +2178,7 @@ func TestACL_ListRoles(t *testing.T) { resultCh := make(chan *res) go func(resultCh chan *res) { - aclRoleReq5 := &structs.ACLRolesListRequest{ + aclRoleReq6 := &structs.ACLRolesListRequest{ QueryOptions: structs.QueryOptions{ Region: DefaultRegion, AuthToken: aclRootToken.SecretID, @@ -2163,9 +2186,9 @@ func TestACL_ListRoles(t *testing.T) { MaxQueryTime: 10 * time.Second, }, } - var aclRoleResp5 structs.ACLRolesListResponse - err = msgpackrpc.CallWithCodec(codec, structs.ACLListRolesRPCMethod, aclRoleReq5, &aclRoleResp5) - resultCh <- &res{err: err, reply: &aclRoleResp5} + var aclRoleResp6 structs.ACLRolesListResponse + err = msgpackrpc.CallWithCodec(codec, structs.ACLListRolesRPCMethod, aclRoleReq6, &aclRoleResp6) + resultCh <- &res{err: err, reply: &aclRoleResp6} }(resultCh) // Delete an ACL role from state which should return the blocking query. @@ -2248,7 +2271,7 @@ func TestACL_GetRolesByID(t *testing.T) { resultCh := make(chan *res) go func(resultCh chan *res) { - aclRoleReq4 := &structs.ACLRolesByIDRequest{ + aclRoleReq5 := &structs.ACLRolesByIDRequest{ ACLRoleIDs: []string{aclRoles[0].ID, aclRoles[1].ID}, QueryOptions: structs.QueryOptions{ Region: DefaultRegion, @@ -2257,9 +2280,9 @@ func TestACL_GetRolesByID(t *testing.T) { MaxQueryTime: 10 * time.Second, }, } - var aclRoleResp4 structs.ACLRolesByIDResponse - err = msgpackrpc.CallWithCodec(codec, structs.ACLGetRolesByIDRPCMethod, aclRoleReq4, &aclRoleResp4) - resultCh <- &res{err: err, reply: &aclRoleResp4} + var aclRoleResp5 structs.ACLRolesByIDResponse + err = msgpackrpc.CallWithCodec(codec, structs.ACLGetRolesByIDRPCMethod, aclRoleReq5, &aclRoleResp5) + resultCh <- &res{err: err, reply: &aclRoleResp5} }(resultCh) // Delete an ACL role from state which should return the blocking query. @@ -2342,6 +2365,41 @@ func TestACL_GetRoleByID(t *testing.T) { err = msgpackrpc.CallWithCodec(codec, structs.ACLGetRoleByIDRPCMethod, aclRoleReq4, &aclRoleResp4) require.NoError(t, err) require.True(t, aclRoleResp4.ACLRole.Equals(aclRoles[1])) + + // Generate and upsert an ACL Token which links to only one of the two + // roles within state. + aclToken := mock.ACLToken() + aclToken.Policies = nil + aclToken.Roles = []*structs.ACLTokenRoleLink{{ID: aclRoles[1].ID}} + + err = testServer.fsm.State().UpsertACLTokens(structs.MsgTypeTestSetup, 20, []*structs.ACLToken{aclToken}) + require.NoError(t, err) + + // Try detailing the role that is tried to our ACL token. + aclRoleReq5 := &structs.ACLRoleByIDRequest{ + RoleID: aclRoles[1].ID, + QueryOptions: structs.QueryOptions{ + Region: DefaultRegion, + AuthToken: aclToken.SecretID, + }, + } + var aclRoleResp5 structs.ACLRoleByIDResponse + err = msgpackrpc.CallWithCodec(codec, structs.ACLGetRoleByIDRPCMethod, aclRoleReq5, &aclRoleResp5) + require.NoError(t, err) + require.NotNil(t, aclRoleResp5.ACLRole) + require.Equal(t, aclRoleResp5.ACLRole.ID, aclRoles[1].ID) + + // Try detailing the role that is NOT tried to our ACL token. + aclRoleReq6 := &structs.ACLRoleByIDRequest{ + RoleID: aclRoles[0].ID, + QueryOptions: structs.QueryOptions{ + Region: DefaultRegion, + AuthToken: aclToken.SecretID, + }, + } + var aclRoleResp6 structs.ACLRoleByIDResponse + err = msgpackrpc.CallWithCodec(codec, structs.ACLGetRoleByIDRPCMethod, aclRoleReq6, &aclRoleResp6) + require.ErrorContains(t, err, "Permission denied") } func TestACL_GetRoleByName(t *testing.T) { @@ -2412,4 +2470,40 @@ func TestACL_GetRoleByName(t *testing.T) { err = msgpackrpc.CallWithCodec(codec, structs.ACLGetRoleByNameRPCMethod, aclRoleReq4, &aclRoleResp4) require.NoError(t, err) require.True(t, aclRoleResp4.ACLRole.Equals(aclRoles[1])) + + // Generate and upsert an ACL Token which links to only one of the two + // roles within state. + aclToken := mock.ACLToken() + aclToken.Policies = nil + aclToken.Roles = []*structs.ACLTokenRoleLink{{ID: aclRoles[1].ID}} + + err = testServer.fsm.State().UpsertACLTokens(structs.MsgTypeTestSetup, 20, []*structs.ACLToken{aclToken}) + require.NoError(t, err) + + // Try detailing the role that is tried to our ACL token. + aclRoleReq5 := &structs.ACLRoleByNameRequest{ + RoleName: aclRoles[1].Name, + QueryOptions: structs.QueryOptions{ + Region: DefaultRegion, + AuthToken: aclToken.SecretID, + }, + } + var aclRoleResp5 structs.ACLRoleByNameResponse + err = msgpackrpc.CallWithCodec(codec, structs.ACLGetRoleByNameRPCMethod, aclRoleReq5, &aclRoleResp5) + require.NoError(t, err) + require.NotNil(t, aclRoleResp5.ACLRole) + require.Equal(t, aclRoleResp5.ACLRole.ID, aclRoles[1].ID) + require.Equal(t, aclRoleResp5.ACLRole.Name, aclRoles[1].Name) + + // Try detailing the role that is NOT tried to our ACL token. + aclRoleReq6 := &structs.ACLRoleByNameRequest{ + RoleName: aclRoles[0].Name, + QueryOptions: structs.QueryOptions{ + Region: DefaultRegion, + AuthToken: aclToken.SecretID, + }, + } + var aclRoleResp6 structs.ACLRoleByNameResponse + err = msgpackrpc.CallWithCodec(codec, structs.ACLGetRoleByNameRPCMethod, aclRoleReq6, &aclRoleResp6) + require.ErrorContains(t, err, "Permission denied") } diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go index 84d59370be2..5a3d6028cfc 100644 --- a/nomad/state/state_store.go +++ b/nomad/state/state_store.go @@ -5690,10 +5690,20 @@ func (s *StateStore) ACLTokenBySecretID(ws memdb.WatchSet, secretID string) (*st } ws.Add(watchCh) - if existing != nil { - return existing.(*structs.ACLToken), nil + // If the existing token is nil, this indicates it does not exist in state. + if existing == nil { + return nil, nil } - return nil, nil + + // Assert the token type which allows us to perform additional work on the + // token that is needed before returning the call. + token := existing.(*structs.ACLToken) + + // Handle potential staleness of ACL role links. + if token, err = s.fixTokenRoleLinks(txn, token); err != nil { + return nil, err + } + return token, nil } // ACLTokenByAccessorIDPrefix is used to lookup tokens by prefix