From 43ea09fa96bc99f5b9477b696248c262c5672898 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 4 Oct 2017 17:01:32 -0700 Subject: [PATCH] FS HTTP API ACL enforcement ACL enforcement for the filesystem HTTP APIs on clients. --- command/agent/fs_endpoint.go | 36 +++++++-- command/agent/fs_endpoint_test.go | 121 ++++++++++++++++++++++++++++++ website/source/api/client.html.md | 36 ++++----- 3 files changed, 170 insertions(+), 23 deletions(-) diff --git a/command/agent/fs_endpoint.go b/command/agent/fs_endpoint.go index 5bb5f8efb..53b2f294b 100644 --- a/command/agent/fs_endpoint.go +++ b/command/agent/fs_endpoint.go @@ -20,6 +20,7 @@ import ( "gopkg.in/tomb.v1" "github.com/docker/docker/pkg/ioutils" + "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/nomad/structs" "github.com/hpcloud/tail/watch" @@ -70,19 +71,44 @@ func (s *HTTPServer) FsRequest(resp http.ResponseWriter, req *http.Request) (int return nil, clientNotRunning } + var secret string + s.parseToken(req, &secret) + + var namespace string + parseNamespace(req, &namespace) + + requireReadFS := func(f func(http.ResponseWriter, *http.Request) (interface{}, error)) (interface{}, error) { + if aclObj, err := s.agent.Client().ResolveToken(secret); err != nil { + return nil, err + } else if aclObj != nil && !aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityReadFS) { + return nil, structs.ErrPermissionDenied + } + return f(resp, req) + } + path := strings.TrimPrefix(req.URL.Path, "/v1/client/fs/") switch { case strings.HasPrefix(path, "ls/"): - return s.DirectoryListRequest(resp, req) + return requireReadFS(s.DirectoryListRequest) case strings.HasPrefix(path, "stat/"): - return s.FileStatRequest(resp, req) + return requireReadFS(s.FileStatRequest) case strings.HasPrefix(path, "readat/"): - return s.FileReadAtRequest(resp, req) + return requireReadFS(s.FileReadAtRequest) case strings.HasPrefix(path, "cat/"): - return s.FileCatRequest(resp, req) + return requireReadFS(s.FileCatRequest) case strings.HasPrefix(path, "stream/"): - return s.Stream(resp, req) + return requireReadFS(s.Stream) case strings.HasPrefix(path, "logs/"): + // Logs can be accessed with ReadFS or ReadLogs caps + if aclObj, err := s.agent.Client().ResolveToken(secret); err != nil { + return nil, err + } else if aclObj != nil { + readfs := aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityReadFS) + logs := aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityReadLogs) + if !readfs && !logs { + return nil, structs.ErrPermissionDenied + } + } return s.Logs(resp, req) default: return nil, CodedError(404, ErrInvalidMethod) diff --git a/command/agent/fs_endpoint_test.go b/command/agent/fs_endpoint_test.go index d484cdaad..6bb9f38c2 100644 --- a/command/agent/fs_endpoint_test.go +++ b/command/agent/fs_endpoint_test.go @@ -18,9 +18,12 @@ import ( "testing" "time" + "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/client/allocdir" + "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/assert" "github.com/ugorji/go/codec" ) @@ -106,6 +109,124 @@ func TestAllocDirFS_ReadAt_MissingParams(t *testing.T) { }) } +func TestAllocDirFS_ACL(t *testing.T) { + t.Parallel() + assert := assert.New(t) + + httpACLTest(t, nil, func(s *TestAgent) { + state := s.Agent.server.State() + + req, err := http.NewRequest("GET", "/v1/client/fs/ls/", nil) + assert.Nil(err) + + // Try request without a token and expect failure + { + respW := httptest.NewRecorder() + _, err := s.Server.FsRequest(respW, req) + assert.NotNil(err) + assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + } + + // Try request with an invalid token and expect failure + { + respW := httptest.NewRecorder() + policy := mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadLogs}) + token := mock.CreatePolicyAndToken(t, state, 1005, "invalid", policy) + setToken(req, token) + _, err := s.Server.FsRequest(respW, req) + assert.NotNil(err) + assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + } + + // Try request with a valid token + // No alloc id set, so expect an error - just not a permissions error + { + respW := httptest.NewRecorder() + policy := mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadFS}) + token := mock.CreatePolicyAndToken(t, state, 1007, "valid", policy) + setToken(req, token) + _, err := s.Server.FsRequest(respW, req) + assert.NotNil(err) + assert.Equal(allocIDNotPresentErr, err) + } + + // Try request with a management token + // No alloc id set, so expect an error - just not a permissions error + { + respW := httptest.NewRecorder() + setToken(req, s.RootToken) + _, err := s.Server.FsRequest(respW, req) + assert.NotNil(err) + assert.Equal(allocIDNotPresentErr, err) + } + }) +} + +func TestAllocDirFS_Logs_ACL(t *testing.T) { + t.Parallel() + assert := assert.New(t) + + httpACLTest(t, nil, func(s *TestAgent) { + state := s.Agent.server.State() + + req, err := http.NewRequest("GET", "/v1/client/fs/logs/", nil) + assert.Nil(err) + + // Try request without a token and expect failure + { + respW := httptest.NewRecorder() + _, err := s.Server.FsRequest(respW, req) + assert.NotNil(err) + assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + } + + // Try request with an invalid token and expect failure + { + respW := httptest.NewRecorder() + policy := mock.NamespacePolicy("other", "", []string{acl.NamespaceCapabilityReadFS}) + token := mock.CreatePolicyAndToken(t, state, 1005, "invalid", policy) + setToken(req, token) + _, err := s.Server.FsRequest(respW, req) + assert.NotNil(err) + assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + } + + // Try request with a valid token (ReadFS) + // No alloc id set, so expect an error - just not a permissions error + { + respW := httptest.NewRecorder() + policy := mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadFS}) + token := mock.CreatePolicyAndToken(t, state, 1007, "valid1", policy) + setToken(req, token) + _, err := s.Server.FsRequest(respW, req) + assert.NotNil(err) + assert.Equal(allocIDNotPresentErr, err) + } + + // Try request with a valid token (ReadLogs) + // No alloc id set, so expect an error - just not a permissions error + { + respW := httptest.NewRecorder() + policy := mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadLogs}) + token := mock.CreatePolicyAndToken(t, state, 1009, "valid2", policy) + setToken(req, token) + _, err := s.Server.FsRequest(respW, req) + assert.NotNil(err) + assert.Equal(allocIDNotPresentErr, err) + } + + // Try request with a management token + // No alloc id set, so expect an error - just not a permissions error + { + respW := httptest.NewRecorder() + setToken(req, s.RootToken) + _, err := s.Server.FsRequest(respW, req) + assert.NotNil(err) + assert.Equal(allocIDNotPresentErr, err) + } + }) +} + type WriteCloseChecker struct { io.WriteCloser Closed bool diff --git a/website/source/api/client.html.md b/website/source/api/client.html.md index dda6d4d4d..0a1ec1b49 100644 --- a/website/source/api/client.html.md +++ b/website/source/api/client.html.md @@ -248,9 +248,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` | `namespace:read-fs` | ### Parameters @@ -293,9 +293,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` | `namespace:read-fs` | ### Parameters @@ -337,9 +337,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` | `namespace:read-fs` | ### Parameters @@ -403,9 +403,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` | `namespace:read-logs` or `namespace:read-fs` | ### Parameters @@ -476,9 +476,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` | `namespace:read-fs` | ### Parameters @@ -529,9 +529,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` | `namespace:read-fs` | ### Parameters