Merge pull request #10793 from hashicorp/dnephin/acl-intentions

acl: small cleanup of a couple Authorization flows

acl/policy_authorizer.go

@@ -524,6 +524,9 @@ func (p *policyAuthorizer) IntentionRead(prefix string, _ *AuthorizerContext) En
 // IntentionWrite checks if writing (creating, updating, or deleting) of an
 // intention is allowed.
 func (p *policyAuthorizer) IntentionWrite(prefix string, _ *AuthorizerContext) EnforcementDecision {
+	if prefix == "" {
+		return Deny
+	}
 	if prefix == "*" {
 		return p.allAllowed(p.intentionRules, AccessWrite)
 	}

agent/consul/acl.go

@@ -2252,23 +2252,6 @@ func vetNodeTxnOp(op *structs.TxnNodeOp, rule acl.Authorizer) error {
 	return nil
 }
 
-// vetServiceTxnOp applies the given ACL policy to a service transaction operation.
-func vetServiceTxnOp(op *structs.TxnServiceOp, rule acl.Authorizer) error {
-	// Fast path if ACLs are not enabled.
-	if rule == nil {
-		return nil
-	}
-
-	var authzContext acl.AuthorizerContext
-	op.FillAuthzContext(&authzContext)
-
-	if rule.ServiceWrite(op.Service.Service, &authzContext) != acl.Allow {
-		return acl.ErrPermissionDenied
-	}
-
-	return nil
-}
-
 // vetCheckTxnOp applies the given ACL policy to a check transaction operation.
 func vetCheckTxnOp(op *structs.TxnCheckOp, rule acl.Authorizer) error {
 	// Fast path if ACLs are not enabled.

agent/consul/catalog_endpoint.go

@@ -85,7 +85,7 @@ func nodePreApply(nodeName, nodeID string) error {
 	return nil
 }
 
-func servicePreApply(service *structs.NodeService, authz acl.Authorizer) error {
+func servicePreApply(service *structs.NodeService, authz acl.Authorizer, authzCtxFill func(*acl.AuthorizerContext)) error {
 	// Validate the service. This is in addition to the below since
 	// the above just hasn't been moved over yet. We should move it over
 	// in time.
@@ -110,7 +110,7 @@ func servicePreApply(service *structs.NodeService, authz acl.Authorizer) error {
 	}
 
 	var authzContext acl.AuthorizerContext
-	service.FillAuthzContext(&authzContext)
+	authzCtxFill(&authzContext)
 
 	// Apply the ACL policy if any. The 'consul' service is excluded
 	// since it is managed automatically internally (that behavior
@@ -175,7 +175,7 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
 
 	// Handle a service registration.
 	if args.Service != nil {
-		if err := servicePreApply(args.Service, authz); err != nil {
+		if err := servicePreApply(args.Service, authz, args.Service.FillAuthzContext); err != nil {
 			return err
 		}
 	}

agent/consul/txn_endpoint.go

@@ -81,18 +81,7 @@ func (t *Txn) preCheck(authorizer acl.Authorizer, ops structs.TxnOps) structs.Tx
 			}
 
 			service := &op.Service.Service
-			// acl.ManageAll is used here because the request will be authorized
+			if err := servicePreApply(service, authorizer, op.Service.FillAuthzContext); err != nil {
-			// later using vetServiceTxnOp.
-			if err := servicePreApply(service, acl.ManageAll()); err != nil {
-				errors = append(errors, &structs.TxnError{
-					OpIndex: i,
-					What:    err.Error(),
-				})
-				break
-			}
-
-			// Check that the token has permissions for the given operation.
-			if err := vetServiceTxnOp(op.Service, authorizer); err != nil {
 				errors = append(errors, &structs.TxnError{
 					OpIndex: i,
 					What:    err.Error(),

agent/structs/intention.go

@@ -322,16 +322,7 @@ func (ixn *Intention) CanRead(authz acl.Authorizer) bool {
 }
 
 func (ixn *Intention) CanWrite(authz acl.Authorizer) bool {
-	if authz == acl.ManageAll() {
-		return true
-	}
 	var authzContext acl.AuthorizerContext
-
-	// TODO: this line seems to require checking 'authz == acl.ManageAll()' above
-	if ixn.DestinationName == "" {
-		return false
-	}
-
 	ixn.FillAuthzContext(&authzContext, true)
 	return authz.IntentionWrite(ixn.DestinationName, &authzContext) == acl.Allow
 }