luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Revert "Implement user lockout log (#23140)" (#23741) (#23765) 

This reverts commit 92fcfda8ad30a539be67b7fb7abff539bf93a098.

Co-authored-by: davidadeleon <56207066+davidadeleon@users.noreply.github.com>
This commit is contained in:
hc-github-team-secure-vault-core 2023-10-25 11:38:58 -04:00 committed by GitHub
parent 5605b59746
commit 9c14ea8114
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 3 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
changelog/23140.txt
View File

@ -1,3 +0,0 @@
```release-note:improvement
core: emit logs when user(s) are locked out and when all lockouts have been cleared
```

1
command/server.go
View File

@ -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 {

1
command/server/config_test_helpers.go
View File

@ -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,

1
http/sys_config_state_test.go
View File

@ -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,

13
internalshared/configutil/config.go
View File

@ -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:"-"`
@ -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")
@ -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

5
internalshared/configutil/merge.go
View File

@ -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

68
vault/core.go
View File

@ -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
@ -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
@ -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
}
@ -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)
@ -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,
@ -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() {
@ -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

12
vault/logical_system_user_lockout.go
View File

@ -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
}

4
vault/request_handling.go
View File

@ -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
}
}
@ -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
@ -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
}