Skip to content

Commit

Permalink
VAULT-8518 Increase HMAC limit to 4096, and limit approle names to th…
Browse files Browse the repository at this point in the history
…e same limit (hashicorp#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
  • Loading branch information
VioletHynes authored and jayant07-yb committed Mar 15, 2023
1 parent 0257045 commit ae68173
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 33 deletions.
44 changes: 24 additions & 20 deletions builtin/credential/approle/path_role.go
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
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
@@ -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
```
47 changes: 47 additions & 0 deletions command/agent/approle_end_to_end_test.go
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
// 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
Expand Down
24 changes: 12 additions & 12 deletions website/content/api-docs/auth/approle.mdx
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

0 comments on commit ae68173

Please sign in to comment.