acl: check ACL against object namespace

Fix a bug where a millicious user can access or manipulate an alloc in a
namespace they don't have access to.  The allocation endpoints perform
ACL checks against the request namespace, not the allocation namespace,
and performs the allocation lookup independently from namespaces.

Here, we check that the requested can access the alloc namespace
regardless of the declared request namespace.

Ideally, we'd enforce that the declared request namespace matches
the actual allocation namespace.  Unfortunately, we haven't documented
alloc endpoints as namespaced functions; we suspect starting to enforce
this will be very disruptive and inappropriate for a nomad point
release.  As such, we maintain current behavior that doesn't require
passing the proper namespace in request.  A future major release may
start enforcing checking declared namespace.

acl/acl.go

@@ -481,3 +481,25 @@ func (a *ACL) AllowQuotaWrite() bool {
 func (a *ACL) IsManagement() bool {
 	return a.management
 }
+
+// NamespaceValidator returns a func that wraps ACL.AllowNamespaceOperation in
+// a list of operations. Returns true (allowed) if acls are disabled or if
+// *any* capabilities match.
+func NamespaceValidator(ops ...string) func(*ACL, string) bool {
+	return func(acl *ACL, ns string) bool {
+		// Always allow if ACLs are disabled.
+		if acl == nil {
+			return true
+		}
+
+		for _, op := range ops {
+			if acl.AllowNamespaceOperation(ns, op) {
+				// An operation is allowed, return true
+				return true
+			}
+		}
+
+		// No operations are allowed by this ACL, return false
+		return false
+	}
+}

client/alloc_endpoint.go

@@ -49,10 +49,15 @@ func (a *Allocations) GarbageCollectAll(args *nstructs.NodeSpecificRequest, repl
 func (a *Allocations) GarbageCollect(args *nstructs.AllocSpecificRequest, reply *nstructs.GenericResponse) error {
 	defer metrics.MeasureSince([]string{"client", "allocations", "garbage_collect"}, time.Now())
 
-	// Check submit job permissions
+	alloc, err := a.c.GetAlloc(args.AllocID)
+	if err != nil {
+		return err
+	}
+
+	// Check namespace submit job permission.
 	if aclObj, err := a.c.ResolveToken(args.AuthToken); err != nil {
 		return err
-	} else if aclObj != nil && !aclObj.AllowNsOp(args.Namespace, acl.NamespaceCapabilitySubmitJob) {
+	} else if aclObj != nil && !aclObj.AllowNsOp(alloc.Namespace, acl.NamespaceCapabilitySubmitJob) {
 		return nstructs.ErrPermissionDenied
 	}
 
@@ -68,10 +73,15 @@ func (a *Allocations) GarbageCollect(args *nstructs.AllocSpecificRequest, reply
 func (a *Allocations) Signal(args *nstructs.AllocSignalRequest, reply *nstructs.GenericResponse) error {
 	defer metrics.MeasureSince([]string{"client", "allocations", "signal"}, time.Now())
 
-	// Check alloc-lifecycle permissions
+	alloc, err := a.c.GetAlloc(args.AllocID)
+	if err != nil {
+		return err
+	}
+
+	// Check namespace alloc-lifecycle permission.
 	if aclObj, err := a.c.ResolveToken(args.AuthToken); err != nil {
 		return err
-	} else if aclObj != nil && !aclObj.AllowNsOp(args.Namespace, acl.NamespaceCapabilityAllocLifecycle) {
+	} else if aclObj != nil && !aclObj.AllowNsOp(alloc.Namespace, acl.NamespaceCapabilityAllocLifecycle) {
 		return nstructs.ErrPermissionDenied
 	}
 
@@ -82,9 +92,15 @@ func (a *Allocations) Signal(args *nstructs.AllocSignalRequest, reply *nstructs.
 func (a *Allocations) Restart(args *nstructs.AllocRestartRequest, reply *nstructs.GenericResponse) error {
 	defer metrics.MeasureSince([]string{"client", "allocations", "restart"}, time.Now())
 
+	alloc, err := a.c.GetAlloc(args.AllocID)
+	if err != nil {
+		return err
+	}
+
+	// Check namespace alloc-lifecycle permission.
 	if aclObj, err := a.c.ResolveToken(args.AuthToken); err != nil {
 		return err
-	} else if aclObj != nil && !aclObj.AllowNsOp(args.Namespace, acl.NamespaceCapabilityAllocLifecycle) {
+	} else if aclObj != nil && !aclObj.AllowNsOp(alloc.Namespace, acl.NamespaceCapabilityAllocLifecycle) {
 		return nstructs.ErrPermissionDenied
 	}
 
@@ -95,10 +111,15 @@ func (a *Allocations) Restart(args *nstructs.AllocRestartRequest, reply *nstruct
 func (a *Allocations) Stats(args *cstructs.AllocStatsRequest, reply *cstructs.AllocStatsResponse) error {
 	defer metrics.MeasureSince([]string{"client", "allocations", "stats"}, time.Now())
 
-	// Check read job permissions
+	alloc, err := a.c.GetAlloc(args.AllocID)
+	if err != nil {
+		return err
+	}
+
+	// Check read-job permission.
 	if aclObj, err := a.c.ResolveToken(args.AuthToken); err != nil {
 		return err
-	} else if aclObj != nil && !aclObj.AllowNsOp(args.Namespace, acl.NamespaceCapabilityReadJob) {
+	} else if aclObj != nil && !aclObj.AllowNsOp(alloc.Namespace, acl.NamespaceCapabilityReadJob) {
 		return nstructs.ErrPermissionDenied
 	}
 
@@ -148,6 +169,20 @@ func (a *Allocations) execImpl(encoder *codec.Encoder, decoder *codec.Decoder, e
 		return nil, structs.ErrPermissionDenied
 	}
 
+	if req.AllocID == "" {
+		return helper.Int64ToPtr(400), allocIDNotPresentErr
+	}
+	ar, err := a.c.getAllocRunner(req.AllocID)
+	if err != nil {
+		code := helper.Int64ToPtr(500)
+		if structs.IsErrUnknownAllocation(err) {
+			code = helper.Int64ToPtr(404)
+		}
+
+		return code, err
+	}
+	alloc := ar.Alloc()
+
 	aclObj, token, err := a.c.resolveTokenAndACL(req.QueryOptions.AuthToken)
 	{
 		// log access
@@ -167,37 +202,21 @@ func (a *Allocations) execImpl(encoder *codec.Encoder, decoder *codec.Decoder, e
 		)
 	}
 
-	// Check read permissions
+	// Check alloc-exec permission.
 	if err != nil {
 		return nil, err
-	} else if aclObj != nil {
-		exec := aclObj.AllowNsOp(req.QueryOptions.Namespace, acl.NamespaceCapabilityAllocExec)
-		if !exec {
-			return nil, structs.ErrPermissionDenied
-		}
+	} else if aclObj != nil && !aclObj.AllowNsOp(alloc.Namespace, acl.NamespaceCapabilityAllocExec) {
+		return nil, structs.ErrPermissionDenied
 	}
 
 	// Validate the arguments
-	if req.AllocID == "" {
-		return helper.Int64ToPtr(400), allocIDNotPresentErr
-	}
 	if req.Task == "" {
 		return helper.Int64ToPtr(400), taskNotPresentErr
 	}
 	if len(req.Cmd) == 0 {
 		return helper.Int64ToPtr(400), errors.New("command is not present")
 	}
 
-	ar, err := a.c.getAllocRunner(req.AllocID)
-	if err != nil {
-		code := helper.Int64ToPtr(500)
-		if structs.IsErrUnknownAllocation(err) {
-			code = helper.Int64ToPtr(404)
-		}
-
-		return code, err
-	}
-
 	capabilities, err := ar.GetTaskDriverCapabilities(req.Task)
 	if err != nil {
 		code := helper.Int64ToPtr(500)
@@ -210,7 +229,7 @@ func (a *Allocations) execImpl(encoder *codec.Encoder, decoder *codec.Decoder, e
 
 	// check node access
 	if aclObj != nil && capabilities.FSIsolation == drivers.FSIsolationNone {
-		exec := aclObj.AllowNsOp(req.QueryOptions.Namespace, acl.NamespaceCapabilityAllocNodeExec)
+		exec := aclObj.AllowNsOp(alloc.Namespace, acl.NamespaceCapabilityAllocNodeExec)
 		if !exec {
 			return nil, structs.ErrPermissionDenied
 		}