diff --git a/command/server.go b/command/server.go index 0aab187f0411..b29b208e8405 100644 --- a/command/server.go +++ b/command/server.go @@ -837,14 +837,7 @@ CLUSTER_SYNTHESIS_COMPLETE: return false } - sealedFunc := func() bool { - if sealed, err := core.Sealed(); err == nil { - return sealed - } - return true - } - - if err := sd.RunServiceDiscovery(c.WaitGroup, c.ShutdownCh, coreConfig.RedirectAddr, activeFunc, sealedFunc); err != nil { + if err := sd.RunServiceDiscovery(c.WaitGroup, c.ShutdownCh, coreConfig.RedirectAddr, activeFunc, core.Sealed); err != nil { c.UI.Error(fmt.Sprintf("Error initializing service discovery: %v", err)) return 1 } diff --git a/http/sys_health.go b/http/sys_health.go index be67f2fa295f..6bde8b068f0f 100644 --- a/http/sys_health.go +++ b/http/sys_health.go @@ -114,7 +114,7 @@ func getSysHealth(core *vault.Core, r *http.Request) (int, *HealthResponse, erro ctx := context.Background() // Check system status - sealed, _ := core.Sealed() + sealed := core.Sealed() standby, _ := core.Standby() var replicationState consts.ReplicationState if standby { diff --git a/http/sys_init_test.go b/http/sys_init_test.go index 205f54fd1369..f4be3413a902 100644 --- a/http/sys_init_test.go +++ b/http/sys_init_test.go @@ -119,11 +119,7 @@ func TestSysInit_put(t *testing.T) { } } - seal, err := core.Sealed() - if err != nil { - t.Fatalf("err: %s", err) - } - if seal { + if core.Sealed() { t.Fatal("should not be sealed") } } diff --git a/http/sys_seal.go b/http/sys_seal.go index d86d7f2d0619..2eebb95cd19c 100644 --- a/http/sys_seal.go +++ b/http/sys_seal.go @@ -95,12 +95,7 @@ func handleSysUnseal(core *vault.Core) http.Handler { } if req.Reset { - sealed, err := core.Sealed() - if err != nil { - respondError(w, http.StatusInternalServerError, err) - return - } - if !sealed { + if !core.Sealed() { respondError(w, http.StatusBadRequest, errors.New("vault is unsealed")) return } @@ -164,13 +159,10 @@ func handleSysSealStatus(core *vault.Core) http.Handler { func handleSysSealStatusRaw(core *vault.Core, w http.ResponseWriter, r *http.Request) { ctx := context.Background() - sealed, err := core.Sealed() - if err != nil { - respondError(w, http.StatusInternalServerError, err) - return - } + sealed := core.Sealed() var sealConfig *vault.SealConfig + var err error if core.SealAccess().RecoveryKeySupported() { sealConfig, err = core.SealAccess().RecoveryConfig(ctx) } else { diff --git a/http/sys_seal_test.go b/http/sys_seal_test.go index 902466ee4d7d..68c9b53075b9 100644 --- a/http/sys_seal_test.go +++ b/http/sys_seal_test.go @@ -75,11 +75,7 @@ func TestSysSeal(t *testing.T) { resp := testHttpPut(t, token, addr+"/v1/sys/seal", nil) testResponseStatus(t, resp, 204) - check, err := core.Sealed() - if err != nil { - t.Fatalf("err: %s", err) - } - if !check { + if !core.Sealed() { t.Fatal("should be sealed") } } @@ -93,11 +89,7 @@ func TestSysSeal_unsealed(t *testing.T) { resp := testHttpPut(t, token, addr+"/v1/sys/seal", nil) testResponseStatus(t, resp, 204) - check, err := core.Sealed() - if err != nil { - t.Fatalf("err: %s", err) - } - if !check { + if !core.Sealed() { t.Fatal("should be sealed") } } diff --git a/vault/cluster_test.go b/vault/cluster_test.go index 282c3701b378..2b5b661efb45 100644 --- a/vault/cluster_test.go +++ b/vault/cluster_test.go @@ -70,11 +70,7 @@ func TestClusterHAFetching(t *testing.T) { } // Verify unsealed - sealed, err := c.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if c.Sealed() { t.Fatal("should not be sealed") } diff --git a/vault/core.go b/vault/core.go index cab0cb373c5e..33515d5942d3 100644 --- a/vault/core.go +++ b/vault/core.go @@ -185,7 +185,7 @@ type Core struct { // stateLock protects mutable state stateLock sync.RWMutex - sealed bool + sealed *uint32 standby bool standbyDoneCh chan struct{} @@ -480,7 +480,7 @@ func NewCore(conf *CoreConfig) (*Core, error) { clusterAddr: conf.ClusterAddr, seal: conf.Seal, router: NewRouter(), - sealed: true, + sealed: new(uint32), standby: true, logger: conf.Logger.Named("core"), defaultLeaseTTL: conf.DefaultLeaseTTL, @@ -503,6 +503,8 @@ func NewCore(conf *CoreConfig) (*Core, error) { keepHALockOnStepDown: new(uint32), } + atomic.StoreUint32(c.sealed, 1) + atomic.StoreUint32(c.replicationState, uint32(consts.ReplicationDRDisabled|consts.ReplicationPerformanceDisabled)) c.localClusterCert.Store(([]byte)(nil)) c.localClusterParsedCert.Store((*x509.Certificate)(nil)) @@ -634,19 +636,6 @@ func NewCore(conf *CoreConfig) (*Core, error) { // happens as quickly as possible. func (c *Core) Shutdown() error { c.logger.Debug("shutdown called") - c.stateLock.RLock() - // Tell any requests that know about this to stop - if c.activeContextCancelFunc != nil { - c.activeContextCancelFunc() - } - c.stateLock.RUnlock() - - c.logger.Debug("shutdown initiating internal seal") - // Seal the Vault, causes a leader stepdown - c.stateLock.Lock() - defer c.stateLock.Unlock() - - c.logger.Debug("shutdown running internal seal") return c.sealInternal(false) } @@ -663,10 +652,8 @@ func (c *Core) GetContext() (context.Context, context.CancelFunc) { } // Sealed checks if the Vault is current sealed -func (c *Core) Sealed() (bool, error) { - c.stateLock.RLock() - defer c.stateLock.RUnlock() - return c.sealed, nil +func (c *Core) Sealed() bool { + return atomic.LoadUint32(c.sealed) == 1 } // SecretProgress returns the number of keys provided so far @@ -686,9 +673,6 @@ func (c *Core) SecretProgress() (int, string) { func (c *Core) ResetUnsealProcess() { c.stateLock.Lock() defer c.stateLock.Unlock() - if !c.sealed { - return - } c.unlockInfo = nil } @@ -732,7 +716,7 @@ func (c *Core) Unseal(key []byte) (bool, error) { } // Check if already unsealed - if !c.sealed { + if !c.Sealed() { return true, nil } @@ -774,7 +758,7 @@ func (c *Core) UnsealWithRecoveryKeys(ctx context.Context, key []byte) (bool, er } // Check if already unsealed - if !c.sealed { + if !c.Sealed() { return true, nil } @@ -918,14 +902,14 @@ func (c *Core) unsealInternal(ctx context.Context, masterKey []byte) (bool, erro go c.runStandby(c.standbyDoneCh, c.manualStepDownCh, c.standbyStopCh) } - // Success! - c.sealed = false - // Force a cache bust here, which will also run migration code if c.seal.RecoveryKeySupported() { c.seal.SetRecoveryConfig(ctx, nil) } + // Success! + atomic.StoreUint32(c.sealed, 0) + if c.ha != nil { sd, ok := c.ha.(physical.ServiceDiscovery) if ok { @@ -944,13 +928,12 @@ func (c *Core) unsealInternal(ctx context.Context, masterKey []byte) (bool, erro func (c *Core) SealWithRequest(req *logical.Request) error { defer metrics.MeasureSince([]string{"core", "seal-with-request"}, time.Now()) - c.stateLock.RLock() - - if c.sealed { - c.stateLock.RUnlock() + if c.Sealed() { return nil } + c.stateLock.RLock() + // This will unlock the read lock // We use background context since we may not be active return c.sealInitCommon(context.Background(), req) @@ -961,13 +944,12 @@ func (c *Core) SealWithRequest(req *logical.Request) error { func (c *Core) Seal(token string) error { defer metrics.MeasureSince([]string{"core", "seal"}, time.Now()) - c.stateLock.RLock() - - if c.sealed { - c.stateLock.RUnlock() + if c.Sealed() { return nil } + c.stateLock.RLock() + req := &logical.Request{ Operation: logical.UpdateOperation, Path: "sys/seal", @@ -1094,17 +1076,9 @@ func (c *Core) sealInitCommon(ctx context.Context, req *logical.Request) (retErr } } - // Tell any requests that know about this to stop - if c.activeContextCancelFunc != nil { - c.activeContextCancelFunc() - } - - // Unlock from the request handling + // Unlock; sealing will grab the lock when needed c.stateLock.RUnlock() - //Seal the Vault - c.stateLock.Lock() - defer c.stateLock.Unlock() sealErr := c.sealInternal(false) if sealErr != nil { @@ -1125,15 +1099,13 @@ func (c *Core) UIHeaders() (http.Header, error) { } // sealInternal is an internal method used to seal the vault. It does not do -// any authorization checking. The stateLock must be held prior to calling. +// any authorization checking. func (c *Core) sealInternal(keepLock bool) error { - if c.sealed { + // Mark sealed, and if already marked return + if swapped := atomic.CompareAndSwapUint32(c.sealed, 0, 1); !swapped { return nil } - // Enable that we are sealed to prevent further transactions - c.sealed = true - c.logger.Debug("marked as sealed") // Clear forwarding clients @@ -1143,15 +1115,29 @@ func (c *Core) sealInternal(keepLock bool) error { // Do pre-seal teardown if HA is not enabled if c.ha == nil { + c.stateLock.Lock() + defer c.stateLock.Unlock() // Even in a non-HA context we key off of this for some things c.standby = true + + // Stop requests from processing + if c.activeContextCancelFunc != nil { + c.activeContextCancelFunc() + } + if err := c.preSeal(); err != nil { c.logger.Error("pre-seal teardown failed", "error", err) return fmt.Errorf("internal error") } } else { + // If we are keeping the lock we already have the state write lock + // held. Otherwise grab it here so that when stopCh is triggered we are + // locked. if keepLock { atomic.StoreUint32(c.keepHALockOnStepDown, 1) + } else { + c.stateLock.Lock() + defer c.stateLock.Unlock() } // If we are trying to acquire the lock, force it to return with nil so // runStandby will exit diff --git a/vault/core_test.go b/vault/core_test.go index ff28434745b9..3a31f02b66ca 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -72,11 +72,7 @@ func TestCore_Unseal_MultiShare(t *testing.T) { t.Fatalf("err: %v", err) } - sealed, err := c.Sealed() - if err != nil { - t.Fatalf("err: %v", err) - } - if !sealed { + if !c.Sealed() { t.Fatalf("should be sealed") } @@ -112,11 +108,7 @@ func TestCore_Unseal_MultiShare(t *testing.T) { } } - sealed, err = c.Sealed() - if err != nil { - t.Fatalf("err: %v", err) - } - if sealed { + if c.Sealed() { t.Fatalf("should not be sealed") } @@ -131,11 +123,7 @@ func TestCore_Unseal_MultiShare(t *testing.T) { t.Fatalf("err: %v", err) } - sealed, err = c.Sealed() - if err != nil { - t.Fatalf("err: %v", err) - } - if !sealed { + if !c.Sealed() { t.Fatalf("should be sealed") } } @@ -160,11 +148,7 @@ func TestCore_Unseal_Single(t *testing.T) { t.Fatalf("err: %v", err) } - sealed, err := c.Sealed() - if err != nil { - t.Fatalf("err: %v", err) - } - if !sealed { + if !c.Sealed() { t.Fatalf("should be sealed") } @@ -184,11 +168,7 @@ func TestCore_Unseal_Single(t *testing.T) { t.Fatalf("bad progress: %d", prog) } - sealed, err = c.Sealed() - if err != nil { - t.Fatalf("err: %v", err) - } - if sealed { + if c.Sealed() { t.Fatalf("should not be sealed") } } @@ -257,8 +237,8 @@ func TestCore_Shutdown(t *testing.T) { if err := c.Shutdown(); err != nil { t.Fatalf("err: %v", err) } - if sealed, err := c.Sealed(); err != nil || !sealed { - t.Fatalf("err: %v", err) + if !c.Sealed() { + t.Fatal("wasn't sealed") } } @@ -268,8 +248,8 @@ func TestCore_Seal_BadToken(t *testing.T) { if err := c.Seal("foo"); err == nil { t.Fatalf("err: %v", err) } - if sealed, err := c.Sealed(); err != nil || sealed { - t.Fatalf("err: %v", err) + if c.Sealed() { + t.Fatal("was sealed") } } @@ -284,8 +264,8 @@ func TestCore_Seal_SingleUse(t *testing.T) { if err := c.Seal("foo"); err != nil { t.Fatalf("err: %v", err) } - if sealed, err := c.Sealed(); err != nil || !sealed { - t.Fatalf("err: %v, sealed: %t", err, sealed) + if !c.Sealed() { + t.Fatal("not sealed") } for i, key := range keys { unseal, err := TestCoreUnseal(c, key) @@ -1146,11 +1126,7 @@ func TestCore_Standby_Seal(t *testing.T) { } // Verify unsealed - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } @@ -1187,11 +1163,7 @@ func TestCore_Standby_Seal(t *testing.T) { } // Verify unsealed - sealed, err = core2.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core2.Sealed() { t.Fatal("should not be sealed") } @@ -1264,11 +1236,7 @@ func TestCore_StepDown(t *testing.T) { } // Verify unsealed - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } @@ -1305,11 +1273,7 @@ func TestCore_StepDown(t *testing.T) { } // Verify unsealed - sealed, err = core2.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core2.Sealed() { t.Fatal("should not be sealed") } @@ -1462,11 +1426,7 @@ func TestCore_CleanLeaderPrefix(t *testing.T) { } // Verify unsealed - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } @@ -1530,11 +1490,7 @@ func TestCore_CleanLeaderPrefix(t *testing.T) { } // Verify unsealed - sealed, err = core2.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core2.Sealed() { t.Fatal("should not be sealed") } @@ -1647,11 +1603,7 @@ func testCore_Standby_Common(t *testing.T, inm physical.Backend, inmha physical. } // Verify unsealed - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } @@ -1702,11 +1654,7 @@ func testCore_Standby_Common(t *testing.T, inm physical.Backend, inmha physical. } // Verify unsealed - sealed, err = core2.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core2.Sealed() { t.Fatal("should not be sealed") } diff --git a/vault/generate_root.go b/vault/generate_root.go index 37f408e02ec3..9a8750a0e9ec 100644 --- a/vault/generate_root.go +++ b/vault/generate_root.go @@ -73,7 +73,7 @@ type GenerateRootResult struct { func (c *Core) GenerateRootProgress() (int, error) { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return 0, consts.ErrSealed } if c.standby { @@ -91,7 +91,7 @@ func (c *Core) GenerateRootProgress() (int, error) { func (c *Core) GenerateRootConfiguration() (*GenerateRootConfig, error) { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil, consts.ErrSealed } if c.standby { @@ -141,7 +141,7 @@ func (c *Core) GenerateRootInit(otp, pgpKey string, strategy GenerateRootStrateg c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return consts.ErrSealed } if c.standby { @@ -211,7 +211,7 @@ func (c *Core) GenerateRootUpdate(ctx context.Context, key []byte, nonce string, // Ensure we are already unsealed c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil, consts.ErrSealed } if c.standby { @@ -349,7 +349,7 @@ func (c *Core) GenerateRootUpdate(ctx context.Context, key []byte, nonce string, func (c *Core) GenerateRootCancel() error { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return consts.ErrSealed } if c.standby { diff --git a/vault/ha.go b/vault/ha.go index f65af4b6a9d9..51fc9f3682f0 100644 --- a/vault/ha.go +++ b/vault/ha.go @@ -37,14 +37,13 @@ func (c *Core) Leader() (isLeader bool, leaderAddr, clusterAddr string, err erro return false, "", "", ErrHANotEnabled } - c.stateLock.RLock() - // Check if sealed - if c.sealed { - c.stateLock.RUnlock() + if c.Sealed() { return false, "", "", consts.ErrSealed } + c.stateLock.RLock() + // Check if we are the leader if !c.standby { c.stateLock.RUnlock() @@ -153,7 +152,7 @@ func (c *Core) StepDown(req *logical.Request) (retErr error) { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil } if c.ha == nil || c.standby { @@ -396,7 +395,7 @@ func (c *Core) waitForLeadership(doneCh, manualStepDownCh, stopCh chan struct{}) // We now have the lock and can use it } - if c.sealed { + if c.Sealed() { c.logger.Warn("grabbed HA lock but already sealed, exiting") lock.Unlock() c.stateLock.Unlock() @@ -475,42 +474,28 @@ func (c *Core) waitForLeadership(doneCh, manualStepDownCh, stopCh chan struct{}) continue } - // Monitor a loss of leadership - releaseHALock := true - grabStateLock := true - select { - case <-leaderLostCh: - c.logger.Warn("leadership lost, stopping active operation") - case <-stopCh: - // This case comes from sealInternal; we will already be having the - // state lock held so we do toggle grabStateLock to false - if atomic.LoadUint32(c.keepHALockOnStepDown) == 1 { - releaseHALock = false - } - grabStateLock = false - case <-manualStepDownCh: - c.logger.Warn("stepping down from active operation to standby") - manualStepDown = true - } + runSealing := func() { + metrics.MeasureSince([]string{"core", "leadership_lost"}, activeTime) - metrics.MeasureSince([]string{"core", "leadership_lost"}, activeTime) + // Tell any requests that know about this to stop + if c.activeContextCancelFunc != nil { + c.activeContextCancelFunc() + } - // Tell any requests that know about this to stop - if c.activeContextCancelFunc != nil { - c.activeContextCancelFunc() - } + c.standby = true - // Attempt the pre-seal process - if grabStateLock { - c.stateLock.Lock() - } - c.standby = true - preSealErr := c.preSeal() - if grabStateLock { - c.stateLock.Unlock() + if err := c.preSeal(); err != nil { + c.logger.Error("pre-seal teardown failed", "error", err) + } } - if releaseHALock { + releaseHALock := func() { + // We may hit this from leaderLostCh or manualStepDownCh if they + // triggered before stopCh, so we check here instead of only in the + // stopCh case so we can try to do the right thing then, too + if atomic.LoadUint32(c.keepHALockOnStepDown) == 1 { + return + } if err := c.clearLeader(uuid); err != nil { c.logger.Error("clearing leader advertisement failed", "error", err) } @@ -518,9 +503,29 @@ func (c *Core) waitForLeadership(doneCh, manualStepDownCh, stopCh chan struct{}) c.heldHALock = nil } - // Check for a failure to prepare to seal - if preSealErr != nil { - c.logger.Error("pre-seal teardown failed", "error", err) + // Monitor a loss of leadership + select { + case <-leaderLostCh: + c.logger.Warn("leadership lost, stopping active operation") + + c.stateLock.Lock() + runSealing() + releaseHALock() + c.stateLock.Unlock() + + case <-stopCh: + runSealing() + releaseHALock() + return + + case <-manualStepDownCh: + manualStepDown = true + c.logger.Warn("stepping down from active operation to standby") + + c.stateLock.Lock() + runSealing() + releaseHALock() + c.stateLock.Unlock() } } } diff --git a/vault/identity_store_entities_test.go b/vault/identity_store_entities_test.go index 29eea085520d..5198204ac48b 100644 --- a/vault/identity_store_entities_test.go +++ b/vault/identity_store_entities_test.go @@ -306,11 +306,7 @@ func TestIdentityStore_LoadingEntities(t *testing.T) { } } - sealed, err := c.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if c.Sealed() { t.Fatal("should not be sealed") } @@ -402,11 +398,7 @@ func TestIdentityStore_LoadingEntities(t *testing.T) { t.Fatalf("failed to seal core: %v", err) } - sealed, err = c.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if !sealed { + if !c.Sealed() { t.Fatal("should be sealed") } @@ -416,11 +408,7 @@ func TestIdentityStore_LoadingEntities(t *testing.T) { } } - sealed, err = c.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if c.Sealed() { t.Fatal("should not be sealed") } diff --git a/vault/init.go b/vault/init.go index 659858e90a40..b463ec2349d8 100644 --- a/vault/init.go +++ b/vault/init.go @@ -259,11 +259,7 @@ func (c *Core) UnsealWithStoredKeys(ctx context.Context) error { return nil } - sealed, err := c.Sealed() - if err != nil { - c.logger.Error("error checking sealed status in auto-unseal", "error", err) - return errwrap.Wrapf("error checking sealed status in auto-unseal: {{err}}", err) - } + sealed := c.Sealed() if !sealed { return nil } diff --git a/vault/logical_system_integ_test.go b/vault/logical_system_integ_test.go index 3a3c2ae4e3a2..d633e833142c 100644 --- a/vault/logical_system_integ_test.go +++ b/vault/logical_system_integ_test.go @@ -48,11 +48,7 @@ func TestSystemBackend_Plugin_secret(t *testing.T) { t.Fatal(err) } } - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } // Wait for active so post-unseal takes place @@ -90,11 +86,7 @@ func TestSystemBackend_Plugin_auth(t *testing.T) { t.Fatal(err) } } - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } // Wait for active so post-unseal takes place @@ -169,11 +161,7 @@ func testPlugin_CatalogRemoved(t *testing.T, btype logical.BackendType, testMoun t.Fatal(err) } } - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } } @@ -282,11 +270,7 @@ func testPlugin_continueOnError(t *testing.T, btype logical.BackendType, mismatc t.Fatal(err) } } - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } } @@ -391,11 +375,7 @@ func TestSystemBackend_Plugin_SealUnseal(t *testing.T) { t.Fatal(err) } } - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } } diff --git a/vault/rekey.go b/vault/rekey.go index a55f184f9193..89b35737076b 100644 --- a/vault/rekey.go +++ b/vault/rekey.go @@ -61,7 +61,7 @@ type RekeyBackup struct { func (c *Core) RekeyThreshold(ctx context.Context, recovery bool) (int, logical.HTTPCodedError) { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return 0, logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -95,7 +95,7 @@ func (c *Core) RekeyThreshold(ctx context.Context, recovery bool) (int, logical. func (c *Core) RekeyProgress(recovery, verification bool) (bool, int, logical.HTTPCodedError) { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return false, 0, logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -126,7 +126,7 @@ func (c *Core) RekeyProgress(recovery, verification bool) (bool, int, logical.HT func (c *Core) RekeyConfig(recovery bool) (*SealConfig, logical.HTTPCodedError) { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil, logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -200,7 +200,7 @@ func (c *Core) BarrierRekeyInit(config *SealConfig) logical.HTTPCodedError { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -250,7 +250,7 @@ func (c *Core) RecoveryRekeyInit(config *SealConfig) logical.HTTPCodedError { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -299,7 +299,7 @@ func (c *Core) BarrierRekeyUpdate(ctx context.Context, key []byte, nonce string) // Ensure we are already unsealed c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil, logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -535,7 +535,7 @@ func (c *Core) RecoveryRekeyUpdate(ctx context.Context, key []byte, nonce string // Ensure we are already unsealed c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil, logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -734,7 +734,7 @@ func (c *Core) RekeyVerify(ctx context.Context, key []byte, nonce string, recove // Ensure we are already unsealed c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil, logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -848,7 +848,7 @@ func (c *Core) RekeyVerify(ctx context.Context, key []byte, nonce string, recove func (c *Core) RekeyCancel(recovery bool) logical.HTTPCodedError { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -871,7 +871,7 @@ func (c *Core) RekeyCancel(recovery bool) logical.HTTPCodedError { func (c *Core) RekeyVerifyRestart(recovery bool) logical.HTTPCodedError { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -906,7 +906,7 @@ func (c *Core) RekeyVerifyRestart(recovery bool) logical.HTTPCodedError { func (c *Core) RekeyRetrieveBackup(ctx context.Context, recovery bool) (*RekeyBackup, logical.HTTPCodedError) { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil, logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { @@ -943,7 +943,7 @@ func (c *Core) RekeyRetrieveBackup(ctx context.Context, recovery bool) (*RekeyBa func (c *Core) RekeyDeleteBackup(ctx context.Context, recovery bool) logical.HTTPCodedError { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return logical.CodedError(http.StatusServiceUnavailable, consts.ErrSealed.Error()) } if c.standby { diff --git a/vault/rekey_test.go b/vault/rekey_test.go index b0407e46f7fb..6b65ce5513d3 100644 --- a/vault/rekey_test.go +++ b/vault/rekey_test.go @@ -226,7 +226,7 @@ func testCore_Rekey_Update_Common(t *testing.T, c *Core, keys [][]byte, root str t.Fatalf("err: %v", err) } } - if sealed, _ := c.Sealed(); sealed { + if c.Sealed() { t.Fatalf("should be unsealed") } } diff --git a/vault/request_handling.go b/vault/request_handling.go index f474e062bae5..5932d5b33149 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -268,7 +268,7 @@ func (c *Core) checkToken(ctx context.Context, req *logical.Request, unauth bool func (c *Core) HandleRequest(req *logical.Request) (resp *logical.Response, err error) { c.stateLock.RLock() defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil, consts.ErrSealed } if c.standby { diff --git a/vault/seal_testing.go b/vault/seal_testing.go index a3d4abf19741..4f6b1d125832 100644 --- a/vault/seal_testing.go +++ b/vault/seal_testing.go @@ -34,18 +34,14 @@ func testCoreUnsealedWithConfigs(t testing.T, barrierConf, recoveryConf *SealCon if err != nil { t.Fatalf("err: %s", err) } - if sealed, _ := core.Sealed(); sealed { + if core.Sealed() { for _, key := range result.SecretShares { if _, err := core.Unseal(TestKeyCopy(key)); err != nil { t.Fatalf("unseal err: %s", err) } } - sealed, err = core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } } @@ -76,11 +72,7 @@ func TestCoreUnsealedWithConfigSealOpts(t testing.T, barrierConf, recoveryConf * } } - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } diff --git a/vault/testing.go b/vault/testing.go index 99f0d65a5b7b..8738bd291e6a 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -260,11 +260,7 @@ func testCoreUnsealed(t testing.T, core *Core) (*Core, [][]byte, string) { } } - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } @@ -293,11 +289,7 @@ func TestCoreUnsealedBackend(t testing.T, backend physical.Backend) (*Core, [][] t.Fatal(err) } - sealed, err := core.Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if core.Sealed() { t.Fatal("should not be sealed") } @@ -740,11 +732,7 @@ func (c *TestCluster) UnsealCoresWithError() error { } // Verify unsealed - sealed, err := c.Cores[0].Sealed() - if err != nil { - return fmt.Errorf("err checking seal status: %s", err) - } - if sealed { + if c.Cores[0].Sealed() { return fmt.Errorf("should not be sealed") } @@ -818,11 +806,7 @@ func (c *TestCluster) ensureCoresSealed() error { if time.Now().After(timeout) { return fmt.Errorf("timeout waiting for core to seal") } - sealed, err := core.Sealed() - if err != nil { - return err - } - if sealed { + if core.Sealed() { break } time.Sleep(250 * time.Millisecond) @@ -842,11 +826,7 @@ func (c *TestCluster) UnsealWithStoredKeys(t testing.T) error { if time.Now().After(timeout) { return fmt.Errorf("timeout waiting for core to unseal") } - sealed, err := core.Sealed() - if err != nil { - return err - } - if !sealed { + if !core.Sealed() { break } time.Sleep(250 * time.Millisecond) @@ -1328,11 +1308,7 @@ func NewTestCluster(t testing.T, base *CoreConfig, opts *TestClusterOptions) *Te } // Verify unsealed - sealed, err := cores[0].Sealed() - if err != nil { - t.Fatalf("err checking seal status: %s", err) - } - if sealed { + if cores[0].Sealed() { t.Fatal("should not be sealed") } diff --git a/vault/token_store.go b/vault/token_store.go index aade87a05270..c62ab4fa52a7 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -85,11 +85,13 @@ func (c *Core) LookupToken(token string) (*logical.TokenEntry, error) { return nil, fmt.Errorf("missing client token") } - c.stateLock.RLock() - defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return nil, consts.ErrSealed } + + c.stateLock.RLock() + defer c.stateLock.RUnlock() + if c.standby { return nil, consts.ErrStandby } diff --git a/vault/wrapping.go b/vault/wrapping.go index 5f2b59d5c528..3dbbfb6cd430 100644 --- a/vault/wrapping.go +++ b/vault/wrapping.go @@ -319,11 +319,12 @@ func (c *Core) ValidateWrappingToken(req *logical.Request) (bool, error) { return false, fmt.Errorf("token is empty") } - c.stateLock.RLock() - defer c.stateLock.RUnlock() - if c.sealed { + if c.Sealed() { return false, consts.ErrSealed } + + c.stateLock.RLock() + defer c.stateLock.RUnlock() if c.standby { return false, consts.ErrStandby }