From ae681738fb1b5a442e9510e5757cdd3661dc8b49 Mon Sep 17 00:00:00 2001 From: Violet Hynes Date: Wed, 2 Nov 2022 10:42:09 -0400 Subject: [PATCH] VAULT-8518 Increase HMAC limit to 4096, and limit approle names to the same limit (#17768) * VAULT-8518 Increase HMAC limit to 4096, and limit approle names to the same limit * VAULT-8518 Changelog * VAULT-8518 Sprintf the byte limit --- builtin/credential/approle/path_role.go | 44 +++++++++++---------- builtin/credential/approle/validation.go | 2 +- changelog/17768.txt | 3 ++ command/agent/approle_end_to_end_test.go | 47 +++++++++++++++++++++++ website/content/api-docs/auth/approle.mdx | 24 ++++++------ 5 files changed, 87 insertions(+), 33 deletions(-) create mode 100644 changelog/17768.txt diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go index b079a9ca6fa6f..20639277a9226 100644 --- a/builtin/credential/approle/path_role.go +++ b/builtin/credential/approle/path_role.go @@ -113,7 +113,7 @@ func rolePaths(b *backend) []*framework.Path { Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "bind_secret_id": { Type: framework.TypeBool, @@ -196,7 +196,7 @@ can only be set during role creation and once set, it can't be reset later.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -210,7 +210,7 @@ can only be set during role creation and once set, it can't be reset later.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "policies": { Type: framework.TypeCommaStringSlice, @@ -235,7 +235,7 @@ can only be set during role creation and once set, it can't be reset later.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "bound_cidr_list": { Type: framework.TypeCommaStringSlice, @@ -256,7 +256,7 @@ of CIDR blocks. If set, specifies the blocks of IP addresses which can perform t Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "secret_id_bound_cidrs": { Type: framework.TypeCommaStringSlice, @@ -277,7 +277,7 @@ IP addresses which can perform the login operation.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "token_bound_cidrs": { Type: framework.TypeCommaStringSlice, @@ -297,7 +297,7 @@ IP addresses which can perform the login operation.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "bind_secret_id": { Type: framework.TypeBool, @@ -318,7 +318,7 @@ IP addresses which can perform the login operation.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "secret_id_num_uses": { Type: framework.TypeInt, @@ -338,7 +338,7 @@ IP addresses which can perform the login operation.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "secret_id_ttl": { Type: framework.TypeDurationSecond, @@ -359,7 +359,7 @@ to 0, meaning no expiration.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "period": { Type: framework.TypeDurationSecond, @@ -384,7 +384,7 @@ to 0, meaning no expiration.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "token_num_uses": { Type: framework.TypeInt, @@ -404,7 +404,7 @@ to 0, meaning no expiration.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "token_ttl": { Type: framework.TypeDurationSecond, @@ -424,7 +424,7 @@ to 0, meaning no expiration.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "token_max_ttl": { Type: framework.TypeDurationSecond, @@ -444,7 +444,7 @@ to 0, meaning no expiration.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "role_id": { Type: framework.TypeString, @@ -463,7 +463,7 @@ to 0, meaning no expiration.`, Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "metadata": { Type: framework.TypeString, @@ -504,7 +504,7 @@ Overrides secret_id_ttl role option when supplied. May not be longer than role's Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "secret_id": { Type: framework.TypeString, @@ -522,7 +522,7 @@ Overrides secret_id_ttl role option when supplied. May not be longer than role's Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "secret_id": { Type: framework.TypeString, @@ -541,7 +541,7 @@ Overrides secret_id_ttl role option when supplied. May not be longer than role's Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "secret_id_accessor": { Type: framework.TypeString, @@ -559,7 +559,7 @@ Overrides secret_id_ttl role option when supplied. May not be longer than role's Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "secret_id_accessor": { Type: framework.TypeString, @@ -578,7 +578,7 @@ Overrides secret_id_ttl role option when supplied. May not be longer than role's Fields: map[string]*framework.FieldSchema{ "role_name": { Type: framework.TypeString, - Description: "Name of the role.", + Description: fmt.Sprintf("Name of the role. Must be less than %d bytes.", maxHmacInputLength), }, "secret_id": { Type: framework.TypeString, @@ -880,6 +880,10 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request return logical.ErrorResponse("missing role_name"), nil } + if len(roleName) > maxHmacInputLength { + return logical.ErrorResponse(fmt.Sprintf("role_name is longer than maximum of %d bytes", maxHmacInputLength)), nil + } + lock := b.roleLock(roleName) lock.Lock() defer lock.Unlock() diff --git a/builtin/credential/approle/validation.go b/builtin/credential/approle/validation.go index 7a129b99a078a..25f343675d02a 100644 --- a/builtin/credential/approle/validation.go +++ b/builtin/credential/approle/validation.go @@ -92,7 +92,7 @@ func verifyCIDRRoleSecretIDSubset(secretIDCIDRs []string, roleBoundCIDRList []st return nil } -const maxHmacInputLength = 1024 +const maxHmacInputLength = 4096 // Creates a SHA256 HMAC of the given 'value' using the given 'key' and returns // a hex encoded string. diff --git a/changelog/17768.txt b/changelog/17768.txt new file mode 100644 index 0000000000000..25246ea08ea4d --- /dev/null +++ b/changelog/17768.txt @@ -0,0 +1,3 @@ +```release-note:change +auth/approle: Add maximum length of 4096 for approle role_names, as this value results in HMAC calculation +``` \ No newline at end of file diff --git a/command/agent/approle_end_to_end_test.go b/command/agent/approle_end_to_end_test.go index 2c3e52dfb3a4d..e3456b3b5c747 100644 --- a/command/agent/approle_end_to_end_test.go +++ b/command/agent/approle_end_to_end_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "os" + "strings" "testing" "time" @@ -400,6 +401,52 @@ func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool, bindSecretID boo } } +// TestAppRoleLongRoleName tests that the creation of an approle is a maximum of 4096 bytes +// Prior to VAULT-8518 being fixed, you were unable to delete an approle value longer than 1024 bytes +// due to a restriction put into place by PR #14746, to prevent unbounded HMAC creation. +func TestAppRoleLongRoleName(t *testing.T) { + approleName := strings.Repeat("a", 5000) + + coreConfig := &vault.CoreConfig{ + DisableMlock: true, + DisableCache: true, + Logger: log.NewNullLogger(), + CredentialBackends: map[string]logical.Factory{ + "approle": credAppRole.Factory, + }, + } + + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + + cluster.Start() + defer cluster.Cleanup() + + cores := cluster.Cores + + vault.TestWaitActive(t, cores[0].Core) + + client := cores[0].Client + + err := client.Sys().EnableAuthWithOptions("approle", &api.EnableAuthOptions{ + Type: "approle", + }) + if err != nil { + t.Fatal(err) + } + + _, err = client.Logical().Write(fmt.Sprintf("auth/approle/role/%s", approleName), map[string]interface{}{ + "token_ttl": "6s", + "token_max_ttl": "10s", + }) + if err != nil { + if !strings.Contains(err.Error(), "role_name is longer than maximum") { + t.Fatal(err) + } + } +} + func TestAppRoleWithWrapping(t *testing.T) { testCases := []struct { bindSecretID bool diff --git a/website/content/api-docs/auth/approle.mdx b/website/content/api-docs/auth/approle.mdx index 4f14d2f353288..2ecfae6d7eba5 100644 --- a/website/content/api-docs/auth/approle.mdx +++ b/website/content/api-docs/auth/approle.mdx @@ -60,7 +60,7 @@ enabled while creating or updating a role. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. - `bind_secret_id` `(bool: true)` - Require `secret_id` to be presented when logging in using this AppRole. - `secret_id_bound_cidrs` `(array: [])` - Comma-separated string or list of CIDR @@ -112,7 +112,7 @@ Reads the properties of an existing AppRole. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. ### Sample Request @@ -155,7 +155,7 @@ Deletes an existing AppRole from the method. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. ### Sample Request @@ -176,7 +176,7 @@ Reads the RoleID of an existing AppRole. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. ### Sample Request @@ -212,7 +212,7 @@ Updates the RoleID of an existing AppRole to a custom value. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. - `role_id` `(string: )` - Value to be set as RoleID. ### Sample Payload @@ -262,7 +262,7 @@ itself, and also to delete the SecretID from the AppRole. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. - `metadata` `(string: "")` - Metadata to be tied to the SecretID. This should be a JSON-formatted string containing the metadata in key-value pairs. This metadata will be set on tokens issued with this SecretID, and is logged in @@ -333,7 +333,7 @@ This includes the accessors for "custom" SecretIDs as well. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. ### Sample Request @@ -376,7 +376,7 @@ Reads out the properties of a SecretID. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. - `secret_id` `(string: )` - Secret ID attached to the role. ### Sample Payload @@ -407,7 +407,7 @@ Destroy an AppRole secret ID. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. - `secret_id` `(string: )` - Secret ID attached to the role. ### Sample Payload @@ -438,7 +438,7 @@ Reads out the properties of a SecretID. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. - `secret_id_accessor` `(string: )` - Secret ID accessor attached to the role. ### Sample Payload @@ -469,7 +469,7 @@ Destroy an AppRole secret ID by its accessor. ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. - `secret_id_accessor` `(string: )` - Secret ID accessor attached to the role. ### Sample Payload @@ -501,7 +501,7 @@ Assigns a "custom" SecretID against an existing AppRole. This is used in the ### Parameters -- `role_name` `(string: )` - Name of the AppRole. +- `role_name` `(string: )` - Name of the AppRole. Must be less than 4096 bytes. - `secret_id` `(string: )` - SecretID to be attached to the Role. - `metadata` `(string: "")` - Metadata to be tied to the SecretID. This should be a JSON-formatted string containing the metadata in key-value pairs. This