diff --git a/vault/identity_store_aliases.go b/vault/identity_store_aliases.go index 97cd9a5cc..f51f2d2e6 100644 --- a/vault/identity_store_aliases.go +++ b/vault/identity_store_aliases.go @@ -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 diff --git a/vault/identity_store_aliases_test.go b/vault/identity_store_aliases_test.go index 5dbb6d926..72b7ef8e9 100644 --- a/vault/identity_store_aliases_test.go +++ b/vault/identity_store_aliases_test.go @@ -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") } } diff --git a/vault/identity_store_entities.go b/vault/identity_store_entities.go index aa02c1ddb..1aeb292fb 100644 --- a/vault/identity_store_entities.go +++ b/vault/identity_store_entities.go @@ -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) diff --git a/vault/identity_store_group_aliases.go b/vault/identity_store_group_aliases.go index 169973347..475d9a328 100644 --- a/vault/identity_store_group_aliases.go +++ b/vault/identity_store_group_aliases.go @@ -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 = "" diff --git a/vault/identity_store_groups.go b/vault/identity_store_groups.go index d3a8fa8c3..8fa550dea 100644 --- a/vault/identity_store_groups.go +++ b/vault/identity_store_groups.go @@ -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 { diff --git a/vault/identity_store_upgrade.go b/vault/identity_store_upgrade.go index 978cfa54d..2c28925d7 100644 --- a/vault/identity_store_upgrade.go +++ b/vault/identity_store_upgrade.go @@ -115,7 +115,7 @@ vault 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 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(), }, diff --git a/vault/identity_store_util.go b/vault/identity_store_util.go index eb52e9693..78a6f2840 100644 --- a/vault/identity_store_util.go +++ b/vault/identity_store_util.go @@ -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