diff --git a/changelog/12668.txt b/changelog/12668.txt new file mode 100644 index 000000000..7006da572 --- /dev/null +++ b/changelog/12668.txt @@ -0,0 +1,3 @@ +```release-note:improvement +sdk/framework: The '+' wildcard is now supported for parameterizing unauthenticated paths. +``` diff --git a/sdk/framework/backend.go b/sdk/framework/backend.go index c2c3f1810..d498dd4ee 100644 --- a/sdk/framework/backend.go +++ b/sdk/framework/backend.go @@ -41,10 +41,8 @@ type Backend struct { // paths, including adding or removing, is not allowed once the // backend is in use). // - // PathsSpecial is the list of path patterns that denote the - // paths above that require special privileges. These can't be - // regular expressions, it is either exact match or prefix match. - // For prefix match, append '*' as a suffix. + // PathsSpecial is the list of path patterns that denote the paths above + // that require special privileges. Paths []*Path PathsSpecial *logical.Paths diff --git a/sdk/logical/logical.go b/sdk/logical/logical.go index db8831535..cec2d19c0 100644 --- a/sdk/logical/logical.go +++ b/sdk/logical/logical.go @@ -117,6 +117,10 @@ type Paths struct { Root []string // Unauthenticated are the paths that can be accessed without any auth. + // These can't be regular expressions, it is either exact match, a prefix + // match and/or a wildcard match. For prefix match, append '*' as a suffix. + // For a wildcard match, use '+' in the segment to match any identifier + // (e.g. 'foo/+/bar'). Note that '+' can't be adjacent to a non-slash. Unauthenticated []string // LocalStorage are paths (prefixes) that are local to this instance; this diff --git a/vault/identity_store.go b/vault/identity_store.go index 98a5350d9..18e3a215a 100644 --- a/vault/identity_store.go +++ b/vault/identity_store.go @@ -89,6 +89,8 @@ func NewIdentityStore(ctx context.Context, core *Core, config *logical.BackendCo PathsSpecial: &logical.Paths{ Unauthenticated: []string{ "oidc/.well-known/*", + "oidc/provider/+/.well-known/*", + "oidc/provider/+/token", }, }, PeriodicFunc: func(ctx context.Context, req *logical.Request) error { diff --git a/vault/plugin_reload.go b/vault/plugin_reload.go index 732d60bfa..e38b5341a 100644 --- a/vault/plugin_reload.go +++ b/vault/plugin_reload.go @@ -188,7 +188,11 @@ func (c *Core) reloadBackendCommon(ctx context.Context, entry *MountEntry, isAut paths := backend.SpecialPaths() if paths != nil { re.rootPaths.Store(pathsToRadix(paths.Root)) - re.loginPaths.Store(pathsToRadix(paths.Unauthenticated)) + loginPathsEntry, err := parseUnauthenticatedPaths(paths.Unauthenticated) + if err != nil { + return err + } + re.loginPaths.Store(loginPathsEntry) } } diff --git a/vault/router.go b/vault/router.go index 6d61d7f0e..6426e4eb8 100644 --- a/vault/router.go +++ b/vault/router.go @@ -3,6 +3,7 @@ package vault import ( "context" "fmt" + "regexp" "strings" "sync" "sync/atomic" @@ -22,6 +23,9 @@ var deniedPassthroughRequestHeaders = []string{ consts.AuthHeaderName, } +// matches when '+' is next to a non-slash char +var wcAdjacentNonSlashRegEx = regexp.MustCompile(`\+[^/]|[^/]\+`).MatchString + // Router is used to do prefix based routing of a request to a logical backend type Router struct { l sync.RWMutex @@ -59,6 +63,19 @@ type routeEntry struct { l sync.RWMutex } +type wildcardPath struct { + // this sits in the hot path of requests so we are micro-optimizing by + // storing pre-split slices of path segments + segments []string + isPrefix bool +} + +// loginPathsEntry is used to hold the routeEntry loginPaths +type loginPathsEntry struct { + paths *radix.Tree + wildcardPaths []wildcardPath +} + type ValidateMountResponse struct { MountType string `json:"mount_type" structs:"mount_type" mapstructure:"mount_type"` MountAccessor string `json:"mount_accessor" structs:"mount_accessor" mapstructure:"mount_accessor"` @@ -137,7 +154,11 @@ func (r *Router) Mount(backend logical.Backend, prefix string, mountEntry *Mount storageView: storageView, } re.rootPaths.Store(pathsToRadix(paths.Root)) - re.loginPaths.Store(pathsToRadix(paths.Unauthenticated)) + loginPathsEntry, err := parseUnauthenticatedPaths(paths.Unauthenticated) + if err != nil { + return err + } + re.loginPaths.Store(loginPathsEntry) switch { case prefix == "": @@ -782,6 +803,10 @@ func (r *Router) RootPath(ctx context.Context, path string) bool { } // LoginPath checks if the given path is used for logins +// Matching Priority +// 1. prefix +// 2. exact +// 3. wildcard func (r *Router) LoginPath(ctx context.Context, path string) bool { ns, err := namespace.FromContext(ctx) if err != nil { @@ -802,20 +827,114 @@ func (r *Router) LoginPath(ctx context.Context, path string) bool { remain := strings.TrimPrefix(adjustedPath, mount) // Check the loginPaths of this backend - loginPaths := re.loginPaths.Load().(*radix.Tree) - match, raw, ok := loginPaths.LongestPrefix(remain) - if !ok { + pe := re.loginPaths.Load().(*loginPathsEntry) + match, raw, ok := pe.paths.LongestPrefix(remain) + if !ok && len(pe.wildcardPaths) == 0 { + // no match found return false } - prefixMatch := raw.(bool) - // Handle the prefix match case - if prefixMatch { - return strings.HasPrefix(remain, match) + if ok { + prefixMatch := raw.(bool) + if prefixMatch { + // Handle the prefix match case + return strings.HasPrefix(remain, match) + } + if match == remain { + // Handle the exact match case + return true + } } - // Handle the exact match case - return match == remain + // check Login Paths containing wildcards + reqPathParts := strings.Split(remain, "/") + for _, w := range pe.wildcardPaths { + if pathMatchesWildcardPath(reqPathParts, w.segments, w.isPrefix) { + return true + } + } + return false +} + +// pathMatchesWildcardPath returns true if the path made up of the path slice +// matches the given wildcard path slice +func pathMatchesWildcardPath(path, wcPath []string, isPrefix bool) bool { + if len(wcPath) == 0 { + return false + } + + if len(path) < len(wcPath) { + // check if the path coming in is shorter; if so it can't match + return false + } + if !isPrefix && len(wcPath) != len(path) { + // If it's not a prefix we expect the same number of segments + return false + } + + for i, wcPathPart := range wcPath { + switch { + case wcPathPart == "+": + case wcPathPart == path[i]: + case isPrefix && i == len(wcPath)-1 && strings.HasPrefix(path[i], wcPathPart): + default: + // we encountered segments that did not match + return false + } + } + return true +} + +func wildcardError(path, msg string) error { + return fmt.Errorf("path %q: invalid use of wildcards %s", path, msg) +} + +func isValidUnauthenticatedPath(path string) (bool, error) { + switch { + case strings.Count(path, "*") > 1: + return false, wildcardError(path, "(multiple '*' is forbidden)") + case strings.Contains(path, "+*"): + return false, wildcardError(path, "('+*' is forbidden)") + case strings.Contains(path, "*") && path[len(path)-1] != '*': + return false, wildcardError(path, "('*' is only allowed at the end of a path)") + case wcAdjacentNonSlashRegEx(path): + return false, wildcardError(path, "('+' is not allowed next to a non-slash)") + } + return true, nil +} + +// parseUnauthenticatedPaths converts a list of special paths to a +// loginPathsEntry +func parseUnauthenticatedPaths(paths []string) (*loginPathsEntry, error) { + var tempPaths []string + tempWildcardPaths := make([]wildcardPath, 0) + for _, path := range paths { + if ok, err := isValidUnauthenticatedPath(path); !ok { + return nil, err + } + + if strings.Contains(path, "+") { + // Paths with wildcards are not stored in the radix tree because + // the radix tree does not handle wildcards in the middle of strings. + isPrefix := false + if path[len(path)-1] == '*' { + isPrefix = true + path = path[0 : len(path)-1] + } + // We are micro-optimizing by storing pre-split slices of path segments + wcPath := wildcardPath{segments: strings.Split(path, "/"), isPrefix: isPrefix} + tempWildcardPaths = append(tempWildcardPaths, wcPath) + } else { + // accumulate paths that do not contain wildcards + // to be stored in the radix tree + tempPaths = append(tempPaths, path) + } + } + + return &loginPathsEntry{ + paths: pathsToRadix(tempPaths), + wildcardPaths: tempWildcardPaths, + }, nil } // pathsToRadix converts a list of special paths to a radix tree. diff --git a/vault/router_test.go b/vault/router_test.go index b20b69894..842d75f62 100644 --- a/vault/router_test.go +++ b/vault/router_test.go @@ -348,6 +348,15 @@ func TestRouter_LoginPath(t *testing.T) { Login: []string{ "login", "oauth/*", + "glob1*", + "+/wildcard/glob2*", + "end1/+", + "end2/+/", + "end3/+/*", + "middle1/+/bar", + "middle2/+/+/bar", + "+/begin", + "+/around/+/", }, } err = r.Mount(n, "auth/foo/", &MountEntry{UUID: meUUID, Accessor: "authfooaccessor", NamespaceID: namespace.RootNamespaceID, namespace: namespace.RootNamespace}, view) @@ -363,8 +372,70 @@ func TestRouter_LoginPath(t *testing.T) { {"random", false}, {"auth/foo/bar", false}, {"auth/foo/login", true}, + {"auth/foo/login/", false}, {"auth/foo/oauth", false}, + {"auth/foo/oauth/", true}, {"auth/foo/oauth/redirect", true}, + {"auth/foo/oauth/redirect/", true}, + {"auth/foo/oauth/redirect/bar", true}, + {"auth/foo/glob1", true}, + {"auth/foo/glob1/", true}, + {"auth/foo/glob1/redirect", true}, + + // Wildcard cases + + // "+/wildcard/glob2*" + {"auth/foo/bar/wildcard/glo", false}, + {"auth/foo/bar/wildcard/glob2", true}, + {"auth/foo/bar/wildcard/glob2222", true}, + {"auth/foo/bar/wildcard/glob2/", true}, + {"auth/foo/bar/wildcard/glob2/baz", true}, + + // "end1/+" + {"auth/foo/end1", false}, + {"auth/foo/end1/", true}, + {"auth/foo/end1/bar", true}, + {"auth/foo/end1/bar/", false}, + {"auth/foo/end1/bar/baz", false}, + // "end2/+/" + {"auth/foo/end2", false}, + {"auth/foo/end2/", false}, + {"auth/foo/end2/bar", false}, + {"auth/foo/end2/bar/", true}, + {"auth/foo/end2/bar/baz", false}, + // "end3/+/*" + {"auth/foo/end3", false}, + {"auth/foo/end3/", false}, + {"auth/foo/end3/bar", false}, + {"auth/foo/end3/bar/", true}, + {"auth/foo/end3/bar/baz", true}, + {"auth/foo/end3/bar/baz/", true}, + {"auth/foo/end3/bar/baz/qux", true}, + {"auth/foo/end3/bar/baz/qux/qoo", true}, + {"auth/foo/end3/bar/baz/qux/qoo/qaa", true}, + // "middle1/+/bar", + {"auth/foo/middle1/bar", false}, + {"auth/foo/middle1/bar/", false}, + {"auth/foo/middle1/bar/qux", false}, + {"auth/foo/middle1/bar/bar", true}, + {"auth/foo/middle1/bar/bar/", false}, + // "middle2/+/+/bar", + {"auth/foo/middle2/bar", false}, + {"auth/foo/middle2/bar/", false}, + {"auth/foo/middle2/bar/baz", false}, + {"auth/foo/middle2/bar/baz/", false}, + {"auth/foo/middle2/bar/baz/bar", true}, + {"auth/foo/middle2/bar/baz/bar/", false}, + // "+/begin" + {"auth/foo/bar/begin", true}, + {"auth/foo/bar/begin/", false}, + {"auth/foo/begin", false}, + // "+/around/+/" + {"auth/foo/bar/around", false}, + {"auth/foo/bar/around/", false}, + {"auth/foo/bar/around/baz", false}, + {"auth/foo/bar/around/baz/", true}, + {"auth/foo/bar/around/baz/qux", false}, } for _, tc := range tcases { @@ -477,3 +548,82 @@ func TestPathsToRadix(t *testing.T) { t.Fatalf("bad: %v (sub/bar)", raw) } } + +func TestParseUnauthenticatedPaths(t *testing.T) { + // inputs + paths := []string{ + "foo", + "foo/*", + "sub/bar*", + } + wildcardPaths := []string{ + "end/+", + "+/begin/*", + "middle/+/bar*", + } + allPaths := append(paths, wildcardPaths...) + + p, err := parseUnauthenticatedPaths(allPaths) + if err != nil { + t.Fatal(err) + } + + // outputs + wildcardPathsEntry := []wildcardPath{ + {segments: []string{"end", "+"}, isPrefix: false}, + {segments: []string{"+", "begin", ""}, isPrefix: true}, + {segments: []string{"middle", "+", "bar"}, isPrefix: true}, + } + expected := &loginPathsEntry{ + paths: pathsToRadix(paths), + wildcardPaths: wildcardPathsEntry, + } + + if !reflect.DeepEqual(expected, p) { + t.Fatalf("expected: %#v\n actual: %#v\n", expected, p) + } +} + +func TestParseUnauthenticatedPaths_Error(t *testing.T) { + type tcase struct { + paths []string + err string + } + tcases := []tcase{ + { + []string{"/foo/+*"}, + "path \"/foo/+*\": invalid use of wildcards ('+*' is forbidden)", + }, + { + []string{"/foo/*/*"}, + "path \"/foo/*/*\": invalid use of wildcards (multiple '*' is forbidden)", + }, + { + []string{"*/foo/*"}, + "path \"*/foo/*\": invalid use of wildcards (multiple '*' is forbidden)", + }, + { + []string{"*/foo/"}, + "path \"*/foo/\": invalid use of wildcards ('*' is only allowed at the end of a path)", + }, + { + []string{"/foo+"}, + "path \"/foo+\": invalid use of wildcards ('+' is not allowed next to a non-slash)", + }, + { + []string{"/+foo"}, + "path \"/+foo\": invalid use of wildcards ('+' is not allowed next to a non-slash)", + }, + { + []string{"/++"}, + "path \"/++\": invalid use of wildcards ('+' is not allowed next to a non-slash)", + }, + } + + for _, tc := range tcases { + _, err := parseUnauthenticatedPaths(tc.paths) + if err == nil || err != nil && !strings.Contains(err.Error(), tc.err) { + t.Fatalf("bad: path: %s expect: %v got %v", tc.paths, tc.err, err) + } + } +}