Skip to content

Commit

Permalink
Implement user lockout log (#23140)
Browse files Browse the repository at this point in the history
* implement user lockout logger

* formatting

* make user lockout log interval configurable

* create func to get locked user count, and fix potential deadlock

* fix test

* fix test

* add changelog
  • Loading branch information
davidadeleon committed Oct 12, 2023
1 parent 1ed2747 commit a272989
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 3 deletions.
3 changes: 3 additions & 0 deletions changelog/23140.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
core: emit logs when user(s) are locked out and when all lockouts have been cleared
```
1 change: 1 addition & 0 deletions command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2827,6 +2827,7 @@ func createCoreConfig(c *ServerCommand, config *server.Config, backend physical.
DisableSSCTokens: config.DisableSSCTokens,
Experiments: config.Experiments,
AdministrativeNamespacePath: config.AdministrativeNamespacePath,
UserLockoutLogInterval: config.UserLockoutLogInterval,
}

if c.flagDev {
Expand Down
1 change: 1 addition & 0 deletions command/server/config_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ func testConfig_Sanitized(t *testing.T) {
"enable_response_header_hostname": false,
"enable_response_header_raft_node_id": false,
"log_requests_level": "basic",
"user_lockout_log_interval": 0 * time.Second,
"ha_storage": map[string]interface{}{
"cluster_addr": "top_level_cluster_addr",
"disable_clustering": true,
Expand Down
1 change: 1 addition & 0 deletions http/sys_config_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ func TestSysConfigState_Sanitized(t *testing.T) {
"enable_response_header_hostname": false,
"enable_response_header_raft_node_id": false,
"log_requests_level": "",
"user_lockout_log_interval": json.Number("0"),
"listeners": []interface{}{
map[string]interface{}{
"config": nil,
Expand Down
13 changes: 12 additions & 1 deletion internalshared/configutil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ type SharedConfig struct {

Listeners []*Listener `hcl:"-"`

UserLockouts []*UserLockout `hcl:"-"`
UserLockouts []*UserLockout `hcl:"-"`
UserLockoutLogInterval time.Duration `hcl:"-"`
UserLockoutLogIntervalRaw interface{} `hcl:"user_lockout_log_interval"`

Seals []*KMS `hcl:"-"`
Entropy *Entropy `hcl:"-"`
Expand Down Expand Up @@ -87,6 +89,14 @@ func ParseConfig(d string) (*SharedConfig, error) {
result.DisableMlockRaw = nil
}

if result.UserLockoutLogIntervalRaw != nil {
if result.UserLockoutLogInterval, err = parseutil.ParseDurationSecond(result.UserLockoutLogIntervalRaw); err != nil {
return nil, err
}
result.FoundKeys = append(result.FoundKeys, "UserLockoutLogInterval")
result.UserLockoutLogIntervalRaw = nil
}

list, ok := obj.Node.(*ast.ObjectList)
if !ok {
return nil, fmt.Errorf("error parsing: file doesn't contain a root object")
Expand Down Expand Up @@ -176,6 +186,7 @@ func (c *SharedConfig) Sanitized() map[string]interface{} {
"pid_file": c.PidFile,
"cluster_name": c.ClusterName,
"administrative_namespace_path": c.AdministrativeNamespacePath,
"user_lockout_log_interval": c.UserLockoutLogInterval,
}

// Optional log related settings
Expand Down
5 changes: 5 additions & 0 deletions internalshared/configutil/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ func (c *SharedConfig) Merge(c2 *SharedConfig) *SharedConfig {
result.DefaultMaxRequestDuration = c2.DefaultMaxRequestDuration
}

result.UserLockoutLogInterval = c.UserLockoutLogInterval
if c2.UserLockoutLogInterval > result.UserLockoutLogInterval {
result.UserLockoutLogInterval = c2.UserLockoutLogInterval
}

result.LogLevel = c.LogLevel
if c2.LogLevel != "" {
result.LogLevel = c2.LogLevel
Expand Down
68 changes: 67 additions & 1 deletion vault/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ const (
// MfaAuthResponse when the value is not specified in the server config
defaultMFAAuthResponseTTL = 300 * time.Second

// defaultUserLockoutLogInterval is the default duration that Vault will
// emit a log informing that a user lockout is in effect when the value
// is not specified in the server config
defaultUserLockoutLogInterval = 1 * time.Minute

// defaultMaxTOTPValidateAttempts is the default value for the number
// of failed attempts to validate a request subject to TOTP MFA. If the
// number of failed totp passcode validations exceeds this max value, the
Expand Down Expand Up @@ -649,6 +654,9 @@ type Core struct {

updateLockedUserEntriesCancel context.CancelFunc

lockoutLoggerCancel context.CancelFunc
userLockoutLogInterval time.Duration

// number of workers to use for lease revocation in the expiration manager
numExpirationWorkers int

Expand Down Expand Up @@ -860,6 +868,8 @@ type CoreConfig struct {
// AdministrativeNamespacePath is used to configure the administrative namespace, which has access to some sys endpoints that are
// only accessible in the root namespace, currently sys/audit-hash and sys/monitor.
AdministrativeNamespacePath string

UserLockoutLogInterval time.Duration
}

// SubloggerHook implements the SubloggerAdder interface. This implementation
Expand Down Expand Up @@ -907,6 +917,10 @@ func CreateCore(conf *CoreConfig) (*Core, error) {
return nil, fmt.Errorf("cannot have DefaultLeaseTTL larger than MaxLeaseTTL")
}

if conf.UserLockoutLogInterval == 0 {
conf.UserLockoutLogInterval = defaultUserLockoutLogInterval
}

// Validate the advertise addr if its given to us
if conf.RedirectAddr != "" {
u, err := url.Parse(conf.RedirectAddr)
Expand Down Expand Up @@ -1028,6 +1042,7 @@ func CreateCore(conf *CoreConfig) (*Core, error) {
disableSSCTokens: conf.DisableSSCTokens,
effectiveSDKVersion: effectiveSDKVersion,
userFailedLoginInfo: make(map[FailedLoginUser]*FailedLoginInfo),
userLockoutLogInterval: conf.UserLockoutLogInterval,
experiments: conf.Experiments,
pendingRemovalMountsAllowed: conf.PendingRemovalMountsAllowed,
expirationRevokeRetryBase: conf.ExpirationRevokeRetryBase,
Expand Down Expand Up @@ -3448,6 +3463,51 @@ func (c *Core) setupCachedMFAResponseAuth() {
return
}

func (c *Core) startLockoutLogger() {
// Are we already running a logger
if c.lockoutLoggerCancel != nil {
return
}

ctx, cancelFunc := context.WithCancel(c.activeContext)
c.lockoutLoggerCancel = cancelFunc

// Perform first check for lockout entries
lockedUserCount := c.getUserFailedLoginCount(ctx)

if lockedUserCount > 0 {
c.Logger().Warn("user lockout(s) in effect")
} else {
// We shouldn't end up here
return
}

// Start lockout watcher
go func() {
ticker := time.NewTicker(c.userLockoutLogInterval)
for {
select {
case <-ticker.C:
// Check for lockout entries
lockedUserCount := c.getUserFailedLoginCount(ctx)

if lockedUserCount > 0 {
c.Logger().Warn("user lockout(s) in effect")
break
}
c.Logger().Info("user lockout(s) cleared")
ticker.Stop()
c.lockoutLoggerCancel = nil
return
case <-ctx.Done():
ticker.Stop()
c.lockoutLoggerCancel = nil
return
}
}
}()
}

// updateLockedUserEntries runs every 15 mins to remove stale user entries from storage
// it also updates the userFailedLoginInfo map with correct information for locked users if incorrect
func (c *Core) updateLockedUserEntries() {
Expand Down Expand Up @@ -3476,7 +3536,13 @@ func (c *Core) updateLockedUserEntries() {
}
}
}()
return
}

func (c *Core) getUserFailedLoginCount(ctx context.Context) int {
c.userFailedLoginInfoLock.Lock()
defer c.userFailedLoginInfoLock.Unlock()

return len(c.userFailedLoginInfo)
}

// runLockedUserEntryUpdates runs updates for locked user storage entries and userFailedLoginInfo map
Expand Down
10 changes: 10 additions & 0 deletions vault/logical_system_user_lockout.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,16 @@ func unlockUser(ctx context.Context, core *Core, mountAccessor string, aliasName
return err
}

// Check if we have no more locked users and cancel any running lockout logger
core.userFailedLoginInfoLock.RLock()
numLockedUsers := len(core.userFailedLoginInfo)
core.userFailedLoginInfoLock.RUnlock()

if numLockedUsers == 0 {
core.Logger().Info("user lockout(s) cleared")
core.lockoutLoggerCancel()
}

return nil
}

Expand Down
4 changes: 3 additions & 1 deletion vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -1464,6 +1464,7 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
return nil, nil, err
}
if isloginUserLocked {
c.startLockoutLogger()
return nil, nil, logical.ErrPermissionDenied
}
}
Expand Down Expand Up @@ -2279,6 +2280,8 @@ func (c *Core) LocalGetUserFailedLoginInfo(ctx context.Context, userKey FailedLo
// LocalUpdateUserFailedLoginInfo updates the failed login information for a user based on alias name and mountAccessor
func (c *Core) LocalUpdateUserFailedLoginInfo(ctx context.Context, userKey FailedLoginUser, failedLoginInfo *FailedLoginInfo, deleteEntry bool) error {
c.userFailedLoginInfoLock.Lock()
defer c.userFailedLoginInfoLock.Unlock()

switch deleteEntry {
case false:
// update entry in the map
Expand Down Expand Up @@ -2321,7 +2324,6 @@ func (c *Core) LocalUpdateUserFailedLoginInfo(ctx context.Context, userKey Faile
// delete the entry from the map, if no key exists it is no-op
delete(c.userFailedLoginInfo, userKey)
}
c.userFailedLoginInfoLock.Unlock()
return nil
}

Expand Down

0 comments on commit a272989

Please sign in to comment.