diff --git a/builtin/logical/pki/storage_migrations.go b/builtin/logical/pki/storage_migrations.go index f4b9237266b7..b89bb0e64bc9 100644 --- a/builtin/logical/pki/storage_migrations.go +++ b/builtin/logical/pki/storage_migrations.go @@ -88,23 +88,40 @@ func migrateStorage(ctx context.Context, b *backend, s logical.Storage) error { var keyIdentifier keyID sc := b.makeStorageContext(ctx, s) if migrationInfo.legacyBundle != nil { - // Generate a unique name for the migrated items in case things were to be re-migrated again - // for some weird reason in the future... - migrationName := fmt.Sprintf("current-%d", time.Now().Unix()) - - b.Logger().Info("performing PKI migration to new keys/issuers layout") - anIssuer, aKey, err := sc.writeCaBundle(migrationInfo.legacyBundle, migrationName, migrationName) - if err != nil { - return err + // When the legacy bundle still exists, there's three scenarios we + // need to worry about: + // + // 1. When we have no migration log, we definitely want to migrate. + haveNoLog := migrationInfo.migrationLog == nil + // 2. When we have an (empty) log and the version is zero, we want to + // migrate. + haveOldVersion := !haveNoLog && migrationInfo.migrationLog.MigrationVersion == 0 + // 3. When we have a log and the version is at least 1 (where this + // migration was introduced), we want to run the migration again + // only if the legacy bundle hash has changed. + isCurrentOrBetterVersion := !haveNoLog && migrationInfo.migrationLog.MigrationVersion >= 1 + haveChange := !haveNoLog && migrationInfo.migrationLog.Hash != migrationInfo.legacyBundleHash + haveVersionWithChange := isCurrentOrBetterVersion && haveChange + + if haveNoLog || haveOldVersion || haveVersionWithChange { + // Generate a unique name for the migrated items in case things were to be re-migrated again + // for some weird reason in the future... + migrationName := fmt.Sprintf("current-%d", time.Now().Unix()) + + b.Logger().Info("performing PKI migration to new keys/issuers layout") + anIssuer, aKey, err := sc.writeCaBundle(migrationInfo.legacyBundle, migrationName, migrationName) + if err != nil { + return err + } + b.Logger().Info("Migration generated the following ids and set them as defaults", + "issuer id", anIssuer.ID, "key id", aKey.ID) + issuerIdentifier = anIssuer.ID + keyIdentifier = aKey.ID + + // Since we do not have all the mount information available we must schedule + // the CRL to be rebuilt at a later time. + b.crlBuilder.requestRebuildIfActiveNode(b) } - b.Logger().Info("Migration generated the following ids and set them as defaults", - "issuer id", anIssuer.ID, "key id", aKey.ID) - issuerIdentifier = anIssuer.ID - keyIdentifier = aKey.ID - - // Since we do not have all the mount information available we must schedule - // the CRL to be rebuilt at a later time. - b.crlBuilder.requestRebuildIfActiveNode(b) } if migrationInfo.migrationLog != nil && migrationInfo.migrationLog.MigrationVersion == 1 { diff --git a/builtin/logical/pki/storage_migrations_test.go b/builtin/logical/pki/storage_migrations_test.go index 754f3993d14b..ef704476e832 100644 --- a/builtin/logical/pki/storage_migrations_test.go +++ b/builtin/logical/pki/storage_migrations_test.go @@ -777,6 +777,98 @@ func TestBackupBundle(t *testing.T) { require.NotEmpty(t, keyIds) } +func TestDeletedIssuersPostMigration(t *testing.T) { + // We want to simulate the following scenario: + // + // 1.10.x: -> Create a CA. + // 1.11.0: -> Migrate to new issuer layout but version 1. + // -> Delete existing issuers, create new ones. + // (now): -> Migrate to version 2 layout, make sure we don't see + // re-migration. + + t.Parallel() + ctx := context.Background() + b, s := CreateBackendWithStorage(t) + sc := b.makeStorageContext(ctx, s) + + // Reset the version the helper above set to 1. + b.pkiStorageVersion.Store(0) + require.True(t, b.useLegacyBundleCaStorage(), "pre migration we should have been told to use legacy storage.") + + // Create a legacy CA bundle and write it out. + bundle := genCertBundle(t, b, s) + json, err := logical.StorageEntryJSON(legacyCertBundlePath, bundle) + require.NoError(t, err) + err = s.Put(ctx, json) + require.NoError(t, err) + legacyContents := requireFileExists(t, sc, legacyCertBundlePath, nil) + + // Do a migration; this should provision an issuer and key. + initReq := &logical.InitializationRequest{Storage: s} + err = b.initialize(ctx, initReq) + require.NoError(t, err) + requireFileExists(t, sc, legacyCertBundlePath, legacyContents) + issuerIds, err := sc.listIssuers() + require.NoError(t, err) + require.NotEmpty(t, issuerIds) + keyIds, err := sc.listKeys() + require.NoError(t, err) + require.NotEmpty(t, keyIds) + + // Hack: reset the version to 1, to simulate a pre-version-2 migration + // log. + info, err := getMigrationInfo(sc.Context, sc.Storage) + require.NoError(t, err, "failed to read migration info") + info.migrationLog.MigrationVersion = 1 + err = setLegacyBundleMigrationLog(sc.Context, sc.Storage, info.migrationLog) + require.NoError(t, err, "failed to write migration info") + + // Now delete all issuers and keys and create some new ones. + for _, issuerId := range issuerIds { + deleted, err := sc.deleteIssuer(issuerId) + require.True(t, deleted, "expected it to be deleted") + require.NoError(t, err, "error removing issuer") + } + for _, keyId := range keyIds { + deleted, err := sc.deleteKey(keyId) + require.True(t, deleted, "expected it to be deleted") + require.NoError(t, err, "error removing key") + } + emptyIssuers, err := sc.listIssuers() + require.NoError(t, err) + require.Empty(t, emptyIssuers) + emptyKeys, err := sc.listKeys() + require.NoError(t, err) + require.Empty(t, emptyKeys) + + // Create a new issuer + key. + bundle = genCertBundle(t, b, s) + _, _, err = sc.writeCaBundle(bundle, "", "") + require.NoError(t, err) + + // List which issuers + keys we currently have. + postDeletionIssuers, err := sc.listIssuers() + require.NoError(t, err) + require.NotEmpty(t, postDeletionIssuers) + postDeletionKeys, err := sc.listKeys() + require.NoError(t, err) + require.NotEmpty(t, postDeletionKeys) + + // Now do another migration from 1->2. This should retain the newly + // created issuers+keys, but not revive any deleted ones. + err = b.initialize(ctx, initReq) + require.NoError(t, err) + requireFileExists(t, sc, legacyCertBundlePath, legacyContents) + postMigrationIssuers, err := sc.listIssuers() + require.NoError(t, err) + require.NotEmpty(t, postMigrationIssuers) + require.Equal(t, postMigrationIssuers, postDeletionIssuers, "regression failed: expected second migration from v1->v2 to not introduce new issuers") + postMigrationKeys, err := sc.listKeys() + require.NoError(t, err) + require.NotEmpty(t, postMigrationKeys) + require.Equal(t, postMigrationKeys, postDeletionKeys, "regression failed: expected second migration from v1->v2 to not introduce new keys") +} + // requireFailInMigration validate that we fail the operation with the appropriate error message to the end-user func requireFailInMigration(t *testing.T, b *backend, s logical.Storage, operation logical.Operation, path string) { resp, err := b.HandleRequest(context.Background(), &logical.Request{ diff --git a/changelog/21316.txt b/changelog/21316.txt new file mode 100644 index 000000000000..5573c7e4d319 --- /dev/null +++ b/changelog/21316.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/pki: Prevent deleted issuers from reappearing when migrating from a version 1 bundle to a version 2 bundle (versions including 1.13.0, 1.12.2, and 1.11.6); when managed keys were removed but referenced in the Vault 1.10 legacy CA bundle, this the error: `no managed key found with uuid`. +``` diff --git a/website/content/docs/upgrading/upgrade-to-1.11.x.mdx b/website/content/docs/upgrading/upgrade-to-1.11.x.mdx index f288e7af1a92..adc3279ceb12 100644 --- a/website/content/docs/upgrading/upgrade-to-1.11.x.mdx +++ b/website/content/docs/upgrading/upgrade-to-1.11.x.mdx @@ -43,3 +43,5 @@ vault write auth/ldap/config max_page_size=-1 #### Impacted Versions Affects Vault 1.11.10. + +@include 'pki-double-migration-bug.mdx' diff --git a/website/content/docs/upgrading/upgrade-to-1.12.x.mdx b/website/content/docs/upgrading/upgrade-to-1.12.x.mdx index c0e62d429cca..368b2c0a63f9 100644 --- a/website/content/docs/upgrading/upgrade-to-1.12.x.mdx +++ b/website/content/docs/upgrading/upgrade-to-1.12.x.mdx @@ -213,3 +213,5 @@ flag for [PKI roles](/vault/api-docs/secret/pki#createupdate-role). #### Impacted Versions Affects Vault 1.12.0+ + +@include 'pki-double-migration-bug.mdx' diff --git a/website/content/docs/upgrading/upgrade-to-1.13.x.mdx b/website/content/docs/upgrading/upgrade-to-1.13.x.mdx index de2c0b17892c..f43bfb05dd2e 100644 --- a/website/content/docs/upgrading/upgrade-to-1.13.x.mdx +++ b/website/content/docs/upgrading/upgrade-to-1.13.x.mdx @@ -159,3 +159,5 @@ Affects Vault 1.13.0+ @include 'perf-standby-token-create-forwarding-failure.mdx' @include 'update-primary-known-issue.mdx' + +@include 'pki-double-migration-bug.mdx' diff --git a/website/content/partials/pki-double-migration-bug.mdx b/website/content/partials/pki-double-migration-bug.mdx new file mode 100644 index 000000000000..e1ff94c63760 --- /dev/null +++ b/website/content/partials/pki-double-migration-bug.mdx @@ -0,0 +1,30 @@ +### PKI storage migration revives deleted issuers + +Vault 1.11 introduced Storage v1, a new storage layout that supported +multiple issuers within a single mount. Bug fixes in Vault 1.11.6, 1.12.2, +and 1.13.0 corrected a write-ordering issue that lead to invalid CA chains. +Specifically, incorrectly ordered writes could fail due to load, resulting +in the mount being re-migrated next time it was loaded or silently +truncating CA chains. This collection of bug fixes introduced Storage v2. + +#### Affected versions + +Vault may incorrectly re-migrated legacy issuers created before Vault 1.11 that +were migrated to Storage v1 and deleted before upgrading to a Vault version with +Storage v2. + +The migration fails when Vault finds managed keys associated with the legacy +issuers that were removed from the managed key repository prior to the upgrade. + +The migration error appears in Vault logs as: + +> Error during migration of PKI mount: +> failed to lookup public key from managed key: +> no managed key found with uuid + + +Issuers created in Vault 1.11+ and direct upgrades to a Storage v2 layout are +not affected. + + +The Storage v1 upgrade bug was fixed in Vault 1.14.1, 1.13.5, and 1.12.9.