Add ability to use path wildcard segments (#6164)

* Path globbing

* Add glob support at the beginning

* Ensure when evaluating an ACL that our path never has a leading slash. This already happens in the normal request path but not in tests; putting it here provides it for tests and extra safety in case the request path changes

* Simplify the algorithm, we don't really need to validate the prefix first as glob won't apply if it doesn't

* Add path segment wildcarding

* Disable path globbing for now

* Remove now-unneeded test

* Remove commented out globbing bits

* Remove more holdover glob bits

* Rename k var to something more clear

vault/acl.go

@@ -25,6 +25,8 @@ type ACL struct {
 	// prefixRules contains the path policies that are a prefix
 	prefixRules *radix.Tree
 
+	segmentWildcardPaths map[string]interface{}
+
 	// root is enabled if the "root" named policy is present.
 	root bool
 
@@ -58,9 +60,10 @@ type ACLResults struct {
 func NewACL(ctx context.Context, policies []*Policy) (*ACL, error) {
 	// Initialize
 	a := &ACL{
-		exactRules:  radix.New(),
-		prefixRules: radix.New(),
-		root:        false,
+		exactRules:           radix.New(),
+		prefixRules:          radix.New(),
+		segmentWildcardPaths: make(map[string]interface{}, len(policies)),
+		root:                 false,
 	}
 
 	ns, err := namespace.FromContext(ctx)
@@ -100,20 +103,35 @@ func NewACL(ctx context.Context, policies []*Policy) (*ACL, error) {
 		}
 
 		for _, pc := range policy.Paths {
-			// Check which tree to use
-			tree := a.exactRules
-			if pc.IsPrefix {
-				tree = a.prefixRules
+			var raw interface{}
+			var ok bool
+			var tree *radix.Tree
+
+			switch {
+			case pc.HasSegmentWildcards:
+				raw, ok = a.segmentWildcardPaths[pc.Path]
+			default:
+				// Check which tree to use
+				tree = a.exactRules
+				if pc.IsPrefix {
+					tree = a.prefixRules
+				}
+
+				// Check for an existing policy
+				raw, ok = tree.Get(pc.Path)
 			}
 
-			// Check for an existing policy
-			raw, ok := tree.Get(pc.Path)
 			if !ok {
 				clonedPerms, err := pc.Permissions.Clone()
 				if err != nil {
 					return nil, errwrap.Wrapf("error cloning ACL permissions: {{err}}", err)
 				}
-				tree.Insert(pc.Path, clonedPerms)
+				switch {
+				case pc.HasSegmentWildcards:
+					a.segmentWildcardPaths[pc.Path] = clonedPerms
+				default:
+					tree.Insert(pc.Path, clonedPerms)
+				}
 				continue
 			}
 
@@ -242,7 +260,12 @@ func NewACL(ctx context.Context, policies []*Policy) (*ACL, error) {
 			}
 
 		INSERT:
-			tree.Insert(pc.Path, existingPerms)
+			switch {
+			case pc.HasSegmentWildcards:
+				a.segmentWildcardPaths[pc.Path] = existingPerms
+			default:
+				tree.Insert(pc.Path, existingPerms)
+			}
 		}
 	}
 	return a, nil
@@ -317,7 +340,17 @@ func (a *ACL) AllowOperation(ctx context.Context, req *logical.Request, capCheck
 	}
 	path := ns.Path + req.Path
 
-	// Find an exact matching rule, look for glob if no match
+	// The request path should take care of this already but this is useful for
+	// tests and as defense in depth
+	for {
+		if len(path) > 0 && path[0] == '/' {
+			path = path[1:]
+		} else {
+			break
+		}
+	}
+
+	// Find an exact matching rule, look for prefix if no match
 	var capabilities uint32
 	raw, ok := a.exactRules.Get(path)
 	if ok {
@@ -334,13 +367,81 @@ func (a *ACL) AllowOperation(ctx context.Context, req *logical.Request, capCheck
 		}
 	}
 
-	// Find a glob rule, default deny if no match
+	// Find a prefix rule, default deny if no match
 	_, raw, ok = a.prefixRules.LongestPrefix(path)
-	if !ok {
-		return
+	if ok {
+		permissions = raw.(*ACLPermissions)
+		capabilities = permissions.CapabilitiesBitmap
+		goto CHECK
 	}
-	permissions = raw.(*ACLPermissions)
-	capabilities = permissions.CapabilitiesBitmap
+
+	if len(a.segmentWildcardPaths) > 0 {
+		pathParts := strings.Split(path, "/")
+		for currWCPath := range a.segmentWildcardPaths {
+			if currWCPath == "" {
+				continue
+			}
+
+			var isPrefix bool
+			var invalid bool
+			origCurrWCPath := currWCPath
+
+			if currWCPath[len(currWCPath)-1] == '*' {
+				isPrefix = true
+				currWCPath = currWCPath[0 : len(currWCPath)-1]
+			}
+			splitCurrWCPath := strings.Split(currWCPath, "/")
+			if len(pathParts) < len(splitCurrWCPath) {
+				// The path coming in is shorter; it can't match
+				continue
+			}
+			if !isPrefix && len(splitCurrWCPath) != len(pathParts) {
+				// If it's not a prefix we expect the same number of segments
+				continue
+			}
+			// We key off splitK here since it might be less than pathParts
+			for i, aclPart := range splitCurrWCPath {
+				if aclPart == "+" {
+					// Matches anything in the segment, so keep checking
+					continue
+				}
+				if i == len(splitCurrWCPath)-1 && isPrefix {
+					// In this case we may have foo* or just * depending on if
+					// originally it was foo* or foo/*.
+					if aclPart == "" {
+						// Ended in /*, so at this point we're at the final
+						// glob which will match anything, so return success
+						break
+					}
+					if !strings.HasPrefix(pathParts[i], aclPart) {
+						// E.g., the final part of the acl is foo* and the
+						// final part of the path is boofar
+						invalid = true
+						break
+					}
+					// Final prefixed matched and the rest is a wildcard,
+					// matches
+					break
+				}
+				if aclPart != pathParts[i] {
+					// Mismatch, exit out
+					invalid = true
+					break
+				}
+			}
+			// If invalid isn't set then we got through the full segmented path
+			// without finding a mismatch, so it's valid
+			if !invalid {
+				permissions = a.segmentWildcardPaths[origCurrWCPath].(*ACLPermissions)
+				capabilities = permissions.CapabilitiesBitmap
+				goto CHECK
+			}
+		}
+	}
+
+	// No exact, prefix, or segment wildcard paths found, return without
+	// setting allowed
+	return
 
 CHECK:
 	// Check if the minimum permissions are met

vault/acl_test.go

@@ -237,6 +237,19 @@ func testACLSingle(t *testing.T, ns *namespace.Namespace) {
 		{logical.ListOperation, "foo/bar", false, true},
 		{logical.UpdateOperation, "foo/bar", false, true},
 		{logical.CreateOperation, "foo/bar", true, true},
+
+		// Path segment wildcards
+		{logical.ReadOperation, "test/foo/bar/segment", false, false},
+		{logical.ReadOperation, "test/foo/segment", true, false},
+		{logical.ReadOperation, "test/bar/segment", true, false},
+		{logical.ReadOperation, "test/segment/at/frond", false, false},
+		{logical.ReadOperation, "test/segment/at/front", true, false},
+		{logical.ReadOperation, "test/segment/at/end/foo", true, false},
+		{logical.ReadOperation, "test/segment/at/end/foo/", false, false},
+		{logical.ReadOperation, "test/segment/at/end/v2/foo/", true, false},
+		{logical.ReadOperation, "test/segment/wildcard/at/foo/", true, false},
+		{logical.ReadOperation, "test/segment/wildcard/at/end", true, false},
+		{logical.ReadOperation, "test/segment/wildcard/at/end/", true, false},
 	}
 
 	for _, tc := range tcases {
@@ -643,6 +656,24 @@ path "sys/*" {
 path "foo/bar" {
 	capabilities = ["read", "create", "sudo"]
 }
+path "test/+/segment" {
+	capabilities = ["read"]
+}
+path "+/segment/at/front" {
+	capabilities = ["read"]
+}
+path "test/segment/at/end/+" {
+	capabilities = ["read"]
+}
+path "test/segment/at/end/v2/+/" {
+	capabilities = ["read"]
+}
+path "test/+/wildcard/+/*" {
+	capabilities = ["read"]
+}
+path "test/+/wildcardglob/+/end*" {
+	capabilities = ["read"]
+}
 `
 
 var aclPolicy2 = `

vault/policy.go

@@ -112,11 +112,12 @@ func (p *Policy) ShallowClone() *Policy {
 
 // PathRules represents a policy for a path in the namespace.
 type PathRules struct {
-	Path         string
-	Policy       string
-	Permissions  *ACLPermissions
-	IsPrefix     bool
-	Capabilities []string
+	Path                string
+	Policy              string
+	Permissions         *ACLPermissions
+	IsPrefix            bool
+	HasSegmentWildcards bool
+	Capabilities        []string
 
 	// These keys are used at the top level to make the HCL nicer; we store in
 	// the ACLPermissions object though
@@ -338,10 +339,18 @@ func parsePaths(result *Policy, list *ast.ObjectList, performTemplating bool, en
 		// Ensure we are using the full request path internally
 		pc.Path = result.namespace.Path + pc.Path
 
-		// Strip the glob character if found
+		if strings.Count(pc.Path, "/+") > 0 || strings.HasPrefix(pc.Path, "+/") {
+			pc.HasSegmentWildcards = true
+		}
+
 		if strings.HasSuffix(pc.Path, "*") {
-			pc.Path = strings.TrimSuffix(pc.Path, "*")
-			pc.IsPrefix = true
+			// If there are segment wildcards, don't actually strip the
+			// trailing asterisk, but don't want to hit the default case
+			if !pc.HasSegmentWildcards {
+				// Strip the glob character if found
+				pc.Path = strings.TrimSuffix(pc.Path, "*")
+				pc.IsPrefix = true
+			}
 		}
 
 		// Map old-style policies into capabilities

vault/policy_test.go

@@ -1,11 +1,11 @@
 package vault
 
 import (
-	"reflect"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/go-test/deep"
 	"github.com/hashicorp/vault/helper/namespace"
 )
 
@@ -95,6 +95,21 @@ path "test/mfa" {
 	capabilities = ["create", "sudo"]
 	mfa_methods = ["my_totp", "my_totp2"]
 }
+path "test/+/segment" {
+	capabilities = ["create", "sudo"]
+}
+path "test/segment/at/end/+" {
+	capabilities = ["create", "sudo"]
+}
+path "test/segment/at/end/v2/+/" {
+	capabilities = ["create", "sudo"]
+}
+path "test/+/wildcard/+/*" {
+	capabilities = ["create", "sudo"]
+}
+path "test/+/wildcard/+/end*" {
+	capabilities = ["create", "sudo"]
+}
 `)
 
 func TestPolicy_Parse(t *testing.T) {
@@ -141,7 +156,6 @@ func TestPolicy_Parse(t *testing.T) {
 				"list",
 			},
 			Permissions: &ACLPermissions{CapabilitiesBitmap: (ReadCapabilityInt | ListCapabilityInt)},
-			IsPrefix:    false,
 		},
 		{
 			Path:   "foo/bar",
@@ -157,11 +171,9 @@ func TestPolicy_Parse(t *testing.T) {
 				MinWrappingTTL:     300 * time.Second,
 				MaxWrappingTTL:     3600 * time.Second,
 			},
-			IsPrefix: false,
 		},
 		{
-			Path:   "foo/bar",
-			Policy: "",
+			Path: "foo/bar",
 			Capabilities: []string{
 				"create",
 				"sudo",
@@ -173,11 +185,9 @@ func TestPolicy_Parse(t *testing.T) {
 				MinWrappingTTL:     300 * time.Second,
 				MaxWrappingTTL:     3600 * time.Second,
 			},
-			IsPrefix: false,
 		},
 		{
-			Path:   "foo/bar",
-			Policy: "",
+			Path: "foo/bar",
 			Capabilities: []string{
 				"create",
 				"sudo",
@@ -187,11 +197,9 @@ func TestPolicy_Parse(t *testing.T) {
 				CapabilitiesBitmap: (CreateCapabilityInt | SudoCapabilityInt),
 				AllowedParameters:  map[string][]interface{}{"zip": {}, "zap": {}},
 			},
-			IsPrefix: false,
 		},
 		{
-			Path:   "baz/bar",
-			Policy: "",
+			Path: "baz/bar",
 			Capabilities: []string{
 				"create",
 				"sudo",
@@ -201,11 +209,9 @@ func TestPolicy_Parse(t *testing.T) {
 				CapabilitiesBitmap: (CreateCapabilityInt | SudoCapabilityInt),
 				DeniedParameters:   map[string][]interface{}{"zip": []interface{}{}, "zap": []interface{}{}},
 			},
-			IsPrefix: false,
 		},
 		{
-			Path:   "biz/bar",
-			Policy: "",
+			Path: "biz/bar",
 			Capabilities: []string{
 				"create",
 				"sudo",
@@ -217,7 +223,6 @@ func TestPolicy_Parse(t *testing.T) {
 				AllowedParameters:  map[string][]interface{}{"zim": {}, "zam": {}},
 				DeniedParameters:   map[string][]interface{}{"zip": {}, "zap": {}},
 			},
-			IsPrefix: false,
 		},
 		{
 			Path:   "test/types",
@@ -236,8 +241,7 @@ func TestPolicy_Parse(t *testing.T) {
 			IsPrefix: false,
 		},
 		{
-			Path:   "test/req",
-			Policy: "",
+			Path: "test/req",
 			Capabilities: []string{
 				"create",
 				"sudo",
@@ -247,11 +251,9 @@ func TestPolicy_Parse(t *testing.T) {
 				CapabilitiesBitmap: (CreateCapabilityInt | SudoCapabilityInt),
 				RequiredParameters: []string{"foo"},
 			},
-			IsPrefix: false,
 		},
 		{
-			Path:   "test/mfa",
-			Policy: "",
+			Path: "test/mfa",
 			Capabilities: []string{
 				"create",
 				"sudo",
@@ -267,12 +269,66 @@ func TestPolicy_Parse(t *testing.T) {
 					"my_totp2",
 				},
 			},
-			IsPrefix: false,
+		},
+		{
+			Path: "test/+/segment",
+			Capabilities: []string{
+				"create",
+				"sudo",
+			},
+			Permissions: &ACLPermissions{
+				CapabilitiesBitmap: (CreateCapabilityInt | SudoCapabilityInt),
+			},
+			HasSegmentWildcards: true,
+		},
+		{
+			Path: "test/segment/at/end/+",
+			Capabilities: []string{
+				"create",
+				"sudo",
+			},
+			Permissions: &ACLPermissions{
+				CapabilitiesBitmap: (CreateCapabilityInt | SudoCapabilityInt),
+			},
+			HasSegmentWildcards: true,
+		},
+		{
+			Path: "test/segment/at/end/v2/+/",
+			Capabilities: []string{
+				"create",
+				"sudo",
+			},
+			Permissions: &ACLPermissions{
+				CapabilitiesBitmap: (CreateCapabilityInt | SudoCapabilityInt),
+			},
+			HasSegmentWildcards: true,
+		},
+		{
+			Path: "test/+/wildcard/+/*",
+			Capabilities: []string{
+				"create",
+				"sudo",
+			},
+			Permissions: &ACLPermissions{
+				CapabilitiesBitmap: (CreateCapabilityInt | SudoCapabilityInt),
+			},
+			HasSegmentWildcards: true,
+		},
+		{
+			Path: "test/+/wildcard/+/end*",
+			Capabilities: []string{
+				"create",
+				"sudo",
+			},
+			Permissions: &ACLPermissions{
+				CapabilitiesBitmap: (CreateCapabilityInt | SudoCapabilityInt),
+			},
+			HasSegmentWildcards: true,
 		},
 	}
 
-	if !reflect.DeepEqual(p.Paths, expect) {
-		t.Errorf("expected \n\n%#v\n\n to be \n\n%#v\n\n", p.Paths, expect)
+	if diff := deep.Equal(p.Paths, expect); diff != nil {
+		t.Error(diff)
 	}
 }