Merge pull request #10742 from hashicorp/dnephin/acl-resolver

acl: move agent/consul vet functions
This commit is contained in:
Daniel Nephin 2021-08-05 15:36:49 -04:00 committed by GitHub
commit e1f04e8c96
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 698 additions and 714 deletions

View File

@ -2048,231 +2048,3 @@ func (r *ACLResolver) filterACL(token string, subj interface{}) error {
return r.filterACLWithAuthorizer(authorizer, subj)
}
// vetRegisterWithACL applies the given ACL's policy to the catalog update and
// determines if it is allowed. Since the catalog register request is so
// dynamic, this is a pretty complex algorithm and was worth breaking out of the
// endpoint. The NodeServices record for the node must be supplied, and can be
// nil.
//
// This is a bit racy because we have to check the state store outside of a
// transaction. It's the best we can do because we don't want to flow ACL
// checking down there. The node information doesn't change in practice, so this
// will be fine. If we expose ways to change node addresses in a later version,
// then we should split the catalog API at the node and service level so we can
// address this race better (even then it would be super rare, and would at
// worst let a service update revert a recent node update, so it doesn't open up
// too much abuse).
func vetRegisterWithACL(rule acl.Authorizer, subj *structs.RegisterRequest,
ns *structs.NodeServices) error {
// Fast path if ACLs are not enabled.
if rule == nil {
return nil
}
var authzContext acl.AuthorizerContext
subj.FillAuthzContext(&authzContext)
// Vet the node info. This allows service updates to re-post the required
// node info for each request without having to have node "write"
// privileges.
needsNode := ns == nil || subj.ChangesNode(ns.Node)
if needsNode && rule.NodeWrite(subj.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
// Vet the service change. This includes making sure they can register
// the given service, and that we can write to any existing service that
// is being modified by id (if any).
if subj.Service != nil {
if rule.ServiceWrite(subj.Service.Service, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
if ns != nil {
other, ok := ns.Services[subj.Service.ID]
if ok {
// This is effectively a delete, so we DO NOT apply the
// sentinel scope to the service we are overwriting, just
// the regular ACL policy.
var secondaryCtx acl.AuthorizerContext
other.FillAuthzContext(&secondaryCtx)
if rule.ServiceWrite(other.Service, &secondaryCtx) != acl.Allow {
return acl.ErrPermissionDenied
}
}
}
}
// Make sure that the member was flattened before we got there. This
// keeps us from having to verify this check as well.
if subj.Check != nil {
return fmt.Errorf("check member must be nil")
}
// Vet the checks. Node-level checks require node write, and
// service-level checks require service write.
for _, check := range subj.Checks {
// Make sure that the node matches - we don't allow you to mix
// checks from other nodes because we'd have to pull a bunch
// more state store data to check this. If ACLs are enabled then
// we simply require them to match in a given request. There's a
// note in state_store.go to ban this down there in Consul 0.8,
// but it's good to leave this here because it's required for
// correctness wrt. ACLs.
if check.Node != subj.Node {
return fmt.Errorf("Node '%s' for check '%s' doesn't match register request node '%s'",
check.Node, check.CheckID, subj.Node)
}
// Node-level check.
if check.ServiceID == "" {
if rule.NodeWrite(subj.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
continue
}
// Service-level check, check the common case where it
// matches the service part of this request, which has
// already been vetted above, and might be being registered
// along with its checks.
if subj.Service != nil && subj.Service.ID == check.ServiceID {
continue
}
// Service-level check for some other service. Make sure they've
// got write permissions for that service.
if ns == nil {
return fmt.Errorf("Unknown service '%s' for check '%s'", check.ServiceID, check.CheckID)
}
other, ok := ns.Services[check.ServiceID]
if !ok {
return fmt.Errorf("Unknown service '%s' for check '%s'", check.ServiceID, check.CheckID)
}
// We are only adding a check here, so we don't add the scope,
// since the sentinel policy doesn't apply to adding checks at
// this time.
var secondaryCtx acl.AuthorizerContext
other.FillAuthzContext(&secondaryCtx)
if rule.ServiceWrite(other.Service, &secondaryCtx) != acl.Allow {
return acl.ErrPermissionDenied
}
}
return nil
}
// vetDeregisterWithACL applies the given ACL's policy to the catalog update and
// determines if it is allowed. Since the catalog deregister request is so
// dynamic, this is a pretty complex algorithm and was worth breaking out of the
// endpoint. The NodeService for the referenced service must be supplied, and can
// be nil; similar for the HealthCheck for the referenced health check.
func vetDeregisterWithACL(rule acl.Authorizer, subj *structs.DeregisterRequest,
ns *structs.NodeService, nc *structs.HealthCheck) error {
// Fast path if ACLs are not enabled.
if rule == nil {
return nil
}
// We don't apply sentinel in this path, since at this time sentinel
// only applies to create and update operations.
var authzContext acl.AuthorizerContext
// fill with the defaults for use with the NodeWrite check
subj.FillAuthzContext(&authzContext)
// Allow service deregistration if the token has write permission for the node.
// This accounts for cases where the agent no longer has a token with write permission
// on the service to deregister it.
if rule.NodeWrite(subj.Node, &authzContext) == acl.Allow {
return nil
}
// This order must match the code in applyDeregister() in
// fsm/commands_oss.go since it also evaluates things in this order,
// and will ignore fields based on this precedence. This lets us also
// ignore them from an ACL perspective.
if subj.ServiceID != "" {
if ns == nil {
return fmt.Errorf("Unknown service '%s'", subj.ServiceID)
}
ns.FillAuthzContext(&authzContext)
if rule.ServiceWrite(ns.Service, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
} else if subj.CheckID != "" {
if nc == nil {
return fmt.Errorf("Unknown check '%s'", subj.CheckID)
}
nc.FillAuthzContext(&authzContext)
if nc.ServiceID != "" {
if rule.ServiceWrite(nc.ServiceName, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
} else {
if rule.NodeWrite(subj.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
}
} else {
// Since NodeWrite is not given - otherwise the earlier check
// would've returned already - we can deny here.
return acl.ErrPermissionDenied
}
return nil
}
// vetNodeTxnOp applies the given ACL policy to a node transaction operation.
func vetNodeTxnOp(op *structs.TxnNodeOp, 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.NodeWrite(op.Node.Node, &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.
if rule == nil {
return nil
}
var authzContext acl.AuthorizerContext
op.FillAuthzContext(&authzContext)
if op.Check.ServiceID == "" {
// Node-level check.
if rule.NodeWrite(op.Check.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
} else {
// Service-level check.
if rule.ServiceWrite(op.Check.ServiceName, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
}
return nil
}

View File

@ -4,7 +4,6 @@ import (
"fmt"
"os"
"reflect"
"strings"
"sync/atomic"
"testing"
"time"
@ -3421,488 +3420,6 @@ func TestACL_unhandledFilterType(t *testing.T) {
srv.filterACL(token, &structs.HealthCheck{})
}
func TestACL_vetRegisterWithACL(t *testing.T) {
t.Parallel()
args := &structs.RegisterRequest{
Node: "nope",
Address: "127.0.0.1",
}
// With a nil ACL, the update should be allowed.
if err := vetRegisterWithACL(nil, args, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Create a basic node policy.
policy, err := acl.NewPolicyFromSource("", 0, `
node "node" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// With that policy, the update should now be blocked for node reasons.
err = vetRegisterWithACL(perms, args, nil)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Now use a permitted node name.
args.Node = "node"
if err := vetRegisterWithACL(perms, args, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Build some node info that matches what we have now.
ns := &structs.NodeServices{
Node: &structs.Node{
Node: "node",
Address: "127.0.0.1",
},
Services: make(map[string]*structs.NodeService),
}
// Try to register a service, which should be blocked.
args.Service = &structs.NodeService{
Service: "service",
ID: "my-id",
}
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Chain on a basic service policy.
policy, err = acl.NewPolicyFromSource("", 0, `
service "service" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// With the service ACL, the update should go through.
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Add an existing service that they are clobbering and aren't allowed
// to write to.
ns.Services["my-id"] = &structs.NodeService{
Service: "other",
ID: "my-id",
}
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Chain on a policy that allows them to write to the other service.
policy, err = acl.NewPolicyFromSource("", 0, `
service "other" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// Now it should go through.
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Try creating the node and the service at once by having no existing
// node record. This should be ok since we have node and service
// permissions.
if err := vetRegisterWithACL(perms, args, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Add a node-level check to the member, which should be rejected.
args.Check = &structs.HealthCheck{
Node: "node",
}
err = vetRegisterWithACL(perms, args, ns)
if err == nil || !strings.Contains(err.Error(), "check member must be nil") {
t.Fatalf("bad: %v", err)
}
// Move the check into the slice, but give a bad node name.
args.Check.Node = "nope"
args.Checks = append(args.Checks, args.Check)
args.Check = nil
err = vetRegisterWithACL(perms, args, ns)
if err == nil || !strings.Contains(err.Error(), "doesn't match register request node") {
t.Fatalf("bad: %v", err)
}
// Fix the node name, which should now go through.
args.Checks[0].Node = "node"
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Add a service-level check.
args.Checks = append(args.Checks, &structs.HealthCheck{
Node: "node",
ServiceID: "my-id",
})
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Try creating everything at once. This should be ok since we have all
// the permissions we need. It also makes sure that we can register a
// new node, service, and associated checks.
if err := vetRegisterWithACL(perms, args, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Nil out the service registration, which'll skip the special case
// and force us to look at the ns data (it will look like we are
// writing to the "other" service which also has "my-id").
args.Service = nil
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Chain on a policy that forbids them to write to the other service.
policy, err = acl.NewPolicyFromSource("", 0, `
service "other" {
policy = "deny"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// This should get rejected.
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Change the existing service data to point to a service name they
// car write to. This should go through.
ns.Services["my-id"] = &structs.NodeService{
Service: "service",
ID: "my-id",
}
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Chain on a policy that forbids them to write to the node.
policy, err = acl.NewPolicyFromSource("", 0, `
node "node" {
policy = "deny"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// This should get rejected because there's a node-level check in here.
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Change the node-level check into a service check, and then it should
// go through.
args.Checks[0].ServiceID = "my-id"
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Finally, attempt to update the node part of the data and make sure
// that gets rejected since they no longer have permissions.
args.Address = "127.0.0.2"
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
}
func TestACL_vetDeregisterWithACL(t *testing.T) {
t.Parallel()
args := &structs.DeregisterRequest{
Node: "nope",
}
// With a nil ACL, the update should be allowed.
if err := vetDeregisterWithACL(nil, args, nil, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Create a basic node policy.
policy, err := acl.NewPolicyFromSource("", 0, `
node "node" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
nodePerms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
policy, err = acl.NewPolicyFromSource("", 0, `
service "my-service" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
servicePerms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
for _, args := range []struct {
DeregisterRequest structs.DeregisterRequest
Service *structs.NodeService
Check *structs.HealthCheck
Perms acl.Authorizer
Expected bool
Name string
}{
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
},
Perms: nodePerms,
Expected: false,
Name: "no right on node",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
},
Perms: servicePerms,
Expected: false,
Name: "right on service but node dergister request",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
ServiceID: "my-service-id",
},
Service: &structs.NodeService{
Service: "my-service",
},
Perms: nodePerms,
Expected: false,
Name: "no rights on node nor service",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
ServiceID: "my-service-id",
},
Service: &structs.NodeService{
Service: "my-service",
},
Perms: servicePerms,
Expected: true,
Name: "no rights on node but rights on service",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
ServiceID: "my-service-id",
CheckID: "my-check",
},
Service: &structs.NodeService{
Service: "my-service",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: nodePerms,
Expected: false,
Name: "no right on node nor service for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
ServiceID: "my-service-id",
CheckID: "my-check",
},
Service: &structs.NodeService{
Service: "my-service",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: servicePerms,
Expected: true,
Name: "no rights on node but rights on service for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
CheckID: "my-check",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: nodePerms,
Expected: false,
Name: "no right on node for node check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
CheckID: "my-check",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: servicePerms,
Expected: false,
Name: "rights on service but no right on node for node check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
},
Perms: nodePerms,
Expected: true,
Name: "rights on node for node",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
},
Perms: servicePerms,
Expected: false,
Name: "rights on service but not on node for node",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
ServiceID: "my-service-id",
},
Service: &structs.NodeService{
Service: "my-service",
},
Perms: nodePerms,
Expected: true,
Name: "rights on node for service",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
ServiceID: "my-service-id",
},
Service: &structs.NodeService{
Service: "my-service",
},
Perms: servicePerms,
Expected: true,
Name: "rights on service for service",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
ServiceID: "my-service-id",
CheckID: "my-check",
},
Service: &structs.NodeService{
Service: "my-service",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: nodePerms,
Expected: true,
Name: "right on node for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
ServiceID: "my-service-id",
CheckID: "my-check",
},
Service: &structs.NodeService{
Service: "my-service",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: servicePerms,
Expected: true,
Name: "rights on service for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
CheckID: "my-check",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: nodePerms,
Expected: true,
Name: "rights on node for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
CheckID: "my-check",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: servicePerms,
Expected: false,
Name: "rights on service for node check",
},
} {
t.Run(args.Name, func(t *testing.T) {
err = vetDeregisterWithACL(args.Perms, &args.DeregisterRequest, args.Service, args.Check)
if !args.Expected {
if err == nil {
t.Errorf("expected error with %+v", args.DeregisterRequest)
}
if !acl.IsErrPermissionDenied(err) {
t.Errorf("expected permission denied error with %+v, instead got %+v", args.DeregisterRequest, err)
}
} else if err != nil {
t.Errorf("expected no error with %+v", args.DeregisterRequest)
}
})
}
}
func TestDedupeServiceIdentities(t *testing.T) {
srvid := func(name string, datacenters ...string) *structs.ACLServiceIdentity {
return &structs.ACLServiceIdentity{

View File

@ -212,6 +212,124 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
return err
}
// vetRegisterWithACL applies the given ACL's policy to the catalog update and
// determines if it is allowed. Since the catalog register request is so
// dynamic, this is a pretty complex algorithm and was worth breaking out of the
// endpoint. The NodeServices record for the node must be supplied, and can be
// nil.
//
// This is a bit racy because we have to check the state store outside of a
// transaction. It's the best we can do because we don't want to flow ACL
// checking down there. The node information doesn't change in practice, so this
// will be fine. If we expose ways to change node addresses in a later version,
// then we should split the catalog API at the node and service level so we can
// address this race better (even then it would be super rare, and would at
// worst let a service update revert a recent node update, so it doesn't open up
// too much abuse).
func vetRegisterWithACL(
authz acl.Authorizer,
subj *structs.RegisterRequest,
ns *structs.NodeServices,
) error {
var authzContext acl.AuthorizerContext
subj.FillAuthzContext(&authzContext)
// Vet the node info. This allows service updates to re-post the required
// node info for each request without having to have node "write"
// privileges.
needsNode := ns == nil || subj.ChangesNode(ns.Node)
if needsNode && authz.NodeWrite(subj.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
// Vet the service change. This includes making sure they can register
// the given service, and that we can write to any existing service that
// is being modified by id (if any).
if subj.Service != nil {
if authz.ServiceWrite(subj.Service.Service, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
if ns != nil {
other, ok := ns.Services[subj.Service.ID]
if ok {
// This is effectively a delete, so we DO NOT apply the
// sentinel scope to the service we are overwriting, just
// the regular ACL policy.
var secondaryCtx acl.AuthorizerContext
other.FillAuthzContext(&secondaryCtx)
if authz.ServiceWrite(other.Service, &secondaryCtx) != acl.Allow {
return acl.ErrPermissionDenied
}
}
}
}
// Make sure that the member was flattened before we got there. This
// keeps us from having to verify this check as well.
if subj.Check != nil {
return fmt.Errorf("check member must be nil")
}
// Vet the checks. Node-level checks require node write, and
// service-level checks require service write.
for _, check := range subj.Checks {
// Make sure that the node matches - we don't allow you to mix
// checks from other nodes because we'd have to pull a bunch
// more state store data to check this. If ACLs are enabled then
// we simply require them to match in a given request. There's a
// note in state_store.go to ban this down there in Consul 0.8,
// but it's good to leave this here because it's required for
// correctness wrt. ACLs.
if check.Node != subj.Node {
return fmt.Errorf("Node '%s' for check '%s' doesn't match register request node '%s'",
check.Node, check.CheckID, subj.Node)
}
// Node-level check.
if check.ServiceID == "" {
if authz.NodeWrite(subj.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
continue
}
// Service-level check, check the common case where it
// matches the service part of this request, which has
// already been vetted above, and might be being registered
// along with its checks.
if subj.Service != nil && subj.Service.ID == check.ServiceID {
continue
}
// Service-level check for some other service. Make sure they've
// got write permissions for that service.
if ns == nil {
return fmt.Errorf("Unknown service '%s' for check '%s'", check.ServiceID, check.CheckID)
}
other, ok := ns.Services[check.ServiceID]
if !ok {
return fmt.Errorf("Unknown service '%s' for check '%s'", check.ServiceID, check.CheckID)
}
// We are only adding a check here, so we don't add the scope,
// since the sentinel policy doesn't apply to adding checks at
// this time.
var secondaryCtx acl.AuthorizerContext
other.FillAuthzContext(&secondaryCtx)
if authz.ServiceWrite(other.Service, &secondaryCtx) != acl.Allow {
return acl.ErrPermissionDenied
}
}
return nil
}
// Deregister is used to remove a service registration for a given node.
func (c *Catalog) Deregister(args *structs.DeregisterRequest, reply *struct{}) error {
if done, err := c.srv.ForwardRPC("Catalog.Deregister", args, reply); done {
@ -261,6 +379,70 @@ func (c *Catalog) Deregister(args *structs.DeregisterRequest, reply *struct{}) e
return err
}
// vetDeregisterWithACL applies the given ACL's policy to the catalog update and
// determines if it is allowed. Since the catalog deregister request is so
// dynamic, this is a pretty complex algorithm and was worth breaking out of the
// endpoint. The NodeService for the referenced service must be supplied, and can
// be nil; similar for the HealthCheck for the referenced health check.
func vetDeregisterWithACL(
authz acl.Authorizer,
subj *structs.DeregisterRequest,
ns *structs.NodeService,
nc *structs.HealthCheck,
) error {
// We don't apply sentinel in this path, since at this time sentinel
// only applies to create and update operations.
var authzContext acl.AuthorizerContext
// fill with the defaults for use with the NodeWrite check
subj.FillAuthzContext(&authzContext)
// Allow service deregistration if the token has write permission for the node.
// This accounts for cases where the agent no longer has a token with write permission
// on the service to deregister it.
if authz.NodeWrite(subj.Node, &authzContext) == acl.Allow {
return nil
}
// This order must match the code in applyDeregister() in
// fsm/commands_oss.go since it also evaluates things in this order,
// and will ignore fields based on this precedence. This lets us also
// ignore them from an ACL perspective.
if subj.ServiceID != "" {
if ns == nil {
return fmt.Errorf("Unknown service '%s'", subj.ServiceID)
}
ns.FillAuthzContext(&authzContext)
if authz.ServiceWrite(ns.Service, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
} else if subj.CheckID != "" {
if nc == nil {
return fmt.Errorf("Unknown check '%s'", subj.CheckID)
}
nc.FillAuthzContext(&authzContext)
if nc.ServiceID != "" {
if authz.ServiceWrite(nc.ServiceName, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
} else {
if authz.NodeWrite(subj.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
}
} else {
// Since NodeWrite is not given - otherwise the earlier check
// would've returned already - we can deny here.
return acl.ErrPermissionDenied
}
return nil
}
// ListDatacenters is used to query for the list of known datacenters
func (c *Catalog) ListDatacenters(args *structs.DatacentersRequest, reply *[]string) error {
dcs, err := c.srv.router.GetDatacentersByDistance()

View File

@ -8,6 +8,10 @@ import (
"testing"
"time"
msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/api"
@ -16,9 +20,6 @@ import (
"github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/hashicorp/consul/testrpc"
"github.com/hashicorp/consul/types"
msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestCatalog_Register(t *testing.T) {
@ -3466,3 +3467,485 @@ service "gateway" {
assert.Equal(r, expect, resp.Services)
})
}
func TestVetRegisterWithACL(t *testing.T) {
t.Parallel()
args := &structs.RegisterRequest{
Node: "nope",
Address: "127.0.0.1",
}
// With an "allow all" authorizer the update should be allowed.
if err := vetRegisterWithACL(acl.ManageAll(), args, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Create a basic node policy.
policy, err := acl.NewPolicyFromSource("", 0, `
node "node" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// With that policy, the update should now be blocked for node reasons.
err = vetRegisterWithACL(perms, args, nil)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Now use a permitted node name.
args.Node = "node"
if err := vetRegisterWithACL(perms, args, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Build some node info that matches what we have now.
ns := &structs.NodeServices{
Node: &structs.Node{
Node: "node",
Address: "127.0.0.1",
},
Services: make(map[string]*structs.NodeService),
}
// Try to register a service, which should be blocked.
args.Service = &structs.NodeService{
Service: "service",
ID: "my-id",
}
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Chain on a basic service policy.
policy, err = acl.NewPolicyFromSource("", 0, `
service "service" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// With the service ACL, the update should go through.
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Add an existing service that they are clobbering and aren't allowed
// to write to.
ns.Services["my-id"] = &structs.NodeService{
Service: "other",
ID: "my-id",
}
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Chain on a policy that allows them to write to the other service.
policy, err = acl.NewPolicyFromSource("", 0, `
service "other" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// Now it should go through.
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Try creating the node and the service at once by having no existing
// node record. This should be ok since we have node and service
// permissions.
if err := vetRegisterWithACL(perms, args, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Add a node-level check to the member, which should be rejected.
args.Check = &structs.HealthCheck{
Node: "node",
}
err = vetRegisterWithACL(perms, args, ns)
if err == nil || !strings.Contains(err.Error(), "check member must be nil") {
t.Fatalf("bad: %v", err)
}
// Move the check into the slice, but give a bad node name.
args.Check.Node = "nope"
args.Checks = append(args.Checks, args.Check)
args.Check = nil
err = vetRegisterWithACL(perms, args, ns)
if err == nil || !strings.Contains(err.Error(), "doesn't match register request node") {
t.Fatalf("bad: %v", err)
}
// Fix the node name, which should now go through.
args.Checks[0].Node = "node"
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Add a service-level check.
args.Checks = append(args.Checks, &structs.HealthCheck{
Node: "node",
ServiceID: "my-id",
})
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Try creating everything at once. This should be ok since we have all
// the permissions we need. It also makes sure that we can register a
// new node, service, and associated checks.
if err := vetRegisterWithACL(perms, args, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Nil out the service registration, which'll skip the special case
// and force us to look at the ns data (it will look like we are
// writing to the "other" service which also has "my-id").
args.Service = nil
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Chain on a policy that forbids them to write to the other service.
policy, err = acl.NewPolicyFromSource("", 0, `
service "other" {
policy = "deny"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// This should get rejected.
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Change the existing service data to point to a service name they
// car write to. This should go through.
ns.Services["my-id"] = &structs.NodeService{
Service: "service",
ID: "my-id",
}
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Chain on a policy that forbids them to write to the node.
policy, err = acl.NewPolicyFromSource("", 0, `
node "node" {
policy = "deny"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
// This should get rejected because there's a node-level check in here.
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
// Change the node-level check into a service check, and then it should
// go through.
args.Checks[0].ServiceID = "my-id"
if err := vetRegisterWithACL(perms, args, ns); err != nil {
t.Fatalf("err: %v", err)
}
// Finally, attempt to update the node part of the data and make sure
// that gets rejected since they no longer have permissions.
args.Address = "127.0.0.2"
err = vetRegisterWithACL(perms, args, ns)
if !acl.IsErrPermissionDenied(err) {
t.Fatalf("bad: %v", err)
}
}
func TestVetDeregisterWithACL(t *testing.T) {
t.Parallel()
args := &structs.DeregisterRequest{
Node: "nope",
}
// With an "allow all" authorizer the update should be allowed.
if err := vetDeregisterWithACL(acl.ManageAll(), args, nil, nil); err != nil {
t.Fatalf("err: %v", err)
}
// Create a basic node policy.
policy, err := acl.NewPolicyFromSource("", 0, `
node "node" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
nodePerms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
policy, err = acl.NewPolicyFromSource("", 0, `
service "my-service" {
policy = "write"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
servicePerms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
for _, args := range []struct {
DeregisterRequest structs.DeregisterRequest
Service *structs.NodeService
Check *structs.HealthCheck
Perms acl.Authorizer
Expected bool
Name string
}{
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
},
Perms: nodePerms,
Expected: false,
Name: "no right on node",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
},
Perms: servicePerms,
Expected: false,
Name: "right on service but node dergister request",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
ServiceID: "my-service-id",
},
Service: &structs.NodeService{
Service: "my-service",
},
Perms: nodePerms,
Expected: false,
Name: "no rights on node nor service",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
ServiceID: "my-service-id",
},
Service: &structs.NodeService{
Service: "my-service",
},
Perms: servicePerms,
Expected: true,
Name: "no rights on node but rights on service",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
ServiceID: "my-service-id",
CheckID: "my-check",
},
Service: &structs.NodeService{
Service: "my-service",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: nodePerms,
Expected: false,
Name: "no right on node nor service for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
ServiceID: "my-service-id",
CheckID: "my-check",
},
Service: &structs.NodeService{
Service: "my-service",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: servicePerms,
Expected: true,
Name: "no rights on node but rights on service for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
CheckID: "my-check",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: nodePerms,
Expected: false,
Name: "no right on node for node check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "nope",
CheckID: "my-check",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: servicePerms,
Expected: false,
Name: "rights on service but no right on node for node check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
},
Perms: nodePerms,
Expected: true,
Name: "rights on node for node",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
},
Perms: servicePerms,
Expected: false,
Name: "rights on service but not on node for node",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
ServiceID: "my-service-id",
},
Service: &structs.NodeService{
Service: "my-service",
},
Perms: nodePerms,
Expected: true,
Name: "rights on node for service",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
ServiceID: "my-service-id",
},
Service: &structs.NodeService{
Service: "my-service",
},
Perms: servicePerms,
Expected: true,
Name: "rights on service for service",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
ServiceID: "my-service-id",
CheckID: "my-check",
},
Service: &structs.NodeService{
Service: "my-service",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: nodePerms,
Expected: true,
Name: "right on node for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
ServiceID: "my-service-id",
CheckID: "my-check",
},
Service: &structs.NodeService{
Service: "my-service",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: servicePerms,
Expected: true,
Name: "rights on service for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
CheckID: "my-check",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: nodePerms,
Expected: true,
Name: "rights on node for check",
},
{
DeregisterRequest: structs.DeregisterRequest{
Node: "node",
CheckID: "my-check",
},
Check: &structs.HealthCheck{
CheckID: "my-check",
},
Perms: servicePerms,
Expected: false,
Name: "rights on service for node check",
},
} {
t.Run(args.Name, func(t *testing.T) {
err = vetDeregisterWithACL(args.Perms, &args.DeregisterRequest, args.Service, args.Check)
if !args.Expected {
if err == nil {
t.Errorf("expected error with %+v", args.DeregisterRequest)
}
if !acl.IsErrPermissionDenied(err) {
t.Errorf("expected permission denied error with %+v, instead got %+v", args.DeregisterRequest, err)
}
} else if err != nil {
t.Errorf("expected no error with %+v", args.DeregisterRequest)
}
})
}
}

View File

@ -108,6 +108,36 @@ func (t *Txn) preCheck(authorizer acl.Authorizer, ops structs.TxnOps) structs.Tx
return errors
}
// vetNodeTxnOp applies the given ACL policy to a node transaction operation.
func vetNodeTxnOp(op *structs.TxnNodeOp, authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
op.FillAuthzContext(&authzContext)
if authz.NodeWrite(op.Node.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
return nil
}
// vetCheckTxnOp applies the given ACL policy to a check transaction operation.
func vetCheckTxnOp(op *structs.TxnCheckOp, authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
op.FillAuthzContext(&authzContext)
if op.Check.ServiceID == "" {
// Node-level check.
if authz.NodeWrite(op.Check.Node, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
} else {
// Service-level check.
if authz.ServiceWrite(op.Check.ServiceName, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
}
return nil
}
// Apply is used to apply multiple operations in a single, atomic transaction.
func (t *Txn) Apply(args *structs.TxnRequest, reply *structs.TxnResponse) error {
if done, err := t.srv.ForwardRPC("Txn.Apply", args, reply); done {