From aefba77281d8bbfd9d46285ad2267a1c0afdd9c2 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 10 Oct 2017 15:04:23 -0700 Subject: [PATCH 1/2] Prefix Search ACL enforcement --- nomad/search_endpoint.go | 46 +++++++++++- nomad/search_endpoint_test.go | 112 ++++++++++++++++++++++++++++++ website/source/api/search.html.md | 11 ++- 3 files changed, 164 insertions(+), 5 deletions(-) diff --git a/nomad/search_endpoint.go b/nomad/search_endpoint.go index 1404a4f41..e0ad2dee4 100644 --- a/nomad/search_endpoint.go +++ b/nomad/search_endpoint.go @@ -4,6 +4,7 @@ import ( "strings" memdb "github.com/hashicorp/go-memdb" + "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" ) @@ -107,8 +108,36 @@ func roundUUIDDownIfOdd(prefix string, context structs.Context) string { // PrefixSearch is used to list matches for a given prefix, and returns // matching jobs, evaluations, allocations, and/or nodes. -func (s *Search) PrefixSearch(args *structs.SearchRequest, - reply *structs.SearchResponse) error { +func (s *Search) PrefixSearch(args *structs.SearchRequest, reply *structs.SearchResponse) error { + aclObj, err := s.srv.ResolveToken(args.SecretID) + if err != nil { + return err + } + + // Require either node:read or namespace:read-job + nodeRead := true + jobRead := true + if aclObj != nil { + nodeRead = aclObj.AllowNodeRead() + jobRead = aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilityReadJob) + if !nodeRead && !jobRead { + return structs.ErrPermissionDenied + } + + // Reject requests that explicitly specify a disallowed context. This + // should give the user better feedback then simply filtering out all + // results and returning an empty list. + if !nodeRead && args.Context == structs.Nodes { + return structs.ErrPermissionDenied + } + if !jobRead { + switch args.Context { + case structs.Allocs, structs.Deployments, structs.Evals, structs.Jobs: + return structs.ErrPermissionDenied + } + } + } + reply.Matches = make(map[structs.Context][]string) reply.Truncations = make(map[structs.Context]bool) @@ -126,6 +155,19 @@ func (s *Search) PrefixSearch(args *structs.SearchRequest, } for _, ctx := range contexts { + if ctx == structs.Nodes && !nodeRead { + // Not allowed to search nodes + continue + } + + if !jobRead { + switch ctx { + case structs.Allocs, structs.Deployments, structs.Evals, structs.Jobs: + // Not allowed to read jobs + continue + } + } + iter, err := getResourceIter(ctx, args.RequestNamespace(), roundUUIDDownIfOdd(args.Prefix, args.Context), ws, state) if err != nil { diff --git a/nomad/search_endpoint_test.go b/nomad/search_endpoint_test.go index 5846bab43..84b6a050f 100644 --- a/nomad/search_endpoint_test.go +++ b/nomad/search_endpoint_test.go @@ -6,6 +6,7 @@ import ( "testing" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" + "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" @@ -59,6 +60,117 @@ func TestSearch_PrefixSearch_Job(t *testing.T) { assert.Equal(uint64(jobIndex), resp.Index) } +func TestSearch_PrefixSearch_ACL(t *testing.T) { + assert := assert.New(t) + jobID := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" + + t.Parallel() + s, root := testACLServer(t, func(c *Config) { + c.NumSchedulers = 0 + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + state := s.fsm.State() + + job := registerAndVerifyJob(s, t, jobID, 0) + assert.Nil(state.UpsertNode(1001, mock.Node())) + + req := &structs.SearchRequest{ + Prefix: "", + Context: structs.Jobs, + QueryOptions: structs.QueryOptions{ + Region: "global", + Namespace: job.Namespace, + }, + } + + // Try without a token and expect failure + { + var resp structs.SearchResponse + err := msgpackrpc.CallWithCodec(codec, "Search.PrefixSearch", req, &resp) + assert.NotNil(err) + assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + } + + // Try with an invalid token and expect failure + { + invalidToken := mock.CreatePolicyAndToken(t, state, 1003, "test-invalid", + mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityListJobs})) + req.SecretID = invalidToken.SecretID + var resp structs.SearchResponse + err := msgpackrpc.CallWithCodec(codec, "Search.PrefixSearch", req, &resp) + assert.NotNil(err) + assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + } + + // Try with a node:read token and expect failure due to Jobs being the context + { + validToken := mock.CreatePolicyAndToken(t, state, 1005, "test-invalid2", mock.NodePolicy(acl.PolicyRead)) + req.SecretID = validToken.SecretID + var resp structs.SearchResponse + err := msgpackrpc.CallWithCodec(codec, "Search.PrefixSearch", req, &resp) + assert.NotNil(err) + assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + } + + // Try with a node:read token and expect success due to All context + { + validToken := mock.CreatePolicyAndToken(t, state, 1007, "test-valid", mock.NodePolicy(acl.PolicyRead)) + req.Context = structs.All + req.SecretID = validToken.SecretID + var resp structs.SearchResponse + assert.Nil(msgpackrpc.CallWithCodec(codec, "Search.PrefixSearch", req, &resp)) + assert.Equal(uint64(1001), resp.Index) + assert.Len(resp.Matches[structs.Nodes], 1) + + // Jobs filtered out since token only has access to node:read + assert.Len(resp.Matches[structs.Jobs], 0) + } + + // Try with a valid token for namespace:read-job + { + validToken := mock.CreatePolicyAndToken(t, state, 1009, "test-valid2", + mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadJob})) + req.SecretID = validToken.SecretID + var resp structs.SearchResponse + assert.Nil(msgpackrpc.CallWithCodec(codec, "Search.PrefixSearch", req, &resp)) + assert.Equal(uint64(1001), resp.Index) + assert.Len(resp.Matches[structs.Jobs], 1) + assert.Equal(job.ID, resp.Matches[structs.Jobs][0]) + + // Nodes filtered out since token only has access to namespace:read-job + assert.Len(resp.Matches[structs.Nodes], 0) + } + + // Try with a valid token for node:read and namespace:read-job + { + validToken := mock.CreatePolicyAndToken(t, state, 1011, "test-valid3", strings.Join([]string{ + mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadJob}), + mock.NodePolicy(acl.PolicyRead), + }, "\n")) + req.SecretID = validToken.SecretID + var resp structs.SearchResponse + assert.Nil(msgpackrpc.CallWithCodec(codec, "Search.PrefixSearch", req, &resp)) + assert.Equal(uint64(1001), resp.Index) + assert.Len(resp.Matches[structs.Jobs], 1) + assert.Equal(job.ID, resp.Matches[structs.Jobs][0]) + assert.Len(resp.Matches[structs.Nodes], 1) + } + + // Try with a management token + { + req.SecretID = root.SecretID + var resp structs.SearchResponse + assert.Nil(msgpackrpc.CallWithCodec(codec, "Search.PrefixSearch", req, &resp)) + assert.Equal(uint64(1001), resp.Index) + assert.Len(resp.Matches[structs.Jobs], 1) + assert.Equal(job.ID, resp.Matches[structs.Jobs][0]) + assert.Len(resp.Matches[structs.Nodes], 1) + } +} + func TestSearch_PrefixSearch_All_JobWithHyphen(t *testing.T) { assert := assert.New(t) prefix := "example-test-------" // Assert that a job with more than 4 hyphens works diff --git a/website/source/api/search.html.md b/website/source/api/search.html.md index ec07f4cf0..52b413bfb 100644 --- a/website/source/api/search.html.md +++ b/website/source/api/search.html.md @@ -20,9 +20,14 @@ 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:read, namespace:read-jobs` | + +When ACLs are enabled, requests must have a token valid for `node:read` or +`namespace:read-jobs` roles. If the token is only valid for `node:read`, then +job related results will not be returned. If the token is only valid for +`namespace:read-jobs`, then node results will not be returned. ### Parameters From f4e3fa6770189ea395da80cd54efca924b8cf76c Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 11 Oct 2017 18:05:27 -0700 Subject: [PATCH 2/2] Refactor permissions checks into funcs funcs are in the _oss file to ease creating Enterprise versions which support Quotas and Namespaces. --- nomad/search_endpoint.go | 56 +++++++---------------------- nomad/search_endpoint_oss.go | 67 +++++++++++++++++++++++++++++++++++ nomad/search_endpoint_test.go | 6 ++-- 3 files changed, 84 insertions(+), 45 deletions(-) diff --git a/nomad/search_endpoint.go b/nomad/search_endpoint.go index e0ad2dee4..14aa98cb9 100644 --- a/nomad/search_endpoint.go +++ b/nomad/search_endpoint.go @@ -4,7 +4,6 @@ import ( "strings" memdb "github.com/hashicorp/go-memdb" - "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" ) @@ -18,8 +17,13 @@ const ( var ( // ossContexts are the oss contexts which are searched to find matches // for a given prefix - ossContexts = []structs.Context{structs.Allocs, structs.Jobs, structs.Nodes, - structs.Evals, structs.Deployments} + ossContexts = []structs.Context{ + structs.Allocs, + structs.Jobs, + structs.Nodes, + structs.Evals, + structs.Deployments, + } ) // Search endpoint is used to look up matches for a given prefix and context @@ -114,28 +118,11 @@ func (s *Search) PrefixSearch(args *structs.SearchRequest, reply *structs.Search return err } - // Require either node:read or namespace:read-job - nodeRead := true - jobRead := true - if aclObj != nil { - nodeRead = aclObj.AllowNodeRead() - jobRead = aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilityReadJob) - if !nodeRead && !jobRead { - return structs.ErrPermissionDenied - } + namespace := args.RequestNamespace() - // Reject requests that explicitly specify a disallowed context. This - // should give the user better feedback then simply filtering out all - // results and returning an empty list. - if !nodeRead && args.Context == structs.Nodes { - return structs.ErrPermissionDenied - } - if !jobRead { - switch args.Context { - case structs.Allocs, structs.Deployments, structs.Evals, structs.Jobs: - return structs.ErrPermissionDenied - } - } + // Require either node:read or namespace:read-job + if !anySearchPerms(aclObj, namespace, args.Context) { + return structs.ErrPermissionDenied } reply.Matches = make(map[structs.Context][]string) @@ -149,27 +136,10 @@ func (s *Search) PrefixSearch(args *structs.SearchRequest, reply *structs.Search iters := make(map[structs.Context]memdb.ResultIterator) - contexts := allContexts - if args.Context != structs.All { - contexts = []structs.Context{args.Context} - } + contexts := searchContexts(aclObj, namespace, args.Context) for _, ctx := range contexts { - if ctx == structs.Nodes && !nodeRead { - // Not allowed to search nodes - continue - } - - if !jobRead { - switch ctx { - case structs.Allocs, structs.Deployments, structs.Evals, structs.Jobs: - // Not allowed to read jobs - continue - } - } - - iter, err := getResourceIter(ctx, args.RequestNamespace(), roundUUIDDownIfOdd(args.Prefix, args.Context), ws, state) - + iter, err := getResourceIter(ctx, namespace, roundUUIDDownIfOdd(args.Prefix, args.Context), ws, state) if err != nil { e := err.Error() switch { diff --git a/nomad/search_endpoint_oss.go b/nomad/search_endpoint_oss.go index 8eb7b859e..43abf38f5 100644 --- a/nomad/search_endpoint_oss.go +++ b/nomad/search_endpoint_oss.go @@ -6,6 +6,7 @@ import ( "fmt" memdb "github.com/hashicorp/go-memdb" + "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" ) @@ -28,3 +29,69 @@ func getEnterpriseResourceIter(context structs.Context, namespace, prefix string // open source contexts. return nil, fmt.Errorf("context must be one of %v or 'all' for all contexts; got %q", allContexts, context) } + +// anySearchPerms returns true if the provided ACL has access to any +// capabilities required for prefix searching. Returns true if aclObj is nil. +func anySearchPerms(aclObj *acl.ACL, namespace string, context structs.Context) bool { + if aclObj == nil { + return true + } + + nodeRead := aclObj.AllowNodeRead() + jobRead := aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityReadJob) + if !nodeRead && !jobRead { + return false + } + + // Reject requests that explicitly specify a disallowed context. This + // should give the user better feedback then simply filtering out all + // results and returning an empty list. + if !nodeRead && context == structs.Nodes { + return false + } + if !jobRead { + switch context { + case structs.Allocs, structs.Deployments, structs.Evals, structs.Jobs: + return false + } + } + + return true +} + +// searchContexts returns the contexts the aclObj is valid for. If aclObj is +// nil all contexts are returned. +func searchContexts(aclObj *acl.ACL, namespace string, context structs.Context) []structs.Context { + var all []structs.Context + + switch context { + case structs.All: + all = make([]structs.Context, len(allContexts)) + copy(all, allContexts) + default: + all = []structs.Context{context} + } + + // If ACLs aren't enabled return all contexts + if aclObj == nil { + return all + } + + jobRead := aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityReadJob) + + // Filter contexts down to those the ACL grants access to + available := make([]structs.Context, 0, len(all)) + for _, c := range all { + switch c { + case structs.Allocs, structs.Jobs, structs.Evals, structs.Deployments: + if jobRead { + available = append(available, c) + } + case structs.Nodes: + if aclObj.AllowNodeRead() { + available = append(available, c) + } + } + } + return available +} diff --git a/nomad/search_endpoint_test.go b/nomad/search_endpoint_test.go index 84b6a050f..6f55bba10 100644 --- a/nomad/search_endpoint_test.go +++ b/nomad/search_endpoint_test.go @@ -136,10 +136,12 @@ func TestSearch_PrefixSearch_ACL(t *testing.T) { req.SecretID = validToken.SecretID var resp structs.SearchResponse assert.Nil(msgpackrpc.CallWithCodec(codec, "Search.PrefixSearch", req, &resp)) - assert.Equal(uint64(1001), resp.Index) assert.Len(resp.Matches[structs.Jobs], 1) assert.Equal(job.ID, resp.Matches[structs.Jobs][0]) + // Index of job - not node - because node context is filtered out + assert.Equal(uint64(1000), resp.Index) + // Nodes filtered out since token only has access to namespace:read-job assert.Len(resp.Matches[structs.Nodes], 0) } @@ -153,10 +155,10 @@ func TestSearch_PrefixSearch_ACL(t *testing.T) { req.SecretID = validToken.SecretID var resp structs.SearchResponse assert.Nil(msgpackrpc.CallWithCodec(codec, "Search.PrefixSearch", req, &resp)) - assert.Equal(uint64(1001), resp.Index) assert.Len(resp.Matches[structs.Jobs], 1) assert.Equal(job.ID, resp.Matches[structs.Jobs][0]) assert.Len(resp.Matches[structs.Nodes], 1) + assert.Equal(uint64(1001), resp.Index) } // Try with a management token