acl: remove acl == nil checks

This commit is contained in:
Daniel Nephin 2021-07-30 14:28:19 -04:00
parent 4f1a36629a
commit dc50b36b0f
10 changed files with 47 additions and 49 deletions

View File

@ -391,7 +391,7 @@ func (s *HTTPHandlers) AgentService(resp http.ResponseWriter, req *http.Request)
} }
var authzContext acl.AuthorizerContext var authzContext acl.AuthorizerContext
svc.FillAuthzContext(&authzContext) svc.FillAuthzContext(&authzContext)
if authz != nil && authz.ServiceRead(svc.Service, &authzContext) != acl.Allow { if authz.ServiceRead(svc.Service, &authzContext) != acl.Allow {
return "", nil, acl.ErrPermissionDenied return "", nil, acl.ErrPermissionDenied
} }
@ -837,7 +837,7 @@ func (s *HTTPHandlers) AgentHealthServiceByID(resp http.ResponseWriter, req *htt
dc := s.agent.config.Datacenter dc := s.agent.config.Datacenter
if service := s.agent.State.Service(sid); service != nil { if service := s.agent.State.Service(sid); service != nil {
if authz != nil && authz.ServiceRead(service.Service, &authzContext) != acl.Allow { if authz.ServiceRead(service.Service, &authzContext) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
code, status, healthChecks := agentHealthService(sid, s) code, status, healthChecks := agentHealthService(sid, s)
@ -886,7 +886,7 @@ func (s *HTTPHandlers) AgentHealthServiceByName(resp http.ResponseWriter, req *h
return nil, err return nil, err
} }
if authz != nil && authz.ServiceRead(serviceName, &authzContext) != acl.Allow { if authz.ServiceRead(serviceName, &authzContext) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }

View File

@ -65,7 +65,7 @@ func (a *Agent) ConnectAuthorize(token string,
return returnErr(err) return returnErr(err)
} }
if authz != nil && authz.ServiceWrite(req.Target, &authzContext) != acl.Allow { if authz.ServiceWrite(req.Target, &authzContext) != acl.Allow {
return returnErr(acl.ErrPermissionDenied) return returnErr(acl.ErrPermissionDenied)
} }

View File

@ -118,14 +118,14 @@ func servicePreApply(service *structs.NodeService, authz acl.Authorizer) error {
// later if version 0.8 is enabled, so we can eventually just // later if version 0.8 is enabled, so we can eventually just
// delete this and do all the ACL checks down there. // delete this and do all the ACL checks down there.
if service.Service != structs.ConsulServiceName { if service.Service != structs.ConsulServiceName {
if authz != nil && authz.ServiceWrite(service.Service, &authzContext) != acl.Allow { if authz.ServiceWrite(service.Service, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
} }
// Proxies must have write permission on their destination // Proxies must have write permission on their destination
if service.Kind == structs.ServiceKindConnectProxy { if service.Kind == structs.ServiceKindConnectProxy {
if authz != nil && authz.ServiceWrite(service.Proxy.DestinationServiceName, &authzContext) != acl.Allow { if authz.ServiceWrite(service.Proxy.DestinationServiceName, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
} }
@ -456,7 +456,7 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
// If we're doing a connect query, we need read access to the service // If we're doing a connect query, we need read access to the service
// we're trying to find proxies for, so check that. // we're trying to find proxies for, so check that.
if args.Connect { if args.Connect {
if authz != nil && authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
// Just return nil, which will return an empty response (tested) // Just return nil, which will return an empty response (tested)
return nil return nil
} }
@ -659,7 +659,7 @@ func (c *Catalog) GatewayServices(args *structs.ServiceSpecificRequest, reply *s
return err return err
} }
if authz != nil && authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

View File

@ -81,7 +81,7 @@ func (c *ConfigEntry) Apply(args *structs.ConfigEntryRequest, reply *bool) error
return err return err
} }
if authz != nil && !args.Entry.CanWrite(authz) { if !args.Entry.CanWrite(authz) {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -122,7 +122,7 @@ func (c *ConfigEntry) Get(args *structs.ConfigEntryQuery, reply *structs.ConfigE
} }
lookupEntry.GetEnterpriseMeta().Merge(&args.EnterpriseMeta) lookupEntry.GetEnterpriseMeta().Merge(&args.EnterpriseMeta)
if authz != nil && !lookupEntry.CanRead(authz) { if !lookupEntry.CanRead(authz) {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -180,7 +180,7 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe
// Filter the entries returned by ACL permissions. // Filter the entries returned by ACL permissions.
filteredEntries := make([]structs.ConfigEntry, 0, len(entries)) filteredEntries := make([]structs.ConfigEntry, 0, len(entries))
for _, entry := range entries { for _, entry := range entries {
if authz != nil && !entry.CanRead(authz) { if !entry.CanRead(authz) {
continue continue
} }
filteredEntries = append(filteredEntries, entry) filteredEntries = append(filteredEntries, entry)
@ -240,7 +240,7 @@ func (c *ConfigEntry) ListAll(args *structs.ConfigEntryListAllRequest, reply *st
// Filter the entries returned by ACL permissions or by the provided kinds. // Filter the entries returned by ACL permissions or by the provided kinds.
filteredEntries := make([]structs.ConfigEntry, 0, len(entries)) filteredEntries := make([]structs.ConfigEntry, 0, len(entries))
for _, entry := range entries { for _, entry := range entries {
if authz != nil && !entry.CanRead(authz) { if !entry.CanRead(authz) {
continue continue
} }
// Doing this filter outside of memdb isn't terribly // Doing this filter outside of memdb isn't terribly
@ -290,7 +290,7 @@ func (c *ConfigEntry) Delete(args *structs.ConfigEntryRequest, reply *struct{})
return err return err
} }
if authz != nil && !args.Entry.CanWrite(authz) { if !args.Entry.CanWrite(authz) {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -315,7 +315,7 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r
if err != nil { if err != nil {
return err return err
} }
if authz != nil && authz.ServiceRead(args.Name, &authzContext) != acl.Allow { if authz.ServiceRead(args.Name, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

View File

@ -207,7 +207,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 // 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. // we're trying to find proxies for, so check that.
if args.Connect || args.Ingress { if args.Connect || args.Ingress {
if authz != nil && authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
// Just return nil, which will return an empty response (tested) // Just return nil, which will return an empty response (tested)
return nil return nil
} }

View File

@ -690,7 +690,7 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In
if prefix, ok := query.GetACLPrefix(); ok { if prefix, ok := query.GetACLPrefix(); ok {
var authzContext acl.AuthorizerContext var authzContext acl.AuthorizerContext
query.FillAuthzContext(&authzContext) query.FillAuthzContext(&authzContext)
if authz != nil && authz.ServiceRead(prefix, &authzContext) != acl.Allow { if authz.ServiceRead(prefix, &authzContext) != acl.Allow {
accessorID := s.aclAccessorID(args.Token) accessorID := s.aclAccessorID(args.Token)
// todo(kit) Migrate intention access denial logging over to audit logging when we implement it // 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) s.logger.Warn("test on intention denied due to ACLs", "prefix", prefix, "accessorID", accessorID)

View File

@ -161,7 +161,7 @@ func (m *Internal) ServiceTopology(args *structs.ServiceSpecificRequest, reply *
if err := m.srv.validateEnterpriseRequest(&args.EnterpriseMeta, false); err != nil { if err := m.srv.validateEnterpriseRequest(&args.EnterpriseMeta, false); err != nil {
return err return err
} }
if authz != nil && authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -254,7 +254,7 @@ 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. // We need read access to the gateway we're trying to find services for, so check that first.
if authz != nil && authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow { if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -338,7 +338,7 @@ 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. // We need read access to the gateway we're trying to find intentions for, so check that first.
if authz != nil && authz.ServiceRead(args.Match.Entries[0].Name, &authzContext) != acl.Allow { if authz.ServiceRead(args.Match.Entries[0].Name, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

View File

@ -39,37 +39,35 @@ func kvsPreApply(logger hclog.Logger, srv *Server, authz acl.Authorizer, op api.
} }
// Apply the ACL policy if any. // Apply the ACL policy if any.
if authz != nil { switch op {
switch op { case api.KVDeleteTree:
case api.KVDeleteTree: var authzContext acl.AuthorizerContext
var authzContext acl.AuthorizerContext dirEnt.FillAuthzContext(&authzContext)
dirEnt.FillAuthzContext(&authzContext)
if authz.KeyWritePrefix(dirEnt.Key, &authzContext) != acl.Allow { if authz.KeyWritePrefix(dirEnt.Key, &authzContext) != acl.Allow {
return false, acl.ErrPermissionDenied return false, acl.ErrPermissionDenied
} }
case api.KVGet, api.KVGetTree: case api.KVGet, api.KVGetTree:
// Filtering for GETs is done on the output side. // Filtering for GETs is done on the output side.
case api.KVCheckSession, api.KVCheckIndex: case api.KVCheckSession, api.KVCheckIndex:
// These could reveal information based on the outcome // These could reveal information based on the outcome
// of the transaction, and they operate on individual // of the transaction, and they operate on individual
// keys so we check them here. // keys so we check them here.
var authzContext acl.AuthorizerContext var authzContext acl.AuthorizerContext
dirEnt.FillAuthzContext(&authzContext) dirEnt.FillAuthzContext(&authzContext)
if authz.KeyRead(dirEnt.Key, &authzContext) != acl.Allow { if authz.KeyRead(dirEnt.Key, &authzContext) != acl.Allow {
return false, acl.ErrPermissionDenied return false, acl.ErrPermissionDenied
} }
default: default:
var authzContext acl.AuthorizerContext var authzContext acl.AuthorizerContext
dirEnt.FillAuthzContext(&authzContext) dirEnt.FillAuthzContext(&authzContext)
if authz.KeyWrite(dirEnt.Key, &authzContext) != acl.Allow { if authz.KeyWrite(dirEnt.Key, &authzContext) != acl.Allow {
return false, acl.ErrPermissionDenied return false, acl.ErrPermissionDenied
}
} }
} }
@ -244,7 +242,7 @@ func (k *KVS) ListKeys(args *structs.KeyListRequest, reply *structs.IndexedKeyLi
return err return err
} }
if authz != nil && k.srv.config.ACLEnableKeyListPolicy && authz.KeyList(args.Prefix, &authzContext) != acl.Allow { if k.srv.config.ACLEnableKeyListPolicy && authz.KeyList(args.Prefix, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

View File

@ -310,7 +310,7 @@ func (s *Session) Renew(args *structs.SessionSpecificRequest,
return nil return nil
} }
if authz != nil && authz.SessionWrite(session.Node, &authzContext) != acl.Allow { if authz.SessionWrite(session.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

View File

@ -81,9 +81,9 @@ func (t *Txn) preCheck(authorizer acl.Authorizer, ops structs.TxnOps) structs.Tx
} }
service := &op.Service.Service service := &op.Service.Service
// This is intentionally nil as we will authorize the request // acl.ManageAll is used here because the request will be authorized
// using vetServiceTxnOp next instead of doing it in servicePreApply // later using vetServiceTxnOp.
if err := servicePreApply(service, nil); err != nil { if err := servicePreApply(service, acl.ManageAll()); err != nil {
errors = append(errors, &structs.TxnError{ errors = append(errors, &structs.TxnError{
OpIndex: i, OpIndex: i,
What: err.Error(), What: err.Error(),