From 1d9e560f0f35e617e607661544d92ef349f0ad3d Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Thu, 6 Jul 2023 16:01:33 -0400 Subject: [PATCH] backport of commit 2f677665b37fcced51737cdef7abbebb4c719529 (#21527) Co-authored-by: Max Bowsher --- api/plugin_helpers.go | 8 +++++--- api/plugin_helpers_test.go | 19 +++++++++++++++++++ changelog/18571.txt | 3 +++ sdk/framework/path.go | 11 +++++++++++ vault/logical_system_paths.go | 4 ++-- vault/logical_system_pprof.go | 2 +- vault/token_store.go | 2 +- 7 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 changelog/18571.txt diff --git a/api/plugin_helpers.go b/api/plugin_helpers.go index 507b72c4c..5bb566300 100644 --- a/api/plugin_helpers.go +++ b/api/plugin_helpers.go @@ -40,7 +40,7 @@ const ( // path matches that path or not (useful specifically for the paths that // contain templated fields.) var sudoPaths = map[string]*regexp.Regexp{ - "/auth/token/accessors/": regexp.MustCompile(`^/auth/token/accessors/?$`), + "/auth/token/accessors": regexp.MustCompile(`^/auth/token/accessors/?$`), "/pki/root": regexp.MustCompile(`^/pki/root$`), "/pki/root/sign-self-issued": regexp.MustCompile(`^/pki/root/sign-self-issued$`), "/sys/audit": regexp.MustCompile(`^/sys/audit$`), @@ -50,7 +50,7 @@ var sudoPaths = map[string]*regexp.Regexp{ "/sys/config/auditing/request-headers": regexp.MustCompile(`^/sys/config/auditing/request-headers$`), "/sys/config/auditing/request-headers/{header}": regexp.MustCompile(`^/sys/config/auditing/request-headers/.+$`), "/sys/config/cors": regexp.MustCompile(`^/sys/config/cors$`), - "/sys/config/ui/headers/": regexp.MustCompile(`^/sys/config/ui/headers/?$`), + "/sys/config/ui/headers": regexp.MustCompile(`^/sys/config/ui/headers/?$`), "/sys/config/ui/headers/{header}": regexp.MustCompile(`^/sys/config/ui/headers/.+$`), "/sys/leases": regexp.MustCompile(`^/sys/leases$`), "/sys/leases/lookup/": regexp.MustCompile(`^/sys/leases/lookup/?$`), @@ -249,7 +249,9 @@ func SudoPaths() map[string]*regexp.Regexp { return sudoPaths } -// Determine whether the given path requires the sudo capability +// Determine whether the given path requires the sudo capability. +// Note that this uses hardcoded static path information, so will return incorrect results for paths in namespaces, +// or for secret engines mounted at non-default paths. func IsSudoPath(path string) bool { // Return early if the path is any of the non-templated sudo paths. if _, ok := sudoPaths[path]; ok { diff --git a/api/plugin_helpers_test.go b/api/plugin_helpers_test.go index 7b3ddbf81..2e97d44cb 100644 --- a/api/plugin_helpers_test.go +++ b/api/plugin_helpers_test.go @@ -12,10 +12,12 @@ func TestIsSudoPath(t *testing.T) { path string expected bool }{ + // Testing: Not a real endpoint { "/not/in/sudo/paths/list", false, }, + // Testing: sys/raw/{path} { "/sys/raw/single-node-path", true, @@ -28,26 +30,43 @@ func TestIsSudoPath(t *testing.T) { "/sys/raw/WEIRD(but_still_valid!)p4Th?🗿笑", true, }, + // Testing: sys/auth/{path}/tune { "/sys/auth/path/in/middle/tune", true, }, + // Testing: sys/plugins/catalog/{type} and sys/plugins/catalog/{name} (regexes overlap) { "/sys/plugins/catalog/some-type", true, }, + // Testing: Not a real endpoint { "/sys/plugins/catalog/some/type/or/name/with/slashes", false, }, + // Testing: sys/plugins/catalog/{type}/{name} { "/sys/plugins/catalog/some-type/some-name", true, }, + // Testing: Not a real endpoint { "/sys/plugins/catalog/some-type/some/name/with/slashes", false, }, + // Testing: auth/token/accessors (an example of a sudo path that only accepts list operations) + // It is matched as sudo without the trailing slash... + { + "/auth/token/accessors", + true, + }, + // ...and also with it. + // (Although at the time of writing, the only caller of IsSudoPath always removes trailing slashes.) + { + "/auth/token/accessors/", + true, + }, } for _, tc := range testCases { diff --git a/changelog/18571.txt b/changelog/18571.txt new file mode 100644 index 000000000..dd811d9fd --- /dev/null +++ b/changelog/18571.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth/token, sys: Fix path-help being unavailable for some list-only endpoints +``` diff --git a/sdk/framework/path.go b/sdk/framework/path.go index e96ba4c41..ccee0fc90 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -56,6 +56,17 @@ type Path struct { // This should be a valid regular expression. Named captures will be // exposed as fields that should map to a schema in Fields. If a named // capture is not a field in the Fields map, then it will be ignored. + // + // The pattern will automatically have a ^ prepended and a $ appended before + // use, if these are not already present, so these may be omitted for clarity. + // + // If a ListOperation is being defined, the pattern must end with /? to match + // a trailing slash optionally, as ListOperations are always processed with a + // trailing slash added to the path if not already present. The match must not + // require the presence of a trailing slash, as HelpOperations, even for a + // path which only implements ListOperation, are processed without a trailing + // slash - so failure to make the trailing slash optional will break the + // `vault path-help` command for the path. Pattern string // Fields is the mapping of data fields to a schema describing that diff --git a/vault/logical_system_paths.go b/vault/logical_system_paths.go index 57e09c658..ce525d5b9 100644 --- a/vault/logical_system_paths.go +++ b/vault/logical_system_paths.go @@ -224,7 +224,7 @@ func (b *SystemBackend) configPaths() []*framework.Path { }, { - Pattern: "config/ui/headers/$", + Pattern: "config/ui/headers/?$", Operations: map[logical.Operation]framework.OperationHandler{ logical.ListOperation: &framework.PathOperation{ @@ -1452,7 +1452,7 @@ func (b *SystemBackend) statusPaths() []*framework.Path { HelpDescription: strings.TrimSpace(sysHelp["ha-status"][1]), }, { - Pattern: "version-history/$", + Pattern: "version-history/?$", DisplayAttrs: &framework.DisplayAttributes{ OperationVerb: "version-history", diff --git a/vault/logical_system_pprof.go b/vault/logical_system_pprof.go index 9b5a5a8fa..b09f2ec87 100644 --- a/vault/logical_system_pprof.go +++ b/vault/logical_system_pprof.go @@ -18,7 +18,7 @@ import ( func (b *SystemBackend) pprofPaths() []*framework.Path { return []*framework.Path{ { - Pattern: "pprof/$", + Pattern: "pprof/?$", DisplayAttrs: &framework.DisplayAttributes{ OperationPrefix: "pprof", diff --git a/vault/token_store.go b/vault/token_store.go index 741444c56..18d5f3780 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -158,7 +158,7 @@ func (ts *TokenStore) paths() []*framework.Path { }, { - Pattern: "accessors/$", + Pattern: "accessors/?$", DisplayAttrs: &framework.DisplayAttributes{ OperationPrefix: operationPrefixToken,