From 5b22fd4b6d7b91e324f125be3dd8489fde35c112 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Thu, 14 Sep 2017 20:41:44 -0700 Subject: [PATCH] Node.Evaluate ACL enforcement --- nomad/node_endpoint.go | 7 ++++ nomad/node_endpoint_test.go | 56 ++++++++++++++++++++++++++++++++ website/source/api/nodes.html.md | 6 ++-- 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index 4e6dd793d..a34d1cccf 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -448,6 +448,13 @@ func (n *Node) Evaluate(args *structs.NodeEvaluateRequest, reply *structs.NodeUp } defer metrics.MeasureSince([]string{"nomad", "client", "evaluate"}, time.Now()) + // Check node write permissions + if aclObj, err := n.srv.resolveToken(args.SecretID); err != nil { + return err + } else if aclObj != nil && !aclObj.AllowNodeWrite() { + return structs.ErrPermissionDenied + } + // Verify the arguments if args.NodeID == "" { return fmt.Errorf("missing node ID for evaluation") diff --git a/nomad/node_endpoint_test.go b/nomad/node_endpoint_test.go index d052c19d0..bf5847724 100644 --- a/nomad/node_endpoint_test.go +++ b/nomad/node_endpoint_test.go @@ -1695,6 +1695,62 @@ func TestClientEndpoint_Evaluate(t *testing.T) { } } +func TestClientEndpoint_Evaluate_ACL(t *testing.T) { + t.Parallel() + s1, root := testACLServer(t, nil) + defer s1.Shutdown() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + assert := assert.New(t) + + // Create the alloc + alloc := mock.Alloc() + node := mock.Node() + node.ID = alloc.NodeID + state := s1.fsm.State() + + assert.Nil(state.UpsertNode(1, node), "UpsertNode") + assert.Nil(state.UpsertJobSummary(2, mock.JobSummary(alloc.JobID)), "UpsertJobSummary") + assert.Nil(state.UpsertAllocs(3, []*structs.Allocation{alloc}), "UpsertAllocs") + + // Create the namespace policy and tokens + validToken := CreatePolicyAndToken(t, state, 1001, "test-valid", NodePolicy(acl.PolicyWrite)) + invalidToken := CreatePolicyAndToken(t, state, 1003, "test-invalid", NodePolicy(acl.PolicyRead)) + + // Re-evaluate without a token and expect failure + req := &structs.NodeEvaluateRequest{ + NodeID: alloc.NodeID, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + { + var resp structs.NodeUpdateResponse + assert.NotNil(msgpackrpc.CallWithCodec(codec, "Node.Evaluate", req, &resp), "RPC") + } + + // Try with a valid token + req.SecretID = validToken.SecretID + { + var resp structs.NodeUpdateResponse + assert.Nil(msgpackrpc.CallWithCodec(codec, "Node.Evaluate", req, &resp), "RPC") + } + + // Try with a invalid token + req.SecretID = invalidToken.SecretID + { + var resp structs.NodeUpdateResponse + err := msgpackrpc.CallWithCodec(codec, "Node.Evaluate", req, &resp) + assert.NotNil(err, "RPC") + assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + } + + // Try with a root token + req.SecretID = root.SecretID + { + var resp structs.NodeUpdateResponse + assert.Nil(msgpackrpc.CallWithCodec(codec, "Node.Evaluate", req, &resp), "RPC") + } +} + func TestClientEndpoint_ListNodes(t *testing.T) { t.Parallel() s1 := testServer(t, nil) diff --git a/website/source/api/nodes.html.md b/website/source/api/nodes.html.md index 97cb883a4..b0ef03aab 100644 --- a/website/source/api/nodes.html.md +++ b/website/source/api/nodes.html.md @@ -523,9 +523,9 @@ The table below shows this endpoint's support for [blocking queries](/api/index.html#blocking-queries) and [required ACLs](/api/index.html#acls). -| Blocking Queries | ACL Required | -| ---------------- | ------------ | -| `NO` | `none` | +| Blocking Queries | ACL Required | +| ---------------- | ------------------ | +| `NO` | `node:write` | ### Parameters