From a5026fc4f0a2b70d63c9f383ef35e5175202f6fe Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 5 Oct 2017 16:49:45 -0400 Subject: [PATCH 1/5] Add the ability to use multiple paths for capability checking. WIP (tests, docs). Fixes #3336 --- vault/logical_system.go | 66 ++++++++++++++++++++---------------- vault/logical_system_test.go | 12 +++++-- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index 3b174baaddde..ea57fed16167 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -100,7 +100,7 @@ func NewSystemBackend(core *Core) *SystemBackend { Description: "Accessor of the token for which capabilities are being queried.", }, "path": &framework.FieldSchema{ - Type: framework.TypeString, + Type: framework.TypeCommaStringSlice, Description: "Path on which capabilities are being queried.", }, }, @@ -150,7 +150,7 @@ func NewSystemBackend(core *Core) *SystemBackend { Description: "Token for which capabilities are being queried.", }, "path": &framework.FieldSchema{ - Type: framework.TypeString, + Type: framework.TypeCommaStringSlice, Description: "Path on which capabilities are being queried.", }, }, @@ -172,7 +172,7 @@ func NewSystemBackend(core *Core) *SystemBackend { Description: "Token for which capabilities are being queried.", }, "path": &framework.FieldSchema{ - Type: framework.TypeString, + Type: framework.TypeCommaStringSlice, Description: "Path on which capabilities are being queried.", }, }, @@ -1234,24 +1234,6 @@ func (b *SystemBackend) handleAuditedHeadersRead(req *logical.Request, d *framew }, nil } -// handleCapabilities returns the ACL capabilities of the token for a given path -func (b *SystemBackend) handleCapabilities(req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - token := d.Get("token").(string) - if token == "" { - token = req.ClientToken - } - capabilities, err := b.Core.Capabilities(token, d.Get("path").(string)) - if err != nil { - return nil, err - } - - return &logical.Response{ - Data: map[string]interface{}{ - "capabilities": capabilities, - }, - }, nil -} - // handleCapabilitiesAccessor returns the ACL capabilities of the // token associted with the given accessor for a given path. func (b *SystemBackend) handleCapabilitiesAccessor(req *logical.Request, d *framework.FieldData) (*logical.Response, error) { @@ -1265,16 +1247,42 @@ func (b *SystemBackend) handleCapabilitiesAccessor(req *logical.Request, d *fram return nil, err } - capabilities, err := b.Core.Capabilities(aEntry.TokenID, d.Get("path").(string)) - if err != nil { - return nil, err + d.Raw["token"] = aEntry.TokenID + return b.handleCapabilities(req, d) +} + +// handleCapabilities returns the ACL capabilities of the token for a given path +func (b *SystemBackend) handleCapabilities(req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + var token string + if strings.HasSuffix(req.Path, "capabilities-self") { + token = req.ClientToken + } else { + tokenRaw, ok := d.Raw["token"] + if ok { + token, _ = tokenRaw.(string) + } + } + if token == "" { + return nil, fmt.Errorf("no token found") } - return &logical.Response{ - Data: map[string]interface{}{ - "capabilities": capabilities, - }, - }, nil + ret := &logical.Response{ + Data: map[string]interface{}{}, + } + + paths := d.Get("path").([]string) + for _, path := range paths { + pathCap, err := b.Core.Capabilities(token, path) + if err != nil { + return nil, err + } + ret.Data[path] = pathCap + } + if len(paths) == 1 { + ret.Data["capabilities"] = ret.Data[paths[0]] + } + + return ret, nil } // handleRekeyRetrieve returns backed-up, PGP-encrypted unseal keys from a diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index 5d643280ade3..cd3e03075493 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -343,7 +343,11 @@ func TestSystemBackend_Capabilities(t *testing.T) { func testCapabilities(t *testing.T, endpoint string) { core, b, rootToken := testCoreSystemBackend(t) req := logical.TestRequest(t, logical.UpdateOperation, endpoint) - req.Data["token"] = rootToken + if endpoint == "capabilities-self" { + req.ClientToken = rootToken + } else { + req.Data["token"] = rootToken + } req.Data["path"] = "any_path" resp, err := b.HandleRequest(req) @@ -368,7 +372,11 @@ func testCapabilities(t *testing.T, endpoint string) { testMakeToken(t, core.tokenStore, rootToken, "tokenid", "", []string{"test"}) req = logical.TestRequest(t, logical.UpdateOperation, endpoint) - req.Data["token"] = "tokenid" + if endpoint == "capabilities-self" { + req.ClientToken = "tokenid" + } else { + req.Data["token"] = "tokenid" + } req.Data["path"] = "foo/bar" resp, err = b.HandleRequest(req) From 5450823e15b60d7d1a46b3e6f6ad76733e9eb781 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Mon, 12 Feb 2018 12:48:36 -0500 Subject: [PATCH 2/5] Added tests --- vault/logical_system.go | 2 + vault/logical_system_test.go | 162 +++++++++++++++++++++++++++++++++-- 2 files changed, 158 insertions(+), 6 deletions(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index ea57fed16167..76ed612ef632 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -1278,6 +1278,8 @@ func (b *SystemBackend) handleCapabilities(req *logical.Request, d *framework.Fi } ret.Data[path] = pathCap } + + // This is only here for backwards compatibility if len(paths) == 1 { ret.Data["capabilities"] = ret.Data[paths[0]] } diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index cd3e03075493..99421e55a056 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -333,9 +333,159 @@ path "foo/bar*" { path "sys/capabilities*" { capabilities = ["update"] } +path "bar/baz" { + capabilities = ["read", "update"] +} +path "bar/baz" { + capabilities = ["delete"] +} ` -func TestSystemBackend_Capabilities(t *testing.T) { +func TestSystemBackend_PathCapabilities(t *testing.T) { + var resp *logical.Response + var err error + + core, b, rootToken := testCoreSystemBackend(t) + + policy, _ := ParseACLPolicy(capabilitiesPolicy) + err = core.policyStore.SetPolicy(policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + path1 := "foo/bar" + path2 := "foo/bar/sample" + path3 := "sys/capabilities" + path4 := "bar/baz" + + rootCheckFunc := func(t *testing.T, resp *logical.Response) { + // All the paths should have "root" as the capability + expectedRoot := []string{"root"} + if !reflect.DeepEqual(resp.Data[path1], expectedRoot) || + !reflect.DeepEqual(resp.Data[path2], expectedRoot) || + !reflect.DeepEqual(resp.Data[path3], expectedRoot) || + !reflect.DeepEqual(resp.Data[path4], expectedRoot) { + t.Fatalf("bad: capabilities; expected: %#v, actual: %#v", expectedRoot, resp.Data) + } + } + + // Check the capabilities using the root token + resp, err = b.HandleRequest(&logical.Request{ + Path: "capabilities", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "path": []string{path1, path2, path3, path4}, + "token": rootToken, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + rootCheckFunc(t, resp) + + // Check the capabilities using capabilities-self + resp, err = b.HandleRequest(&logical.Request{ + ClientToken: rootToken, + Path: "capabilities-self", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "path": []string{path1, path2, path3, path4}, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + rootCheckFunc(t, resp) + + // Lookup the accessor of the root token + te, err := core.tokenStore.Lookup(rootToken) + if err != nil { + t.Fatal(err) + } + + // Check the capabilities using capabilities-accessor endpoint + resp, err = b.HandleRequest(&logical.Request{ + Path: "capabilities-accessor", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "path": []string{path1, path2, path3, path4}, + "accessor": te.Accessor, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + rootCheckFunc(t, resp) + + // Create a non-root token + testMakeToken(t, core.tokenStore, rootToken, "tokenid", "", []string{"test"}) + + nonRootCheckFunc := func(t *testing.T, resp *logical.Response) { + expected1 := []string{"create", "sudo", "update"} + expected2 := expected1 + expected3 := []string{"update"} + expected4 := []string{"delete", "read", "update"} + + if !reflect.DeepEqual(resp.Data[path1], expected1) || + !reflect.DeepEqual(resp.Data[path2], expected2) || + !reflect.DeepEqual(resp.Data[path3], expected3) || + !reflect.DeepEqual(resp.Data[path4], expected4) { + t.Fatalf("bad: capabilities; actual: %#v", resp.Data) + } + } + + // Check the capabilities using a non-root token + resp, err = b.HandleRequest(&logical.Request{ + Path: "capabilities", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "path": []string{path1, path2, path3, path4}, + "token": "tokenid", + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + nonRootCheckFunc(t, resp) + + // Check the capabilities of a non-root token using capabilities-self + // endpoint + resp, err = b.HandleRequest(&logical.Request{ + ClientToken: "tokenid", + Path: "capabilities-self", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "path": []string{path1, path2, path3, path4}, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + nonRootCheckFunc(t, resp) + + // Lookup the accessor of the non-root token + te, err = core.tokenStore.Lookup("tokenid") + if err != nil { + t.Fatal(err) + } + + // Check the capabilities using a non-root token using + // capabilities-accessor endpoint + resp, err = b.HandleRequest(&logical.Request{ + Path: "capabilities-accessor", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "path": []string{path1, path2, path3, path4}, + "accessor": te.Accessor, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + nonRootCheckFunc(t, resp) +} + +func TestSystemBackend_Capabilities_BC(t *testing.T) { testCapabilities(t, "capabilities") testCapabilities(t, "capabilities-self") } @@ -394,7 +544,7 @@ func testCapabilities(t *testing.T, endpoint string) { } } -func TestSystemBackend_CapabilitiesAccessor(t *testing.T) { +func TestSystemBackend_CapabilitiesAccessor_BC(t *testing.T) { core, b, rootToken := testCoreSystemBackend(t) te, err := core.tokenStore.Lookup(rootToken) if err != nil { @@ -1734,22 +1884,22 @@ func TestSystemBackend_rotate(t *testing.T) { func testSystemBackend(t *testing.T) logical.Backend { c, _, _ := TestCoreUnsealed(t) - return testSystemBackendInternal(t, c) + return c.systemBackend } func testSystemBackendRaw(t *testing.T) logical.Backend { c, _, _ := TestCoreUnsealedRaw(t) - return testSystemBackendInternal(t, c) + return c.systemBackend } func testCoreSystemBackend(t *testing.T) (*Core, logical.Backend, string) { c, _, root := TestCoreUnsealed(t) - return c, testSystemBackendInternal(t, c), root + return c, c.systemBackend, root } func testCoreSystemBackendRaw(t *testing.T) (*Core, logical.Backend, string) { c, _, root := TestCoreUnsealedRaw(t) - return c, testSystemBackendInternal(t, c), root + return c, c.systemBackend, root } func testSystemBackendInternal(t *testing.T, c *Core) logical.Backend { From eda270131821f3c75c27bf919d67121e00e6199a Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Mon, 12 Feb 2018 13:58:31 -0500 Subject: [PATCH 3/5] added 'paths' field --- vault/logical_system.go | 29 +++++++++++++++++++++++++---- vault/logical_system_test.go | 12 ++++++------ 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index fa3fedcb31b6..703cbd1a973d 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -102,7 +102,11 @@ func NewSystemBackend(core *Core) *SystemBackend { }, "path": &framework.FieldSchema{ Type: framework.TypeCommaStringSlice, - Description: "Path on which capabilities are being queried.", + Description: "(DEPRECATED) Path on which capabilities are being queried. Use 'paths' instead.", + }, + "paths": &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: "Paths on which capabilities are being queried.", }, }, @@ -152,7 +156,11 @@ func NewSystemBackend(core *Core) *SystemBackend { }, "path": &framework.FieldSchema{ Type: framework.TypeCommaStringSlice, - Description: "Path on which capabilities are being queried.", + Description: "(DEPRECATED) Path on which capabilities are being queried. Use 'paths' instead.", + }, + "paths": &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: "Paths on which capabilities are being queried.", }, }, @@ -174,7 +182,11 @@ func NewSystemBackend(core *Core) *SystemBackend { }, "path": &framework.FieldSchema{ Type: framework.TypeCommaStringSlice, - Description: "Path on which capabilities are being queried.", + Description: "(DEPRECATED) Path on which capabilities are being queried. Use 'paths' instead.", + }, + "paths": &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: "Paths on which capabilities are being queried.", }, }, @@ -1300,7 +1312,16 @@ func (b *SystemBackend) handleCapabilities(ctx context.Context, req *logical.Req Data: map[string]interface{}{}, } - paths := d.Get("path").([]string) + paths := d.Get("paths").([]string) + if len(paths) == 0 { + // Read from the deprecated field + paths = d.Get("path").([]string) + } + + if len(paths) == 0 { + return nil, nil + } + for _, path := range paths { pathCap, err := b.Core.Capabilities(ctx, token, path) if err != nil { diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index 783d8e76efc4..84fb35cbb3d9 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -374,7 +374,7 @@ func TestSystemBackend_PathCapabilities(t *testing.T) { Path: "capabilities", Operation: logical.UpdateOperation, Data: map[string]interface{}{ - "path": []string{path1, path2, path3, path4}, + "paths": []string{path1, path2, path3, path4}, "token": rootToken, }, }) @@ -389,7 +389,7 @@ func TestSystemBackend_PathCapabilities(t *testing.T) { Path: "capabilities-self", Operation: logical.UpdateOperation, Data: map[string]interface{}{ - "path": []string{path1, path2, path3, path4}, + "paths": []string{path1, path2, path3, path4}, }, }) if err != nil || (resp != nil && resp.IsError()) { @@ -408,7 +408,7 @@ func TestSystemBackend_PathCapabilities(t *testing.T) { Path: "capabilities-accessor", Operation: logical.UpdateOperation, Data: map[string]interface{}{ - "path": []string{path1, path2, path3, path4}, + "paths": []string{path1, path2, path3, path4}, "accessor": te.Accessor, }, }) @@ -439,7 +439,7 @@ func TestSystemBackend_PathCapabilities(t *testing.T) { Path: "capabilities", Operation: logical.UpdateOperation, Data: map[string]interface{}{ - "path": []string{path1, path2, path3, path4}, + "paths": []string{path1, path2, path3, path4}, "token": "tokenid", }, }) @@ -455,7 +455,7 @@ func TestSystemBackend_PathCapabilities(t *testing.T) { Path: "capabilities-self", Operation: logical.UpdateOperation, Data: map[string]interface{}{ - "path": []string{path1, path2, path3, path4}, + "paths": []string{path1, path2, path3, path4}, }, }) if err != nil || (resp != nil && resp.IsError()) { @@ -475,7 +475,7 @@ func TestSystemBackend_PathCapabilities(t *testing.T) { Path: "capabilities-accessor", Operation: logical.UpdateOperation, Data: map[string]interface{}{ - "path": []string{path1, path2, path3, path4}, + "paths": []string{path1, path2, path3, path4}, "accessor": te.Accessor, }, }) From b4b88af4720c83180bbd3dfe9568b1fdb2253b7b Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 22 Feb 2018 13:53:33 -0500 Subject: [PATCH 4/5] Update docs --- .../api/system/capabilities-accessor.html.md | 21 +++++++++++++----- .../api/system/capabilities-self.html.md | 17 +++++++++----- .../source/api/system/capabilities.html.md | 22 +++++++++++++------ 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/website/source/api/system/capabilities-accessor.html.md b/website/source/api/system/capabilities-accessor.html.md index 011850ce721b..730a9c9f0710 100644 --- a/website/source/api/system/capabilities-accessor.html.md +++ b/website/source/api/system/capabilities-accessor.html.md @@ -26,18 +26,18 @@ for the given path. ### Parameters -- `accessor` `(string: )` – Specifies the accessor of the token to - check. +- `accessor` `(string: )` – Accessor of the token for which + capabilities are being queried. -- `path` `(string: )` – Specifies the path on which the token's - capabilities will be checked. +- `paths` `(list: )` – Paths on which capabilities are being + queried. ### Sample Payload ```json { "accessor": "abcd1234", - "path": "secret/foo" + "paths": ["secret/foo", "secret/bar"] } ``` @@ -55,6 +55,15 @@ $ curl \ ```json { - "capabilities": ["read", "list"] + "secret/bar": [ + "sudo", + "update" + ], + "secret/foo": [ + "delete", + "list", + "read", + "update" + ] } ``` diff --git a/website/source/api/system/capabilities-self.html.md b/website/source/api/system/capabilities-self.html.md index 6957ea9efd7e..89f1570ba98f 100644 --- a/website/source/api/system/capabilities-self.html.md +++ b/website/source/api/system/capabilities-self.html.md @@ -26,14 +26,13 @@ client token is the Vault token with which this API call is made. ### Parameters -- `path` `(string: )` – Specifies the path on which the client token's - capabilities will be checked. +- `paths` `(list: )` – Paths on which capabilities are being queried. ### Sample Payload ```json { - "path": "secret/foo" + "paths": ["secret/foo", "secret/bar"] } ``` @@ -51,6 +50,14 @@ $ curl \ ```json { - "capabilities": ["read", "list"] + "secret/bar": [ + "sudo", + "update" + ], + "secret/foo": [ + "delete", + "list", + "read", + "update" + ] } -``` diff --git a/website/source/api/system/capabilities.html.md b/website/source/api/system/capabilities.html.md index c2449b03c0fc..2bf25806c644 100644 --- a/website/source/api/system/capabilities.html.md +++ b/website/source/api/system/capabilities.html.md @@ -24,18 +24,17 @@ This endpoint returns the list of capabilities for a provided token. ### Parameters -- `path` `(string: )` – Specifies the path against which to check the - token's capabilities. +- `paths` `(list: )` – Paths on which capabilities are being queried. -- `token` `(string: )` – Specifies the token for which to check - capabilities. +- `token` `(string: )` – Token for which capabilities are being + queried. ### Sample Payload ```json { - "path": "secret/foo", - "token": "abcd1234" + "token": "abcd1234", + "paths": ["secret/foo", "secret/bar"] } ``` @@ -53,6 +52,15 @@ $ curl \ ```json { - "capabilities": ["read", "list"] + "secret/bar": [ + "sudo", + "update" + ], + "secret/foo": [ + "delete", + "list", + "read", + "update" + ] } ``` From dc88bbcc5581bc9303095a9de0c81e7a83474bea Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Fri, 23 Feb 2018 22:18:08 -0500 Subject: [PATCH 5/5] return error if paths is not supplied --- vault/logical_system.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index b80b4d81f401..6d3d8d71fdf2 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -1319,7 +1319,7 @@ func (b *SystemBackend) handleCapabilities(ctx context.Context, req *logical.Req } if len(paths) == 0 { - return nil, nil + return logical.ErrorResponse("paths must be supplied"), nil } for _, path := range paths {