feat(auth): per-account brute-force lockout with exponential backoff
Existing protection was per-IP only (login_attempts table). On a loopback
deployment everyone hits 127.0.0.1, so the per-IP counter is mostly
ornamental — the real attacker is on the same machine. Adds a second
defense layer that tracks failures per username with an exponential
backoff schedule.
Schema:
account_lockouts (username TEXT PK, failed_count INT,
locked_until DATETIME, last_attempt_at, last_attempt_ip)
Backoff after threshold (4+ failures):
1, 2, 3 failures → no lockout (grace window for typos)
4th → 60 s
5th → 5 min
6th → 15 min
7th → 1 h
8th → 6 h
9th and beyond → 24 h (capped)
Counter resets to 0 on successful login or reauth. Old non-locked rows
older than 30 days are pruned by CleanupExpired alongside the existing
sessions / audit_log / login_attempts cleanups.
Wiring:
- HandleLogin / HandleReauth both check RejectIfAccountLocked() before
touching the users table. Lockout responses are 429 with JSON body
{ error, retry_after } and a Retry-After header.
- Failed attempts are recorded against the username even when the user
doesn't exist, preventing account enumeration via differential
"is this account locked?" probes.
- PBKDF2 hash comparison was already constant-time (ConstantTimeEquals);
no change there.
Client (js/app.js):
- api() now preserves response status + body on Error so callers can
distinguish 429-lockout from other errors.
- New showLockoutCountdown(seconds) renders a live "Account locked —
try again in Xm Ys" message in #authHint, disables #loginBtn until
the countdown reaches 0, then re-enables it.
- doLogin / doUnlock both branch on err.status === 429 + retry_after
to call showLockoutCountdown instead of a generic error toast.
Known limitation: an attacker can DoS-lock arbitrary usernames by
spamming /login with that name. This is intentional — the alternative
(per-(username,IP) tracking) would let attackers enumerate accounts.
DoS-lock is acceptable; auth bypass is not.
This commit is contained in:
@@ -171,6 +171,11 @@ begin
|
||||
LBody.Free;
|
||||
end;
|
||||
|
||||
// Per-username lockout check — runs BEFORE touching the users table, so
|
||||
// attackers can't probe account existence via timing differences between
|
||||
// "locked" and "not found" responses.
|
||||
if RejectIfAccountLocked(AResponse, LUser) then Exit;
|
||||
|
||||
DB.Lock;
|
||||
try
|
||||
LQ := TFDQuery.Create(nil);
|
||||
@@ -182,7 +187,13 @@ begin
|
||||
LQ.Open;
|
||||
if LQ.IsEmpty then
|
||||
begin
|
||||
// Unknown username — still record the failure against this username
|
||||
// so attackers can't enumerate accounts by observing which usernames
|
||||
// can be locked vs not. TCriticalSection is reentrant for the same
|
||||
// thread, so calling RecordAttempt/RecordFailedAccountAttempt from
|
||||
// inside our DB.Lock block is safe (they re-acquire the same lock).
|
||||
RecordAttempt(LIP);
|
||||
RecordFailedAccountAttempt(LUser, LIP);
|
||||
TJSONHelper.SendError(AResponse, 401, 'Invalid credentials');
|
||||
Exit;
|
||||
end;
|
||||
@@ -208,6 +219,7 @@ begin
|
||||
begin
|
||||
// Not implemented in Delphi backend yet
|
||||
RecordAttempt(LIP);
|
||||
RecordFailedAccountAttempt(LUser, LIP);
|
||||
LogAudit(LUserId, 'failed_login_bcrypt', LIP);
|
||||
TJSONHelper.SendError(AResponse, 501,
|
||||
'This account was created with bcrypt (PHP). The Delphi backend does ' +
|
||||
@@ -218,12 +230,14 @@ begin
|
||||
if not LValid then
|
||||
begin
|
||||
RecordAttempt(LIP);
|
||||
RecordFailedAccountAttempt(LUser, LIP);
|
||||
LogAudit(LUserId, 'failed_login', LIP);
|
||||
TJSONHelper.SendError(AResponse, 401, 'Invalid credentials');
|
||||
Exit;
|
||||
end;
|
||||
|
||||
ClearAttempts(LIP);
|
||||
ClearAccountLockout(LUser);
|
||||
DeleteAllUserSessions(LUserId);
|
||||
EnsureDefaultFolders(LUserId);
|
||||
CreateSession(LUserId, LToken, LCSRF);
|
||||
@@ -264,7 +278,7 @@ procedure HandleReauth(ARequest: TIdHTTPRequestInfo;
|
||||
var
|
||||
LUserId: Integer;
|
||||
LBody: TJSONObject;
|
||||
LPwd, LStoredHash, LSalt, LAlgo, LIP, LComputed: string;
|
||||
LUser, LPwd, LStoredHash, LSalt, LAlgo, LIP, LComputed: string;
|
||||
LQ: TFDQuery;
|
||||
LValid: Boolean;
|
||||
begin
|
||||
@@ -294,7 +308,9 @@ begin
|
||||
LQ := TFDQuery.Create(nil);
|
||||
try
|
||||
LQ.Connection := DB.Connection;
|
||||
LQ.SQL.Text := 'SELECT password_hash, salt, hash_algo FROM users WHERE id = :uid';
|
||||
// Pull username too — needed for the per-account lockout calls.
|
||||
LQ.SQL.Text :=
|
||||
'SELECT username, password_hash, salt, hash_algo FROM users WHERE id = :uid';
|
||||
LQ.ParamByName('uid').AsInteger := LUserId;
|
||||
LQ.Open;
|
||||
if LQ.IsEmpty then
|
||||
@@ -303,6 +319,7 @@ begin
|
||||
TJSONHelper.SendError(AResponse, 401, 'User not found');
|
||||
Exit;
|
||||
end;
|
||||
LUser := LQ.FieldByName('username').AsString;
|
||||
LStoredHash := LQ.FieldByName('password_hash').AsString;
|
||||
LSalt := LQ.FieldByName('salt').AsString;
|
||||
LAlgo := LQ.FieldByName('hash_algo').AsString;
|
||||
@@ -314,6 +331,12 @@ begin
|
||||
DB.Unlock;
|
||||
end;
|
||||
|
||||
// Check account lockout AFTER we have the username. Even though the user
|
||||
// is already authenticated by their session token, the master-pw re-prompt
|
||||
// is itself brute-forceable (e.g. attacker hijacked a session and now tries
|
||||
// to escalate by guessing the master pw to unlock the JS crypto key).
|
||||
if RejectIfAccountLocked(AResponse, LUser) then Exit;
|
||||
|
||||
LValid := False;
|
||||
if SameText(LAlgo, 'pbkdf2') then
|
||||
begin
|
||||
@@ -324,12 +347,14 @@ begin
|
||||
if not LValid then
|
||||
begin
|
||||
RecordAttempt(LIP);
|
||||
RecordFailedAccountAttempt(LUser, LIP);
|
||||
LogAudit(LUserId, 'failed_reauth', LIP);
|
||||
TJSONHelper.SendError(AResponse, 401, 'Invalid password');
|
||||
Exit;
|
||||
end;
|
||||
|
||||
ClearAttempts(LIP);
|
||||
ClearAccountLockout(LUser);
|
||||
LogAudit(LUserId, 'reauth', LIP);
|
||||
TJSONHelper.SendOK(AResponse, 'OK');
|
||||
end;
|
||||
|
||||
Reference in New Issue
Block a user