Merge Identity Entities if two claim the same alias (#5075)

* Merge Identity Entities if two claim the same alias

Past bugs/race conditions meant two entities could be created each
claiming the same alias. There are planned longer term fixes for this
(outside of the race condition being fixed in 0.10.4) that involve
changing the data model, but this is an immediate workaround that has
the same net effect: if two entities claim the same alias, assume they
were created due to this race condition and merge them.

In this situation, also automatically merge policies so we don't lose
e.g. RGPs.
This commit is contained in:
Jeff Mitchell 2018-08-09 15:37:36 -05:00 committed by GitHub
parent 70d516b34d
commit 2ed2e696a7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 260 additions and 82 deletions

View File

@ -329,3 +329,20 @@ func AppendIfMissing(slice []string, i string) []string {
}
return append(slice, i)
}
// MergeSlices adds an arbitrary number of slices together, uniquely
func MergeSlices(args ...[]string) []string {
all := map[string]struct{}{}
for _, slice := range args {
for _, v := range slice {
all[v] = struct{}{}
}
}
result := make([]string, 0, len(all))
for k, _ := range all {
result = append(result, k)
}
sort.Strings(result)
return result
}

View File

@ -449,3 +449,13 @@ func TestStrUtil_ParseStringSlice(t *testing.T) {
}
}
}
func TestStrUtil_MergeSlices(t *testing.T) {
res := MergeSlices([]string{"a", "c", "d"}, []string{}, []string{"c", "f", "a"}, nil, []string{"foo"})
expect := []string{"a", "c", "d", "f", "foo"}
if !reflect.DeepEqual(res, expect) {
t.Fatalf("expected %v, got %v", expect, res)
}
}

View File

