From 4134ef2e98fff876df436939baafdf1709463642 Mon Sep 17 00:00:00 2001 From: ncabatoff Date: Mon, 10 Aug 2020 08:35:57 -0400 Subject: [PATCH] Ensure that perf standbys can perform seal migrations. (#9690) --- helper/testhelpers/testhelpers_oss.go | 14 ++ vault/core.go | 152 ++++++++---------- .../sealmigration/seal_migration_test.go | 27 ++-- vault/seal.go | 35 ++-- vault/seal_autoseal.go | 4 +- 5 files changed, 104 insertions(+), 128 deletions(-) create mode 100644 helper/testhelpers/testhelpers_oss.go diff --git a/helper/testhelpers/testhelpers_oss.go b/helper/testhelpers/testhelpers_oss.go new file mode 100644 index 000000000..1d42a30f3 --- /dev/null +++ b/helper/testhelpers/testhelpers_oss.go @@ -0,0 +1,14 @@ +// +build !enterprise + +package testhelpers + +import ( + "github.com/hashicorp/vault/vault" + "github.com/mitchellh/go-testing-interface" +) + +// WaitForActiveNodeAndStandbys does nothing more than wait for the active node +// on OSS. On enterprise it waits for perf standbys to be healthy too. +func WaitForActiveNodeAndStandbys(t testing.T, cluster *vault.TestCluster) { + WaitForActiveNode(t, cluster) +} diff --git a/vault/core.go b/vault/core.go index 0d3f5f5d4..234cd7d1d 100644 --- a/vault/core.go +++ b/vault/core.go @@ -240,8 +240,7 @@ type Core struct { sealMigrated *uint32 inSealMigration *uberAtomic.Bool - // unwrapSeal is the seal to use on Enterprise to unwrap values wrapped - // with the previous seal. + // unwrapSeal is the old seal when migrating to a new seal. unwrapSeal Seal // barrier is the security barrier wrapping the physical backend @@ -1322,7 +1321,7 @@ func (c *Core) migrateSeal(ctx context.Context) error { return nil } - existBarrierSealConfig, _, err := c.PhysicalSealConfigs(ctx) + existBarrierSealConfig, existRecoverySealConfig, err := c.PhysicalSealConfigs(ctx) if err != nil { return fmt.Errorf("failed to read existing seal configuration during migration: %v", err) } @@ -1336,6 +1335,8 @@ func (c *Core) migrateSeal(ctx context.Context) error { } c.logger.Info("seal migration initiated") + // We need to update the cached seal configs because they may have been wiped out by various means. + c.adjustSealConfigDuringMigration(existBarrierSealConfig, existRecoverySealConfig) switch { case c.migrationInfo.seal.RecoveryKeySupported() && c.seal.RecoveryKeySupported(): @@ -2214,10 +2215,11 @@ func (c *Core) PhysicalSealConfigs(ctx context.Context) (*SealConfig, *SealConfi return barrierConf, recoveryConf, nil } +// adjustForSealMigration takes the unwrapSeal (in a migration scenario, this +// is the old seal we're migrating from; in any scenario, it's the seal that +// the master key is currently encrypted with). After doing some sanity checking +// it sets up the seals and migrationInfo to allow for a migration if needed. func (c *Core) adjustForSealMigration(unwrapSeal Seal) error { - - barrierSeal := c.seal - existBarrierSealConfig, existRecoverySealConfig, err := c.PhysicalSealConfigs(context.Background()) if err != nil { return fmt.Errorf("Error checking for existing seal: %s", err) @@ -2232,112 +2234,96 @@ func (c *Core) adjustForSealMigration(unwrapSeal Seal) error { if unwrapSeal == nil { // We have the same barrier type and the unwrap seal is nil so we're not // migrating from same to same, IOW we assume it's not a migration - if existBarrierSealConfig.Type == barrierSeal.BarrierType() { + if existBarrierSealConfig.Type == c.seal.BarrierType() { return nil } // If we're not coming from Shamir, and the existing type doesn't match // the barrier type, we need both the migration seal and the new seal - if existBarrierSealConfig.Type != wrapping.Shamir && barrierSeal.BarrierType() != wrapping.Shamir { + if existBarrierSealConfig.Type != wrapping.Shamir && c.seal.BarrierType() != wrapping.Shamir { return errors.New(`Trying to migrate from auto-seal to auto-seal but no "disabled" seal stanza found`) } - } else { - if unwrapSeal.BarrierType() == wrapping.Shamir { - return errors.New("Shamir seals cannot be set disabled (they should simply not be set)") - } - } - if existBarrierSealConfig.Type != wrapping.Shamir && existRecoverySealConfig == nil { - return errors.New("Recovery seal configuration not found for existing seal") - } - - var migrationSeal Seal - var newSeal Seal - - // Determine the migrationSeal. This is either going to be an instance of - // shamir or the unwrapSeal. - switch { - case unwrapSeal != nil: - // If we're not coming from Shamir we expect the previous seal to be - // in the config and disabled. - migrationSeal = unwrapSeal - - case existBarrierSealConfig.Type == wrapping.Shamir: - // The value reflected in config is what we're going to - migrationSeal = NewDefaultSeal(&vaultseal.Access{ + c.unwrapSeal = NewDefaultSeal(&vaultseal.Access{ Wrapper: aeadwrapper.NewShamirWrapper(&wrapping.WrapperOptions{ Logger: c.logger.Named("shamir"), }), }) + } else { + // If we're not coming from Shamir we expect the previous seal to be + // in the config and disabled. + if unwrapSeal.BarrierType() == wrapping.Shamir { + return errors.New("Shamir seals cannot be set disabled (they should simply not be set)") + } + c.unwrapSeal = unwrapSeal + } + c.unwrapSeal.SetCore(c) - default: - return errors.New("failed to determine the migration seal") + // If we've reached this point it's a migration attempt. + if existBarrierSealConfig.Type != wrapping.Shamir && existRecoverySealConfig == nil { + entry, err := c.physical.Get(c.activeContext, recoverySealConfigPlaintextPath) + if err != nil { + return errwrap.Wrapf(fmt.Sprintf("failed to read %q seal configuration: {{err}}", existBarrierSealConfig.Type), err) + } + if entry == nil { + return errors.New("Recovery seal configuration not found for existing seal") + } + return errors.New("Cannot migrate seals while using a legacy recovery seal config") } - // newSeal will be the barrierSeal - newSeal = barrierSeal - - if migrationSeal != nil && newSeal != nil && migrationSeal.BarrierType() == newSeal.BarrierType() { + if c.unwrapSeal.BarrierType() == c.seal.BarrierType() { return errors.New("Migrating between same seal types is currently not supported") } - if unwrapSeal != nil && existBarrierSealConfig.Type == barrierSeal.BarrierType() { - // In this case our migration seal is set so we are using it - // (potentially) for unwrapping. Set it on core for that purpose then - // exit. - c.setSealsForMigration(nil, nil, unwrapSeal) + // Not entirely sure why this is here, but I theorize it could be to handle + // the case where the migration has already completed, e.g. on another node, + // but the disabled seal stanza wasn't removed from the HCL config yet. + if existBarrierSealConfig.Type == c.seal.BarrierType() { return nil } - // Set the appropriate barrier and recovery configs. - switch { - case migrationSeal != nil && newSeal != nil && migrationSeal.RecoveryKeySupported() && newSeal.RecoveryKeySupported(): - // Migrating from auto->auto, copy the configs over - newSeal.SetCachedBarrierConfig(existBarrierSealConfig) - newSeal.SetCachedRecoveryConfig(existRecoverySealConfig) - case migrationSeal != nil && newSeal != nil && migrationSeal.RecoveryKeySupported(): - // Migrating from auto->shamir, clone auto's recovery config and set - // stored keys to 1. - newSealConfig := existRecoverySealConfig.Clone() - newSealConfig.StoredShares = 1 - newSeal.SetCachedBarrierConfig(newSealConfig) - case newSeal != nil && newSeal.RecoveryKeySupported(): - // Migrating from shamir->auto, set a new barrier config and set - // recovery config to a clone of shamir's barrier config with stored - // keys set to 0. - newBarrierSealConfig := &SealConfig{ - Type: newSeal.BarrierType(), - SecretShares: 1, - SecretThreshold: 1, - StoredShares: 1, - } - newSeal.SetCachedBarrierConfig(newBarrierSealConfig) - - newRecoveryConfig := existBarrierSealConfig.Clone() - newRecoveryConfig.StoredShares = 0 - newSeal.SetCachedRecoveryConfig(newRecoveryConfig) + c.migrationInfo = &migrationInformation{ + seal: c.unwrapSeal, } - - c.setSealsForMigration(migrationSeal, newSeal, unwrapSeal) + c.adjustSealConfigDuringMigration(existBarrierSealConfig, existRecoverySealConfig) + c.initSealsForMigration() + c.logger.Warn("entering seal migration mode; Vault will not automatically unseal even if using an autoseal", "from_barrier_type", c.migrationInfo.seal.BarrierType(), "to_barrier_type", c.seal.BarrierType()) return nil } -func (c *Core) setSealsForMigration(migrationSeal, newSeal, unwrapSeal Seal) { - c.unwrapSeal = unwrapSeal - if c.unwrapSeal != nil { - c.unwrapSeal.SetCore(c) +func (c *Core) adjustSealConfigDuringMigration(existBarrierSealConfig, existRecoverySealConfig *SealConfig) { + if c.migrationInfo == nil { + return } - if newSeal != nil && migrationSeal != nil { - c.migrationInfo = &migrationInformation{ - seal: migrationSeal, + + switch { + case c.unwrapSeal.RecoveryKeySupported() && c.seal.RecoveryKeySupported(): + // Migrating from auto->auto, copy the configs over + c.seal.SetCachedBarrierConfig(existBarrierSealConfig) + c.seal.SetCachedRecoveryConfig(existRecoverySealConfig) + case c.unwrapSeal.RecoveryKeySupported(): + // Migrating from auto->shamir, clone auto's recovery config and set + // stored keys to 1. + newSealConfig := existRecoverySealConfig.Clone() + newSealConfig.StoredShares = 1 + c.seal.SetCachedBarrierConfig(newSealConfig) + case c.seal.RecoveryKeySupported(): + // Migrating from shamir->auto, set a new barrier config and set + // recovery config to a clone of shamir's barrier config with stored + // keys set to 0. + newBarrierSealConfig := &SealConfig{ + Type: c.seal.BarrierType(), + SecretShares: 1, + SecretThreshold: 1, + StoredShares: 1, } - c.migrationInfo.seal.SetCore(c) - c.seal = newSeal - c.seal.SetCore(c) - c.logger.Warn("entering seal migration mode; Vault will not automatically unseal even if using an autoseal", "from_barrier_type", c.migrationInfo.seal.BarrierType(), "to_barrier_type", c.seal.BarrierType()) - c.initSealsForMigration() + c.seal.SetCachedBarrierConfig(newBarrierSealConfig) + + newRecoveryConfig := existBarrierSealConfig.Clone() + newRecoveryConfig.StoredShares = 0 + c.seal.SetCachedRecoveryConfig(newRecoveryConfig) } } diff --git a/vault/external_tests/sealmigration/seal_migration_test.go b/vault/external_tests/sealmigration/seal_migration_test.go index 032a7099e..2fefdcc25 100644 --- a/vault/external_tests/sealmigration/seal_migration_test.go +++ b/vault/external_tests/sealmigration/seal_migration_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/base64" "fmt" + "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/sdk/helper/logging" "sync/atomic" "testing" "time" @@ -12,14 +14,12 @@ import ( "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping" - "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/testhelpers" sealhelper "github.com/hashicorp/vault/helper/testhelpers/seal" "github.com/hashicorp/vault/helper/testhelpers/teststorage" vaulthttp "github.com/hashicorp/vault/http" "github.com/hashicorp/vault/physical/raft" - "github.com/hashicorp/vault/sdk/helper/logging" "github.com/hashicorp/vault/vault" ) @@ -126,9 +126,7 @@ func migrateFromShamirToTransit_Pre14(t *testing.T, logger hclog.Logger, storage var transitSeal vault.Seal - var conf = vault.CoreConfig{ - DisablePerformanceStandby: true, - } + var conf = vault.CoreConfig{} var opts = vault.TestClusterOptions{ Logger: logger.Named("migrateFromShamirToTransit"), HandlerFunc: vaulthttp.Handler, @@ -356,8 +354,8 @@ func migratePost14(t *testing.T, logger hclog.Logger, storage teststorage.Reusab cluster.StartCore(t, i, opts) unsealMigrate(t, cluster.Cores[i].Client, unsealKeys, true) - time.Sleep(5 * time.Second) } + testhelpers.WaitForActiveNodeAndStandbys(t, cluster) // Step down the active node which will kick off the migration on one of the // other nodes. @@ -528,9 +526,7 @@ func initializeShamir(t *testing.T, logger hclog.Logger, storage teststorage.Reu var baseClusterPort = basePort + 10 // Start the cluster - var conf = vault.CoreConfig{ - DisablePerformanceStandby: true, - } + var conf = vault.CoreConfig{} var opts = vault.TestClusterOptions{ Logger: logger.Named("initializeShamir"), HandlerFunc: vaulthttp.Handler, @@ -581,9 +577,7 @@ func runShamir(t *testing.T, logger hclog.Logger, storage teststorage.ReusableSt var baseClusterPort = basePort + 10 // Start the cluster - var conf = vault.CoreConfig{ - DisablePerformanceStandby: true, - } + var conf = vault.CoreConfig{} var opts = vault.TestClusterOptions{ Logger: logger.Named("runShamir"), HandlerFunc: vaulthttp.Handler, @@ -655,9 +649,7 @@ func initializeTransit(t *testing.T, logger hclog.Logger, storage teststorage.Re var baseClusterPort = basePort + 10 // Start the cluster - var conf = vault.CoreConfig{ - DisablePerformanceStandby: true, - } + var conf = vault.CoreConfig{} var opts = vault.TestClusterOptions{ Logger: logger, HandlerFunc: vaulthttp.Handler, @@ -684,7 +676,7 @@ func initializeTransit(t *testing.T, logger hclog.Logger, storage teststorage.Re t.Fatal(err) } } - testhelpers.WaitForNCoresUnsealed(t, cluster, len(cluster.Cores)) + testhelpers.WaitForActiveNodeAndStandbys(t, cluster) err := client.Sys().Mount("kv-wrapped", &api.MountInput{ SealWrap: true, @@ -710,8 +702,7 @@ func runTransit(t *testing.T, logger hclog.Logger, storage teststorage.ReusableS // Start the cluster var conf = vault.CoreConfig{ - DisablePerformanceStandby: true, - Seal: transitSeal, + Seal: transitSeal, } var opts = vault.TestClusterOptions{ Logger: logger.Named("runTransit"), diff --git a/vault/seal.go b/vault/seal.go index 6697f1512..e7589a6de 100644 --- a/vault/seal.go +++ b/vault/seal.go @@ -6,13 +6,13 @@ import ( "encoding/base64" "encoding/json" "fmt" + "github.com/hashicorp/vault/sdk/helper/jsonutil" + "github.com/hashicorp/vault/sdk/physical" "sync/atomic" "github.com/golang/protobuf/proto" "github.com/hashicorp/errwrap" wrapping "github.com/hashicorp/go-kms-wrapping" - "github.com/hashicorp/vault/sdk/helper/jsonutil" - "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/vault/seal" "github.com/keybase/go-crypto/openpgp" "github.com/keybase/go-crypto/openpgp/packet" @@ -127,15 +127,8 @@ func (d *defaultSeal) BarrierType() string { } func (d *defaultSeal) StoredKeysSupported() seal.StoredKeysSupport { - isLegacy, err := d.LegacySeal() - if err != nil { - if d.core != nil && d.core.logger != nil { - d.core.logger.Error("no seal config found, can't determine if legacy or new-style shamir") - } - return seal.StoredKeysInvalid - } switch { - case isLegacy: + case d.LegacySeal(): return seal.StoredKeysNotSupported default: return seal.StoredKeysSupportedShamirMaster @@ -147,30 +140,22 @@ func (d *defaultSeal) RecoveryKeySupported() bool { } func (d *defaultSeal) SetStoredKeys(ctx context.Context, keys [][]byte) error { - isLegacy, err := d.LegacySeal() - if err != nil { - return err - } - if isLegacy { + if d.LegacySeal() { return fmt.Errorf("stored keys are not supported") } return writeStoredKeys(ctx, d.core.physical, d.access, keys) } -func (d *defaultSeal) LegacySeal() (bool, error) { +func (d *defaultSeal) LegacySeal() bool { cfg := d.config.Load().(*SealConfig) if cfg == nil { - return false, fmt.Errorf("no seal config found, can't determine if legacy or new-style shamir") + return false } - return cfg.StoredShares == 0, nil + return cfg.StoredShares == 0 } func (d *defaultSeal) GetStoredKeys(ctx context.Context) ([][]byte, error) { - isLegacy, err := d.LegacySeal() - if err != nil { - return nil, err - } - if isLegacy { + if d.LegacySeal() { return nil, fmt.Errorf("stored keys are not supported") } keys, err := readStoredKeys(ctx, d.core.physical, d.access) @@ -224,7 +209,7 @@ func (d *defaultSeal) BarrierConfig(ctx context.Context) (*SealConfig, error) { return nil, errwrap.Wrapf("seal validation failed: {{err}}", err) } - d.config.Store(&conf) + d.SetCachedBarrierConfig(&conf) return conf.Clone(), nil } @@ -266,7 +251,7 @@ func (d *defaultSeal) SetBarrierConfig(ctx context.Context, config *SealConfig) return errwrap.Wrapf("failed to write seal configuration: {{err}}", err) } - d.config.Store(config.Clone()) + d.SetCachedBarrierConfig(config.Clone()) return nil } diff --git a/vault/seal_autoseal.go b/vault/seal_autoseal.go index 6ff171ec5..cc13a12a1 100644 --- a/vault/seal_autoseal.go +++ b/vault/seal_autoseal.go @@ -198,7 +198,7 @@ func (d *autoSeal) BarrierConfig(ctx context.Context) (*SealConfig, error) { return nil, fmt.Errorf("barrier seal type of %q does not match loaded type of %q", conf.Type, d.BarrierType()) } - d.barrierConfig.Store(conf) + d.SetCachedBarrierConfig(conf) return conf.Clone(), nil } @@ -231,7 +231,7 @@ func (d *autoSeal) SetBarrierConfig(ctx context.Context, conf *SealConfig) error return errwrap.Wrapf("failed to write barrier seal configuration: {{err}}", err) } - d.barrierConfig.Store(conf.Clone()) + d.SetCachedBarrierConfig(conf.Clone()) return nil }