diff --git a/.changelog/10798.txt b/.changelog/10798.txt new file mode 100644 index 000000000000..f9ffd0a52266 --- /dev/null +++ b/.changelog/10798.txt @@ -0,0 +1,4 @@ +```release-note:bug +txn: fixes Txn.Apply to properly authorize service registrations. +``` + diff --git a/agent/consul/acl.go b/agent/consul/acl.go index 78a4b501bdac..73100b0d2154 100644 --- a/agent/consul/acl.go +++ b/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. diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 90e7d00f59f7..556521435d56 100644 --- a/agent/consul/catalog_endpoint.go +++ b/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 } } diff --git a/agent/consul/txn_endpoint.go b/agent/consul/txn_endpoint.go index 9819d63704b1..ead3dcea6285 100644 --- a/agent/consul/txn_endpoint.go +++ b/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(),