From f5fedb026fa5ae3b14a624afff3c77a42b28822a Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Mon, 27 Nov 2023 16:21:51 -0500 Subject: [PATCH] backport of commit c329ed8d3b02b92dfded30065317c82648d3cae3 (#24260) Co-authored-by: Steven Clark --- changelog/24256.txt | 4 ++++ vault/ha.go | 23 ++++++++++++++++------- vault/logical_system.go | 17 ++++++++++++----- vault/raft.go | 3 +++ 4 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 changelog/24256.txt diff --git a/changelog/24256.txt b/changelog/24256.txt new file mode 100644 index 000000000..74124710b --- /dev/null +++ b/changelog/24256.txt @@ -0,0 +1,4 @@ +```release-note:bug +api: Fix deadlock on calls to sys/leader with a namespace configured +on the request. +``` diff --git a/vault/ha.go b/vault/ha.go index b18d33df0..6a8b00845 100644 --- a/vault/ha.go +++ b/vault/ha.go @@ -150,30 +150,41 @@ func (c *Core) Leader() (isLeader bool, leaderAddr, clusterAddr string, err erro if c.Sealed() { return false, "", "", consts.ErrSealed } - c.stateLock.RLock() + defer c.stateLock.RUnlock() + + return c.LeaderLocked() +} + +func (c *Core) LeaderLocked() (isLeader bool, leaderAddr, clusterAddr string, err error) { + // Check if HA enabled. We don't need the lock for this check as it's set + // on startup and never modified + if c.ha == nil { + return false, "", "", ErrHANotEnabled + } + + // Check if sealed + if c.Sealed() { + return false, "", "", consts.ErrSealed + } // Check if we are the leader if !c.standby { - c.stateLock.RUnlock() return true, c.redirectAddr, c.ClusterAddr(), nil } // Initialize a lock lock, err := c.ha.LockWith(CoreLockPath, "read") if err != nil { - c.stateLock.RUnlock() return false, "", "", err } // Read the value held, leaderUUID, err := lock.Value() if err != nil { - c.stateLock.RUnlock() return false, "", "", err } if !held { - c.stateLock.RUnlock() return false, "", "", nil } @@ -188,13 +199,11 @@ func (c *Core) Leader() (isLeader bool, leaderAddr, clusterAddr string, err erro // If the leader hasn't changed, return the cached value; nothing changes // mid-leadership, and the barrier caches anyways if leaderUUID == localLeaderUUID && localRedirectAddr != "" { - c.stateLock.RUnlock() return false, localRedirectAddr, localClusterAddr, nil } c.logger.Trace("found new active node information, refreshing") - defer c.stateLock.RUnlock() c.leaderParamsLock.Lock() defer c.leaderParamsLock.Unlock() diff --git a/vault/logical_system.go b/vault/logical_system.go index b4b6dff75..b371e7a45 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -4811,8 +4811,15 @@ type LeaderResponse struct { } func (core *Core) GetLeaderStatus() (*LeaderResponse, error) { + core.stateLock.RLock() + defer core.stateLock.RUnlock() + + return core.GetLeaderStatusLocked() +} + +func (core *Core) GetLeaderStatusLocked() (*LeaderResponse, error) { haEnabled := true - isLeader, address, clusterAddr, err := core.Leader() + isLeader, address, clusterAddr, err := core.LeaderLocked() if errwrap.Contains(err, ErrHANotEnabled.Error()) { haEnabled = false err = nil @@ -4826,10 +4833,10 @@ func (core *Core) GetLeaderStatus() (*LeaderResponse, error) { IsSelf: isLeader, LeaderAddress: address, LeaderClusterAddress: clusterAddr, - PerfStandby: core.PerfStandby(), + PerfStandby: core.perfStandby, } if isLeader { - resp.ActiveTime = core.ActiveTime() + resp.ActiveTime = core.activeTime } if resp.PerfStandby { resp.PerfStandbyLastRemoteWAL = LastRemoteWAL(core) @@ -4837,7 +4844,7 @@ func (core *Core) GetLeaderStatus() (*LeaderResponse, error) { resp.LastWAL = LastWAL(core) } - resp.RaftCommittedIndex, resp.RaftAppliedIndex = core.GetRaftIndexes() + resp.RaftCommittedIndex, resp.RaftAppliedIndex = core.GetRaftIndexesLocked() return resp, nil } @@ -4861,7 +4868,7 @@ func (b *SystemBackend) handleSealStatus(ctx context.Context, req *logical.Reque } func (b *SystemBackend) handleLeaderStatus(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - status, err := b.Core.GetLeaderStatus() + status, err := b.Core.GetLeaderStatusLocked() if err != nil { return nil, err } diff --git a/vault/raft.go b/vault/raft.go index aa16bf61b..a40360e29 100644 --- a/vault/raft.go +++ b/vault/raft.go @@ -70,7 +70,10 @@ func (c *Core) GetRaftNodeID() string { func (c *Core) GetRaftIndexes() (committed uint64, applied uint64) { c.stateLock.RLock() defer c.stateLock.RUnlock() + return c.GetRaftIndexesLocked() +} +func (c *Core) GetRaftIndexesLocked() (committed uint64, applied uint64) { raftStorage, ok := c.underlyingPhysical.(*raft.RaftBackend) if !ok { return 0, 0