diff --git a/agent/acl.go b/agent/acl.go index f98340628..91f53430d 100644 --- a/agent/acl.go +++ b/agent/acl.go @@ -42,25 +42,15 @@ func (a *Agent) resolveTokenAndDefaultMeta(id string, entMeta *structs.Enterpris } // resolveIdentityFromToken is used to resolve an ACLToken's secretID to a structs.ACLIdentity -func (a *Agent) resolveIdentityFromToken(secretID string) (bool, structs.ACLIdentity, error) { - // ACLs are disabled - if !a.delegate.ACLsEnabled() { - return false, nil, nil - } - - // Disable ACLs if version 8 enforcement isn't enabled. - if !a.config.ACLEnforceVersion8 { - return false, nil, nil - } - - return a.delegate.ResolveIdentityFromToken(secretID) +func (a *Agent) resolveIdentityFromToken(secretID string) (structs.ACLIdentity, error) { + return a.delegate.ResolveTokenToIdentity(secretID) } // aclAccessorID is used to convert an ACLToken's secretID to its accessorID for non- // critical purposes, such as logging. Therefore we interpret all errors as empty-string // so we can safely log it without handling non-critical errors at the usage site. func (a *Agent) aclAccessorID(secretID string) string { - _, ident, err := a.resolveIdentityFromToken(secretID) + ident, err := a.resolveIdentityFromToken(secretID) if acl.IsErrNotFound(err) { return "" } diff --git a/agent/acl_test.go b/agent/acl_test.go index ef2a74f19..b3bdc9439 100644 --- a/agent/acl_test.go +++ b/agent/acl_test.go @@ -21,6 +21,9 @@ import ( "github.com/stretchr/testify/require" ) +type authzResolver func(string) (structs.ACLIdentity, acl.Authorizer, error) +type identResolver func(string) (structs.ACLIdentity, error) + type TestACLAgent struct { // Name is an optional name of the agent. Name string @@ -43,16 +46,17 @@ type TestACLAgent struct { // Shutdown() is called. DataDir string - resolveTokenFn func(string) (structs.ACLIdentity, acl.Authorizer, error) + resolveAuthzFn authzResolver + resolveIdentFn identResolver *Agent } -// NewTestACLAGent does just enough so that all the code within agent/acl.go can work +// NewTestACLAgent does just enough so that all the code within agent/acl.go can work // Basically it needs a local state for some of the vet* functions, a logger and a delegate. // The key is that we are the delegate so we can control the ResolveToken responses -func NewTestACLAgent(t *testing.T, name string, hcl string, resolveFn func(string) (structs.ACLIdentity, acl.Authorizer, error)) *TestACLAgent { - a := &TestACLAgent{Name: name, HCL: hcl, resolveTokenFn: resolveFn} +func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzResolver, resolveIdent identResolver) *TestACLAgent { + a := &TestACLAgent{Name: name, HCL: hcl, resolveAuthzFn: resolveAuthz, resolveIdentFn: resolveIdent} hclDataDir := `data_dir = "acl-agent"` logOutput := testutil.TestWriter(t) @@ -68,9 +72,7 @@ func NewTestACLAgent(t *testing.T, name string, hcl string, resolveFn func(strin ) agent, err := New(a.Config, logger) - if err != nil { - panic(fmt.Sprintf("Error creating agent: %v", err)) - } + require.NoError(t, err) a.Agent = agent agent.LogOutput = logOutput @@ -93,20 +95,28 @@ func (a *TestACLAgent) UseLegacyACLs() bool { } func (a *TestACLAgent) ResolveToken(secretID string) (acl.Authorizer, error) { - if a.resolveTokenFn == nil { - panic("This agent is useless without providing a token resolution function") + if a.resolveAuthzFn == nil { + return nil, fmt.Errorf("ResolveToken call is unexpected - no authz resolver callback set") } - _, authz, err := a.resolveTokenFn(secretID) + _, authz, err := a.resolveAuthzFn(secretID) return authz, err } func (a *TestACLAgent) ResolveTokenToIdentityAndAuthorizer(secretID string) (structs.ACLIdentity, acl.Authorizer, error) { - if a.resolveTokenFn == nil { - panic("This agent is useless without providing a token resolution function") + if a.resolveAuthzFn == nil { + return nil, nil, fmt.Errorf("ResolveTokenToIdentityAndAuthorizer call is unexpected - no authz resolver callback set") } - return a.resolveTokenFn(secretID) + return a.resolveAuthzFn(secretID) +} + +func (a *TestACLAgent) ResolveTokenToIdentity(secretID string) (structs.ACLIdentity, error) { + if a.resolveIdentFn == nil { + return nil, fmt.Errorf("ResolveTokenToIdentity call is unexpected - no ident resolver callback set") + } + + return a.resolveIdentFn(secretID) } func (a *TestACLAgent) ResolveTokenAndDefaultMeta(secretID string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (acl.Authorizer, error) { @@ -129,19 +139,6 @@ func (a *TestACLAgent) ResolveTokenAndDefaultMeta(secretID string, entMeta *stru return authz, err } -func (a *TestACLAgent) ResolveIdentityFromToken(secretID string) (bool, structs.ACLIdentity, error) { - if a.resolveTokenFn == nil { - panic("This agent is useless without providing a token resolution function") - } - - identity, _, err := a.resolveTokenFn(secretID) - if err != nil { - return true, nil, err - } - - return true, identity, nil -} - // All of these are stubs to satisfy the interface func (a *TestACLAgent) GetLANCoordinate() (lib.CoordinateSet, error) { return nil, fmt.Errorf("Unimplemented") @@ -188,14 +185,9 @@ func TestACL_Version8(t *testing.T) { t.Parallel() t.Run("version 8 disabled", func(t *testing.T) { - resolveFn := func(string) (structs.ACLIdentity, acl.Authorizer, error) { - require.Fail(t, "should not have called delegate.ResolveToken") - return nil, nil, fmt.Errorf("should not have called delegate.ResolveToken") - } - a := NewTestACLAgent(t, t.Name(), TestACLConfig()+` acl_enforce_version_8 = false - `, resolveFn) + `, nil, nil) token, err := a.resolveToken("nope") require.Nil(t, token) @@ -210,7 +202,7 @@ func TestACL_Version8(t *testing.T) { } a := NewTestACLAgent(t, t.Name(), TestACLConfig()+` acl_enforce_version_8 = true - `, resolveFn) + `, resolveFn, nil) _, err := a.resolveToken("nope") require.Error(t, err) @@ -221,12 +213,7 @@ func TestACL_Version8(t *testing.T) { func TestACL_AgentMasterToken(t *testing.T) { t.Parallel() - resolveFn := func(string) (structs.ACLIdentity, acl.Authorizer, error) { - require.Fail(t, "should not have called delegate.ResolveToken") - return nil, nil, fmt.Errorf("should not have called delegate.ResolveToken") - } - - a := NewTestACLAgent(t, t.Name(), TestACLConfig(), resolveFn) + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), nil, nil) a.loadTokens(a.config) authz, err := a.resolveToken("towel") require.NotNil(t, authz) @@ -241,12 +228,7 @@ func TestACL_AgentMasterToken(t *testing.T) { func TestACL_RootAuthorizersDenied(t *testing.T) { t.Parallel() - resolveFn := func(string) (structs.ACLIdentity, acl.Authorizer, error) { - require.Fail(t, "should not have called delegate.ResolveToken") - return nil, nil, fmt.Errorf("should not have called delegate.ResolveToken") - } - - a := NewTestACLAgent(t, t.Name(), TestACLConfig(), resolveFn) + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), nil, nil) authz, err := a.resolveToken("deny") require.Nil(t, authz) require.Error(t, err) @@ -280,35 +262,35 @@ var ( otherRWSecret = "a38e8016-91b6-4876-b3e7-a307abbb2002" testTokens = map[string]testToken{ - nodeROSecret: testToken{ + nodeROSecret: { token: structs.ACLToken{ AccessorID: "9df2d1a4-2d07-414e-8ead-6053f56ed2eb", SecretID: nodeROSecret, }, rules: `node_prefix "Node" { policy = "read" }`, }, - nodeRWSecret: testToken{ + nodeRWSecret: { token: structs.ACLToken{ AccessorID: "efb6b7d5-d343-47c1-b4cb-aa6b94d2f490", SecretID: nodeROSecret, }, rules: `node_prefix "Node" { policy = "write" }`, }, - serviceROSecret: testToken{ + serviceROSecret: { token: structs.ACLToken{ AccessorID: "0da53edb-36e5-4603-9c31-79965bad45f5", SecretID: serviceROSecret, }, rules: `service_prefix "service" { policy = "read" }`, }, - serviceRWSecret: testToken{ + serviceRWSecret: { token: structs.ACLToken{ AccessorID: "52504258-137a-41e6-9326-01f40e80872e", SecretID: serviceRWSecret, }, rules: `service_prefix "service" { policy = "write" }`, }, - otherRWSecret: testToken{ + otherRWSecret: { token: structs.ACLToken{ AccessorID: "5e032c5b-c39e-4552-b5ad-8a9365b099c4", SecretID: otherRWSecret, @@ -333,9 +315,18 @@ func catalogPolicy(token string) (structs.ACLIdentity, acl.Authorizer, error) { return &tok.token, authz, err } +func catalogIdent(token string) (structs.ACLIdentity, error) { + tok, ok := testTokens[token] + if !ok { + return nil, acl.ErrNotFound + } + + return &tok.token, nil +} + func TestACL_vetServiceRegister(t *testing.T) { t.Parallel() - a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy) + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent) // Register a new service, with permission. err := a.vetServiceRegister(serviceRWSecret, &structs.NodeService{ @@ -366,7 +357,7 @@ func TestACL_vetServiceRegister(t *testing.T) { func TestACL_vetServiceUpdate(t *testing.T) { t.Parallel() - a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy) + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent) // Update a service that doesn't exist. err := a.vetServiceUpdate(serviceRWSecret, structs.NewServiceID("my-service", nil)) @@ -389,7 +380,7 @@ func TestACL_vetServiceUpdate(t *testing.T) { func TestACL_vetCheckRegister(t *testing.T) { t.Parallel() - a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy) + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent) // Register a new service check with write privs. err := a.vetCheckRegister(serviceRWSecret, &structs.HealthCheck{ @@ -455,7 +446,7 @@ func TestACL_vetCheckRegister(t *testing.T) { func TestACL_vetCheckUpdate(t *testing.T) { t.Parallel() - a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy) + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent) // Update a check that doesn't exist. err := a.vetCheckUpdate(nodeRWSecret, structs.NewCheckID("my-check", nil)) @@ -495,7 +486,7 @@ func TestACL_vetCheckUpdate(t *testing.T) { func TestACL_filterMembers(t *testing.T) { t.Parallel() - a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy) + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent) var members []serf.Member require.NoError(t, a.filterMembers(nodeROSecret, &members)) @@ -514,7 +505,7 @@ func TestACL_filterMembers(t *testing.T) { func TestACL_filterServices(t *testing.T) { t.Parallel() - a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy) + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent) services := make(map[structs.ServiceID]*structs.NodeService) require.NoError(t, a.filterServices(nodeROSecret, &services)) @@ -528,7 +519,7 @@ func TestACL_filterServices(t *testing.T) { func TestACL_filterChecks(t *testing.T) { t.Parallel() - a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy) + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent) checks := make(map[structs.CheckID]*structs.HealthCheck) require.NoError(t, a.filterChecks(nodeROSecret, &checks)) @@ -555,3 +546,21 @@ func TestACL_filterChecks(t *testing.T) { _, ok = checks[structs.NewCheckID("my-other", nil)] require.False(t, ok) } + +func TestACL_ResolveIdentity(t *testing.T) { + t.Parallel() + a := NewTestACLAgent(t, t.Name(), TestACLConfig(), nil, catalogIdent) + + // this test is meant to ensure we are calling the correct function + // which is ResolveTokenToIdentity on the Agent delegate. Our + // nil authz resolver will cause it to emit an error if used + ident, err := a.resolveIdentityFromToken(nodeROSecret) + require.NoError(t, err) + require.NotNil(t, ident) + + // just double checkingto ensure if we had used the wrong function + // that an error would be produced + _, err = a.resolveToken(nodeROSecret) + require.Error(t, err) + +} diff --git a/agent/agent.go b/agent/agent.go index 100be93d7..79c534756 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -136,8 +136,8 @@ type delegate interface { JoinLAN(addrs []string) (n int, err error) RemoveFailedNode(node string, prune bool) error ResolveToken(secretID string) (acl.Authorizer, error) + ResolveTokenToIdentity(secretID string) (structs.ACLIdentity, error) ResolveTokenAndDefaultMeta(secretID string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (acl.Authorizer, error) - ResolveIdentityFromToken(secretID string) (bool, structs.ACLIdentity, error) RPC(method string, args interface{}, reply interface{}) error ACLsEnabled() bool UseLegacyACLs() bool diff --git a/agent/consul/acl.go b/agent/consul/acl.go index cccb54a02..6460f66fd 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -1114,6 +1114,30 @@ func (r *ACLResolver) ResolveToken(token string) (acl.Authorizer, error) { return authz, err } +func (r *ACLResolver) ResolveTokenToIdentity(token string) (structs.ACLIdentity, error) { + if !r.ACLsEnabled() { + return nil, nil + } + + if acl.RootAuthorizer(token) != nil { + return nil, acl.ErrRootDenied + } + + // handle the anonymous token + if token == "" { + token = anonymousToken + } + + if r.delegate.UseLegacyACLs() { + identity, _, err := r.resolveTokenLegacy(token) + return identity, r.disableACLsWhenUpstreamDisabled(err) + } + + defer metrics.MeasureSince([]string{"acl", "ResolveTokenToIdentity"}, time.Now()) + + return r.resolveIdentityFromToken(token) +} + func (r *ACLResolver) ACLsEnabled() bool { // Whether we desire ACLs to be enabled according to configuration if !r.delegate.ACLsEnabled() { diff --git a/agent/consul/acl_client.go b/agent/consul/acl_client.go index e2e14dca3..e9e501fa5 100644 --- a/agent/consul/acl_client.go +++ b/agent/consul/acl_client.go @@ -94,6 +94,13 @@ func (c *Client) ResolveToken(token string) (acl.Authorizer, error) { return c.acls.ResolveToken(token) } +func (c *Client) ResolveTokenToIdentity(token string) (structs.ACLIdentity, error) { + // not using ResolveTokenToIdentityAndAuthorizer because in this case we don't + // need to resolve the roles, policies and namespace but just want the identity + // information such as accessor id. + return c.acls.ResolveTokenToIdentity(token) +} + func (c *Client) ResolveTokenToIdentityAndAuthorizer(token string) (structs.ACLIdentity, acl.Authorizer, error) { return c.acls.ResolveTokenToIdentityAndAuthorizer(token) } diff --git a/agent/consul/acl_server.go b/agent/consul/acl_server.go index cec67062b..a6892b545 100644 --- a/agent/consul/acl_server.go +++ b/agent/consul/acl_server.go @@ -222,6 +222,13 @@ func (s *Server) ResolveToken(token string) (acl.Authorizer, error) { return authz, err } +func (s *Server) ResolveTokenToIdentity(token string) (structs.ACLIdentity, error) { + // not using ResolveTokenToIdentityAndAuthorizer because in this case we don't + // need to resolve the roles, policies and namespace but just want the identity + // information such as accessor id. + return s.acls.ResolveTokenToIdentity(token) +} + func (s *Server) ResolveTokenToIdentityAndAuthorizer(token string) (structs.ACLIdentity, acl.Authorizer, error) { if id, authz := s.ResolveEntTokenToIdentityAndAuthorizer(token); id != nil && authz != nil { return id, authz, nil diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index c39e0ceae..73816c36a 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -462,6 +462,14 @@ type ACLResolverTestDelegate struct { policyResolveFn func(*structs.ACLPolicyBatchGetRequest, *structs.ACLPolicyBatchResponse) error roleResolveFn func(*structs.ACLRoleBatchGetRequest, *structs.ACLRoleBatchResponse) error + localTokenResolutions int32 + remoteTokenResolutions int32 + localPolicyResolutions int32 + remotePolicyResolutions int32 + localRoleResolutions int32 + remoteRoleResolutions int32 + remoteLegacyResolutions int32 + // state for the optional default resolver function defaultTokenReadFn tokenCached bool // state for the optional default resolver function defaultPolicyResolveFn @@ -570,6 +578,7 @@ func (d *ACLResolverTestDelegate) ResolveIdentityFromToken(token string) (bool, return false, nil, nil } + atomic.AddInt32(&d.localTokenResolutions, 1) return testIdentityForToken(token) } @@ -578,6 +587,7 @@ func (d *ACLResolverTestDelegate) ResolvePolicyFromID(policyID string) (bool, *s return false, nil, nil } + atomic.AddInt32(&d.localPolicyResolutions, 1) return testPolicyForID(policyID) } @@ -586,27 +596,32 @@ func (d *ACLResolverTestDelegate) ResolveRoleFromID(roleID string) (bool, *struc return false, nil, nil } + atomic.AddInt32(&d.localRoleResolutions, 1) return testRoleForID(roleID) } func (d *ACLResolverTestDelegate) RPC(method string, args interface{}, reply interface{}) error { switch method { case "ACL.GetPolicy": + atomic.AddInt32(&d.remoteLegacyResolutions, 1) if d.getPolicyFn != nil { return d.getPolicyFn(args.(*structs.ACLPolicyResolveLegacyRequest), reply.(*structs.ACLPolicyResolveLegacyResponse)) } panic("Bad Test Implementation: should provide a getPolicyFn to the ACLResolverTestDelegate") case "ACL.TokenRead": + atomic.AddInt32(&d.remoteTokenResolutions, 1) if d.tokenReadFn != nil { return d.tokenReadFn(args.(*structs.ACLTokenGetRequest), reply.(*structs.ACLTokenResponse)) } panic("Bad Test Implementation: should provide a tokenReadFn to the ACLResolverTestDelegate") case "ACL.PolicyResolve": + atomic.AddInt32(&d.remotePolicyResolutions, 1) if d.policyResolveFn != nil { return d.policyResolveFn(args.(*structs.ACLPolicyBatchGetRequest), reply.(*structs.ACLPolicyBatchResponse)) } panic("Bad Test Implementation: should provide a policyResolveFn to the ACLResolverTestDelegate") case "ACL.RoleResolve": + atomic.AddInt32(&d.remoteRoleResolutions, 1) if d.roleResolveFn != nil { return d.roleResolveFn(args.(*structs.ACLRoleBatchGetRequest), reply.(*structs.ACLRoleBatchResponse)) } @@ -1446,6 +1461,79 @@ func TestACLResolver_Client(t *testing.T) { require.Equal(t, policyResolves, int32(3)) }) + t.Run("Resolve-Identity", func(t *testing.T) { + t.Parallel() + + delegate := &ACLResolverTestDelegate{ + enabled: true, + datacenter: "dc1", + legacy: false, + localTokens: false, + localPolicies: false, + } + + delegate.tokenReadFn = delegate.plainTokenReadFn + delegate.policyResolveFn = delegate.plainPolicyResolveFn + delegate.roleResolveFn = delegate.plainRoleResolveFn + + r := newTestACLResolver(t, delegate, nil) + + ident, err := r.ResolveTokenToIdentity("found-policy-and-role") + require.NoError(t, err) + require.NotNil(t, ident) + require.Equal(t, "5f57c1f6-6a89-4186-9445-531b316e01df", ident.ID()) + require.EqualValues(t, 0, delegate.localTokenResolutions) + require.EqualValues(t, 1, delegate.remoteTokenResolutions) + require.EqualValues(t, 0, delegate.localPolicyResolutions) + require.EqualValues(t, 0, delegate.remotePolicyResolutions) + require.EqualValues(t, 0, delegate.localRoleResolutions) + require.EqualValues(t, 0, delegate.remoteRoleResolutions) + require.EqualValues(t, 0, delegate.remoteLegacyResolutions) + }) + + t.Run("Resolve-Identity-Legacy", func(t *testing.T) { + t.Parallel() + + delegate := &ACLResolverTestDelegate{ + enabled: true, + datacenter: "dc1", + legacy: true, + localTokens: false, + localPolicies: false, + getPolicyFn: func(args *structs.ACLPolicyResolveLegacyRequest, reply *structs.ACLPolicyResolveLegacyResponse) error { + reply.Parent = "deny" + reply.TTL = 30 + reply.ETag = "nothing" + reply.Policy = &acl.Policy{ + ID: "not-needed", + PolicyRules: acl.PolicyRules{ + Nodes: []*acl.NodeRule{ + &acl.NodeRule{ + Name: "foo", + Policy: acl.PolicyWrite, + }, + }, + }, + } + return nil + }, + } + + r := newTestACLResolver(t, delegate, nil) + + ident, err := r.ResolveTokenToIdentity("found-policy-and-role") + require.NoError(t, err) + require.NotNil(t, ident) + require.Equal(t, "legacy-token", ident.ID()) + require.EqualValues(t, 0, delegate.localTokenResolutions) + require.EqualValues(t, 0, delegate.remoteTokenResolutions) + require.EqualValues(t, 0, delegate.localPolicyResolutions) + require.EqualValues(t, 0, delegate.remotePolicyResolutions) + require.EqualValues(t, 0, delegate.localRoleResolutions) + require.EqualValues(t, 0, delegate.remoteRoleResolutions) + require.EqualValues(t, 1, delegate.remoteLegacyResolutions) + }) + t.Run("Concurrent-Token-Resolve", func(t *testing.T) { t.Parallel() diff --git a/agent/local/state.go b/agent/local/state.go index 3bfa3956f..460009a0d 100644 --- a/agent/local/state.go +++ b/agent/local/state.go @@ -121,7 +121,7 @@ func (c *CheckState) CriticalFor() time.Duration { type rpc interface { RPC(method string, args interface{}, reply interface{}) error - ResolveIdentityFromToken(secretID string) (bool, structs.ACLIdentity, error) + ResolveTokenToIdentity(secretID string) (structs.ACLIdentity, error) } // State is used to represent the node's services, @@ -1364,7 +1364,7 @@ func (l *State) notifyIfAliased(serviceID structs.ServiceID) { // critical purposes, such as logging. Therefore we interpret all errors as empty-string // so we can safely log it without handling non-critical errors at the usage site. func (l *State) aclAccessorID(secretID string) string { - _, ident, err := l.Delegate.ResolveIdentityFromToken(secretID) + ident, err := l.Delegate.ResolveTokenToIdentity(secretID) if acl.IsErrNotFound(err) { return "" }