From ac9d09cf942582128d1ce815081c7b86780c4791 Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Wed, 31 Jul 2019 14:44:24 +0200 Subject: [PATCH 1/4] Fix create token sudo non-root namespace check --- vault/token_store.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 2a34d3289b60..af3ac6828726 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -2094,8 +2094,18 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque logical.ErrInvalidRequest } + // Get the context namespace + ns, err := namespace.FromContext(ctx) + if err != nil { + return nil, err + } + // Check if the client token has sudo/root privileges for the requested path - isSudo := ts.System().SudoPrivilege(ctx, req.MountPoint+req.Path, req.ClientToken) + mountPath := req.MountPoint+req.Path + if ns != namespace.RootNamespace { + mountPath = strings.TrimPrefix(mountPath, ns.Path) + } + isSudo := ts.System().SudoPrivilege(ctx, mountPath, req.ClientToken) // Read and parse the fields var data struct { @@ -2122,10 +2132,6 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // If the context's namespace is different from the parent and this is an // orphan token creation request, then this is an admin token generation for // the namespace - ns, err := namespace.FromContext(ctx) - if err != nil { - return nil, err - } if ns.ID != parent.NamespaceID { parentNS, err := NamespaceByID(ctx, parent.NamespaceID, ts.core) if err != nil { From e8bf16770370cfd2302d2695dbe20136390fca8a Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Wed, 31 Jul 2019 16:21:13 +0200 Subject: [PATCH 2/4] Moved path trimming to SudoPrivilege --- vault/dynamic_system_view.go | 4 ++++ vault/token_store.go | 16 +++++----------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/vault/dynamic_system_view.go b/vault/dynamic_system_view.go index 1cd74af4e583..38e6629b6519 100644 --- a/vault/dynamic_system_view.go +++ b/vault/dynamic_system_view.go @@ -3,6 +3,7 @@ package vault import ( "context" "fmt" + "strings" "time" "github.com/hashicorp/errwrap" @@ -88,6 +89,9 @@ func (d dynamicSystemView) SudoPrivilege(ctx context.Context, path string, token return false } + // AllowOperation path should be namespace scoped + path = strings.TrimPrefix(path, tokenNS.Path) + // Add identity policies from all the namespaces entity, identityPolicies, err := d.core.fetchEntityAndDerivedPolicies(ctx, tokenNS, te.EntityID) if err != nil { diff --git a/vault/token_store.go b/vault/token_store.go index af3ac6828726..2a34d3289b60 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -2094,18 +2094,8 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque logical.ErrInvalidRequest } - // Get the context namespace - ns, err := namespace.FromContext(ctx) - if err != nil { - return nil, err - } - // Check if the client token has sudo/root privileges for the requested path - mountPath := req.MountPoint+req.Path - if ns != namespace.RootNamespace { - mountPath = strings.TrimPrefix(mountPath, ns.Path) - } - isSudo := ts.System().SudoPrivilege(ctx, mountPath, req.ClientToken) + isSudo := ts.System().SudoPrivilege(ctx, req.MountPoint+req.Path, req.ClientToken) // Read and parse the fields var data struct { @@ -2132,6 +2122,10 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // If the context's namespace is different from the parent and this is an // orphan token creation request, then this is an admin token generation for // the namespace + ns, err := namespace.FromContext(ctx) + if err != nil { + return nil, err + } if ns.ID != parent.NamespaceID { parentNS, err := NamespaceByID(ctx, parent.NamespaceID, ts.core) if err != nil { From 5c4270c7ad8a2fa1a418959e576419e43ad37c51 Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Thu, 1 Aug 2019 13:23:48 +0200 Subject: [PATCH 3/4] Changed to tokenCtx instead of request ctx --- vault/dynamic_system_view.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/dynamic_system_view.go b/vault/dynamic_system_view.go index 38e6629b6519..a147d8248544 100644 --- a/vault/dynamic_system_view.go +++ b/vault/dynamic_system_view.go @@ -118,7 +118,7 @@ func (d dynamicSystemView) SudoPrivilege(ctx context.Context, path string, token req := new(logical.Request) req.Operation = logical.ReadOperation req.Path = path - authResults := acl.AllowOperation(ctx, req, true) + authResults := acl.AllowOperation(tokenCtx, req, true) return authResults.RootPrivs } From d5731e0ec40966f8e810b3cae79b3f3428bb4870 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 5 Aug 2019 15:42:36 -0400 Subject: [PATCH 4/4] Use root context for AllowOperation; details in comment --- vault/dynamic_system_view.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/vault/dynamic_system_view.go b/vault/dynamic_system_view.go index a147d8248544..18c2d2518aea 100644 --- a/vault/dynamic_system_view.go +++ b/vault/dynamic_system_view.go @@ -3,7 +3,6 @@ package vault import ( "context" "fmt" - "strings" "time" "github.com/hashicorp/errwrap" @@ -89,9 +88,6 @@ func (d dynamicSystemView) SudoPrivilege(ctx context.Context, path string, token return false } - // AllowOperation path should be namespace scoped - path = strings.TrimPrefix(path, tokenNS.Path) - // Add identity policies from all the namespaces entity, identityPolicies, err := d.core.fetchEntityAndDerivedPolicies(ctx, tokenNS, te.EntityID) if err != nil { @@ -114,11 +110,13 @@ func (d dynamicSystemView) SudoPrivilege(ctx context.Context, path string, token // The operation type isn't important here as this is run from a path the // user has already been given access to; we only care about whether they - // have sudo + // have sudo. Note that we use root context because the path that comes in + // must be fully-qualified already so we don't want AllowOperation to + // prepend a namespace prefix onto it. req := new(logical.Request) req.Operation = logical.ReadOperation req.Path = path - authResults := acl.AllowOperation(tokenCtx, req, true) + authResults := acl.AllowOperation(namespace.RootContext(ctx), req, true) return authResults.RootPrivs }