From bf58843b0a35e8d3f9c51e7c2123628bab8cf229 Mon Sep 17 00:00:00 2001 From: Scott Miller Date: Wed, 1 Feb 2023 13:34:53 -0600 Subject: [PATCH] Revert #18683 (#18942) * Revert "Don't execute the seal recovery tests on ENT. (#18841)" This reverts commit 990d3bacc203c229d0f6729929d7562e678a1ac2. * Revert "Add the ability to unseal using recovery keys via an explicit seal option. (#18683)" This reverts commit 2ffe49aab0fc1a527c5182637c8fa3ac39b08d45. --- api/sys_init.go | 28 ++-- changelog/18683.txt | 4 - command/operator_init.go | 23 +--- command/server.go | 13 +- command/server/config.go | 6 +- http/sys_init.go | 24 ++-- internalshared/configutil/kms.go | 11 -- vault/core.go | 123 ++++-------------- .../sealmigration/seal_migration_oss_test.go | 10 -- .../sealmigration/testshared.go | 63 +-------- vault/init.go | 12 -- vault/rekey.go | 29 +---- vault/seal.go | 85 +++--------- vault/seal_autoseal.go | 20 +-- 14 files changed, 92 insertions(+), 359 deletions(-) delete mode 100644 changelog/18683.txt delete mode 100644 vault/external_tests/sealmigration/seal_migration_oss_test.go diff --git a/api/sys_init.go b/api/sys_init.go index a1b1edaac7..05dea86f6a 100644 --- a/api/sys_init.go +++ b/api/sys_init.go @@ -51,15 +51,14 @@ func (c *Sys) InitWithContext(ctx context.Context, opts *InitRequest) (*InitResp } type InitRequest struct { - SecretShares int `json:"secret_shares"` - SecretThreshold int `json:"secret_threshold"` - StoredShares int `json:"stored_shares"` - PGPKeys []string `json:"pgp_keys"` - RecoveryShares int `json:"recovery_shares"` - RecoveryThreshold int `json:"recovery_threshold"` - RecoveryPGPKeys []string `json:"recovery_pgp_keys"` - RootTokenPGPKey string `json:"root_token_pgp_key"` - UnsealRecoveryDisabled bool `json:"disable_unseal_recovery"` + SecretShares int `json:"secret_shares"` + SecretThreshold int `json:"secret_threshold"` + StoredShares int `json:"stored_shares"` + PGPKeys []string `json:"pgp_keys"` + RecoveryShares int `json:"recovery_shares"` + RecoveryThreshold int `json:"recovery_threshold"` + RecoveryPGPKeys []string `json:"recovery_pgp_keys"` + RootTokenPGPKey string `json:"root_token_pgp_key"` } type InitStatusResponse struct { @@ -67,10 +66,9 @@ type InitStatusResponse struct { } type InitResponse struct { - Keys []string `json:"keys"` - KeysB64 []string `json:"keys_base64"` - RecoveryKeys []string `json:"recovery_keys"` - RecoveryKeysB64 []string `json:"recovery_keys_base64"` - RootToken string `json:"root_token"` - UnsealRecoveryAvailable bool `json:"unseal_recovery_available"` + Keys []string `json:"keys"` + KeysB64 []string `json:"keys_base64"` + RecoveryKeys []string `json:"recovery_keys"` + RecoveryKeysB64 []string `json:"recovery_keys_base64"` + RootToken string `json:"root_token"` } diff --git a/changelog/18683.txt b/changelog/18683.txt deleted file mode 100644 index c75126818a..0000000000 --- a/changelog/18683.txt +++ /dev/null @@ -1,4 +0,0 @@ -```release-note:change -core: recovery keys may now be used to unseal an auto-sealed Vault via an -opt-out unseal recovery mode. -``` \ No newline at end of file diff --git a/command/operator_init.go b/command/operator_init.go index 4c0ce953b8..3b0dfe3de2 100644 --- a/command/operator_init.go +++ b/command/operator_init.go @@ -29,11 +29,10 @@ type OperatorInitCommand struct { flagRootTokenPGPKey string // Auto Unseal - flagRecoveryShares int - flagRecoveryThreshold int - flagRecoveryPGPKeys []string - flagStoredShares int - flagDisableUnsealRecovery bool + flagRecoveryShares int + flagRecoveryThreshold int + flagRecoveryPGPKeys []string + flagStoredShares int // Consul flagConsulAuto bool @@ -150,13 +149,6 @@ func (c *OperatorInitCommand) Flags() *FlagSets { Usage: "DEPRECATED: This flag does nothing. It will be removed in Vault 1.3.", }) - f.BoolVar(&BoolVar{ - Name: "disable-unseal-recovery", - Target: &c.flagDisableUnsealRecovery, - Default: false, - Usage: "If disabled, unsealing Vault using recovery keys is not possible.", - }) - // Consul Options f = set.NewFlagSet("Consul Options") @@ -288,10 +280,9 @@ func (c *OperatorInitCommand) Run(args []string) int { PGPKeys: c.flagPGPKeys, RootTokenPGPKey: c.flagRootTokenPGPKey, - RecoveryShares: c.flagRecoveryShares, - RecoveryThreshold: c.flagRecoveryThreshold, - RecoveryPGPKeys: c.flagRecoveryPGPKeys, - UnsealRecoveryDisabled: c.flagDisableUnsealRecovery, + RecoveryShares: c.flagRecoveryShares, + RecoveryThreshold: c.flagRecoveryThreshold, + RecoveryPGPKeys: c.flagRecoveryPGPKeys, } // Check auto mode diff --git a/command/server.go b/command/server.go index c785b47f8a..e3da4072a5 100644 --- a/command/server.go +++ b/command/server.go @@ -2378,7 +2378,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma config.Seals = append(config.Seals, &configutil.KMS{Type: wrapping.WrapperTypeShamir.String()}) } } - createdSeals := make([]vault.Seal, len(config.Seals)) + var createdSeals []vault.Seal = make([]vault.Seal, len(config.Seals)) for _, configSeal := range config.Seals { sealType := wrapping.WrapperTypeShamir.String() if !configSeal.Disabled && os.Getenv("VAULT_SEAL_TYPE") != "" { @@ -2403,11 +2403,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma "Error parsing Seal configuration: %s", sealConfigError) } } - if configSeal.Recover { - seal = vault.NewRecoverySeal(&vaultseal.Access{ - Wrapper: aeadwrapper.NewShamirWrapper(), - }) - } else if wrapper == nil { + if wrapper == nil { seal = defaultSeal } else { var err error @@ -2432,7 +2428,6 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma } createdSeals = append(createdSeals, seal) } - return barrierSeal, barrierWrapper, unwrapSeal, createdSeals, sealConfigError, nil } @@ -2622,7 +2617,9 @@ func runUnseal(c *ServerCommand, core *vault.Core, ctx context.Context) { } } -func createCoreConfig(c *ServerCommand, config *server.Config, backend physical.Backend, configSR sr.ServiceRegistration, barrierSeal, unwrapSeal vault.Seal, metricsHelper *metricsutil.MetricsHelper, metricSink *metricsutil.ClusterMetricSink, secureRandomReader io.Reader) vault.CoreConfig { +func createCoreConfig(c *ServerCommand, config *server.Config, backend physical.Backend, configSR sr.ServiceRegistration, barrierSeal, unwrapSeal vault.Seal, + metricsHelper *metricsutil.MetricsHelper, metricSink *metricsutil.ClusterMetricSink, secureRandomReader io.Reader, +) vault.CoreConfig { coreConfig := &vault.CoreConfig{ RawConfig: config, Physical: backend, diff --git a/command/server/config.go b/command/server/config.go index 068a72e452..d2d30b40ed 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -482,16 +482,12 @@ func CheckConfig(c *Config, e error) (*Config, error) { return c, e } - switch len(c.Seals) { - case 2: - // Two seals indicates a seal migration, but one and only one must be disabled + if len(c.Seals) == 2 { switch { case c.Seals[0].Disabled && c.Seals[1].Disabled: return nil, errors.New("seals: two seals provided but both are disabled") case !c.Seals[0].Disabled && !c.Seals[1].Disabled: return nil, errors.New("seals: two seals provided but neither is disabled") - case (!c.Seals[0].Disabled && c.Seals[0].Recover) || (!c.Seals[1].Disabled && c.Seals[1].Recover): - return nil, errors.New("seals: migration target seal cannot be in recovery mode") } } diff --git a/http/sys_init.go b/http/sys_init.go index 012480fd63..ae3059462b 100644 --- a/http/sys_init.go +++ b/http/sys_init.go @@ -61,10 +61,9 @@ func handleSysInitPut(core *vault.Core, w http.ResponseWriter, r *http.Request) } recoveryConfig := &vault.SealConfig{ - SecretShares: req.RecoveryShares, - SecretThreshold: req.RecoveryThreshold, - PGPKeys: req.RecoveryPGPKeys, - DisableUnsealRecovery: req.DisableUnsealRecovery, + SecretShares: req.RecoveryShares, + SecretThreshold: req.RecoveryThreshold, + PGPKeys: req.RecoveryPGPKeys, } initParams := &vault.InitParams{ @@ -116,15 +115,14 @@ func handleSysInitPut(core *vault.Core, w http.ResponseWriter, r *http.Request) } type InitRequest struct { - SecretShares int `json:"secret_shares"` - SecretThreshold int `json:"secret_threshold"` - StoredShares int `json:"stored_shares"` - PGPKeys []string `json:"pgp_keys"` - RecoveryShares int `json:"recovery_shares"` - RecoveryThreshold int `json:"recovery_threshold"` - RecoveryPGPKeys []string `json:"recovery_pgp_keys"` - RootTokenPGPKey string `json:"root_token_pgp_key"` - DisableUnsealRecovery bool `json:"disable_unseal_recovery"` + SecretShares int `json:"secret_shares"` + SecretThreshold int `json:"secret_threshold"` + StoredShares int `json:"stored_shares"` + PGPKeys []string `json:"pgp_keys"` + RecoveryShares int `json:"recovery_shares"` + RecoveryThreshold int `json:"recovery_threshold"` + RecoveryPGPKeys []string `json:"recovery_pgp_keys"` + RootTokenPGPKey string `json:"root_token_pgp_key"` } type InitResponse struct { diff --git a/internalshared/configutil/kms.go b/internalshared/configutil/kms.go index 2166f9dd4c..614a6ec8e5 100644 --- a/internalshared/configutil/kms.go +++ b/internalshared/configutil/kms.go @@ -51,7 +51,6 @@ type KMS struct { Purpose []string `hcl:"-"` Disabled bool - Recover bool Config map[string]string } @@ -100,15 +99,6 @@ func parseKMS(result *[]*KMS, list *ast.ObjectList, blockName string, maxKMS int delete(m, "disabled") } - var recover bool - if v, ok := m["recovery_keys_fallback"]; ok { - recover, err = parseutil.ParseBool(v) - if err != nil { - return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key)) - } - delete(m, "recovery_keys_fallback") - } - strMap := make(map[string]string, len(m)) for k, v := range m { s, err := parseutil.ParseString(v) @@ -122,7 +112,6 @@ func parseKMS(result *[]*KMS, list *ast.ObjectList, blockName string, maxKMS int Type: strings.ToLower(key), Purpose: purpose, Disabled: disabled, - Recover: recover, } if len(strMap) > 0 { seal.Config = strMap diff --git a/vault/core.go b/vault/core.go index cdc875a503..4c1528b1db 100644 --- a/vault/core.go +++ b/vault/core.go @@ -1696,11 +1696,7 @@ func (c *Core) sealMigrated(ctx context.Context) (bool, error) { return false, nil } - // If the types of the seals differ, e.g. auto->shamir or shamir->auto, we're done. BUT, - // with an auto seal in recovery mode as the migration seal, they will match even though - // the migration seal was really an auto seal - if c.seal.BarrierType() != c.migrationInfo.seal.BarrierType() || - (isAutoSeal(c.migrationInfo.seal) && c.seal.BarrierType() == wrapping.WrapperTypeShamir) { + if c.seal.BarrierType() != c.migrationInfo.seal.BarrierType() { return true, nil } @@ -1752,34 +1748,20 @@ func (c *Core) migrateSeal(ctx context.Context) error { return fmt.Errorf("error getting recovery key to set on new seal: %w", err) } - if err := c.migrateAutoToAuto(ctx, recoveryKey); err != nil { - return err + if err := c.seal.SetRecoveryKey(ctx, recoveryKey); err != nil { + return fmt.Errorf("error setting new recovery key information during migrate: %w", err) } - case isUnsealRecoverySeal(c.migrationInfo.seal) && c.seal.RecoveryKeySupported(): - c.logger.Info("migrating from one auto-unseal to another", "from", - c.migrationInfo.seal.BarrierType(), "to", c.seal.BarrierType()) - recoveryKey, err := c.migrationInfo.seal.UnsealRecoveryKey(ctx) + barrierKeys, err := c.migrationInfo.seal.GetStoredKeys(ctx) if err != nil { - return fmt.Errorf("error getting recovery key to set on new seal: %w", err) + return fmt.Errorf("error getting stored keys to set on new seal: %w", err) } - if err := c.migrateAutoToAuto(ctx, recoveryKey); err != nil { - return err - } - case isUnsealRecoverySeal(c.migrationInfo.seal): - c.logger.Info("migrating from one auto-unseal to shamir", "from", c.migrationInfo.seal.BarrierType()) - // Auto to Shamir, since recovery key isn't supported on new seal - - recoveryKey, err := c.migrationInfo.seal.UnsealRecoveryKey(ctx) - if err != nil { - return fmt.Errorf("error getting recovery key to set on new seal: %w", err) + if err := c.seal.SetStoredKeys(ctx, barrierKeys); err != nil { + return fmt.Errorf("error setting new barrier key information during migrate: %w", err) } - if err := c.migrateAutoToShamir(ctx, recoveryKey); err != nil { - return err - } - case isAutoSeal(c.migrationInfo.seal): + case c.migrationInfo.seal.RecoveryKeySupported(): c.logger.Info("migrating from one auto-unseal to shamir", "from", c.migrationInfo.seal.BarrierType()) // Auto to Shamir, since recovery key isn't supported on new seal @@ -1788,9 +1770,21 @@ func (c *Core) migrateSeal(ctx context.Context) error { return fmt.Errorf("error getting recovery key to set on new seal: %w", err) } - if err := c.migrateAutoToShamir(ctx, recoveryKey); err != nil { - return err + // We have recovery keys; we're going to use them as the new shamir KeK. + err = c.seal.GetAccess().Wrapper.(*aeadwrapper.ShamirWrapper).SetAesGcmKeyBytes(recoveryKey) + if err != nil { + return fmt.Errorf("failed to set master key in seal: %w", err) } + + barrierKeys, err := c.migrationInfo.seal.GetStoredKeys(ctx) + if err != nil { + return fmt.Errorf("error getting stored keys to set on new seal: %w", err) + } + + if err := c.seal.SetStoredKeys(ctx, barrierKeys); err != nil { + return fmt.Errorf("error setting new barrier key information during migrate: %w", err) + } + case c.seal.RecoveryKeySupported(): c.logger.Info("migrating from shamir to auto-unseal", "to", c.seal.BarrierType()) // Migration is happening from shamir -> auto. In this case use the shamir @@ -1816,16 +1810,6 @@ func (c *Core) migrateSeal(ctx context.Context) error { return fmt.Errorf("error storing new master key: %w", err) } - // Store the unseal recovery key - wrapper := aeadwrapper.NewShamirWrapper() - wrapper.SetAesGcmKeyBytes(c.migrationInfo.unsealKey) - recoverySeal := NewRecoverySeal(&vaultseal.Access{ - Wrapper: wrapper, - }) - recoverySeal.SetCore(c) - if err := recoverySeal.SetStoredKeys(ctx, [][]byte{newMasterKey}); err != nil { - c.logger.Error("failed to store recovery unseal keys", "error", err) - } default: return errors.New("unhandled migration case (shamir to shamir)") } @@ -1842,40 +1826,6 @@ func (c *Core) migrateSeal(ctx context.Context) error { return nil } -func (c *Core) migrateAutoToAuto(ctx context.Context, recoveryKey []byte) error { - if err := c.seal.SetRecoveryKey(ctx, recoveryKey); err != nil { - return fmt.Errorf("error setting new recovery key information during migrate: %w", err) - } - - barrierKeys, err := c.migrationInfo.seal.GetStoredKeys(ctx) - if err != nil { - return fmt.Errorf("error getting stored keys to set on new seal: %w", err) - } - - if err := c.seal.SetStoredKeys(ctx, barrierKeys); err != nil { - return fmt.Errorf("error setting new barrier key information during migrate: %w", err) - } - return nil -} - -func (c *Core) migrateAutoToShamir(ctx context.Context, recoveryKey []byte) error { - // We have recovery keys; we're going to use them as the new shamir KeK. - err := c.seal.GetAccess().Wrapper.(*aeadwrapper.ShamirWrapper).SetAesGcmKeyBytes(recoveryKey) - if err != nil { - return fmt.Errorf("failed to set master key in seal: %w", err) - } - - barrierKeys, err := c.migrationInfo.seal.GetStoredKeys(ctx) - if err != nil { - return fmt.Errorf("error getting stored keys to set on new seal: %w", err) - } - - if err := c.seal.SetStoredKeys(ctx, barrierKeys); err != nil { - return fmt.Errorf("error setting new barrier key information during migrate: %w", err) - } - return nil -} - // unsealInternal takes in the master key and attempts to unseal the barrier. // N.B.: This must be called with the state write lock held. func (c *Core) unsealInternal(ctx context.Context, masterKey []byte) error { @@ -2805,12 +2755,6 @@ func (c *Core) adjustForSealMigration(unwrapSeal Seal) error { // 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. return nil - case c.seal.BarrierType() == wrapping.WrapperTypeShamir && isUnsealRecoverySeal(c.seal): - // The stored barrier config is not shamir, but we have a shamir seal anyway, because - // seal recovery mode has been requested. Note that this isn't for migration, but this function - // is called regardless of whether migration is occurring or not, and this is a valid state - // for the seal to be in, thus we mustn't reject it. - return nil case c.seal.BarrierType() == wrapping.WrapperTypeShamir: // The stored barrier config is not shamir, there is no disabled seal // in config, and either no configured seal (which equates to Shamir) @@ -2834,7 +2778,7 @@ func (c *Core) adjustForSealMigration(unwrapSeal Seal) error { // If we're not coming from Shamir we expect the previous seal to be // in the config and disabled. - if unwrapSeal.BarrierType() == wrapping.WrapperTypeShamir && !isUnsealRecoverySeal(unwrapSeal) { + if unwrapSeal.BarrierType() == wrapping.WrapperTypeShamir { return errors.New("Shamir seals cannot be set disabled (they should simply not be set)") } } @@ -2870,21 +2814,6 @@ func (c *Core) adjustForSealMigration(unwrapSeal Seal) error { return nil } -// With the addition of unseal recovery mode, we need a more subtle check for whether a Seal is an auto -// seal, as it may be a Shamir seal in due to recovery mode but would otherwise have been an auto seal. -func isAutoSeal(seal Seal) bool { - return seal.RecoveryKeySupported() || isUnsealRecoverySeal(seal) -} - -// Returns whether a seal is a recovery seal, e.g. a shamir seal pointing at the root key encrypted by -// recovery keys rather than the seal wrapper. -func isUnsealRecoverySeal(seal Seal) bool { - if ds, ok := seal.(*defaultSeal); ok { - return ds.unsealKeyPath == recoveryUnsealKeyPath - } - return false -} - func (c *Core) migrateSealConfig(ctx context.Context) error { existBarrierSealConfig, existRecoverySealConfig, err := c.PhysicalSealConfigs(ctx) if err != nil { @@ -2894,10 +2823,10 @@ func (c *Core) migrateSealConfig(ctx context.Context) error { var bc, rc *SealConfig switch { - case isAutoSeal(c.migrationInfo.seal) && c.seal.RecoveryKeySupported(): + case c.migrationInfo.seal.RecoveryKeySupported() && c.seal.RecoveryKeySupported(): // Migrating from auto->auto, copy the configs over bc, rc = existBarrierSealConfig, existRecoverySealConfig - case isAutoSeal(c.migrationInfo.seal): + case c.migrationInfo.seal.RecoveryKeySupported(): // Migrating from auto->shamir, clone auto's recovery config and set // stored keys to 1. bc = existRecoverySealConfig.Clone() @@ -2934,7 +2863,7 @@ func (c *Core) migrateSealConfig(ctx context.Context) error { func (c *Core) adjustSealConfigDuringMigration(existBarrierSealConfig, existRecoverySealConfig *SealConfig) { switch { - case isAutoSeal(c.migrationInfo.seal) && existRecoverySealConfig != nil: + case c.migrationInfo.seal.RecoveryKeySupported() && existRecoverySealConfig != nil: // Migrating from auto->shamir, clone auto's recovery config and set // stored keys to 1. Unless the recover config doesn't exist, in which // case the migration is assumed to already have been performed. diff --git a/vault/external_tests/sealmigration/seal_migration_oss_test.go b/vault/external_tests/sealmigration/seal_migration_oss_test.go deleted file mode 100644 index 16760d57b7..0000000000 --- a/vault/external_tests/sealmigration/seal_migration_oss_test.go +++ /dev/null @@ -1,10 +0,0 @@ -//go:build !enterprise - -package sealmigration - -import "testing" - -func TestSealMigration_TransitToShamir_Recovery(t *testing.T) { - t.Parallel() - testVariousBackends(t, ParamTestSealMigrationTransitToShamir_Recovery, BasePort_TransitToShamir_Recovery, true) -} diff --git a/vault/external_tests/sealmigration/testshared.go b/vault/external_tests/sealmigration/testshared.go index 01fb98edbd..4cce304492 100644 --- a/vault/external_tests/sealmigration/testshared.go +++ b/vault/external_tests/sealmigration/testshared.go @@ -7,9 +7,6 @@ import ( "testing" "time" - "github.com/hashicorp/go-kms-wrapping/wrappers/aead/v2" - "github.com/hashicorp/vault/vault/seal" - "github.com/go-test/deep" "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping/v2" @@ -28,12 +25,11 @@ const ( keyShares = 3 keyThreshold = 3 - BasePort_ShamirToTransit_Pre14 = 20000 - BasePort_TransitToShamir_Pre14 = 21000 - BasePort_ShamirToTransit_Post14 = 22000 - BasePort_TransitToShamir_Post14 = 23000 - BasePort_TransitToTransit = 24000 - BasePort_TransitToShamir_Recovery = 25000 + BasePort_ShamirToTransit_Pre14 = 20000 + BasePort_TransitToShamir_Pre14 = 21000 + BasePort_ShamirToTransit_Post14 = 22000 + BasePort_TransitToShamir_Post14 = 23000 + BasePort_TransitToTransit = 24000 ) func ParamTestSealMigrationTransitToShamir_Pre14(t *testing.T, logger hclog.Logger, storage teststorage.ReusableStorage, basePort int) { @@ -124,55 +120,6 @@ func ParamTestSealMigrationTransitToShamir_Post14(t *testing.T, logger hclog.Log runShamir(t, logger, storage, basePort, rootToken, recoveryKeys) } -func ParamTestSealMigrationTransitToShamir_Recovery(t *testing.T, logger hclog.Logger, storage teststorage.ReusableStorage, basePort int) { - // Create the transit server. - tss := sealhelper.NewTransitSealServer(t, 0) - defer func() { - if tss != nil { - tss.Cleanup() - } - }() - sealKeyName := "transit-seal-key-1" - tss.MakeKey(t, sealKeyName) - - // Initialize the backend with transit. - cluster, opts := InitializeTransit(t, logger, storage, basePort, tss, sealKeyName) - rootToken, recoveryKeys := cluster.RootToken, cluster.RecoveryKeys - - // Disable the transit seal, forcing recovery - tss.Cleanup() - tss = nil - - // conf := cluster.Cores[0].GetCoreConfigInternal() - // conf.Seals[0].Recover = true - - opts.UnwrapSealFunc = func() vault.Seal { - seal := vault.NewRecoverySeal(&seal.Access{ - Wrapper: aead.NewShamirWrapper(), - WrapperType: wrapping.WrapperTypeShamir, - }) - seal.SetCachedBarrierConfig(&vault.SealConfig{ - SecretShares: len(recoveryKeys), - SecretThreshold: len(recoveryKeys), - StoredShares: len(recoveryKeys), - }) - return seal - } - - // Migrate the backend from transit to shamir - // opts.UnwrapSealFunc = opts.SealFunc - opts.SealFunc = func() vault.Seal { return nil } - leaderIdx := migratePost14(t, storage, cluster, opts, cluster.RecoveryKeys) - validateMigration(t, storage, cluster, leaderIdx, verifySealConfigShamir) - - cluster.Cleanup() - storage.Cleanup(t, cluster) - - // Run the backend with shamir. Note that the recovery keys are now the - // barrier keys. - runShamir(t, logger, storage, basePort, rootToken, recoveryKeys) -} - func ParamTestSealMigrationShamirToTransit_Post14(t *testing.T, logger hclog.Logger, storage teststorage.ReusableStorage, basePort int) { // Initialize the backend using shamir cluster, opts := initializeShamir(t, logger, storage, basePort) diff --git a/vault/init.go b/vault/init.go index e8a154e26a..e148ef3657 100644 --- a/vault/init.go +++ b/vault/init.go @@ -386,18 +386,6 @@ func (c *Core) Initialize(ctx context.Context, initParams *InitParams) (*InitRes } results.RecoveryShares = recoveryUnsealKeys - - if !initParams.RecoveryConfig.DisableUnsealRecovery { - wrapper := aeadwrapper.NewShamirWrapper() - wrapper.SetAesGcmKeyBytes(recoveryKey) - recoverySeal := NewRecoverySeal(&seal.Access{ - Wrapper: wrapper, - }) - recoverySeal.SetCore(c) - if err := recoverySeal.SetStoredKeys(ctx, [][]byte{barrierKey}); err != nil { - c.logger.Error("failed to store recovery unseal keys", "error", err) - } - } } } diff --git a/vault/rekey.go b/vault/rekey.go index f003e0e8d2..742543a55c 100644 --- a/vault/rekey.go +++ b/vault/rekey.go @@ -252,9 +252,7 @@ func (c *Core) RecoveryRekeyInit(config *SealConfig) logical.HTTPCodedError { c.logger.Error("invalid recovery configuration", "error", err) return logical.CodedError(http.StatusInternalServerError, fmt.Errorf("invalid recovery configuration: %w", err).Error()) } - if isUnsealRecoverySeal(c.seal) { - return logical.CodedError(http.StatusBadRequest, "cannot rekey while in unseal recovery mode") - } + if !c.seal.RecoveryKeySupported() { return logical.CodedError(http.StatusBadRequest, "recovery keys not supported") } @@ -759,31 +757,6 @@ func (c *Core) RecoveryRekeyUpdate(ctx context.Context, key []byte, nonce string return nil, logical.CodedError(http.StatusInternalServerError, fmt.Errorf("failed to perform recovery rekey: %w", err).Error()) } - // Store the unseal recovery key - wrapper := aeadwrapper.NewShamirWrapper() - // TODO, just test if keys existed in the recovery path - - keys, err := c.physical.Get(ctx, recoveryUnsealKeyPath) - if err != nil { - return nil, logical.CodedError(http.StatusInternalServerError, fmt.Errorf("failed to perform recovery rekey, failed testing for recovery unseal keys: %w", err).Error()) - } - if keys != nil { - rootKeys, err := c.seal.GetStoredKeys(ctx) - if err != nil { - return nil, logical.CodedError(http.StatusInternalServerError, fmt.Errorf("failed to perform recovery rekey, failed retrieving root keys: %w", err).Error()) - } - - wrapper.SetAesGcmKeyBytes(newRecoveryKey) - recoverySeal := NewRecoverySeal(&seal.Access{ - Wrapper: wrapper, - }) - recoverySeal.SetCore(c) - - if err := recoverySeal.SetStoredKeys(ctx, rootKeys); err != nil { - c.logger.Error("failed to store recovery unseal keys", "error", err) - } - } - c.recoveryRekeyConfig = nil return results, nil } diff --git a/vault/seal.go b/vault/seal.go index b02c4f5fb3..d8bafc6165 100644 --- a/vault/seal.go +++ b/vault/seal.go @@ -10,12 +10,6 @@ import ( "strings" "sync/atomic" - "github.com/hashicorp/go-kms-wrapping/wrappers/aead/v2" - - log "github.com/hashicorp/go-hclog" - - "github.com/hashicorp/go-multierror" - "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/physical" @@ -46,12 +40,6 @@ const ( // recoveryKeyPath is the path to the recovery key recoveryKeyPath = "core/recovery-key" - // recoveryUnsealKeyPath is the path to a copy of the root key, - // encrypted using the shamir-combined recovery keys, just like - // StoredBarrierKeysPath is the root key encrypted by the seal - // (which in the case of a shamir seal is the shamir-combined unseal keys.) - recoveryUnsealKeyPath = "core/recovery-unseal-key" - // StoredBarrierKeysPath is the path used for storing HSM-encrypted unseal keys StoredBarrierKeysPath = "core/hsm/barrier-unseal-keys" @@ -80,7 +68,6 @@ type Seal interface { RecoveryType() string RecoveryConfig(context.Context) (*SealConfig, error) RecoveryKey(context.Context) ([]byte, error) - UnsealRecoveryKey(ctx context.Context) ([]byte, error) SetRecoveryConfig(context.Context, *SealConfig) error SetCachedRecoveryConfig(*SealConfig) SetRecoveryKey(context.Context, []byte) error @@ -89,26 +76,14 @@ type Seal interface { } type defaultSeal struct { - access *seal.Access - config atomic.Value - logger log.Logger - core *Core - unsealKeyPath string + access *seal.Access + config atomic.Value + core *Core } func NewDefaultSeal(lowLevel *seal.Access) Seal { ret := &defaultSeal{ - access: lowLevel, - unsealKeyPath: StoredBarrierKeysPath, - } - ret.config.Store((*SealConfig)(nil)) - return ret -} - -func NewRecoverySeal(lowLevel *seal.Access) Seal { - ret := &defaultSeal{ - access: lowLevel, - unsealKeyPath: recoveryUnsealKeyPath, + access: lowLevel, } ret.config.Store((*SealConfig)(nil)) return ret @@ -135,14 +110,6 @@ func (d *defaultSeal) SetAccess(access *seal.Access) { func (d *defaultSeal) SetCore(core *Core) { d.core = core - if d.logger == nil { - if isUnsealRecoverySeal(d) { - d.logger = d.core.Logger().Named("recoveryseal") - } else { - d.logger = d.core.Logger().Named("defaultseal") - } - d.core.AddLogger(d.logger) - } } func (d *defaultSeal) Init(ctx context.Context) error { @@ -174,7 +141,7 @@ func (d *defaultSeal) SetStoredKeys(ctx context.Context, keys [][]byte) error { if d.LegacySeal() { return fmt.Errorf("stored keys are not supported") } - return writeStoredKeys(ctx, d.core.physical, d.access, keys, d.unsealKeyPath) + return writeStoredKeys(ctx, d.core.physical, d.access, keys) } func (d *defaultSeal) LegacySeal() bool { @@ -189,7 +156,7 @@ func (d *defaultSeal) GetStoredKeys(ctx context.Context) ([][]byte, error) { if d.LegacySeal() { return nil, fmt.Errorf("stored keys are not supported") } - keys, err := readStoredKeys(ctx, d.core.physical, d.access, d.unsealKeyPath) + keys, err := readStoredKeys(ctx, d.core.physical, d.access) return keys, err } @@ -230,10 +197,8 @@ func (d *defaultSeal) BarrierConfig(ctx context.Context) (*SealConfig, error) { conf.Type = d.BarrierType().String() case d.BarrierType().String(): default: - if conf.Type == wrapping.WrapperTypeShamir.String() || d.unsealKeyPath != recoveryUnsealKeyPath { - d.core.logger.Error("barrier seal type does not match expected type", "barrier_seal_type", conf.Type, "loaded_seal_type", d.BarrierType()) - return nil, fmt.Errorf("barrier seal type of %q does not match expected type of %q", conf.Type, d.BarrierType()) - } + d.core.logger.Error("barrier seal type does not match expected type", "barrier_seal_type", conf.Type, "loaded_seal_type", d.BarrierType()) + return nil, fmt.Errorf("barrier seal type of %q does not match expected type of %q", conf.Type, d.BarrierType()) } // Check for a valid seal configuration @@ -301,10 +266,6 @@ func (d *defaultSeal) RecoveryConfig(ctx context.Context) (*SealConfig, error) { return nil, fmt.Errorf("recovery not supported") } -func (d *defaultSeal) UnsealRecoveryKey(ctx context.Context) ([]byte, error) { - return d.access.Wrapper.(*aead.ShamirWrapper).KeyBytes(ctx) -} - func (d *defaultSeal) RecoveryKey(ctx context.Context) ([]byte, error) { return nil, fmt.Errorf("recovery not supported") } @@ -320,10 +281,6 @@ func (d *defaultSeal) VerifyRecoveryKey(ctx context.Context, key []byte) error { return fmt.Errorf("recovery not supported") } -func (d *defaultSeal) VerifyRecoveryUnsealKey(ctx context.Context, key []byte) error { - return fmt.Errorf("recovery unseal not supported") -} - func (d *defaultSeal) SetRecoveryKey(ctx context.Context, key []byte) error { return fmt.Errorf("recovery not supported") } @@ -359,9 +316,6 @@ type SealConfig struct { // How many keys to store, for seals that support storage. Always 0 or 1. StoredShares int `json:"stored_shares" mapstructure:"stored_shares"` - // Whether unseal using recovery keys should be disabled - DisableUnsealRecovery bool `json:"disable_unseal_recovery" mapstructure:"disable_unseal_recovery"` - // Stores the progress of the rekey operation (key shares) RekeyProgress [][]byte `json:"-"` @@ -475,7 +429,7 @@ func (e *ErrDecrypt) Is(target error) bool { return ok || errors.Is(e.Err, target) } -func writeStoredKeys(ctx context.Context, storage physical.Backend, encryptor *seal.Access, keys [][]byte, path string) error { +func writeStoredKeys(ctx context.Context, storage physical.Backend, encryptor *seal.Access, keys [][]byte) error { if keys == nil { return fmt.Errorf("keys were nil") } @@ -501,7 +455,7 @@ func writeStoredKeys(ctx context.Context, storage physical.Backend, encryptor *s // Store the seal configuration. pe := &physical.Entry{ - Key: path, + Key: StoredBarrierKeysPath, Value: value, } @@ -512,8 +466,8 @@ func writeStoredKeys(ctx context.Context, storage physical.Backend, encryptor *s return nil } -func readStoredKeys(ctx context.Context, storage physical.Backend, encryptor *seal.Access, path string) ([][]byte, error) { - pe, err := storage.Get(ctx, path) +func readStoredKeys(ctx context.Context, storage physical.Backend, encryptor *seal.Access) ([][]byte, error) { + pe, err := storage.Get(ctx, StoredBarrierKeysPath) if err != nil { return nil, fmt.Errorf("failed to fetch stored keys: %w", err) } @@ -524,22 +478,17 @@ func readStoredKeys(ctx context.Context, storage physical.Backend, encryptor *se return nil, nil } - var blobInfo wrapping.BlobInfo - // Read as a multi-blob first - if err := proto.Unmarshal(pe.Value, &blobInfo); err != nil { + blobInfo := &wrapping.BlobInfo{} + if err := proto.Unmarshal(pe.Value, blobInfo); err != nil { return nil, fmt.Errorf("failed to proto decode stored keys: %w", err) } - pt, err := encryptor.Decrypt(ctx, &blobInfo, nil) + pt, err := encryptor.Decrypt(ctx, blobInfo, nil) if err != nil { if strings.Contains(err.Error(), "message authentication failed") { - err = multierror.Append(err, &ErrInvalidKey{Reason: fmt.Sprintf("failed to decrypt keys from storage: %v", err)}) - } else { - err = multierror.Append(err, &ErrDecrypt{Err: fmt.Errorf("failed to decrypt keys from storage: %w", err)}) + return nil, &ErrInvalidKey{Reason: fmt.Sprintf("failed to decrypt keys from storage: %v", err)} } - } - if len(pt) == 0 { - return nil, err + return nil, &ErrDecrypt{Err: fmt.Errorf("failed to decrypt keys from storage: %w", err)} } // Decode the barrier entry diff --git a/vault/seal_autoseal.go b/vault/seal_autoseal.go index be1498b88a..59afd3596b 100644 --- a/vault/seal_autoseal.go +++ b/vault/seal_autoseal.go @@ -108,13 +108,13 @@ func (d *autoSeal) RecoveryKeySupported() bool { // SetStoredKeys uses the autoSeal.Access.Encrypts method to wrap the keys. The stored entry // does not need to be seal wrapped in this case. func (d *autoSeal) SetStoredKeys(ctx context.Context, keys [][]byte) error { - return writeStoredKeys(ctx, d.core.physical, d.Access, keys, StoredBarrierKeysPath) + return writeStoredKeys(ctx, d.core.physical, d.Access, keys) } // GetStoredKeys retrieves the key shares by unwrapping the encrypted key using the // autoseal. func (d *autoSeal) GetStoredKeys(ctx context.Context) ([][]byte, error) { - return readStoredKeys(ctx, d.core.physical, d.Access, StoredBarrierKeysPath) + return readStoredKeys(ctx, d.core.physical, d.Access) } func (d *autoSeal) upgradeStoredKeys(ctx context.Context) error { @@ -435,22 +435,14 @@ func (d *autoSeal) RecoveryKey(ctx context.Context) ([]byte, error) { return d.getRecoveryKeyInternal(ctx) } -func (d *autoSeal) UnsealRecoveryKey(ctx context.Context) ([]byte, error) { - return nil, fmt.Errorf("unseal recovery not supported") -} - func (d *autoSeal) getRecoveryKeyInternal(ctx context.Context) ([]byte, error) { - return getRecoveryKeyInternal(ctx, d.core.physical, d.logger, d.Access) -} - -func getRecoveryKeyInternal(ctx context.Context, storage physical.Backend, logger log.Logger, access *seal.Access) ([]byte, error) { - pe, err := storage.Get(ctx, recoveryKeyPath) + pe, err := d.core.physical.Get(ctx, recoveryKeyPath) if err != nil { - logger.Error("failed to read recovery key", "error", err) + d.logger.Error("failed to read recovery key", "error", err) return nil, fmt.Errorf("failed to read recovery key: %w", err) } if pe == nil { - logger.Warn("no recovery key found") + d.logger.Warn("no recovery key found") return nil, fmt.Errorf("no recovery key found") } @@ -459,7 +451,7 @@ func getRecoveryKeyInternal(ctx context.Context, storage physical.Backend, logge return nil, fmt.Errorf("failed to proto decode stored keys: %w", err) } - pt, err := access.Decrypt(ctx, blobInfo, nil) + pt, err := d.Decrypt(ctx, blobInfo, nil) if err != nil { return nil, fmt.Errorf("failed to decrypt encrypted stored keys: %w", err) }