From f5bb678c98b4e14a0a23bf19a8cd42bb8d8fdec4 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Mon, 17 Jul 2023 12:46:00 -0400 Subject: [PATCH] backport of commit e1a9d85a18858bccf4de71ddc0ce7592170ab894 (#21879) Co-authored-by: akshya96 <87045294+akshya96@users.noreply.github.com> --- changelog/20965.txt | 3 ++ vault/core.go | 4 --- vault/identity_store.go | 66 ++++++++++++------------------------ vault/identity_store_test.go | 56 ++++++++++++++++++++++++++++++ vault/identity_store_util.go | 22 +++++------- 5 files changed, 89 insertions(+), 62 deletions(-) create mode 100644 changelog/20965.txt diff --git a/changelog/20965.txt b/changelog/20965.txt new file mode 100644 index 000000000..43c1d97cc --- /dev/null +++ b/changelog/20965.txt @@ -0,0 +1,3 @@ +```release-note:bug +identity: Remove caseSensitivityKey to prevent errors while loading groups which could result in missing groups in memDB when duplicates are found. +``` \ No newline at end of file diff --git a/vault/core.go b/vault/core.go index e36a882d0..b19927469 100644 --- a/vault/core.go +++ b/vault/core.go @@ -593,10 +593,6 @@ type Core struct { // active, or give up active as soon as it gets it neverBecomeActive *uint32 - // loadCaseSensitiveIdentityStore enforces the loading of identity store - // artifacts in a case sensitive manner. To be used only in testing. - loadCaseSensitiveIdentityStore bool - // clusterListener starts up and manages connections on the cluster ports clusterListener *atomic.Value diff --git a/vault/identity_store.go b/vault/identity_store.go index e6beec3b2..bcdfdb2c1 100644 --- a/vault/identity_store.go +++ b/vault/identity_store.go @@ -646,14 +646,31 @@ func (i *IdentityStore) initialize(ctx context.Context, req *logical.Initializat return err } - entry, err := logical.StorageEntryJSON(caseSensitivityKey, &casesensitivity{ - DisableLowerCasedNames: i.disableLowerCasedNames, - }) + // if the storage entry for caseSensitivityKey exists, remove it + storageEntry, err := i.view.Get(ctx, caseSensitivityKey) if err != nil { - return err + i.logger.Error("could not get storage entry for case sensitivity key", "error", err) + return nil } - return i.view.Put(ctx, entry) + if storageEntry != nil { + var setting casesensitivity + err := storageEntry.DecodeJSON(&setting) + switch err { + case nil: + i.logger.Debug("removing storage entry for case sensitivity key", "value", setting.DisableLowerCasedNames) + default: + i.logger.Error("failed to decode case sensitivity key, removing its storage entry anyway", "error", err) + } + + err = i.view.Delete(ctx, caseSensitivityKey) + if err != nil { + i.logger.Error("could not delete storage entry for case sensitivity key", "error", err) + return nil + } + } + + return nil } // Invalidate is a callback wherein the backend is informed that the value at @@ -667,45 +684,6 @@ func (i *IdentityStore) Invalidate(ctx context.Context, key string) { defer i.lock.Unlock() switch { - case key == caseSensitivityKey: - entry, err := i.view.Get(ctx, caseSensitivityKey) - if err != nil { - i.logger.Error("failed to read case sensitivity setting during invalidation", "error", err) - return - } - if entry == nil { - return - } - - var setting casesensitivity - if err := entry.DecodeJSON(&setting); err != nil { - i.logger.Error("failed to decode case sensitivity setting during invalidation", "error", err) - return - } - - // Fast return if the setting is the same - if i.disableLowerCasedNames == setting.DisableLowerCasedNames { - return - } - - // If the setting is different, reset memdb and reload all the artifacts - i.disableLowerCasedNames = setting.DisableLowerCasedNames - if err := i.resetDB(ctx); err != nil { - i.logger.Error("failed to reset memdb during invalidation", "error", err) - return - } - if err := i.loadEntities(ctx); err != nil { - i.logger.Error("failed to load entities during invalidation", "error", err) - return - } - if err := i.loadGroups(ctx); err != nil { - i.logger.Error("failed to load groups during invalidation", "error", err) - return - } - if err := i.loadOIDCClients(ctx); err != nil { - i.logger.Error("failed to load OIDC clients during invalidation", "error", err) - return - } // Check if the key is a storage entry key for an entity bucket case strings.HasPrefix(key, storagepacker.StoragePackerBucketsPrefix): // Create a MemDB transaction diff --git a/vault/identity_store_test.go b/vault/identity_store_test.go index f9dc7cac1..7fa0b93b0 100644 --- a/vault/identity_store_test.go +++ b/vault/identity_store_test.go @@ -857,3 +857,59 @@ func TestIdentityStore_UpdateAliasMetadataPerAccessor(t *testing.T) { t.Fatalf("wrong alias index changed. Expected 1, got %d", i) } } + +// TestIdentityStore_DeleteCaseSensitivityKey tests that +// casesensitivity key gets removed from storage if it exists upon +// initializing identity store. +func TestIdentityStore_DeleteCaseSensitivityKey(t *testing.T) { + c, unsealKey, root := TestCoreUnsealed(t) + ctx := context.Background() + + // add caseSensitivityKey to storage + entry, err := logical.StorageEntryJSON(caseSensitivityKey, &casesensitivity{ + DisableLowerCasedNames: true, + }) + if err != nil { + t.Fatal(err) + } + err = c.identityStore.view.Put(ctx, entry) + if err != nil { + t.Fatal(err) + } + + // check if the value is stored in storage + storageEntry, err := c.identityStore.view.Get(ctx, caseSensitivityKey) + if err != nil { + t.Fatal(err) + } + + if storageEntry == nil { + t.Fatalf("bad: expected a non-nil entry for casesensitivity key") + } + + // Seal and unseal to trigger identityStore initialize + if err = c.Seal(root); err != nil { + t.Fatal(err) + } + + var unsealed bool + for i := 0; i < len(unsealKey); i++ { + unsealed, err = c.Unseal(unsealKey[i]) + if err != nil { + t.Fatal(err) + } + } + if !unsealed { + t.Fatal("still sealed") + } + + // check if caseSensitivityKey exists after initialize + storageEntry, err = c.identityStore.view.Get(ctx, caseSensitivityKey) + if err != nil { + t.Fatal(err) + } + + if storageEntry != nil { + t.Fatalf("bad: expected no entry for casesensitivity key") + } +} diff --git a/vault/identity_store_util.go b/vault/identity_store_util.go index 4310fdf6a..b84db8dd5 100644 --- a/vault/identity_store_util.go +++ b/vault/identity_store_util.go @@ -31,10 +31,6 @@ var ( tmpSuffix = ".tmp" ) -func (c *Core) SetLoadCaseSensitiveIdentityStore(caseSensitive bool) { - c.loadCaseSensitiveIdentityStore = caseSensitive -} - func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error { if c.identityStore == nil { c.logger.Warn("identity store is not setup, skipping loading") @@ -58,16 +54,14 @@ func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error { return nil } - if !c.loadCaseSensitiveIdentityStore { - // Load everything when memdb is set to operate on lower cased names - err := loadFunc(ctx) - switch { - case err == nil: - // If it succeeds, all is well - return nil - case !errwrap.Contains(err, errDuplicateIdentityName.Error()): - return err - } + // Load everything when memdb is set to operate on lower cased names + err := loadFunc(ctx) + switch { + case err == nil: + // If it succeeds, all is well + return nil + case !errwrap.Contains(err, errDuplicateIdentityName.Error()): + return err } c.identityStore.logger.Warn("enabling case sensitive identity names")