Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support trimming trailing slashes via a mount tuneable to support CMPv2 #28752

Merged
merged 20 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions api/sys_mounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,23 +290,23 @@ type MountInput struct {
}

type MountConfigInput struct {
Options map[string]string `json:"options" mapstructure:"options"`
DefaultLeaseTTL string `json:"default_lease_ttl" mapstructure:"default_lease_ttl"`
Description *string `json:"description,omitempty" mapstructure:"description"`
MaxLeaseTTL string `json:"max_lease_ttl" mapstructure:"max_lease_ttl"`
ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"`
AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"`
AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"`
ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"`
PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"`
AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"`
TokenType string `json:"token_type,omitempty" mapstructure:"token_type"`
AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"`
PluginVersion string `json:"plugin_version,omitempty"`
UserLockoutConfig *UserLockoutConfigInput `json:"user_lockout_config,omitempty"`
DelegatedAuthAccessors []string `json:"delegated_auth_accessors,omitempty" mapstructure:"delegated_auth_accessors"`
IdentityTokenKey string `json:"identity_token_key,omitempty" mapstructure:"identity_token_key"`

Options map[string]string `json:"options" mapstructure:"options"`
DefaultLeaseTTL string `json:"default_lease_ttl" mapstructure:"default_lease_ttl"`
Description *string `json:"description,omitempty" mapstructure:"description"`
MaxLeaseTTL string `json:"max_lease_ttl" mapstructure:"max_lease_ttl"`
ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"`
AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"`
AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"`
ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"`
PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"`
AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"`
TokenType string `json:"token_type,omitempty" mapstructure:"token_type"`
AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"`
PluginVersion string `json:"plugin_version,omitempty"`
UserLockoutConfig *UserLockoutConfigInput `json:"user_lockout_config,omitempty"`
DelegatedAuthAccessors []string `json:"delegated_auth_accessors,omitempty" mapstructure:"delegated_auth_accessors"`
IdentityTokenKey string `json:"identity_token_key,omitempty" mapstructure:"identity_token_key"`
TrimRequestTrailingSlashes bool `json:"trim_request_trailing_slashes,omitempty" mapstructure:"trim_request_trailing_slashes"`
// Deprecated: This field will always be blank for newer server responses.
PluginName string `json:"plugin_name,omitempty" mapstructure:"plugin_name"`
}
Expand Down
3 changes: 3 additions & 0 deletions changelog/28752.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
core: Add a mount tuneable that trims trailing slashes of request paths during POST. Needed to support CMPv2 in PKI.
```
39 changes: 25 additions & 14 deletions command/secrets_tune.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,21 @@
type SecretsTuneCommand struct {
*BaseCommand

flagAuditNonHMACRequestKeys []string
flagAuditNonHMACResponseKeys []string
flagDefaultLeaseTTL time.Duration
flagDescription string
flagListingVisibility string
flagMaxLeaseTTL time.Duration
flagPassthroughRequestHeaders []string
flagAllowedResponseHeaders []string
flagOptions map[string]string
flagVersion int
flagPluginVersion string
flagAllowedManagedKeys []string
flagDelegatedAuthAccessors []string
flagIdentityTokenKey string
flagAuditNonHMACRequestKeys []string
flagAuditNonHMACResponseKeys []string
flagDefaultLeaseTTL time.Duration
flagDescription string
flagListingVisibility string
flagMaxLeaseTTL time.Duration
flagPassthroughRequestHeaders []string
flagAllowedResponseHeaders []string
flagOptions map[string]string
flagVersion int
flagPluginVersion string
flagAllowedManagedKeys []string
flagDelegatedAuthAccessors []string
flagIdentityTokenKey string
flagTrimRequestTrailingSlashes bool
}

func (c *SecretsTuneCommand) Synopsis() string {
Expand Down Expand Up @@ -175,6 +176,13 @@
Usage: "Select the key used to sign plugin identity tokens.",
})

f.BoolVar(&BoolVar{
Name: flagNameTrimRequestTrailingSlashes,

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / artifacts / (linux, amd64)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (0)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (0)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests tagged with testonly / test-go (1)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests tagged with testonly / test-go (1)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Deprecated functions

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (16)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Code checks

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests with data race detection / test-go (1)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests with data race detection / test-go (1)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (7)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (7)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests with data race detection / test-go (9)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 180 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests with data race detection / test-go (9)

undefined: flagNameTrimRequestTrailingSlashes
Target: &c.flagTrimRequestTrailingSlashes,
Default: false,
Usage: "Whether to trim trailing slashes for incoming requests to this mount",
})

return set
}

Expand Down Expand Up @@ -267,6 +275,9 @@
if fl.Name == flagNameIdentityTokenKey {
mountConfigInput.IdentityTokenKey = c.flagIdentityTokenKey
}
if fl.Name == flagNameTrimRequestTrailingSlashes {

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / artifacts / (linux, amd64)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (0)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (0)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests tagged with testonly / test-go (1)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests tagged with testonly / test-go (1)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Deprecated functions

undefined: flagNameTrimRequestTrailingSlashes (compile)

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (16)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Code checks

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests with data race detection / test-go (1)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests with data race detection / test-go (1)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (7)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests / test-go (7)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests with data race detection / test-go (9)

undefined: flagNameTrimRequestTrailingSlashes

Check failure on line 278 in command/secrets_tune.go

View workflow job for this annotation

GitHub Actions / Run Go tests with data race detection / test-go (9)

undefined: flagNameTrimRequestTrailingSlashes
mountConfigInput.TrimRequestTrailingSlashes = c.flagTrimRequestTrailingSlashes
}
})

if err := client.Sys().TuneMount(mountPath, mountConfigInput); err != nil {
Expand Down
16 changes: 16 additions & 0 deletions vault/logical_system_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -3826,6 +3826,10 @@ func (b *SystemBackend) authPaths() []*framework.Path {
Description: strings.TrimSpace(sysHelp["identity_token_key"][0]),
Required: false,
},
"trim_request_trailing_slashes": {
Type: framework.TypeBool,
Required: false,
},
},
Operations: map[logical.Operation]framework.OperationHandler{
logical.ReadOperation: &framework.PathOperation{
Expand Down Expand Up @@ -3916,6 +3920,10 @@ func (b *SystemBackend) authPaths() []*framework.Path {
Type: framework.TypeString,
Required: false,
},
"trim_request_trailing_slashes": {
Type: framework.TypeBool,
Required: false,
},
},
}},
},
Expand Down Expand Up @@ -4686,6 +4694,10 @@ func (b *SystemBackend) mountPaths() []*framework.Path {
Type: framework.TypeString,
Description: strings.TrimSpace(sysHelp["identity_token_key"][0]),
},
"trim_request_trailing_slashes": {
Type: framework.TypeBool,
Description: strings.TrimSpace(sysHelp["trim_request_trailing_slashes"][0]),
},
},

Operations: map[logical.Operation]framework.OperationHandler{
Expand Down Expand Up @@ -4788,6 +4800,10 @@ func (b *SystemBackend) mountPaths() []*framework.Path {
Type: framework.TypeString,
Required: false,
},
"trim_request_trailing_slashes": {
Type: framework.TypeBool,
Required: false,
},
},
}},
},
Expand Down
27 changes: 14 additions & 13 deletions vault/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,19 +349,20 @@ type MountEntry struct {

// MountConfig is used to hold settable options
type MountConfig struct {
DefaultLeaseTTL time.Duration `json:"default_lease_ttl,omitempty" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"` // Override for global default
MaxLeaseTTL time.Duration `json:"max_lease_ttl,omitempty" structs:"max_lease_ttl" mapstructure:"max_lease_ttl"` // Override for global default
ForceNoCache bool `json:"force_no_cache,omitempty" structs:"force_no_cache" mapstructure:"force_no_cache"` // Override for global default
AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" structs:"audit_non_hmac_request_keys" mapstructure:"audit_non_hmac_request_keys"`
AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" structs:"audit_non_hmac_response_keys" mapstructure:"audit_non_hmac_response_keys"`
ListingVisibility ListingVisibilityType `json:"listing_visibility,omitempty" structs:"listing_visibility" mapstructure:"listing_visibility"`
PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" structs:"passthrough_request_headers" mapstructure:"passthrough_request_headers"`
AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" structs:"allowed_response_headers" mapstructure:"allowed_response_headers"`
TokenType logical.TokenType `json:"token_type,omitempty" structs:"token_type" mapstructure:"token_type"`
AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"`
UserLockoutConfig *UserLockoutConfig `json:"user_lockout_config,omitempty" mapstructure:"user_lockout_config"`
DelegatedAuthAccessors []string `json:"delegated_auth_accessors,omitempty" mapstructure:"delegated_auth_accessors"`
IdentityTokenKey string `json:"identity_token_key,omitempty" mapstructure:"identity_token_key"`
DefaultLeaseTTL time.Duration `json:"default_lease_ttl,omitempty" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"` // Override for global default
MaxLeaseTTL time.Duration `json:"max_lease_ttl,omitempty" structs:"max_lease_ttl" mapstructure:"max_lease_ttl"` // Override for global default
ForceNoCache bool `json:"force_no_cache,omitempty" structs:"force_no_cache" mapstructure:"force_no_cache"` // Override for global default
AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" structs:"audit_non_hmac_request_keys" mapstructure:"audit_non_hmac_request_keys"`
AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" structs:"audit_non_hmac_response_keys" mapstructure:"audit_non_hmac_response_keys"`
ListingVisibility ListingVisibilityType `json:"listing_visibility,omitempty" structs:"listing_visibility" mapstructure:"listing_visibility"`
PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" structs:"passthrough_request_headers" mapstructure:"passthrough_request_headers"`
AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" structs:"allowed_response_headers" mapstructure:"allowed_response_headers"`
TokenType logical.TokenType `json:"token_type,omitempty" structs:"token_type" mapstructure:"token_type"`
AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"`
UserLockoutConfig *UserLockoutConfig `json:"user_lockout_config,omitempty" mapstructure:"user_lockout_config"`
DelegatedAuthAccessors []string `json:"delegated_auth_accessors,omitempty" mapstructure:"delegated_auth_accessors"`
IdentityTokenKey string `json:"identity_token_key,omitempty" mapstructure:"identity_token_key"`
TrimRequestTrailingSlashes bool `json:"trim_request_trailing_slashes,omitempty` // If requests to this mount should have trailing slashes trimmed

