From 0a0efd911521499f476e4d2b60abe25e5a293c59 Mon Sep 17 00:00:00 2001 From: Daniel Redetzke Date: Mon, 5 Oct 2026 00:23:05 +0300 Subject: [PATCH] Cap concurrent password checks and count attempts before checking Every argon2 run takes 64 MiB and nothing limited how many ran at once, so parallel sign-in attempts could run the server out of memory (8 at once used about 600 MB). At most two now run at once; at most 16 sign-ins wait for one, more get HTTP 429. 30 parallel sign-ins peaked at 275 MB. A sign-in attempt now counts toward the lockout before its password is checked, so parallel attempts cannot get past it; a right password takes its own attempt back. IPv6 addresses are locked out by /64. --- api.go | 2 +- auth.go | 86 +++++++++++++++++++++++++++++++++++++--------------- main_test.go | 55 +++++++++++++++++++++++++++++++++ mfa.go | 27 ++++++++++++----- 4 files changed, 136 insertions(+), 34 deletions(-) diff --git a/api.go b/api.go index 8197757..ebef872 100644 --- a/api.go +++ b/api.go @@ -232,7 +232,7 @@ func (a *App) login(w http.ResponseWriter, r *http.Request) { if err != nil { slog.Warn("login failed", "user", in.Username, "remote", ip, "reason", err.Error()) code := http.StatusUnauthorized - if errors.Is(err, errLocked) { + if errors.Is(err, errLocked) || errors.Is(err, errBusy) { code = http.StatusTooManyRequests } writeJSON(w, code, map[string]string{"error": err.Error()}) diff --git a/auth.go b/auth.go index df3660f..60e4c19 100644 --- a/auth.go +++ b/auth.go @@ -10,6 +10,7 @@ import ( "fmt" "net" "net/http" + "net/netip" "strings" "sync" "time" @@ -26,12 +27,23 @@ const ( argonKeyLen = 32 ) +// Every argon2 run takes argonMemory (64 MiB). argonSlots caps how many run +// at once, so a burst of sign-ins cannot run the server out of memory: two +// slots are 128 MiB at most. +var argonSlots = make(chan struct{}, 2) + +func argonKey(pw, salt []byte, t, m uint32, p uint8, n uint32) []byte { + argonSlots <- struct{}{} + defer func() { <-argonSlots }() + return argon2.IDKey(pw, salt, t, m, p, n) +} + func hashPassword(pw string) (string, error) { salt := make([]byte, 16) if _, err := rand.Read(salt); err != nil { return "", err } - key := argon2.IDKey([]byte(pw), salt, argonTime, argonMemory, argonThreads, argonKeyLen) + key := argonKey([]byte(pw), salt, argonTime, argonMemory, argonThreads, argonKeyLen) b64 := base64.RawStdEncoding return fmt.Sprintf("$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s", argon2.Version, argonMemory, argonTime, argonThreads, b64.EncodeToString(salt), b64.EncodeToString(key)), nil @@ -54,7 +66,7 @@ func verifyPassword(encoded, pw string) bool { if err1 != nil || err2 != nil { return false } - got := argon2.IDKey([]byte(pw), salt, t, m, p, uint32(len(want))) + got := argonKey([]byte(pw), salt, t, m, p, uint32(len(want))) return subtle.ConstantTimeCompare(got, want) == 1 } @@ -139,14 +151,18 @@ type Auth struct { mu sync.Mutex sessions map[string]*session used map[string]tokenUse - logins map[string]tokenUse // last sign-in per user ID - fails map[string]*failState + logins map[string]tokenUse // last sign-in per user ID + fails map[string]*failState // by lockKey + waiting int // sign-ins waiting for or running a password check mfa mfaState } const ( maxFailures = 5 lockoutTime = 15 * time.Minute + // maxWaiting sign-ins may wait for a password check; more are turned + // away until the queue is shorter. + maxWaiting = 16 ) func newAuth(s *Store) *Auth { @@ -155,23 +171,51 @@ func newAuth(s *Store) *Auth { func cookieName() string { return appName + "_session" } -var errLocked = errors.New("too many failed attempts, try again later") +var ( + errLocked = errors.New("too many failed attempts, try again later") + errBusy = errors.New("too many sign-ins at once, try again in a moment") +) + +// lockKey is what failed sign-ins are counted by: the IPv4 address, or the +// /64 network of an IPv6 address, since one device can pick any address in +// its /64. +func lockKey(ip string) string { + a, err := netip.ParseAddr(ip) + if err != nil || a.Unmap().Is4() { + return ip + } + p, _ := a.Prefix(64) + return p.String() +} // Login checks the credentials and returns a new session id, or, for a user // with two-step sign-in, a ticket for the second step. func (a *Auth) Login(user, pw, ip string) (sessionID, ticket string, err error) { - a.mu.Lock() - f := a.fails[ip] - if f != nil && time.Now().Before(f.until) { - a.mu.Unlock() - return "", "", errLocked - } - a.mu.Unlock() - cfg := a.store.Get() if !cfg.passwordSet() { return "", "", errors.New("no password is set; run: " + appName + " passwd") } + // The attempt counts as failed before the password is checked, so + // parallel attempts cannot get past the lockout; a right password takes + // it back. + a.mu.Lock() + if a.lockedLocked(ip) { + a.mu.Unlock() + return "", "", errLocked + } + if a.waiting >= maxWaiting { + a.mu.Unlock() + return "", "", errBusy + } + a.waiting++ + undo := a.failLocked(ip) + a.mu.Unlock() + defer func() { + a.mu.Lock() + a.waiting-- + a.mu.Unlock() + }() + // An unknown username costs as much time as a wrong password, so the // answer time does not tell which usernames exist. u := cfg.userByName(strings.TrimSpace(user)) @@ -185,21 +229,13 @@ func (a *Auth) Login(user, pw, ip string) (sessionID, ticket string, err error) a.mu.Lock() defer a.mu.Unlock() if !okUser || !okPw { - if f == nil { - f = &failState{} - a.fails[ip] = f - } - f.count++ - if f.count >= maxFailures { - f.count = 0 - f.until = time.Now().Add(lockoutTime) - } return "", "", errors.New("wrong username or password") } + undo() if u.hasMFA() { return "", a.newTicketLocked(u, ip), nil } - delete(a.fails, ip) + delete(a.fails, lockKey(ip)) a.logins[u.ID] = tokenUse{At: time.Now(), IP: ip} return a.newSessionLocked(cfg, u, sessionInfo{Started: time.Now(), IP: ip}), "", nil } @@ -321,9 +357,9 @@ func (a *Auth) sweep() { delete(a.sessions, id) } } - for ip, f := range a.fails { + for key, f := range a.fails { if now.After(f.until) && f.count == 0 { - delete(a.fails, ip) + delete(a.fails, key) } } for id, t := range a.mfa.tickets { diff --git a/main_test.go b/main_test.go index 6c2fe0a..2a369be 100644 --- a/main_test.go +++ b/main_test.go @@ -14,6 +14,7 @@ import ( "path/filepath" "slices" "strings" + "sync" "testing" "time" ) @@ -409,6 +410,60 @@ func TestSysctlConf(t *testing.T) { } } +func TestLoginLockout(t *testing.T) { + store, err := openStore(filepath.Join(t.TempDir(), "config.json")) + if err != nil { + t.Fatal(err) + } + hash, _ := hashPassword("a long test password") + _ = store.Update(func(c *Config) error { c.Users[0].PasswordHash = hash; return nil }) + a := newAuth(store) + const right, wrong = "a long test password", "a wrong password" + + // Ten wrong attempts at once from one /64: five are checked, the others + // are locked out before any password check. + var wg sync.WaitGroup + var mu sync.Mutex + got := map[string]int{} + for i := range 10 { + wg.Add(1) + go func() { + defer wg.Done() + _, _, err := a.Login("admin", wrong, fmt.Sprintf("2001:db8::%x", i+1)) + mu.Lock() + got[err.Error()]++ + mu.Unlock() + }() + } + wg.Wait() + if got["wrong username or password"] != 5 || got[errLocked.Error()] != 5 { + t.Fatalf("parallel attempts: %v", got) + } + if _, _, err := a.Login("admin", right, "2001:db8::ffff"); !errors.Is(err, errLocked) { + t.Fatalf("same /64: %v, want locked", err) + } + if _, _, err := a.Login("admin", right, "2001:db8:0:1::1"); err != nil { + t.Fatalf("other /64: %v", err) + } + + // A right password takes its own attempt back. With two-step sign-in + // the earlier failures stay, so wrong codes still lead to the lockout. + _ = store.Update(func(c *Config) error { c.Users[0].MFA = &UserMFA{TOTPSecret: newTOTPSecret()}; return nil }) + ip := "192.0.2.7" + for range maxFailures - 1 { + _, _, _ = a.Login("admin", wrong, ip) + } + if _, tk, err := a.Login("admin", right, ip); err != nil || tk == "" { + t.Fatalf("5th attempt, right password: ticket %q, %v", tk, err) + } + if _, _, err := a.Login("admin", wrong, ip); err == nil || errors.Is(err, errLocked) { + t.Fatalf("6th attempt: %v, want wrong password", err) + } + if _, _, err := a.Login("admin", right, ip); !errors.Is(err, errLocked) { + t.Fatalf("7th attempt: %v, want locked", err) + } +} + func TestWriteIfChanged(t *testing.T) { p := filepath.Join(t.TempDir(), "x.conf") if ch, err := writeIfChanged(p, "a\n", 0o644); !ch || err != nil { diff --git a/mfa.go b/mfa.go index 1b346fc..3dad0a8 100644 --- a/mfa.go +++ b/mfa.go @@ -236,23 +236,34 @@ func newMFAState() mfaState { var errBadTicket = errors.New("the sign-in expired; enter your password again") -// failLocked counts a failed attempt from ip toward the lockout. a.mu must -// be held. -func (a *Auth) failLocked(ip string) { - f := a.fails[ip] +// failLocked counts a failed attempt from ip toward the lockout and returns +// a function that takes it back, for an attempt counted before it was +// checked. a.mu must be held, also when calling undo. +func (a *Auth) failLocked(ip string) (undo func()) { + key := lockKey(ip) + f := a.fails[key] if f == nil { f = &failState{} - a.fails[ip] = f + a.fails[key] = f } f.count++ - if f.count >= maxFailures { + locked := f.count >= maxFailures + if locked { f.count = 0 f.until = time.Now().Add(lockoutTime) } + return func() { + switch { + case locked: + f.count, f.until = maxFailures-1, time.Time{} + case f.count > 0: + f.count-- + } + } } func (a *Auth) lockedLocked(ip string) bool { - f := a.fails[ip] + f := a.fails[lockKey(ip)] return f != nil && time.Now().Before(f.until) } @@ -302,7 +313,7 @@ func (a *Auth) finishSignIn(u *User, ip string) string { cfg := a.store.Get() a.mu.Lock() defer a.mu.Unlock() - delete(a.fails, ip) + delete(a.fails, lockKey(ip)) a.logins[u.ID] = tokenUse{At: time.Now(), IP: ip} return a.newSessionLocked(cfg, u, sessionInfo{Started: time.Now(), IP: ip}) }