diff --git a/.changelog/12470.txt b/.changelog/12470.txt new file mode 100644 index 0000000000..a5395a637a --- /dev/null +++ b/.changelog/12470.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +acl: Provide fuller detail in the error messsage when an ACL denies access. +``` \ No newline at end of file diff --git a/acl/authorizer.go b/acl/authorizer.go index 8c4f7aa926..91474b467b 100644 --- a/acl/authorizer.go +++ b/acl/authorizer.go @@ -161,6 +161,279 @@ type Authorizer interface { // Embedded Interface for Consul Enterprise specific ACL enforcement enterpriseAuthorizer + + // ToAllowAuthorizer is needed until we can use ResolveResult in all the places this interface is used. + ToAllowAuthorizer() AllowAuthorizer +} + +// AllowAuthorizer is a wrapper to expose the *Allowed methods. +// This and the ToAllowAuthorizer function exist to tide us over until the ResolveResult struct +// is moved into acl. +type AllowAuthorizer struct { + Authorizer +} + +// ACLReadAllowed checks for permission to list all the ACLs +func (a AllowAuthorizer) ACLReadAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.ACLRead(ctx) != Allow { + return PermissionDeniedByACLUnnamed(a, ctx, ResourceACL, AccessRead) + } + return nil +} + +// ACLWriteAllowed checks for permission to manipulate ACLs +func (a AllowAuthorizer) ACLWriteAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.ACLWrite(ctx) != Allow { + return PermissionDeniedByACLUnnamed(a, ctx, ResourceACL, AccessWrite) + } + return nil +} + +// AgentReadAllowed checks for permission to read from agent endpoints for a +// given node. +func (a AllowAuthorizer) AgentReadAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.AgentRead(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceAgent, AccessRead, name) + } + return nil +} + +// AgentWriteAllowed checks for permission to make changes via agent endpoints +// for a given node. +func (a AllowAuthorizer) AgentWriteAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.AgentWrite(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceAgent, AccessWrite, name) + } + return nil +} + +// EventReadAllowed determines if a specific event can be queried. +func (a AllowAuthorizer) EventReadAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.EventRead(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceEvent, AccessRead, name) + } + return nil +} + +// EventWriteAllowed determines if a specific event may be fired. +func (a AllowAuthorizer) EventWriteAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.EventWrite(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceEvent, AccessWrite, name) + } + return nil +} + +// IntentionDefaultAllowAllowed determines the default authorized behavior +// when no intentions match a Connect request. +func (a AllowAuthorizer) IntentionDefaultAllowAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.IntentionDefaultAllow(ctx) != Allow { + // This is a bit nuanced, in that this isn't set by a rule, but inherited globally + // TODO(acl-error-enhancements) revisit when we have full accessor info + return PermissionDeniedError{Cause: "Denied by intention default"} + } + return nil +} + +// IntentionReadAllowed determines if a specific intention can be read. +func (a AllowAuthorizer) IntentionReadAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.IntentionRead(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceIntention, AccessRead, name) + } + return nil +} + +// IntentionWriteAllowed determines if a specific intention can be +// created, modified, or deleted. +func (a AllowAuthorizer) IntentionWriteAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.IntentionWrite(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceIntention, AccessWrite, name) + } + return nil +} + +// KeyListAllowed checks for permission to list keys under a prefix +func (a AllowAuthorizer) KeyListAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.KeyList(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceKey, AccessList, name) + } + return nil +} + +// KeyReadAllowed checks for permission to read a given key +func (a AllowAuthorizer) KeyReadAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.KeyRead(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceKey, AccessRead, name) + } + return nil +} + +// KeyWriteAllowed checks for permission to write a given key +func (a AllowAuthorizer) KeyWriteAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.KeyWrite(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceKey, AccessWrite, name) + } + return nil +} + +// KeyWritePrefixAllowed checks for permission to write to an +// entire key prefix. This means there must be no sub-policies +// that deny a write. +func (a AllowAuthorizer) KeyWritePrefixAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.KeyWritePrefix(name, ctx) != Allow { + // TODO(acl-error-enhancements) revisit this message; we may need to do some extra plumbing inside of KeyWritePrefix to + // return properly detailed information. + return PermissionDeniedByACL(a, ctx, ResourceKey, AccessWrite, name) + } + return nil +} + +// KeyringReadAllowed determines if the encryption keyring used in +// the gossip layer can be read. +func (a AllowAuthorizer) KeyringReadAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.KeyringRead(ctx) != Allow { + return PermissionDeniedByACLUnnamed(a, ctx, ResourceKeyring, AccessRead) + } + return nil +} + +// KeyringWriteAllowed determines if the keyring can be manipulated +func (a AllowAuthorizer) KeyringWriteAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.KeyringWrite(ctx) != Allow { + return PermissionDeniedByACLUnnamed(a, ctx, ResourceKeyring, AccessWrite) + } + return nil +} + +// MeshReadAllowed determines if the read-only Consul mesh functions +// can be used. +func (a AllowAuthorizer) MeshReadAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.MeshRead(ctx) != Allow { + return PermissionDeniedByACLUnnamed(a, ctx, ResourceMesh, AccessRead) + } + return nil +} + +// MeshWriteAllowed determines if the state-changing Consul mesh +// functions can be used. +func (a AllowAuthorizer) MeshWriteAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.MeshWrite(ctx) != Allow { + return PermissionDeniedByACLUnnamed(a, ctx, ResourceMesh, AccessWrite) + } + return nil +} + +// NodeReadAllowed checks for permission to read (discover) a given node. +func (a AllowAuthorizer) NodeReadAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.NodeRead(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceNode, AccessRead, name) + } + return nil +} + +// NodeReadAllAllowed checks for permission to read (discover) all nodes. +func (a AllowAuthorizer) NodeReadAllAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.NodeReadAll(ctx) != Allow { + // This is only used to gate certain UI functions right now (e.g metrics) + return PermissionDeniedByACL(a, ctx, ResourceNode, AccessRead, "all nodes") + } + return nil +} + +// NodeWriteAllowed checks for permission to create or update (register) a +// given node. +func (a AllowAuthorizer) NodeWriteAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.NodeWrite(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceNode, AccessWrite, name) + } + return nil +} + +// OperatorReadAllowed determines if the read-only Consul operator functions +// can be used. +func (a AllowAuthorizer) OperatorReadAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.OperatorRead(ctx) != Allow { + return PermissionDeniedByACLUnnamed(a, ctx, ResourceOperator, AccessRead) + } + return nil +} + +// OperatorWriteAllowed determines if the state-changing Consul operator +// functions can be used. +func (a AllowAuthorizer) OperatorWriteAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.OperatorWrite(ctx) != Allow { + return PermissionDeniedByACLUnnamed(a, ctx, ResourceOperator, AccessWrite) + } + return nil +} + +// PreparedQueryReadAllowed determines if a specific prepared query can be read +// to show its contents (this is not used for execution). +func (a AllowAuthorizer) PreparedQueryReadAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.PreparedQueryRead(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceQuery, AccessRead, name) + } + return nil +} + +// PreparedQueryWriteAllowed determines if a specific prepared query can be +// created, modified, or deleted. +func (a AllowAuthorizer) PreparedQueryWriteAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.PreparedQueryWrite(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceQuery, AccessWrite, name) + } + return nil +} + +// ServiceReadAllowed checks for permission to read a given service +func (a AllowAuthorizer) ServiceReadAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.ServiceRead(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceService, AccessRead, name) + } + return nil +} + +// ServiceReadAllAllowed checks for permission to read all services +func (a AllowAuthorizer) ServiceReadAllAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.ServiceReadAll(ctx) != Allow { + // This is only used to gate certain UI functions right now (e.g metrics) + return PermissionDeniedByACL(a, ctx, ResourceService, AccessRead, "all services") // read + } + return nil +} + +// ServiceWriteAllowed checks for permission to create or update a given +// service +func (a AllowAuthorizer) ServiceWriteAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.ServiceWrite(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceService, AccessWrite, name) + } + return nil +} + +// SessionReadAllowed checks for permission to read sessions for a given node. +func (a AllowAuthorizer) SessionReadAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.SessionRead(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceSession, AccessRead, name) + } + return nil +} + +// SessionWriteAllowed checks for permission to create sessions for a given +// node. +func (a AllowAuthorizer) SessionWriteAllowed(name string, ctx *AuthorizerContext) error { + if a.Authorizer.SessionWrite(name, ctx) != Allow { + return PermissionDeniedByACL(a, ctx, ResourceSession, AccessWrite, name) + } + return nil +} + +// SnapshotAllowed checks for permission to take and restore snapshots. +func (a AllowAuthorizer) SnapshotAllowed(ctx *AuthorizerContext) error { + if a.Authorizer.Snapshot(ctx) != Allow { + // Implementation of this currently just checks acl write + return PermissionDeniedByACLUnnamed(a, ctx, ResourceACL, AccessWrite) + } + return nil } func Enforce(authz Authorizer, rsc Resource, segment string, access string, ctx *AuthorizerContext) (EnforcementDecision, error) { diff --git a/acl/authorizer_test.go b/acl/authorizer_test.go index d74029f239..63eb57fd7b 100644 --- a/acl/authorizer_test.go +++ b/acl/authorizer_test.go @@ -204,6 +204,10 @@ func (m *mockAuthorizer) Snapshot(ctx *AuthorizerContext) EnforcementDecision { return ret.Get(0).(EnforcementDecision) } +func (p *mockAuthorizer) ToAllowAuthorizer() AllowAuthorizer { + return AllowAuthorizer{Authorizer: p} +} + func TestACL_Enforce(t *testing.T) { type testCase struct { method string diff --git a/acl/chained_authorizer.go b/acl/chained_authorizer.go index 1b3aed4978..f0d7fc3294 100644 --- a/acl/chained_authorizer.go +++ b/acl/chained_authorizer.go @@ -256,3 +256,7 @@ func (c *ChainedAuthorizer) Snapshot(entCtx *AuthorizerContext) EnforcementDecis return authz.Snapshot(entCtx) }) } + +func (c *ChainedAuthorizer) ToAllowAuthorizer() AllowAuthorizer { + return AllowAuthorizer{Authorizer: c} +} diff --git a/acl/chained_authorizer_test.go b/acl/chained_authorizer_test.go index 7a1aba2396..f6ca7184d1 100644 --- a/acl/chained_authorizer_test.go +++ b/acl/chained_authorizer_test.go @@ -99,6 +99,10 @@ func (authz testAuthorizer) Snapshot(*AuthorizerContext) EnforcementDecision { return EnforcementDecision(authz) } +func (authz testAuthorizer) ToAllowAuthorizer() AllowAuthorizer { + return AllowAuthorizer{Authorizer: &authz} +} + func TestChainedAuthorizer(t *testing.T) { t.Run("No Authorizers", func(t *testing.T) { authz := NewChainedAuthorizer([]Authorizer{}) diff --git a/acl/errors.go b/acl/errors.go index 4e0290f848..c462923395 100644 --- a/acl/errors.go +++ b/acl/errors.go @@ -106,7 +106,7 @@ func (e PermissionDeniedError) Error() string { fmt.Fprintf(&message, " lacks permission '%s:%s'", e.Resource, e.AccessLevel.String()) if e.ResourceID.Name != "" { - fmt.Fprintf(&message, " %s", e.ResourceID.ToString()) + fmt.Fprintf(&message, " on %s", e.ResourceID.ToString()) } return message.String() } diff --git a/acl/errors_test.go b/acl/errors_test.go index ea377febed..5b73a156ee 100644 --- a/acl/errors_test.go +++ b/acl/errors_test.go @@ -29,7 +29,7 @@ func TestPermissionDeniedError(t *testing.T) { }, { err: PermissionDeniedByACL(&auth1, nil, ResourceService, AccessRead, "foobar"), - expected: "Permission denied: provided accessor lacks permission 'service:read' foobar", + expected: "Permission denied: provided accessor lacks permission 'service:read' on foobar", }, { err: PermissionDeniedByACLUnnamed(&auth1, nil, ResourceService, AccessRead), diff --git a/acl/policy_authorizer.go b/acl/policy_authorizer.go index f5ef33e23b..1fdf44543b 100644 --- a/acl/policy_authorizer.go +++ b/acl/policy_authorizer.go @@ -787,3 +787,7 @@ func (p *policyAuthorizer) SessionWrite(node string, _ *AuthorizerContext) Enfor } return Default } + +func (p *policyAuthorizer) ToAllowAuthorizer() AllowAuthorizer { + return AllowAuthorizer{Authorizer: p} +} diff --git a/acl/static_authorizer.go b/acl/static_authorizer.go index f257d6b68a..1807d06847 100644 --- a/acl/static_authorizer.go +++ b/acl/static_authorizer.go @@ -240,6 +240,10 @@ func (s *staticAuthorizer) Snapshot(_ *AuthorizerContext) EnforcementDecision { return Deny } +func (s *staticAuthorizer) ToAllowAuthorizer() AllowAuthorizer { + return AllowAuthorizer{Authorizer: s} +} + // AllowAll returns an Authorizer that allows all operations func AllowAll() Authorizer { return allowAll diff --git a/acl/testing.go b/acl/testing.go new file mode 100644 index 0000000000..1d4873cd91 --- /dev/null +++ b/acl/testing.go @@ -0,0 +1,47 @@ +package acl + +import ( + "github.com/stretchr/testify/require" + "regexp" + "testing" +) + +func RequirePermissionDeniedError(t testing.TB, err error, _ Authorizer, _ *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) { + t.Helper() + if err == nil { + t.Fatal("An error is expected but got nil.") + } + if v, ok := err.(PermissionDeniedError); ok { + require.Equal(t, v.Resource, resource) + require.Equal(t, v.AccessLevel, accessLevel) + require.Equal(t, v.ResourceID.Name, resourceID) + } else { + t.Fatalf("Expected a permission denied error got %T %vp", err, err) + } +} + +func RequirePermissionDeniedMessage(t testing.TB, msg string, auth Authorizer, _ *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) { + require.NotEmpty(t, msg, "expected non-empty error message") + + var resourceIDFound string + if auth == nil { + expr := "^Permission denied" + `: provided accessor lacks permission '(\S*):(\S*)' on (.*)\s*$` + re, _ := regexp.Compile(expr) + matched := re.FindStringSubmatch(msg) + + require.Equal(t, string(resource), matched[1], "resource") + require.Equal(t, accessLevel.String(), matched[2], "access level") + resourceIDFound = matched[3] + } else { + expr := "^Permission denied" + `: accessor '(\S*)' lacks permission '(\S*):(\S*)' on (.*)\s*$` + re, _ := regexp.Compile(expr) + matched := re.FindStringSubmatch(msg) + + require.Equal(t, auth, matched[1], "auth") + require.Equal(t, string(resource), matched[2], "resource") + require.Equal(t, accessLevel.String(), matched[3], "access level") + resourceIDFound = matched[4] + } + // AuthorizerContext information should be checked here + require.Contains(t, resourceIDFound, resourceID, "resource id") +} diff --git a/agent/acl.go b/agent/acl.go index 94ec536e37..b4108b4848 100644 --- a/agent/acl.go +++ b/agent/acl.go @@ -43,15 +43,15 @@ func (a *Agent) vetServiceRegisterWithAuthorizer(authz acl.Authorizer, service * // Vet the service itself. service.FillAuthzContext(&authzContext) - if authz.ServiceWrite(service.Service, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, service.Service) + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(service.Service, &authzContext); err != nil { + return err } // Vet any service that might be getting overwritten. if existing := a.State.Service(service.CompoundServiceID()); existing != nil { existing.FillAuthzContext(&authzContext) - if authz.ServiceWrite(existing.Service, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, existing.Service) + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(existing.Service, &authzContext); err != nil { + return err } } @@ -59,8 +59,8 @@ func (a *Agent) vetServiceRegisterWithAuthorizer(authz acl.Authorizer, service * // since it can be discovered as an instance of that service. if service.Kind == structs.ServiceKindConnectProxy { service.FillAuthzContext(&authzContext) - if authz.ServiceWrite(service.Proxy.DestinationServiceName, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, service.Proxy.DestinationServiceName) + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(service.Proxy.DestinationServiceName, &authzContext); err != nil { + return err } } @@ -73,8 +73,9 @@ func (a *Agent) vetServiceUpdateWithAuthorizer(authz acl.Authorizer, serviceID s // Vet any changes based on the existing services's info. if existing := a.State.Service(serviceID); existing != nil { existing.FillAuthzContext(&authzContext) - if authz.ServiceWrite(existing.Service, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, existing.Service) + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(existing.Service, &authzContext); err != nil { + return err + } } else { // Take care if modifying this error message. @@ -95,13 +96,13 @@ func (a *Agent) vetCheckRegisterWithAuthorizer(authz acl.Authorizer, check *stru // Vet the check itself. if len(check.ServiceName) > 0 { - if authz.ServiceWrite(check.ServiceName, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, check.ServiceName) + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(check.ServiceName, &authzContext); err != nil { + return err } } else { // N.B. Should this authzContext be derived from a.AgentEnterpriseMeta() - if authz.NodeWrite(a.config.NodeName, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceNode, acl.AccessWrite, a.config.NodeName) + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(a.config.NodeName, &authzContext); err != nil { + return err } } @@ -109,13 +110,13 @@ func (a *Agent) vetCheckRegisterWithAuthorizer(authz acl.Authorizer, check *stru if existing := a.State.Check(check.CompoundCheckID()); existing != nil { if len(existing.ServiceName) > 0 { // N.B. Should this authzContext be derived from existing.EnterpriseMeta? - if authz.ServiceWrite(existing.ServiceName, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, existing.ServiceName) + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(existing.ServiceName, &authzContext); err != nil { + return err } } else { // N.B. Should this authzContext be derived from a.AgentEnterpriseMeta() - if authz.NodeWrite(a.config.NodeName, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceNode, acl.AccessWrite, a.config.NodeName) + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(a.config.NodeName, &authzContext); err != nil { + return err } } } @@ -130,12 +131,12 @@ func (a *Agent) vetCheckUpdateWithAuthorizer(authz acl.Authorizer, checkID struc // Vet any changes based on the existing check's info. if existing := a.State.Check(checkID); existing != nil { if len(existing.ServiceName) > 0 { - if authz.ServiceWrite(existing.ServiceName, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, existing.ServiceName) + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(existing.ServiceName, &authzContext); err != nil { + return err } } else { - if authz.NodeWrite(a.config.NodeName, &authzContext) != acl.Allow { - return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceNode, acl.AccessWrite, a.config.NodeName) + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(a.config.NodeName, &authzContext); err != nil { + return err } } } else { diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index 1b5804cd16..781ccddc65 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -60,8 +60,8 @@ func (s *HTTPHandlers) AgentSelf(resp http.ResponseWriter, req *http.Request) (i // Authorize using the agent's own enterprise meta, not the token. var authzContext acl.AuthorizerContext s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow { - return nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().AgentReadAllowed(s.agent.config.NodeName, &authzContext); err != nil { + return nil, err } var cs lib.CoordinateSet @@ -150,8 +150,8 @@ func (s *HTTPHandlers) AgentMetrics(resp http.ResponseWriter, req *http.Request) // Authorize using the agent's own enterprise meta, not the token. var authzContext acl.AuthorizerContext s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow { - return nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().AgentReadAllowed(s.agent.config.NodeName, &authzContext); err != nil { + return nil, err } if enablePrometheusOutput(req) { if s.agent.config.Telemetry.PrometheusOpts.Expiration < 1 { @@ -187,8 +187,8 @@ func (s *HTTPHandlers) AgentMetricsStream(resp http.ResponseWriter, req *http.Re // Authorize using the agent's own enterprise meta, not the token. var authzContext acl.AuthorizerContext s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow { - return nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().AgentReadAllowed(s.agent.config.NodeName, &authzContext); err != nil { + return nil, err } flusher, ok := resp.(http.Flusher) @@ -240,8 +240,8 @@ func (s *HTTPHandlers) AgentReload(resp http.ResponseWriter, req *http.Request) // Authorize using the agent's own enterprise meta, not the token. var authzContext acl.AuthorizerContext s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow { - return nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().AgentWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil { + return nil, err } return nil, s.agent.ReloadConfig() @@ -440,8 +440,8 @@ func (s *HTTPHandlers) AgentService(resp http.ResponseWriter, req *http.Request) } var authzContext acl.AuthorizerContext svc.FillAuthzContext(&authzContext) - if authz.ServiceRead(svc.Service, &authzContext) != acl.Allow { - return "", nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(svc.Service, &authzContext); err != nil { + return "", nil, err } // Calculate the content hash over the response, minus the hash field @@ -621,8 +621,9 @@ func (s *HTTPHandlers) AgentJoin(resp http.ResponseWriter, req *http.Request) (i // Authorize using the agent's own enterprise meta, not the token. var authzContext acl.AuthorizerContext s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow { - return nil, acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceAgent, acl.AccessWrite, s.agent.config.NodeName) + + if err := authz.ToAllowAuthorizer().AgentWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil { + return nil, err } // Get the request partition and default to that of the agent. @@ -666,8 +667,8 @@ func (s *HTTPHandlers) AgentLeave(resp http.ResponseWriter, req *http.Request) ( // Authorize using the agent's own enterprise meta, not the token. var authzContext acl.AuthorizerContext s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow { - return nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().AgentWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil { + return nil, err } if err := s.agent.Leave(); err != nil { @@ -685,8 +686,8 @@ func (s *HTTPHandlers) AgentForceLeave(resp http.ResponseWriter, req *http.Reque return nil, err } // TODO(partitions): should this be possible in a partition? - if authz.OperatorWrite(nil) != acl.Allow { - return nil, acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessWrite) + if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil { + return nil, err } // Get the request partition and default to that of the agent. @@ -1007,8 +1008,8 @@ func (s *HTTPHandlers) AgentHealthServiceByID(resp http.ResponseWriter, req *htt dc := s.agent.config.Datacenter if service := s.agent.State.Service(sid); service != nil { - if authz.ServiceRead(service.Service, &authzContext) != acl.Allow { - return nil, acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessRead, service.Service) + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(service.Service, &authzContext); err != nil { + return nil, err } code, status, healthChecks := agentHealthService(sid, s) if returnTextPlain(req) { @@ -1060,8 +1061,8 @@ func (s *HTTPHandlers) AgentHealthServiceByName(resp http.ResponseWriter, req *h return nil, err } - if authz.ServiceRead(serviceName, &authzContext) != acl.Allow { - return nil, acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessRead, serviceName) + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(serviceName, &authzContext); err != nil { + return nil, err } if !s.validateRequestPartition(resp, &entMeta) { @@ -1374,8 +1375,8 @@ func (s *HTTPHandlers) AgentNodeMaintenance(resp http.ResponseWriter, req *http. var authzContext acl.AuthorizerContext s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.NodeWrite(s.agent.config.NodeName, &authzContext) != acl.Allow { - return nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil { + return nil, err } if enable { @@ -1399,8 +1400,8 @@ func (s *HTTPHandlers) AgentMonitor(resp http.ResponseWriter, req *http.Request) // Authorize using the agent's own enterprise meta, not the token. var authzContext acl.AuthorizerContext s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow { - return nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().AgentReadAllowed(s.agent.config.NodeName, &authzContext); err != nil { + return nil, err } // Get the provided loglevel. @@ -1482,8 +1483,8 @@ func (s *HTTPHandlers) AgentToken(resp http.ResponseWriter, req *http.Request) ( // Authorize using the agent's own enterprise meta, not the token. var authzContext acl.AuthorizerContext s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow { - return nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().AgentWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil { + return nil, err } // The body is just the token, but it's in a JSON object so we can add @@ -1683,8 +1684,8 @@ func (s *HTTPHandlers) AgentHost(resp http.ResponseWriter, req *http.Request) (i } // TODO(partitions): should this be possible in a partition? - if authz.OperatorRead(nil) != acl.Allow { - return nil, acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessRead) + if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil { + return nil, err } return debug.CollectHostInfo(), nil diff --git a/agent/connect_auth.go b/agent/connect_auth.go index d27e98bd74..bc89d50afd 100644 --- a/agent/connect_auth.go +++ b/agent/connect_auth.go @@ -65,8 +65,8 @@ func (a *Agent) ConnectAuthorize(token string, return returnErr(err) } - if authz.ServiceWrite(req.Target, &authzContext) != acl.Allow { - return returnErr(acl.ErrPermissionDenied) + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(req.Target, &authzContext); err != nil { + return returnErr(err) } if !uriService.MatchesPartition(req.TargetPartition()) { diff --git a/agent/consul/acl_endpoint.go b/agent/consul/acl_endpoint.go index d19836466f..eeb2e8085c 100644 --- a/agent/consul/acl_endpoint.go +++ b/agent/consul/acl_endpoint.go @@ -286,8 +286,8 @@ func (a *ACL) TokenRead(args *structs.ACLTokenGetRequest, reply *structs.ACLToke // secrets will be redacted if authz, err = a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } } @@ -354,8 +354,8 @@ func (a *ACL) TokenClone(args *structs.ACLTokenSetRequest, reply *structs.ACLTok authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.ACLToken.EnterpriseMeta, &authzContext) if err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } _, token, err := a.srv.fsm.State().ACLTokenGetByAccessor(nil, args.ACLToken.AccessorID, &args.ACLToken.EnterpriseMeta) @@ -425,8 +425,8 @@ func (a *ACL) TokenSet(args *structs.ACLTokenSetRequest, reply *structs.ACLToken var authzContext acl.AuthorizerContext if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.ACLToken.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } return a.tokenSetInternal(args, reply, false) @@ -830,8 +830,8 @@ func (a *ACL) TokenDelete(args *structs.ACLTokenDeleteRequest, reply *string) er var authzContext acl.AuthorizerContext if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } if _, err := uuid.ParseUUID(args.TokenID); err != nil { @@ -919,8 +919,8 @@ func (a *ACL) TokenList(args *structs.ACLTokenListRequest, reply *structs.ACLTok // merge the token default meta into the requests meta args.EnterpriseMeta.Merge(&requestMeta) args.EnterpriseMeta.FillAuthzContext(&authzContext) - if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } var methodMeta *structs.EnterpriseMeta @@ -1026,8 +1026,8 @@ func (a *ACL) PolicyRead(args *structs.ACLPolicyGetRequest, reply *structs.ACLPo var authzContext acl.AuthorizerContext if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta, @@ -1107,8 +1107,8 @@ func (a *ACL) PolicySet(args *structs.ACLPolicySetRequest, reply *structs.ACLPol if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.Policy.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } policy := &args.Policy @@ -1237,8 +1237,8 @@ func (a *ACL) PolicyDelete(args *structs.ACLPolicyDeleteRequest, reply *string) if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } _, policy, err := a.srv.fsm.State().ACLPolicyGetByID(nil, args.PolicyID, &args.EnterpriseMeta) @@ -1288,8 +1288,8 @@ func (a *ACL) PolicyList(args *structs.ACLPolicyListRequest, reply *structs.ACLP authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext) if err != nil { return err - } else if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta, @@ -1412,8 +1412,8 @@ func (a *ACL) RoleRead(args *structs.ACLRoleGetRequest, reply *structs.ACLRoleRe if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta, @@ -1493,8 +1493,8 @@ func (a *ACL) RoleSet(args *structs.ACLRoleSetRequest, reply *structs.ACLRole) e if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.Role.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } role := &args.Role @@ -1651,8 +1651,8 @@ func (a *ACL) RoleDelete(args *structs.ACLRoleDeleteRequest, reply *string) erro if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } _, role, err := a.srv.fsm.State().ACLRoleGetByID(nil, args.RoleID, &args.EnterpriseMeta) @@ -1698,8 +1698,8 @@ func (a *ACL) RoleList(args *structs.ACLRoleListRequest, reply *structs.ACLRoleL authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext) if err != nil { return err - } else if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta, @@ -1797,8 +1797,8 @@ func (a *ACL) BindingRuleRead(args *structs.ACLBindingRuleGetRequest, reply *str authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext) if err != nil { return err - } else if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta, @@ -1840,8 +1840,8 @@ func (a *ACL) BindingRuleSet(args *structs.ACLBindingRuleSetRequest, reply *stru // Verify token is permitted to modify ACLs if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.BindingRule.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } var existing *structs.ACLBindingRule @@ -1969,8 +1969,8 @@ func (a *ACL) BindingRuleDelete(args *structs.ACLBindingRuleDeleteRequest, reply // Verify token is permitted to modify ACLs if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } _, rule, err := a.srv.fsm.State().ACLBindingRuleGetByID(nil, args.BindingRuleID, &args.EnterpriseMeta) @@ -2017,8 +2017,8 @@ func (a *ACL) BindingRuleList(args *structs.ACLBindingRuleListRequest, reply *st authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext) if err != nil { return err - } else if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta, @@ -2056,8 +2056,8 @@ func (a *ACL) AuthMethodRead(args *structs.ACLAuthMethodGetRequest, reply *struc if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta, @@ -2101,8 +2101,8 @@ func (a *ACL) AuthMethodSet(args *structs.ACLAuthMethodSetRequest, reply *struct if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.AuthMethod.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } method := &args.AuthMethod @@ -2213,8 +2213,8 @@ func (a *ACL) AuthMethodDelete(args *structs.ACLAuthMethodDeleteRequest, reply * if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil { return err - } else if authz.ACLWrite(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil { + return err } _, method, err := a.srv.fsm.State().ACLAuthMethodGetByName(nil, args.AuthMethodName, &args.EnterpriseMeta) @@ -2267,8 +2267,8 @@ func (a *ACL) AuthMethodList(args *structs.ACLAuthMethodListRequest, reply *stru authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext) if err != nil { return err - } else if authz.ACLRead(&authzContext) != acl.Allow { - return acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil { + return err } return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta, diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 7363516ea0..bb97dcd6a7 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -192,15 +192,15 @@ func servicePreApply(service *structs.NodeService, authz acl.Authorizer, authzCt // later if version 0.8 is enabled, so we can eventually just // delete this and do all the ACL checks down there. if service.Service != structs.ConsulServiceName { - if authz.ServiceWrite(service.Service, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(service.Service, &authzContext); err != nil { + return err } } // Proxies must have write permission on their destination if service.Kind == structs.ServiceKindConnectProxy { - if authz.ServiceWrite(service.Proxy.DestinationServiceName, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(service.Proxy.DestinationServiceName, &authzContext); err != nil { + return err } } @@ -241,16 +241,18 @@ func vetRegisterWithACL( // privileges. needsNode := ns == nil || subj.ChangesNode(ns.Node) - if needsNode && authz.NodeWrite(subj.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if needsNode { + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(subj.Node, &authzContext); err != nil { + return err + } } // Vet the service change. This includes making sure they can register // the given service, and that we can write to any existing service that // is being modified by id (if any). if subj.Service != nil { - if authz.ServiceWrite(subj.Service.Service, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(subj.Service.Service, &authzContext); err != nil { + return err } if ns != nil { @@ -263,7 +265,7 @@ func vetRegisterWithACL( var secondaryCtx acl.AuthorizerContext other.FillAuthzContext(&secondaryCtx) - if authz.ServiceWrite(other.Service, &secondaryCtx) != acl.Allow { + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(other.Service, &secondaryCtx); err != nil { return acl.ErrPermissionDenied } } @@ -293,8 +295,8 @@ func vetRegisterWithACL( // Node-level check. if check.ServiceID == "" { - if authz.NodeWrite(subj.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(subj.Node, &authzContext); err != nil { + return err } continue } @@ -324,8 +326,8 @@ func vetRegisterWithACL( var secondaryCtx acl.AuthorizerContext other.FillAuthzContext(&secondaryCtx) - if authz.ServiceWrite(other.Service, &secondaryCtx) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(other.Service, &secondaryCtx); err != nil { + return err } } @@ -408,7 +410,8 @@ func vetDeregisterWithACL( // Allow service deregistration if the token has write permission for the node. // This accounts for cases where the agent no longer has a token with write permission // on the service to deregister it. - if authz.NodeWrite(subj.Node, &authzContext) == acl.Allow { + nodeWriteErr := authz.ToAllowAuthorizer().NodeWriteAllowed(subj.Node, &authzContext) + if nodeWriteErr == nil { return nil } @@ -423,8 +426,8 @@ func vetDeregisterWithACL( ns.FillAuthzContext(&authzContext) - if authz.ServiceWrite(ns.Service, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(ns.Service, &authzContext); err != nil { + return err } } else if subj.CheckID != "" { if nc == nil { @@ -434,18 +437,18 @@ func vetDeregisterWithACL( nc.FillAuthzContext(&authzContext) if nc.ServiceID != "" { - if authz.ServiceWrite(nc.ServiceName, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(nc.ServiceName, &authzContext); err != nil { + return err } } else { - if authz.NodeWrite(subj.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(subj.Node, &authzContext); err != nil { + return err } } } else { // Since NodeWrite is not given - otherwise the earlier check // would've returned already - we can deny here. - return acl.ErrPermissionDenied + return nodeWriteErr } return nil @@ -647,6 +650,8 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru // If we're doing a connect query, we need read access to the service // we're trying to find proxies for, so check that. if args.Connect { + // TODO(acl-error-enhancements) can this be improved? What happens if we returned an error here? + // Is this similar to filters where we might want to return a hint? if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { // Just return nil, which will return an empty response (tested) return nil @@ -862,8 +867,8 @@ func (c *Catalog) GatewayServices(args *structs.ServiceSpecificRequest, reply *s return err } - if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.ServiceName, &authzContext); err != nil { + return err } return c.srv.blockingQuery( @@ -926,8 +931,8 @@ func (c *Catalog) VirtualIPForService(args *structs.ServiceSpecificRequest, repl return err } - if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.ServiceName, &authzContext); err != nil { + return err } state := c.srv.fsm.State() diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index 063d4412b4..e501aca0d6 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -90,7 +90,7 @@ func (c *ConfigEntry) Apply(args *structs.ConfigEntryRequest, reply *bool) error } if !args.Entry.CanWrite(authz) { - return acl.ErrPermissionDenied + return acl.ErrPermissionDenied // TODO(acl-error-enhancements) Better errors await refactoring of CanWrite above. } if args.Op != structs.ConfigEntryUpsert && args.Op != structs.ConfigEntryUpsertCAS { @@ -439,8 +439,8 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r if err != nil { return err } - if authz.ServiceRead(args.Name, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.Name, &authzContext); err != nil { + return err } var ( diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index a08cf27cc5..bf68611fca 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/agent/consul/connect_ca_endpoint.go @@ -65,8 +65,8 @@ func (s *ConnectCA) ConfigurationGet( if err != nil { return err } - if authz.OperatorWrite(nil) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil { + return err } state := s.srv.fsm.State() @@ -97,8 +97,8 @@ func (s *ConnectCA) ConfigurationSet( if err != nil { return err } - if authz.OperatorWrite(nil) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil { + return err } return s.srv.caManager.UpdateConfiguration(args) @@ -175,8 +175,8 @@ func (s *ConnectCA) Sign( if isService { entMeta.Merge(serviceID.GetEnterpriseMeta()) entMeta.FillAuthzContext(&authzContext) - if authz.ServiceWrite(serviceID.Service, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(serviceID.Service, &authzContext); err != nil { + return err } // Verify that the DC in the service URI matches us. We might relax this @@ -187,8 +187,8 @@ func (s *ConnectCA) Sign( } } else if isAgent { agentID.GetEnterpriseMeta().FillAuthzContext(&authzContext) - if authz.NodeWrite(agentID.Agent, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(agentID.Agent, &authzContext); err != nil { + return err } } @@ -223,8 +223,8 @@ func (s *ConnectCA) SignIntermediate( if err != nil { return err } - if authz.OperatorWrite(nil) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil { + return err } provider, _ := s.srv.caManager.getCAProvider() diff --git a/agent/consul/coordinate_endpoint.go b/agent/consul/coordinate_endpoint.go index 26aaa8536f..886df889e1 100644 --- a/agent/consul/coordinate_endpoint.go +++ b/agent/consul/coordinate_endpoint.go @@ -152,8 +152,8 @@ func (c *Coordinate) Update(args *structs.CoordinateUpdateRequest, reply *struct return err } - if authz.NodeWrite(args.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(args.Node, &authzContext); err != nil { + return err } // Add the coordinate to the map of pending updates. @@ -245,8 +245,8 @@ func (c *Coordinate) Node(args *structs.NodeSpecificRequest, reply *structs.Inde return err } - if authz.NodeRead(args.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().NodeReadAllowed(args.Node, &authzContext); err != nil { + return err } return c.srv.blockingQuery(&args.QueryOptions, diff --git a/agent/consul/discovery_chain_endpoint.go b/agent/consul/discovery_chain_endpoint.go index 501dc4f454..e354d00644 100644 --- a/agent/consul/discovery_chain_endpoint.go +++ b/agent/consul/discovery_chain_endpoint.go @@ -36,8 +36,8 @@ func (c *DiscoveryChain) Get(args *structs.DiscoveryChainRequest, reply *structs if err != nil { return err } - if authz.ServiceRead(args.Name, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.Name, &authzContext); err != nil { + return err } if args.Name == "" { diff --git a/agent/consul/federation_state_endpoint.go b/agent/consul/federation_state_endpoint.go index ccbbfff13c..6aade94883 100644 --- a/agent/consul/federation_state_endpoint.go +++ b/agent/consul/federation_state_endpoint.go @@ -9,7 +9,6 @@ import ( "github.com/armon/go-metrics/prometheus" memdb "github.com/hashicorp/go-memdb" - "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" ) @@ -63,8 +62,8 @@ func (c *FederationState) Apply(args *structs.FederationStateRequest, reply *boo if err != nil { return err } - if authz.OperatorWrite(nil) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil { + return err } if args.State == nil || args.State.Datacenter == "" { @@ -109,8 +108,8 @@ func (c *FederationState) Get(args *structs.FederationStateQuery, reply *structs if err != nil { return err } - if authz.OperatorRead(nil) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil { + return err } return c.srv.blockingQuery( @@ -148,8 +147,8 @@ func (c *FederationState) List(args *structs.DCSpecificRequest, reply *structs.I if err != nil { return err } - if authz.OperatorRead(nil) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil { + return err } return c.srv.blockingQuery( diff --git a/agent/consul/health_endpoint.go b/agent/consul/health_endpoint.go index 2fe95e6db1..f9268c21c4 100644 --- a/agent/consul/health_endpoint.go +++ b/agent/consul/health_endpoint.go @@ -220,6 +220,7 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc // If we're doing a connect or ingress query, we need read access to the service // we're trying to find proxies for, so check that. if args.Connect || args.Ingress { + // TODO(acl-error-enhancements) Look for ways to percolate this information up to give any feedback to the user. if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { // Just return nil, which will return an empty response (tested) return nil diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index b31e98ceb9..89a5f219a3 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -610,11 +610,13 @@ func (s *Intention) Match(args *structs.IntentionQueryRequest, reply *structs.In // matching, if you have it on the dest then perform a dest type match. for _, entry := range args.Match.Entries { entry.FillAuthzContext(&authzContext) - if prefix := entry.Name; prefix != "" && authz.IntentionRead(prefix, &authzContext) != acl.Allow { - accessorID := authz.AccessorID() - // todo(kit) Migrate intention access denial logging over to audit logging when we implement it - s.logger.Warn("Operation on intention prefix denied due to ACLs", "prefix", prefix, "accessorID", accessorID) - return acl.ErrPermissionDenied + if prefix := entry.Name; prefix != "" { + if err := authz.ToAllowAuthorizer().IntentionReadAllowed(prefix, &authzContext); err != nil { + accessorID := authz.AccessorID() + // todo(kit) Migrate intention access denial logging over to audit logging when we implement it + s.logger.Warn("Operation on intention prefix denied due to ACLs", "prefix", prefix, "accessorID", accessorID) + return err + } } } @@ -733,11 +735,11 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In if prefix, ok := query.GetACLPrefix(); ok { var authzContext acl.AuthorizerContext query.FillAuthzContext(&authzContext) - if authz.ServiceRead(prefix, &authzContext) != acl.Allow { + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(prefix, &authzContext); err != nil { accessorID := authz.AccessorID() // todo(kit) Migrate intention access denial logging over to audit logging when we implement it s.logger.Warn("test on intention denied due to ACLs", "prefix", prefix, "accessorID", accessorID) - return acl.ErrPermissionDenied + return err } } diff --git a/agent/consul/internal_endpoint.go b/agent/consul/internal_endpoint.go index a89ea5f547..62fb667b1c 100644 --- a/agent/consul/internal_endpoint.go +++ b/agent/consul/internal_endpoint.go @@ -163,8 +163,8 @@ func (m *Internal) ServiceTopology(args *structs.ServiceSpecificRequest, reply * if err := m.srv.validateEnterpriseRequest(&args.EnterpriseMeta, false); err != nil { return err } - if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.ServiceName, &authzContext); err != nil { + return err } return m.srv.blockingQuery( @@ -272,8 +272,8 @@ func (m *Internal) GatewayServiceDump(args *structs.ServiceSpecificRequest, repl } // We need read access to the gateway we're trying to find services for, so check that first. - if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.ServiceName, &authzContext); err != nil { + return err } err = m.srv.blockingQuery( @@ -356,8 +356,8 @@ func (m *Internal) GatewayIntentions(args *structs.IntentionQueryRequest, reply } // We need read access to the gateway we're trying to find intentions for, so check that first. - if authz.ServiceRead(args.Match.Entries[0].Name, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.Match.Entries[0].Name, &authzContext); err != nil { + return err } return m.srv.blockingQuery( @@ -428,10 +428,10 @@ func (m *Internal) EventFire(args *structs.EventFireRequest, return err } - if authz.EventWrite(args.Name, nil) != acl.Allow { + if err := authz.ToAllowAuthorizer().EventWriteAllowed(args.Name, nil); err != nil { accessorID := authz.AccessorID() m.logger.Warn("user event blocked by ACLs", "event", args.Name, "accessorID", accessorID) - return acl.ErrPermissionDenied + return err } // Set the query meta data @@ -464,16 +464,16 @@ func (m *Internal) KeyringOperation( } switch args.Operation { case structs.KeyringList: - if authz.KeyringRead(nil) != acl.Allow { - return fmt.Errorf("Reading keyring denied by ACLs") + if err := authz.ToAllowAuthorizer().KeyringReadAllowed(nil); err != nil { + return err } case structs.KeyringInstall: fallthrough case structs.KeyringUse: fallthrough case structs.KeyringRemove: - if authz.KeyringWrite(nil) != acl.Allow { - return fmt.Errorf("Modifying keyring denied due to ACLs") + if err := authz.ToAllowAuthorizer().KeyringWriteAllowed(nil); err != nil { + return err } default: panic("Invalid keyring operation") diff --git a/agent/consul/kvs_endpoint.go b/agent/consul/kvs_endpoint.go index 9befae5d6a..d90e740c6a 100644 --- a/agent/consul/kvs_endpoint.go +++ b/agent/consul/kvs_endpoint.go @@ -44,8 +44,8 @@ func kvsPreApply(logger hclog.Logger, srv *Server, authz acl.Authorizer, op api. var authzContext acl.AuthorizerContext dirEnt.FillAuthzContext(&authzContext) - if authz.KeyWritePrefix(dirEnt.Key, &authzContext) != acl.Allow { - return false, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().KeyWritePrefixAllowed(dirEnt.Key, &authzContext); err != nil { + return false, err } case api.KVGet, api.KVGetTree: @@ -58,16 +58,16 @@ func kvsPreApply(logger hclog.Logger, srv *Server, authz acl.Authorizer, op api. var authzContext acl.AuthorizerContext dirEnt.FillAuthzContext(&authzContext) - if authz.KeyRead(dirEnt.Key, &authzContext) != acl.Allow { - return false, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().KeyReadAllowed(dirEnt.Key, &authzContext); err != nil { + return false, err } default: var authzContext acl.AuthorizerContext dirEnt.FillAuthzContext(&authzContext) - if authz.KeyWrite(dirEnt.Key, &authzContext) != acl.Allow { - return false, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().KeyWriteAllowed(dirEnt.Key, &authzContext); err != nil { + return false, err } } @@ -155,8 +155,8 @@ func (k *KVS) Get(args *structs.KeyRequest, reply *structs.IndexedDirEntries) er if err != nil { return err } - if authz.KeyRead(args.Key, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().KeyReadAllowed(args.Key, &authzContext); err != nil { + return err } if ent == nil { @@ -187,8 +187,10 @@ func (k *KVS) List(args *structs.KeyRequest, reply *structs.IndexedDirEntries) e return err } - if k.srv.config.ACLEnableKeyListPolicy && authz.KeyList(args.Key, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if k.srv.config.ACLEnableKeyListPolicy { + if err := authz.ToAllowAuthorizer().KeyListAllowed(args.Key, &authzContext); err != nil { + return err + } } return k.srv.blockingQuery( @@ -240,8 +242,10 @@ func (k *KVS) ListKeys(args *structs.KeyListRequest, reply *structs.IndexedKeyLi return err } - if k.srv.config.ACLEnableKeyListPolicy && authz.KeyList(args.Prefix, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if k.srv.config.ACLEnableKeyListPolicy { + if err := authz.ToAllowAuthorizer().KeyListAllowed(args.Prefix, &authzContext); err != nil { + return err + } } return k.srv.blockingQuery( diff --git a/agent/consul/operator_autopilot_endpoint.go b/agent/consul/operator_autopilot_endpoint.go index ccda62a5e0..0b3aee53f2 100644 --- a/agent/consul/operator_autopilot_endpoint.go +++ b/agent/consul/operator_autopilot_endpoint.go @@ -2,11 +2,9 @@ package consul import ( "fmt" - autopilot "github.com/hashicorp/raft-autopilot" "github.com/hashicorp/serf/serf" - "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/structs" ) @@ -24,8 +22,9 @@ func (op *Operator) AutopilotGetConfiguration(args *structs.DCSpecificRequest, r if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil { return err } - if authz.OperatorRead(nil) != acl.Allow { - return acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessRead) + + if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil { + return err } state := op.srv.fsm.State() @@ -56,8 +55,9 @@ func (op *Operator) AutopilotSetConfiguration(args *structs.AutopilotSetConfigRe if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil { return err } - if authz.OperatorWrite(nil) != acl.Allow { - return acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessWrite) + + if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil { + return err } // Apply the update @@ -91,8 +91,9 @@ func (op *Operator) ServerHealth(args *structs.DCSpecificRequest, reply *structs if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil { return err } - if authz.OperatorRead(nil) != acl.Allow { - return acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessRead) + + if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil { + return err } state := op.srv.autopilot.GetState() @@ -158,8 +159,9 @@ func (op *Operator) AutopilotState(args *structs.DCSpecificRequest, reply *autop if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil { return err } - if authz.OperatorRead(nil) != acl.Allow { - return acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessRead) + + if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil { + return err } state := op.srv.autopilot.GetState() diff --git a/agent/consul/operator_raft_endpoint.go b/agent/consul/operator_raft_endpoint.go index 33f9ad7ff1..328f8ff964 100644 --- a/agent/consul/operator_raft_endpoint.go +++ b/agent/consul/operator_raft_endpoint.go @@ -7,7 +7,6 @@ import ( "github.com/hashicorp/raft" "github.com/hashicorp/serf/serf" - "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/metadata" "github.com/hashicorp/consul/agent/structs" ) @@ -23,8 +22,8 @@ func (op *Operator) RaftGetConfiguration(args *structs.DCSpecificRequest, reply if err != nil { return err } - if authz.OperatorRead(nil) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil { + return err } // We can't fetch the leader and the configuration atomically with @@ -88,8 +87,8 @@ func (op *Operator) RaftRemovePeerByAddress(args *structs.RaftRemovePeerRequest, if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil { return err } - if authz.OperatorWrite(nil) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil { + return err } // Since this is an operation designed for humans to use, we will return @@ -141,8 +140,8 @@ func (op *Operator) RaftRemovePeerByID(args *structs.RaftRemovePeerRequest, repl if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil { return err } - if authz.OperatorWrite(nil) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil { + return err } // Since this is an operation designed for humans to use, we will return diff --git a/agent/consul/prepared_query_endpoint.go b/agent/consul/prepared_query_endpoint.go index ce88a177c1..15f818171d 100644 --- a/agent/consul/prepared_query_endpoint.go +++ b/agent/consul/prepared_query_endpoint.go @@ -86,9 +86,9 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string) // need to make sure they have write access for whatever they are // proposing. if prefix, ok := args.Query.GetACLPrefix(); ok { - if authz.PreparedQueryWrite(prefix, nil) != acl.Allow { + if err := authz.ToAllowAuthorizer().PreparedQueryWriteAllowed(prefix, nil); err != nil { p.logger.Warn("Operation on prepared query denied due to ACLs", "query", args.Query.ID) - return acl.ErrPermissionDenied + return err } } @@ -106,9 +106,9 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string) } if prefix, ok := query.GetACLPrefix(); ok { - if authz.PreparedQueryWrite(prefix, nil) != acl.Allow { + if err := authz.ToAllowAuthorizer().PreparedQueryWriteAllowed(prefix, nil); err != nil { p.logger.Warn("Operation on prepared query denied due to ACLs", "query", args.Query.ID) - return acl.ErrPermissionDenied + return err } } } diff --git a/agent/consul/session_endpoint.go b/agent/consul/session_endpoint.go index bb65938fef..3affe357de 100644 --- a/agent/consul/session_endpoint.go +++ b/agent/consul/session_endpoint.go @@ -82,13 +82,13 @@ func (s *Session) Apply(args *structs.SessionRequest, reply *string) error { if existing == nil { return nil } - if authz.SessionWrite(existing.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().SessionWriteAllowed(existing.Node, &authzContext); err != nil { + return err } case structs.SessionCreate: - if authz.SessionWrite(args.Session.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().SessionWriteAllowed(args.Session.Node, &authzContext); err != nil { + return err } default: @@ -303,8 +303,8 @@ func (s *Session) Renew(args *structs.SessionSpecificRequest, return nil } - if authz.SessionWrite(session.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().SessionWriteAllowed(session.Node, &authzContext); err != nil { + return err } // Reset the session TTL timer. diff --git a/agent/consul/snapshot_endpoint.go b/agent/consul/snapshot_endpoint.go index ded145851d..80e255bd9e 100644 --- a/agent/consul/snapshot_endpoint.go +++ b/agent/consul/snapshot_endpoint.go @@ -18,7 +18,6 @@ import ( "github.com/hashicorp/consul-net-rpc/go-msgpack/codec" - "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/pool" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/snapshot" @@ -62,8 +61,8 @@ func (s *Server) dispatchSnapshotRequest(args *structs.SnapshotRequest, in io.Re // all the ACLs and you could escalate from there. if authz, err := s.ResolveToken(args.Token); err != nil { return nil, err - } else if authz.Snapshot(nil) != acl.Allow { - return nil, acl.ErrPermissionDenied + } else if err := authz.ToAllowAuthorizer().SnapshotAllowed(nil); err != nil { + return nil, err } // Dispatch the operation. diff --git a/agent/consul/txn_endpoint.go b/agent/consul/txn_endpoint.go index 5a17d7bfd1..6b1e65163d 100644 --- a/agent/consul/txn_endpoint.go +++ b/agent/consul/txn_endpoint.go @@ -113,8 +113,8 @@ func vetNodeTxnOp(op *structs.TxnNodeOp, authz acl.Authorizer) error { var authzContext acl.AuthorizerContext op.FillAuthzContext(&authzContext) - if authz.NodeWrite(op.Node.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(op.Node.Node, &authzContext); err != nil { + return err } return nil } @@ -126,13 +126,13 @@ func vetCheckTxnOp(op *structs.TxnCheckOp, authz acl.Authorizer) error { if op.Check.ServiceID == "" { // Node-level check. - if authz.NodeWrite(op.Check.Node, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().NodeWriteAllowed(op.Check.Node, &authzContext); err != nil { + return err } } else { // Service-level check. - if authz.ServiceWrite(op.Check.ServiceName, &authzContext) != acl.Allow { - return acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(op.Check.ServiceName, &authzContext); err != nil { + return err } } return nil diff --git a/agent/consul/txn_endpoint_test.go b/agent/consul/txn_endpoint_test.go index 0d7393dab2..3c0defc5e8 100644 --- a/agent/consul/txn_endpoint_test.go +++ b/agent/consul/txn_endpoint_test.go @@ -554,57 +554,64 @@ func TestTxn_Apply_ACLDeny(t *testing.T) { } // Verify the transaction's return value. - var expected structs.TxnResponse + var outPos int for i, op := range arg.Ops { + err := out.Errors[outPos] switch { case op.KV != nil: switch op.KV.Verb { case api.KVGet, api.KVGetTree: // These get filtered but won't result in an error. - + case api.KVSet, api.KVDelete, api.KVDeleteCAS, api.KVDeleteTree, api.KVCAS, api.KVLock, api.KVUnlock, api.KVCheckNotExists: + require.Equal(t, err.OpIndex, i) + acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceKey, acl.AccessWrite, "nope") + outPos++ default: - expected.Errors = append(expected.Errors, &structs.TxnError{ - OpIndex: i, - What: acl.ErrPermissionDenied.Error(), - }) + require.Equal(t, err.OpIndex, i) + acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceKey, acl.AccessRead, "nope") + outPos++ } case op.Node != nil: switch op.Node.Verb { case api.NodeGet: // These get filtered but won't result in an error. - + case api.NodeSet, api.NodeDelete, api.NodeDeleteCAS, api.NodeCAS: + require.Equal(t, err.OpIndex, i) + acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessWrite, "nope") + outPos++ default: - expected.Errors = append(expected.Errors, &structs.TxnError{ - OpIndex: i, - What: acl.ErrPermissionDenied.Error(), - }) + require.Equal(t, err.OpIndex, i) + acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessRead, "nope") + outPos++ } case op.Service != nil: switch op.Service.Verb { case api.ServiceGet: // These get filtered but won't result in an error. - + case api.ServiceSet, api.ServiceCAS, api.ServiceDelete, api.ServiceDeleteCAS: + require.Equal(t, err.OpIndex, i) + acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceService, acl.AccessWrite, "nope") + outPos++ default: - expected.Errors = append(expected.Errors, &structs.TxnError{ - OpIndex: i, - What: acl.ErrPermissionDenied.Error(), - }) + require.Equal(t, err.OpIndex, i) + acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceService, acl.AccessRead, "nope") + outPos++ } case op.Check != nil: switch op.Check.Verb { case api.CheckGet: // These get filtered but won't result in an error. - + case api.CheckSet, api.CheckCAS, api.CheckDelete, api.CheckDeleteCAS: + require.Equal(t, err.OpIndex, i) + acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessWrite, "nope") + outPos++ default: - expected.Errors = append(expected.Errors, &structs.TxnError{ - OpIndex: i, - What: acl.ErrPermissionDenied.Error(), - }) + require.Equal(t, err.OpIndex, i) + acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessRead, "nope") + outPos++ } } } - - require.Equal(t, expected, out) } func TestTxn_Apply_LockDelay(t *testing.T) { @@ -927,10 +934,9 @@ func TestTxn_Read_ACLDeny(t *testing.T) { var out structs.TxnReadResponse err := msgpackrpc.CallWithCodec(codec, "Txn.Read", &arg, &out) require.NoError(t, err) - require.Equal(t, structs.TxnErrors{ - {OpIndex: 0, What: acl.ErrPermissionDenied.Error()}, - {OpIndex: 1, What: acl.ErrPermissionDenied.Error()}, - }, out.Errors) + acl.RequirePermissionDeniedMessage(t, out.Errors[0].What, nil, nil, acl.ResourceKey, acl.AccessRead, "nope") + acl.RequirePermissionDeniedMessage(t, out.Errors[1].What, nil, nil, acl.ResourceKey, acl.AccessRead, "nope") + require.Empty(t, out.Results) }) } diff --git a/agent/http.go b/agent/http.go index b470547ed1..d4f3331f88 100644 --- a/agent/http.go +++ b/agent/http.go @@ -273,6 +273,7 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler { // If the token provided does not have the necessary permissions, // write a forbidden response // TODO(partitions): should this be possible in a partition? + // TODO((acl-error-enhancements)): We should return error details somehow here. if authz.OperatorRead(nil) != acl.Allow { resp.WriteHeader(http.StatusForbidden) return diff --git a/agent/ui_endpoint.go b/agent/ui_endpoint.go index cba1d1222a..f794f2f669 100644 --- a/agent/ui_endpoint.go +++ b/agent/ui_endpoint.go @@ -642,8 +642,11 @@ func (s *HTTPHandlers) UIMetricsProxy(resp http.ResponseWriter, req *http.Reques wildcardEntMeta := structs.WildcardEnterpriseMetaInPartition(structs.WildcardSpecifier) wildcardEntMeta.FillAuthzContext(&authzContext) - if authz.NodeReadAll(&authzContext) != acl.Allow || authz.ServiceReadAll(&authzContext) != acl.Allow { - return nil, acl.ErrPermissionDenied + if err := authz.ToAllowAuthorizer().NodeReadAllAllowed(&authzContext); err != nil { + return nil, err + } + if err := authz.ToAllowAuthorizer().ServiceReadAllAllowed(&authzContext); err != nil { + return nil, err } log := s.agent.logger.Named(logging.UIMetricsProxy) diff --git a/agent/xds/delta_test.go b/agent/xds/delta_test.go index de1e9e13a7..e223da80f2 100644 --- a/agent/xds/delta_test.go +++ b/agent/xds/delta_test.go @@ -1156,7 +1156,10 @@ func TestServer_DeltaAggregatedResources_v3_ACLEnforcement(t *testing.T) { case err := <-errCh: if tt.wantDenied { require.Error(t, err) - require.Contains(t, err.Error(), "permission denied") + status, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, codes.PermissionDenied, status.Code()) + require.Contains(t, err.Error(), "Permission denied") mgr.AssertWatchCancelled(t, sid) } else { require.NoError(t, err) diff --git a/agent/xds/server.go b/agent/xds/server.go index d0e1934ad3..4431eedbca 100644 --- a/agent/xds/server.go +++ b/agent/xds/server.go @@ -266,7 +266,7 @@ func (s *Server) authorize(ctx context.Context, cfgSnap *proxycfg.ConfigSnapshot if acl.IsErrNotFound(err) { return status.Errorf(codes.Unauthenticated, "unauthenticated: %v", err) } else if acl.IsErrPermissionDenied(err) { - return status.Errorf(codes.PermissionDenied, "permission denied: %v", err) + return status.Error(codes.PermissionDenied, err.Error()) } else if err != nil { return status.Errorf(codes.Internal, "error resolving acl token: %v", err) } @@ -275,13 +275,13 @@ func (s *Server) authorize(ctx context.Context, cfgSnap *proxycfg.ConfigSnapshot switch cfgSnap.Kind { case structs.ServiceKindConnectProxy: cfgSnap.ProxyID.EnterpriseMeta.FillAuthzContext(&authzContext) - if authz.ServiceWrite(cfgSnap.Proxy.DestinationServiceName, &authzContext) != acl.Allow { - return status.Errorf(codes.PermissionDenied, "permission denied") + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(cfgSnap.Proxy.DestinationServiceName, &authzContext); err != nil { + return status.Errorf(codes.PermissionDenied, err.Error()) } case structs.ServiceKindMeshGateway, structs.ServiceKindTerminatingGateway, structs.ServiceKindIngressGateway: cfgSnap.ProxyID.EnterpriseMeta.FillAuthzContext(&authzContext) - if authz.ServiceWrite(cfgSnap.Service, &authzContext) != acl.Allow { - return status.Errorf(codes.PermissionDenied, "permission denied") + if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(cfgSnap.Service, &authzContext); err != nil { + return status.Errorf(codes.PermissionDenied, err.Error()) } default: return status.Errorf(codes.Internal, "Invalid service kind")