Reload seals if necessary when a node gains leadership. (#26098)

As part of the process of becoming a leader node, check to see if the seal
configuration needs to be reloaded. Reloading may be necessary if the seal
generation information computed during start up is outdated. For example, a new
node that has just joined the cluster will have incorrect seal generation
information in memory, even if it has the correct seal configuration, since it
did not have access to the stored seal generation information.

command/server.go

@@ -148,6 +148,8 @@ type ServerCommand struct {
 	flagTestServerConfig   bool
 	flagDevConsul          bool
 	flagExitOnCoreShutdown bool
+
+	sealsToFinalize []*vault.Seal
 }
 
 func (c *ServerCommand) Synopsis() string {
@@ -1254,8 +1256,10 @@ func (c *ServerCommand) Run(args []string) int {
 		return 1
 	}
 
-	currentSeals := setSealResponse.getCreatedSeals()
-	defer c.finalizeSeals(ctx, &currentSeals)
+	c.setSealsToFinalize(setSealResponse.getCreatedSeals())
+	defer func() {
+		c.finalizeSeals(ctx, c.sealsToFinalize)
+	}()
 
 	coreConfig := createCoreConfig(c, config, backend, configSR, setSealResponse.barrierSeal, setSealResponse.unwrapSeal, metricsHelper, metricSink, secureRandomReader)
 	if c.flagDevThreeNode {
@@ -1541,6 +1545,22 @@ func (c *ServerCommand) Run(args []string) int {
 		}
 	}
 
+	core.SetSealReloadFunc(func(ctx context.Context) error {
+		// This function performs the same seal reloading functionality as in the SIGHUP handler below.
+		config, _, err := c.reloadConfigFiles()
+		if err != nil {
+			return err
+		}
+		if config == nil {
+			return errors.New("no config found at reload time")
+		}
+		reloaded, err := c.reloadSeals(ctx, false, core, config)
+		if reloaded {
+			core.SetConfig(config)
+		}
+		return err
+	})
+
 	// Output the header that the server has started
 	if !c.logFlags.flagCombineLogs {
 		c.UI.Output("==> Vault server started! Log data will stream in below:\n")
@@ -1623,24 +1643,12 @@ func (c *ServerCommand) Run(args []string) int {
 			c.notifySystemd(systemd.SdNotifyReloading)
 
 			// Check for new log level
-			var config *server.Config
-			var configErrors []configutil.ConfigError
-			for _, path := range c.flagConfigs {
-				current, err := server.LoadConfig(path)
+			config, configErrors, err := c.reloadConfigFiles()
 			if err != nil {
-					c.logger.Error("could not reload config", "path", path, "error", err)
+				c.logger.Error("could not reload config", "error", err)
 				goto RUNRELOADFUNCS
 			}
 
-				configErrors = append(configErrors, current.Validate(path)...)
-
-				if config == nil {
-					config = current
-				} else {
-					config = config.Merge(current)
-				}
-			}
-
 			// Ensure at least one config was found.
 			if config == nil {
 				c.logger.Error("no config found at reload time")
@@ -1652,17 +1660,14 @@ func (c *ServerCommand) Run(args []string) int {
 				c.logger.Warn(cErr.String())
 			}
 
-			if !cmp.Equal(core.GetCoreConfigInternal().Seals, config.Seals) {
-				setSealResponse, err = c.reloadSeals(ctx, core, config)
-				if err != nil {
+			// Note that seal reloading can also be triggered via Core.TriggerSealReload.
+			// See the call to Core.SetSealReloadFunc above.
+			if reloaded, err := c.reloadSealsLocking(ctx, core, config); 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, &currentSeals)
-					currentSeals = setSealResponse.getCreatedSeals()
-				}
+			} else if !reloaded {
+				config.Seals = core.GetCoreConfigInternal().Seals
 			}
 
 			core.SetConfig(config)
@@ -1824,6 +1829,27 @@ func (c *ServerCommand) Run(args []string) int {
 	return retCode
 }
 
+func (c *ServerCommand) reloadConfigFiles() (*server.Config, []configutil.ConfigError, error) {
+	var config *server.Config
+	var configErrors []configutil.ConfigError
+	for _, path := range c.flagConfigs {
+		current, err := server.LoadConfig(path)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		configErrors = append(configErrors, current.Validate(path)...)
+
+		if config == nil {
+			config = current
+		} else {
+			config = config.Merge(current)
+		}
+	}
+
+	return config, configErrors, nil
+}
+
 func (c *ServerCommand) configureSeals(ctx context.Context, config *server.Config, backend physical.Backend, infoKeys []string, info map[string]string) (*SetSealResponse, io.Reader, error) {
 	existingSealGenerationInfo, err := vault.PhysicalSealGenInfo(ctx, backend)
 	if err != nil {
@@ -1865,8 +1891,15 @@ func (c *ServerCommand) configureSeals(ctx context.Context, config *server.Confi
 	return setSealResponse, secureRandomReader, nil
 }
 
-func (c *ServerCommand) finalizeSeals(ctx context.Context, seals *[]*vault.Seal) {
-	for _, seal := range *seals {
+func (c *ServerCommand) setSealsToFinalize(seals []*vault.Seal) {
+	prev := c.sealsToFinalize
+	c.sealsToFinalize = seals
+
+	c.finalizeSeals(context.Background(), prev)
+}
+
+func (c *ServerCommand) finalizeSeals(ctx context.Context, seals []*vault.Seal) {
+	for _, seal := range seals {
 		// Ensure that the seal finalizer is called, even if using verify-only
 		err := (*seal).Finalize(ctx)
 		if err != nil {
@@ -3343,39 +3376,98 @@ func startHttpServers(c *ServerCommand, core *vault.Core, config *server.Config,
 	return nil
 }
 
-func (c *ServerCommand) reloadSeals(ctx context.Context, core *vault.Core, config *server.Config) (*SetSealResponse, error) {
-	if len(config.Seals) == 1 && config.Seals[0].Disabled {
-		return nil, errors.New("moving from autoseal to shamir requires seal migration")
+func (c *ServerCommand) reloadSealsLocking(ctx context.Context, core *vault.Core, config *server.Config) (bool, error) {
+	return c.reloadSeals(ctx, true, core, config)
 }
 
+// reloadSeals reloads configuration files and determines whether it needs to re-create the Seal.Access() objects.
+// This function needs do detect that core.SealAccess() is no longer using the seal Wrapper that is specified
+// in the seal configuration files.
+// This function returns true if the newConfig was used to re-create the Seal.Access() objects. In other words,
+// if false is returned, there were no changes done to the seals.
+func (c *ServerCommand) reloadSeals(ctx context.Context, grabStateLock bool, core *vault.Core, newConfig *server.Config) (ret bool, err error) {
+	defer func() {
+		if err != nil {
+			// We do not log here, as the error will be logged higher in the call chain
+			return
+		}
+		if ret {
+			c.logger.Info("seal configuration reloaded successfully")
+		} else {
+			c.logger.Info("seal configuration was not reloaded")
+		}
+	}()
+
+	if core.IsInSealMigrationMode(grabStateLock) {
+		return false, nil
+	}
+
+	currentConfig := core.GetCoreConfigInternal()
+
 	if core.SealAccess().BarrierSealConfigType() == vault.SealConfigTypeShamir {
-		return nil, errors.New("moving from shamir to autoseal requires seal migration")
+		switch {
+		case len(newConfig.Seals) == 0:
+			// We are fine, our ServerCommand.reloadConfigFiles() does not do the "automagic" creation
+			// of the Shamir seal configuration.
+			return false, nil
+
+		case len(newConfig.Seals) == 1 && newConfig.Seals[0].Disabled:
+			// If we have only one seal and it is disabled, it means that the newConfig wants to migrate
+			// to Shamir, which is not supported by seal reloading.
+			return false, nil
+
+		case len(newConfig.Seals) == 1 && newConfig.Seals[0].Type == vault.SealConfigTypeShamir.String():
+			// Having a single Shamir seal in newConfig is not really possible, since a Shamir seal
+			// is specified in configuration by *not* having a seal stanza. If we were to hit this
+			// case, though, it is equivalent to trying to migrate to Shamir, which is not supported
+			// by seal reloading.
+			return false, nil
+		}
+	}
+
+	if cmp.Equal(currentConfig.Seals, newConfig.Seals) {
+		return false, nil
+	}
+
+	// Verify that the new config we picked up is not trying to migrate from autoseal to shamir
+	if len(newConfig.Seals) == 1 && newConfig.Seals[0].Disabled {
+		// If we get here, it means the node was not started in migration mode, but the new config says
+		// we should go into migration mode.
+		return false, errors.New("moving from autoseal to shamir requires seal migration")
+	}
+
+	// Verify that the new config we picked up is not trying to migrate shamir to autoseal
+	if core.SealAccess().BarrierSealConfigType() == vault.SealConfigTypeShamir {
+		return false, errors.New("moving from shamir to autoseal requires seal migration")
 	}
 
 	infoKeysReload := make([]string, 0)
 	infoReload := make(map[string]string)
 
-	core.SetMultisealEnabled(config.IsMultisealEnabled())
-	setSealResponse, secureRandomReader, err := c.configureSeals(ctx, config, core.PhysicalAccess(), infoKeysReload, infoReload)
+	core.SetMultisealEnabled(newConfig.IsMultisealEnabled())
+	setSealResponse, secureRandomReader, err := c.configureSeals(ctx, newConfig, core.PhysicalAccess(), infoKeysReload, infoReload)
 	if err != nil {
-		return nil, err
+		return false, err
 	}
 	if setSealResponse.sealConfigError != nil {
-		return nil, err
+		return false, err
 	}
 
 	newGen := setSealResponse.barrierSeal.GetAccess().GetSealGenerationInfo()
 
-	err = core.SetSeals(setSealResponse.barrierSeal, secureRandomReader, !newGen.IsRewrapped() || setSealResponse.hasPartiallyWrappedPaths)
+	err = core.SetSeals(ctx, grabStateLock, setSealResponse.barrierSeal, secureRandomReader, !newGen.IsRewrapped() || setSealResponse.hasPartiallyWrappedPaths)
 	if err != nil {
-		return nil, fmt.Errorf("error setting seal: %s", err)
+		return false, fmt.Errorf("error setting seal: %s", err)
 	}
 
 	if err := core.SetPhysicalSealGenInfo(ctx, newGen); err != nil {
 		c.logger.Warn("could not update seal information in storage", "err", err)
 	}
 
-	return setSealResponse, nil
+	// finalize the old seals and set the new seals as the current ones
+	c.setSealsToFinalize(setSealResponse.getCreatedSeals())
+
+	return true, nil
 }
 
 func SetStorageMigration(b physical.Backend, active bool) error {

command/server_test.go

@@ -412,14 +412,14 @@ func TestReloadSeals(t *testing.T) {
 	_, testCommand := testServerCommand(t)
 	testConfig := server.Config{SharedConfig: &configutil.SharedConfig{}}
 
-	_, err := testCommand.reloadSeals(context.Background(), testCore, &testConfig)
-	if err == nil {
-		t.Fatal("expected error, got nil")
-	}
+	testCommand.logger = corehelpers.NewTestLogger(t)
+	ctx := context.Background()
+	reloaded, err := testCommand.reloadSealsLocking(ctx, testCore, &testConfig)
+	require.NoError(t, err)
+	require.False(t, reloaded, "reloadSeals does not support Shamir seals")
 
 	testConfig = server.Config{SharedConfig: &configutil.SharedConfig{Seals: []*configutil.KMS{{Disabled: true}}}}
-	_, err = testCommand.reloadSeals(context.Background(), testCore, &testConfig)
-	if err == nil {
-		t.Fatal("expected error, got nil")
-	}
+	reloaded, err = testCommand.reloadSealsLocking(ctx, testCore, &testConfig)
+	require.NoError(t, err)
+	require.False(t, reloaded, "reloadSeals does not support Shamir seals")
 }

vault/core.go

@@ -282,6 +282,9 @@ type Core struct {
 	// seal is our seal, for seal configuration information
 	seal Seal
 
+	// sealReloadFunc is a function that can be used to trigger seal configuration reloading
+	sealReloadFunc func(context.Context) error
+
 	// raftJoinDoneCh is used by the raft retry join routine to inform unseal process
 	// that the join is complete
 	raftJoinDoneCh chan struct{}
@@ -3401,6 +3404,20 @@ func (c *Core) IsSealMigrated(lock bool) bool {
 	return done
 }
 
+func (c *Core) SetSealReloadFunc(f func(context.Context) error) {
+	c.sealReloadFunc = f
+}
+
+// TriggerSealReload triggers reloading of the seal configuration and resetting of the seal.
+// The caller should hold the write statelock.
+func (c *Core) TriggerSealReload(ctx context.Context) error {
+	if c.sealReloadFunc == nil {
+		return nil
+	}
+
+	return c.sealReloadFunc(ctx)
+}
+
 func (c *Core) BarrierEncryptorAccess() *BarrierEncryptorAccess {
 	return NewBarrierEncryptorAccess(c.barrier)
 }
@@ -4449,11 +4466,13 @@ func (c *Core) Events() *eventbus.EventBus {
 	return c.events
 }
 
-func (c *Core) SetSeals(barrierSeal Seal, secureRandomReader io.Reader, shouldRewrap bool) error {
-	ctx, _ := c.GetContext()
+func (c *Core) SetSeals(ctx context.Context, grabLock bool, barrierSeal Seal, secureRandomReader io.Reader, shouldRewrap bool) error {
+	if grabLock {
+		ctx, _ = c.GetContext()
 
 		c.stateLock.Lock()
 		defer c.stateLock.Unlock()
+	}
 
 	currentSealBarrierConfig, err := c.SealAccess().BarrierConfig(ctx)
 	if err != nil {

vault/core_test.go

@@ -3460,7 +3460,8 @@ func TestSetSeals(t *testing.T) {
 		Generation:   2,
 	})
 
-	err := testCore.SetSeals(newSeal, nil, false)
+	ctx := context.Background()
+	err := testCore.SetSeals(ctx, true, newSeal, nil, false)
 	if err != nil {
 		t.Fatal(err)
 	}

vault/ha.go

@@ -580,6 +580,20 @@ func (c *Core) waitForLeadership(newLeaderCh chan func(), manualStepDownCh, stop
 		c.activeContext = activeCtx
 		c.activeContextCancelFunc.Store(activeCtxCancel)
 
+		// Trigger a seal reload if necessary. A seal reload is necessary when a node
+		// becomes the leader since its seal generation information may be out of
+		// date (as is the case, for example, when a new node joins the cluster).
+		if err := c.TriggerSealReload(c.activeContext); err != nil {
+			c.logger.Error("seal configuration reload error", "error", err)
+			c.barrier.Seal()
+			c.logger.Warn("vault is sealed")
+			c.heldHALock = nil
+			lock.Unlock()
+			close(continueCh)
+			c.stateLock.Unlock()
+			return
+		}
+
 		// Perform seal migration
 		if err := c.migrateSeal(c.activeContext); err != nil {
 			c.logger.Error("seal migration error", "error", err)