diff --git a/CHANGELOG.md b/CHANGELOG.md index ac723e2d78f0..8c9663efd281 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ SECURITY: - * The Vault team identified a race condition that could occur if a token's + * Tokens: A race condition was identified that could occur if a token's lease expired while Vault was not running. In this case, when Vault came back online, sometimes it would properly revoke the lease but other times it would not, leading to a Vault token that no longer had an expiration and had @@ -12,6 +12,11 @@ SECURITY: future issues. In addition, the logic we have put in place ensures that such lease-less tokens can no longer be used (unless they are root tokens that never had an expiration to begin with). + * AppRole case-sensitive role name secret-id leaking: When using a mixed-case + role name via AppRole, deleting a secret-id via accessor or other operations + could end up leaving the secret-id behind and valid but without an accessor. + This has now been fixed, and we have put checks in place to prevent these + secret-ids from being used. DEPRECATIONS/CHANGES: diff --git a/builtin/credential/approle/backend_test.go b/builtin/credential/approle/backend_test.go index b9ee1ab1bf5f..35d4b5544031 100644 --- a/builtin/credential/approle/backend_test.go +++ b/builtin/credential/approle/backend_test.go @@ -2,6 +2,7 @@ package approle import ( "context" + "strings" "testing" "github.com/hashicorp/vault/logical" @@ -24,3 +25,334 @@ func createBackendWithStorage(t *testing.T) (*backend, logical.Storage) { } return b, config.StorageView } + +func TestAppRole_RoleNameCaseSensitivity(t *testing.T) { + testFunc := func(t *testing.T, roleName string) { + var resp *logical.Response + var err error + b, s := createBackendWithStorage(t) + + // Create the role + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + roleName, + Operation: logical.CreateOperation, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr:%v", resp, err) + } + + // Get the role-id + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + roleName + "/role-id", + Operation: logical.ReadOperation, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + roleID := resp.Data["role_id"] + + // Create a secret-id + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + roleName + "/secret-id", + Operation: logical.UpdateOperation, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + secretID := resp.Data["secret_id"] + secretIDAccessor := resp.Data["secret_id_accessor"] + + // Ensure login works + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + if resp.Auth == nil { + t.Fatalf("failed to perform login") + } + + // Destroy secret ID accessor + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + roleName + "/secret-id-accessor/destroy", + Operation: logical.UpdateOperation, + Storage: s, + Data: map[string]interface{}{ + "secret_id_accessor": secretIDAccessor, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + + // Login again using the accessor's corresponding secret ID should fail + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatalf("expected error due to invalid secret ID") + } + + // Generate another secret ID + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + roleName + "/secret-id", + Operation: logical.UpdateOperation, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + secretID = resp.Data["secret_id"] + secretIDAccessor = resp.Data["secret_id_accessor"] + + // Ensure login works + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + if resp.Auth == nil { + t.Fatalf("failed to perform login") + } + + // Destroy the secret ID + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + roleName + "/secret-id/destroy", + Operation: logical.UpdateOperation, + Storage: s, + Data: map[string]interface{}{ + "secret_id": secretID, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + + // Login again using the same secret ID should fail + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatalf("expected error due to invalid secret ID") + } + + // Generate another secret ID + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + roleName + "/secret-id", + Operation: logical.UpdateOperation, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + secretID = resp.Data["secret_id"] + secretIDAccessor = resp.Data["secret_id_accessor"] + + // Ensure login works + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + if resp.Auth == nil { + t.Fatalf("failed to perform login") + } + + // Destroy the secret ID using lower cased role name + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + strings.ToLower(roleName) + "/secret-id/destroy", + Operation: logical.UpdateOperation, + Storage: s, + Data: map[string]interface{}{ + "secret_id": secretID, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + + // Login again using the same secret ID should fail + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatalf("expected error due to invalid secret ID") + } + + // Generate another secret ID + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + roleName + "/secret-id", + Operation: logical.UpdateOperation, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + secretID = resp.Data["secret_id"] + secretIDAccessor = resp.Data["secret_id_accessor"] + + // Ensure login works + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + if resp.Auth == nil { + t.Fatalf("failed to perform login") + } + + // Destroy the secret ID using upper cased role name + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + strings.ToUpper(roleName) + "/secret-id/destroy", + Operation: logical.UpdateOperation, + Storage: s, + Data: map[string]interface{}{ + "secret_id": secretID, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + + // Login again using the same secret ID should fail + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatalf("expected error due to invalid secret ID") + } + + // Generate another secret ID + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/" + roleName + "/secret-id", + Operation: logical.UpdateOperation, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + secretID = resp.Data["secret_id"] + secretIDAccessor = resp.Data["secret_id_accessor"] + + // Ensure login works + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + if resp.Auth == nil { + t.Fatalf("failed to perform login") + } + + // Destroy the secret ID using mixed case name + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/saMpleRolEnaMe/secret-id/destroy", + Operation: logical.UpdateOperation, + Storage: s, + Data: map[string]interface{}{ + "secret_id": secretID, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + + // Login again using the same secret ID should fail + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + Storage: s, + }) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatalf("expected error due to invalid secret ID") + } + } + + // Lower case role name + testFunc(t, "samplerolename") + // Upper case role name + testFunc(t, "SAMPLEROLENAME") + // Mixed case role name + testFunc(t, "SampleRoleName") +} diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index 71e5afb05442..fce24f540ba5 100644 --- a/builtin/credential/approle/path_login.go +++ b/builtin/credential/approle/path_login.go @@ -66,7 +66,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat return nil, err } if roleIDIndex == nil { - return logical.ErrorResponse(fmt.Sprintf("invalid role_id %q", roleID)), nil + return logical.ErrorResponse("invalid role ID"), nil } roleName := roleIDIndex.Name @@ -80,7 +80,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat return nil, err } if role == nil { - return logical.ErrorResponse(fmt.Sprintf("invalid role_id %q", roleID)), nil + return logical.ErrorResponse("invalid role ID"), nil } var metadata map[string]string @@ -90,16 +90,12 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat return logical.ErrorResponse("missing secret_id"), nil } - if role.LowerCaseRoleName { - roleName = strings.ToLower(roleName) - } - secretIDHMAC, err := createHMAC(role.HMACKey, secretID) if err != nil { return nil, errwrap.Wrapf("failed to create HMAC of secret_id: {{err}}", err) } - roleNameHMAC, err := createHMAC(role.HMACKey, roleName) + roleNameHMAC, err := createHMAC(role.HMACKey, role.name) if err != nil { return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } @@ -109,18 +105,53 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat secretIDLock := b.secretIDLock(secretIDHMAC) secretIDLock.RLock() + unlockFunc := secretIDLock.RUnlock + defer func() { + unlockFunc() + }() + entry, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) if err != nil { - secretIDLock.RUnlock() return nil, err } else if entry == nil { + return logical.ErrorResponse("invalid secret id"), nil + } + + // If a secret ID entry does not have a corresponding accessor + // entry, revoke the secret ID immediately + accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, entry.SecretIDAccessor, role.SecretIDPrefix) + if err != nil { + return nil, errwrap.Wrapf("failed to read secret ID accessor entry: {{err}}", err) + } + if accessorEntry == nil { + // Switch the locks and recheck the conditions secretIDLock.RUnlock() - return logical.ErrorResponse(fmt.Sprintf("invalid secret_id %q", secretID)), nil + secretIDLock.Lock() + unlockFunc = secretIDLock.Unlock + + entry, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) + if err != nil { + return nil, err + } + if entry == nil { + return logical.ErrorResponse("invalid secret id"), nil + } + + accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, entry.SecretIDAccessor, role.SecretIDPrefix) + if err != nil { + return nil, errwrap.Wrapf("failed to read secret ID accessor entry: {{err}}", err) + } + + if accessorEntry == nil { + if err := req.Storage.Delete(ctx, entryIndex); err != nil { + return nil, errwrap.Wrapf(fmt.Sprintf("error deleting secret ID %q from storage: {{err}}", secretIDHMAC), err) + } + } + return logical.ErrorResponse("invalid secret id"), nil } switch { case entry.SecretIDNumUses == 0: - defer secretIDLock.RUnlock() // // SecretIDNumUses will be zero only if the usage limit was not set at all, // in which case, the SecretID will remain to be valid as long as it is not @@ -155,7 +186,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat secretIDLock.RUnlock() secretIDLock.Lock() - defer secretIDLock.Unlock() + unlockFunc = secretIDLock.Unlock // Lock switching may change the data. Refresh the contents. entry, err = b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) @@ -231,13 +262,13 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat } // Always include the role name, for later filtering - metadata["role_name"] = roleName + metadata["role_name"] = role.name auth := &logical.Auth{ NumUses: role.TokenNumUses, Period: role.Period, InternalData: map[string]interface{}{ - "role_name": roleName, + "role_name": role.name, }, Metadata: metadata, Policies: role.Policies, @@ -268,7 +299,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, data defer lock.RUnlock() // Ensure that the Role still exists. - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, errwrap.Wrapf(fmt.Sprintf("failed to validate role %q during renewal: {{err}}", roleName), err) } diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go index 4c057b6d3561..8fc93a80bceb 100644 --- a/builtin/credential/approle/path_role.go +++ b/builtin/credential/approle/path_role.go @@ -6,7 +6,6 @@ import ( "strings" "time" - "github.com/fatih/structs" "github.com/hashicorp/errwrap" "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/helper/cidrutil" @@ -20,62 +19,67 @@ import ( // roleStorageEntry stores all the options that are set on an role type roleStorageEntry struct { + // Name of the role. This field is not persisted on disk. After the role is + // read out of disk, the sanitized version of name is set in this field for + // subsequent use of role name elsewhere. + name string + // UUID that uniquely represents this role. This serves as a credential // to perform login using this role. - RoleID string `json:"role_id" structs:"role_id" mapstructure:"role_id"` + RoleID string `json:"role_id" mapstructure:"role_id"` // UUID that serves as the HMAC key for the hashing the 'secret_id's // of the role - HMACKey string `json:"hmac_key" structs:"hmac_key" mapstructure:"hmac_key"` + HMACKey string `json:"hmac_key" mapstructure:"hmac_key"` // Policies that are to be required by the token to access this role - Policies []string `json:"policies" structs:"policies" mapstructure:"policies"` + Policies []string `json:"policies" mapstructure:"policies"` // Number of times the SecretID generated against this role can be // used to perform login operation - SecretIDNumUses int `json:"secret_id_num_uses" structs:"secret_id_num_uses" mapstructure:"secret_id_num_uses"` + SecretIDNumUses int `json:"secret_id_num_uses" mapstructure:"secret_id_num_uses"` // Duration (less than the backend mount's max TTL) after which a // SecretID generated against the role will expire - SecretIDTTL time.Duration `json:"secret_id_ttl" structs:"secret_id_ttl" mapstructure:"secret_id_ttl"` + SecretIDTTL time.Duration `json:"secret_id_ttl" mapstructure:"secret_id_ttl"` // TokenNumUses defines the number of allowed uses of the token issued - TokenNumUses int `json:"token_num_uses" mapstructure:"token_num_uses" structs:"token_num_uses"` + TokenNumUses int `json:"token_num_uses" mapstructure:"token_num_uses"` // Duration before which an issued token must be renewed - TokenTTL time.Duration `json:"token_ttl" structs:"token_ttl" mapstructure:"token_ttl"` + TokenTTL time.Duration `json:"token_ttl" mapstructure:"token_ttl"` // Duration after which an issued token should not be allowed to be renewed - TokenMaxTTL time.Duration `json:"token_max_ttl" structs:"token_max_ttl" mapstructure:"token_max_ttl"` + TokenMaxTTL time.Duration `json:"token_max_ttl" mapstructure:"token_max_ttl"` // A constraint, if set, requires 'secret_id' credential to be presented during login - BindSecretID bool `json:"bind_secret_id" structs:"bind_secret_id" mapstructure:"bind_secret_id"` + BindSecretID bool `json:"bind_secret_id" mapstructure:"bind_secret_id"` // A constraint, if set, specifies the CIDR blocks from which logins should be allowed BoundCIDRListOld string `json:"bound_cidr_list,omitempty"` // A constraint, if set, specifies the CIDR blocks from which logins should be allowed - BoundCIDRList []string `json:"bound_cidr_list_list" structs:"bound_cidr_list" mapstructure:"bound_cidr_list"` + BoundCIDRList []string `json:"bound_cidr_list_list" mapstructure:"bound_cidr_list"` // Period, if set, indicates that the token generated using this role // should never expire. The token should be renewed within the duration // specified by this value. The renewal duration will be fixed if the // value is not modified on the role. If the `Period` in the role is modified, // a token will pick up the new value during its next renewal. - Period time.Duration `json:"period" mapstructure:"period" structs:"period"` + Period time.Duration `json:"period" mapstructure:"period"` // LowerCaseRoleName enforces the lower casing of role names for all the // roles that get created since this field was introduced. - LowerCaseRoleName bool `json:"lower_case_role_name" mapstructure:"lower_case_role_name" structs:"lower_case_role_name"` + LowerCaseRoleName bool `json:"lower_case_role_name" mapstructure:"lower_case_role_name"` // SecretIDPrefix is the storage prefix for persisting secret IDs. This // differs based on whether the secret IDs are cluster local or not. - SecretIDPrefix string `json:"secret_id_prefix" mapstructure:"secret_id_prefix" structs:"secret_id_prefix"` + SecretIDPrefix string `json:"secret_id_prefix" mapstructure:"secret_id_prefix"` } // roleIDStorageEntry represents the reverse mapping from RoleID to Role type roleIDStorageEntry struct { - Name string `json:"name" structs:"name" mapstructure:"name"` + Name string `json:"name" mapstructure:"name"` } // rolePaths creates all the paths that are used to register and manage an role. @@ -551,7 +555,7 @@ func (b *backend) pathRoleExistenceCheck(ctx context.Context, req *logical.Reque lock.RLock() defer lock.RUnlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return false, err } @@ -585,7 +589,7 @@ func (b *backend) pathRoleSecretIDList(ctx context.Context, req *logical.Request defer lock.RUnlock() // Get the role entry - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -593,15 +597,11 @@ func (b *backend) pathRoleSecretIDList(ctx context.Context, req *logical.Request return logical.ErrorResponse(fmt.Sprintf("role %q does not exist", roleName)), nil } - if role.LowerCaseRoleName { - roleName = strings.ToLower(roleName) - } - // Guard the list operation with an outer lock b.secretIDListingLock.RLock() defer b.secretIDListingLock.RUnlock() - roleNameHMAC, err := createHMAC(role.HMACKey, roleName) + roleNameHMAC, err := createHMAC(role.HMACKey, role.name) if err != nil { return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } @@ -772,6 +772,11 @@ func (b *backend) roleEntry(ctx context.Context, s logical.Storage, roleName str } } + role.name = roleName + if role.LowerCaseRoleName { + role.name = strings.ToLower(roleName) + } + return &role, nil } @@ -794,16 +799,18 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request } // Create a new entry object if this is a CreateOperation - if role == nil && req.Operation == logical.CreateOperation { + switch { + case role == nil && req.Operation == logical.CreateOperation: hmacKey, err := uuid.GenerateUUID() if err != nil { return nil, errwrap.Wrapf("failed to create role_id: {{err}}", err) } role = &roleStorageEntry{ + name: strings.ToLower(roleName), HMACKey: hmacKey, LowerCaseRoleName: true, } - } else if role == nil { + case role == nil: return logical.ErrorResponse(fmt.Sprintf("role name %q doesn't exist", roleName)), nil } @@ -922,7 +929,7 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request } // Store the entry. - return resp, b.setRoleEntry(ctx, req.Storage, roleName, role, previousRoleID) + return resp, b.setRoleEntry(ctx, req.Storage, role.name, role, previousRoleID) } // pathRoleRead grabs a read lock and reads the options set on the role from the storage @@ -936,7 +943,7 @@ func (b *backend) pathRoleRead(ctx context.Context, req *logical.Request, data * lock.RLock() lockRelease := lock.RUnlock - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { lockRelease() return nil, err @@ -996,7 +1003,7 @@ func (b *backend) pathRoleRead(ctx context.Context, req *logical.Request, data * if roleIDIndex == nil { // Create a new index err = b.setRoleIDEntry(ctx, req.Storage, role.RoleID, &roleIDStorageEntry{ - Name: roleName, + Name: role.name, }) if err != nil { lockRelease() @@ -1022,7 +1029,7 @@ func (b *backend) pathRoleDelete(ctx context.Context, req *logical.Request, data lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1031,17 +1038,17 @@ func (b *backend) pathRoleDelete(ctx context.Context, req *logical.Request, data } // Just before the role is deleted, remove all the SecretIDs issued as part of the role. - if err = b.flushRoleSecrets(ctx, req.Storage, roleName, role.HMACKey, role.SecretIDPrefix); err != nil { - return nil, errwrap.Wrapf(fmt.Sprintf("failed to invalidate the secrets belonging to role %q: {{err}}", roleName), err) + if err = b.flushRoleSecrets(ctx, req.Storage, role.name, role.HMACKey, role.SecretIDPrefix); err != nil { + return nil, errwrap.Wrapf(fmt.Sprintf("failed to invalidate the secrets belonging to role %q: {{err}}", role.name), err) } // Delete the reverse mapping from RoleID to the role if err = b.roleIDEntryDelete(ctx, req.Storage, role.RoleID); err != nil { - return nil, errwrap.Wrapf(fmt.Sprintf("failed to delete the mapping from RoleID to role %q: {{err}}", roleName), err) + return nil, errwrap.Wrapf(fmt.Sprintf("failed to delete the mapping from RoleID to role %q: {{err}}", role.name), err) } // After deleting the SecretIDs and the RoleID, delete the role itself - if err = req.Storage.Delete(ctx, "role/"+strings.ToLower(roleName)); err != nil { + if err = req.Storage.Delete(ctx, "role/"+strings.ToLower(role.name)); err != nil { return nil, err } @@ -1065,7 +1072,7 @@ func (b *backend) pathRoleSecretIDLookupUpdate(ctx context.Context, req *logical defer lock.RUnlock() // Fetch the role - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1073,10 +1080,6 @@ func (b *backend) pathRoleSecretIDLookupUpdate(ctx context.Context, req *logical return nil, fmt.Errorf("role %q does not exist", roleName) } - if role.LowerCaseRoleName { - roleName = strings.ToLower(roleName) - } - // Create the HMAC of the secret ID using the per-role HMAC key secretIDHMAC, err := createHMAC(role.HMACKey, secretID) if err != nil { @@ -1084,7 +1087,7 @@ func (b *backend) pathRoleSecretIDLookupUpdate(ctx context.Context, req *logical } // Create the HMAC of the roleName using the per-role HMAC key - roleNameHMAC, err := createHMAC(role.HMACKey, roleName) + roleNameHMAC, err := createHMAC(role.HMACKey, role.name) if err != nil { return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } @@ -1092,44 +1095,47 @@ func (b *backend) pathRoleSecretIDLookupUpdate(ctx context.Context, req *logical // Create the index at which the secret_id would've been stored entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) - return b.secretIDCommon(ctx, req.Storage, entryIndex, secretIDHMAC) -} + secretLock := b.secretIDLock(secretIDHMAC) + secretLock.Lock() + defer secretLock.Unlock() -func (b *backend) secretIDCommon(ctx context.Context, s logical.Storage, entryIndex, secretIDHMAC string) (*logical.Response, error) { - lock := b.secretIDLock(secretIDHMAC) - lock.RLock() - defer lock.RUnlock() - - result := secretIDStorageEntry{} - if entry, err := s.Get(ctx, entryIndex); err != nil { + secretIDEntry, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) + if err != nil { return nil, err - } else if entry == nil { + } + if secretIDEntry == nil { return nil, nil - } else if err := entry.DecodeJSON(&result); err != nil { - return nil, err } - result.SecretIDTTL /= time.Second - d := structs.New(result).Map() - - // Converting the time values to RFC3339Nano format. - // - // Map() from 'structs' package formats time in RFC3339Nano. - // In order to not break the API due to a modification in the - // third party package, converting the time values again. - d["creation_time"] = result.CreationTime.Format(time.RFC3339Nano) - d["expiration_time"] = result.ExpirationTime.Format(time.RFC3339Nano) - d["last_updated_time"] = result.LastUpdatedTime.Format(time.RFC3339Nano) - - resp := &logical.Response{ - Data: d, + // If a secret ID entry does not have a corresponding accessor + // entry, revoke the secret ID immediately + accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDEntry.SecretIDAccessor, role.SecretIDPrefix) + if err != nil { + return nil, errwrap.Wrapf("failed to read secret ID accessor entry: {{err}}", err) } - - if _, ok := d["SecretIDNumUses"]; ok { - resp.AddWarning("The field SecretIDNumUses is deprecated and will be removed in a future release; refer to secret_id_num_uses instead") + if accessorEntry == nil { + if err := req.Storage.Delete(ctx, entryIndex); err != nil { + return nil, errwrap.Wrapf(fmt.Sprintf("error deleting secret ID %q from storage: {{err}}", secretIDHMAC), err) + } + return logical.ErrorResponse("invalid secret id"), nil } - return resp, nil + return &logical.Response{ + Data: secretIDEntry.ToResponseData(), + }, nil +} + +func (entry *secretIDStorageEntry) ToResponseData() map[string]interface{} { + return map[string]interface{}{ + "secret_id_accessor": entry.SecretIDAccessor, + "secret_id_num_uses": entry.SecretIDNumUses, + "secret_id_ttl": entry.SecretIDTTL / time.Second, + "creation_time": entry.CreationTime, + "expiration_time": entry.ExpirationTime, + "last_updated_time": entry.LastUpdatedTime, + "metadata": entry.Metadata, + "cidr_list": entry.CIDRList, + } } func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1147,7 +1153,7 @@ func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req * roleLock.RLock() defer roleLock.RUnlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1160,7 +1166,7 @@ func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req * return nil, errwrap.Wrapf("failed to create HMAC of secret_id: {{err}}", err) } - roleNameHMAC, err := createHMAC(role.HMACKey, roleName) + roleNameHMAC, err := createHMAC(role.HMACKey, role.name) if err != nil { return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } @@ -1171,17 +1177,16 @@ func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req * lock.Lock() defer lock.Unlock() - result := secretIDStorageEntry{} - if entry, err := req.Storage.Get(ctx, entryIndex); err != nil { + entry, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) + if err != nil { return nil, err - } else if entry == nil { + } + if entry == nil { return nil, nil - } else if err := entry.DecodeJSON(&result); err != nil { - return nil, err } // Delete the accessor of the SecretID first - if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor, role.SecretIDPrefix); err != nil { + if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, entry.SecretIDAccessor, role.SecretIDPrefix); err != nil { return nil, err } @@ -1214,7 +1219,7 @@ func (b *backend) pathRoleSecretIDAccessorLookupUpdate(ctx context.Context, req lock.RLock() defer lock.RUnlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1230,14 +1235,26 @@ func (b *backend) pathRoleSecretIDAccessorLookupUpdate(ctx context.Context, req return nil, fmt.Errorf("failed to find accessor entry for secret_id_accessor: %q", secretIDAccessor) } - roleNameHMAC, err := createHMAC(role.HMACKey, roleName) + roleNameHMAC, err := createHMAC(role.HMACKey, role.name) if err != nil { return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } - entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC) + secretLock := b.secretIDLock(accessorEntry.SecretIDHMAC) + secretLock.RLock() + defer secretLock.RUnlock() - return b.secretIDCommon(ctx, req.Storage, entryIndex, accessorEntry.SecretIDHMAC) + secretIDEntry, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC) + if err != nil { + return nil, err + } + if secretIDEntry == nil { + return nil, nil + } + + return &logical.Response{ + Data: secretIDEntry.ToResponseData(), + }, nil } func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1255,7 +1272,7 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex // Get the role details to fetch the RoleID and accessor to get // the HMACed SecretID. - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1271,7 +1288,7 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex return nil, fmt.Errorf("failed to find accessor entry for secret_id_accessor: %q", secretIDAccessor) } - roleNameHMAC, err := createHMAC(role.HMACKey, roleName) + roleNameHMAC, err := createHMAC(role.HMACKey, role.name) if err != nil { return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } @@ -1306,7 +1323,7 @@ func (b *backend) pathRoleBoundCIDRListUpdate(ctx context.Context, req *logical. defer lock.Unlock() // Re-read the role after grabbing the lock - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1327,7 +1344,7 @@ func (b *backend) pathRoleBoundCIDRListUpdate(ctx context.Context, req *logical. return logical.ErrorResponse("failed to validate CIDR blocks"), nil } - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRoleBoundCIDRListRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1340,17 +1357,19 @@ func (b *backend) pathRoleBoundCIDRListRead(ctx context.Context, req *logical.Re lock.Lock() defer lock.Unlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - return &logical.Response{ - Data: map[string]interface{}{ - "bound_cidr_list": role.BoundCIDRList, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "bound_cidr_list": role.BoundCIDRList, + }, + }, nil } func (b *backend) pathRoleBoundCIDRListDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1363,7 +1382,7 @@ func (b *backend) pathRoleBoundCIDRListDelete(ctx context.Context, req *logical. lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1374,7 +1393,7 @@ func (b *backend) pathRoleBoundCIDRListDelete(ctx context.Context, req *logical. // Deleting a field implies setting the value to it's default value. role.BoundCIDRList = data.GetDefaultOrZero("bound_cidr_list").([]string) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRoleBindSecretIDUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1387,7 +1406,7 @@ func (b *backend) pathRoleBindSecretIDUpdate(ctx context.Context, req *logical.R lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1397,7 +1416,7 @@ func (b *backend) pathRoleBindSecretIDUpdate(ctx context.Context, req *logical.R if bindSecretIDRaw, ok := data.GetOk("bind_secret_id"); ok { role.BindSecretID = bindSecretIDRaw.(bool) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } else { return logical.ErrorResponse("missing bind_secret_id"), nil } @@ -1413,17 +1432,19 @@ func (b *backend) pathRoleBindSecretIDRead(ctx context.Context, req *logical.Req lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - return &logical.Response{ - Data: map[string]interface{}{ - "bind_secret_id": role.BindSecretID, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "bind_secret_id": role.BindSecretID, + }, + }, nil } func (b *backend) pathRoleBindSecretIDDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1436,7 +1457,7 @@ func (b *backend) pathRoleBindSecretIDDelete(ctx context.Context, req *logical.R lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1447,7 +1468,7 @@ func (b *backend) pathRoleBindSecretIDDelete(ctx context.Context, req *logical.R // Deleting a field implies setting the value to it's default value. role.BindSecretID = data.GetDefaultOrZero("bind_secret_id").(bool) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRoleLocalSecretIDsRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1460,21 +1481,24 @@ func (b *backend) pathRoleLocalSecretIDsRead(ctx context.Context, req *logical.R lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - localSecretIDs := false - if role.SecretIDPrefix == secretIDLocalPrefix { - localSecretIDs = true - } - return &logical.Response{ - Data: map[string]interface{}{ - "local_secret_ids": localSecretIDs, - }, - }, nil } + + localSecretIDs := false + if role.SecretIDPrefix == secretIDLocalPrefix { + localSecretIDs = true + } + + return &logical.Response{ + Data: map[string]interface{}{ + "local_secret_ids": localSecretIDs, + }, + }, nil } func (b *backend) pathRolePoliciesUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1487,7 +1511,7 @@ func (b *backend) pathRolePoliciesUpdate(ctx context.Context, req *logical.Reque lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1502,7 +1526,7 @@ func (b *backend) pathRolePoliciesUpdate(ctx context.Context, req *logical.Reque role.Policies = policyutil.ParsePolicies(policiesRaw) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRolePoliciesRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1515,17 +1539,19 @@ func (b *backend) pathRolePoliciesRead(ctx context.Context, req *logical.Request lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - return &logical.Response{ - Data: map[string]interface{}{ - "policies": role.Policies, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "policies": role.Policies, + }, + }, nil } func (b *backend) pathRolePoliciesDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1538,7 +1564,7 @@ func (b *backend) pathRolePoliciesDelete(ctx context.Context, req *logical.Reque lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1548,7 +1574,7 @@ func (b *backend) pathRolePoliciesDelete(ctx context.Context, req *logical.Reque role.Policies = []string{} - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRoleSecretIDNumUsesUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1561,7 +1587,7 @@ func (b *backend) pathRoleSecretIDNumUsesUpdate(ctx context.Context, req *logica lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1574,7 +1600,7 @@ func (b *backend) pathRoleSecretIDNumUsesUpdate(ctx context.Context, req *logica if role.SecretIDNumUses < 0 { return logical.ErrorResponse("secret_id_num_uses cannot be negative"), nil } - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } else { return logical.ErrorResponse("missing secret_id_num_uses"), nil } @@ -1590,7 +1616,7 @@ func (b *backend) pathRoleRoleIDUpdate(ctx context.Context, req *logical.Request lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1604,7 +1630,7 @@ func (b *backend) pathRoleRoleIDUpdate(ctx context.Context, req *logical.Request return logical.ErrorResponse("missing role_id"), nil } - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, previousRoleID) + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, previousRoleID) } func (b *backend) pathRoleRoleIDRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1617,17 +1643,19 @@ func (b *backend) pathRoleRoleIDRead(ctx context.Context, req *logical.Request, lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - return &logical.Response{ - Data: map[string]interface{}{ - "role_id": role.RoleID, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "role_id": role.RoleID, + }, + }, nil } func (b *backend) pathRoleSecretIDNumUsesRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1640,17 +1668,19 @@ func (b *backend) pathRoleSecretIDNumUsesRead(ctx context.Context, req *logical. lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - return &logical.Response{ - Data: map[string]interface{}{ - "secret_id_num_uses": role.SecretIDNumUses, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "secret_id_num_uses": role.SecretIDNumUses, + }, + }, nil } func (b *backend) pathRoleSecretIDNumUsesDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1663,7 +1693,7 @@ func (b *backend) pathRoleSecretIDNumUsesDelete(ctx context.Context, req *logica lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1673,7 +1703,7 @@ func (b *backend) pathRoleSecretIDNumUsesDelete(ctx context.Context, req *logica role.SecretIDNumUses = data.GetDefaultOrZero("secret_id_num_uses").(int) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRoleSecretIDTTLUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1686,7 +1716,7 @@ func (b *backend) pathRoleSecretIDTTLUpdate(ctx context.Context, req *logical.Re lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1696,7 +1726,7 @@ func (b *backend) pathRoleSecretIDTTLUpdate(ctx context.Context, req *logical.Re if secretIDTTLRaw, ok := data.GetOk("secret_id_ttl"); ok { role.SecretIDTTL = time.Second * time.Duration(secretIDTTLRaw.(int)) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } else { return logical.ErrorResponse("missing secret_id_ttl"), nil } @@ -1712,18 +1742,19 @@ func (b *backend) pathRoleSecretIDTTLRead(ctx context.Context, req *logical.Requ lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - role.SecretIDTTL /= time.Second - return &logical.Response{ - Data: map[string]interface{}{ - "secret_id_ttl": role.SecretIDTTL, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "secret_id_ttl": role.SecretIDTTL / time.Second, + }, + }, nil } func (b *backend) pathRoleSecretIDTTLDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1736,7 +1767,7 @@ func (b *backend) pathRoleSecretIDTTLDelete(ctx context.Context, req *logical.Re lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1746,7 +1777,7 @@ func (b *backend) pathRoleSecretIDTTLDelete(ctx context.Context, req *logical.Re role.SecretIDTTL = time.Second * time.Duration(data.GetDefaultOrZero("secret_id_ttl").(int)) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRolePeriodUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1759,7 +1790,7 @@ func (b *backend) pathRolePeriodUpdate(ctx context.Context, req *logical.Request lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1772,7 +1803,7 @@ func (b *backend) pathRolePeriodUpdate(ctx context.Context, req *logical.Request if role.Period > b.System().MaxLeaseTTL() { return logical.ErrorResponse(fmt.Sprintf("period of %q is greater than the backend's maximum lease TTL of %q", role.Period.String(), b.System().MaxLeaseTTL().String())), nil } - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } else { return logical.ErrorResponse("missing period"), nil } @@ -1788,18 +1819,19 @@ func (b *backend) pathRolePeriodRead(ctx context.Context, req *logical.Request, lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - role.Period /= time.Second - return &logical.Response{ - Data: map[string]interface{}{ - "period": role.Period, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "period": role.Period / time.Second, + }, + }, nil } func (b *backend) pathRolePeriodDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1812,7 +1844,7 @@ func (b *backend) pathRolePeriodDelete(ctx context.Context, req *logical.Request lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1822,7 +1854,7 @@ func (b *backend) pathRolePeriodDelete(ctx context.Context, req *logical.Request role.Period = time.Second * time.Duration(data.GetDefaultOrZero("period").(int)) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRoleTokenNumUsesUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1835,7 +1867,7 @@ func (b *backend) pathRoleTokenNumUsesUpdate(ctx context.Context, req *logical.R lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1845,7 +1877,7 @@ func (b *backend) pathRoleTokenNumUsesUpdate(ctx context.Context, req *logical.R if tokenNumUsesRaw, ok := data.GetOk("token_num_uses"); ok { role.TokenNumUses = tokenNumUsesRaw.(int) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } else { return logical.ErrorResponse("missing token_num_uses"), nil } @@ -1861,17 +1893,19 @@ func (b *backend) pathRoleTokenNumUsesRead(ctx context.Context, req *logical.Req lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - return &logical.Response{ - Data: map[string]interface{}{ - "token_num_uses": role.TokenNumUses, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "token_num_uses": role.TokenNumUses, + }, + }, nil } func (b *backend) pathRoleTokenNumUsesDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1884,7 +1918,7 @@ func (b *backend) pathRoleTokenNumUsesDelete(ctx context.Context, req *logical.R lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1894,7 +1928,7 @@ func (b *backend) pathRoleTokenNumUsesDelete(ctx context.Context, req *logical.R role.TokenNumUses = data.GetDefaultOrZero("token_num_uses").(int) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRoleTokenTTLUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1907,7 +1941,7 @@ func (b *backend) pathRoleTokenTTLUpdate(ctx context.Context, req *logical.Reque lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1920,7 +1954,7 @@ func (b *backend) pathRoleTokenTTLUpdate(ctx context.Context, req *logical.Reque if role.TokenMaxTTL > time.Duration(0) && role.TokenTTL > role.TokenMaxTTL { return logical.ErrorResponse("token_ttl should not be greater than token_max_ttl"), nil } - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } else { return logical.ErrorResponse("missing token_ttl"), nil } @@ -1936,18 +1970,19 @@ func (b *backend) pathRoleTokenTTLRead(ctx context.Context, req *logical.Request lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - role.TokenTTL /= time.Second - return &logical.Response{ - Data: map[string]interface{}{ - "token_ttl": role.TokenTTL, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "token_ttl": role.TokenTTL / time.Second, + }, + }, nil } func (b *backend) pathRoleTokenTTLDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1960,7 +1995,7 @@ func (b *backend) pathRoleTokenTTLDelete(ctx context.Context, req *logical.Reque lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1970,7 +2005,7 @@ func (b *backend) pathRoleTokenTTLDelete(ctx context.Context, req *logical.Reque role.TokenTTL = time.Second * time.Duration(data.GetDefaultOrZero("token_ttl").(int)) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRoleTokenMaxTTLUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -1983,7 +2018,7 @@ func (b *backend) pathRoleTokenMaxTTLUpdate(ctx context.Context, req *logical.Re lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -1996,7 +2031,7 @@ func (b *backend) pathRoleTokenMaxTTLUpdate(ctx context.Context, req *logical.Re if role.TokenMaxTTL > time.Duration(0) && role.TokenTTL > role.TokenMaxTTL { return logical.ErrorResponse("token_max_ttl should be greater than or equal to token_ttl"), nil } - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } else { return logical.ErrorResponse("missing token_max_ttl"), nil } @@ -2012,18 +2047,19 @@ func (b *backend) pathRoleTokenMaxTTLRead(ctx context.Context, req *logical.Requ lock.RLock() defer lock.RUnlock() - if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + role, err := b.roleEntry(ctx, req.Storage, roleName) + if err != nil { return nil, err - } else if role == nil { + } + if role == nil { return nil, nil - } else { - role.TokenMaxTTL /= time.Second - return &logical.Response{ - Data: map[string]interface{}{ - "token_max_ttl": role.TokenMaxTTL, - }, - }, nil } + + return &logical.Response{ + Data: map[string]interface{}{ + "token_max_ttl": role.TokenMaxTTL / time.Second, + }, + }, nil } func (b *backend) pathRoleTokenMaxTTLDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -2036,7 +2072,7 @@ func (b *backend) pathRoleTokenMaxTTLDelete(ctx context.Context, req *logical.Re lock.Lock() defer lock.Unlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -2046,7 +2082,7 @@ func (b *backend) pathRoleTokenMaxTTLDelete(ctx context.Context, req *logical.Re role.TokenMaxTTL = time.Second * time.Duration(data.GetDefaultOrZero("token_max_ttl").(int)) - return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") + return nil, b.setRoleEntry(ctx, req.Storage, role.name, role, "") } func (b *backend) pathRoleSecretIDUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -2075,7 +2111,7 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req lock.RLock() defer lock.RUnlock() - role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)) + role, err := b.roleEntry(ctx, req.Storage, roleName) if err != nil { return nil, err } @@ -2116,11 +2152,7 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req return logical.ErrorResponse(fmt.Sprintf("failed to parse metadata: %v", err)), nil } - if role.LowerCaseRoleName { - roleName = strings.ToLower(roleName) - } - - if secretIDStorage, err = b.registerSecretIDEntry(ctx, req.Storage, roleName, secretID, role.HMACKey, role.SecretIDPrefix, secretIDStorage); err != nil { + if secretIDStorage, err = b.registerSecretIDEntry(ctx, req.Storage, role.name, secretID, role.HMACKey, role.SecretIDPrefix, secretIDStorage); err != nil { return nil, errwrap.Wrapf("failed to store secret_id: {{err}}", err) } @@ -2137,7 +2169,7 @@ func (b *backend) roleIDLock(roleID string) *locksutil.LockEntry { } func (b *backend) roleLock(roleName string) *locksutil.LockEntry { - return locksutil.LockForKey(b.roleLocks, roleName) + return locksutil.LockForKey(b.roleLocks, strings.ToLower(roleName)) } // setRoleIDEntry creates a storage entry that maps RoleID to Role diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go index e3a757f74661..68d951d55f38 100644 --- a/builtin/credential/approle/path_role_test.go +++ b/builtin/credential/approle/path_role_test.go @@ -45,7 +45,7 @@ func TestAppRole_LocalSecretIDsRead(t *testing.T) { } } -func TestApprole_LocalNonLocalSecretIDs(t *testing.T) { +func TestAppRole_LocalNonLocalSecretIDs(t *testing.T) { var resp *logical.Response var err error @@ -131,7 +131,7 @@ func TestApprole_LocalNonLocalSecretIDs(t *testing.T) { } } -func TestApprole_UpgradeSecretIDPrefix(t *testing.T) { +func TestAppRole_UpgradeSecretIDPrefix(t *testing.T) { var resp *logical.Response var err error @@ -173,7 +173,7 @@ func TestApprole_UpgradeSecretIDPrefix(t *testing.T) { } } -func TestApprole_LocalSecretIDImmutability(t *testing.T) { +func TestAppRole_LocalSecretIDImmutability(t *testing.T) { var resp *logical.Response var err error @@ -209,7 +209,7 @@ func TestApprole_LocalSecretIDImmutability(t *testing.T) { } } -func TestApprole_UpgradeBoundCIDRList(t *testing.T) { +func TestAppRole_UpgradeBoundCIDRList(t *testing.T) { var resp *logical.Response var err error @@ -309,7 +309,7 @@ func TestApprole_UpgradeBoundCIDRList(t *testing.T) { } } -func TestApprole_RoleNameLowerCasing(t *testing.T) { +func TestAppRole_RoleNameLowerCasing(t *testing.T) { var resp *logical.Response var err error var roleID, secretID string diff --git a/builtin/credential/approle/path_tidy_user_id.go b/builtin/credential/approle/path_tidy_user_id.go index 9cab00d3daa5..1a385efd2aa5 100644 --- a/builtin/credential/approle/path_tidy_user_id.go +++ b/builtin/credential/approle/path_tidy_user_id.go @@ -77,17 +77,32 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error { return err } + // If a secret ID entry does not have a corresponding accessor + // entry, revoke the secret ID immediately + accessorEntry, err := b.secretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse) + if err != nil { + return errwrap.Wrapf("failed to read secret ID accessor entry: {{err}}", err) + } + if accessorEntry == nil { + if err := s.Delete(ctx, entryIndex); err != nil { + return errwrap.Wrapf(fmt.Sprintf("error deleting secret ID %q from storage: {{err}}", secretIDHMAC), err) + } + return nil + } + // ExpirationTime not being set indicates non-expiring SecretIDs if !result.ExpirationTime.IsZero() && time.Now().After(result.ExpirationTime) { // Clean up the accessor of the secret ID first err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse) if err != nil { - return err + return errwrap.Wrapf("failed to delete secret ID accessor entry: {{err}}", err) } if err := s.Delete(ctx, entryIndex); err != nil { return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err) } + + return nil } // At this point, the secret ID is not expired and is valid. Delete diff --git a/builtin/credential/approle/validation.go b/builtin/credential/approle/validation.go index bdccad57b109..c7cadf6bc4d3 100644 --- a/builtin/credential/approle/validation.go +++ b/builtin/credential/approle/validation.go @@ -24,36 +24,36 @@ type secretIDStorageEntry struct { // the SecretID it belongs to, and hence can be used for listing // and deleting SecretIDs. Accessors cannot be used as valid // SecretIDs during login. - SecretIDAccessor string `json:"secret_id_accessor" structs:"secret_id_accessor" mapstructure:"secret_id_accessor"` + SecretIDAccessor string `json:"secret_id_accessor" mapstructure:"secret_id_accessor"` // Number of times this SecretID can be used to perform the login // operation - SecretIDNumUses int `json:"secret_id_num_uses" structs:"secret_id_num_uses" mapstructure:"secret_id_num_uses"` + SecretIDNumUses int `json:"secret_id_num_uses" mapstructure:"secret_id_num_uses"` // Duration after which this SecretID should expire. This is capped by // the backend mount's max TTL value. - SecretIDTTL time.Duration `json:"secret_id_ttl" structs:"secret_id_ttl" mapstructure:"secret_id_ttl"` + SecretIDTTL time.Duration `json:"secret_id_ttl" mapstructure:"secret_id_ttl"` // The time when the SecretID was created - CreationTime time.Time `json:"creation_time" structs:"creation_time" mapstructure:"creation_time"` + CreationTime time.Time `json:"creation_time" mapstructure:"creation_time"` // The time when the SecretID becomes eligible for tidy operation. // Tidying is performed by the PeriodicFunc of the backend which is 1 // minute apart. - ExpirationTime time.Time `json:"expiration_time" structs:"expiration_time" mapstructure:"expiration_time"` + ExpirationTime time.Time `json:"expiration_time" mapstructure:"expiration_time"` // The time representing the last time this storage entry was modified - LastUpdatedTime time.Time `json:"last_updated_time" structs:"last_updated_time" mapstructure:"last_updated_time"` + LastUpdatedTime time.Time `json:"last_updated_time" mapstructure:"last_updated_time"` // Metadata that belongs to the SecretID - Metadata map[string]string `json:"metadata" structs:"metadata" mapstructure:"metadata"` + Metadata map[string]string `json:"metadata" mapstructure:"metadata"` // CIDRList is a set of CIDR blocks that impose source address // restrictions on the usage of SecretID - CIDRList []string `json:"cidr_list" structs:"cidr_list" mapstructure:"cidr_list"` + CIDRList []string `json:"cidr_list" mapstructure:"cidr_list"` // This is a deprecated field - SecretIDNumUsesDeprecated int `json:"SecretIDNumUses" structs:"SecretIDNumUses" mapstructure:"SecretIDNumUses"` + SecretIDNumUsesDeprecated int `json:"SecretIDNumUses" mapstructure:"SecretIDNumUses"` } // Represents the payload of the storage entry of the accessor that maps to a @@ -63,7 +63,7 @@ type secretIDStorageEntry struct { type secretIDAccessorStorageEntry struct { // Hash of the SecretID which can be used to find the storage index at which // properties of SecretID is stored. - SecretIDHMAC string `json:"secret_id_hmac" structs:"secret_id_hmac" mapstructure:"secret_id_hmac"` + SecretIDHMAC string `json:"secret_id_hmac" mapstructure:"secret_id_hmac"` } // verifyCIDRRoleSecretIDSubset checks if the CIDR blocks set on the secret ID diff --git a/builtin/credential/approle/validation_test.go b/builtin/credential/approle/validation_test.go index 5493cfe471aa..10392d07387f 100644 --- a/builtin/credential/approle/validation_test.go +++ b/builtin/credential/approle/validation_test.go @@ -51,9 +51,7 @@ func TestAppRole_SecretIDNumUsesUpgrade(t *testing.T) { } // Check if the response contains the value set for secret_id_num_uses - // and not SecretIDNumUses - if resp.Data["secret_id_num_uses"] != 10 || - resp.Data["SecretIDNumUses"] != 0 { + if resp.Data["secret_id_num_uses"] != 10 { t.Fatal("invalid secret_id_num_uses") } }