status-im
/
consul
mirror of https://github.com/status-im/consul.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

acl: use acl.ManangeAll when ACLs are disabled 

Instead of returning nil and checking for nilness

Removes a bunch of nil checks, and fixes one test failures.
This commit is contained in:
Daniel Nephin 2021-07-15 17:17:49 -04:00
parent b2480d229f
commit 84fac3ce0e
16 changed files with 44 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
agent/acl.go
View File

@ -75,6 +75,7 @@ func (a *Agent) vetServiceRegisterWithAuthorizer(authz acl.Authorizer, service *
// vetServiceUpdate makes sure the service update action is allowed by the given // vetServiceUpdate makes sure the service update action is allowed by the given
// token. // token.
// TODO: move to test package
func (a *Agent) vetServiceUpdate(token string, serviceID structs.ServiceID) error { func (a *Agent) vetServiceUpdate(token string, serviceID structs.ServiceID) error {
// Resolve the token and bail if ACLs aren't enabled. // Resolve the token and bail if ACLs aren't enabled.
authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil) authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
@ -86,10 +87,6 @@ func (a *Agent) vetServiceUpdate(token string, serviceID structs.ServiceID) erro
} }
func (a *Agent) vetServiceUpdateWithAuthorizer(authz acl.Authorizer, serviceID structs.ServiceID) error { func (a *Agent) vetServiceUpdateWithAuthorizer(authz acl.Authorizer, serviceID structs.ServiceID) error {
if authz == nil {
return nil
}
var authzContext acl.AuthorizerContext var authzContext acl.AuthorizerContext
// Vet any changes based on the existing services's info. // Vet any changes based on the existing services's info.
@ -100,7 +97,7 @@ func (a *Agent) vetServiceUpdateWithAuthorizer(authz acl.Authorizer, serviceID s
return acl.PermissionDenied("Missing service:write on %s", serviceName.String()) return acl.PermissionDenied("Missing service:write on %s", serviceName.String())
} }
} else { } else {
return fmt.Errorf("Unknown service %q", serviceID) return NotFoundError{Reason: fmt.Sprintf("Unknown service %q", serviceID)}
} }
return nil return nil

2
agent/acl_endpoint.go
View File