// PluginName is the name of the plugin registered in the catalog.
//
Expand Down
29 changes: 17 additions & 12 deletions vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,18 +612,6 @@ func (c *Core) switchedLockHandleRequest(httpCtx context.Context, req *logical.R
}

func (c *Core) handleCancelableRequest(ctx context.Context, req *logical.Request) (resp *logical.Response, err error) {
// Allowing writing to a path ending in / makes it extremely difficult to
// understand user intent for the filesystem-like backends (kv,
// cubbyhole) -- did they want a key named foo/ or did they want to write
// to a directory foo/ with no (or forgotten) key, or...? It also affects
// lookup, because paths ending in / are considered prefixes by some
// backends. Basically, it's all just terrible, so don't allow it.
if strings.HasSuffix(req.Path, "/") &&
(req.Operation == logical.UpdateOperation ||
req.Operation == logical.CreateOperation ||
req.Operation == logical.PatchOperation) {
return logical.ErrorResponse("cannot write to a path ending in '/'"), nil
}
waitGroup, err := waitForReplicationState(ctx, c, req)
if err != nil {
return nil, err
Expand Down Expand Up @@ -1007,6 +995,23 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
if rawVals, ok := entry.synthesizedConfigCache.Load("audit_non_hmac_request_keys"); ok {
nonHMACReqDataKeys = rawVals.([]string)
}

// Allowing writing to a path ending in / makes it extremely difficult to
// understand user intent for the filesystem-like backends (kv,
// cubbyhole) -- did they want a key named foo/ or did they want to write
// to a directory foo/ with no (or forgotten) key, or...? It also affects
// lookup, because paths ending in / are considered prefixes by some
// backends. Basically, it's all just terrible, so don't allow it.
if strings.HasSuffix(req.Path, "/") &&
(req.Operation == logical.UpdateOperation ||
req.Operation == logical.CreateOperation ||
req.Operation == logical.PatchOperation) {
if !entry.Config.TrimRequestTrailingSlashes {
return logical.ErrorResponse("cannot write to a path ending in '/'"), nil, nil
} else {
req.Path = strings.TrimSuffix(req.Path, "/")
}
}
}

ns, err := namespace.FromContext(ctx)
Expand Down
9 changes: 5 additions & 4 deletions website/content/docs/secrets/pki/cmpv2.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,18 @@ To get an authentication mount's accessor field, the following command can be us
$ vault read -field=accessor sys/auth/auth/cert
```

For CMP to work within certain clients, a few response headers need to be explicitly allowed
along with configuring the list of accessors the mount can delegate authentication towards.
The following will grant the required response headers, you will need to replace the values for the `delegated-auth-accessors`
to match your values.
For CMP to work within certain clients, a few response headers need to be explicitly
allowed, trailing slashes must be trimmed, and the list of accessors the mount can delegate authentication towards
must be configured. The following will grant the required response headers, you will need to replace the values for
the `delegated-auth-accessors` to match your values.

```shell-session
$ vault secrets tune \
-allowed-response-headers="Content-Transfer-Encoding" \
-allowed-response-headers="Content-Length" \
-allowed-response-headers="WWW-Authenticate" \
-delegated-auth-accessors="auth_cert_4088ac2d" \
-trim-request-trailing-slashes="true" \
pki
```

Expand Down
Loading