From ee2e7d1d008de1e89bb9cb04c61ea19b991cb04f Mon Sep 17 00:00:00 2001 From: James Rasell Date: Fri, 15 Jul 2022 15:20:50 +0200 Subject: [PATCH] acl: add token expiry checking to ACL token resolution. (#13756) This commit adds basic expiry checking when performing ACL token resolution. This expiry checking is local to each server and does not at this time take into account potential time skew on server hosts. A new error message has been created so clients whose token has expired get a clear message, rather than a generic token not found. The ACL resolution tests have been refactored into table driven tests, so additions are easier in the future. --- nomad/acl.go | 6 + nomad/acl_test.go | 309 +++++++++++++++++++++++++++------------- nomad/structs/errors.go | 2 + 3 files changed, 219 insertions(+), 98 deletions(-) diff --git a/nomad/acl.go b/nomad/acl.go index 90ecb417b..70587dd19 100644 --- a/nomad/acl.go +++ b/nomad/acl.go @@ -54,6 +54,9 @@ func resolveTokenFromSnapshotCache(snap *state.StateSnapshot, cache *lru.TwoQueu if token == nil { return nil, structs.ErrTokenNotFound } + if token.IsExpired(time.Now().UTC()) { + return nil, structs.ErrTokenExpired + } } // Check if this is a management token @@ -114,6 +117,9 @@ func (s *Server) ResolveSecretToken(secretID string) (*structs.ACLToken, error) if token == nil { return nil, structs.ErrTokenNotFound } + if token.IsExpired(time.Now().UTC()) { + return nil, structs.ErrTokenExpired + } } return token, nil diff --git a/nomad/acl_test.go b/nomad/acl_test.go index 867150639..14104a1df 100644 --- a/nomad/acl_test.go +++ b/nomad/acl_test.go @@ -2,133 +2,246 @@ package nomad import ( "testing" + "time" - lru "github.com/hashicorp/golang-lru" "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/ci" + "github.com/hashicorp/nomad/helper/pointer" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" - "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestResolveACLToken(t *testing.T) { ci.Parallel(t) - // Create mock state store and cache - state := state.TestStateStore(t) - cache, err := lru.New2Q(16) - assert.Nil(t, err) + testServer, _, testServerCleanup := TestACLServer(t, nil) + defer testServerCleanup() + testutil.WaitForLeader(t, testServer.RPC) - // Create a policy / token - policy := mock.ACLPolicy() - policy2 := mock.ACLPolicy() - token := mock.ACLToken() - token.Policies = []string{policy.Name, policy2.Name} - token2 := mock.ACLToken() - token2.Type = structs.ACLManagementToken - token2.Policies = nil - err = state.UpsertACLPolicies(structs.MsgTypeTestSetup, 100, []*structs.ACLPolicy{policy, policy2}) - assert.Nil(t, err) - err = state.UpsertACLTokens(structs.MsgTypeTestSetup, 110, []*structs.ACLToken{token, token2}) - assert.Nil(t, err) + testCases := []struct { + name string + testFn func(testServer *Server) + }{ + { + name: "leader token", + testFn: func(testServer *Server) { - snap, err := state.Snapshot() - assert.Nil(t, err) + // Check the leader ACL token is correctly set. + leaderACL := testServer.getLeaderAcl() + require.NotEmpty(t, leaderACL) - // Attempt resolution of blank token. Should return anonymous policy - aclObj, err := resolveTokenFromSnapshotCache(snap, cache, "") - assert.Nil(t, err) - assert.NotNil(t, aclObj) + // Resolve the token and ensure it's a management token. + aclResp, err := testServer.ResolveToken(leaderACL) + require.NoError(t, err) + require.NotNil(t, aclResp) + require.True(t, aclResp.IsManagement()) + }, + }, + { + name: "anonymous token", + testFn: func(testServer *Server) { - // Attempt resolution of unknown token. Should fail. - randID := uuid.Generate() - aclObj, err = resolveTokenFromSnapshotCache(snap, cache, randID) - assert.Equal(t, structs.ErrTokenNotFound, err) - assert.Nil(t, aclObj) + // Call the function with an empty input secret ID which is + // classed as representing anonymous access in clusters with + // ACLs enabled. + aclResp, err := testServer.ResolveToken("") + require.NoError(t, err) + require.NotNil(t, aclResp) + require.False(t, aclResp.IsManagement()) + }, + }, + { + name: "token not found", + testFn: func(testServer *Server) { - // Attempt resolution of management token. Should get singleton. - aclObj, err = resolveTokenFromSnapshotCache(snap, cache, token2.SecretID) - assert.Nil(t, err) - assert.NotNil(t, aclObj) - assert.Equal(t, true, aclObj.IsManagement()) - if aclObj != acl.ManagementACL { - t.Fatalf("expected singleton") + // Call the function with randomly generated secret ID which + // does not exist within state. + aclResp, err := testServer.ResolveToken(uuid.Generate()) + require.Equal(t, structs.ErrTokenNotFound, err) + require.Nil(t, aclResp) + }, + }, + { + name: "token expired", + testFn: func(testServer *Server) { + + // Create a mock token with an expiration time long in the + // past, and upsert. + token := mock.ACLToken() + token.ExpirationTime = pointer.Of(time.Date( + 1970, time.January, 1, 0, 0, 0, 0, time.UTC)) + + err := testServer.State().UpsertACLTokens( + structs.MsgTypeTestSetup, 10, []*structs.ACLToken{token}) + require.NoError(t, err) + + // Perform the function call which should result in finding the + // token has expired. + aclResp, err := testServer.ResolveToken(uuid.Generate()) + require.Equal(t, structs.ErrTokenNotFound, err) + require.Nil(t, aclResp) + }, + }, + { + name: "management token", + testFn: func(testServer *Server) { + + // Generate a management token and upsert this. + managementToken := mock.ACLToken() + managementToken.Type = structs.ACLManagementToken + managementToken.Policies = nil + + err := testServer.State().UpsertACLTokens( + structs.MsgTypeTestSetup, 10, []*structs.ACLToken{managementToken}) + require.NoError(t, err) + + // Resolve the token and check that we received a management + // ACL. + aclResp, err := testServer.ResolveToken(managementToken.SecretID) + require.Nil(t, err) + require.NotNil(t, aclResp) + require.True(t, aclResp.IsManagement()) + require.Equal(t, acl.ManagementACL, aclResp) + }, + }, + { + name: "client token", + testFn: func(testServer *Server) { + + // Generate a client token with associated policies and upsert + // these. + policy1 := mock.ACLPolicy() + policy2 := mock.ACLPolicy() + err := testServer.State().UpsertACLPolicies( + structs.MsgTypeTestSetup, 10, []*structs.ACLPolicy{policy1, policy2}) + + clientToken := mock.ACLToken() + clientToken.Policies = []string{policy1.Name, policy2.Name} + err = testServer.State().UpsertACLTokens( + structs.MsgTypeTestSetup, 20, []*structs.ACLToken{clientToken}) + require.NoError(t, err) + + // Resolve the token and check that we received a client + // ACL with appropriate permissions. + aclResp, err := testServer.ResolveToken(clientToken.SecretID) + require.Nil(t, err) + require.NotNil(t, aclResp) + require.False(t, aclResp.IsManagement()) + + allowed := aclResp.AllowNamespaceOperation("default", acl.NamespaceCapabilityListJobs) + require.True(t, allowed) + allowed = aclResp.AllowNamespaceOperation("other", acl.NamespaceCapabilityListJobs) + require.False(t, allowed) + + // Resolve the same token again and ensure we get the same + // result. + aclResp2, err := testServer.ResolveToken(clientToken.SecretID) + require.Nil(t, err) + require.NotNil(t, aclResp2) + require.Equal(t, aclResp, aclResp2) + + // Bust the cache by upserting the policy + err = testServer.State().UpsertACLPolicies( + structs.MsgTypeTestSetup, 30, []*structs.ACLPolicy{policy1}) + require.Nil(t, err) + + // Resolve the same token again, should get different value + aclResp3, err := testServer.ResolveToken(clientToken.SecretID) + require.Nil(t, err) + require.NotNil(t, aclResp3) + require.NotEqual(t, aclResp2, aclResp3) + }, + }, } - // Attempt resolution of client token - aclObj, err = resolveTokenFromSnapshotCache(snap, cache, token.SecretID) - assert.Nil(t, err) - assert.NotNil(t, aclObj) - - // Check that the ACL object is sane - assert.Equal(t, false, aclObj.IsManagement()) - allowed := aclObj.AllowNamespaceOperation("default", acl.NamespaceCapabilityListJobs) - assert.Equal(t, true, allowed) - allowed = aclObj.AllowNamespaceOperation("other", acl.NamespaceCapabilityListJobs) - assert.Equal(t, false, allowed) - - // Resolve the same token again, should get cache value - aclObj2, err := resolveTokenFromSnapshotCache(snap, cache, token.SecretID) - assert.Nil(t, err) - assert.NotNil(t, aclObj2) - if aclObj != aclObj2 { - t.Fatalf("expected cached value") - } - - // Bust the cache by upserting the policy - err = state.UpsertACLPolicies(structs.MsgTypeTestSetup, 120, []*structs.ACLPolicy{policy}) - assert.Nil(t, err) - snap, err = state.Snapshot() - assert.Nil(t, err) - - // Resolve the same token again, should get different value - aclObj3, err := resolveTokenFromSnapshotCache(snap, cache, token.SecretID) - assert.Nil(t, err) - assert.NotNil(t, aclObj3) - if aclObj == aclObj3 { - t.Fatalf("unexpected cached value") - } -} - -func TestResolveACLToken_LeaderToken(t *testing.T) { - ci.Parallel(t) - assert := assert.New(t) - s1, _, cleanupS1 := TestACLServer(t, nil) - defer cleanupS1() - testutil.WaitForLeader(t, s1.RPC) - - leaderAcl := s1.getLeaderAcl() - assert.NotEmpty(leaderAcl) - token, err := s1.ResolveToken(leaderAcl) - assert.Nil(err) - if assert.NotNil(token) { - assert.True(token.IsManagement()) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.testFn(testServer) + }) } } func TestResolveSecretToken(t *testing.T) { ci.Parallel(t) - s1, _, cleanupS1 := TestACLServer(t, nil) - defer cleanupS1() - testutil.WaitForLeader(t, s1.RPC) + testServer, _, testServerCleanup := TestACLServer(t, nil) + defer testServerCleanup() + testutil.WaitForLeader(t, testServer.RPC) - state := s1.State() - leaderToken := s1.getLeaderAcl() - assert.NotEmpty(t, leaderToken) + testCases := []struct { + name string + testFn func(testServer *Server) + }{ + { + name: "valid token", + testFn: func(testServer *Server) { - token := mock.ACLToken() + // Generate and upsert a token. + token := mock.ACLToken() + err := testServer.State().UpsertACLTokens( + structs.MsgTypeTestSetup, 10, []*structs.ACLToken{token}) + require.NoError(t, err) - err := state.UpsertACLTokens(structs.MsgTypeTestSetup, 110, []*structs.ACLToken{token}) - assert.Nil(t, err) + // Attempt to look up the token and perform checks. + tokenResp, err := testServer.ResolveSecretToken(token.SecretID) + require.NoError(t, err) + require.NotNil(t, tokenResp) + require.Equal(t, token, tokenResp) + }, + }, + { + name: "anonymous token", + testFn: func(testServer *Server) { - respToken, err := s1.ResolveSecretToken(token.SecretID) - assert.Nil(t, err) - if assert.NotNil(t, respToken) { - assert.NotEmpty(t, respToken.AccessorID) + // Call the function with an empty input secret ID which is + // classed as representing anonymous access in clusters with + // ACLs enabled. + tokenResp, err := testServer.ResolveSecretToken("") + require.NoError(t, err) + require.NotNil(t, tokenResp) + require.Equal(t, structs.AnonymousACLToken, tokenResp) + }, + }, + { + name: "token not found", + testFn: func(testServer *Server) { + + // Call the function with randomly generated secret ID which + // does not exist within state. + tokenResp, err := testServer.ResolveSecretToken(uuid.Generate()) + require.Equal(t, structs.ErrTokenNotFound, err) + require.Nil(t, tokenResp) + }, + }, + { + name: "token expired", + testFn: func(testServer *Server) { + + // Create a mock token with an expiration time long in the + // past, and upsert. + token := mock.ACLToken() + token.ExpirationTime = pointer.Of(time.Date( + 1970, time.January, 1, 0, 0, 0, 0, time.UTC)) + + err := testServer.State().UpsertACLTokens( + structs.MsgTypeTestSetup, 10, []*structs.ACLToken{token}) + require.NoError(t, err) + + // Perform the function call which should result in finding the + // token has expired. + tokenResp, err := testServer.ResolveSecretToken(uuid.Generate()) + require.Equal(t, structs.ErrTokenNotFound, err) + require.Nil(t, tokenResp) + }, + }, } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.testFn(testServer) + }) + } } diff --git a/nomad/structs/errors.go b/nomad/structs/errors.go index b98d81476..c96a5e9d0 100644 --- a/nomad/structs/errors.go +++ b/nomad/structs/errors.go @@ -12,6 +12,7 @@ const ( errNotReadyForConsistentReads = "Not ready to serve consistent reads" errNoRegionPath = "No path to region" errTokenNotFound = "ACL token not found" + errTokenExpired = "ACL token expired" errPermissionDenied = "Permission denied" errJobRegistrationDisabled = "Job registration, dispatch, and scale are disabled by the scheduler configuration" errNoNodeConn = "No path to node" @@ -48,6 +49,7 @@ var ( ErrNotReadyForConsistentReads = errors.New(errNotReadyForConsistentReads) ErrNoRegionPath = errors.New(errNoRegionPath) ErrTokenNotFound = errors.New(errTokenNotFound) + ErrTokenExpired = errors.New(errTokenExpired) ErrPermissionDenied = errors.New(errPermissionDenied) ErrJobRegistrationDisabled = errors.New(errJobRegistrationDisabled) ErrNoNodeConn = errors.New(errNoNodeConn)