txn: add ACL enforcement/validation to new txn ops

agent/consul/acl.go

@@ -1351,3 +1351,99 @@ func vetDeregisterWithACL(rule acl.Authorizer, subj *structs.DeregisterRequest,
 
 	return nil
 }
+
+// vetNodeTxnOp applies the given ACL policy to a node transaction operation.
+func vetNodeTxnOp(op *structs.TxnNodeOp, rule acl.Authorizer) error {
+	node := op.Node
+
+	// Filtering for GETs is done on the output side.
+	if op.Verb == api.NodeGet {
+		return nil
+	}
+
+	n := &api.Node{
+		Node:            node.Node,
+		ID:              string(node.ID),
+		Address:         node.Address,
+		Datacenter:      node.Datacenter,
+		TaggedAddresses: node.TaggedAddresses,
+		Meta:            node.Meta,
+	}
+
+	// Sentinel doesn't apply to deletes, only creates/updates, so we don't need the scopeFn.
+	var scope func() map[string]interface{}
+	if op.Verb != api.NodeDelete && op.Verb != api.NodeDeleteCAS {
+		scope = func() map[string]interface{} {
+			return sentinel.ScopeCatalogUpsert(n, nil)
+		}
+	}
+
+	if !rule.NodeWrite(node.Node, scope) {
+		return acl.ErrPermissionDenied
+	}
+
+	return nil
+}
+
+// vetServiceTxnOp applies the given ACL policy to a service transaction operation.
+func vetServiceTxnOp(op *structs.TxnServiceOp, rule acl.Authorizer) error {
+	service := op.Service
+
+	// Filtering for GETs is done on the output side.
+	if op.Verb == api.ServiceGet {
+		return nil
+	}
+
+	n := &api.Node{Node: op.Node}
+	svc := &api.AgentService{
+		ID:                service.ID,
+		Service:           service.Service,
+		Tags:              service.Tags,
+		Meta:              service.Meta,
+		Address:           service.Address,
+		Port:              service.Port,
+		EnableTagOverride: service.EnableTagOverride,
+	}
+	scope := func() map[string]interface{} {
+		return sentinel.ScopeCatalogUpsert(n, svc)
+	}
+	if !rule.ServiceWrite(service.Service, scope) {
+		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 {
+	// Filtering for GETs is done on the output side.
+	if op.Verb == api.CheckGet {
+		return nil
+	}
+
+	n := &api.Node{Node: op.Check.Node}
+	svc := &api.AgentService{
+		ID:      op.Check.ServiceID,
+		Service: op.Check.ServiceID,
+		Tags:    op.Check.ServiceTags,
+	}
+	if op.Check.ServiceID == "" {
+		// Node-level check.
+		scope := func() map[string]interface{} {
+			return sentinel.ScopeCatalogUpsert(n, svc)
+		}
+		if !rule.NodeWrite(op.Check.Node, scope) {
+			return acl.ErrPermissionDenied
+		}
+	} else {
+		// Service-level check.
+		scope := func() map[string]interface{} {
+			return sentinel.ScopeCatalogUpsert(n, svc)
+		}
+		if !rule.ServiceWrite(op.Check.ServiceName, scope) {
+			return acl.ErrPermissionDenied
+		}
+	}
+
+	return nil
+}

agent/consul/catalog_endpoint.go

@@ -20,6 +20,65 @@ type Catalog struct {
 	srv *Server
 }
 
+// nodePreApply does the verification of a node before it is applied to Raft.
+func nodePreApply(nodeName, nodeID string) error {
+	if nodeName == "" {
+		return fmt.Errorf("Must provide node")
+	}
+	if nodeID != "" {
+		if _, err := uuid.ParseUUID(nodeID); err != nil {
+			return fmt.Errorf("Bad node ID: %v", err)
+		}
+	}
+
+	return nil
+}
+
+func servicePreApply(service *structs.NodeService, rule acl.Authorizer) 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.
+	if err := service.Validate(); err != nil {
+		return err
+	}
+
+	// If no service id, but service name, use default
+	if service.ID == "" && service.Service != "" {
+		service.ID = service.Service
+	}
+
+	// Verify ServiceName provided if ID.
+	if service.ID != "" && service.Service == "" {
+		return fmt.Errorf("Must provide service name with ID")
+	}
+
+	// Check the service address here and in the agent endpoint
+	// since service registration isn't synchronous.
+	if ipaddr.IsAny(service.Address) {
+		return fmt.Errorf("Invalid service address")
+	}
+
+	// Apply the ACL policy if any. The 'consul' service is excluded
+	// since it is managed automatically internally (that behavior
+	// is going away after version 0.8). We check this same policy
+	// later if version 0.8 is enabled, so we can eventually just
+	// delete this and do all the ACL checks down there.
+	if service.Service != structs.ConsulServiceName {
+		if rule != nil && !rule.ServiceWrite(service.Service, nil) {
+			return acl.ErrPermissionDenied
+		}
+	}
+
+	// Proxies must have write permission on their destination
+	if service.Kind == structs.ServiceKindConnectProxy {
+		if rule != nil && !rule.ServiceWrite(service.Proxy.DestinationServiceName, nil) {
+			return acl.ErrPermissionDenied
+		}
+	}
+
+	return nil
+}
+
 // checkPreApply does the verification of a check before it is applied to Raft.
 func checkPreApply(check *structs.HealthCheck) {
 	if check.CheckID == "" && check.Name != "" {
@@ -34,67 +93,25 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
 	}
 	defer metrics.MeasureSince([]string{"catalog", "register"}, time.Now())
 
-	// Verify the args.
-	if args.Node == "" {
-		return fmt.Errorf("Must provide node")
-	}
-	if args.Address == "" && !args.SkipNodeUpdate {
-		return fmt.Errorf("Must provide address if SkipNodeUpdate is not set")
-	}
-	if args.ID != "" {
-		if _, err := uuid.ParseUUID(string(args.ID)); err != nil {
-			return fmt.Errorf("Bad node ID: %v", err)
-		}
-	}
-
 	// Fetch the ACL token, if any.
 	rule, err := c.srv.ResolveToken(args.Token)
 	if err != nil {
 		return err
 	}
 
+	// Verify the args.
+	if err := nodePreApply(args.Node, string(args.ID)); err != nil {
+		return err
+	}
+	if args.Address == "" && !args.SkipNodeUpdate {
+		return fmt.Errorf("Must provide address if SkipNodeUpdate is not set")
+	}
+
 	// Handle a service registration.
 	if args.Service != nil {
-		// Validate the service. This is in addition to the below since
+		if err := servicePreApply(args.Service, rule); err != nil {
-		// the above just hasn't been moved over yet. We should move it over
-		// in time.
-		if err := args.Service.Validate(); err != nil {
 			return err
 		}
-
-		// If no service id, but service name, use default
-		if args.Service.ID == "" && args.Service.Service != "" {
-			args.Service.ID = args.Service.Service
-		}
-
-		// Verify ServiceName provided if ID.
-		if args.Service.ID != "" && args.Service.Service == "" {
-			return fmt.Errorf("Must provide service name with ID")
-		}
-
-		// Check the service address here and in the agent endpoint
-		// since service registration isn't synchronous.
-		if ipaddr.IsAny(args.Service.Address) {
-			return fmt.Errorf("Invalid service address")
-		}
-
-		// Apply the ACL policy if any. The 'consul' service is excluded
-		// since it is managed automatically internally (that behavior
-		// is going away after version 0.8). We check this same policy
-		// later if version 0.8 is enabled, so we can eventually just
-		// delete this and do all the ACL checks down there.
-		if args.Service.Service != structs.ConsulServiceName {
-			if rule != nil && !rule.ServiceWrite(args.Service.Service, nil) {
-				return acl.ErrPermissionDenied
-			}
-		}
-
-		// Proxies must have write permission on their destination
-		if args.Service.Kind == structs.ServiceKindConnectProxy {
-			if rule != nil && !rule.ServiceWrite(args.Service.Proxy.DestinationServiceName, nil) {
-				return acl.ErrPermissionDenied
-			}
-		}
 	}
 
 	// Move the old format single check into the slice, and fixup IDs.

agent/consul/filter.go

@@ -61,8 +61,18 @@ func (t *txnResultsFilter) Len() int {
 
 func (t *txnResultsFilter) Filter(i int) bool {
 	result := t.results[i]
-	if result.KV != nil {
+	switch {
+	case result.KV != nil:
 		return !t.authorizer.KeyRead(result.KV.Key)
+	case result.Node != nil:
+		return !t.authorizer.NodeRead(result.Node.Node)
+	case result.Service != nil:
+		return !t.authorizer.ServiceRead(result.Service.Service)
+	case result.Check != nil:
+		if result.Check.ServiceName != "" {
+			return !t.authorizer.ServiceRead(result.Check.ServiceName)
+		}
+		return !t.authorizer.NodeRead(result.Check.Node)
 	}
 	return false
 }

agent/consul/txn_endpoint.go

@@ -36,8 +36,50 @@ func (t *Txn) preCheck(authorizer acl.Authorizer, ops structs.TxnOps) structs.Tx
 					What:    err.Error(),
 				})
 			}
+		case op.Node != nil:
+			node := op.Node.Node
+			if err := nodePreApply(node.Node, string(node.ID)); 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 := vetNodeTxnOp(op.Node, authorizer); err != nil {
+				errors = append(errors, &structs.TxnError{
+					OpIndex: i,
+					What:    err.Error(),
+				})
+			}
+		case op.Service != nil:
+			service := &op.Service.Service
+			if err := servicePreApply(service, authorizer); 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(),
+				})
+			}
 		case op.Check != nil:
 			checkPreApply(&op.Check.Check)
+
+			// Check that the token has permissions for the given operation.
+			if err := vetCheckTxnOp(op.Check, authorizer); err != nil {
+				errors = append(errors, &structs.TxnError{
+					OpIndex: i,
+					What:    err.Error(),
+				})
+			}
 		}
 	}