Merge pull request #10798 from hashicorp/dnephin/backport-txn-authz-fix

[1.10.x] acl: fix txn_endpoint to properly authorize service registrations

.changelog/10798.txt

@@ -0,0 +1,4 @@
+```release-note:bug
+txn: fixes Txn.Apply to properly authorize service registrations.
+```
+

agent/consul/acl.go

@@ -2212,23 +2212,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

@@ -37,7 +37,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.
@@ -62,7 +62,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
@@ -127,7 +127,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

@@ -68,18 +68,7 @@ func (t *Txn) preCheck(authorizer acl.Authorizer, ops structs.TxnOps) structs.Tx
 			}
 
 			service := &op.Service.Service
-			// This is intentionally nil as we will authorize the request
-			// using vetServiceTxnOp next instead of doing it in servicePreApply
-			if err := servicePreApply(service, nil); 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 {
+			if err := servicePreApply(service, authorizer, op.Service.FillAuthzContext); err != nil {
 				errors = append(errors, &structs.TxnError{
 					OpIndex: i,
 					What:    err.Error(),