diff --git a/acl/chained_authorizer.go b/acl/chained_authorizer.go index 3d80f8be1..e43e60552 100644 --- a/acl/chained_authorizer.go +++ b/acl/chained_authorizer.go @@ -19,6 +19,10 @@ func NewChainedAuthorizer(chain []Authorizer) *ChainedAuthorizer { } } +func (c *ChainedAuthorizer) AuthorizerChain() []Authorizer { + return c.chain +} + func (c *ChainedAuthorizer) executeChain(enforce func(authz Authorizer) EnforcementDecision) EnforcementDecision { for _, authz := range c.chain { decision := enforce(authz) diff --git a/agent/consul/acl.go b/agent/consul/acl.go index 74ac14314..6d1715c59 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -97,6 +97,7 @@ type ACLResolverDelegate interface { ResolvePolicyFromID(policyID string) (bool, *structs.ACLPolicy, error) ResolveRoleFromID(roleID string) (bool, *structs.ACLRole, error) RPC(method string, args interface{}, reply interface{}) error + EnterpriseACLResolverDelegate } type policyOrRoleTokenError struct { @@ -291,7 +292,12 @@ func (r *ACLResolver) resolveTokenLegacy(token string) (acl.Authorizer, error) { return nil, err } - return policies.Compile(acl.RootAuthorizer(r.config.ACLDefaultPolicy), r.cache, r.entConf) + authz, err := policies.Compile(r.cache, r.entConf) + if err != nil { + return nil, err + } + + return acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.RootAuthorizer(r.config.ACLDefaultPolicy)}), nil } return nil, err @@ -992,7 +998,7 @@ func (r *ACLResolver) ResolveToken(token string) (acl.Authorizer, error) { defer metrics.MeasureSince([]string{"acl", "ResolveToken"}, time.Now()) - policies, err := r.resolveTokenToPolicies(token) + identity, policies, err := r.resolveTokenToIdentityAndPolicies(token) if err != nil { r.disableACLsWhenUpstreamDisabled(err) if IsACLRemoteError(err) { @@ -1004,9 +1010,27 @@ func (r *ACLResolver) ResolveToken(token string) (acl.Authorizer, error) { } // Build the Authorizer - authorizer, err := policies.Compile(acl.RootAuthorizer(r.config.ACLDefaultPolicy), r.cache, r.entConf) - return authorizer, err + var chain []acl.Authorizer + authz, err := policies.Compile(r.cache, r.entConf) + if err != nil { + return nil, err + } + chain = append(chain, authz) + + authz, err = r.resolveEnterpriseDefaultsForIdentity(identity) + if err != nil { + if IsACLRemoteError(err) { + r.logger.Printf("[ERR] consul.acl: %v", err) + return r.down, nil + } + return nil, err + } else if authz != nil { + chain = append(chain, authz) + } + + chain = append(chain, acl.RootAuthorizer(r.config.ACLDefaultPolicy)) + return acl.NewChainedAuthorizer(chain), nil } func (r *ACLResolver) ACLsEnabled() bool { diff --git a/agent/consul/acl_oss.go b/agent/consul/acl_oss.go index 5f3d4800b..be493e5a3 100644 --- a/agent/consul/acl_oss.go +++ b/agent/consul/acl_oss.go @@ -6,8 +6,16 @@ import ( "log" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/structs" ) +// EnterpriseACLResolverDelegate stub +type EnterpriseACLResolverDelegate interface{} + func newEnterpriseACLConfig(*log.Logger) *acl.EnterpriseACLConfig { return nil } + +func (r *ACLResolver) resolveEnterpriseDefaultsForIdentity(identity structs.ACLIdentity) (acl.Authorizer, error) { + return nil, nil +} diff --git a/agent/consul/acl_oss_test.go b/agent/consul/acl_oss_test.go new file mode 100644 index 000000000..5b125d942 --- /dev/null +++ b/agent/consul/acl_oss_test.go @@ -0,0 +1,28 @@ +// +build !consulent + +package consul + +import ( + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/structs" +) + +func testIdentityForTokenEnterprise(string) (bool, structs.ACLIdentity, error) { + return true, nil, acl.ErrNotFound +} + +func testPolicyForIDEnterprise(string) (bool, *structs.ACLPolicy, error) { + return true, nil, acl.ErrNotFound +} + +func testRoleForIDEnterprise(string) (bool, *structs.ACLRole, error) { + return true, nil, acl.ErrNotFound +} + +// EnterpriseACLResolverTestDelegate stub +type EnterpriseACLResolverTestDelegate struct{} + +// RPC stub for the EnterpriseACLResolverTestDelegate +func (d *EnterpriseACLResolverTestDelegate) RPC(string, interface{}, interface{}) (bool, error) { + return false, nil +} diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index 1de2ec4d8..cd9ff4f3b 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -40,6 +40,24 @@ type asyncResolutionResult struct { err error } +func verifyAuthorizerChain(t *testing.T, expected acl.Authorizer, actual acl.Authorizer) { + expectedChainAuthz, ok := expected.(*acl.ChainedAuthorizer) + require.True(t, ok, "expected Authorizer is not a ChainedAuthorizer") + actualChainAuthz, ok := actual.(*acl.ChainedAuthorizer) + require.True(t, ok, "actual Authorizer is not a ChainedAuthorizer") + + expectedChain := expectedChainAuthz.AuthorizerChain() + actualChain := actualChainAuthz.AuthorizerChain() + + require.Equal(t, len(expectedChain), len(actualChain), "ChainedAuthorizers have different length chains") + for idx, expectedAuthz := range expectedChain { + actualAuthz := actualChain[idx] + + // pointer equality - because we want to verify authorizer reuse + require.True(t, expectedAuthz == actualAuthz, "Authorizer pointers are not equal") + } +} + func resolveTokenAsync(r *ACLResolver, token string, ch chan *asyncResolutionResult) { authz, err := r.ResolveToken(token) ch <- &asyncResolutionResult{authz: authz, err: err} @@ -226,7 +244,7 @@ func testIdentityForToken(token string) (bool, structs.ACLIdentity, error) { }, }, nil default: - return true, nil, acl.ErrNotFound + return testIdentityForTokenEnterprise(token) } } @@ -289,7 +307,7 @@ func testPolicyForID(policyID string) (bool, *structs.ACLPolicy, error) { RaftIndex: structs.RaftIndex{CreateIndex: 1, ModifyIndex: 2}, }, nil default: - return true, nil, acl.ErrNotFound + return testPolicyForIDEnterprise(policyID) } } @@ -424,7 +442,7 @@ func testRoleForID(roleID string) (bool, *structs.ACLRole, error) { }, }, nil default: - return true, nil, acl.ErrNotFound + return testRoleForIDEnterprise(roleID) } } @@ -448,6 +466,8 @@ type ACLResolverTestDelegate struct { policyCached bool // state for the optional default resolver function defaultRoleResolveFn roleCached bool + + EnterpriseACLResolverTestDelegate } func (d *ACLResolverTestDelegate) Reset() { @@ -590,6 +610,9 @@ func (d *ACLResolverTestDelegate) RPC(method string, args interface{}, reply int } panic("Bad Test Implementation: should provide a roleResolveFn to the ACLResolverTestDelegate") } + if handled, err := d.EnterpriseACLResolverTestDelegate.RPC(method, args, reply); handled { + return err + } panic("Bad Test Implementation: Was the ACLResolver updated to use new RPC methods") } @@ -773,7 +796,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { authz2, err := r.ResolveToken("found") require.NoError(t, err) require.NotNil(t, authz2) - require.False(t, authz == authz2) + require.NotEqual(t, authz, authz2) require.Equal(t, acl.Deny, authz2.NodeWrite("foo", nil)) requirePolicyCached(t, r, "node-wr", false, "expired") // from "found" token @@ -839,8 +862,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { authz2, err := r.ResolveToken("found") require.NoError(t, err) require.NotNil(t, authz2) - // testing pointer equality - these will be the same object because it is cached. - require.True(t, authz == authz2) + verifyAuthorizerChain(t, authz, authz2) require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil)) }) @@ -872,7 +894,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { require.NoError(t, err) require.NotNil(t, authz2) // testing pointer equality - these will be the same object because it is cached. - require.True(t, authz == authz2) + verifyAuthorizerChain(t, authz, authz2) require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil)) }) @@ -906,7 +928,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { authz2, err := r.ResolveToken("found") require.NoError(t, err) require.NotNil(t, authz2) - require.True(t, authz == authz2) + verifyAuthorizerChain(t, authz, authz2) require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil)) requirePolicyCached(t, r, "node-wr", true, "still cached") // from "found" token @@ -941,7 +963,8 @@ func TestACLResolver_DownPolicy(t *testing.T) { authz2, err := r.ResolveToken("found-role") require.NoError(t, err) require.NotNil(t, authz2) - require.True(t, authz == authz2) + verifyAuthorizerChain(t, authz, authz2) + require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil)) }) @@ -978,12 +1001,9 @@ func TestACLResolver_DownPolicy(t *testing.T) { require.NoError(t, err) require.NotNil(t, authz2) // testing pointer equality - these will be the same object because it is cached. - require.True(t, authz == authz2) + verifyAuthorizerChain(t, authz, authz2) require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil)) - requirePolicyCached(t, r, "node-wr", true, "cached") // from "found" token - requirePolicyCached(t, r, "dc2-key-wr", true, "cached") // from "found" token - // the go routine spawned will eventually return with a authz that doesn't have the policy retry.Run(t, func(t *retry.R) { authz3, err := r.ResolveToken("found") @@ -1027,7 +1047,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { require.NoError(t, err) require.NotNil(t, authz2) // testing pointer equality - these will be the same object because it is cached. - require.True(t, authz == authz2) + verifyAuthorizerChain(t, authz, authz2) require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil)) // the go routine spawned will eventually return with a authz that doesn't have the policy @@ -1071,7 +1091,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { require.NoError(t, err) require.NotNil(t, authz2) // testing pointer equality - these will be the same object because it is cached. - require.True(t, authz == authz2) + verifyAuthorizerChain(t, authz, authz2) require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil)) }) @@ -1108,7 +1128,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { require.NoError(t, err) require.NotNil(t, authz2) // testing pointer equality - these will be the same object because it is cached. - require.True(t, authz == authz2, "\n[1]={%+v} != \n[2]={%+v}", authz, authz2) + verifyAuthorizerChain(t, authz, authz2) require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil)) }) @@ -1140,12 +1160,9 @@ func TestACLResolver_DownPolicy(t *testing.T) { authz2, err := r.ResolveToken("found") require.NoError(t, err) require.NotNil(t, authz2) - // testing pointer equality - these will be the same object because it is cached. - require.True(t, authz == authz2) + verifyAuthorizerChain(t, authz, authz2) require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil)) - requireIdentityCached(t, r, "found", true, "cached") - // the go routine spawned will eventually return and this will be a not found error retry.Run(t, func(t *retry.R) { authz3, err := r.ResolveToken("found") diff --git a/agent/structs/acl.go b/agent/structs/acl.go index 801277a0e..55f09161d 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -169,7 +169,6 @@ func (s *ACLServiceIdentity) EstimateSize() int { func (s *ACLServiceIdentity) SyntheticPolicy(entMeta *EnterpriseMeta) *ACLPolicy { // Given that we validate this string name before persisting, we do not // have to escape it before doing the following interpolation. - // TODO (namespaces) include namespace rules := aclServiceIdentityRules(s.ServiceName, entMeta) hasher := fnv.New128a() @@ -673,7 +672,7 @@ func (policies ACLPolicies) resolveWithCache(cache *ACLCaches, entConf *acl.Ente return parsed, nil } -func (policies ACLPolicies) Compile(parent acl.Authorizer, cache *ACLCaches, entConf *acl.EnterpriseACLConfig) (acl.Authorizer, error) { +func (policies ACLPolicies) Compile(cache *ACLCaches, entConf *acl.EnterpriseACLConfig) (acl.Authorizer, error) { // Determine the cache key cacheKey := policies.HashKey() entry := cache.GetAuthorizer(cacheKey) @@ -688,7 +687,7 @@ func (policies ACLPolicies) Compile(parent acl.Authorizer, cache *ACLCaches, ent } // Create the ACL object - authorizer, err := acl.NewPolicyAuthorizerWithDefaults(parent, parsed, entConf) + authorizer, err := acl.NewPolicyAuthorizer(parsed, entConf) if err != nil { return nil, fmt.Errorf("failed to construct ACL Authorizer: %v", err) } @@ -1083,8 +1082,6 @@ type ACLTokenListResponse struct { type ACLTokenBatchGetRequest struct { AccessorIDs []string // List of accessor ids to fetch Datacenter string // The datacenter to perform the request within - // TODO (namespaces) should this use enterprise meta? - it would be hard to - // update in a backwards compatible manner an accessor ids should be unique QueryOptions } @@ -1111,7 +1108,6 @@ type ACLTokenBatchSetRequest struct { // multiple tokens need to be removed from the local DCs state. type ACLTokenBatchDeleteRequest struct { TokenIDs []string // Tokens to delete - // TODO (namespaces) should we update with ent meta? } // ACLTokenBootstrapRequest is used only at the Raft layer @@ -1496,8 +1492,6 @@ func (r *ACLLoginRequest) RequestDatacenter() string { type ACLLogoutRequest struct { Datacenter string // The datacenter to perform the request within - // TODO (namespaces) do we need the ent meta here? tokens are again - // unique across namespaces so its likely we don't need it. WriteRequest } diff --git a/agent/structs/acl_test.go b/agent/structs/acl_test.go index fc18de44d..546e2113e 100644 --- a/agent/structs/acl_test.go +++ b/agent/structs/acl_test.go @@ -661,7 +661,7 @@ func TestStructs_ACLPolicies_Compile(t *testing.T) { } t.Run("Cache Miss", func(t *testing.T) { - authz, err := testPolicies.Compile(acl.DenyAll(), cache, nil) + authz, err := testPolicies.Compile(cache, nil) require.NoError(t, err) require.NotNil(t, authz) @@ -669,7 +669,7 @@ func TestStructs_ACLPolicies_Compile(t *testing.T) { require.Equal(t, acl.Allow, authz.AgentRead("foo", nil)) require.Equal(t, acl.Allow, authz.KeyRead("foo", nil)) require.Equal(t, acl.Allow, authz.ServiceRead("foo", nil)) - require.Equal(t, acl.Deny, authz.ACLRead(nil)) + require.Equal(t, acl.Default, authz.ACLRead(nil)) }) t.Run("Check Cache", func(t *testing.T) { @@ -682,18 +682,18 @@ func TestStructs_ACLPolicies_Compile(t *testing.T) { require.Equal(t, acl.Allow, authz.AgentRead("foo", nil)) require.Equal(t, acl.Allow, authz.KeyRead("foo", nil)) require.Equal(t, acl.Allow, authz.ServiceRead("foo", nil)) - require.Equal(t, acl.Deny, authz.ACLRead(nil)) + require.Equal(t, acl.Default, authz.ACLRead(nil)) // setup the cache for the next test cache.PutAuthorizer(testPolicies.HashKey(), acl.DenyAll()) }) t.Run("Cache Hit", func(t *testing.T) { - authz, err := testPolicies.Compile(acl.DenyAll(), cache, nil) + authz, err := testPolicies.Compile(cache, nil) require.NoError(t, err) require.NotNil(t, authz) - // we reset the Authorizer in the cache so now everything should be denied + // we reset the Authorizer in the cache so now everything should be defaulted require.Equal(t, acl.Deny, authz.NodeRead("foo", nil)) require.Equal(t, acl.Deny, authz.AgentRead("foo", nil)) require.Equal(t, acl.Deny, authz.KeyRead("foo", nil))