Prevent entity alias creation when entity is in different NS than mount (#943) (#6886)

This commit is contained in:
Jeff Mitchell 2019-06-14 12:53:00 -04:00 committed by GitHub
parent 402ba1b0f0
commit 1ea0c0314a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 290 additions and 169 deletions

View File

@ -46,7 +46,7 @@ This field is deprecated, use canonical_id.`,
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: i.handleAliasUpdateCommon(),
logical.UpdateOperation: i.handleAliasCreateUpdate(),
},
HelpSynopsis: strings.TrimSpace(aliasHelp["alias"][0]),
@ -79,7 +79,7 @@ This field is deprecated, use canonical_id.`,
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: i.handleAliasUpdateCommon(),
logical.UpdateOperation: i.handleAliasCreateUpdate(),
logical.ReadOperation: i.pathAliasIDRead(),
logical.DeleteOperation: i.pathAliasIDDelete(),
},
@ -99,31 +99,25 @@ This field is deprecated, use canonical_id.`,
}
}
// handleAliasUpdateCommon is used to update an alias
func (i *IdentityStore) handleAliasUpdateCommon() framework.OperationFunc {
// handleAliasCreateUpdate is used to create or update an alias
func (i *IdentityStore) handleAliasCreateUpdate() framework.OperationFunc {
return func(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
var err error
var alias *identity.Alias
var entity *identity.Entity
var previousEntity *identity.Entity
i.lock.Lock()
defer i.lock.Unlock()
// Check for update or create
aliasID := d.Get("id").(string)
if aliasID != "" {
alias, err = i.MemDBAliasByID(aliasID, true, false)
if err != nil {
return nil, err
}
if alias == nil {
return logical.ErrorResponse("invalid alias id"), nil
}
} else {
alias = &identity.Alias{}
ns, err := namespace.FromContext(ctx)
if err != nil {
return nil, err
}
// Get alias name, if any
name := d.Get("name").(string)
// Get mount accessor, if any
mountAccessor := d.Get("mount_accessor").(string)
// Get ID, if any
id := d.Get("id").(string)
// Get entity id
canonicalID := d.Get("canonical_id").(string)
if canonicalID == "" {
@ -131,155 +125,252 @@ func (i *IdentityStore) handleAliasUpdateCommon() framework.OperationFunc {
canonicalID = d.Get("entity_id").(string)
}
// Get alias name
if aliasName := d.Get("name").(string); aliasName == "" {
if alias.Name == "" {
return logical.ErrorResponse("missing alias name"), nil
}
} else {
alias.Name = aliasName
}
i.lock.Lock()
defer i.lock.Unlock()
// Get mount accessor
if mountAccessor := d.Get("mount_accessor").(string); mountAccessor == "" {
if alias.MountAccessor == "" {
return logical.ErrorResponse("missing mount_accessor"), nil
}
} else {
alias.MountAccessor = mountAccessor
}
mountValidationResp := i.core.router.validateMountByAccessor(alias.MountAccessor)
if mountValidationResp == nil {
return logical.ErrorResponse(fmt.Sprintf("invalid mount accessor %q", alias.MountAccessor)), nil
}
if mountValidationResp.MountLocal {
return logical.ErrorResponse(fmt.Sprintf("mount_accessor %q is of a local mount", alias.MountAccessor)), nil
}
// Verify that the combination of alias name and mount is not
// already tied to a different alias
aliasByFactors, err := i.MemDBAliasByFactors(mountValidationResp.MountAccessor, alias.Name, false, false)
if err != nil {
return nil, err
}
if aliasByFactors != nil {
// If it's a create we won't have an alias ID so this will correctly
// bail. If it's an update alias will be the same as aliasbyfactors so
// we don't need to transfer any info over
if aliasByFactors.ID != alias.ID {
return logical.ErrorResponse("combination of mount and alias name is already in use"), nil
}
// Fetch the entity to which the alias is tied. We don't need to append
// here, so the only further checking is whether the canonical ID is
// different
entity, err = i.MemDBEntityByAliasID(alias.ID, true)
if err != nil {
return nil, err
}
if entity == nil {
return nil, fmt.Errorf("existing alias is not associated with an entity")
}
} else if alias.ID != "" {
// This is an update, not a create; if we have an associated entity
// already, load it
entity, err = i.MemDBEntityByAliasID(alias.ID, true)
if err != nil {
return nil, err
}
}
resp := &logical.Response{}
// If we found an existing alias we won't hit this condition because
// canonicalID being empty will result in nil being returned in the block
// above, so in this case we know that creating a new entity is the right
// thing.
if canonicalID == "" {
entity = &identity.Entity{
Aliases: []*identity.Alias{
alias,
},
}
} else {
// If we can look up by the given canonical ID, see if this is a
// transfer; otherwise if we found no previous entity but we find one
// here, use it.
canonicalEntity, err := i.MemDBEntityByID(canonicalID, true)
if err != nil {
return nil, err
}
if canonicalEntity == nil {
return logical.ErrorResponse("invalid canonical ID"), nil
}
if entity == nil {
// If entity is nil, we didn't find a previous alias from factors,
// so append to this entity
entity = canonicalEntity
entity.Aliases = append(entity.Aliases, alias)
} else if entity.ID != canonicalEntity.ID {
// In this case we found an entity from alias factors or given
// alias ID but it's not the same, so it's a migration
previousEntity = entity
entity = canonicalEntity
for aliasIndex, item := range previousEntity.Aliases {
if item.ID == alias.ID {
previousEntity.Aliases = append(previousEntity.Aliases[:aliasIndex], previousEntity.Aliases[aliasIndex+1:]...)
break
}
// This block is run if they provided an ID
{
// If they provide an ID it must be an update. Find the alias, perform
// due diligence, call the update function.
if id != "" {
alias, err := i.MemDBAliasByID(id, true, false)
if err != nil {
return nil, err
}
if alias == nil {
return logical.ErrorResponse("invalid alias ID provided"), nil
}
if alias.NamespaceID != ns.ID {
return logical.ErrorResponse("cannot modify aliases across namespaces"), logical.ErrPermissionDenied
}
entity.Aliases = append(entity.Aliases, alias)
resp.AddWarning(fmt.Sprintf("alias is being transferred from entity %q to %q", previousEntity.ID, entity.ID))
switch {
case mountAccessor == "" && name == "":
// Just a canonical ID update, maybe
if canonicalID == "" {
// Nothing to do, so be idempotent
return nil, nil
}
name = alias.Name
mountAccessor = alias.MountAccessor
case mountAccessor == "":
// No change to mount accessor
mountAccessor = alias.MountAccessor
case name == "":
// No change to mount name
name = alias.Name
default:
// Both provided
}
return i.handleAliasUpdate(ctx, req, canonicalID, name, mountAccessor, alias)
}
}
// ID creation and other validations; This is more useful for new entities
// and may not perform anything for the existing entities. Placing the
// check here to make the flow common for both new and existing entities.
err = i.sanitizeEntity(ctx, entity)
// If they didn't provide an ID, we must have both accessor and name provided
if mountAccessor == "" || name == "" {
return logical.ErrorResponse("'id' or 'mount_accessor' and 'name' must be provided"), nil
}
// Look up the alias by factors; if it's found it's an update
mountEntry := i.core.router.MatchingMountByAccessor(mountAccessor)
if mountEntry == nil {
return logical.ErrorResponse(fmt.Sprintf("invalid mount accessor %q", mountAccessor)), nil
}
if mountEntry.Local {
return logical.ErrorResponse(fmt.Sprintf("mount accessor %q is of a local mount", mountAccessor)), nil
}
if mountEntry.NamespaceID != ns.ID {
return logical.ErrorResponse("matching mount is in a different namespace than request"), logical.ErrPermissionDenied
}
alias, err := i.MemDBAliasByFactors(mountAccessor, name, false, false)
if err != nil {
return nil, err
}
// Explicitly set to empty as in the past we incorrectly saved it
alias.MountPath = ""
alias.MountType = ""
// Set the canonical ID in the alias index. This should be done after
// sanitizing entity.
alias.CanonicalID = entity.ID
// ID creation and other validations
err = i.sanitizeAlias(ctx, alias)
if err != nil {
return nil, err
}
for index, item := range entity.Aliases {
if item.ID == alias.ID {
entity.Aliases[index] = alias
if alias != nil {
if alias.NamespaceID != ns.ID {
return logical.ErrorResponse("cannot modify aliases across namespaces"), logical.ErrPermissionDenied
}
return i.handleAliasUpdate(ctx, req, alias.CanonicalID, name, mountAccessor, alias)
}
// Index entity and its aliases in MemDB and persist entity along with
// aliases in storage. If the alias is being transferred over from
// one entity to another, previous entity needs to get refreshed in MemDB
// and persisted in storage as well.
if err := i.upsertEntity(ctx, entity, previousEntity, true); err != nil {
// At this point we know it's a new creation request
return i.handleAliasCreate(ctx, req, canonicalID, name, mountAccessor)
}
}
func (i *IdentityStore) handleAliasCreate(ctx context.Context, req *logical.Request, canonicalID, name, mountAccessor string) (*logical.Response, error) {
ns, err := namespace.FromContext(ctx)
if err != nil {
return nil, err
}
alias := &identity.Alias{
MountAccessor: mountAccessor,
Name: name,
}
entity := &identity.Entity{}
// If a canonical ID is provided pull up the entity and make sure we're in
// the right NS
if canonicalID != "" {
entity, err = i.MemDBEntityByID(canonicalID, true)
if err != nil {
return nil, err
}
if entity == nil {
return logical.ErrorResponse("invalid canonical ID"), nil
}
if entity.NamespaceID != ns.ID {
return logical.ErrorResponse("entity found with 'canonical_id' not in request namespace"), logical.ErrPermissionDenied
}
}
// Return ID of both alias and entity
resp.Data = map[string]interface{}{
entity.Aliases = append(entity.Aliases, alias)
// ID creation and other validations; This is more useful for new entities
// and may not perform anything for the existing entities. Placing the
// check here to make the flow common for both new and existing entities.
err = i.sanitizeEntity(ctx, entity)
if err != nil {
return nil, err
}
// Set the canonical ID in the alias index. This should be done after
// sanitizing entity in case it's a new entity that didn't have an ID.
alias.CanonicalID = entity.ID
// ID creation and other validations
err = i.sanitizeAlias(ctx, alias)
if err != nil {
return nil, err
}
// Index entity and its aliases in MemDB and persist entity along with
// aliases in storage.
if err := i.upsertEntity(ctx, entity, nil, true); err != nil {
return nil, err
}
// Return ID of both alias and entity
return &logical.Response{
Data: map[string]interface{}{
"id": alias.ID,
"canonical_id": entity.ID,
},
}, nil
}
func (i *IdentityStore) handleAliasUpdate(ctx context.Context, req *logical.Request, canonicalID, name, mountAccessor string, alias *identity.Alias) (*logical.Response, error) {
if name == alias.Name &&
mountAccessor == alias.MountAccessor &&
(canonicalID == alias.CanonicalID || canonicalID == "") {
// Nothing to do; return nil to be idempotent
return nil, nil
}
alias.LastUpdateTime = ptypes.TimestampNow()
// If we're changing one or the other or both of these, make sure that
// there isn't a matching alias already, and make sure it's in the same
// namespace.
if name != alias.Name || mountAccessor != alias.MountAccessor {
// Check here to see if such an alias already exists, if so bail
mountEntry := i.core.router.MatchingMountByAccessor(mountAccessor)
if mountEntry == nil {
return logical.ErrorResponse(fmt.Sprintf("invalid mount accessor %q", mountAccessor)), nil
}
if mountEntry.Local {
return logical.ErrorResponse(fmt.Sprintf("mount_accessor %q is of a local mount", mountAccessor)), nil
}
if mountEntry.NamespaceID != alias.NamespaceID {
return logical.ErrorResponse("given mount accessor is not in the same namespace as the existing alias"), logical.ErrPermissionDenied
}
return resp, nil
existingAlias, err := i.MemDBAliasByFactors(mountAccessor, name, false, false)
if err != nil {
return nil, err
}
// Bail unless it's just a case change
if existingAlias != nil && !strings.EqualFold(existingAlias.Name, name) {
return logical.ErrorResponse("alias with combination of mount accessor and name already exists"), nil
}
// Update the values in the alias
alias.Name = name
alias.MountAccessor = mountAccessor
}
// Get our current entity, which may be the same as the new one if the
// canonical ID hasn't changed
currentEntity, err := i.MemDBEntityByAliasID(alias.ID, true)
if err != nil {
return nil, err
}
if currentEntity == nil {
return logical.ErrorResponse("given alias is not associated with an entity"), nil
}
if currentEntity.NamespaceID != alias.NamespaceID {
return logical.ErrorResponse("alias associated with an entity in a different namespace"), logical.ErrPermissionDenied
}
newEntity := currentEntity
if canonicalID != "" && canonicalID != alias.CanonicalID {
newEntity, err = i.MemDBEntityByID(canonicalID, true)
if err != nil {
return nil, err
}
if newEntity == nil {
return logical.ErrorResponse("given 'canonical_id' is not associated with an entity"), nil
}
if newEntity.NamespaceID != alias.NamespaceID {
return logical.ErrorResponse("given 'canonical_id' associated with entity in a different namespace from the alias"), logical.ErrPermissionDenied
}
// Update the canonical ID value and move it from the current enitity to the new one
alias.CanonicalID = newEntity.ID
newEntity.Aliases = append(newEntity.Aliases, alias)
for aliasIndex, item := range currentEntity.Aliases {
if item.ID == alias.ID {
currentEntity.Aliases = append(currentEntity.Aliases[:aliasIndex], currentEntity.Aliases[aliasIndex+1:]...)
break
}
}
} else {
// If it's not moving we still need to update it in the existing
// entity's aliases
for aliasIndex, item := range currentEntity.Aliases {
if item.ID == alias.ID {
currentEntity.Aliases[aliasIndex] = alias
break
}
}
// newEntity will be pointing to the same entity; set currentEntity nil
// so the upsertCall gets nil for the previous entity as we're only
// changing one.
currentEntity = nil
}
// Index entity and its aliases in MemDB and persist entity along with
// aliases in storage. If the alias is being transferred over from
// one entity to another, previous entity needs to get refreshed in MemDB
// and persisted in storage as well.
if err := i.upsertEntity(ctx, newEntity, currentEntity, true); err != nil {
return nil, err
}
// Return ID of both alias and entity
return &logical.Response{
Data: map[string]interface{}{
"id": alias.ID,
"canonical_id": newEntity.ID,
},
}, nil
}
// pathAliasIDRead returns the properties of an alias for a given
@ -310,7 +401,7 @@ func (i *IdentityStore) handleAliasReadCommon(ctx context.Context, alias *identi
return nil, err
}
if ns.ID != alias.NamespaceID {
return nil, nil
return logical.ErrorResponse("alias and request are in different namespaces"), logical.ErrPermissionDenied
}
respData := map[string]interface{}{}
@ -320,6 +411,7 @@ func (i *IdentityStore) handleAliasReadCommon(ctx context.Context, alias *identi
respData["metadata"] = alias.Metadata
respData["name"] = alias.Name
respData["merged_from_canonical_ids"] = alias.MergedFromCanonicalIDs
respData["namespace_id"] = alias.NamespaceID
if mountValidationResp := i.core.router.validateMountByAccessor(alias.MountAccessor); mountValidationResp != nil {
respData["mount_path"] = mountValidationResp.MountPath
@ -366,7 +458,7 @@ func (i *IdentityStore) pathAliasIDDelete() framework.OperationFunc {
return nil, err
}
if ns.ID != alias.NamespaceID {
return nil, logical.ErrUnsupportedPath
return logical.ErrorResponse("request and alias are in different namespaces"), logical.ErrPermissionDenied
}
// Fetch the associated entity

View File

@ -196,8 +196,8 @@ func TestIdentityStore_AliasSameAliasNames(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if resp == nil || !resp.IsError() {
t.Fatalf("expected an error due to alias name not being unique")
if resp != nil {
t.Fatalf("expected no response since this modification should be idempotent")
}
}

View File

@ -337,6 +337,7 @@ func (i *IdentityStore) handleEntityReadCommon(ctx context.Context, entity *iden
respData["merged_entity_ids"] = entity.MergedEntityIDs
respData["policies"] = entity.Policies
respData["disabled"] = entity.Disabled
respData["namespace_id"] = entity.NamespaceID
// Convert protobuf timestamp into RFC3339 format
respData["creation_time"] = ptypes.TimestampString(entity.CreationTime)

View File

@ -152,16 +152,16 @@ func (i *IdentityStore) handleGroupAliasUpdateCommon(ctx context.Context, req *l
return logical.ErrorResponse("missing mount_accessor"), nil
}
mountValidationResp := i.core.router.validateMountByAccessor(mountAccessor)
if mountValidationResp == nil {
mountEntry := i.core.router.MatchingMountByAccessor(mountAccessor)
if mountEntry == nil {
return logical.ErrorResponse(fmt.Sprintf("invalid mount accessor %q", mountAccessor)), nil
}
if mountValidationResp.MountLocal {
if mountEntry.Local {
return logical.ErrorResponse(fmt.Sprintf("mount_accessor %q is of a local mount", mountAccessor)), nil
}
groupAliasByFactors, err := i.MemDBAliasByFactors(mountValidationResp.MountAccessor, groupAliasName, false, true)
groupAliasByFactors, err := i.MemDBAliasByFactors(mountEntry.Accessor, groupAliasName, false, true)
if err != nil {
return nil, err
}
@ -208,8 +208,13 @@ func (i *IdentityStore) handleGroupAliasUpdateCommon(ctx context.Context, req *l
group.Alias = groupAlias
}
// Check if the group we found belongs to a different namespace than the mount accessor
if group.NamespaceID != mountEntry.NamespaceID {
return logical.ErrorResponse("mount accessor and group are located in different namespaces"), nil
}
group.Alias.Name = groupAliasName
group.Alias.MountAccessor = mountValidationResp.MountAccessor
group.Alias.MountAccessor = mountEntry.Accessor
// Explicitly correct for previous versions that persisted this
group.Alias.MountType = ""

View File

@ -335,6 +335,7 @@ func (i *IdentityStore) handleGroupReadCommon(ctx context.Context, group *identi
respData["last_update_time"] = ptypes.TimestampString(group.LastUpdateTime)
respData["modify_index"] = group.ModifyIndex
respData["type"] = group.Type
respData["namespace_id"] = group.NamespaceID
aliasMap := map[string]interface{}{}
if group.Alias != nil {

View File

@ -115,7 +115,7 @@ vault <command> <path> metadata=key1=value1 metadata=key2=value2
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: i.handleAliasUpdateCommon(),
logical.UpdateOperation: i.handleAliasCreateUpdate(),
},
HelpSynopsis: strings.TrimSpace(aliasHelp["alias"][0]),
@ -147,7 +147,7 @@ vault <command> <path> metadata=key1=value1 metadata=key2=value2
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: i.handleAliasUpdateCommon(),
logical.UpdateOperation: i.handleAliasCreateUpdate(),
logical.ReadOperation: i.pathAliasIDRead(),
logical.DeleteOperation: i.pathAliasIDDelete(),
},

View File

@ -332,11 +332,19 @@ func (i *IdentityStore) upsertEntityInTxn(ctx context.Context, txn *memdb.Txn, e
var err error
if txn == nil {
return fmt.Errorf("txn is nil")
return errors.New("txn is nil")
}
if entity == nil {
return fmt.Errorf("entity is nil")
return errors.New("entity is nil")
}
if entity.NamespaceID == "" {
entity.NamespaceID = namespace.RootNamespaceID
}
if previousEntity != nil && previousEntity.NamespaceID != entity.NamespaceID {
return errors.New("entity and previous entity are not in the same namespace")
}
aliasFactors := make([]string, len(entity.Aliases))
@ -348,12 +356,24 @@ func (i *IdentityStore) upsertEntityInTxn(ctx context.Context, txn *memdb.Txn, e
return err
}
if alias.NamespaceID == "" {
alias.NamespaceID = namespace.RootNamespaceID
}
switch {
case aliasByFactors == nil:
// Not found, no merging needed
// Not found, no merging needed, just check namespace
if alias.NamespaceID != entity.NamespaceID {
return errors.New("alias and entity are not in the same namespace")
}
case aliasByFactors.CanonicalID == entity.ID:
// Lookup found the same entity, so it's already attached to the
// right place
if aliasByFactors.NamespaceID != entity.NamespaceID {
return errors.New("alias from factors and entity are not in the same namespace")
}
case previousEntity != nil && aliasByFactors.CanonicalID == previousEntity.ID:
// previousEntity isn't upserted yet so may still contain the old
// alias reference in memdb if it was just changed; validate
@ -372,8 +392,10 @@ func (i *IdentityStore) upsertEntityInTxn(ctx context.Context, txn *memdb.Txn, e
}
// Otherwise it's still tied to previousEntity and fall through
// into merging
// into merging. We don't need a namespace check here as existing
// checks when creating the aliases should ensure that all line up.
fallthrough
default:
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, "entity_aliases", entity.Aliases, "alias_by_factors", aliasByFactors)
@ -943,7 +965,7 @@ func (i *IdentityStore) sanitizeAlias(ctx context.Context, alias *identity.Alias
return err
}
if ns.ID != alias.NamespaceID {
return fmt.Errorf("alias belongs to a different namespace")
return errors.New("alias belongs to a different namespace")
}
// Set the creation and last update times
@ -983,7 +1005,7 @@ func (i *IdentityStore) sanitizeEntity(ctx context.Context, entity *identity.Ent
entity.NamespaceID = ns.ID
}
if ns.ID != entity.NamespaceID {
return fmt.Errorf("entity does not belong to this namespace")
return errors.New("entity does not belong to this namespace")
}
// Create a name if there isn't one already
@ -1047,7 +1069,7 @@ func (i *IdentityStore) sanitizeAndUpsertGroup(ctx context.Context, group *ident
return err
}
if ns.ID != group.NamespaceID {
return fmt.Errorf("group does not belong to this namespace")
return errors.New("group does not belong to this namespace")
}
// Create a name if there isn't one already