@ -2,6 +2,7 @@ package vault
import (
"context"
"errors"
"fmt"
"strings"
@ -10,6 +11,7 @@ import (
memdb "github.com/hashicorp/go-memdb"
"github.com/hashicorp/vault/helper/identity"
"github.com/hashicorp/vault/helper/storagepacker"
"github.com/hashicorp/vault/helper/strutil"
"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/logical/framework"
)
@ -143,9 +145,6 @@ func (i *IdentityStore) pathEntityMergeID() framework.OperationFunc {
force := d.Get("force").(bool)
i.lock.Lock()
defer i.lock.Unlock()
// Create a MemDB transaction to merge entities
txn := i.db.Txn(true)
defer txn.Abort()
@ -155,85 +154,12 @@ func (i *IdentityStore) pathEntityMergeID() framework.OperationFunc {
return nil, err
}
if toEntity == nil {
return logical.ErrorResponse("entity id to merge to is invalid"), nil
userErr, intErr := i.mergeEntity(txn, toEntity, fromEntityIDs, force, true, false)
if userErr != nil {
return logical.ErrorResponse(userErr.Error()), nil
}
var conflictErrors error
for _, fromEntityID := range fromEntityIDs {
if fromEntityID == toEntityID {
return logical.ErrorResponse("to_entity_id should not be present in from_entity_ids"), nil
}
fromEntity, err := i.MemDBEntityByID(fromEntityID, false)
if err != nil {
return nil, err
}
if fromEntity == nil {
return logical.ErrorResponse("entity id to merge from is invalid"), nil
}
for _, alias := range fromEntity.Aliases {
// Set the desired canonical ID
alias.CanonicalID = toEntity.ID
alias.MergedFromCanonicalIDs = append(alias.MergedFromCanonicalIDs, fromEntity.ID)
err = i.MemDBUpsertAliasInTxn(txn, alias, false)
if err != nil {
return nil, errwrap.Wrapf("failed to update alias during merge: {{err}}", err)
}
// Add the alias to the desired entity
toEntity.Aliases = append(toEntity.Aliases, alias)
}
// If the entity from which we are merging from was already a merged
// entity, transfer over the Merged set to the entity we are
// merging into.
toEntity.MergedEntityIDs = append(toEntity.MergedEntityIDs, fromEntity.MergedEntityIDs...)
// Add the entity from which we are merging from to the list of entities
// the entity we are merging into is composed of.
toEntity.MergedEntityIDs = append(toEntity.MergedEntityIDs, fromEntity.ID)
// Delete the entity which we are merging from in MemDB using the same transaction
err = i.MemDBDeleteEntityByIDInTxn(txn, fromEntity.ID)
if err != nil {
return nil, err
}
// Delete the entity which we are merging from in storage
err = i.entityPacker.DeleteItem(fromEntity.ID)
if err != nil {
return nil, err
}
}
if conflictErrors != nil && !force {
return logical.ErrorResponse(conflictErrors.Error()), nil
}
// Update MemDB with changes to the entity we are merging to
err = i.MemDBUpsertEntityInTxn(txn, toEntity)
if err != nil {
return nil, err
}
// Persist the entity which we are merging to
toEntityAsAny, err := ptypes.MarshalAny(toEntity)
if err != nil {
return nil, err
}
item := &storagepacker.Item{
ID: toEntity.ID,
Message: toEntityAsAny,
}
err = i.entityPacker.PutItem(item)
if err != nil {
return nil, err
if intErr != nil {
return nil, intErr
}
// Committing the transaction *after* successfully performing storage
@ -549,6 +475,101 @@ func (i *IdentityStore) pathEntityIDList() framework.OperationFunc {
}
}
func (i *IdentityStore) mergeEntity(txn *memdb.Txn, toEntity *identity.Entity, fromEntityIDs []string, force, grabLock, mergePolicies bool) (error, error) {
if grabLock {
i.lock.Lock()
defer i.lock.Unlock()
}
if toEntity == nil {
return errors.New("entity id to merge to is invalid"), nil
}
var conflictErrors error
for _, fromEntityID := range fromEntityIDs {
if fromEntityID == toEntity.ID {
return errors.New("to_entity_id should not be present in from_entity_ids"), nil
}
fromEntity, err := i.MemDBEntityByID(fromEntityID, false)
if err != nil {
return nil, err
}
if fromEntity == nil {
return errors.New("entity id to merge from is invalid"), nil
}
for _, alias := range fromEntity.Aliases {
// Set the desired canonical ID
alias.CanonicalID = toEntity.ID
alias.MergedFromCanonicalIDs = append(alias.MergedFromCanonicalIDs, fromEntity.ID)
err = i.MemDBUpsertAliasInTxn(txn, alias, false)
if err != nil {
return nil, errwrap.Wrapf("failed to update alias during merge: {{err}}", err)
}
// Add the alias to the desired entity
toEntity.Aliases = append(toEntity.Aliases, alias)
}
// If told to, merge policies
if mergePolicies {
toEntity.Policies = strutil.MergeSlices(toEntity.Policies, fromEntity.Policies)
}
// If the entity from which we are merging from was already a merged
// entity, transfer over the Merged set to the entity we are
// merging into.
toEntity.MergedEntityIDs = append(toEntity.MergedEntityIDs, fromEntity.MergedEntityIDs...)
// Add the entity from which we are merging from to the list of entities
// the entity we are merging into is composed of.
toEntity.MergedEntityIDs = append(toEntity.MergedEntityIDs, fromEntity.ID)
// Delete the entity which we are merging from in MemDB using the same transaction
err = i.MemDBDeleteEntityByIDInTxn(txn, fromEntity.ID)
if err != nil {
return nil, err
}
// Delete the entity which we are merging from in storage
err = i.entityPacker.DeleteItem(fromEntity.ID)
if err != nil {
return nil, err
}
}
if conflictErrors != nil && !force {
return conflictErrors, nil
}
// Update MemDB with changes to the entity we are merging to
err := i.MemDBUpsertEntityInTxn(txn, toEntity)
if err != nil {
return nil, err
}
// Persist the entity which we are merging to
toEntityAsAny, err := ptypes.MarshalAny(toEntity)
if err != nil {
return nil, err
}
item := &storagepacker.Item{
ID: toEntity.ID,
Message: toEntityAsAny,
}
err = i.entityPacker.PutItem(item)
if err != nil {
return nil, err
}
return nil, nil
}
var entityHelp = map[string][2]string{
"entity": {
"Create a new entity",

View File

@ -5,8 +5,12 @@ import (
"testing"
"time"
"github.com/go-test/deep"
"github.com/golang/protobuf/ptypes"
uuid "github.com/hashicorp/go-uuid"
credGithub "github.com/hashicorp/vault/builtin/credential/github"
"github.com/hashicorp/vault/helper/identity"
"github.com/hashicorp/vault/helper/storagepacker"
"github.com/hashicorp/vault/logical"
)
@ -296,6 +300,122 @@ func TestIdentityStore_TokenEntityInheritance(t *testing.T) {
}
}
func TestIdentityStore_MergeConflictingAliases(t *testing.T) {
err := AddTestCredentialBackend("github", credGithub.Factory)
if err != nil {
t.Fatalf("err: %s", err)
}
c, unsealKey, root := TestCoreUnsealed(t)
meGH := &MountEntry{
Table: credentialTableType,
Path: "github/",
Type: "github",
Description: "github auth",
}
err = c.enableCredential(context.Background(), meGH)
if err != nil {
t.Fatal(err)
}
alias := &identity.Alias{
ID: "alias1",
CanonicalID: "entity1",
MountType: "github",
MountAccessor: meGH.Accessor,
Name: "githubuser",
}
entity := &identity.Entity{
ID: "entity1",
Name: "name1",
Policies: []string{"foo", "bar"},
Aliases: []*identity.Alias{
alias,
},
}
entity.BucketKeyHash = c.identityStore.entityPacker.BucketKeyHashByItemID(entity.ID)
// Now add the alias to two entities, skipping all existing checking by
// writing directly
entityAny, err := ptypes.MarshalAny(entity)
if err != nil {
t.Fatal(err)
}
item := &storagepacker.Item{
ID: entity.ID,
Message: entityAny,
}
if err = c.identityStore.entityPacker.PutItem(item); err != nil {
t.Fatal(err)
}
entity.ID = "entity2"
entity.Name = "name2"
entity.Policies = []string{"bar", "baz"}
alias.ID = "alias2"
alias.CanonicalID = "entity2"
entity.BucketKeyHash = c.identityStore.entityPacker.BucketKeyHashByItemID(entity.ID)
entityAny, err = ptypes.MarshalAny(entity)
if err != nil {
t.Fatal(err)
}
item = &storagepacker.Item{
ID: entity.ID,
Message: entityAny,
}
if err = c.identityStore.entityPacker.PutItem(item); err != nil {
t.Fatal(err)
}
// Seal and unseal. If things are broken, we will now fail to unseal.
if err = c.Seal(root); err != nil {
t.Fatal(err)
}
var unsealed bool
for i := 0; i < 3; i++ {
unsealed, err = c.Unseal(unsealKey[i])
if err != nil {
t.Fatal(err)
}
}
if !unsealed {
t.Fatal("still sealed")
}
newEntity, err := c.identityStore.CreateOrFetchEntity(&logical.Alias{
MountAccessor: meGH.Accessor,
MountType: "github",
Name: "githubuser",
})
if err != nil {
t.Fatal(err)
}
if newEntity == nil {
t.Fatal("nil new entity")
}
entityToUse := "entity1"
if newEntity.ID == "entity1" {
entityToUse = "entity2"
}
if len(newEntity.MergedEntityIDs) != 1 || newEntity.MergedEntityIDs[0] != entityToUse {
t.Fatalf("bad merged entity ids: %v", newEntity.MergedEntityIDs)
}
if diff := deep.Equal(newEntity.Policies, []string{"bar", "baz", "foo"}); diff != nil {
t.Fatal(diff)
}
newEntity, err = c.identityStore.MemDBEntityByID(entityToUse, false)
if err != nil {
t.Fatal(err)
}
if newEntity != nil {
t.Fatal("got a non-nil entity")
}
}
func testCoreWithIdentityTokenGithub(t *testing.T) (*Core, *IdentityStore, *TokenStore, string) {
is, ghAccessor, core := testIdentityStoreWithGithubAuth(t)
return core, is, core.tokenStore, ghAccessor

View File

@ -229,7 +229,17 @@ func (i *IdentityStore) upsertEntityInTxn(txn *memdb.Txn, entity *identity.Entit
}
if aliasByFactors != nil && aliasByFactors.CanonicalID != entity.ID {
return fmt.Errorf("alias %q is already tied to a different entity %q", alias.ID, aliasByFactors.CanonicalID)
i.logger.Warn("alias is already tied to a different entity; these entities are being merged", "alias_id", alias.ID, "other_entity_id", aliasByFactors.CanonicalID)
respErr, intErr := i.mergeEntity(txn, entity, []string{aliasByFactors.CanonicalID}, true, false, true)
switch {
case respErr != nil:
return respErr
case intErr != nil:
return intErr
}
// The entity and aliases will be loaded into memdb and persisted
// as a result of the merge so we are done here
return nil
}
// Insert or update alias in MemDB using the transaction created above