mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-11-05 01:23:24 -08:00
[Server] Inform client of login/registration rate limit timeout (#7468)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Desktop / Prune caches (push) Waiting to run
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Desktop / Prune caches (push) Waiting to run
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
* [Security] Add per-address rate limiting for auth endpoints Introduce a thread-safe RateLimiter that tracks attempts per key (IP address) within a sliding time window, and wire it into the authentication endpoints: - Login: failed login attempts from an address are counted; once the configured maximum is exceeded within the window, further logins from that address are rejected with RespTooManyRequests. A successful login clears the failed attempts for that address. - Registration: implement the previously stubbed tooManyRegistrationAttempts, limiting how many accounts can be created per address per window. - Forgot-password: throttle both the email-request and the email-challenge paths per address. New [security] settings with defaults: max_login_attempts_per_ip=5 / login_attempt_window_seconds=900 max_registrations_per_ip=2 / registration_window_seconds=3600 max_forgot_password_requests_per_ip=3 / forgot_password_window_seconds=3600 Adds unit tests for the RateLimiter (window limit, over-limit blocking, clearing, per-key independence). Took 3 minutes * [Server] Fix rate limiter: disable semantics, draining window, bounded keys, pre-work login gate - recordAttempt()/isBlocked() return false when maxAttempts <= 0 or windowSeconds <= 0, so the documented 'set to 0 to disable' actually disables instead of blocking every address permanently - Keys at their limit are rejected without recording, so repeated attempts at the attacker's pace cannot keep sliding the window and holding the lockout open; a block now clears once the recorded attempts age out of windowSeconds - Attempts map is bounded: an opportunistic prune (every 60s) erases keys whose attempts have fully aged out - Login is throttled before the work: isLoginRateLimited() (a new virtual on Server, backed by isBlocked()) gates loginUser(), so a locked-out address no longer burns a database round trip and password verification per attempt - recordAttemptAt()/isBlockedAt() time seams make the sliding-window behaviour deterministically testable; tests rewritten for the real semantics (no reliance on the maxAttempts=0 bug, no sleeps) * [Server] Inform client of login/registration rate limit timeout - Add retryAfterSeconds/retryAfterSecondsAt to RateLimiter to compute seconds until oldest attempt exits the sliding window - Expose loginRateLimitRemainingSeconds via Server/Servatrice - Include denied_end_time (absolute epoch) in RespTooManyRequests for login pre-auth and when transitioning to lockout after failed attempt - Include denied_end_time in registration rate limit response - Client: show retry time for login RespTooManyRequests - Client: simplify registration RespTooManyRequests message to show retry time when provided - Add test for retryAfterSecondsAt * [Client] Restore registration guidance and drop duplicated rate limit reason The registration RespTooManyRequests case now only shows the retry-after copy when the server actually sent a denied_end_time; the per-email account cap path (no end time) falls back to the previous guidance about already having an account and contacting the server operator. Drop the canned denied_reason_str from the new login rate limit branches so the client no longer prints the same sentence twice. * [Server] Exempt trusted_sources from auth rate limits Addresses in security/trusted_sources bypass per-address limits, matching the existing max_users_per_address bypass in initSession. Without this, Webatrice behind local nginx (all clients as 127.0.0.1) would lock out every web user after 5 mistyped passwords, and cap the whole server at 2 registrations / 3 forgot-password requests per window. Adds Servatrice::isRateLimitExempt helper; recordFailedLogin, isLoginRateLimited, recordRegistrationAttempt and recordForgotPasswordAttempt all check it. # Conflicts: # servatrice/src/servatrice.cpp * [Server] Key login rate limit by (address, username) A successful login cleared the whole per-IP failure history, so 4 bad guesses plus 1 self-login (or guest UnknownUser login) from the same address bypassed the limiter. Failures are now bucketed per (address, lowercase username) and a success clears only its own pair. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
parent
213ce691dd
commit
456d9d9e70
12 changed files with 600 additions and 9 deletions
|
|
@ -187,6 +187,39 @@ public:
|
|||
virtual void observeGameStartDurationMs(qint64 /* elapsedMs */)
|
||||
{
|
||||
}
|
||||
/**
|
||||
* @brief Record a failed login attempt for the (address, username) pair; returns true if now locked out.
|
||||
*
|
||||
* Keyed by pair so a successful login as one user cannot wipe out another
|
||||
* user's failure history from the same address (e.g. a guest login or the
|
||||
* attacker's own account clearing the way for further guessing).
|
||||
*/
|
||||
virtual bool recordFailedLogin(const QString & /*ipAddress*/, const QString & /*userName*/)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
/**
|
||||
* @brief True if the (address, username) pair is already locked out of logging in.
|
||||
*
|
||||
* Consulted before any authentication work (database round trip, password
|
||||
* verification) so a blocked pair cannot burn server CPU per attempt.
|
||||
*/
|
||||
virtual bool isLoginRateLimited(const QString & /*ipAddress*/, const QString & /*userName*/)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
/**
|
||||
* @brief Seconds remaining before the login rate limit for the (address, username) pair lifts.
|
||||
* Returns 0 if not limited.
|
||||
*/
|
||||
virtual int loginRateLimitRemainingSeconds(const QString & /*ipAddress*/, const QString & /*userName*/)
|
||||
{
|
||||
return 0;
|
||||
}
|
||||
/** @brief Clear any failed-login lockout for the (address, username) pair, e.g. after a successful login. */
|
||||
virtual void clearFailedLogins(const QString & /*ipAddress*/, const QString & /*userName*/)
|
||||
{
|
||||
}
|
||||
|
||||
Server_DatabaseInterface *getDatabaseInterface() const;
|
||||
int getNextLocalGameId()
|
||||
|
|
|
|||
|
|
@ -592,6 +592,17 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd
|
|||
QString reasonStr;
|
||||
int banSecondsLeft = 0;
|
||||
QString connectionType = getConnectionType();
|
||||
// Throttle before doing any work: a locked-out address must not get a full
|
||||
// database round trip and password verification on every attempt.
|
||||
if (server->isLoginRateLimited(getAddress(), userName)) {
|
||||
int remaining = server->loginRateLimitRemainingSeconds(getAddress(), userName);
|
||||
if (remaining > 0) {
|
||||
auto *re = new Response_Login;
|
||||
re->set_denied_end_time(QDateTime::currentDateTime().addSecs(remaining).toSecsSinceEpoch());
|
||||
rc.setResponseExtension(re);
|
||||
}
|
||||
return Response::RespTooManyRequests;
|
||||
}
|
||||
AuthenticationResult res = server->loginUser(this, userName, password, needsHash, reasonStr, banSecondsLeft,
|
||||
clientId, clientVersion, connectionType);
|
||||
switch (res) {
|
||||
|
|
@ -605,10 +616,20 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd
|
|||
return Response::RespUserIsBanned;
|
||||
}
|
||||
case NotLoggedIn:
|
||||
if (server->recordFailedLogin(getAddress(), userName)) {
|
||||
int remaining = server->loginRateLimitRemainingSeconds(getAddress(), userName);
|
||||
if (remaining > 0) {
|
||||
auto *re = new Response_Login;
|
||||
re->set_denied_end_time(QDateTime::currentDateTime().addSecs(remaining).toSecsSinceEpoch());
|
||||
rc.setResponseExtension(re);
|
||||
}
|
||||
return Response::RespTooManyRequests;
|
||||
}
|
||||
return Response::RespWrongPassword;
|
||||
case WouldOverwriteOldSession:
|
||||
return Response::RespWouldOverwriteOldSession;
|
||||
case UsernameInvalid: {
|
||||
server->recordFailedLogin(getAddress(), userName);
|
||||
auto *re = new Response_Login;
|
||||
re->set_denied_reason_str(reasonStr.toStdString());
|
||||
rc.setResponseExtension(re);
|
||||
|
|
@ -619,10 +640,12 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd
|
|||
case ClientIdRequired:
|
||||
return Response::RespClientIdRequired;
|
||||
case UserIsInactive:
|
||||
server->recordFailedLogin(getAddress(), userName);
|
||||
return Response::RespAccountNotActivated;
|
||||
case PasswordChangeRequired:
|
||||
return Response::RespPasswordChangeRequired;
|
||||
default:
|
||||
server->clearFailedLogins(getAddress(), userName);
|
||||
authState = res;
|
||||
usingRealPassword = needsHash;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue