Skip to content

Commit

Permalink
Merge pull request from GHSA-2vgg-9h6w-m454
Browse files Browse the repository at this point in the history
* feat: pick random user and exclude admin user and current user from deletion candidates

Signed-off-by: pashakostohrys <pavel@codefresh.io>

* feat: increase default max cache size

Signed-off-by: pashakostohrys <pavel@codefresh.io>

* add nil protection

Signed-off-by: pashakostohrys <pavel@codefresh.io>

* Update util/session/sessionmanager.go

Signed-off-by: Dan Garfield <dan@codefresh.io>

Signed-off-by: Dan Garfield <dan@codefresh.io>

* chore: fix linter issues

Signed-off-by: pashakostohrys <pavel@codefresh.io>

---------

Signed-off-by: pashakostohrys <pavel@codefresh.io>
Signed-off-by: Dan Garfield <dan@codefresh.io>
Co-authored-by: Dan Garfield <dan@codefresh.io>
  • Loading branch information
pasha-codefresh and todaywasawesome committed Mar 18, 2024
1 parent ab7e45d commit cebb653
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 15 deletions.
36 changes: 21 additions & 15 deletions util/session/sessionmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const (
// Maximum length of username, too keep the cache's memory signature low
maxUsernameLength = 32
// The default maximum session cache size
defaultMaxCacheSize = 1000
defaultMaxCacheSize = 10000
// The default number of maximum login failures before delay kicks in
defaultMaxLoginFailures = 5
// The default time in seconds for the failure window
Expand Down Expand Up @@ -310,6 +310,22 @@ func expireOldFailedAttempts(maxAge time.Duration, failures *map[string]LoginAtt
return expiredCount
}

// Protect admin user from login attempt reset caused by attempts to overflow cache in a brute force attack. Instead remove random non-admin to make room in cache.
func pickRandomNonAdminLoginFailure(failures map[string]LoginAttempts, username string) *string {
idx := rand.Intn(len(failures) - 1)
i := 0
for key := range failures {
if i == idx {
if key == common.ArgoCDAdminUsername || key == username {
return pickRandomNonAdminLoginFailure(failures, username)
}
return &key
}
i++
}
return nil
}

// Updates the failure count for a given username. If failed is true, increases the counter. Otherwise, sets counter back to 0.
func (mgr *SessionManager) updateFailureCount(username string, failed bool) {

Expand All @@ -327,23 +343,13 @@ func (mgr *SessionManager) updateFailureCount(username string, failed bool) {
// prevent overbloating the cache with fake entries, as this could lead to
// memory exhaustion and ultimately in a DoS. We remove a single entry to
// replace it with the new one.
//
// Chances are that we remove the one that is under active attack, but this
// chance is low (1:cache_size)
if failed && len(failures) >= getMaximumCacheSize() {
log.Warnf("Session cache size exceeds %d entries, removing random entry", getMaximumCacheSize())
idx := rand.Intn(len(failures) - 1)
var rmUser string
i := 0
for key := range failures {
if i == idx {
rmUser = key
delete(failures, key)
break
}
i++
rmUser := pickRandomNonAdminLoginFailure(failures, username)
if rmUser != nil {
delete(failures, *rmUser)
log.Infof("Deleted entry for user %s from cache", *rmUser)
}
log.Infof("Deleted entry for user %s from cache", rmUser)
}

attempt, ok := failures[username]
Expand Down
39 changes: 39 additions & 0 deletions util/session/sessionmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1173,3 +1173,42 @@ requestedScopes: ["oidc"]`, oidcTestServer.URL),
assert.ErrorIs(t, err, common.TokenVerificationErr)
})
}

func Test_PickFailureAttemptWhenOverflowed(t *testing.T) {
t.Run("Not pick admin user from the queue", func(t *testing.T) {
failures := map[string]LoginAttempts{
"admin": {
FailCount: 1,
},
"test2": {
FailCount: 1,
},
}

// inside pickRandomNonAdminLoginFailure, it uses random, so we need to test it multiple times
for i := 0; i < 1000; i++ {
user := pickRandomNonAdminLoginFailure(failures, "test")
assert.Equal(t, "test2", *user)
}
})

t.Run("Not pick admin user and current user from the queue", func(t *testing.T) {
failures := map[string]LoginAttempts{
"test": {
FailCount: 1,
},
"admin": {
FailCount: 1,
},
"test2": {
FailCount: 1,
},
}

// inside pickRandomNonAdminLoginFailure, it uses random, so we need to test it multiple times
for i := 0; i < 1000; i++ {
user := pickRandomNonAdminLoginFailure(failures, "test")
assert.Equal(t, "test2", *user)
}
})
}

0 comments on commit cebb653

Please sign in to comment.