scottk/vault
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/vault.git synced 2025-10-29 17:52:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

identity: Resolve conflicts with rename (#29356)

Browse Source 
This PR introduces a new type of conflict resolution for duplicate
Entities and Groups. Renaming provides a way of preventing Vault from
entering case-sensitive mode, which is the current behavior for any kind
of duplicate.

Renames append the conflicting identity artifact's UUID to its name and
updates a metadata field to indicate the pre-existing artifact's UUID.

The feature is gated by the force-identity-deduplication activation flag.

In order to maintain consistent behavior between the reporting resolver
and the rename operation, we need to adjust the behavior of generated
reports. Previously, they intentionally preserved existing Group merge
determinism, wherein the last MemDB update would win and all others
would be renamed. This approach is more complicated for the rename
resolver, since we would need to update any duplicated entity in the
cache while inserting the new duplicate (resulting in two MemDB
operations). Though we can ensure atomic updates of the two identity
artifacts with transactions (which we could get for groups with a minor
adjustment, and we will get along with batching of Entity upserts on 
load), it's far simpler to just rename all but the first insert as proposed
in the current PR.

Since the feature is gated by an activation flag with appropriate 
warnings of potential changes via the reporting resolver, we opt
for simplicity over maintaining pre-existing behavior. We can revisit
this assumption later if we think alignment with existing behavior
outweighs any potential complexity in the rename operation.

Entity alias resolution is left alone as a destructive merge operation
to prevent a potentially high-impact change in existing behavior.
This commit is contained in:
Mike Palmiotto
2025-01-15 14:24:49 -05:00
committed by GitHub
parent a0ecbe993c
commit f503f739de
11 changed files with 275 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
changelog/29356.txt Normal file
View File

@@ -0,0 +1,5 @@
```release-note:feature
**Identity De-duplication**: Vault can now automatically resolve duplicate
Entities and Groups by renaming them. This feature is disabled by default and
can be enabled through the `force_identity_deduplication` activation flag.
```

1
helper/activationflags/activation_flags.go
View File

@@ -14,6 +14,7 @@ import (
const (
storagePathActivationFlags = "activation-flags"
IdentityDeduplication = "force-identity-deduplication"
)
type FeatureActivationFlags struct {

16
helper/identity/identity.go
View File

@@ -58,6 +58,22 @@ func (e *Entity) UpsertAlias(alias *Alias) {
e.Aliases = append(e.Aliases, alias)
}
func (e *Entity) DeleteAliasByID(aliasID string) {
idx := -1
for i, item := range e.Aliases {
if item.ID == aliasID {
idx = i
break
}
}
if idx < 0 {
return
}
e.Aliases = append(e.Aliases[:idx], e.Aliases[idx+1:]...)
}
func (p *Alias) Clone() (*Alias, error) {
if p == nil {
return nil, fmt.Errorf("nil alias")

4
vault/identity_store.go
View File

@@ -45,7 +45,7 @@ func (c *Core) IdentityStore() *IdentityStore {
return c.identityStore
}
func (i *IdentityStore) resetDB(ctx context.Context) error {
func (i *IdentityStore) resetDB() error {
var err error
i.db, err = memdb.NewMemDB(identityStoreSchema(!i.disableLowerCasedNames))
@@ -76,7 +76,7 @@ func NewIdentityStore(ctx context.Context, core *Core, config *logical.BackendCo
// Create a memdb instance, which by default, operates on lower cased
// identity names
err := iStore.resetDB(ctx)
err := iStore.resetDB()
if err != nil {
return nil, err
}

75
vault/identity_store_conflicts.go
View File

@@ -27,7 +27,7 @@ type ConflictResolver interface {
ResolveAliases(ctx context.Context, parent *identity.Entity, existing, duplicate *identity.Alias) error
}
// The errorResolver is a ConflictResolver that logs a warning message when a
// errorResolver is a ConflictResolver that logs a warning message when a
// pre-existing Identity artifact is found with the same factors as a new one.
type errorResolver struct {
logger log.Logger
@@ -207,7 +207,7 @@ func (r *duplicateReportingErrorResolver) Report() identityDuplicateReport {
index: idx,
numOthers: len(entities) - 1,
}
if idx < len(entities)-1 {
if idx > 0 {
r.resolutionHint = fmt.Sprintf("would rename to %s-%s", entity.Name, entity.ID)
} else {
r.resolutionHint = "would not rename"
@@ -234,7 +234,7 @@ func (r *duplicateReportingErrorResolver) Report() identityDuplicateReport {
index: idx,
numOthers: len(groups) - 1,
}
if idx < len(groups)-1 {
if idx > 0 {
r.resolutionHint = fmt.Sprintf("would rename to %s-%s", group.Name, group.ID)
} else {
r.resolutionHint = "would not rename"
@@ -278,8 +278,8 @@ func reportAliases(report *identityDuplicateReport, seen map[string][]*identity.
index: idx,
numOthers: len(aliases) - 1,
}
if idx < len(aliases)-1 {
r.resolutionHint = fmt.Sprintf("would merge into entity %s", aliases[len(aliases)-1].CanonicalID)
if idx > 0 {
r.resolutionHint = fmt.Sprintf("would merge into entity %s", aliases[0].CanonicalID)
} else {
r.resolutionHint = "would merge others into this entity"
}
@@ -361,3 +361,68 @@ func (r *duplicateReportingErrorResolver) LogReport(log Warner) {
log.Warn("end of identity duplicate report, refer to " +
identityDuplicateReportUrl + " for resolution.")
}
// renameResolver is a ConflictResolver that appends the artifact's UUID to its
// name to resolve potential conflicts. The renamed resource is associated with
// the duplicated artifact by adding a `duplicate_of_canonical_id` metadata
// field.
type renameResolver struct {
logger log.Logger
}
// ResolveEntities renames an entity duplicate in a deterministic way so that
// all entities end up addressable by a unique name still. We rename the
// pre-existing entity such that only the last occurrence retains its unmodified
// name. Note that this is potentially destructive but is the best option
// available to resolve duplicates in storage caused by bugs in our validation.
func (r *renameResolver) ResolveEntities(ctx context.Context, existing, duplicate *identity.Entity) error {
if existing == nil {
return nil
}
duplicate.Name = duplicate.Name + "-" + duplicate.ID
if duplicate.Metadata == nil {
duplicate.Metadata = make(map[string]string)
}
duplicate.Metadata["duplicate_of_canonical_id"] = existing.ID
r.logger.Warn("renaming entity with duplicate name",
"namespace_id", duplicate.NamespaceID,
"entity_id", duplicate.ID,
"duplicate_of_canonical_id", existing.ID,
"renamed_from", duplicate.Name,
"renamed_to", duplicate.Name,
)
return nil
}
// ResolveGroups deals with group name duplicates by renaming those that
// were "hidden" in memDB so they are queryable. It's important this is
// deterministic so we don't end up with different group names on different
// nodes. We use the ID to ensure the new name is unique bit also
// deterministic. For now, don't persist this. The user can choose to
// resolve it permanently by renaming or deleting explicitly.
func (r *renameResolver) ResolveGroups(ctx context.Context, existing, duplicate *identity.Group) error {
if existing == nil {
return nil
}
duplicate.Name = duplicate.Name + "-" + duplicate.ID
if duplicate.Metadata == nil {
duplicate.Metadata = make(map[string]string)
}
duplicate.Metadata["duplicate_of_canonical_id"] = existing.ID
r.logger.Warn("renaming group with duplicate name",
"namespace_id", duplicate.NamespaceID,
"group_id", duplicate.ID,
"duplicate_of_canonical_id", existing.ID,
"new_name", duplicate.Name,
)
return nil
}
// ResolveAliases is a no-op for the renameResolver implementation.
func (r *renameResolver) ResolveAliases(ctx context.Context, parent *identity.Entity, existing, duplicate *identity.Alias) error {
return nil
}

121
vault/identity_store_conflicts_test.go
View File

@@ -67,37 +67,37 @@ func TestDuplicateReportingErrorResolver(t *testing.T) {
expectReport := `
DUPLICATES DETECTED, see following logs for details and refer to https://developer.hashicorp.com/vault/docs/upgrading/identity-deduplication for resolution.:
1 different-case local entity alias duplicates found (potential security risk):
local entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" canonical_id="11111111-0000-0000-0000-000000000009" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000010"
local entity-alias "different-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000010" canonical_id="11111111-0000-0000-0000-000000000010" force_deduplication="would merge others into this entity"
local entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" canonical_id="11111111-0000-0000-0000-000000000009" force_deduplication="would merge others into this entity"
local entity-alias "different-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000010" canonical_id="11111111-0000-0000-0000-000000000010" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000009"
end of different-case local entity-alias duplicates:
2 different-case entity alias duplicates found (potential security risk):
entity-alias "different-case-alias-1" with mount accessor "mount1" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" canonical_id="11111111-0000-0000-0000-000000000004" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000005"
entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "mount1" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" canonical_id="11111111-0000-0000-0000-000000000005" force_deduplication="would merge others into this entity"
entity-alias "different-case-alias-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000006" canonical_id="11111111-0000-0000-0000-000000000006" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000008"
entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000007" canonical_id="11111111-0000-0000-0000-000000000007" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000008"
entity-alias "different-CASE-ALIAS-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000008" canonical_id="11111111-0000-0000-0000-000000000008" force_deduplication="would merge others into this entity"
entity-alias "different-case-alias-1" with mount accessor "mount1" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" canonical_id="11111111-0000-0000-0000-000000000004" force_deduplication="would merge others into this entity"
entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "mount1" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" canonical_id="11111111-0000-0000-0000-000000000005" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000004"
entity-alias "different-case-alias-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000006" canonical_id="11111111-0000-0000-0000-000000000006" force_deduplication="would merge others into this entity"
entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000007" canonical_id="11111111-0000-0000-0000-000000000007" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000006"
entity-alias "different-CASE-ALIAS-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000008" canonical_id="11111111-0000-0000-0000-000000000008" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000006"
end of different-case entity-alias duplicates:
4 entity duplicates found:
entity "different-case-dupe-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000010" force_deduplication="would rename to different-case-dupe-1-00000000-0000-0000-0000-000000000010"
entity "different-case-dupe-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000010" force_deduplication="would not rename"
entity "DIFFERENT-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000011" force_deduplication="would rename to DIFFERENT-case-DUPE-1-00000000-0000-0000-0000-000000000011"
entity "different-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000012" force_deduplication="would not rename"
entity "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000006" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000006"
entity "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000007" force_deduplication="would not rename"
entity "different-case-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000008" force_deduplication="would rename to different-case-dupe-1-00000000-0000-0000-0000-000000000008"
entity "DIFFERENT-CASE-DUPE-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" force_deduplication="would not rename"
entity "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000004"
entity "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" force_deduplication="would not rename"
entity "different-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000012" force_deduplication="would rename to different-case-DUPE-1-00000000-0000-0000-0000-000000000012"
entity "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000006" force_deduplication="would not rename"
entity "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000007" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000007"
entity "different-case-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000008" force_deduplication="would not rename"
entity "DIFFERENT-CASE-DUPE-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" force_deduplication="would rename to DIFFERENT-CASE-DUPE-1-00000000-0000-0000-0000-000000000009"
entity "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" force_deduplication="would not rename"
entity "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000005"
end of entity duplicates:
4 group duplicates found:
group "different-case-dupe-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000010" force_deduplication="would rename to different-case-dupe-1-00000000-0000-0000-0000-000000000010"
group "different-case-dupe-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000010" force_deduplication="would not rename"
group "DIFFERENT-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000011" force_deduplication="would rename to DIFFERENT-case-DUPE-1-00000000-0000-0000-0000-000000000011"
group "different-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000012" force_deduplication="would not rename"
group "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000006" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000006"
group "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000007" force_deduplication="would not rename"
group "different-case-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000008" force_deduplication="would rename to different-case-dupe-1-00000000-0000-0000-0000-000000000008"
group "DIFFERENT-CASE-DUPE-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" force_deduplication="would not rename"
group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000004"
group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" force_deduplication="would not rename"
group "different-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000012" force_deduplication="would rename to different-case-DUPE-1-00000000-0000-0000-0000-000000000012"
group "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000006" force_deduplication="would not rename"
group "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000007" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000007"
group "different-case-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000008" force_deduplication="would not rename"
group "DIFFERENT-CASE-DUPE-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" force_deduplication="would rename to DIFFERENT-CASE-DUPE-1-00000000-0000-0000-0000-000000000009"
group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" force_deduplication="would not rename"
group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000005"
end of group duplicates:
end of identity duplicate report, refer to https://developer.hashicorp.com/vault/docs/upgrading/identity-deduplication for resolution.:
`
@@ -178,3 +178,78 @@ func (l *identityTestWarnLogger) Warn(msg string, args ...interface{}) {
}
l.buf.WriteString("\n")
}
// TestDuplicateRenameResolver tests that the rename resolver
// correctly renames pre-existing entities and groups.
func TestDuplicateRenameResolver(t *testing.T) {
t.Parallel()
entities := map[string][]string{
// 2 non-duplicates and 2 duplicates
"root": {
"foo",
"bar",
"exact-dupe-1", "exact-dupe-1",
"different-case-dupe-1", "DIFFERENT-CASE-DUPE-1",
},
// 1 non-duplicate and 3 duplicates
"admin": {
"foo",
"exact-dupe-1", "exact-dupe-1",
"different-case-dupe-1", "DIFFERENT-case-DUPE-1", "different-case-DUPE-1",
},
"developers": {"BAR"},
}
// Create a new errorResolver
r := &renameResolver{log.NewNullLogger()}
seenEntities := make(map[string]*identity.Entity)
seenGroups := make(map[string]*identity.Group)
for ns, entityList := range entities {
for i, name := range entityList {
id := fmt.Sprintf("00000000-0000-0000-0000-%012d", i)
// Create a new entity with the name/ns pair
entity := &identity.Entity{
ID: id,
Name: name,
NamespaceID: ns,
}
// Simulate a MemDB lookup
existingEntity := seenEntities[name]
err := r.ResolveEntities(context.Background(), existingEntity, entity)
require.NoError(t, err)
if existingEntity != nil {
require.Equal(t, name+"-"+id, entity.Name)
require.Equal(t, existingEntity.ID, entity.Metadata["duplicate_of_canonical_id"])
} else {
seenEntities[name] = entity
}
// Also, since the data model is the same, pretend these are groups too
group := &identity.Group{
ID: id,
Name: name,
NamespaceID: ns,
}
// More MemDB mocking
existingGroup := seenGroups[name]
err = r.ResolveGroups(context.Background(), existingGroup, group)
require.NoError(t, err)
if existingGroup != nil {
require.Equal(t, name+"-"+id, group.Name)
require.Equal(t, existingGroup.ID, group.Metadata["duplicate_of_canonical_id"])
} else {
seenGroups[name] = group
}
}
}
// No need to test entity alias merges here, since that's handled separately.
}

6
vault/identity_store_entities.go
View File

@@ -1038,12 +1038,16 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
if err != nil {
return nil, fmt.Errorf("aborting entity merge - failed to delete orphaned alias %q during merge into entity %q: %w", toAliasId, toEntity.ID, err), nil
}
// Remove the alias from the entity's list in memory too!
toEntity.DeleteAliasByID(toAliasId)
} else if strutil.StrListContains(conflictingAliasIDsToKeep, toAliasId) {
i.logger.Info("Deleting from_entity alias during entity merge", "from_entity", fromEntityID, "deleted_alias", fromAlias.ID)
err := i.MemDBDeleteAliasByIDInTxn(txn, fromAlias.ID, false)
if err != nil {
return nil, fmt.Errorf("aborting entity merge - failed to delete orphaned alias %q during merge into entity %q: %w", fromAlias.ID, toEntity.ID, err), nil
}
// Remove the alias from the entity's list in memory too!
toEntity.DeleteAliasByID(toAliasId)
// Continue to next alias, as there's no alias to merge left in the from_entity
continue
@@ -1053,6 +1057,8 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
if err != nil {
return nil, fmt.Errorf("aborting entity merge - failed to delete orphaned alias %q during merge into entity %q: %w", toAliasId, toEntity.ID, err), nil
}
// Remove the alias from the entity's list in memory too!
toEntity.DeleteAliasByID(toAliasId)
} else {
return fmt.Errorf("conflicting mount accessors in following alias IDs and neither were present in conflicting_alias_ids_to_keep: %s, %s", fromAlias.ID, toAliasId), nil, nil
}

39
vault/identity_store_test.go
View File

@@ -21,6 +21,7 @@ import (
credGithub "github.com/hashicorp/vault/builtin/credential/github"
"github.com/hashicorp/vault/builtin/credential/userpass"
credUserpass "github.com/hashicorp/vault/builtin/credential/userpass"
"github.com/hashicorp/vault/helper/activationflags"
"github.com/hashicorp/vault/helper/identity"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/helper/storagepacker"
@@ -1411,14 +1412,27 @@ func TestIdentityStoreInvalidate_TemporaryEntity(t *testing.T) {
assert.Nil(t, item)
}
// TestEntityStoreLoadingIsDeterministic is a property-based test that ensures
// the loading logic of the entity store is deterministic. This is important
// because we perform certain merges and corrections of duplicates on load and
// non-deterministic order can cause divergence between different nodes or even
// after seal/unseal cycles on one node. Loading _should_ be deterministic
// anyway if all data in storage was correct see comments inline for examples of
// ways storage can be corrupt with respect to the expected schema invariants.
func TestEntityStoreLoadingIsDeterministic(t *testing.T) {
// TestIdentityStoreLoadingIsDeterministic tests the default error resolver and
// the identity cleanup rename resolver to ensure that loading is deterministic
// for both.
func TestIdentityStoreLoadingIsDeterministic(t *testing.T) {
t.Run(t.Name()+"error-resolver", func(t *testing.T) {
identityStoreLoadingIsDeterministic(t, false)
})
t.Run(t.Name()+"identity-cleanup", func(t *testing.T) {
identityStoreLoadingIsDeterministic(t, true)
})
}
// identityStoreLoadingIsDeterministic is a property-based test helper that
// ensures the loading logic of the entity store is deterministic. This is
// important because we perform certain merges and corrections of duplicates on
// load and non-deterministic order can cause divergence between different nodes
// or even after seal/unseal cycles on one node. Loading _should_ be
// deterministic anyway if all data in storage was correct see comments inline
// for examples of ways storage can be corrupt with respect to the expected
// schema invariants.
func identityStoreLoadingIsDeterministic(t *testing.T, identityDeduplication bool) {
// Create some state in store that could trigger non-deterministic behavior.
// The nature of the identity store schema is such that the order of loading
// entities etc shouldn't matter even if it was non-deterministic, however due
@@ -1456,6 +1470,11 @@ func TestEntityStoreLoadingIsDeterministic(t *testing.T) {
ctx := context.Background()
if identityDeduplication {
err = c.FeatureActivationFlags.Write(ctx, activationflags.IdentityDeduplication, true)
require.NoError(t, err)
}
// We create 100 entities each with 1 non-local alias and 1 local alias. We
// then randomly create duplicate alias or local alias entries with a
// probability that is unrealistic but ensures we have duplicates on every
@@ -1626,9 +1645,9 @@ func TestEntityStoreLoadingIsDeterministic(t *testing.T) {
}
}
// TestEntityStoreLoadingDuplicateReporting tests the reporting of different
// TestIdentityStoreLoadingDuplicateReporting tests the reporting of different
// types of duplicates during unseal when in case-sensitive mode.
func TestEntityStoreLoadingDuplicateReporting(t *testing.T) {
func TestIdentityStoreLoadingDuplicateReporting(t *testing.T) {
logger := corehelpers.NewTestLogger(t)
ims, err := inmem.NewTransactionalInmemHA(nil, logger)
require.NoError(t, err)

63
vault/identity_store_util.go
View File

@@ -17,15 +17,17 @@ import (
memdb "github.com/hashicorp/go-memdb"
"github.com/hashicorp/go-secure-stdlib/strutil"
uuid "github.com/hashicorp/go-uuid"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/timestamppb"
"github.com/hashicorp/vault/helper/activationflags"
"github.com/hashicorp/vault/helper/identity"
"github.com/hashicorp/vault/helper/identity/mfa"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/helper/storagepacker"
"github.com/hashicorp/vault/sdk/helper/consts"
"github.com/hashicorp/vault/sdk/logical"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/timestamppb"
)
var (
@@ -33,54 +35,73 @@ var (
tmpSuffix = ".tmp"
)
// loadIdentityStoreArtifacts is responsible for loading entities, groups, and aliases from storage into MemDB.
func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
if c.identityStore == nil {
c.logger.Warn("identity store is not setup, skipping loading")
return nil
}
// Resolve all conflicts by logging a warning and returning
// errDuplicateIdentityName. The error will flip the identityStore into
// case-sensitive mode by switching the underlying schema to one with a
// relaxed lowerCase constraint and reload all artifacts into MemDB.
c.identityStore.conflictResolver = &errorResolver{c.identityStore.logger}
loadFunc := func(context.Context) error {
if err := c.identityStore.loadEntities(ctx); err != nil {
return err
return fmt.Errorf("failed to load entities: %w", err)
}
if err := c.identityStore.loadGroups(ctx); err != nil {
return err
return fmt.Errorf("failed to load groups: %w", err)
}
if err := c.identityStore.loadOIDCClients(ctx); err != nil {
return err
return fmt.Errorf("failed to load OIDC clients: %w", err)
}
if err := c.identityStore.loadCachedEntitiesOfLocalAliases(ctx); err != nil {
return err
return fmt.Errorf("failed to load cached local alias entities: %w", err)
}
return nil
}
// Load everything when memdb is set to operate on lower cased names
// Resolve all conflicts by logging a warning and returning
// errDuplicateIdentityName by default. The error will flip the
// identityStore into case-sensitive mode by switching the underlying
// schema to one with a relaxed lowerCase constraint and reload all
// artifacts into MemDB.
c.identityStore.conflictResolver = &errorResolver{c.identityStore.logger}
// If the identity deduplication cleanup flag is activated, instead
// deal with duplicate entities and groups by renaming with a -UUID
// suffix. N.B. *entity alias* duplicates will still be merged as before.
if c.FeatureActivationFlags.IsActivationFlagEnabled(activationflags.IdentityDeduplication) {
c.identityStore.conflictResolver = &renameResolver{c.identityStore.logger}
}
// Load everything when MemDB is set to operate on lower cased names.
// errDuplicateIdentityName below should only happen if we're using the
// errorResolver (i.e. identity deduplication is not activated) and we
// encounter non-alias duplicates.
err := loadFunc(ctx)
switch {
case err == nil:
// If it succeeds, all is well
// No error implies we've loaded the artifacts successfully
// with no duplicates detected. This means there were no
// unmerged duplicates detected while loading with the
// errorResolver, or the renameResolver was activated and
// resolved all duplicates. In either case, we can return
// early, since there's nothing left to do.
return nil
case !errwrap.Contains(err, errDuplicateIdentityName.Error()):
// All other errors are unexpected and should be returned.
return err
}
// If we're here, it means we've encountered duplicates while loading
// with the errorResolver.
c.identityStore.logger.Warn("enabling case sensitive identity names")
// Set identity store to operate on case sensitive identity names
c.identityStore.disableLowerCasedNames = true
// Swap the memdb instance by the one which operates on case sensitive
// names, hence obviating the need to unload anything that's already
// loaded.
if err := c.identityStore.resetDB(ctx); err != nil {
// Swap out the MemDB instance and reload artifacts with the
// new schema.
if err := c.identityStore.resetDB(); err != nil {
return err
}
@@ -339,7 +360,6 @@ func (i *IdentityStore) loadCachedEntitiesOfLocalAliases(ctx context.Context) er
return err
}
nsCtx := namespace.ContextWithNamespace(ctx, ns)
err = i.upsertEntity(nsCtx, entity, nil, false)
if err != nil {
return fmt.Errorf("failed to update entity in MemDB: %w", err)
@@ -1857,7 +1877,7 @@ func (i *IdentityStore) UpsertGroup(ctx context.Context, group *identity.Group,
txn := i.db.Txn(true)
defer txn.Abort()
err := i.UpsertGroupInTxn(ctx, txn, group, true)
err := i.UpsertGroupInTxn(ctx, txn, group, persist)
if err != nil {
return err
}
@@ -2695,6 +2715,7 @@ func attachAlias(t *testing.T, e *identity.Entity, name string, me *MountEntry)
if e.NamespaceID != me.NamespaceID {
panic("mount and entity in different namespaces")
}
require.NoError(t, err)
a := &identity.Alias{
ID: id,
Name: name,

4
vault/logical_system_activation_flags.go
View File

@@ -9,6 +9,7 @@ import (
"slices"
"strings"
"github.com/hashicorp/vault/helper/activationflags"
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/logical"
)
@@ -35,7 +36,6 @@ This path responds to the following HTTP methods.
PUT|POST /<feature-name>/activate
Activates the specified feature. Cannot be undone.`
activationFlagIdentityCleanup = "force-identity-deduplication"
activationFlagTest = "activation-test"
)
@@ -86,7 +86,7 @@ func (b *SystemBackend) activationFlagsPaths() []*framework.Path {
HelpDescription: helpDescription,
},
{
Pattern: fmt.Sprintf("%s/%s/%s", prefixActivationFlags, activationFlagIdentityCleanup, verbActivationFlagsActivate),
Pattern: fmt.Sprintf("%s/%s/%s", prefixActivationFlags, activationflags.IdentityDeduplication, verbActivationFlagsActivate),
DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: prefixActivationFlags,
OperationVerb: verbActivationFlagsActivate,

5
vault/logical_system_activation_flags_test.go
View File

@@ -10,6 +10,7 @@ import (
"fmt"
"testing"
"github.com/hashicorp/vault/helper/activationflags"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/sdk/logical"
"github.com/stretchr/testify/require"
@@ -94,7 +95,7 @@ func TestActivationFlags_Write(t *testing.T) {
context.Background(),
&logical.Request{
Operation: logical.UpdateOperation,
Path: fmt.Sprintf("%s/%s/%s", prefixActivationFlags, activationFlagIdentityCleanup, verbActivationFlagsActivate),
Path: fmt.Sprintf("%s/%s/%s", prefixActivationFlags, activationflags.IdentityDeduplication, verbActivationFlagsActivate),
Storage: core.systemBarrierView,
},
)
@@ -103,6 +104,6 @@ func TestActivationFlags_Write(t *testing.T) {
require.NotNil(t, resp)
require.NotEmpty(t, resp.Data)
require.NotNil(t, resp.Data["activated"])
require.Contains(t, resp.Data["activated"], activationFlagIdentityCleanup)
require.Contains(t, resp.Data["activated"], activationflags.IdentityDeduplication)
})
}