@ -110,7 +110,7 @@ func (s *HTTPHandlers) ACLRulesTranslate(resp http.ResponseWriter, req *http.Req
} }
// Should this require lesser permissions? Really the only reason to require authorization at all is // Should this require lesser permissions? Really the only reason to require authorization at all is
// to prevent external entities from DoS Consul with repeated rule translation requests // to prevent external entities from DoS Consul with repeated rule translation requests
if rule != nil && rule.ACLRead(nil) != acl.Allow { if rule.ACLRead(nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }

20
agent/agent_endpoint.go
View File

@ -55,7 +55,7 @@ func (s *HTTPHandlers) AgentSelf(resp http.ResponseWriter, req *http.Request) (i
if err != nil { if err != nil {
return nil, err return nil, err
} }
if rule != nil && rule.AgentRead(s.agent.config.NodeName, nil) != acl.Allow { if rule.AgentRead(s.agent.config.NodeName, nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
@ -144,7 +144,7 @@ func (s *HTTPHandlers) AgentMetrics(resp http.ResponseWriter, req *http.Request)
if err != nil { if err != nil {
return nil, err return nil, err
} }
if rule != nil && rule.AgentRead(s.agent.config.NodeName, nil) != acl.Allow { if rule.AgentRead(s.agent.config.NodeName, nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
if enablePrometheusOutput(req) { if enablePrometheusOutput(req) {
@ -224,7 +224,7 @@ func (s *HTTPHandlers) AgentReload(resp http.ResponseWriter, req *http.Request)
if err != nil { if err != nil {
return nil, err return nil, err
} }
if rule != nil && rule.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow { if rule.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
@ -512,7 +512,7 @@ func (s *HTTPHandlers) AgentJoin(resp http.ResponseWriter, req *http.Request) (i
if err != nil { if err != nil {
return nil, err return nil, err
} }
if rule != nil && rule.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow { if rule.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
@ -544,7 +544,7 @@ func (s *HTTPHandlers) AgentLeave(resp http.ResponseWriter, req *http.Request) (
if err != nil { if err != nil {
return nil, err return nil, err
} }
if rule != nil && rule.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow { if rule.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
@ -562,7 +562,7 @@ func (s *HTTPHandlers) AgentForceLeave(resp http.ResponseWriter, req *http.Reque
if err != nil { if err != nil {
return nil, err return nil, err
} }
if rule != nil && rule.OperatorWrite(nil) != acl.Allow { if rule.OperatorWrite(nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
@ -1198,7 +1198,7 @@ func (s *HTTPHandlers) AgentNodeMaintenance(resp http.ResponseWriter, req *http.
if err != nil { if err != nil {
return nil, err return nil, err
} }
if rule != nil && rule.NodeWrite(s.agent.config.NodeName, nil) != acl.Allow { if rule.NodeWrite(s.agent.config.NodeName, nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
@ -1219,7 +1219,7 @@ func (s *HTTPHandlers) AgentMonitor(resp http.ResponseWriter, req *http.Request)
if err != nil { if err != nil {
return nil, err return nil, err
} }
if rule != nil && rule.AgentRead(s.agent.config.NodeName, nil) != acl.Allow { if rule.AgentRead(s.agent.config.NodeName, nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
@ -1298,7 +1298,7 @@ func (s *HTTPHandlers) AgentToken(resp http.ResponseWriter, req *http.Request) (
if err != nil { if err != nil {
return nil, err return nil, err
} }
if rule != nil && rule.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow { if rule.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }
@ -1476,7 +1476,7 @@ func (s *HTTPHandlers) AgentHost(resp http.ResponseWriter, req *http.Request) (i
return nil, err return nil, err
} }
if rule != nil && rule.OperatorRead(nil) != acl.Allow { if rule.OperatorRead(nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }

9
agent/agent_endpoint_test.go
View File

@ -4522,12 +4522,9 @@ func TestAgent_ServiceMaintenance_BadRequest(t *testing.T) {
t.Run("bad service id", func(t *testing.T) { t.Run("bad service id", func(t *testing.T) {
req, _ := http.NewRequest("PUT", "/v1/agent/service/maintenance/_nope_?enable=true", nil) req, _ := http.NewRequest("PUT", "/v1/agent/service/maintenance/_nope_?enable=true", nil)
resp := httptest.NewRecorder() resp := httptest.NewRecorder()
if _, err := a.srv.AgentServiceMaintenance(resp, req); err != nil { a.srv.h.ServeHTTP(resp, req)
t.Fatalf("err: %s", err) require.Equal(t, 404, resp.Code)
} require.Contains(t, resp.Body.String(), `Unknown service "_nope_"`)
if resp.Code != 404 {
t.Fatalf("expected 404, got %d", resp.Code)
}
}) })
} }

2
agent/consul/acl.go
View File

@ -1186,7 +1186,7 @@ func (r *ACLResolver) resolveLocallyManagedToken(token string) (structs.ACLIdent
func (r *ACLResolver) ResolveTokenToIdentityAndAuthorizer(token string) (structs.ACLIdentity, acl.Authorizer, error) { func (r *ACLResolver) ResolveTokenToIdentityAndAuthorizer(token string) (structs.ACLIdentity, acl.Authorizer, error) {
if !r.ACLsEnabled() { if !r.ACLsEnabled() {
return nil, nil, nil return nil, acl.ManageAll(), nil
} }
if acl.RootAuthorizer(token) != nil { if acl.RootAuthorizer(token) != nil {

2
agent/consul/acl_test.go
View File

@ -757,7 +757,7 @@ func TestACLResolver_Disabled(t *testing.T) {
r := newTestACLResolver(t, delegate, nil) r := newTestACLResolver(t, delegate, nil)
authz, err := r.ResolveToken("does not exist") authz, err := r.ResolveToken("does not exist")
require.Nil(t, authz) require.Equal(t, acl.ManageAll(), authz)
require.Nil(t, err) require.Nil(t, err)
} }

11
agent/consul/connect_ca_endpoint.go
View File

@ -65,7 +65,7 @@ func (s *ConnectCA) ConfigurationGet(
if err != nil { if err != nil {
return err return err
} }
if rule != nil && rule.OperatorWrite(nil) != acl.Allow { if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -97,7 +97,7 @@ func (s *ConnectCA) ConfigurationSet(
if err != nil { if err != nil {
return err return err
} }
if rule != nil && rule.OperatorWrite(nil) != acl.Allow { if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -175,7 +175,7 @@ func (s *ConnectCA) Sign(
if isService { if isService {
entMeta.Merge(serviceID.GetEnterpriseMeta()) entMeta.Merge(serviceID.GetEnterpriseMeta())
entMeta.FillAuthzContext(&authzContext) entMeta.FillAuthzContext(&authzContext)
if rule != nil && rule.ServiceWrite(serviceID.Service, &authzContext) != acl.Allow { if rule.ServiceWrite(serviceID.Service, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -186,8 +186,9 @@ func (s *ConnectCA) Sign(
"we are %s", serviceID.Datacenter, s.srv.config.Datacenter) "we are %s", serviceID.Datacenter, s.srv.config.Datacenter)
} }
} else if isAgent { } else if isAgent {
structs.DefaultEnterpriseMetaInDefaultPartition().FillAuthzContext(&authzContext) structs.DefaultEnterpriseMetaInDefaultPartition().FillAuthzContext(&authzContext)
if rule != nil && rule.NodeWrite(agentID.Agent, &authzContext) != acl.Allow { if rule.NodeWrite(agentID.Agent, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
} }
@ -223,7 +224,7 @@ func (s *ConnectCA) SignIntermediate(
if err != nil { if err != nil {
return err return err
} }
if rule != nil && rule.OperatorWrite(nil) != acl.Allow { if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

2
agent/consul/discovery_chain_endpoint.go
View File

@ -35,7 +35,7 @@ func (c *DiscoveryChain) Get(args *structs.DiscoveryChainRequest, reply *structs
if err != nil { if err != nil {
return err return err
} }
if rule != nil && rule.ServiceRead(args.Name, &authzContext) != acl.Allow { if rule.ServiceRead(args.Name, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

6
agent/consul/federation_state_endpoint.go
View File

@ -63,7 +63,7 @@ func (c *FederationState) Apply(args *structs.FederationStateRequest, reply *boo
if err != nil { if err != nil {
return err return err
} }
if rule != nil && rule.OperatorWrite(nil) != acl.Allow { if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -109,7 +109,7 @@ func (c *FederationState) Get(args *structs.FederationStateQuery, reply *structs
if err != nil { if err != nil {
return err return err
} }
if rule != nil && rule.OperatorRead(nil) != acl.Allow { if rule.OperatorRead(nil) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -150,7 +150,7 @@ func (c *FederationState) List(args *structs.DCSpecificRequest, reply *structs.I
if err != nil { if err != nil {
return err return err
} }
if rule != nil && rule.OperatorRead(nil) != acl.Allow { if rule.OperatorRead(nil) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

2
agent/consul/internal_endpoint.go
View File

@ -409,7 +409,7 @@ func (m *Internal) EventFire(args *structs.EventFireRequest,
return err return err
} }
if rule != nil && rule.EventWrite(args.Name, nil) != acl.Allow { if rule.EventWrite(args.Name, nil) != acl.Allow {
accessorID := m.aclAccessorID(args.Token) accessorID := m.aclAccessorID(args.Token)
m.logger.Warn("user event blocked by ACLs", "event", args.Name, "accessorID", accessorID) m.logger.Warn("user event blocked by ACLs", "event", args.Name, "accessorID", accessorID)
return acl.ErrPermissionDenied return acl.ErrPermissionDenied

8
agent/consul/operator_autopilot_endpoint.go
View File

@ -24,7 +24,7 @@ func (op *Operator) AutopilotGetConfiguration(args *structs.DCSpecificRequest, r
if err := op.srv.validateEnterpriseToken(identity); err != nil { if err := op.srv.validateEnterpriseToken(identity); err != nil {
return err return err
} }
if rule != nil && rule.OperatorRead(nil) != acl.Allow { if rule.OperatorRead(nil) != acl.Allow {
return acl.PermissionDenied("Missing operator:read permissions") return acl.PermissionDenied("Missing operator:read permissions")
} }
@ -56,7 +56,7 @@ func (op *Operator) AutopilotSetConfiguration(args *structs.AutopilotSetConfigRe
if err := op.srv.validateEnterpriseToken(identity); err != nil { if err := op.srv.validateEnterpriseToken(identity); err != nil {
return err return err
} }
if rule != nil && rule.OperatorWrite(nil) != acl.Allow { if rule.OperatorWrite(nil) != acl.Allow {
return acl.PermissionDenied("Missing operator:write permissions") return acl.PermissionDenied("Missing operator:write permissions")
} }
@ -91,7 +91,7 @@ func (op *Operator) ServerHealth(args *structs.DCSpecificRequest, reply *structs
if err := op.srv.validateEnterpriseToken(identity); err != nil { if err := op.srv.validateEnterpriseToken(identity); err != nil {
return err return err
} }
if rule != nil && rule.OperatorRead(nil) != acl.Allow { if rule.OperatorRead(nil) != acl.Allow {
return acl.PermissionDenied("Missing operator:read permissions") return acl.PermissionDenied("Missing operator:read permissions")
} }
@ -158,7 +158,7 @@ func (op *Operator) AutopilotState(args *structs.DCSpecificRequest, reply *autop
if err := op.srv.validateEnterpriseToken(identity); err != nil { if err := op.srv.validateEnterpriseToken(identity); err != nil {
return err return err
} }
if rule != nil && rule.OperatorRead(nil) != acl.Allow { if rule.OperatorRead(nil) != acl.Allow {
return acl.PermissionDenied("Missing operator:read permissions") return acl.PermissionDenied("Missing operator:read permissions")
} }

6
agent/consul/operator_raft_endpoint.go
View File

@ -23,7 +23,7 @@ func (op *Operator) RaftGetConfiguration(args *structs.DCSpecificRequest, reply
if err != nil { if err != nil {
return err return err
} }
if rule != nil && rule.OperatorRead(nil) != acl.Allow { if rule.OperatorRead(nil) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -88,7 +88,7 @@ func (op *Operator) RaftRemovePeerByAddress(args *structs.RaftRemovePeerRequest,
if err := op.srv.validateEnterpriseToken(identity); err != nil { if err := op.srv.validateEnterpriseToken(identity); err != nil {
return err return err
} }
if rule != nil && rule.OperatorWrite(nil) != acl.Allow { if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -141,7 +141,7 @@ func (op *Operator) RaftRemovePeerByID(args *structs.RaftRemovePeerRequest, repl
if err := op.srv.validateEnterpriseToken(identity); err != nil { if err := op.srv.validateEnterpriseToken(identity); err != nil {
return err return err
} }
if rule != nil && rule.OperatorWrite(nil) != acl.Allow { if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

4
agent/consul/prepared_query_endpoint.go
View File

@ -86,7 +86,7 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string)
// need to make sure they have write access for whatever they are // need to make sure they have write access for whatever they are
// proposing. // proposing.
if prefix, ok := args.Query.GetACLPrefix(); ok { if prefix, ok := args.Query.GetACLPrefix(); ok {
if rule != nil && rule.PreparedQueryWrite(prefix, nil) != acl.Allow { if rule.PreparedQueryWrite(prefix, nil) != acl.Allow {
p.logger.Warn("Operation on prepared query denied due to ACLs", "query", args.Query.ID) p.logger.Warn("Operation on prepared query denied due to ACLs", "query", args.Query.ID)
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }
@ -106,7 +106,7 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string)
} }
if prefix, ok := query.GetACLPrefix(); ok { if prefix, ok := query.GetACLPrefix(); ok {
if rule != nil && rule.PreparedQueryWrite(prefix, nil) != acl.Allow { if rule.PreparedQueryWrite(prefix, nil) != acl.Allow {
p.logger.Warn("Operation on prepared query denied due to ACLs", "query", args.Query.ID) p.logger.Warn("Operation on prepared query denied due to ACLs", "query", args.Query.ID)
return acl.ErrPermissionDenied return acl.ErrPermissionDenied
} }

5
agent/consul/snapshot_endpoint.go
View File

@ -16,11 +16,12 @@ import (
"net" "net"
"time" "time"
"github.com/hashicorp/go-msgpack/codec"
"github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/pool" "github.com/hashicorp/consul/agent/pool"
"github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/snapshot" "github.com/hashicorp/consul/snapshot"
"github.com/hashicorp/go-msgpack/codec"
) )
// dispatchSnapshotRequest takes an incoming request structure with possibly some // dispatchSnapshotRequest takes an incoming request structure with possibly some
@ -61,7 +62,7 @@ func (s *Server) dispatchSnapshotRequest(args *structs.SnapshotRequest, in io.Re
// all the ACLs and you could escalate from there. // all the ACLs and you could escalate from there.
if rule, err := s.ResolveToken(args.Token); err != nil { if rule, err := s.ResolveToken(args.Token); err != nil {
return nil, err return nil, err
} else if rule != nil && rule.Snapshot(nil) != acl.Allow { } else if rule.Snapshot(nil) != acl.Allow {
return nil, acl.ErrPermissionDenied return nil, acl.ErrPermissionDenied
} }

2
agent/http.go
View File

@ -253,7 +253,7 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler {
// If the token provided does not have the necessary permissions, // If the token provided does not have the necessary permissions,
// write a forbidden response // write a forbidden response
if rule != nil && rule.OperatorRead(nil) != acl.Allow { if rule.OperatorRead(nil) != acl.Allow {
resp.WriteHeader(http.StatusForbidden) resp.WriteHeader(http.StatusForbidden)
return return
} }

4
agent/xds/server.go
View File

@ -583,12 +583,12 @@ func (s *Server) checkStreamACLs(streamCtx context.Context, cfgSnap *proxycfg.Co
switch cfgSnap.Kind { switch cfgSnap.Kind {
case structs.ServiceKindConnectProxy: case structs.ServiceKindConnectProxy:
cfgSnap.ProxyID.EnterpriseMeta.FillAuthzContext(&authzContext) cfgSnap.ProxyID.EnterpriseMeta.FillAuthzContext(&authzContext)
if rule != nil && rule.ServiceWrite(cfgSnap.Proxy.DestinationServiceName, &authzContext) != acl.Allow { if rule.ServiceWrite(cfgSnap.Proxy.DestinationServiceName, &authzContext) != acl.Allow {
return status.Errorf(codes.PermissionDenied, "permission denied") return status.Errorf(codes.PermissionDenied, "permission denied")
} }
case structs.ServiceKindMeshGateway, structs.ServiceKindTerminatingGateway, structs.ServiceKindIngressGateway: case structs.ServiceKindMeshGateway, structs.ServiceKindTerminatingGateway, structs.ServiceKindIngressGateway:
cfgSnap.ProxyID.EnterpriseMeta.FillAuthzContext(&authzContext) cfgSnap.ProxyID.EnterpriseMeta.FillAuthzContext(&authzContext)
if rule != nil && rule.ServiceWrite(cfgSnap.Service, &authzContext) != acl.Allow { if rule.ServiceWrite(cfgSnap.Service, &authzContext) != acl.Allow {
return status.Errorf(codes.PermissionDenied, "permission denied") return status.Errorf(codes.PermissionDenied, "permission denied")
} }
default: default: