Fix regexes for sys/raw/ and sys/leases/lookup/ to match prevailing conventions (#21760)

* Fix regexes for `sys/raw/` and `sys/leases/lookup/` to match prevailing conventions

There are several endpoints in Vault which take an arbitrary path as the
last parameter. Many of these are defined in terms of the
`framework.MatchAllRegex` helper. Some were not, and were defined using
custom regexes which gave rise to multiple OpenAPI endpoints - one with
the path parameter, and one without.

We need to fix these definitions, because they give rise to a very
unnatural result when used to generate a client API - for example, you
end up with `LeasesLookUp()` which is only capable of being used to list
the very top level of the hierarchical collection of leases, and
`LeasesLookUpWithPrefix(prefix)` which must be used for all deeper
levels.

This PR changes the regexes used for `sys/raw/` and `sys/leases/lookup/`
to be consistent with the approach used for other well-known similar
endpoints, such as `cubbyhole/`, `kv-v1/` and `kv-v2/metadata/`.

This PR does have a very small compatibility issue, which I think is
tolerable: prior to this change, `sys/raw` with no trailing slash was
considered a valid endpoint, and now it will no longer be.

One way to observe this is to try `vault path-help sys/raw` - before
this change, it would work, after, it will not. You would have to
instead use `vault path-help sys/raw/foobar` to see the help.

I also considered whether losing the ability to read/write/delete
`sys/raw` would be an issue. In each case, the precise HTTP result code
will change, but each of these were meaningless operations that make no
sense - you cannot read/write/delete a "file" at the "root directory" of
the underlying Vault storage.

In fact, during testing, I discovered that currently, `vault write
sys/raw x=y` when using Raft storage, will permanently break the Vault
instance - it causes a panic within the Raft FSM, which re-occurs
immediately on restarting the server! This PR also closes off that
footgun / DoS vector.

None of these issues apply to `sys/leases/lookup/`, as the existing
regex in that case was already not matching the path without the
trailing slash.

* changelog

* Realign hardcoded sudo paths with updated OpenAPI spec
This commit is contained in:
Max Bowsher
2023-07-21 18:50:07 +01:00
committed by GitHub
parent e91b507996
commit 25540ad222
4 changed files with 7 additions and 16 deletions

View File

@@ -32,15 +32,13 @@ var sudoPaths = map[string]*regexp.Regexp{
// This entry is a bit wrong... sys/leases/lookup does NOT require sudo. But sys/leases/lookup/ with a trailing
// slash DOES require sudo. But the part of the Vault CLI that uses this logic doesn't pass operation-appropriate
// trailing slashes, it always strips them off, so we end up giving the wrong answer for one of these.
"/sys/leases/lookup": regexp.MustCompile(`^/sys/leases/lookup/?$`),
"/sys/leases/lookup/{prefix}": regexp.MustCompile(`^/sys/leases/lookup/.+$`),
"/sys/leases/lookup/{prefix}": regexp.MustCompile(`^/sys/leases/lookup(?:/.+)?$`),
"/sys/leases/revoke-force/{prefix}": regexp.MustCompile(`^/sys/leases/revoke-force/.+$`),
"/sys/leases/revoke-prefix/{prefix}": regexp.MustCompile(`^/sys/leases/revoke-prefix/.+$`),
"/sys/plugins/catalog/{name}": regexp.MustCompile(`^/sys/plugins/catalog/[^/]+$`),
"/sys/plugins/catalog/{type}": regexp.MustCompile(`^/sys/plugins/catalog/[\w-]+$`),
"/sys/plugins/catalog/{type}/{name}": regexp.MustCompile(`^/sys/plugins/catalog/[\w-]+/[^/]+$`),
"/sys/raw": regexp.MustCompile(`^/sys/raw$`),
"/sys/raw/{path}": regexp.MustCompile(`^/sys/raw/.+$`),
"/sys/raw/{path}": regexp.MustCompile(`^/sys/raw(?:/.+)?$`),
"/sys/remount": regexp.MustCompile(`^/sys/remount$`),
"/sys/revoke-force/{prefix}": regexp.MustCompile(`^/sys/revoke-force/.+$`),
"/sys/revoke-prefix/{prefix}": regexp.MustCompile(`^/sys/revoke-prefix/.+$`),

3
changelog/21760.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:improvement
core: Fix regexes for `sys/raw/` and `sys/leases/lookup/` to match prevailing conventions
```

View File

@@ -296,7 +296,7 @@ func (b *RawBackend) existenceCheck(ctx context.Context, request *logical.Reques
func rawPaths(prefix string, r *RawBackend) []*framework.Path {
return []*framework.Path{
{
Pattern: prefix + "(raw/?$|raw/(?P<path>.+))",
Pattern: prefix + "raw/" + framework.MatchAllRegex("path"),
Fields: map[string]*framework.FieldSchema{
"path": {
@@ -322,7 +322,6 @@ func rawPaths(prefix string, r *RawBackend) []*framework.Path {
DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: "raw",
OperationVerb: "read",
OperationSuffix: "|path",
},
Responses: map[int][]framework.Response{
http.StatusOK: {{
@@ -342,7 +341,6 @@ func rawPaths(prefix string, r *RawBackend) []*framework.Path {
DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: "raw",
OperationVerb: "write",
OperationSuffix: "|path",
},
Responses: map[int][]framework.Response{
http.StatusOK: {{
@@ -353,11 +351,6 @@ func rawPaths(prefix string, r *RawBackend) []*framework.Path {
},
logical.CreateOperation: &framework.PathOperation{
Callback: r.handleRawWrite,
DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: "raw",
OperationVerb: "write",
OperationSuffix: "|path",
},
Responses: map[int][]framework.Response{
http.StatusNoContent: {{
Description: "OK",
@@ -370,7 +363,6 @@ func rawPaths(prefix string, r *RawBackend) []*framework.Path {
DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: "raw",
OperationVerb: "delete",
OperationSuffix: "|path",
},
Responses: map[int][]framework.Response{
http.StatusNoContent: {{
@@ -384,7 +376,6 @@ func rawPaths(prefix string, r *RawBackend) []*framework.Path {
DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: "raw",
OperationVerb: "list",
OperationSuffix: "|path",
},
Responses: map[int][]framework.Response{
http.StatusOK: {{

View File

@@ -2696,12 +2696,11 @@ func (b *SystemBackend) capabilitiesPaths() []*framework.Path {
func (b *SystemBackend) leasePaths() []*framework.Path {
return []*framework.Path{
{
Pattern: "leases/lookup/(?P<prefix>.+?)?",
Pattern: "leases/lookup/" + framework.MatchAllRegex("prefix"),
DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: "leases",
OperationVerb: "look-up",
OperationSuffix: "|with-prefix",
},
Fields: map[string]*framework.FieldSchema{