luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Add support to parameterize unauthenticated paths (#12668) 

* store unauthenticated path wildcards in map

* working unauthenticated paths with basic unit tests

* refactor wildcard logic

* add parseUnauthenticatedPaths unit tests

* use parseUnauthenticatedPaths when reloading backend

* add more wildcard test cases

* update special paths doc; add changelog

* remove buggy prefix check; add test cases

* prevent false positives for prefix matches

If we ever encounter a mismatched segment, break and set a flag to
prevent false positives for prefix matches.

If it is a match we need to do a prefix check. But we should not return
unless HasPrefix also evaluates to true. Otherwise we should let the for
loop continue to check other possibilities and only return false once
all wildcard paths have been evaluated.

* refactor switch and add more test cases

* remove comment leftover from debug session

* add more wildcard path validation and test cases

* update changelong; feature -> improvement

* simplify wildcard segment matching logic

* refactor wildcard matching into func

* fix glob matching, add more wildcard validation, refactor

* refactor common wildcard errors to func

* move doc comment to logical.Paths

* optimize wildcard paths storage with pre-split slices

* fix comment typo

* fix test case after changing wildcard paths storage type

* move prefix check to parseUnauthenticatedPaths

* tweak regex, remove unneeded array copy, refactor

* add test case around wildcard and glob matching
This commit is contained in:
John-Michael Faircloth 2021-10-13 11:51:20 -05:00 committed by GitHub
parent ad2ef412cc
commit 2abf916ddb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 295 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
changelog/12668.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
sdk/framework: The '+' wildcard is now supported for parameterizing unauthenticated paths.
```

6
sdk/framework/backend.go
View File

@ -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

4
sdk/logical/logical.go
View File

@ -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

2
vault/identity_store.go
View File

@ -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 {

6
vault/plugin_reload.go
View File

@ -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)
}
}

139
vault/router.go
View File

@ -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.

150
vault/router_test.go
View File

@ -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)
}
}
}