From c6da02962d6e43c4d4bed2a967f4317fedcd81cf Mon Sep 17 00:00:00 2001 From: Scott Miller Date: Fri, 22 Mar 2024 10:23:05 -0400 Subject: [PATCH] Add a configuration flag for enabling multiseal (Seal HA), CE side (#25908) * Add a configuration flag for enabling multiseal (Seal HA), CE side * imports * no quotes * get rid of dep on ent config * Abstract enableMultiSeal for a build time switch * license headers * wip * gate physical seal gen fetch by a param * docs tweak, remove core flag * updates from the ent pr * update stub * update test fixtures for enable_multiseal * use accessor * add a test fixture for non-multiseal diagnose * remove debugging crtuch * Do handle phys seal gen info even if multiseal is off, in order to facilitate enable/disable safeties * more enabled flag handling * Accept seal gen info if we were previously disabled, and persist it * update unit test * Validation happens postUnseal, so this test is invalid * Dont continue setting conf if seal loading fails during SIGHUP * Update website/content/docs/configuration/seal/seal-ha.mdx Thanks, that does sound much clearer Co-authored-by: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> * use validation if previous gen was enabled * unit test update * stub SetMultisealEnabled * bring over more changes from ent * this was an unfix --------- Co-authored-by: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> --- command/operator_diagnose.go | 14 +++- command/operator_diagnose_test.go | 8 +- command/server.go | 13 +-- command/server/config.go | 2 +- .../{config_stubs_oss.go => config_oss.go} | 4 +- command/server/config_util.go | 2 +- .../test-fixtures/config_diagnose_ok.hcl | 1 + .../config_diagnose_ok_singleseal.hcl | 47 +++++++++++ .../diagnose_seal_transit_tls_check.hcl | 1 + vault/core.go | 83 ++++++++++--------- vault/core_stubs_oss.go | 4 + vault/seal.go | 9 -- vault/seal/seal.go | 13 +++ .../docs/configuration/seal/seal-ha.mdx | 17 +++- 14 files changed, 153 insertions(+), 65 deletions(-) rename command/server/{config_stubs_oss.go => config_oss.go} (55%) create mode 100644 command/server/test-fixtures/config_diagnose_ok_singleseal.hcl diff --git a/command/operator_diagnose.go b/command/operator_diagnose.go index 460a24f101..46930451d3 100644 --- a/command/operator_diagnose.go +++ b/command/operator_diagnose.go @@ -14,6 +14,8 @@ import ( "sync" "time" + "github.com/hashicorp/vault/vault/seal" + "github.com/hashicorp/cli" "github.com/hashicorp/consul/api" log "github.com/hashicorp/go-hclog" @@ -432,10 +434,14 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error sealcontext, sealspan := diagnose.StartSpan(ctx, "Create Vault Server Configuration Seals") var setSealResponse *SetSealResponse - existingSealGenerationInfo, err := vault.PhysicalSealGenInfo(sealcontext, *backend) - if err != nil { - diagnose.Fail(sealcontext, fmt.Sprintf("Unable to get Seal generation information from storage: %s.", err.Error())) - goto SEALFAIL + var err error + var existingSealGenerationInfo *seal.SealGenerationInfo + if config.IsMultisealEnabled() { + existingSealGenerationInfo, err = vault.PhysicalSealGenInfo(sealcontext, *backend) + if err != nil { + diagnose.Fail(sealcontext, fmt.Sprintf("Unable to get Seal generation information from storage: %s.", err.Error())) + goto SEALFAIL + } } setSealResponse, err = setSeal(server, config, make([]string, 0), make(map[string]string), existingSealGenerationInfo, false /* unsealed vault has no partially wrapped paths */) diff --git a/command/operator_diagnose_test.go b/command/operator_diagnose_test.go index 9f7fbde762..8528637dc2 100644 --- a/command/operator_diagnose_test.go +++ b/command/operator_diagnose_test.go @@ -14,7 +14,7 @@ import ( "testing" "github.com/hashicorp/cli" - "github.com/hashicorp/vault/command/server" + "github.com/hashicorp/vault/helper/constants" "github.com/hashicorp/vault/vault/diagnose" ) @@ -41,7 +41,7 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) { { "diagnose_ok", []string{ - "-config", "./server/test-fixtures/config_diagnose_ok.hcl", + "-config", "./server/test-fixtures/config_diagnose_ok_singleseal.hcl", }, []*diagnose.Result{ { @@ -611,9 +611,9 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) { for _, tc := range cases { tc := tc t.Run(tc.name, func(t *testing.T) { - if tc.name == "diagnose_ok" && server.IsMultisealSupported() { + if tc.name == "diagnose_ok" && constants.IsEnterprise { t.Skip("Test not valid in ENT") - } else if tc.name == "diagnose_ok_multiseal" && !server.IsMultisealSupported() { + } else if tc.name == "diagnose_ok_multiseal" && !constants.IsEnterprise { t.Skip("Test not valid in community edition") } else { t.Parallel() diff --git a/command/server.go b/command/server.go index dc558c5c34..77942ede7f 100644 --- a/command/server.go +++ b/command/server.go @@ -1657,6 +1657,7 @@ func (c *ServerCommand) Run(args []string) int { if err != nil { c.UI.Error(fmt.Errorf("error reloading seal config: %s", err).Error()) config.Seals = core.GetCoreConfigInternal().Seals + goto RUNRELOADFUNCS } else { // finalize the old seals and set the new seals as the current ones c.finalizeSeals(ctx, ¤tSeals) @@ -2671,7 +2672,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma wrapper = aeadwrapper.NewShamirWrapper() } configuredSeals++ - } else if server.IsMultisealSupported() { + } else if config.IsMultisealEnabled() { recordSealConfigWarning(fmt.Errorf("error configuring seal: %v", wrapperConfigError)) } else { // It seems that we are checking for this particular error here is to distinguish between a @@ -2739,7 +2740,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Compute seal generation - sealGenerationInfo, err := c.computeSealGenerationInfo(existingSealGenerationInfo, allSealKmsConfigs, hasPartiallyWrappedPaths) + sealGenerationInfo, err := c.computeSealGenerationInfo(existingSealGenerationInfo, allSealKmsConfigs, hasPartiallyWrappedPaths, config.IsMultisealEnabled()) if err != nil { return nil, err } @@ -2797,7 +2798,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma } unwrapSeal = vault.NewDefaultSeal(a) - case server.IsMultisealSupported(): + case config.IsMultisealEnabled(): // We know we are not using Shamir seal, that we are not migrating away from one, and multi seal is supported, // so just put enabled and disabled wrappers on the same seal Access allSealWrappers := append(enabledSealWrappers, disabledSealWrappers...) @@ -2838,7 +2839,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma }, nil } -func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal.SealGenerationInfo, sealConfigs []*configutil.KMS, hasPartiallyWrappedPaths bool) (*vaultseal.SealGenerationInfo, error) { +func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal.SealGenerationInfo, sealConfigs []*configutil.KMS, hasPartiallyWrappedPaths, multisealEnabled bool) (*vaultseal.SealGenerationInfo, error) { generation := uint64(1) if existingSealGenInfo != nil { @@ -2858,9 +2859,10 @@ func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal newSealGenInfo := &vaultseal.SealGenerationInfo{ Generation: generation, Seals: sealConfigs, + Enabled: multisealEnabled, } - if server.IsMultisealSupported() { + if multisealEnabled || (existingSealGenInfo != nil && existingSealGenInfo.Enabled) { err := newSealGenInfo.Validate(existingSealGenInfo, hasPartiallyWrappedPaths) if err != nil { return nil, err @@ -3353,6 +3355,7 @@ func (c *ServerCommand) reloadSeals(ctx context.Context, core *vault.Core, confi infoKeysReload := make([]string, 0) infoReload := make(map[string]string) + core.SetMultisealEnabled(config.IsMultisealEnabled()) setSealResponse, secureRandomReader, err := c.configureSeals(ctx, config, core.PhysicalAccess(), infoKeysReload, infoReload) if err != nil { return nil, err diff --git a/command/server/config.go b/command/server/config.go index 938683d6a7..9d31ed67b3 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -748,7 +748,7 @@ func ParseConfig(d, source string) (*Config, error) { return nil, fmt.Errorf("error validating experiment(s) from config: %w", err) } - if err := result.parseConfig(list); err != nil { + if err := result.parseConfig(list, source); err != nil { return nil, fmt.Errorf("error parsing enterprise config: %w", err) } diff --git a/command/server/config_stubs_oss.go b/command/server/config_oss.go similarity index 55% rename from command/server/config_stubs_oss.go rename to command/server/config_oss.go index 5c2e7e7bb3..22abae3003 100644 --- a/command/server/config_stubs_oss.go +++ b/command/server/config_oss.go @@ -5,8 +5,6 @@ package server -//go:generate go run github.com/hashicorp/vault/tools/stubmaker - -func IsMultisealSupported() bool { +func (c *Config) IsMultisealEnabled() bool { return false } diff --git a/command/server/config_util.go b/command/server/config_util.go index 1114be8ebb..9447ded652 100644 --- a/command/server/config_util.go +++ b/command/server/config_util.go @@ -14,7 +14,7 @@ import ( type entConfig struct{} -func (ec *entConfig) parseConfig(list *ast.ObjectList) error { +func (ec *entConfig) parseConfig(list *ast.ObjectList, source string) error { return nil } diff --git a/command/server/test-fixtures/config_diagnose_ok.hcl b/command/server/test-fixtures/config_diagnose_ok.hcl index 761d87e7b2..5e19867628 100644 --- a/command/server/test-fixtures/config_diagnose_ok.hcl +++ b/command/server/test-fixtures/config_diagnose_ok.hcl @@ -45,3 +45,4 @@ pid_file = "./pidfile" raw_storage_endpoint = true disable_sealwrap = true disable_printable_check = true +enable_multiseal = true \ No newline at end of file diff --git a/command/server/test-fixtures/config_diagnose_ok_singleseal.hcl b/command/server/test-fixtures/config_diagnose_ok_singleseal.hcl new file mode 100644 index 0000000000..761d87e7b2 --- /dev/null +++ b/command/server/test-fixtures/config_diagnose_ok_singleseal.hcl @@ -0,0 +1,47 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 + +disable_cache = true +disable_mlock = true + +ui = true + +listener "tcp" { + address = "127.0.0.1:1024" + tls_disable = true +} + +backend "consul" { + address = "127.0.0.1:1025" +} + +ha_backend "consul" { + address = "127.0.0.1:8500" + bar = "baz" + advertise_addr = "https://127.0.0.1:8500" + disable_clustering = "true" +} + +service_registration "consul" { + address = "127.0.0.1:8500" + foo = "bar" +} + +telemetry { + statsd_address = "bar" + usage_gauge_period = "5m" + maximum_gauge_cardinality = 100 + + statsite_address = "foo" + dogstatsd_addr = "127.0.0.1:7254" + dogstatsd_tags = ["tag_1:val_1", "tag_2:val_2"] + metrics_prefix = "myprefix" +} + +max_lease_ttl = "10h" +default_lease_ttl = "10h" +cluster_name = "testcluster" +pid_file = "./pidfile" +raw_storage_endpoint = true +disable_sealwrap = true +disable_printable_check = true diff --git a/command/server/test-fixtures/diagnose_seal_transit_tls_check.hcl b/command/server/test-fixtures/diagnose_seal_transit_tls_check.hcl index 17f3274fb5..a7007d5731 100644 --- a/command/server/test-fixtures/diagnose_seal_transit_tls_check.hcl +++ b/command/server/test-fixtures/diagnose_seal_transit_tls_check.hcl @@ -57,3 +57,4 @@ pid_file = "./pidfile" raw_storage_endpoint = true disable_sealwrap = true disable_printable_check = true +enable_multiseal = true \ No newline at end of file diff --git a/vault/core.go b/vault/core.go index 25cd1e1b0f..5fd3c799a6 100644 --- a/vault/core.go +++ b/vault/core.go @@ -2638,48 +2638,57 @@ func (c *Core) runUnsealSetupForPrimary(ctx context.Context, logger log.Logger) return err } - // Retrieve the seal generation information from storage - existingGenerationInfo, err := PhysicalSealGenInfo(ctx, c.physical) - if err != nil { - logger.Error("cannot read existing seal generation info from storage", "error", err) - return err - } - - sealGenerationInfo := c.seal.GetAccess().GetSealGenerationInfo() - - switch { - case existingGenerationInfo == nil: - // This is the first time we store seal generation information - fallthrough - case existingGenerationInfo.Generation < sealGenerationInfo.Generation: - // We have incremented the seal generation - if err := c.SetPhysicalSealGenInfo(ctx, sealGenerationInfo); err != nil { - logger.Error("failed to store seal generation info", "error", err) + if c.IsMultisealEnabled() { + // Retrieve the seal generation information from storage + existingGenerationInfo, err := PhysicalSealGenInfo(ctx, c.physical) + if err != nil { + logger.Error("cannot read existing seal generation info from storage", "error", err) return err } - case existingGenerationInfo.Generation == sealGenerationInfo.Generation: - // Same generation, update the rewrapped flag in case the previous active node - // changed its value. In other words, a rewrap may have happened, or a rewrap may have been - // started but not completed. - c.seal.GetAccess().GetSealGenerationInfo().SetRewrapped(existingGenerationInfo.IsRewrapped()) + sealGenerationInfo := c.seal.GetAccess().GetSealGenerationInfo() - case existingGenerationInfo.Generation > sealGenerationInfo.Generation: - // Our seal information is out of date. The previous active node used a newer generation. - logger.Error("A newer seal generation was found in storage. The seal configuration in this node should be updated to match that of the previous active node, and this node should be restarted.") - return errors.New("newer seal generation found in storage, in memory seal configuration is out of date") + switch { + case existingGenerationInfo == nil: + // This is the first time we store seal generation information + fallthrough + case existingGenerationInfo.Generation < sealGenerationInfo.Generation || !existingGenerationInfo.Enabled: + // We have incremented the seal generation or we've just become enabled again after previously being disabled, + // trust the operator in the latter case + if err := c.SetPhysicalSealGenInfo(ctx, sealGenerationInfo); err != nil { + logger.Error("failed to store seal generation info", "error", err) + return err + } + + case existingGenerationInfo.Generation == sealGenerationInfo.Generation: + // Same generation, update the rewrapped flag in case the previous active node + // changed its value. In other words, a rewrap may have happened, or a rewrap may have been + // started but not completed. + c.seal.GetAccess().GetSealGenerationInfo().SetRewrapped(existingGenerationInfo.IsRewrapped()) + if !existingGenerationInfo.Enabled { + // Weren't enabled but are now, persist the flag + if err := c.SetPhysicalSealGenInfo(ctx, sealGenerationInfo); err != nil { + logger.Error("failed to store seal generation info", "error", err) + return err + } + } + case existingGenerationInfo.Generation > sealGenerationInfo.Generation: + // Our seal information is out of date. The previous active node used a newer generation. + logger.Error("A newer seal generation was found in storage. The seal configuration in this node should be updated to match that of the previous active node, and this node should be restarted.") + return errors.New("newer seal generation found in storage, in memory seal configuration is out of date") + } + + if !sealGenerationInfo.IsRewrapped() { + + // Set the migration done flag so that a seal-rewrap gets triggered later. + // Note that in the case where multi seal is not supported, Core.migrateSeal() takes care of + // triggering the rewrap when necessary. + logger.Trace("seal generation information indicates that a seal-rewrap is needed", "generation", sealGenerationInfo.Generation) + atomic.StoreUint32(c.sealMigrationDone, 1) + } + startPartialSealRewrapping(c) } - if server.IsMultisealSupported() && !sealGenerationInfo.IsRewrapped() { - // Set the migration done flag so that a seal-rewrap gets triggered later. - // Note that in the case where multi seal is not supported, Core.migrateSeal() takes care of - // triggering the rewrap when necessary. - logger.Trace("seal generation information indicates that a seal-rewrap is needed", "generation", sealGenerationInfo.Generation) - atomic.StoreUint32(c.sealMigrationDone, 1) - } - - startPartialSealRewrapping(c) - return nil } @@ -3054,7 +3063,7 @@ func (c *Core) checkForSealMigration(ctx context.Context, unwrapSeal Seal) (seal // This is a migration away from Shamir. return sealMigrationCheckAdjust, nil - case configuredType == SealConfigTypeMultiseal && server.IsMultisealSupported(): + case configuredType == SealConfigTypeMultiseal && c.IsMultisealEnabled(): // We are going from a single non-shamir seal to multiseal, and multi seal is supported. // This scenario is not considered a migration in the sense of requiring an unwrapSeal, // but we will update the stored SealConfig later (see Core.migrateMultiSealConfig). diff --git a/vault/core_stubs_oss.go b/vault/core_stubs_oss.go index 9fba0c19f1..c1178da796 100644 --- a/vault/core_stubs_oss.go +++ b/vault/core_stubs_oss.go @@ -100,3 +100,7 @@ func (c *Core) EntWaitUntilWALShipped(ctx context.Context, index uint64) bool { } func (c *Core) SecretsSyncLicensedActivated() bool { return false } + +func (c *Core) IsMultisealEnabled() bool { return false } + +func (c *Core) SetMultisealEnabled(_ bool) {} diff --git a/vault/seal.go b/vault/seal.go index 0bc9cebf53..2eb9904e16 100644 --- a/vault/seal.go +++ b/vault/seal.go @@ -10,7 +10,6 @@ import ( "fmt" "sync/atomic" - "github.com/hashicorp/vault/command/server" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/vault/seal" @@ -340,10 +339,6 @@ func readStoredKeys(ctx context.Context, storage physical.Backend, encryptor sea } func (c *Core) SetPhysicalSealGenInfo(ctx context.Context, sealGenInfo *seal.SealGenerationInfo) error { - if !server.IsMultisealSupported() { - return nil - } - if sealGenInfo == nil { return errors.New("invalid seal generation information: generation is unknown") } @@ -368,10 +363,6 @@ func (c *Core) SetPhysicalSealGenInfo(ctx context.Context, sealGenInfo *seal.Sea } func PhysicalSealGenInfo(ctx context.Context, storage physical.Backend) (*seal.SealGenerationInfo, error) { - if !server.IsMultisealSupported() { - return nil, nil - } - pe, err := storage.Get(ctx, SealGenInfoPath) if err != nil { return nil, fmt.Errorf("failed to fetch seal generation info: %w", err) diff --git a/vault/seal/seal.go b/vault/seal/seal.go index 78c98a892a..9fb67b017e 100644 --- a/vault/seal/seal.go +++ b/vault/seal/seal.go @@ -59,6 +59,7 @@ type SealGenerationInfo struct { Generation uint64 Seals []*configutil.KMS rewrapped atomic.Bool + Enabled bool } // Validate is used to sanity check the seal generation info being created @@ -81,6 +82,15 @@ func (sgi *SealGenerationInfo) Validate(existingSgi *SealGenerationInfo, hasPart return nil } + // Validate that we're in a safe spot with respect to disabling multiseal + if existingSgi.Enabled && !sgi.Enabled { + if len(existingSgi.Seals) > 1 { + return fmt.Errorf("multi-seal is disabled but previous configuration had multiple seals. re-enable and migrate to a single seal before disabling multi-seal") + } else if !existingSgi.IsRewrapped() { + return fmt.Errorf("multi-seal is disabled but previous storage was not fully re-wrapped, re-enable multi-seal and allow rewrapping to complete before disabling multi-seal") + } + } + existingSealNameAndType := sealNameAndTypeAsStr(existingSgi.Seals) previousShamirConfigured := false @@ -234,6 +244,7 @@ type sealGenerationInfoJson struct { Generation uint64 Seals []*configutil.KMS Rewrapped bool + Enabled bool } func (sgi *SealGenerationInfo) MarshalJSON() ([]byte, error) { @@ -241,6 +252,7 @@ func (sgi *SealGenerationInfo) MarshalJSON() ([]byte, error) { Generation: sgi.Generation, Seals: sgi.Seals, Rewrapped: sgi.IsRewrapped(), + Enabled: sgi.Enabled, }) } @@ -253,6 +265,7 @@ func (sgi *SealGenerationInfo) UnmarshalJSON(b []byte) error { sgi.Generation = value.Generation sgi.Seals = value.Seals sgi.SetRewrapped(value.Rewrapped) + sgi.Enabled = value.Enabled return nil } diff --git a/website/content/docs/configuration/seal/seal-ha.mdx b/website/content/docs/configuration/seal/seal-ha.mdx index 5cb3f71c13..b34e4e3e96 100644 --- a/website/content/docs/configuration/seal/seal-ha.mdx +++ b/website/content/docs/configuration/seal/seal-ha.mdx @@ -18,7 +18,12 @@ Using Seal HA involves configuring extra seals in Vault's server configuration f and restarting Vault or triggering a reload of it's configuration via sending it the SIGHUP signal. -Before using Seal HA, one must have upgraded to Vault 1.16 or higher. +Before using Seal HA, one must have upgraded to Vault 1.16 or higher. Seal HA is enabled +by adding the following line to Vault's configuration: + +``` +enable_multiseal = true +``` ## Adding and Removing Seals @@ -137,3 +142,13 @@ instructions to add a second auto seal. Correspondingly, to migrate back to a shamir seal, first use the above instructions to move to a single auto seal, and use the traditional migration method to migrate back to a Shamir seal. + +### Removing Seal HA + +Migrating back to a single seal may result in data loss if an operator does not +use the HA seal feature. To migrate back to a single seal: + +1. Perform a [seal migration](/vault/docs/concepts/seal#seal-migration) as described. +2. Monitor [`sys/sealwrap/rewrap`](/vault/docs/api-docs/system/sealwrap-rewrap) until the API returns `fully_wrapped=true`. +3. Remove `enable_multiseal` from all Vault configuration files in the cluster. +4. Restart Vault. \ No newline at end of file