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.

Sign up for GitHub

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

Jump to bottom

VAULT-8518 Increase HMAC limit to 4096, and limit approle names to the same limit #17768

Merged
merged 3 commits into from Nov 2, 2022
Merged

VAULT-8518 Increase HMAC limit to 4096, and limit approle names to the same limit #17768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f48ccd
VAULT-8518 Increase HMAC limit to 4096, and limit approle names to th…
VioletHynes Nov 1, 2022
1a92a35
VAULT-8518 Changelog
VioletHynes Nov 1, 2022
60a9bde
VAULT-8518 Sprintf the byte limit
VioletHynes Nov 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 24 additions & 20 deletions builtin/credential/approle/path_role.go
View file
Open in desktop
Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion builtin/credential/approle/validation.go
View file
Open in desktop
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions changelog/17768.txt
View file
Open in desktop
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am bit confused by this phrase. Do you mean that HMAC result is of length 4096?

```
47 changes: 47 additions & 0 deletions command/agent/approle_end_to_end_test.go
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io/ioutil"
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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
VioletHynes marked this conversation as resolved.
Show resolved Hide resolved
// 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{
mpalmi marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
24 changes: 12 additions & 12 deletions website/content/api-docs/auth/approle.mdx
View file
Open in desktop
Expand Up @@ -60,7 +60,7 @@ enabled while creating or updating a role.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - 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
Expand Down Expand Up @@ -112,7 +112,7 @@ Reads the properties of an existing AppRole.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.

### Sample Request

Expand Down Expand Up @@ -155,7 +155,7 @@ Deletes an existing AppRole from the method.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.

### Sample Request

Expand All @@ -176,7 +176,7 @@ Reads the RoleID of an existing AppRole.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.

### Sample Request

Expand Down Expand Up @@ -212,7 +212,7 @@ Updates the RoleID of an existing AppRole to a custom value.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.
- `role_id` `(string: <required>)` - Value to be set as RoleID.

### Sample Payload
Expand Down Expand Up @@ -262,7 +262,7 @@ itself, and also to delete the SecretID from the AppRole.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - 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
Expand Down Expand Up @@ -333,7 +333,7 @@ This includes the accessors for "custom" SecretIDs as well.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.

### Sample Request

Expand Down Expand Up @@ -376,7 +376,7 @@ Reads out the properties of a SecretID.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.
- `secret_id` `(string: <required>)` - Secret ID attached to the role.

### Sample Payload
Expand Down Expand Up @@ -407,7 +407,7 @@ Destroy an AppRole secret ID.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.
- `secret_id` `(string: <required>)` - Secret ID attached to the role.

### Sample Payload
Expand Down Expand Up @@ -438,7 +438,7 @@ Reads out the properties of a SecretID.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.
- `secret_id_accessor` `(string: <required>)` - Secret ID accessor attached to the role.

### Sample Payload
Expand Down Expand Up @@ -469,7 +469,7 @@ Destroy an AppRole secret ID by its accessor.

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.
- `secret_id_accessor` `(string: <required>)` - Secret ID accessor attached to the role.

### Sample Payload
Expand Down Expand Up @@ -501,7 +501,7 @@ Assigns a "custom" SecretID against an existing AppRole. This is used in the

### Parameters

- `role_name` `(string: <required>)` - Name of the AppRole.
- `role_name` `(string: <required>)` - Name of the AppRole. Must be less than 4096 bytes.
- `secret_id` `(string: <required>)` - 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
Expand Down