From cdf6bf0669a975147f67e8dd98dc0ff74fbb5aa1 Mon Sep 17 00:00:00 2001 From: Divya Pola <87338962+divyapola5@users.noreply.github.com> Date: Thu, 31 Aug 2023 14:32:22 -0500 Subject: [PATCH] Add checks to see if current seal generation is in middle of rewrapping (#22692) * Add checks to see if current seal generation is in middle of rewrapping * Check for rewrap flag only if thr previous configuration is not shamir --- command/operator_diagnose.go | 8 +- command/server.go | 54 +++++-- command/server_sealgenerationinfo_test.go | 180 +++++++++++++++++++--- vault/seal/seal.go | 6 +- vault/seal_stubs_oss.go | 10 ++ 5 files changed, 216 insertions(+), 42 deletions(-) diff --git a/command/operator_diagnose.go b/command/operator_diagnose.go index 0fa75fb88b..32c1620727 100644 --- a/command/operator_diagnose.go +++ b/command/operator_diagnose.go @@ -434,13 +434,19 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error sealcontext, sealspan := diagnose.StartSpan(ctx, "Create Vault Server Configuration Seals") var setSealResponse *SetSealResponse + var hasPartialPaths bool existingSealGenerationInfo, err := vault.PhysicalSealGenInfo(sealcontext, *backend) if err != nil { diagnose.Fail(sealcontext, fmt.Sprintf("Unable to get Seal genration information from storage: %s.", err.Error())) goto SEALFAIL } - setSealResponse, err = setSeal(server, config, make([]string, 0), make(map[string]string), existingSealGenerationInfo, server.logger) + hasPartialPaths, err = hasPartiallyWrappedPaths(context.Background(), *backend) + if err != nil { + diagnose.Fail(sealcontext, fmt.Sprintf("Cannot determine if there are parrtially seal wrapped entries in storage: %s.", err.Error())) + goto SEALFAIL + } + setSealResponse, err = setSeal(server, config, make([]string, 0), make(map[string]string), existingSealGenerationInfo, hasPartialPaths) if err != nil { diagnose.Advise(ctx, "For assistance with the seal stanza, see the Vault configuration documentation.") diagnose.Fail(sealcontext, fmt.Sprintf("Seal creation resulted in the following error: %s.", err.Error())) diff --git a/command/server.go b/command/server.go index 7aa2e15c38..eb8767b4d9 100644 --- a/command/server.go +++ b/command/server.go @@ -560,13 +560,19 @@ func (c *ServerCommand) runRecoveryMode() int { return 1 } - existingSealGenenrationInfo, err := vault.PhysicalSealGenInfo(context.Background(), backend) + ctx := context.Background() + existingSealGenerationInfo, err := vault.PhysicalSealGenInfo(ctx, backend) if err != nil { c.UI.Error(fmt.Sprintf("Error getting seal generation info: %v", err)) return 1 } - setSealResponse, err := setSeal(c, config, infoKeys, info, existingSealGenenrationInfo, c.logger) + hasPartialPaths, err := hasPartiallyWrappedPaths(ctx, backend) + if err != nil { + c.UI.Error(fmt.Sprintf("Cannot determine if there are parrtially seal wrapped entries in storage: %v", err)) + return 1 + } + setSealResponse, err := setSeal(c, config, infoKeys, info, existingSealGenerationInfo, hasPartialPaths) if err != nil { c.UI.Error(err.Error()) return 1 @@ -579,7 +585,7 @@ func (c *ServerCommand) runRecoveryMode() int { // Ensure that the seal finalizer is called, even if using verify-only defer func() { - err = barrierSeal.Finalize(context.Background()) + err = barrierSeal.Finalize(ctx) if err != nil { c.UI.Error(fmt.Sprintf("Error finalizing seals: %v", err)) } @@ -605,7 +611,7 @@ func (c *ServerCommand) runRecoveryMode() int { } } - if err := core.InitializeRecovery(context.Background()); err != nil { + if err := core.InitializeRecovery(ctx); err != nil { c.UI.Error(fmt.Sprintf("Error initializing core in recovery mode: %s", err)) return 1 } @@ -706,7 +712,7 @@ func (c *ServerCommand) runRecoveryMode() int { } if sealConfigError != nil { - init, err := core.InitializedLocally(context.Background()) + init, err := core.InitializedLocally(ctx) if err != nil { c.UI.Error(fmt.Sprintf("Error checking if core is initialized: %v", err)) return 1 @@ -1223,13 +1229,19 @@ func (c *ServerCommand) Run(args []string) int { infoKeys = append(infoKeys, expKey) } - existingSealGenenrationInfo, err := vault.PhysicalSealGenInfo(context.Background(), backend) + ctx := context.Background() + existingSealGenerationInfo, err := vault.PhysicalSealGenInfo(ctx, backend) if err != nil { c.UI.Error(fmt.Sprintf("Error getting seal generation info: %v", err)) return 1 } - setSealResponse, err := setSeal(c, config, infoKeys, info, existingSealGenenrationInfo, c.logger) + hasPartialPaths, err := hasPartiallyWrappedPaths(ctx, backend) + if err != nil { + c.UI.Error(fmt.Sprintf("Cannot determine if there are parrtially seal wrapped entries in storage: %v", err)) + return 1 + } + setSealResponse, err := setSeal(c, config, infoKeys, info, existingSealGenerationInfo, hasPartialPaths) if err != nil { c.UI.Error(err.Error()) return 1 @@ -1239,7 +1251,7 @@ func (c *ServerCommand) Run(args []string) int { seal := seal // capture range variable // Ensure that the seal finalizer is called, even if using verify-only defer func(seal *vault.Seal) { - err = (*seal).Finalize(context.Background()) + err = (*seal).Finalize(ctx) if err != nil { c.UI.Error(fmt.Sprintf("Error finalizing seals: %v", err)) } @@ -1489,14 +1501,14 @@ func (c *ServerCommand) Run(args []string) int { // uninitialized. Once one server initializes the storage backend, this // goroutine will pick up the unseal keys and unseal this instance. if !core.IsInSealMigrationMode() { - go runUnseal(c, core, context.Background()) + go runUnseal(c, core, ctx) } // When the underlying storage is raft, kick off retry join if it was specified // in the configuration // TODO: Should we also support retry_join for ha_storage? if config.Storage.Type == storageTypeRaft { - if err := core.InitiateRetryJoin(context.Background()); err != nil { + if err := core.InitiateRetryJoin(ctx); err != nil { c.UI.Error(fmt.Sprintf("Failed to initiate raft retry join, %q", err.Error())) return 1 } @@ -1542,7 +1554,7 @@ func (c *ServerCommand) Run(args []string) int { } if setSealResponse.sealConfigError != nil { - init, err := core.InitializedLocally(context.Background()) + init, err := core.InitializedLocally(ctx) if err != nil { c.UI.Error(fmt.Sprintf("Error checking if core is initialized: %v", err)) return 1 @@ -2532,7 +2544,7 @@ func (r *SetSealResponse) getCreatedSeals() []*vault.Seal { // setSeal return barrierSeal, barrierWrapper, unwrapSeal, all the created seals, and all the provided seals from the configs so we can close them in Run // The two errors are the sealConfigError and the regular error -func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info map[string]string, existingSealGenerationInfo *vaultseal.SealGenerationInfo, sealLogger hclog.Logger) (*SetSealResponse, error) { +func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info map[string]string, existingSealGenerationInfo *vaultseal.SealGenerationInfo, hasPartiallyWrappedPaths bool) (*SetSealResponse, error) { if c.flagDevAutoSeal { access, _ := vaultseal.NewTestSeal(nil) barrierSeal := vault.NewAutoSeal(access) @@ -2654,7 +2666,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Compute seal generation - sealGenerationInfo, err := c.computeSealGenerationInfo(existingSealGenerationInfo, allSealKmsConfigs) + sealGenerationInfo, err := c.computeSealGenerationInfo(existingSealGenerationInfo, allSealKmsConfigs, hasPartiallyWrappedPaths) if err != nil { return nil, err } @@ -2678,6 +2690,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma var barrierSeal vault.Seal var unwrapSeal vault.Seal + sealLogger := c.logger switch { case len(enabledSealWrappers) == 0: return nil, errors.New("no enabled Seals in configuration") @@ -2723,7 +2736,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma }, nil } -func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal.SealGenerationInfo, sealConfigs []*configutil.KMS) (*vaultseal.SealGenerationInfo, error) { +func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal.SealGenerationInfo, sealConfigs []*configutil.KMS, hasPartiallyWrappedPaths bool) (*vaultseal.SealGenerationInfo, error) { generation := uint64(1) if existingSealGenInfo != nil { @@ -2732,7 +2745,7 @@ func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal } generation = existingSealGenInfo.Generation + 1 } - c.logger.Info("incrementing seal geneneration", "generation", generation) + c.logger.Info("incrementing seal generation", "generation", generation) // If the stored copy doesn't match the current configuration, we introduce a new generation // which keeps track if a rewrap of all CSPs and seal wrapped values has completed (initially false). @@ -2741,7 +2754,7 @@ func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal Seals: sealConfigs, } - err := newSealGenInfo.Validate(existingSealGenInfo) + err := newSealGenInfo.Validate(existingSealGenInfo, hasPartiallyWrappedPaths) if err != nil { return nil, err } @@ -2749,6 +2762,15 @@ func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal return newSealGenInfo, nil } +func hasPartiallyWrappedPaths(ctx context.Context, backend physical.Backend) (bool, error) { + paths, err := vault.GetPartiallySealWrappedPaths(ctx, backend) + if err != nil { + return false, err + } + + return len(paths) > 0, nil +} + func initHaBackend(c *ServerCommand, config *server.Config, coreConfig *vault.CoreConfig, backend physical.Backend) (bool, error) { // Initialize the separate HA storage backend, if it exists var ok bool diff --git a/command/server_sealgenerationinfo_test.go b/command/server_sealgenerationinfo_test.go index 15977d1c63..dc3e6fce49 100644 --- a/command/server_sealgenerationinfo_test.go +++ b/command/server_sealgenerationinfo_test.go @@ -23,12 +23,14 @@ func init() { func TestMultiSealCases(t *testing.T) { cases := []struct { - name string - existingSealGenInfo *seal.SealGenerationInfo - allSealKmsConfigs []*configutil.KMS - expectedSealGenInfo *seal.SealGenerationInfo - isErrorExpected bool - expectedErrorMsg string + name string + existingSealGenInfo *seal.SealGenerationInfo + allSealKmsConfigs []*configutil.KMS + expectedSealGenInfo *seal.SealGenerationInfo + isRewrapped bool + hasPartiallyWrappedPaths bool + isErrorExpected bool + expectedErrorMsg string }{ // none_to_shamir { @@ -123,6 +125,7 @@ func TestMultiSealCases(t *testing.T) { }, }, }, + isRewrapped: false, }, // shamir_to_multi { @@ -149,6 +152,7 @@ func TestMultiSealCases(t *testing.T) { Priority: 3, }, }, + isRewrapped: false, isErrorExpected: true, expectedErrorMsg: "cannot add more than one seal", }, @@ -172,8 +176,10 @@ func TestMultiSealCases(t *testing.T) { Priority: 1, }, }, - isErrorExpected: true, - expectedErrorMsg: "must have at least one seal in common with the old generation", + isRewrapped: true, + hasPartiallyWrappedPaths: false, + isErrorExpected: true, + expectedErrorMsg: "must have at least one seal in common with the old generation", }, // auto_to_shamir_with_common_seal { @@ -217,6 +223,8 @@ func TestMultiSealCases(t *testing.T) { }, }, }, + isRewrapped: true, + hasPartiallyWrappedPaths: false, }, // auto_to_auto_no_common_seal { @@ -238,8 +246,10 @@ func TestMultiSealCases(t *testing.T) { Priority: 1, }, }, - isErrorExpected: true, - expectedErrorMsg: "must have at least one seal in common with the old generation", + isRewrapped: true, + hasPartiallyWrappedPaths: false, + isErrorExpected: true, + expectedErrorMsg: "must have at least one seal in common with the old generation", }, // auto_to_auto_with_common_seal { @@ -283,6 +293,8 @@ func TestMultiSealCases(t *testing.T) { }, }, }, + isRewrapped: true, + hasPartiallyWrappedPaths: false, }, // auto_to_multi_add_one { @@ -324,6 +336,8 @@ func TestMultiSealCases(t *testing.T) { }, }, }, + isRewrapped: true, + hasPartiallyWrappedPaths: false, }, // auto_to_multi_add_two { @@ -355,8 +369,10 @@ func TestMultiSealCases(t *testing.T) { Priority: 3, }, }, - isErrorExpected: true, - expectedErrorMsg: "cannot add more than one seal", + isRewrapped: true, + hasPartiallyWrappedPaths: false, + isErrorExpected: true, + expectedErrorMsg: "cannot add more than one seal", }, // multi_to_auto_delete_one { @@ -393,6 +409,8 @@ func TestMultiSealCases(t *testing.T) { }, }, }, + isRewrapped: true, + hasPartiallyWrappedPaths: false, }, // multi_to_auto_delete_two { @@ -424,8 +442,10 @@ func TestMultiSealCases(t *testing.T) { Priority: 1, }, }, - isErrorExpected: true, - expectedErrorMsg: "cannot delete more than one seal", + isRewrapped: true, + hasPartiallyWrappedPaths: false, + isErrorExpected: true, + expectedErrorMsg: "cannot delete more than one seal", }, // disable_two_auto { @@ -491,6 +511,8 @@ func TestMultiSealCases(t *testing.T) { }, }, }, + isRewrapped: true, + hasPartiallyWrappedPaths: false, }, } @@ -498,7 +520,10 @@ func TestMultiSealCases(t *testing.T) { t.Run(tc.name, func(t *testing.T) { cmd := &ServerCommand{} cmd.logger = corehelpers.NewTestLogger(t) - sealGenInfo, err := cmd.computeSealGenerationInfo(tc.existingSealGenInfo, tc.allSealKmsConfigs) + if tc.existingSealGenInfo != nil { + tc.existingSealGenInfo.SetRewrapped(tc.isRewrapped) + } + sealGenInfo, err := cmd.computeSealGenerationInfo(tc.existingSealGenInfo, tc.allSealKmsConfigs, tc.hasPartiallyWrappedPaths) switch { case tc.isErrorExpected: require.Error(t, err) @@ -512,11 +537,13 @@ func TestMultiSealCases(t *testing.T) { } cases2 := []struct { - name string - existingSealGenInfo *seal.SealGenerationInfo - newSealGenInfo *seal.SealGenerationInfo - isErrorExpected bool - expectedErrorMsg string + name string + existingSealGenInfo *seal.SealGenerationInfo + newSealGenInfo *seal.SealGenerationInfo + isRewrapped bool + hasPartiallyWrappedPaths bool + isErrorExpected bool + expectedErrorMsg string }{ // same_generation_different_seals { @@ -551,8 +578,10 @@ func TestMultiSealCases(t *testing.T) { }, }, }, - isErrorExpected: true, - expectedErrorMsg: "existing seal generation is the same, but the configured seals are different", + isRewrapped: true, + hasPartiallyWrappedPaths: false, + isErrorExpected: true, + expectedErrorMsg: "existing seal generation is the same, but the configured seals are different", }, // same_generation_same_seals @@ -588,12 +617,115 @@ func TestMultiSealCases(t *testing.T) { }, }, }, - isErrorExpected: false, + isRewrapped: true, + hasPartiallyWrappedPaths: false, + isErrorExpected: false, + }, + // existing seal gen info rewrapped is set to false + { + name: "existing_sgi_rewrapped_false", + existingSealGenInfo: &seal.SealGenerationInfo{ + Generation: 2, + Seals: []*configutil.KMS{ + { + Type: "pkcs11", + Name: "autoSeal1", + Priority: 1, + }, + { + Type: "pkcs11", + Name: "autoSeal2", + Priority: 2, + }, + }, + }, + newSealGenInfo: &seal.SealGenerationInfo{ + Generation: 1, + Seals: []*configutil.KMS{ + { + Type: "pkcs11", + Name: "autoSeal1", + Priority: 1, + }, + }, + }, + isRewrapped: false, + hasPartiallyWrappedPaths: false, + isErrorExpected: true, + expectedErrorMsg: "cannot make seal config changes while seal re-wrap is in progress, please revert any seal configuration changes", + }, + // have partially wrapped paths + { + name: "have_partially_wrapped_paths", + existingSealGenInfo: &seal.SealGenerationInfo{ + Generation: 2, + Seals: []*configutil.KMS{ + { + Type: "pkcs11", + Name: "autoSeal1", + Priority: 1, + }, + { + Type: "pkcs11", + Name: "autoSeal2", + Priority: 2, + }, + }, + }, + newSealGenInfo: &seal.SealGenerationInfo{ + Generation: 1, + Seals: []*configutil.KMS{ + { + Type: "pkcs11", + Name: "autoSeal1", + Priority: 1, + }, + }, + }, + isRewrapped: true, + hasPartiallyWrappedPaths: true, + isErrorExpected: true, + expectedErrorMsg: "cannot make seal config changes while seal re-wrap is in progress, please revert any seal configuration changes", + }, + // no partially wrapped paths + { + name: "no_partially_wrapped_paths", + existingSealGenInfo: &seal.SealGenerationInfo{ + Generation: 2, + Seals: []*configutil.KMS{ + { + Type: "pkcs11", + Name: "autoSeal1", + Priority: 1, + }, + { + Type: "pkcs11", + Name: "autoSeal2", + Priority: 2, + }, + }, + }, + newSealGenInfo: &seal.SealGenerationInfo{ + Generation: 1, + Seals: []*configutil.KMS{ + { + Type: "pkcs11", + Name: "autoSeal1", + Priority: 1, + }, + }, + }, + isRewrapped: true, + hasPartiallyWrappedPaths: false, + isErrorExpected: false, }, } for _, tc := range cases2 { t.Run(tc.name, func(t *testing.T) { - err := tc.newSealGenInfo.Validate(tc.existingSealGenInfo) + if tc.existingSealGenInfo != nil { + tc.existingSealGenInfo.SetRewrapped(tc.isRewrapped) + } + err := tc.newSealGenInfo.Validate(tc.existingSealGenInfo, tc.hasPartiallyWrappedPaths) switch { case tc.isErrorExpected: require.Error(t, err) diff --git a/vault/seal/seal.go b/vault/seal/seal.go index ca08f6d42c..8fca0d4a33 100644 --- a/vault/seal/seal.go +++ b/vault/seal/seal.go @@ -55,7 +55,7 @@ type SealGenerationInfo struct { } // Validate is used to sanity check the seal generation info being created -func (sgi *SealGenerationInfo) Validate(existingSgi *SealGenerationInfo) error { +func (sgi *SealGenerationInfo) Validate(existingSgi *SealGenerationInfo, hasPartiallyWrappedPaths bool) error { existingSealsLen := 0 previousShamirConfigured := false if existingSgi != nil { @@ -73,6 +73,10 @@ func (sgi *SealGenerationInfo) Validate(existingSgi *SealGenerationInfo) error { break } } + + if !previousShamirConfigured && (!existingSgi.IsRewrapped() || hasPartiallyWrappedPaths) { + return errors.New("cannot make seal config changes while seal re-wrap is in progress, please revert any seal configuration changes") + } } numSealsToAdd := 0 diff --git a/vault/seal_stubs_oss.go b/vault/seal_stubs_oss.go index 10c20a8ee5..2c054d3651 100644 --- a/vault/seal_stubs_oss.go +++ b/vault/seal_stubs_oss.go @@ -2,6 +2,12 @@ package vault +import ( + "context" + + "github.com/hashicorp/vault/sdk/physical" +) + //go:generate go run github.com/hashicorp/vault/tools/stubmaker // isSealOldKeyError returns true if a value was decrypted using the @@ -13,3 +19,7 @@ func isSealOldKeyError(err error) bool { func startPartialSealRewrapping(c *Core) { // nothing to do } + +func GetPartiallySealWrappedPaths(ctx context.Context, backend physical.Backend) ([]string, error) { + return nil, nil +}