Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow globbing dis/allowed_policies in token roles (with caveat) #5815

Closed
wants to merge 1 commit into from
Closed

Allow globbing dis/allowed_policies in token roles (with caveat) #5815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions helper/strutil/strutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ func StrListContains(haystack []string, needle string) bool {
return false
}

// StrListSubsetGlob checks if a given list is a subset of
// another set, allowing for globs.
func StrListSubsetGlob(super, sub []string) bool {
for _, item := range sub {
if !StrListContainsGlob(super, item) {
return false
}
}
return true
}

// StrListSubset checks if a given list is a subset
// of another set
func StrListSubset(super, sub []string) bool {
Expand Down Expand Up @@ -239,6 +250,18 @@ func RemoveDuplicates(items []string, lowercase bool) []string {
return items
}

// RemoveGlobs removes any elements containing globs from a slice of strings.
func RemoveGlobs(items []string) []string {
ret := make([]string, 0, len(items))
for _, item := range items {
// glob.GLOB is "*"; ignore items containing that
if !strings.Contains(item, glob.GLOB) {
ret = append(ret, item)
}
}
return ret
}

// EquivalentSlices checks whether the given string sets are equivalent, as in,
// they contain the same values.
func EquivalentSlices(a, b []string) bool {
Expand Down
59 changes: 59 additions & 0 deletions helper/strutil/strutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,43 @@ func TestStrutil_ListContains(t *testing.T) {
}
}

func TestStrutil_ListSubsetGlob(t *testing.T) {
parent := []string{
"dev",
"ops*",
"root/*",
"*-dev",
"_*_",
}
if StrListSubsetGlob(parent, []string{"tubez", "dev", "root/admin"}) {
t.Fatalf("Bad")
}
if StrListSubsetGlob(parent, []string{"devops", "ops-dev"}) {
t.Fatalf("Bad")
}
if StrListSubsetGlob(nil, parent) {
t.Fatalf("Bad")
}
if !StrListSubsetGlob(parent, []string{"root/test", "dev", "_test_"}) {
t.Fatalf("Bad")
}
if !StrListSubsetGlob(parent, []string{"ops_test", "ops", "devops-dev"}) {
t.Fatalf("Bad")
}
if !StrListSubsetGlob(parent, []string{"ops"}) {
t.Fatalf("Bad")
}
if !StrListSubsetGlob(parent, []string{"test-dev"}) {
t.Fatalf("Bad")
}
if !StrListSubsetGlob(parent, []string{"_test_"}) {
t.Fatalf("Bad")
}
if !StrListSubsetGlob(parent, nil) {
t.Fatalf("Bad")
}
}

func TestStrutil_ListSubset(t *testing.T) {
parent := []string{
"dev",
Expand Down Expand Up @@ -424,6 +461,28 @@ func TestStrUtil_RemoveDuplicates(t *testing.T) {
}
}

func TestStrUtil_RemoveGlobs(t *testing.T) {
type tCase struct {
input []string
expect []string
}

tCases := []tCase{
tCase{[]string{}, []string{}},
tCase{[]string{"hello"}, []string{"hello"}},
tCase{[]string{"h*i"}, []string{}},
tCase{[]string{"one", "two*", "*three", "f*our", "five"}, []string{"one", "five"}},
}

for _, tc := range tCases {
actual := RemoveGlobs(tc.input)

if !reflect.DeepEqual(actual, tc.expect) {
t.Fatalf("Bad testcase %#v, expected %v, got %v", tc, tc.expect, actual)
}
}
}

func TestStrUtil_ParseStringSlice(t *testing.T) {
type tCase struct {
input string
Expand Down
12 changes: 8 additions & 4 deletions vault/token_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2311,7 +2311,7 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque
if len(finalPolicies) == 0 {
finalPolicies = sanitizedRolePolicies
} else {
if !strutil.StrListSubset(sanitizedRolePolicies, finalPolicies) {
if !strutil.StrListSubsetGlob(sanitizedRolePolicies, finalPolicies) {
return logical.ErrorResponse(fmt.Sprintf("token policies (%q) must be subset of the role's allowed policies (%q)", finalPolicies, sanitizedRolePolicies)), logical.ErrInvalidRequest
}
}
Expand All @@ -2328,7 +2328,7 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque
sanitizedRolePolicies = strutil.RemoveDuplicates(role.DisallowedPolicies, true)

for _, finalPolicy := range finalPolicies {
if strutil.StrListContains(sanitizedRolePolicies, finalPolicy) {
if strutil.StrListContainsGlob(sanitizedRolePolicies, finalPolicy) {
return logical.ErrorResponse(fmt.Sprintf("token policy %q is disallowed by this role", finalPolicy)), logical.ErrInvalidRequest
}
}
Expand Down Expand Up @@ -2386,6 +2386,9 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque
}
}

// Remove any glob strings from policy list
te.Policies = strutil.RemoveGlobs(te.Policies)

if strutil.StrListContains(te.Policies, "root") {
// Prevent attempts to create a root token without an actual root token as parent.
// This is to thwart privilege escalation by tokens having 'sudo' privileges.
Expand Down Expand Up @@ -3218,9 +3221,10 @@ as revocation of tokens. The tokens are renewable if associated with a lease.`
tokenAllowedPoliciesHelp = `If set, tokens can be created with any subset of the policies in this
list, rather than the normal semantics of tokens being a subset of the
calling token's policies. The parameter is a comma-delimited string of
policy names.`
policy names or globs.`
tokenDisallowedPoliciesHelp = `If set, successful token creation via this role will require that
no policies in the given list are requested. The parameter is a comma-delimited string of policy names.`
no policies in the given list are requested. The parameter is a comma-delimited string of policy names
or globs.`
tokenOrphanHelp = `If true, tokens created via this role
will be orphan tokens (have no parent)`
tokenPeriodHelp = `If set, tokens created via this role
Expand Down
79 changes: 77 additions & 2 deletions vault/token_store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2728,7 +2728,7 @@ func TestTokenStore_RoleDisallowedPolicies(t *testing.T) {
ts := core.tokenStore
ps := core.policyStore

// Create 3 different policies
// Create 4 different policies
policy, _ := ParseACLPolicy(namespace.RootNamespace, tokenCreationPolicy)
policy.Name = "test1"
if err := ps.SetPolicy(namespace.RootContext(nil), policy); err != nil {
Expand All @@ -2747,6 +2747,12 @@ func TestTokenStore_RoleDisallowedPolicies(t *testing.T) {
t.Fatal(err)
}

policy, _ = ParseACLPolicy(namespace.RootNamespace, tokenCreationPolicy)
policy.Name = "test3b"
if err := ps.SetPolicy(namespace.RootContext(nil), policy); err != nil {
t.Fatal(err)
}

// Create roles with different disallowed_policies configuration
req = logical.TestRequest(t, logical.UpdateOperation, "roles/test1")
req.ClientToken = root
Expand Down Expand Up @@ -2778,10 +2784,20 @@ func TestTokenStore_RoleDisallowedPolicies(t *testing.T) {
t.Fatalf("err:%v resp:%v", err, resp)
}

req = logical.TestRequest(t, logical.UpdateOperation, "roles/testnot23")
req.ClientToken = root
req.Data = map[string]interface{}{
"disallowed_policies": "test2,test3*",
}
resp, err = ts.HandleRequest(namespace.RootContext(nil), req)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%v", err, resp)
}

// Create a token that has all the policies defined above
req = logical.TestRequest(t, logical.UpdateOperation, "create")
req.ClientToken = root
req.Data["policies"] = []string{"test1", "test2", "test3"}
req.Data["policies"] = []string{"test1", "test2", "test3", "test3b"}
resp = testMakeTokenViaRequest(t, ts, req)
if resp == nil || resp.Auth == nil {
t.Fatal("got nil response")
Expand Down Expand Up @@ -2812,13 +2828,44 @@ func TestTokenStore_RoleDisallowedPolicies(t *testing.T) {
t.Fatalf("expected an error response, got %#v", resp)
}

req = logical.TestRequest(t, logical.UpdateOperation, "create/testnot23")
req.ClientToken = parentToken
resp, err = ts.HandleRequest(namespace.RootContext(nil), req)
if err == nil || resp != nil && !resp.IsError() {
t.Fatalf("expected an error response, got %#v", resp)
}

// Disallowed should act as a blacklist so make sure we can still make
// something with other policies in the request
req = logical.TestRequest(t, logical.UpdateOperation, "create/test123")
req.Data["policies"] = []string{"foo", "bar"}
req.ClientToken = parentToken
testMakeTokenViaRequest(t, ts, req)

// Check to be sure 'test3*' matches 'test3'
req = logical.TestRequest(t, logical.UpdateOperation, "create/testnot23")
req.Data["policies"] = []string{"test3"}
req.ClientToken = parentToken
resp, err = ts.HandleRequest(namespace.RootContext(nil), req)
if err == nil || resp != nil && !resp.IsError() {
t.Fatalf("expected an error response, got %#v", resp)
}

// Check to be sure 'test3*' matches 'test3b'
req = logical.TestRequest(t, logical.UpdateOperation, "create/testnot23")
req.Data["policies"] = []string{"test3b"}
req.ClientToken = parentToken
resp, err = ts.HandleRequest(namespace.RootContext(nil), req)
if err == nil || resp != nil && !resp.IsError() {
t.Fatalf("expected an error response, got %#v", resp)
}

// Check that non-blacklisted policies still work
req = logical.TestRequest(t, logical.UpdateOperation, "create/testnot23")
req.Data["policies"] = []string{"test1"}
req.ClientToken = parentToken
testMakeTokenViaRequest(t, ts, req)

// Create a role to have 'default' policy disallowed
req = logical.TestRequest(t, logical.UpdateOperation, "roles/default")
req.ClientToken = root
Expand Down Expand Up @@ -2871,6 +2918,34 @@ func TestTokenStore_RoleAllowedPolicies(t *testing.T) {
t.Fatalf("bad: %#v", resp)
}

// allowed_policies should also support globs
req = logical.TestRequest(t, logical.UpdateOperation, "roles/test")
req.ClientToken = root
req.Data = map[string]interface{}{
"allowed_policies": "test*",
}

resp, err = ts.HandleRequest(namespace.RootContext(nil), req)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err: %v\nresp: %#v", err, resp)
}
if resp != nil {
t.Fatalf("expected a nil response")
}

req.Path = "create/test"
req.Data["policies"] = []string{"footest"}
resp, err = ts.HandleRequest(namespace.RootContext(nil), req)
if err == nil {
t.Fatalf("expected error")
}

req.Data["policies"] = []string{"testfoo", "test2"}
resp = testMakeTokenViaRequest(t, ts, req)
if resp.Auth.ClientToken == "" {
t.Fatalf("bad: %#v", resp)
}

// When allowed_policies is blank, should fall back to a subset of the parent policies
req = logical.TestRequest(t, logical.UpdateOperation, "roles/test")
req.ClientToken = root
Expand Down