From 3d2023550eeb3cab644a417fb43db8c6e16325e7 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 | 130 ++++++++---------- .../sealmigration/seal_migration_test.go | 27 ++-- vault/seal.go | 35 ++--- vault/seal_autoseal.go | 4 +- 5 files changed, 93 insertions(+), 117 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 000000000000..1d42a30f3843 --- /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 0d3f5f5d467e..234cd7d1d66a 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`) } + + 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) + + // If we've reached this point it's a migration attempt. if existBarrierSealConfig.Type != wrapping.Shamir && existRecoverySealConfig == nil { - return errors.New("Recovery seal configuration not found for existing seal") + 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") } - 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{ - Wrapper: aeadwrapper.NewShamirWrapper(&wrapping.WrapperOptions{ - Logger: c.logger.Named("shamir"), - }), - }) - - default: - return errors.New("failed to determine the migration seal") - + if c.unwrapSeal.BarrierType() == c.seal.BarrierType() { + return errors.New("Migrating between same seal types is currently not supported") } - // newSeal will be the barrierSeal - newSeal = barrierSeal + // 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 + } - if migrationSeal != nil && newSeal != nil && migrationSeal.BarrierType() == newSeal.BarrierType() { - return errors.New("Migrating between same seal types is currently not supported") + c.migrationInfo = &migrationInformation{ + seal: c.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()) - 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) - return nil + return nil +} + +func (c *Core) adjustSealConfigDuringMigration(existBarrierSealConfig, existRecoverySealConfig *SealConfig) { + if c.migrationInfo == nil { + return } - // Set the appropriate barrier and recovery configs. switch { - case migrationSeal != nil && newSeal != nil && migrationSeal.RecoveryKeySupported() && newSeal.RecoveryKeySupported(): + case c.unwrapSeal.RecoveryKeySupported() && c.seal.RecoveryKeySupported(): // Migrating from auto->auto, copy the configs over - newSeal.SetCachedBarrierConfig(existBarrierSealConfig) - newSeal.SetCachedRecoveryConfig(existRecoverySealConfig) - case migrationSeal != nil && newSeal != nil && migrationSeal.RecoveryKeySupported(): + 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 - newSeal.SetCachedBarrierConfig(newSealConfig) - case newSeal != nil && newSeal.RecoveryKeySupported(): + 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: newSeal.BarrierType(), + Type: c.seal.BarrierType(), SecretShares: 1, SecretThreshold: 1, StoredShares: 1, } - newSeal.SetCachedBarrierConfig(newBarrierSealConfig) + c.seal.SetCachedBarrierConfig(newBarrierSealConfig) newRecoveryConfig := existBarrierSealConfig.Clone() newRecoveryConfig.StoredShares = 0 - newSeal.SetCachedRecoveryConfig(newRecoveryConfig) - } - - c.setSealsForMigration(migrationSeal, newSeal, unwrapSeal) - - return nil -} - -func (c *Core) setSealsForMigration(migrationSeal, newSeal, unwrapSeal Seal) { - c.unwrapSeal = unwrapSeal - if c.unwrapSeal != nil { - c.unwrapSeal.SetCore(c) - } - if newSeal != nil && migrationSeal != nil { - c.migrationInfo = &migrationInformation{ - seal: migrationSeal, - } - 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.SetCachedRecoveryConfig(newRecoveryConfig) } } diff --git a/vault/external_tests/sealmigration/seal_migration_test.go b/vault/external_tests/sealmigration/seal_migration_test.go index 032a7099ecc1..2fefdcc2517b 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 6697f15123d4..e7589a6de0eb 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 6ff171ec5ccb..cc13a12a134e 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 }