From ede8db07200f8ce059e1037590f3a39c941a96eb Mon Sep 17 00:00:00 2001 From: Chris Hoffman Date: Mon, 3 Sep 2018 09:43:25 -0400 Subject: [PATCH 1/3] fixing capabilities check for templated policies --- vault/acl.go | 4 +- vault/capabilities.go | 29 +++----------- vault/capabilities_test.go | 77 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 26 deletions(-) diff --git a/vault/acl.go b/vault/acl.go index d027729d3807..6132ab762366 100644 --- a/vault/acl.go +++ b/vault/acl.go @@ -231,10 +231,8 @@ func (a *ACL) Capabilities(path string) (pathCapabilities []string) { _, raw, ok = a.globRules.LongestPrefix(path) if !ok { return []string{DenyCapability} - } else { - perm := raw.(*ACLPermissions) - capabilities = perm.CapabilitiesBitmap } + capabilities = raw.(*ACLPermissions).CapabilitiesBitmap CHECK: if capabilities&SudoCapabilityInt > 0 { diff --git a/vault/capabilities.go b/vault/capabilities.go index c3ffa9ee9af5..84212397956e 100644 --- a/vault/capabilities.go +++ b/vault/capabilities.go @@ -25,46 +25,29 @@ func (c *Core) Capabilities(ctx context.Context, token, path string) ([]string, return nil, &logical.StatusBadRequest{Err: "invalid token"} } - if te.Policies == nil { - return []string{DenyCapability}, nil - } - - var policies []*Policy - for _, tePolicy := range te.Policies { - policy, err := c.policyStore.GetPolicy(ctx, tePolicy, PolicyTypeToken) - if err != nil { - return nil, err - } - policies = append(policies, policy) - } + // Start with token entry policies + policies := te.Policies + // Fetch entity and entity group policies entity, derivedPolicies, err := c.fetchEntityAndDerivedPolicies(te.EntityID) if err != nil { return nil, err } - if entity != nil && entity.Disabled { c.logger.Warn("permission denied as the entity on the token is disabled") return nil, logical.ErrPermissionDenied } - if te != nil && te.EntityID != "" && entity == nil { + if te.EntityID != "" && entity == nil { c.logger.Warn("permission denied as the entity on the token is invalid") return nil, logical.ErrPermissionDenied } - - for _, item := range derivedPolicies { - policy, err := c.policyStore.GetPolicy(ctx, item, PolicyTypeToken) - if err != nil { - return nil, err - } - policies = append(policies, policy) - } + policies = append(policies, derivedPolicies...) if len(policies) == 0 { return []string{DenyCapability}, nil } - acl, err := NewACL(policies) + acl, err := c.policyStore.ACL(ctx, entity, policies...) if err != nil { return nil, err } diff --git a/vault/capabilities_test.go b/vault/capabilities_test.go index 368478838381..b03c545f170a 100644 --- a/vault/capabilities_test.go +++ b/vault/capabilities_test.go @@ -2,6 +2,7 @@ package vault import ( "context" + "fmt" "reflect" "sort" "testing" @@ -115,6 +116,82 @@ path "secret/sample" { } } +func TestCapabilities_TemplatedPolicies(t *testing.T) { + var resp *logical.Response + var err error + + i, _, c := testIdentityStoreWithGithubAuth(t) + + // Create an entity and assign policy1 to it + entityReq := &logical.Request{ + Path: "entity", + Operation: logical.UpdateOperation, + } + resp, err = i.HandleRequest(context.Background(), entityReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %#v\n", resp, err) + } + entityID := resp.Data["id"].(string) + + // Create a token for the entity and assign policy2 on the token + ent := &logical.TokenEntry{ + ID: "capabilitiestoken", + Path: "auth/token/create", + Policies: []string{"testpolicy"}, + EntityID: entityID, + TTL: time.Hour, + } + testMakeTokenDirectly(t, c.tokenStore, ent) + + tCases := []struct { + policy string + path string + expected []string + }{ + { + `name = "testpolicy" + path "secret/{{identity.entity.id}}/sample" { + capabilities = ["update", "create"] + } + `, + fmt.Sprintf("secret/%s/sample", entityID), + []string{"update", "create"}, + }, + { + `{"name": "testpolicy", "path": {"secret/{{identity.entity.id}}/sample": {"capabilities": ["read", "create"]}}}`, + fmt.Sprintf("secret/%s/sample", entityID), + []string{"read", "create"}, + }, + { + `{"name": "testpolicy", "path": {"secret/sample": {"capabilities": ["read"]}}}`, + "secret/sample", + []string{"read"}, + }, + } + + for _, tCase := range tCases { + // Create the above policies + policy, err := ParseACLPolicy(tCase.policy) + if err != nil { + t.Fatalf("err: %v", err) + } + err = c.policyStore.SetPolicy(context.Background(), policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + actual, err := c.Capabilities(context.Background(), "capabilitiestoken", tCase.path) + if err != nil { + t.Fatalf("err: %v", err) + } + sort.Strings(actual) + sort.Strings(tCase.expected) + if !reflect.DeepEqual(actual, tCase.expected) { + t.Fatalf("bad: got\n%#v\nexpected\n%#v\n", actual, tCase.expected) + } + } +} + func TestCapabilities(t *testing.T) { c, _, token := TestCoreUnsealed(t) From b7ff7d139c8e8f1c7cfc99b21f34dea09c382afa Mon Sep 17 00:00:00 2001 From: Chris Hoffman Date: Tue, 4 Sep 2018 14:16:07 -0400 Subject: [PATCH 2/3] remove unnecessary change --- vault/acl.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vault/acl.go b/vault/acl.go index 6132ab762366..17d6be6997eb 100644 --- a/vault/acl.go +++ b/vault/acl.go @@ -231,8 +231,10 @@ func (a *ACL) Capabilities(path string) (pathCapabilities []string) { _, raw, ok = a.globRules.LongestPrefix(path) if !ok { return []string{DenyCapability} + } else { + perm := raw.(*ACLPermissions) + capabilities = perm.CapabilitiesBitmap } - capabilities = raw.(*ACLPermissions).CapabilitiesBitmap CHECK: if capabilities&SudoCapabilityInt > 0 { From 4266722184777b80bcde5125018b62eb8e15bbd0 Mon Sep 17 00:00:00 2001 From: Chris Hoffman Date: Tue, 4 Sep 2018 14:16:45 -0400 Subject: [PATCH 3/3] formatting --- vault/acl.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vault/acl.go b/vault/acl.go index 17d6be6997eb..d027729d3807 100644 --- a/vault/acl.go +++ b/vault/acl.go @@ -231,8 +231,8 @@ func (a *ACL) Capabilities(path string) (pathCapabilities []string) { _, raw, ok = a.globRules.LongestPrefix(path) if !ok { return []string{DenyCapability} - } else { - perm := raw.(*ACLPermissions) + } else { + perm := raw.(*ACLPermissions) capabilities = perm.CapabilitiesBitmap }