diff --git a/changelog/27750.txt b/changelog/27750.txt new file mode 100644 index 000000000000..04c24fe59e7f --- /dev/null +++ b/changelog/27750.txt @@ -0,0 +1,3 @@ +```release-note:bug +core/identity: Fixed an issue where deleted/reassigned entity-aliases were not removed from in-memory database. +``` diff --git a/vault/identity_store.go b/vault/identity_store.go index 8d53f4c35682..22196269c8ff 100644 --- a/vault/identity_store.go +++ b/vault/identity_store.go @@ -721,34 +721,53 @@ func (i *IdentityStore) invalidateEntityBucket(ctx context.Context, key string) } } - // If the entity is not in MemDB or if it is but differs from the - // state that's in the bucket storage entry, upsert it into MemDB. - // We've considered the use of github.com/google/go-cmp here, // but opted for sticking with reflect.DeepEqual because go-cmp // is intended for testing and is able to panic in some // situations. - if memDBEntity == nil || !reflect.DeepEqual(memDBEntity, bucketEntity) { - // The entity is not in MemDB, it's a new entity. Add it to MemDB. - err = i.upsertEntityInTxn(ctx, txn, bucketEntity, nil, false) - if err != nil { - i.logger.Error("failed to update entity in MemDB", "entity_id", bucketEntity.ID, "error", err) + if memDBEntity != nil && reflect.DeepEqual(memDBEntity, bucketEntity) { + // No changes on this entity, move on to the next one. + continue + } + + // If the entity exists in MemDB it must differ from the entity in + // the storage bucket because of above test. Blindly delete the + // current aliases associated with the MemDB entity. The correct set + // of aliases will be created in MemDB by the upsertEntityInTxn + // function. We need to do this because the upsertEntityInTxn + // function does not delete those aliases, it only creates missing + // ones. + if memDBEntity != nil { + if err := i.deleteAliasesInEntityInTxn(txn, memDBEntity, memDBEntity.Aliases); err != nil { + i.logger.Error("failed to remove entity aliases from changed entity", "entity_id", memDBEntity.ID, "error", err) return } - // If this is a performance secondary, the entity created on - // this node would have been cached in a local cache based on - // the result of the CreateEntity RPC call to the primary - // cluster. Since this invalidation is signaling that the - // entity is now in the primary cluster's storage, the locally - // cached entry can be removed. - if i.localNode.ReplicationState().HasState(consts.ReplicationPerformanceSecondary) && i.localNode.HAState() == consts.Active { - if err := i.localAliasPacker.DeleteItem(ctx, bucketEntity.ID+tmpSuffix); err != nil { - i.logger.Error("failed to clear local alias entity cache", "error", err, "entity_id", bucketEntity.ID) - return - } + if err := i.MemDBDeleteEntityByIDInTxn(txn, memDBEntity.ID); err != nil { + i.logger.Error("failed to delete changed entity", "entity_id", memDBEntity.ID, "error", err) + return } } + + err = i.upsertEntityInTxn(ctx, txn, bucketEntity, nil, false) + if err != nil { + i.logger.Error("failed to update entity in MemDB", "entity_id", bucketEntity.ID, "error", err) + return + } + + // If this is a performance secondary, the entity created on + // this node would have been cached in a local cache based on + // the result of the CreateEntity RPC call to the primary + // cluster. Since this invalidation is signaling that the + // entity is now in the primary cluster's storage, the locally + // cached entry can be removed. + if i.localNode.ReplicationState().HasState(consts.ReplicationPerformanceSecondary) && i.localNode.HAState() == consts.Active { + if err := i.localAliasPacker.DeleteItem(ctx, bucketEntity.ID+tmpSuffix); err != nil { + i.logger.Error("failed to clear local alias entity cache", "error", err, "entity_id", bucketEntity.ID) + return + } + } + } } diff --git a/vault/identity_store_test.go b/vault/identity_store_test.go index 7c826dfa0c33..da16ae6fac92 100644 --- a/vault/identity_store_test.go +++ b/vault/identity_store_test.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/anypb" ) @@ -992,6 +993,278 @@ func TestIdentityStoreInvalidate_Entities(t *testing.T) { txn.Commit() } +// TestIdentityStoreInvalidate_EntityAliasDelete verifies that the +// invalidateEntityBucket method properly cleans up aliases from +// MemDB that are no longer associated with the entity in the +// storage bucket. +func TestIdentityStoreInvalidate_EntityAliasDelete(t *testing.T) { + ctx := namespace.ContextWithNamespace(context.Background(), namespace.RootNamespace) + c, _, root := TestCoreUnsealed(t) + + // Enable a No-Op auth method + c.credentialBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { + return &NoopBackend{ + BackendType: logical.TypeCredential, + }, nil + } + mountAccessor1 := "noop-accessor1" + mountAccessor2 := "noop-accessor2" + mountAccessor3 := "noon-accessor3" + + createMountEntry := func(path, uuid, mountAccessor string, local bool) *MountEntry { + return &MountEntry{ + Table: credentialTableType, + Path: path, + Type: "noop", + UUID: uuid, + Accessor: mountAccessor, + BackendAwareUUID: uuid + "backend", + NamespaceID: namespace.RootNamespaceID, + namespace: namespace.RootNamespace, + Local: local, + } + } + + c.auth = &MountTable{ + Type: credentialTableType, + Entries: []*MountEntry{ + createMountEntry("/noop1", "abcd", mountAccessor1, false), + createMountEntry("/noop2", "ghij", mountAccessor2, false), + createMountEntry("/noop3", "mnop", mountAccessor3, true), + }, + } + + require.NoError(t, c.setupCredentials(context.Background())) + + // Create an entity + req := &logical.Request{ + ClientToken: root, + Operation: logical.UpdateOperation, + Path: "entity", + Data: map[string]interface{}{ + "name": "alice", + }, + } + + resp, err := c.identityStore.HandleRequest(ctx, req) + require.NoError(t, err) + require.NotNil(t, resp) + require.Contains(t, resp.Data, "id") + + entityID := resp.Data["id"].(string) + + createEntityAlias := func(name, mountAccessor string) string { + req = &logical.Request{ + ClientToken: root, + Operation: logical.UpdateOperation, + Path: "entity-alias", + Data: map[string]interface{}{ + "name": name, + "canonical_id": entityID, + "mount_accessor": mountAccessor, + }, + } + + resp, err = c.identityStore.HandleRequest(ctx, req) + require.NoError(t, err) + require.NotNil(t, resp) + require.Contains(t, resp.Data, "id") + + return resp.Data["id"].(string) + } + + alias1ID := createEntityAlias("alias1", mountAccessor1) + alias2ID := createEntityAlias("alias2", mountAccessor2) + alias3ID := createEntityAlias("alias3", mountAccessor3) + + // Update the entity in storage only to remove alias2 then call invalidate + bucketKey := c.identityStore.entityPacker.BucketKey(entityID) + bucket, err := c.identityStore.entityPacker.GetBucket(context.Background(), bucketKey) + require.NoError(t, err) + require.NotNil(t, bucket) + + bucketEntityItem := bucket.Items[0] // since there's only 1 entity + bucketEntity, err := c.identityStore.parseEntityFromBucketItem(context.Background(), bucketEntityItem) + require.NoError(t, err) + require.NotNil(t, bucketEntity) + + replacementAliases := make([]*identity.Alias, 1) + for _, a := range bucketEntity.Aliases { + if a.ID != alias2ID { + replacementAliases[0] = a + break + } + } + + bucketEntity.Aliases = replacementAliases + + bucketEntityItem.Message, err = anypb.New(bucketEntity) + require.NoError(t, err) + + require.NoError(t, c.identityStore.entityPacker.PutItem(context.Background(), bucketEntityItem)) + + c.identityStore.Invalidate(context.Background(), bucketKey) + + alias1, err := c.identityStore.MemDBAliasByID(alias1ID, false, false) + assert.NoError(t, err) + assert.NotNil(t, alias1) + + alias2, err := c.identityStore.MemDBAliasByID(alias2ID, false, false) + assert.NoError(t, err) + assert.Nil(t, alias2) + + alias3, err := c.identityStore.MemDBAliasByID(alias3ID, false, false) + assert.NoError(t, err) + assert.NotNil(t, alias3) +} + +// TestIdentityStoreInvalidate_EntityLocalAliasDelete verifies that the +// invalidateLocalAliasesBucket method properly cleans up aliases from +// MemDB that are no longer associated with the entity in the +// storage bucket. +func TestIdentityStoreInvalidate_EntityLocalAliasDelete(t *testing.T) { + ctx := namespace.ContextWithNamespace(context.Background(), namespace.RootNamespace) + c, _, root := TestCoreUnsealed(t) + + // Enable a No-Op auth method + c.credentialBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { + return &NoopBackend{ + BackendType: logical.TypeCredential, + }, nil + } + mountAccessor1 := "noop-accessor1" + mountAccessor2 := "noop-accessor2" + mountAccessor3 := "noon-accessor3" + + createMountEntry := func(path, uuid, mountAccessor string, local bool) *MountEntry { + return &MountEntry{ + Table: credentialTableType, + Path: path, + Type: "noop", + UUID: uuid, + Accessor: mountAccessor, + BackendAwareUUID: uuid + "backend", + NamespaceID: namespace.RootNamespaceID, + namespace: namespace.RootNamespace, + Local: local, + } + } + + c.auth = &MountTable{ + Type: credentialTableType, + Entries: []*MountEntry{ + createMountEntry("/noop1", "abcd", mountAccessor1, true), + createMountEntry("/noop2", "ghij", mountAccessor2, true), + createMountEntry("/noop3", "mnop", mountAccessor3, true), + }, + } + + require.NoError(t, c.setupCredentials(context.Background())) + + // Create an entity + req := &logical.Request{ + ClientToken: root, + Operation: logical.UpdateOperation, + Path: "entity", + Data: map[string]interface{}{ + "name": "alice", + }, + } + + resp, err := c.identityStore.HandleRequest(ctx, req) + require.NoError(t, err) + require.NotNil(t, resp) + require.Contains(t, resp.Data, "id") + + entityID := resp.Data["id"].(string) + + createEntityAlias := func(name, mountAccessor string) string { + req = &logical.Request{ + ClientToken: root, + Operation: logical.UpdateOperation, + Path: "entity-alias", + Data: map[string]interface{}{ + "name": name, + "canonical_id": entityID, + "mount_accessor": mountAccessor, + }, + } + + resp, err = c.identityStore.HandleRequest(ctx, req) + require.NoError(t, err) + require.NotNil(t, resp) + require.Contains(t, resp.Data, "id") + + return resp.Data["id"].(string) + } + + alias1ID := createEntityAlias("alias1", mountAccessor1) + alias2ID := createEntityAlias("alias2", mountAccessor2) + alias3ID := createEntityAlias("alias3", mountAccessor3) + + for i, aliasID := range []string{alias1ID, alias2ID, alias3ID} { + alias, err := c.identityStore.MemDBAliasByID(aliasID, false, false) + require.NoError(t, err, i) + require.NotNil(t, alias, i) + } + + // // Update the entity in storage only to remove alias2 then call invalidate + bucketKey := c.identityStore.entityPacker.BucketKey(entityID) + bucket, err := c.identityStore.entityPacker.GetBucket(context.Background(), bucketKey) + require.NoError(t, err) + require.NotNil(t, bucket) + + bucketEntityItem := bucket.Items[0] // since there's only 1 entity + bucketEntity, err := c.identityStore.parseEntityFromBucketItem(context.Background(), bucketEntityItem) + require.NoError(t, err) + require.NotNil(t, bucketEntity) + + bucketKey = c.identityStore.localAliasPacker.BucketKey(entityID) + bucketLocalAlias, err := c.identityStore.localAliasPacker.GetBucket(context.Background(), bucketKey) + require.NoError(t, err) + require.NotNil(t, bucketLocalAlias) + + bucketLocalAliasItem := bucketLocalAlias.Items[0] + require.Equal(t, entityID, bucketLocalAliasItem.ID) + + var localAliases identity.LocalAliases + + err = anypb.UnmarshalTo(bucketLocalAliasItem.Message, &localAliases, proto.UnmarshalOptions{}) + require.NoError(t, err) + + memDBEntity, err := c.identityStore.MemDBEntityByID(entityID, false) + require.NoError(t, err) + require.NotNil(t, memDBEntity) + + replacementAliases := make([]*identity.Alias, 0) + for _, a := range memDBEntity.Aliases { + if a.ID != alias2ID { + replacementAliases = append(replacementAliases, a) + } + } + + localAliases.Aliases = replacementAliases + + bucketLocalAliasItem.Message, err = anypb.New(&localAliases) + require.NoError(t, err) + + require.NoError(t, c.identityStore.localAliasPacker.PutItem(context.Background(), bucketLocalAliasItem)) + + c.identityStore.Invalidate(context.Background(), bucketKey) + + alias1, err := c.identityStore.MemDBAliasByID(alias1ID, false, false) + assert.NoError(t, err) + assert.NotNil(t, alias1) + + alias2, err := c.identityStore.MemDBAliasByID(alias2ID, false, false) + assert.NoError(t, err) + assert.Nil(t, alias2) + + alias3, err := c.identityStore.MemDBAliasByID(alias3ID, false, false) + assert.NoError(t, err) + assert.NotNil(t, alias3) +} + // TestIdentityStoreInvalidate_LocalAliasesWithEntity verifies the correct // handling of local aliases in the Invalidate method. func TestIdentityStoreInvalidate_LocalAliasesWithEntity(t *testing.T) { diff --git a/website/content/docs/release-notes/1.16.1.mdx b/website/content/docs/release-notes/1.16.1.mdx index f9f79bf884b7..a5a324516240 100644 --- a/website/content/docs/release-notes/1.16.1.mdx +++ b/website/content/docs/release-notes/1.16.1.mdx @@ -24,6 +24,7 @@ description: |- | 1.16.1 - 1.16.3 | [New nodes added by autopilot upgrades provisioned with the wrong version](/vault/docs/upgrading/upgrade-to-1.15.x#new-nodes-added-by-autopilot-upgrades-provisioned-with-the-wrong-version) | | 1.15.8+ | [Autopilot upgrade for Vault Enterprise fails](/vault/docs/upgrading/upgrade-to-1.15.x#autopilot) | | 1.16.5 | [Listener stops listening on untrusted upstream connection with particular config settings](/vault/docs/upgrading/upgrade-to-1.16.x#listener-proxy-protocol-config) | +| 1.16.3 - 1.16.6 | [Vault standby nodes not deleting removed entity-aliases from in-memory database](/vault/docs/upgrade-to-1.16.x#dangling-entity-alias-in-memory) | ## Vault companion updates diff --git a/website/content/docs/release-notes/1.17.0.mdx b/website/content/docs/release-notes/1.17.0.mdx index e0557991f715..7bfeaf4b707d 100644 --- a/website/content/docs/release-notes/1.17.0.mdx +++ b/website/content/docs/release-notes/1.17.0.mdx @@ -22,6 +22,7 @@ description: |- | Known issue (1.17.0) | [Vault Agent and Vault Proxy consume excessive amounts of CPU](/vault/docs/upgrading/upgrade-to-1.17.x#agent-proxy-cpu-1-17) | | Known issue (1.15.8+) | [Autopilot upgrade for Vault Enterprise fails](/vault/docs/upgrading/upgrade-to-1.15.x#autopilot) | | Known issue (1.17.1) | [Listener stops listening on untrusted upstream connection with particular config settings](/vault/docs/upgrading/upgrade-to-1.17.x#listener-proxy-protocol-config) | +| Known issue (1.17.0 - 1.17.2) | [Vault standby nodes not deleting removed entity-aliases from in-memory database](/vault/docs/upgrade-to-1.17.x#dangling-entity-alias-in-memory) ## Vault companion updates diff --git a/website/content/docs/upgrading/upgrade-to-1.16.x.mdx b/website/content/docs/upgrading/upgrade-to-1.16.x.mdx index 0e3d6ea7f436..74c511f0b3a7 100644 --- a/website/content/docs/upgrading/upgrade-to-1.16.x.mdx +++ b/website/content/docs/upgrading/upgrade-to-1.16.x.mdx @@ -115,3 +115,5 @@ decides to trigger the flag. More information can be found in the @include 'known-issues/1_16_secrets-sync-chroot-activation.mdx' @include 'known-issues/config_listener_proxy_protocol_behavior_issue.mdx' + +@include 'known-issues/dangling-entity-aliases-in-memory.mdx' diff --git a/website/content/docs/upgrading/upgrade-to-1.17.x.mdx b/website/content/docs/upgrading/upgrade-to-1.17.x.mdx index f0651364ddf5..813d922f39bf 100644 --- a/website/content/docs/upgrading/upgrade-to-1.17.x.mdx +++ b/website/content/docs/upgrading/upgrade-to-1.17.x.mdx @@ -90,3 +90,5 @@ incorrectly. For additional details, refer to the @include 'known-issues/config_listener_proxy_protocol_behavior_issue.mdx' @include 'known-issues/transit-input-on-cmac-response.mdx' + +@include 'known-issues/dangling-entity-aliases-in-memory.mdx' diff --git a/website/content/partials/known-issues/dangling-entity-aliases-in-memory.mdx b/website/content/partials/known-issues/dangling-entity-aliases-in-memory.mdx new file mode 100644 index 000000000000..c825725552e3 --- /dev/null +++ b/website/content/partials/known-issues/dangling-entity-aliases-in-memory.mdx @@ -0,0 +1,24 @@ + + +### Deleting an entity-aliases does not remove it from the in-memory database on standby nodes + +#### Affected versions + +##### Vault Community Edition + +- 1.16.3 +- 1.17.0 - 1.17.2 + +##### Enterprise + +- 1.16.3+ent - 1.16.6+ent +- 1.17.0+ent - 1.17.2+ent + +#### Issue + +Entity-aliases deleted from Vault are not removed from the in-memory database on +standby nodes within a cluster. As a result, API requests to create a new +entity-alias with the same mount accessor and name sent to a standby node will +fail. + +This bug is fixed in Vault 1.16.7+ent, 1.17.3, 1.17.3+ent and later.