From 10512bb6c47b0e1cde32bfc888e9ac7bd7fc7c20 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: Mon, 10 Jul 2023 17:34:10 -0400 Subject: [PATCH] backport of commit 3bf1299814af605b534a8c20b207790d3de21bcd (#21715) Co-authored-by: Max Bowsher Co-authored-by: Anton Averchenkov <84287187+averche@users.noreply.github.com> --- changelog/18556.txt | 3 + vault/token_store.go | 387 +++++++++++++++---------------------------- 2 files changed, 135 insertions(+), 255 deletions(-) create mode 100644 changelog/18556.txt diff --git a/changelog/18556.txt b/changelog/18556.txt new file mode 100644 index 0000000000..a48dacde5b --- /dev/null +++ b/changelog/18556.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth/token: Fix parsing of `auth/token/create` fields to avoid incorrect warnings about ignored parameters +``` diff --git a/vault/token_store.go b/vault/token_store.go index 18d5f37806..d25aa629df 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -41,7 +41,6 @@ import ( "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/plugin/pb" "github.com/hashicorp/vault/vault/tokens" - "github.com/mitchellh/mapstructure" ) const ( @@ -138,6 +137,77 @@ var ( ) func (ts *TokenStore) paths() []*framework.Path { + commonFieldsForCreate := map[string]*framework.FieldSchema{ + "display_name": { + Type: framework.TypeString, + Description: "Name to associate with this token", + }, + "explicit_max_ttl": { + Type: framework.TypeString, + Description: "Explicit Max TTL of this token", + }, + "entity_alias": { + Type: framework.TypeString, + Description: "Name of the entity alias to associate with this token", + }, + "num_uses": { + Type: framework.TypeInt, + Description: "Max number of uses for this token", + }, + "period": { + Type: framework.TypeString, + Description: "Renew period", + }, + "renewable": { + Type: framework.TypeBool, + Description: "Allow token to be renewed past its initial TTL up to system/mount maximum TTL", + Default: true, + }, + "ttl": { + Type: framework.TypeString, + Description: "Time to live for this token", + }, + "lease": { + Type: framework.TypeString, + Description: "Use 'ttl' instead", + Deprecated: true, + }, + "type": { + Type: framework.TypeString, + Description: "Token type", + }, + "no_default_policy": { + Type: framework.TypeBool, + Description: "Do not include default policy for this token", + }, + "id": { + Type: framework.TypeString, + Description: "Value for the token", + }, + "meta": { + Type: framework.TypeKVPairs, + Description: "Arbitrary key=value metadata to associate with the token", + }, + "no_parent": { + Type: framework.TypeBool, + Description: "Create the token with no parent", + }, + "policies": { + Type: framework.TypeStringSlice, + Description: "List of policies for the token", + }, + } + + fieldsForCreateWithRole := map[string]*framework.FieldSchema{ + "role_name": { + Type: framework.TypeString, + Description: "Name of the role", + }, + } + for k, v := range commonFieldsForCreate { + fieldsForCreateWithRole[k] = v + } + const operationPrefixToken = "token" p := []*framework.Path{ @@ -176,76 +246,14 @@ func (ts *TokenStore) paths() []*framework.Path { { Pattern: "create-orphan$", + Fields: commonFieldsForCreate, + DisplayAttrs: &framework.DisplayAttributes{ OperationPrefix: operationPrefixToken, OperationVerb: "create", OperationSuffix: "orphan", }, - Fields: map[string]*framework.FieldSchema{ - "role_name": { - Type: framework.TypeString, - Description: "Name of the role", - }, - "display_name": { - Type: framework.TypeString, - Description: "Name to associate with this token", - }, - "explicit_max_ttl": { - Type: framework.TypeString, - Description: "Explicit Max TTL of this token", - }, - "entity_alias": { - Type: framework.TypeString, - Description: "Name of the entity alias to associate with this token", - }, - "num_uses": { - Type: framework.TypeInt, - Description: "Max number of uses for this token", - }, - "period": { - Type: framework.TypeString, - Description: "Renew period", - }, - "renewable": { - Type: framework.TypeBool, - Description: "Allow token to be renewed past its initial TTL up to system/mount maximum TTL", - }, - "ttl": { - Type: framework.TypeString, - Description: "Time to live for this token", - }, - "type": { - Type: framework.TypeString, - Description: "Token type", - }, - "no_default_policy": { - Type: framework.TypeBool, - Description: "Do not include default policy for this token", - }, - "id": { - Type: framework.TypeString, - Description: "Value for the token", - }, - "metadata": { - Type: framework.TypeMap, - Description: "Arbitrary key=value metadata to associate with the token", - }, - "no_parent": { - Type: framework.TypeBool, - Description: "Create the token with no parent", - }, - "policies": { - Type: framework.TypeStringSlice, - Description: "List of policies for the token", - }, - "format": { - Type: framework.TypeString, - Query: true, - Description: "Return json formatted output", - }, - }, - Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: ts.handleCreateOrphan, }, @@ -257,76 +265,14 @@ func (ts *TokenStore) paths() []*framework.Path { { Pattern: "create/" + framework.GenericNameRegex("role_name"), + Fields: fieldsForCreateWithRole, + DisplayAttrs: &framework.DisplayAttributes{ OperationPrefix: operationPrefixToken, OperationVerb: "create", OperationSuffix: "against-role", }, - Fields: map[string]*framework.FieldSchema{ - "role_name": { - Type: framework.TypeString, - Description: "Name of the role", - }, - "display_name": { - Type: framework.TypeString, - Description: "Name to associate with this token", - }, - "explicit_max_ttl": { - Type: framework.TypeString, - Description: "Explicit Max TTL of this token", - }, - "entity_alias": { - Type: framework.TypeString, - Description: "Name of the entity alias to associate with this token", - }, - "num_uses": { - Type: framework.TypeInt, - Description: "Max number of uses for this token", - }, - "period": { - Type: framework.TypeString, - Description: "Renew period", - }, - "renewable": { - Type: framework.TypeBool, - Description: "Allow token to be renewed past its initial TTL up to system/mount maximum TTL", - }, - "ttl": { - Type: framework.TypeString, - Description: "Time to live for this token", - }, - "type": { - Type: framework.TypeString, - Description: "Token type", - }, - "no_default_policy": { - Type: framework.TypeBool, - Description: "Do not include default policy for this token", - }, - "id": { - Type: framework.TypeString, - Description: "Value for the token", - }, - "metadata": { - Type: framework.TypeMap, - Description: "Arbitrary key=value metadata to associate with the token", - }, - "no_parent": { - Type: framework.TypeBool, - Description: "Create the token with no parent", - }, - "policies": { - Type: framework.TypeStringSlice, - Description: "List of policies for the token", - }, - "format": { - Type: framework.TypeString, - Query: true, - Description: "Return json formatted output", - }, - }, - Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: ts.handleCreateAgainstRole, }, @@ -338,71 +284,13 @@ func (ts *TokenStore) paths() []*framework.Path { { Pattern: "create$", + Fields: commonFieldsForCreate, + DisplayAttrs: &framework.DisplayAttributes{ OperationPrefix: operationPrefixToken, OperationVerb: "create", }, - Fields: map[string]*framework.FieldSchema{ - "display_name": { - Type: framework.TypeString, - Description: "Name to associate with this token", - }, - "explicit_max_ttl": { - Type: framework.TypeString, - Description: "Explicit Max TTL of this token", - }, - "entity_alias": { - Type: framework.TypeString, - Description: "Name of the entity alias to associate with this token", - }, - "num_uses": { - Type: framework.TypeInt, - Description: "Max number of uses for this token", - }, - "period": { - Type: framework.TypeString, - Description: "Renew period", - }, - "renewable": { - Type: framework.TypeBool, - Description: "Allow token to be renewed past its initial TTL up to system/mount maximum TTL", - }, - "ttl": { - Type: framework.TypeString, - Description: "Time to live for this token", - }, - "type": { - Type: framework.TypeString, - Description: "Token type", - }, - "no_default_policy": { - Type: framework.TypeBool, - Description: "Do not include default policy for this token", - }, - "id": { - Type: framework.TypeString, - Description: "Value for the token", - }, - "metadata": { - Type: framework.TypeMap, - Description: "Arbitrary key=value metadata to associate with the token", - }, - "no_parent": { - Type: framework.TypeBool, - Description: "Create the token with no parent", - }, - "policies": { - Type: framework.TypeStringSlice, - Description: "List of policies for the token", - }, - "format": { - Type: framework.TypeString, - Query: true, - Description: "Return json formatted output", - }, - }, - Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: ts.handleCreate, }, @@ -2728,27 +2616,7 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // Check if the client token has sudo/root privileges for the requested path isSudo := ts.System().(extendedSystemView).SudoPrivilege(ctx, req.MountPoint+req.Path, req.ClientToken) - // Read and parse the fields - var data struct { - ID string - Policies []string - Metadata map[string]string `mapstructure:"meta"` - NoParent bool `mapstructure:"no_parent"` - NoDefaultPolicy bool `mapstructure:"no_default_policy"` - Lease string - TTL string - Renewable *bool - ExplicitMaxTTL string `mapstructure:"explicit_max_ttl"` - DisplayName string `mapstructure:"display_name"` - NumUses int `mapstructure:"num_uses"` - Period string - Type string `mapstructure:"type"` - EntityAlias string `mapstructure:"entity_alias"` - } - if err := mapstructure.WeakDecode(req.Data, &data); err != nil { - return logical.ErrorResponse(fmt.Sprintf( - "Error decoding request: %s", err)), logical.ErrInvalidRequest - } + policies := d.Get("policies").([]string) // If the context's namespace is different from the parent and this is an // orphan token creation request, then this is an admin token generation for @@ -2772,18 +2640,13 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque return logical.ErrorResponse("root or sudo privileges required to directly generate a token in a child namespace"), logical.ErrInvalidRequest } - if strutil.StrListContains(data.Policies, "root") { + if strutil.StrListContains(policies, "root") { return logical.ErrorResponse("root tokens may not be created from a parent namespace"), logical.ErrInvalidRequest } } - renewable := true - if data.Renewable != nil { - renewable = *data.Renewable - } - tokenType := logical.TokenTypeService - tokenTypeStr := data.Type + tokenTypeStr := d.Get("type").(string) if role != nil { switch role.TokenType { case logical.TokenTypeDefault, logical.TokenTypeDefaultService: @@ -2801,23 +2664,28 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque return logical.ErrorResponse(fmt.Sprintf("role being used for token creation contains invalid token type %q", role.TokenType.String())), nil } } + + renewable := d.Get("renewable").(bool) + explicitMaxTTL := d.Get("explicit_max_ttl").(string) + numUses := d.Get("num_uses").(int) + period := d.Get("period").(string) switch tokenTypeStr { case "", "service": case "batch": var badReason string switch { - case data.ExplicitMaxTTL != "": - dur, err := parseutil.ParseDurationSecond(data.ExplicitMaxTTL) + case explicitMaxTTL != "": + dur, err := parseutil.ParseDurationSecond(explicitMaxTTL) if err != nil { return logical.ErrorResponse(`"explicit_max_ttl" value could not be parsed`), nil } if dur != 0 { badReason = "explicit_max_ttl" } - case data.NumUses != 0: + case numUses != 0: badReason = "num_uses" - case data.Period != "": - dur, err := parseutil.ParseDurationSecond(data.Period) + case period != "": + dur, err := parseutil.ParseDurationSecond(period) if err != nil { return logical.ErrorResponse(`"period" value could not be parsed`), nil } @@ -2835,14 +2703,14 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque } // Verify the number of uses is positive - if data.NumUses < 0 { + if numUses < 0 { return logical.ErrorResponse("number of uses cannot be negative"), logical.ErrInvalidRequest } // Verify the entity alias var explicitEntityID string - if data.EntityAlias != "" { + if entityAliasRaw := d.Get("entity_alias").(string); entityAliasRaw != "" { // Parameter is only allowed in combination with token role if role == nil { return logical.ErrorResponse("'entity_alias' is only allowed in combination with token role"), logical.ErrInvalidRequest @@ -2850,8 +2718,8 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // Convert entity alias to lowercase to match the fact that role.AllowedEntityAliases // has also been lowercased. An entity alias will keep its case formatting, but be - // treated as lowercase during any value cheek anywhere. - entityAlias := strings.ToLower(data.EntityAlias) + // treated as lowercase during any value check anywhere. + entityAlias := strings.ToLower(entityAliasRaw) // Check if there is a concrete match if !strutil.StrListContains(role.AllowedEntityAliases, entityAlias) && @@ -2867,7 +2735,7 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // Create alias for later processing alias := &logical.Alias{ - Name: data.EntityAlias, + Name: entityAliasRaw, MountAccessor: mountValidationResp.Accessor, MountType: mountValidationResp.Type, } @@ -2900,7 +2768,15 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque explicitEntityID = entity.ID } - // Setup the token entry + // GetOk is used here solely to preserve the distinction between an absent/nil map and an empty map, to match the + // behaviour of previous Vault versions - rather than introducing a potential slight compatibility issue for users. + meta, ok := d.GetOk("meta") + var metaMap map[string]string + if ok { + metaMap = meta.(map[string]string) + } + + // Set up the token entry te := logical.TokenEntry{ Parent: req.ClientToken, @@ -2909,9 +2785,9 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // have an official mount Path: fmt.Sprintf("auth/token/%s", req.Path), - Meta: data.Metadata, + Meta: metaMap, DisplayName: "token", - NumUses: data.NumUses, + NumUses: numUses, CreationTime: time.Now().Unix(), NamespaceID: ns.ID, Type: tokenType, @@ -2947,15 +2823,15 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque } // Attach the given display name if any - if data.DisplayName != "" { - full := "token-" + data.DisplayName + if displayName := d.Get("display_name").(string); displayName != "" { + full := "token-" + displayName full = displayNameSanitize.ReplaceAllString(full, "-") full = strings.TrimSuffix(full, "-") te.DisplayName = full } // Allow specifying the ID of the token if the client has root or sudo privileges - if data.ID != "" { + if id := d.Get("id").(string); id != "" { if !isSudo { return logical.ErrorResponse("root or sudo privileges required to specify token id"), logical.ErrInvalidRequest @@ -2964,7 +2840,7 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque return logical.ErrorResponse("token IDs can only be manually specified in the root namespace"), logical.ErrInvalidRequest } - te.ID = data.ID + te.ID = id } resp := &logical.Response{} @@ -2976,6 +2852,7 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // should be stripped out regardless, *but*, the logic of when it should // and shouldn't be added is kept because we want to do subset comparisons // based on adding default when it's correct to do so. + noDefaultPolicy := d.Get("no_default_policy").(bool) switch { case role != nil && (len(role.AllowedPolicies) > 0 || len(role.DisallowedPolicies) > 0 || len(role.AllowedPoliciesGlob) > 0 || len(role.DisallowedPoliciesGlob) > 0): @@ -2990,15 +2867,15 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // isn't in the disallowed list, add it. This is in line with the idea // that roles, when allowed/disallowed ar set, allow a subset of // policies to be set disjoint from the parent token's policies. - if !data.NoDefaultPolicy && !role.TokenNoDefaultPolicy && + if !noDefaultPolicy && !role.TokenNoDefaultPolicy && !strutil.StrListContains(role.DisallowedPolicies, "default") && !strutil.StrListContainsGlob(role.DisallowedPoliciesGlob, "default") { localAddDefault = true } // Start with passed-in policies as a baseline, if they exist - if len(data.Policies) > 0 { - finalPolicies = policyutil.SanitizePolicies(data.Policies, localAddDefault) + if len(policies) > 0 { + finalPolicies = policyutil.SanitizePolicies(policies, localAddDefault) } var sanitizedRolePolicies, sanitizedRolePoliciesGlob []string @@ -3045,17 +2922,17 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque } } - data.Policies = finalPolicies + policies = finalPolicies // We are creating a token from a parent namespace. We should only use the input // policies. case ns.ID != parent.NamespaceID: - addDefault = !data.NoDefaultPolicy + addDefault = !noDefaultPolicy // No policies specified, inherit parent - case len(data.Policies) == 0: + case len(policies) == 0: // Only inherit "default" if the parent already has it, so don't touch addDefault here - data.Policies = policyutil.SanitizePolicies(parent.Policies, policyutil.DoNotAddDefaultPolicy) + policies = policyutil.SanitizePolicies(parent.Policies, policyutil.DoNotAddDefaultPolicy) // When a role is not in use or does not specify allowed/disallowed, only // permit policies to be a subset unless the client has root or sudo @@ -3063,7 +2940,7 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // the client specified for it not to be added. case !isSudo: // Sanitize passed-in and parent policies before comparison - sanitizedInputPolicies := policyutil.SanitizePolicies(data.Policies, policyutil.DoNotAddDefaultPolicy) + sanitizedInputPolicies := policyutil.SanitizePolicies(policies, policyutil.DoNotAddDefaultPolicy) sanitizedParentPolicies := policyutil.SanitizePolicies(parent.Policies, policyutil.DoNotAddDefaultPolicy) if !strutil.StrListSubset(sanitizedParentPolicies, sanitizedInputPolicies) { @@ -3074,19 +2951,19 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque // add it. Note that if they have explicitly put "default" in // data.Policies it will still be added because NoDefaultPolicy // controls *automatic* adding. - if !data.NoDefaultPolicy && strutil.StrListContains(parent.Policies, "default") { + if !noDefaultPolicy && strutil.StrListContains(parent.Policies, "default") { addDefault = true } // Add default by default in this case unless requested not to case isSudo: - addDefault = !data.NoDefaultPolicy + addDefault = !noDefaultPolicy } - te.Policies = policyutil.SanitizePolicies(data.Policies, addDefault) + te.Policies = policyutil.SanitizePolicies(policies, addDefault) // Yes, this is a little inefficient to do it like this, but meh - if data.NoDefaultPolicy { + if noDefaultPolicy { te.Policies = strutil.StrListDelete(te.Policies, "default") } @@ -3125,7 +3002,7 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque te.BoundCIDRs = role.TokenBoundCIDRs } - case data.NoParent: + case d.Get("no_parent").(bool): // Only allow an orphan token if the client has sudo policy if !isSudo { return logical.ErrorResponse("root or sudo privileges required to create orphan token"), @@ -3162,8 +3039,8 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque } var explicitMaxTTLToUse time.Duration - if data.ExplicitMaxTTL != "" { - dur, err := parseutil.ParseDurationSecond(data.ExplicitMaxTTL) + if explicitMaxTTL != "" { + dur, err := parseutil.ParseDurationSecond(explicitMaxTTL) if err != nil { return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest } @@ -3175,8 +3052,8 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque } var periodToUse time.Duration - if data.Period != "" { - dur, err := parseutil.ParseDurationSecond(data.Period) + if period != "" { + dur, err := parseutil.ParseDurationSecond(period) if err != nil { return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest } @@ -3196,8 +3073,8 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque } // Parse the TTL/lease if any - if data.TTL != "" { - dur, err := parseutil.ParseDurationSecond(data.TTL) + if ttl := d.Get("ttl").(string); ttl != "" { + dur, err := parseutil.ParseDurationSecond(ttl) if err != nil { return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest } @@ -3205,9 +3082,9 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque return logical.ErrorResponse("ttl must be positive"), logical.ErrInvalidRequest } te.TTL = dur - } else if data.Lease != "" { + } else if lease := d.Get("lease").(string); lease != "" { // This block is compatibility - dur, err := parseutil.ParseDurationSecond(data.Lease) + dur, err := parseutil.ParseDurationSecond(lease) if err != nil { return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest }