Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of Revert "Implement user lockout log (#23140)" into release/1.14.x #23765

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions changelog/23140.txt

This file was deleted.

1 change: 0 additions & 1 deletion command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2829,7 +2829,6 @@ 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: 0 additions & 1 deletion command/server/config_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,6 @@ 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: 0 additions & 1 deletion http/sys_config_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ 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: 1 addition & 12 deletions internalshared/configutil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ type SharedConfig struct {

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

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

Seals []*KMS `hcl:"-"`
Entropy *Entropy `hcl:"-"`
Expand Down Expand Up @@ -89,14 +87,6 @@ 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 @@ -186,7 +176,6 @@ 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: 0 additions & 5 deletions internalshared/configutil/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ 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: 1 addition & 67 deletions vault/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,6 @@ 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 @@ -663,9 +658,6 @@ 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 @@ -879,8 +871,6 @@ type CoreConfig struct {
// only accessible in the root namespace, currently sys/audit-hash and sys/monitor.
AdministrativeNamespacePath string

UserLockoutLogInterval time.Duration

NumRollbackWorkers int
}

Expand Down Expand Up @@ -929,10 +919,6 @@ 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 @@ -1057,7 +1043,6 @@ 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 @@ -3537,51 +3522,6 @@ 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 @@ -3610,13 +3550,7 @@ func (c *Core) updateLockedUserEntries() {
}
}
}()
}

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

return len(c.userFailedLoginInfo)
return
}

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

if core.lockoutLoggerCancel != nil {
// 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: 1 addition & 3 deletions vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -1464,7 +1464,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
}
}
Expand Down Expand Up @@ -2280,8 +2279,6 @@ 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 @@ -2324,6 +2321,7 @@ 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
Loading