Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Aug 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changelog/10798.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
```release-note:bug
txn: fixes Txn.Apply to properly authorize service registrations.
```

17 changes: 0 additions & 17 deletions agent/consul/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions agent/consul/catalog_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
}
}
Expand Down
13 changes: 1 addition & 12 deletions agent/consul/txn_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,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(),
Expand Down