diff --git a/builtin/credential/approle/backend.go b/builtin/credential/approle/backend.go index 6b914685ef55..01911441297b 100644 --- a/builtin/credential/approle/backend.go +++ b/builtin/credential/approle/backend.go @@ -11,6 +11,13 @@ import ( "github.com/hashicorp/vault/logical/framework" ) +const ( + secretIDPrefix = "secret_id/" + secretIDLocalPrefix = "secret_id_local/" + secretIDAccessorPrefix = "accessor/" + secretIDAccessorLocalPrefix = "accessor_local/" +) + type backend struct { *framework.Backend @@ -90,6 +97,10 @@ func Backend(conf *logical.BackendConfig) (*backend, error) { Unauthenticated: []string{ "login", }, + LocalStorage: []string{ + secretIDLocalPrefix, + secretIDAccessorLocalPrefix, + }, }, Paths: framework.PathAppend( rolePaths(b), diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go index b0d1e8314702..4c057b6d3561 100644 --- a/builtin/credential/approle/path_role.go +++ b/builtin/credential/approle/path_role.go @@ -67,6 +67,10 @@ type roleStorageEntry struct { // 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"` + + // 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"` } // roleIDStorageEntry represents the reverse mapping from RoleID to Role @@ -163,6 +167,11 @@ TTL will be set to the value of this parameter.`, Type: framework.TypeString, Description: "Identifier of the role. Defaults to a UUID.", }, + "local_secret_ids": &framework.FieldSchema{ + Type: framework.TypeBool, + Description: `If set, the secret IDs generated using this role will be cluster local. This +can only be set during role creation and once set, it can't be reset later.`, + }, }, ExistenceCheck: b.pathRoleExistenceCheck, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -174,6 +183,20 @@ TTL will be set to the value of this parameter.`, HelpSynopsis: strings.TrimSpace(roleHelp["role"][0]), HelpDescription: strings.TrimSpace(roleHelp["role"][1]), }, + &framework.Path{ + Pattern: "role/" + framework.GenericNameRegex("role_name") + "/local-secret-ids$", + Fields: map[string]*framework.FieldSchema{ + "role_name": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "Name of the role.", + }, + }, + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.ReadOperation: b.pathRoleLocalSecretIDsRead, + }, + HelpSynopsis: strings.TrimSpace(roleHelp["role-local-secret-ids"][0]), + HelpDescription: strings.TrimSpace(roleHelp["role-local-secret-ids"][1]), + }, &framework.Path{ Pattern: "role/" + framework.GenericNameRegex("role_name") + "/policies$", Fields: map[string]*framework.FieldSchema{ @@ -585,7 +608,7 @@ func (b *backend) pathRoleSecretIDList(ctx context.Context, req *logical.Request // Listing works one level at a time. Get the first level of data // which could then be used to get the actual SecretID storage entries. - secretIDHMACs, err := req.Storage.List(ctx, fmt.Sprintf("secret_id/%s/", roleNameHMAC)) + secretIDHMACs, err := req.Storage.List(ctx, fmt.Sprintf("%s%s/", role.SecretIDPrefix, roleNameHMAC)) if err != nil { return nil, err } @@ -598,7 +621,7 @@ func (b *backend) pathRoleSecretIDList(ctx context.Context, req *logical.Request } // Prepare the full index of the SecretIDs. - entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC) + entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) // SecretID locks are not indexed by SecretIDs itself. // This is because SecretIDs are not stored in plaintext @@ -731,6 +754,11 @@ func (b *backend) roleEntry(ctx context.Context, s logical.Storage, roleName str needsUpgrade = true } + if role.SecretIDPrefix == "" { + role.SecretIDPrefix = secretIDPrefix + needsUpgrade = true + } + if needsUpgrade && (b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary)) { entry, err := logical.StorageEntryJSON("role/"+strings.ToLower(roleName), &role) if err != nil { @@ -776,7 +804,20 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request LowerCaseRoleName: true, } } else if role == nil { - return logical.ErrorResponse(fmt.Sprintf("invalid role name")), nil + return logical.ErrorResponse(fmt.Sprintf("role name %q doesn't exist", roleName)), nil + } + + localSecretIDsRaw, ok := data.GetOk("local_secret_ids") + if ok { + switch { + case req.Operation == logical.CreateOperation: + localSecretIDs := localSecretIDsRaw.(bool) + if localSecretIDs { + role.SecretIDPrefix = secretIDLocalPrefix + } + default: + return logical.ErrorResponse("local_secret_ids can only be modified during role creation"), nil + } } previousRoleID := role.RoleID @@ -916,6 +957,11 @@ func (b *backend) pathRoleRead(ctx context.Context, req *logical.Request, data * "token_max_ttl": role.TokenMaxTTL / time.Second, "token_num_uses": role.TokenNumUses, "token_ttl": role.TokenTTL / time.Second, + "local_secret_ids": false, + } + + if role.SecretIDPrefix == secretIDLocalPrefix { + respData["local_secret_ids"] = true } resp := &logical.Response{ @@ -985,7 +1031,7 @@ 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); err != nil { + 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) } @@ -1044,7 +1090,7 @@ func (b *backend) pathRoleSecretIDLookupUpdate(ctx context.Context, req *logical } // Create the index at which the secret_id would've been stored - entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC) + entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) return b.secretIDCommon(ctx, req.Storage, entryIndex, secretIDHMAC) } @@ -1119,7 +1165,7 @@ func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req * return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } - entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC) + entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) lock := b.secretIDLock(secretIDHMAC) lock.Lock() @@ -1135,7 +1181,7 @@ func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req * } // Delete the accessor of the SecretID first - if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor); err != nil { + if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor, role.SecretIDPrefix); err != nil { return nil, err } @@ -1176,7 +1222,7 @@ func (b *backend) pathRoleSecretIDAccessorLookupUpdate(ctx context.Context, req return nil, fmt.Errorf("role %q does not exist", roleName) } - accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor) + accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix) if err != nil { return nil, err } @@ -1189,7 +1235,7 @@ func (b *backend) pathRoleSecretIDAccessorLookupUpdate(ctx context.Context, req return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } - entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, accessorEntry.SecretIDHMAC) + entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC) return b.secretIDCommon(ctx, req.Storage, entryIndex, accessorEntry.SecretIDHMAC) } @@ -1217,7 +1263,7 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex return nil, fmt.Errorf("role %q does not exist", roleName) } - accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor) + accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix) if err != nil { return nil, err } @@ -1230,14 +1276,14 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } - entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, accessorEntry.SecretIDHMAC) + entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC) lock := b.secretIDLock(accessorEntry.SecretIDHMAC) lock.Lock() defer lock.Unlock() // Delete the accessor of the SecretID first - if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, secretIDAccessor); err != nil { + if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix); err != nil { return nil, err } @@ -1404,6 +1450,33 @@ func (b *backend) pathRoleBindSecretIDDelete(ctx context.Context, req *logical.R return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "") } +func (b *backend) pathRoleLocalSecretIDsRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { + roleName := data.Get("role_name").(string) + if roleName == "" { + return logical.ErrorResponse("missing role_name"), nil + } + + lock := b.roleLock(roleName) + lock.RLock() + defer lock.RUnlock() + + if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil { + return nil, err + } else 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 + } +} + func (b *backend) pathRolePoliciesUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { roleName := data.Get("role_name").(string) if roleName == "" { @@ -2047,7 +2120,7 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req roleName = strings.ToLower(roleName) } - if secretIDStorage, err = b.registerSecretIDEntry(ctx, req.Storage, roleName, secretID, role.HMACKey, secretIDStorage); err != nil { + if secretIDStorage, err = b.registerSecretIDEntry(ctx, req.Storage, roleName, secretID, role.HMACKey, role.SecretIDPrefix, secretIDStorage); err != nil { return nil, errwrap.Wrapf("failed to store secret_id: {{err}}", err) } @@ -2265,4 +2338,10 @@ duration specified by this value. The renewal duration will be fixed. If the Period in the role is modified, the token will pick up the new value during its next renewal.`, }, + "role-local-secret-ids": { + "Enables cluster local secret IDs", + `If set, the secret IDs generated using this role will be cluster local. +This can only be set during role creation and once set, it can't be +reset later.`, + }, } diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go index 1e07c32484fe..e3a757f74661 100644 --- a/builtin/credential/approle/path_role_test.go +++ b/builtin/credential/approle/path_role_test.go @@ -12,6 +12,203 @@ import ( "github.com/mitchellh/mapstructure" ) +func TestAppRole_LocalSecretIDsRead(t *testing.T) { + var resp *logical.Response + var err error + b, storage := createBackendWithStorage(t) + + roleData := map[string]interface{}{ + "local_secret_ids": true, + "bind_secret_id": true, + } + + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/testrole", + Storage: storage, + Data: roleData, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Storage: storage, + Path: "role/testrole/local-secret-ids", + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + if !resp.Data["local_secret_ids"].(bool) { + t.Fatalf("expected local_secret_ids to be returned") + } +} + +func TestApprole_LocalNonLocalSecretIDs(t *testing.T) { + var resp *logical.Response + var err error + + b, storage := createBackendWithStorage(t) + + // Create a role with local_secret_ids set + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole1", + Operation: logical.CreateOperation, + Storage: storage, + Data: map[string]interface{}{ + "policies": []string{"default", "role1policy"}, + "bind_secret_id": true, + "local_secret_ids": true, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: err: %v\n resp: %#v", err, resp) + } + + // Create another role without setting local_secret_ids + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole2", + Operation: logical.CreateOperation, + Storage: storage, + Data: map[string]interface{}{ + "policies": []string{"default", "role1policy"}, + "bind_secret_id": true, + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: err: %v\n resp: %#v", err, resp) + } + + count := 10 + // Create secret IDs on testrole1 + for i := 0; i < count; i++ { + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole1/secret-id", + Operation: logical.UpdateOperation, + Storage: storage, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + } + + // Check the number of secret IDs generated + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole1/secret-id", + Operation: logical.ListOperation, + Storage: storage, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + if len(resp.Data["keys"].([]string)) != count { + t.Fatalf("failed to list secret IDs") + } + + // Create secret IDs on testrole1 + for i := 0; i < count; i++ { + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole2/secret-id", + Operation: logical.UpdateOperation, + Storage: storage, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + } + + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole2/secret-id", + Operation: logical.ListOperation, + Storage: storage, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + if len(resp.Data["keys"].([]string)) != count { + t.Fatalf("failed to list secret IDs") + } +} + +func TestApprole_UpgradeSecretIDPrefix(t *testing.T) { + var resp *logical.Response + var err error + + b, storage := createBackendWithStorage(t) + + // Create a role entry directly in storage without SecretIDPrefix + err = b.setRoleEntry(context.Background(), storage, "testrole", &roleStorageEntry{ + RoleID: "testroleid", + HMACKey: "testhmackey", + Policies: []string{"default"}, + BindSecretID: true, + BoundCIDRListOld: "127.0.0.1/18,192.178.1.2/24", + }, "") + if err != nil { + t.Fatal(err) + } + + // Reading the role entry should upgrade it to contain SecretIDPrefix + role, err := b.roleEntry(context.Background(), storage, "testrole") + if err != nil { + t.Fatal(err) + } + if role.SecretIDPrefix == "" { + t.Fatalf("expected SecretIDPrefix to be set") + } + + // Ensure that the API response contains local_secret_ids + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole", + Operation: logical.ReadOperation, + Storage: storage, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: err: %v\n resp: %#v", err, resp) + } + _, ok := resp.Data["local_secret_ids"] + if !ok { + t.Fatalf("expected local_secret_ids to be present in the response") + } +} + +func TestApprole_LocalSecretIDImmutability(t *testing.T) { + var resp *logical.Response + var err error + + b, storage := createBackendWithStorage(t) + + roleData := map[string]interface{}{ + "policies": []string{"default"}, + "bind_secret_id": true, + "bound_cidr_list": []string{"127.0.0.1/18", "192.178.1.2/24"}, + "local_secret_ids": true, + } + + // Create a role with local_secret_ids set + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole", + Operation: logical.CreateOperation, + Storage: storage, + Data: roleData, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: err: %v\nresp: %#v", err, resp) + } + + // Attempt to modify local_secret_ids should fail + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole", + Operation: logical.UpdateOperation, + Storage: storage, + Data: roleData, + }) + if resp == nil || !resp.IsError() { + t.Fatalf("expected an error since local_secret_ids can't be overwritten") + } +} + func TestApprole_UpgradeBoundCIDRList(t *testing.T) { var resp *logical.Response var err error @@ -59,6 +256,7 @@ func TestApprole_UpgradeBoundCIDRList(t *testing.T) { Policies: []string{"default"}, BindSecretID: true, BoundCIDRListOld: "127.0.0.1/18,192.178.1.2/24", + SecretIDPrefix: secretIDPrefix, } err = b.setRoleEntry(context.Background(), storage, "testrole", role, "") if err != nil { @@ -120,10 +318,11 @@ func TestApprole_RoleNameLowerCasing(t *testing.T) { // Save a role with out LowerCaseRoleName set role := &roleStorageEntry{ - RoleID: "testroleid", - HMACKey: "testhmackey", - Policies: []string{"default"}, - BindSecretID: true, + RoleID: "testroleid", + HMACKey: "testhmackey", + Policies: []string{"default"}, + BindSecretID: true, + SecretIDPrefix: secretIDPrefix, } err = b.setRoleEntry(context.Background(), storage, "testRoleName", role, "") if err != nil { diff --git a/builtin/credential/approle/path_tidy_user_id.go b/builtin/credential/approle/path_tidy_user_id.go index 27e0d0300ab5..9cab00d3daa5 100644 --- a/builtin/credential/approle/path_tidy_user_id.go +++ b/builtin/credential/approle/path_tidy_user_id.go @@ -34,69 +34,58 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error { return fmt.Errorf("SecretID tidy operation already running") } - roleNameHMACs, err := s.List(ctx, "secret_id/") - if err != nil { - return err - } + var result error - // List all the accessors and add them all to a map - accessorHashes, err := s.List(ctx, "accessor/") - if err != nil { - return err - } - accessorMap := make(map[string]bool, len(accessorHashes)) - for _, accessorHash := range accessorHashes { - accessorMap[accessorHash] = true - } + tidyFunc := func(secretIDPrefixToUse, accessorIDPrefixToUse string) error { + roleNameHMACs, err := s.List(ctx, secretIDPrefixToUse) + if err != nil { + return err + } - var result error - for _, roleNameHMAC := range roleNameHMACs { - // roleNameHMAC will already have a '/' suffix. Don't append another one. - secretIDHMACs, err := s.List(ctx, fmt.Sprintf("secret_id/%s", roleNameHMAC)) + // List all the accessors and add them all to a map + accessorHashes, err := s.List(ctx, accessorIDPrefixToUse) if err != nil { return err } - for _, secretIDHMAC := range secretIDHMACs { - // In order to avoid lock swroleing in case there is need to delete, - // grab the write lock. + accessorMap := make(map[string]bool, len(accessorHashes)) + for _, accessorHash := range accessorHashes { + accessorMap[accessorHash] = true + } + + secretIDCleanupFunc := func(secretIDHMAC, roleNameHMAC, secretIDPrefixToUse string) error { lock := b.secretIDLock(secretIDHMAC) lock.Lock() - // roleNameHMAC will already have a '/' suffix. Don't append another one. - entryIndex := fmt.Sprintf("secret_id/%s%s", roleNameHMAC, secretIDHMAC) + defer lock.Unlock() + + entryIndex := fmt.Sprintf("%s%s%s", secretIDPrefixToUse, roleNameHMAC, secretIDHMAC) secretIDEntry, err := s.Get(ctx, entryIndex) if err != nil { - lock.Unlock() return errwrap.Wrapf(fmt.Sprintf("error fetching SecretID %q: {{err}}", secretIDHMAC), err) } if secretIDEntry == nil { result = multierror.Append(result, fmt.Errorf("entry for SecretID %q is nil", secretIDHMAC)) - lock.Unlock() - continue + return nil } if secretIDEntry.Value == nil || len(secretIDEntry.Value) == 0 { - lock.Unlock() return fmt.Errorf("found entry for SecretID %q but actual SecretID is empty", secretIDHMAC) } var result secretIDStorageEntry if err := secretIDEntry.DecodeJSON(&result); err != nil { - lock.Unlock() return err } // 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) + err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse) if err != nil { - lock.Unlock() return err } if err := s.Delete(ctx, entryIndex); err != nil { - lock.Unlock() return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err) } } @@ -107,25 +96,48 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error { // up later. salt, err := b.Salt(ctx) if err != nil { - lock.Unlock() return err } delete(accessorMap, salt.SaltID(result.SecretIDAccessor)) - lock.Unlock() + return nil } - } - // Accessor indexes were not getting cleaned up until 0.9.3. This is a fix - // to clean up the dangling accessor entries. - for accessorHash, _ := range accessorMap { - // Ideally, locking should be performed here. But for that, accessors - // are required in plaintext, which are not available. Hence performing - // a racy cleanup. - err = s.Delete(ctx, "accessor/"+accessorHash) - if err != nil { - return err + for _, roleNameHMAC := range roleNameHMACs { + secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", secretIDPrefixToUse, roleNameHMAC)) + if err != nil { + return err + } + for _, secretIDHMAC := range secretIDHMACs { + err = secretIDCleanupFunc(secretIDHMAC, roleNameHMAC, secretIDPrefixToUse) + if err != nil { + return err + } + } } + + // Accessor indexes were not getting cleaned up until 0.9.3. This is a fix + // to clean up the dangling accessor entries. + for accessorHash, _ := range accessorMap { + // Ideally, locking should be performed here. But for that, accessors + // are required in plaintext, which are not available. Hence performing + // a racy cleanup. + err = s.Delete(ctx, secretIDAccessorPrefix+accessorHash) + if err != nil { + return err + } + } + + return nil + } + + err := tidyFunc(secretIDPrefix, secretIDAccessorPrefix) + if err != nil { + return err + } + err = tidyFunc(secretIDLocalPrefix, secretIDAccessorLocalPrefix) + if err != nil { + return err } return result diff --git a/builtin/credential/approle/validation.go b/builtin/credential/approle/validation.go index 955eae81ec60..d42d68fd0c39 100644 --- a/builtin/credential/approle/validation.go +++ b/builtin/credential/approle/validation.go @@ -128,7 +128,7 @@ func (b *backend) validateCredentials(ctx context.Context, req *logical.Request, // Check if the SecretID supplied is valid. If use limit was specified // on the SecretID, it will be decremented in this call. var valid bool - valid, metadata, err = b.validateBindSecretID(ctx, req, roleName, secretID, role.HMACKey, role.BoundCIDRList) + valid, metadata, err = b.validateBindSecretID(ctx, req, roleName, secretID, role) if err != nil { return nil, "", metadata, "", nil, err } @@ -156,19 +156,19 @@ func (b *backend) validateCredentials(ctx context.Context, req *logical.Request, } // validateBindSecretID is used to determine if the given SecretID is a valid one. -func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request, roleName, secretID, - hmacKey string, roleBoundCIDRList []string) (bool, map[string]string, error) { - secretIDHMAC, err := createHMAC(hmacKey, secretID) +func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request, roleName, secretID string, + role *roleStorageEntry) (bool, map[string]string, error) { + secretIDHMAC, err := createHMAC(role.HMACKey, secretID) if err != nil { return false, nil, errwrap.Wrapf("failed to create HMAC of secret_id: {{err}}", err) } - roleNameHMAC, err := createHMAC(hmacKey, roleName) + roleNameHMAC, err := createHMAC(role.HMACKey, roleName) if err != nil { return false, nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) } - entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC) + entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) // SecretID locks are always index based on secretIDHMACs. This helps // acquiring the locks when the SecretIDs are listed. This allows grabbing @@ -176,7 +176,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request lock := b.secretIDLock(secretIDHMAC) lock.RLock() - result, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, roleNameHMAC, secretIDHMAC) + result, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) if err != nil { lock.RUnlock() return false, nil, err @@ -192,7 +192,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request // Ensure that the CIDRs on the secret ID are still a subset of that of // role's if err := verifyCIDRRoleSecretIDSubset(result.CIDRList, - roleBoundCIDRList); err != nil { + role.BoundCIDRList); err != nil { return false, nil, err } @@ -221,7 +221,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request defer lock.Unlock() // Lock switching may change the data. Refresh the contents. - result, err = b.nonLockedSecretIDStorageEntry(ctx, req.Storage, roleNameHMAC, secretIDHMAC) + result, err = b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) if err != nil { return false, nil, err } @@ -234,7 +234,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request // requests to use the same SecretID will fail. if result.SecretIDNumUses == 1 { // Delete the secret IDs accessor first - if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor); err != nil { + if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor, role.SecretIDPrefix); err != nil { return false, nil, err } if err := req.Storage.Delete(ctx, entryIndex); err != nil { @@ -254,7 +254,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request // Ensure that the CIDRs on the secret ID are still a subset of that of // role's if err := verifyCIDRRoleSecretIDSubset(result.CIDRList, - roleBoundCIDRList); err != nil { + role.BoundCIDRList); err != nil { return false, nil, err } @@ -313,7 +313,7 @@ func (b *backend) secretIDAccessorLock(secretIDAccessor string) *locksutil.LockE // storage. The entry will be indexed based on the given HMACs of both role // name and the secret ID. This method will not acquire secret ID lock to fetch // the storage entry. Locks need to be acquired before calling this method. -func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.Storage, roleNameHMAC, secretIDHMAC string) (*secretIDStorageEntry, error) { +func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.Storage, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC string) (*secretIDStorageEntry, error) { if secretIDHMAC == "" { return nil, fmt.Errorf("missing secret ID HMAC") } @@ -323,7 +323,7 @@ func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.S } // Prepare the storage index at which the secret ID will be stored - entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC) + entryIndex := fmt.Sprintf("%s%s/%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC) entry, err := s.Get(ctx, entryIndex) if err != nil { @@ -353,7 +353,7 @@ func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.S } if persistNeeded { - if err := b.nonLockedSetSecretIDStorageEntry(ctx, s, roleNameHMAC, secretIDHMAC, &result); err != nil { + if err := b.nonLockedSetSecretIDStorageEntry(ctx, s, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC, &result); err != nil { return nil, errwrap.Wrapf("failed to upgrade role storage entry {{err}}", err) } } @@ -366,7 +366,10 @@ func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.S // role name and the secret ID. This method will not acquire secret ID lock to // create/update the storage entry. Locks need to be acquired before calling // this method. -func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logical.Storage, roleNameHMAC, secretIDHMAC string, secretEntry *secretIDStorageEntry) error { +func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logical.Storage, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC string, secretEntry *secretIDStorageEntry) error { + if roleSecretIDPrefix == "" { + return fmt.Errorf("missing secret ID prefix") + } if secretIDHMAC == "" { return fmt.Errorf("missing secret ID HMAC") } @@ -379,7 +382,7 @@ func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logica return fmt.Errorf("nil secret entry") } - entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC) + entryIndex := fmt.Sprintf("%s%s/%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC) if entry, err := logical.StorageEntryJSON(entryIndex, secretEntry); err != nil { return err @@ -391,7 +394,7 @@ func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logica } // registerSecretIDEntry creates a new storage entry for the given SecretID. -func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage, roleName, secretID, hmacKey string, secretEntry *secretIDStorageEntry) (*secretIDStorageEntry, error) { +func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage, roleName, secretID, hmacKey, roleSecretIDPrefix string, secretEntry *secretIDStorageEntry) (*secretIDStorageEntry, error) { secretIDHMAC, err := createHMAC(hmacKey, secretID) if err != nil { return nil, errwrap.Wrapf("failed to create HMAC of secret ID: {{err}}", err) @@ -404,7 +407,7 @@ func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage, lock := b.secretIDLock(secretIDHMAC) lock.RLock() - entry, err := b.nonLockedSecretIDStorageEntry(ctx, s, roleNameHMAC, secretIDHMAC) + entry, err := b.nonLockedSecretIDStorageEntry(ctx, s, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC) if err != nil { lock.RUnlock() return nil, err @@ -421,7 +424,7 @@ func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage, defer lock.Unlock() // But before saving a new entry, check if the secretID entry was created during the lock switch. - entry, err = b.nonLockedSecretIDStorageEntry(ctx, s, roleNameHMAC, secretIDHMAC) + entry, err = b.nonLockedSecretIDStorageEntry(ctx, s, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC) if err != nil { return nil, err } @@ -450,11 +453,11 @@ func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage, } // Before storing the SecretID, store its accessor. - if err := b.createSecretIDAccessorEntry(ctx, s, secretEntry, secretIDHMAC); err != nil { + if err := b.createSecretIDAccessorEntry(ctx, s, secretEntry, secretIDHMAC, roleSecretIDPrefix); err != nil { return nil, err } - if err := b.nonLockedSetSecretIDStorageEntry(ctx, s, roleNameHMAC, secretIDHMAC, secretEntry); err != nil { + if err := b.nonLockedSetSecretIDStorageEntry(ctx, s, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC, secretEntry); err != nil { return nil, err } @@ -463,7 +466,7 @@ func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage, // secretIDAccessorEntry is used to read the storage entry that maps an // accessor to a secret_id. -func (b *backend) secretIDAccessorEntry(ctx context.Context, s logical.Storage, secretIDAccessor string) (*secretIDAccessorStorageEntry, error) { +func (b *backend) secretIDAccessorEntry(ctx context.Context, s logical.Storage, secretIDAccessor, roleSecretIDPrefix string) (*secretIDAccessorStorageEntry, error) { if secretIDAccessor == "" { return nil, fmt.Errorf("missing secretIDAccessor") } @@ -475,7 +478,11 @@ func (b *backend) secretIDAccessorEntry(ctx context.Context, s logical.Storage, if err != nil { return nil, err } - entryIndex := "accessor/" + salt.SaltID(secretIDAccessor) + accessorPrefix := secretIDAccessorPrefix + if roleSecretIDPrefix == secretIDLocalPrefix { + accessorPrefix = secretIDAccessorLocalPrefix + } + entryIndex := accessorPrefix + salt.SaltID(secretIDAccessor) accessorLock := b.secretIDAccessorLock(secretIDAccessor) accessorLock.RLock() @@ -495,7 +502,7 @@ func (b *backend) secretIDAccessorEntry(ctx context.Context, s logical.Storage, // createSecretIDAccessorEntry creates an identifier for the SecretID. A storage index, // mapping the accessor to the SecretID is also created. This method should // be called when the lock for the corresponding SecretID is held. -func (b *backend) createSecretIDAccessorEntry(ctx context.Context, s logical.Storage, entry *secretIDStorageEntry, secretIDHMAC string) error { +func (b *backend) createSecretIDAccessorEntry(ctx context.Context, s logical.Storage, entry *secretIDStorageEntry, secretIDHMAC, roleSecretIDPrefix string) error { // Create a random accessor accessorUUID, err := uuid.GenerateUUID() if err != nil { @@ -508,7 +515,12 @@ func (b *backend) createSecretIDAccessorEntry(ctx context.Context, s logical.Sto if err != nil { return err } - entryIndex := "accessor/" + salt.SaltID(entry.SecretIDAccessor) + + accessorPrefix := secretIDAccessorPrefix + if roleSecretIDPrefix == secretIDLocalPrefix { + accessorPrefix = secretIDAccessorLocalPrefix + } + entryIndex := accessorPrefix + salt.SaltID(entry.SecretIDAccessor) accessorLock := b.secretIDAccessorLock(accessorUUID) accessorLock.Lock() @@ -526,19 +538,24 @@ func (b *backend) createSecretIDAccessorEntry(ctx context.Context, s logical.Sto } // deleteSecretIDAccessorEntry deletes the storage index mapping the accessor to a SecretID. -func (b *backend) deleteSecretIDAccessorEntry(ctx context.Context, s logical.Storage, secretIDAccessor string) error { +func (b *backend) deleteSecretIDAccessorEntry(ctx context.Context, s logical.Storage, secretIDAccessor, roleSecretIDPrefix string) error { salt, err := b.Salt(ctx) if err != nil { return err } - accessorEntryIndex := "accessor/" + salt.SaltID(secretIDAccessor) + + accessorPrefix := secretIDAccessorPrefix + if roleSecretIDPrefix == secretIDLocalPrefix { + accessorPrefix = secretIDAccessorLocalPrefix + } + entryIndex := accessorPrefix + salt.SaltID(secretIDAccessor) accessorLock := b.secretIDAccessorLock(secretIDAccessor) accessorLock.Lock() defer accessorLock.Unlock() // Delete the accessor of the SecretID first - if err := s.Delete(ctx, accessorEntryIndex); err != nil { + if err := s.Delete(ctx, entryIndex); err != nil { return errwrap.Wrapf("failed to delete accessor storage entry: {{err}}", err) } @@ -547,7 +564,7 @@ func (b *backend) deleteSecretIDAccessorEntry(ctx context.Context, s logical.Sto // flushRoleSecrets deletes all the SecretIDs that belong to the given // RoleID. -func (b *backend) flushRoleSecrets(ctx context.Context, s logical.Storage, roleName, hmacKey string) error { +func (b *backend) flushRoleSecrets(ctx context.Context, s logical.Storage, roleName, hmacKey, roleSecretIDPrefix string) error { roleNameHMAC, err := createHMAC(hmacKey, roleName) if err != nil { return errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err) @@ -557,7 +574,7 @@ func (b *backend) flushRoleSecrets(ctx context.Context, s logical.Storage, roleN b.secretIDListingLock.RLock() defer b.secretIDListingLock.RUnlock() - secretIDHMACs, err := s.List(ctx, fmt.Sprintf("secret_id/%s/", roleNameHMAC)) + secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s/", roleSecretIDPrefix, roleNameHMAC)) if err != nil { return err } @@ -565,7 +582,7 @@ func (b *backend) flushRoleSecrets(ctx context.Context, s logical.Storage, roleN // Acquire the lock belonging to the SecretID lock := b.secretIDLock(secretIDHMAC) lock.Lock() - entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC) + entryIndex := fmt.Sprintf("%s%s/%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC) if err := s.Delete(ctx, entryIndex); err != nil { lock.Unlock() return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err) diff --git a/website/source/api/auth/approle/index.html.md b/website/source/api/auth/approle/index.html.md index 183921bae548..0d4ba1223204 100644 --- a/website/source/api/auth/approle/index.html.md +++ b/website/source/api/auth/approle/index.html.md @@ -94,6 +94,9 @@ enabled while creating or updating a role. but the TTL set on the token at each renewal is fixed to the value specified here. If this value is modified, the token will pick up the new value at its next renewal. +- `enable_local_secret_ids` `(bool: false)` - If set, the secret IDs generated + using this role will be cluster local. This can only be set during role + creation and once set, it can't be reset later. ### Sample Payload