luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Merge pull request #10632 from hashicorp/pairing/acl-authorizer-when-acl-disabled 

acls: Update ACL authorizer to return meaningful permission when ACLs are disabled
This commit is contained in:
Daniel Nephin 2021-07-30 13:22:55 -04:00 committed by GitHub
parent 766b64eac2 da96c645b2
commit 475fec5670
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 52 additions and 52 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
// token.
// TODO: move to test package
func (a *Agent) vetServiceUpdate(token string, serviceID structs.ServiceID) error {
// Resolve the token and bail if ACLs aren't enabled.
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 {
if authz == nil {
return nil
}
var authzContext acl.AuthorizerContext
// 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())
}
} else {
return fmt.Errorf("Unknown service %q", serviceID)
return NotFoundError{Reason: fmt.Sprintf("Unknown service %q", serviceID)}
}
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
// 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
}

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 {
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
}
@ -144,7 +144,7 @@ func (s *HTTPHandlers) AgentMetrics(resp http.ResponseWriter, req *http.Request)
if err != nil {
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
}
if enablePrometheusOutput(req) {
@ -224,7 +224,7 @@ func (s *HTTPHandlers) AgentReload(resp http.ResponseWriter, req *http.Request)
if err != nil {
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
}
@ -512,7 +512,7 @@ func (s *HTTPHandlers) AgentJoin(resp http.ResponseWriter, req *http.Request) (i
if err != nil {
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
}
@ -544,7 +544,7 @@ func (s *HTTPHandlers) AgentLeave(resp http.ResponseWriter, req *http.Request) (
if err != nil {
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
}
@ -562,7 +562,7 @@ func (s *HTTPHandlers) AgentForceLeave(resp http.ResponseWriter, req *http.Reque
if err != nil {
return nil, err
}
if rule != nil && rule.OperatorWrite(nil) != acl.Allow {
if rule.OperatorWrite(nil) != acl.Allow {
return nil, acl.ErrPermissionDenied
}
@ -1198,7 +1198,7 @@ func (s *HTTPHandlers) AgentNodeMaintenance(resp http.ResponseWriter, req *http.
if err != nil {
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
}
@ -1219,7 +1219,7 @@ func (s *HTTPHandlers) AgentMonitor(resp http.ResponseWriter, req *http.Request)
if err != nil {
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
}
@ -1298,7 +1298,7 @@ func (s *HTTPHandlers) AgentToken(resp http.ResponseWriter, req *http.Request) (
if err != nil {
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
}
@ -1476,7 +1476,7 @@ func (s *HTTPHandlers) AgentHost(resp http.ResponseWriter, req *http.Request) (i
return nil, err
}
if rule != nil && rule.OperatorRead(nil) != acl.Allow {
if rule.OperatorRead(nil) != acl.Allow {
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) {
req, _ := http.NewRequest("PUT", "/v1/agent/service/maintenance/_nope_?enable=true", nil)
resp := httptest.NewRecorder()
if _, err := a.srv.AgentServiceMaintenance(resp, req); err != nil {
t.Fatalf("err: %s", err)
}
if resp.Code != 404 {
t.Fatalf("expected 404, got %d", resp.Code)
}
a.srv.h.ServeHTTP(resp, req)
require.Equal(t, 404, resp.Code)
require.Contains(t, resp.Body.String(), `Unknown service "_nope_"`)
})
}

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) {
if !r.ACLsEnabled() {
return nil, nil, nil
return nil, acl.ManageAll(), 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)
authz, err := r.ResolveToken("does not exist")
require.Nil(t, authz)
require.Equal(t, acl.ManageAll(), authz)
require.Nil(t, err)
}

11
agent/consul/connect_ca_endpoint.go
View File

@ -65,7 +65,7 @@ func (s *ConnectCA) ConfigurationGet(
if err != nil {
return err
}
if rule != nil && rule.OperatorWrite(nil) != acl.Allow {
if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied
}
@ -97,7 +97,7 @@ func (s *ConnectCA) ConfigurationSet(
if err != nil {
return err
}
if rule != nil && rule.OperatorWrite(nil) != acl.Allow {
if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied
}
@ -175,7 +175,7 @@ func (s *ConnectCA) Sign(
if isService {
entMeta.Merge(serviceID.GetEnterpriseMeta())
entMeta.FillAuthzContext(&authzContext)
if rule != nil && rule.ServiceWrite(serviceID.Service, &authzContext) != acl.Allow {
if rule.ServiceWrite(serviceID.Service, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
@ -186,8 +186,9 @@ func (s *ConnectCA) Sign(
"we are %s", serviceID.Datacenter, s.srv.config.Datacenter)
}
} else if isAgent {
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
}
}
@ -223,7 +224,7 @@ func (s *ConnectCA) SignIntermediate(
if err != nil {
return err
}
if rule != nil && rule.OperatorWrite(nil) != acl.Allow {
if rule.OperatorWrite(nil) != acl.Allow {
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 {
return err
}
if rule != nil && rule.ServiceRead(args.Name, &authzContext) != acl.Allow {
if rule.ServiceRead(args.Name, &authzContext) != acl.Allow {
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 {
return err
}
if rule != nil && rule.OperatorWrite(nil) != acl.Allow {
if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied
}
@ -109,7 +109,7 @@ func (c *FederationState) Get(args *structs.FederationStateQuery, reply *structs
if err != nil {
return err
}
if rule != nil && rule.OperatorRead(nil) != acl.Allow {
if rule.OperatorRead(nil) != acl.Allow {
return acl.ErrPermissionDenied
}
@ -150,7 +150,7 @@ func (c *FederationState) List(args *structs.DCSpecificRequest, reply *structs.I
if err != nil {
return err
}
if rule != nil && rule.OperatorRead(nil) != acl.Allow {
if rule.OperatorRead(nil) != acl.Allow {
return acl.ErrPermissionDenied
}

2
agent/consul/internal_endpoint.go
View File

@ -409,7 +409,7 @@ func (m *Internal) EventFire(args *structs.EventFireRequest,
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)
m.logger.Warn("user event blocked by ACLs", "event", args.Name, "accessorID", accessorID)
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 {
return err
}
if rule != nil && rule.OperatorRead(nil) != acl.Allow {
if rule.OperatorRead(nil) != acl.Allow {
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 {
return err
}
if rule != nil && rule.OperatorWrite(nil) != acl.Allow {
if rule.OperatorWrite(nil) != acl.Allow {
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 {
return err
}
if rule != nil && rule.OperatorRead(nil) != acl.Allow {
if rule.OperatorRead(nil) != acl.Allow {
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 {
return err
}
if rule != nil && rule.OperatorRead(nil) != acl.Allow {
if rule.OperatorRead(nil) != acl.Allow {
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 {
return err
}
if rule != nil && rule.OperatorRead(nil) != acl.Allow {
if rule.OperatorRead(nil) != acl.Allow {
return acl.ErrPermissionDenied
}
@ -88,7 +88,7 @@ func (op *Operator) RaftRemovePeerByAddress(args *structs.RaftRemovePeerRequest,
if err := op.srv.validateEnterpriseToken(identity); err != nil {
return err
}
if rule != nil && rule.OperatorWrite(nil) != acl.Allow {
if rule.OperatorWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied
}
@ -141,7 +141,7 @@ func (op *Operator) RaftRemovePeerByID(args *structs.RaftRemovePeerRequest, repl
if err := op.srv.validateEnterpriseToken(identity); err != nil {
return err
}
if rule != nil && rule.OperatorWrite(nil) != acl.Allow {
if rule.OperatorWrite(nil) != acl.Allow {
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
// proposing.
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)
return acl.ErrPermissionDenied
}
@ -106,7 +106,7 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string)
}
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)
return acl.ErrPermissionDenied
}

5
agent/consul/snapshot_endpoint.go
View File

@ -16,11 +16,12 @@ import (
"net"
"time"
"github.com/hashicorp/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"
"github.com/hashicorp/go-msgpack/codec"
)
// 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.
if rule, err := s.ResolveToken(args.Token); err != nil {
return nil, err
} else if rule != nil && rule.Snapshot(nil) != acl.Allow {
} else if rule.Snapshot(nil) != acl.Allow {
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,
// write a forbidden response
if rule != nil && rule.OperatorRead(nil) != acl.Allow {
if rule.OperatorRead(nil) != acl.Allow {
resp.WriteHeader(http.StatusForbidden)
return
}

2
agent/structs/intention.go
View File

@ -325,7 +325,7 @@ func (ixn *Intention) CanRead(authz acl.Authorizer) bool {
}
func (ixn *Intention) CanWrite(authz acl.Authorizer) bool {
if authz == nil {
if authz == nil || authz == acl.ManageAll() {
return true
}
var authzContext acl.AuthorizerContext

4
agent/xds/server.go
View File

@ -583,12 +583,12 @@ func (s *Server) checkStreamACLs(streamCtx context.Context, cfgSnap *proxycfg.Co
switch cfgSnap.Kind {
case structs.ServiceKindConnectProxy:
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")
}
case structs.ServiceKindMeshGateway, structs.ServiceKindTerminatingGateway, structs.ServiceKindIngressGateway:
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")
}
default:

8
api/agent_test.go
View File

@ -618,7 +618,11 @@ func TestAPI_AgentServiceAddress(t *testing.T) {
require.Equal(t, services["foo2"].TaggedAddresses["wan"].Address, "198.18.0.1")
require.Equal(t, services["foo2"].TaggedAddresses["wan"].Port, 80)
if err := agent.ServiceDeregister("foo"); err != nil {
if err := agent.ServiceDeregister("foo1"); err != nil {
t.Fatalf("err: %v", err)
}
if err := agent.ServiceDeregister("foo2"); err != nil {
t.Fatalf("err: %v", err)
}
}
@ -1641,7 +1645,7 @@ func TestAPI_AgentConnectAuthorize(t *testing.T) {
auth, err := agent.ConnectAuthorize(params)
require.Nil(err)
require.True(auth.Authorized)
require.Equal(auth.Reason, "ACLs disabled, access is allowed by default")
require.Equal(auth.Reason, "Default behavior configured by ACLs")
}
func TestAPI_AgentHealthServiceOpts(t *testing.T) {

2
command/maint/maint_test.go
View File

@ -256,7 +256,7 @@ func TestMaintCommand_ServiceMaintenance_NoService(t *testing.T) {
t.Fatalf("expected response code 1, got %d", code)
}
if !strings.Contains(ui.ErrorWriter.String(), "No service registered") {
if !strings.Contains(ui.ErrorWriter.String(), "Unknown service") {
t.Fatalf("bad: %#v", ui.ErrorWriter.String())
}
}