diff --git a/vault/core.go b/vault/core.go index f5bcc880da..80aaa92127 100644 --- a/vault/core.go +++ b/vault/core.go @@ -3662,7 +3662,10 @@ func (c *Core) setupCachedMFAResponseAuth() { return } -func (c *Core) startLockoutLogger() { +// startLockoutLogger starts a background goroutine to emit a log while a user lockout +// exists anywhere in Vault. locked should be set to true if we need to hold the +// userFailedLoginInfoLock when getting the locked user count. +func (c *Core) startLockoutLogger(locked bool) { // Are we already running a logger if c.lockoutLoggerCancel.Load() != nil { return @@ -3672,7 +3675,7 @@ func (c *Core) startLockoutLogger() { c.lockoutLoggerCancel.Store(&cancelFunc) // Perform first check for lockout entries - lockedUserCount := c.getUserFailedLoginCount(ctx) + lockedUserCount := c.getUserFailedLoginCount(locked, ctx) if lockedUserCount > 0 { c.Logger().Warn("user lockout(s) in effect; review by using /sys/locked-users endpoint") @@ -3687,7 +3690,7 @@ func (c *Core) startLockoutLogger() { select { case <-ticker.C: // Check for lockout entries - lockedUserCount := c.getUserFailedLoginCount(ctx) + lockedUserCount := c.getUserFailedLoginCount(true, ctx) if lockedUserCount > 0 { c.Logger().Warn("user lockout(s) in effect; review by using /sys/locked-users endpoint") @@ -3736,9 +3739,11 @@ func (c *Core) updateLockedUserEntries() { }() } -func (c *Core) getUserFailedLoginCount(ctx context.Context) int { - c.userFailedLoginInfoLock.Lock() - defer c.userFailedLoginInfoLock.Unlock() +func (c *Core) getUserFailedLoginCount(locked bool, ctx context.Context) int { + if locked { + c.userFailedLoginInfoLock.Lock() + defer c.userFailedLoginInfoLock.Unlock() + } return len(c.userFailedLoginInfo) } diff --git a/vault/external_tests/identity/userlockouts_test.go b/vault/external_tests/identity/userlockouts_test.go index 7d1627eb57..59a84f34df 100644 --- a/vault/external_tests/identity/userlockouts_test.go +++ b/vault/external_tests/identity/userlockouts_test.go @@ -6,6 +6,11 @@ package identity import ( "bufio" "bytes" + "os" + "strings" + "testing" + "time" + "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/builtin/credential/userpass" @@ -14,10 +19,6 @@ import ( "github.com/hashicorp/vault/sdk/helper/logging" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault" - "os" - "strings" - "testing" - "time" ) const ( @@ -286,7 +287,6 @@ func TestUserLockoutLogger_ManualUnlockTest(t *testing.T) { if !(strings.Count(result, expected) > 1) { t.Fatalf("expected log to contain %s, got %s", expected, result) } - } // TestIdentityStore_DisableUserLockoutTest tests that user login will diff --git a/vault/logical_system_user_lockout.go b/vault/logical_system_user_lockout.go index 8c31cc947c..7578369872 100644 --- a/vault/logical_system_user_lockout.go +++ b/vault/logical_system_user_lockout.go @@ -56,13 +56,12 @@ func unlockUser(ctx context.Context, core *Core, mountAccessor string, aliasName numLockedUsers := len(core.userFailedLoginInfo) core.userFailedLoginInfoLock.RUnlock() - if numLockedUsers == 0 { - core.Logger().Info("user lockout(s) cleared") - if core.lockoutLoggerCancel.Load() == nil { - return nil + if core.lockoutLoggerCancel.Load() != nil { + if numLockedUsers == 0 { + core.Logger().Info("user lockout(s) cleared") + cancelFunc := *core.lockoutLoggerCancel.Load() + cancelFunc() } - cancelFunc := *core.lockoutLoggerCancel.Load() - cancelFunc() } return nil diff --git a/vault/request_handling.go b/vault/request_handling.go index b1282816d8..75403f2488 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -1501,7 +1501,6 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re return nil, nil, err } if isLoginUserLocked { - c.startLockoutLogger() return nil, nil, logical.ErrPermissionDenied } } @@ -2472,6 +2471,9 @@ func (c *Core) LocalUpdateUserFailedLoginInfo(ctx context.Context, userKey Faile return err } + // Start lockout logger + // We run this as locked false since we already hold the userFailedLoginInfoLock here + c.startLockoutLogger(false) } default: