[Security] Add challenge-response auth with scrypt verifiers and stop storing plaintext passwords (#7193)

* [Security] Add challenge-response auth with scrypt verifiers and stop storing plaintext passwords

Challenge-response authentication: the client derives a scrypt verifier
(RFC 7914, EVP_PBE_scrypt, N=32768, r=8, p=1) and authenticates with
HMAC-SHA256(key, nonce), so neither the password nor its hash is
transmitted. The stored format becomes
"$scrypt$<n>$<r>$<p>$<salt>$<verifier>" and Response_PasswordSalt
now carries the cost parameters. Strict servers only accept scrypt
verifiers; legacy accounts are migrated after a successful login.

Fix #344 for challenge-response servers: a saved profile stores the
derived verifier under the password key instead of the plaintext password.
The connect dialog loads it without revealing it, autoconnect passes it
through, the change-password dialog no longer prefills the old password
field with it, and the client only persists the verifier when
"Save password" is checked.

Took 3 minutes

Took 1 minute

Took 10 seconds

Took 7 minutes

* [Security] Harden credential format acceptance and challenge nonce validation

* [Security] Refuse KDF downgrade; trim profile name and warn on silent credential drop

* [Security] Add SupportsAll ServerOptions enumerator to fix enum validation assert

The server advertised both SupportsPasswordHash (1) and
SupportsChallengeResponseAuth (2) via a bitwise-OR that produced the
value 3, which no enumerator declared. Protobuf's proto2 enum
validation rejects the value in _internal_set_server_options, crashing
servatrice on the debug assertion. Add a SupportsAll = 3 enumerator with
an identical wire value so the bitmask reads unchanged while the value
is now a documented, valid member of the enum.

* [Security] Normalize username when issuing challenge nonce

cmdRequestPasswordSalt bound the challenge nonce to the raw, un-simplified
username while cmdLogin verifies it against the simplified name. A user
who typed leading or trailing whitespace in the username box would be
silently rejected during challenge-response login even with the correct
password. Simplify at the nonce-issuing site so both commands agree on
the normalized name.

Also drive-by formatting: collapse the now single-expression
serverOptions initializer onto one line.

* [Security] Keep an encrypted legacy password backup for old-client logins

Migrating an account to scrypt stranded pre-challenge clients, which can
only present the legacy salt+hash: after migration the server served them
a scrypt salt they cannot use, so the only recovery was a password reset.

When security/legacy_backup_key is configured (64 hex chars,
openssl rand -hex 32), submitPasswordVerifier now seals the legacy
credential with AES-256-GCM under that key before overwriting the row,
storing it in password_legacy_backup with the time it was last used. Old
clients keep working: cmdRequestPasswordSalt serves the backup's 16-char
salt in password_salt so they can derive the legacy hash they were
always able to compute, while new clients derive the verifier from the
new scrypt_salt field, which always carries the verifier salt. The
non-challenge login path accepts the plaintext or legacy hash against
the decrypted backup and refreshes its window; a migrated row with no
usable backup fails closed.

Backups age out so a database dump taken after the window closes never
carries a crackable legacy hash: getUsableLegacyBackup purges on first
use past security/legacy_backup_ttl_days (default 90, clamped 1..3650),
and a six-hourly sweep drops backups idle accounts never touch.
changeUserPassword clears the backup since a new credential invalidates
it. Without a configured key nothing is sealed or purged, so a
temporarily missing key cannot destroy existing backups.

The AAD is the simplified user name on both sides, sealing the blob to
its row so it cannot be moved between accounts. Sealing failures
degrade to the previous lockout behaviour rather than blocking
migration. The columns extend the unmerged 0036->0037 migration, so the
schema version stays 37.

* [Security] Address review: proto compat, plaintext verify, AAD case, strict scope

Fix the two blockers and follow-ups from the 2026-10-16 review:

- Advertise challenge-response in a new supports_challenge_response
  field instead of the ServerOptions enum and drop SupportsAll:
  proto2 enums are closed, so already-released clients parsed
  server_options = 3 as NoOptions and fell back to plaintext
  passwords. server_options stays exactly SupportsPasswordHash
  on the wire; old clients ignore the new field.
- Verify a plaintext password directly against a migrated scrypt
  row with verifyPassword() so email changes (cmdAccountEdit)
  and old clients sending password keep working without a
  legacy backup key. Pre-hashed values are still checked
  against the backup, and its TTL window is refreshed only
  when the backup is what actually matched.
- Use the stored row name as the AES-GCM AAD on both seal
  and open so a login whose case differs from the stored
  name (utf8mb4_unicode_ci) still opens its backup.
- Require challenge-response in strict mode only for
  registered SQL accounts (userExists gate), leaving
  guest/unregistered logins and none/password auth
  methods unaffected.
- Document in servatrice.ini.example that a captured
  legacy hash can take over a not-yet-migrated account
  and that strict only constrains registered SQL
  accounts. Schema version is 38.

* [Security] Raise legacy backup TTL default to 180 days

Gives operators and slow-to-upgrade users a longer downgrade
window for pre-challenge clients before unused encrypted
legacy password backups age out.

* [Security] Fix servatrice.cpp formatting per clang-format-18

* [Security] Add missing forward declaration for Command_SubmitPasswordVerifier

The cmdSubmitPasswordVerifier declaration references the proto-generated
type, which otherwise does not name a type in this header and breaks
every C++ build.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: BruebachL <bruebachl@cockatrice.example>

libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp

@@ -41,7 +41,8 @@
 #include <libcockatrice/protocol/pending_command.h>
 
 AbstractClient::AbstractClient(QObject *parent)
-    : QObject(parent), nextCmdId(0), status(StatusDisconnected), serverSupportsPasswordHash(false)
+    : QObject(parent), nextCmdId(0), status(StatusDisconnected), serverSupportsPasswordHash(false),
+      serverSupportsChallengeResponse(false)
 {
     qRegisterMetaType<QVariant>("QVariant");
     qRegisterMetaType<CommandContainer>("CommandContainer");

libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h

@@ -133,6 +133,7 @@ protected:
     QMap<int, PendingCommand *> pendingCommands;
     QString userName, password, email, country, realName, token;
     bool serverSupportsPasswordHash;
+    bool serverSupportsChallengeResponse;
     void setStatus(ClientStatus _status);
     int getNewCmdId()
     {
@@ -166,6 +167,10 @@ public:
     {
         return serverSupportsPasswordHash;
     }
+    bool getServerSupportsChallengeResponse() const
+    {
+        return serverSupportsChallengeResponse;
+    }
     const QString &getUserName() const
     {
         return userName;

libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp

@@ -42,7 +42,8 @@ static const unsigned int protocolVersion = 14;
 
 RemoteClient::RemoteClient(QObject *parent, INetworkSettingsProvider *_networkSettingsProvider)
     : AbstractClient(parent), networkSettingsProvider(_networkSettingsProvider), timeRunning(0), lastDataReceived(0),
-      messageInProgress(false), handshakeStarted(false), usingWebSocket(false), messageLength(0), hashedPassword()
+      messageInProgress(false), handshakeStarted(false), usingWebSocket(false), messageLength(0), hashedPassword(),
+      passwordNeedsMigration(false)
 {
 
     clearNewClientFeatures();
@@ -129,6 +130,11 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica
         return;
     }
     serverSupportsPasswordHash = event.server_options() & Event_ServerIdentification::SupportsPasswordHash;
+    // Challenge-response is advertised in its own field so server_options stays
+    // exactly SupportsPasswordHash on the wire for already-released clients
+    // (proto2 enums are closed: an unknown value parses as NoOptions and those
+    // clients would fall back to plaintext passwords).
+    serverSupportsChallengeResponse = event.has_supports_challenge_response() && event.supports_challenge_response();
 
     if (getStatus() == StatusRequestingForgotPassword) {
         Command_ForgotPasswordRequest cmdForgotPasswordRequest;
@@ -145,7 +151,13 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica
         cmdForgotPasswordReset.set_user_name(userName.toStdString());
         cmdForgotPasswordReset.set_clientid(getSrvClientID(lastHostname).toStdString());
         cmdForgotPasswordReset.set_token(token.toStdString());
-        if (!password.isEmpty() && serverSupportsPasswordHash) {
+        if (!password.isEmpty() && serverSupportsChallengeResponse) {
+            hashedPassword = PasswordHasher::generatePasswordVerifier(password);
+            // Only this branch yields a challenge-response verifier worth persisting; the
+            // legacy-hash and plaintext branches below must not be saved under the password key.
+            derivedVerifier = hashedPassword;
+            cmdForgotPasswordReset.set_hashed_new_password(hashedPassword.toStdString());
+        } else if (!password.isEmpty() && serverSupportsPasswordHash) {
             auto passwordSalt = PasswordHasher::generateRandomSalt();
             hashedPassword = PasswordHasher::computeHash(password, passwordSalt);
             cmdForgotPasswordReset.set_hashed_new_password(hashedPassword.toStdString());
@@ -173,7 +185,10 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica
     if (getStatus() == StatusRegistering) {
         Command_Register cmdRegister;
         cmdRegister.set_user_name(userName.toStdString());
-        if (!password.isEmpty() && serverSupportsPasswordHash) {
+        if (!password.isEmpty() && serverSupportsChallengeResponse) {
+            hashedPassword = PasswordHasher::generatePasswordVerifier(password);
+            cmdRegister.set_hashed_password(hashedPassword.toStdString());
+        } else if (!password.isEmpty() && serverSupportsPasswordHash) {
             auto passwordSalt = PasswordHasher::generateRandomSalt();
             hashedPassword = PasswordHasher::computeHash(password, passwordSalt);
             cmdRegister.set_hashed_password(hashedPassword.toStdString());
@@ -238,7 +253,9 @@ Command_Login RemoteClient::generateCommandLogin()
 
 void RemoteClient::doLogin()
 {
-    if (!password.isEmpty() && serverSupportsPasswordHash) {
+    if ((!password.isEmpty() || !storedVerifier.isEmpty()) && serverSupportsChallengeResponse) {
+        doRequestPasswordSalt(); // ask salt + nonce to build the challenge response
+    } else if (!password.isEmpty() && serverSupportsPasswordHash) {
         //! \todo Store and log in using stored hashed password.
         if (hashedPassword.isEmpty()) {
             doRequestPasswordSalt(); // ask salt to create hashedPassword, then log in
@@ -272,6 +289,30 @@ void RemoteClient::doHashedLogin()
     sendCommand(pend);
 }
 
+void RemoteClient::doSubmitPasswordVerifier()
+{
+    pendingVerifier = PasswordHasher::generatePasswordVerifier(password);
+    Command_SubmitPasswordVerifier cmdSubmitVerifier;
+    cmdSubmitVerifier.set_password_verifier(pendingVerifier.toStdString());
+
+    PendingCommand *pend = prepareSessionCommand(cmdSubmitVerifier);
+    connect(pend, &PendingCommand::finished, this, &RemoteClient::submitPasswordVerifierResponse);
+    sendCommand(pend);
+}
+
+void RemoteClient::submitPasswordVerifierResponse(const Response &response)
+{
+    if (response.response_code() == Response::RespOk) {
+        qCDebug(RemoteClientLog) << "Password verifier migrated successfully";
+        if (!pendingVerifier.isEmpty()) {
+            emit sigPasswordVerifierReady(pendingVerifier);
+            pendingVerifier.clear();
+        }
+    } else {
+        qCWarning(RemoteClientLog) << "Failed to migrate password verifier:" << response.response_code();
+    }
+}
+
 void RemoteClient::processConnectionClosedEvent(const Event_ConnectionClosed & /*event*/)
 {
     doDisconnectFromServer();
@@ -284,7 +325,70 @@ void RemoteClient::passwordSaltResponse(const Response &response)
         auto passwordSalt = QString::fromStdString(resp.password_salt());
         if (passwordSalt.isEmpty()) { // the server does not recognize the user but allows them to enter unregistered
             password.clear();         // the password will not be used
+            storedVerifier.clear();
             doLogin();
+        } else if (serverSupportsChallengeResponse && resp.has_nonce()) {
+            const QByteArray nonce = QByteArray::fromStdString(resp.nonce());
+            // needs_migration is a server-controlled flag over an unauthenticated
+            // transport (plain TCP by default). If we already hold a scrypt
+            // verifier for this account, a server telling us to fall back to the
+            // 1000-round legacy hash is a downgrade of the KDF — refuse it rather
+            // than hand an attacker a weakly-keyed HMAC they can attack offline.
+            if (resp.needs_migration() && PasswordHasher::parsePasswordVerifier(storedVerifier).isValid) {
+                emit loginError(Response::RespClientUpdateRequired,
+                                QStringLiteral("Server asked to downgrade authentication for a migrated account."), 0,
+                                {});
+                return;
+            }
+            QByteArray key;
+            if (resp.needs_migration()) {
+                // The account still uses the legacy format; the legacy full hash
+                // is only derivable from the plaintext password.
+                if (password.isEmpty()) {
+                    emit loginError(Response::RespClientUpdateRequired,
+                                    QStringLiteral("This account must be logged in with its password once."), 0, {});
+                    return;
+                }
+                key = PasswordHasher::computeHash(password, passwordSalt).toUtf8();
+            } else if (!password.isEmpty()) {
+                // A hostile server must not be able to make us run or allocate for
+                // unreasonable scrypt parameters.
+                const int n = resp.has_n() ? resp.n() : SCRYPT_N;
+                const int r = resp.has_r() ? resp.r() : SCRYPT_R;
+                const int p = resp.has_p() ? resp.p() : SCRYPT_P;
+                if (!PasswordHasher::costParamsAreSane(n, r, p)) {
+                    emit loginError(Response::RespClientUpdateRequired,
+                                    QStringLiteral("The server requested unreasonable scrypt cost parameters."), 0, {});
+                    return;
+                }
+                // password_salt may carry a legacy backup salt for pre-challenge clients;
+                // the verifier itself is always derived from scrypt_salt when present.
+                const QString scryptSalt =
+                    resp.has_scrypt_salt() ? QString::fromStdString(resp.scrypt_salt()) : passwordSalt;
+                key = PasswordHasher::deriveKey(password, QByteArray::fromBase64(scryptSalt.toUtf8()), n, r, p);
+                if (key.isEmpty()) {
+                    emit loginError(Response::RespClientUpdateRequired, QStringLiteral("Unable to derive verifier."), 0,
+                                    {});
+                    return;
+                }
+                derivedVerifier =
+                    QString("$scrypt$%1$%2$%3$%4$%5").arg(n).arg(r).arg(p).arg(scryptSalt).arg(QString(key.toBase64()));
+            } else if (!storedVerifier.isEmpty()) {
+                const PasswordVerifier verifier = PasswordHasher::parsePasswordVerifier(storedVerifier);
+                if (!verifier.isValid) {
+                    emit loginError(Response::RespClientUpdateRequired, QStringLiteral("Stored verifier is invalid."),
+                                    0, {});
+                    return;
+                }
+                key = verifier.verifier;
+            } else {
+                emit loginError(Response::RespLoginNeeded, {}, 0, {});
+                return;
+            }
+            passwordNeedsMigration = resp.needs_migration();
+            const QByteArray responseBytes = PasswordHasher::computeResponse(key, nonce);
+            hashedPassword = "$challenge$" + QString(nonce.toBase64()) + "$" + QString(responseBytes.toBase64());
+            doHashedLogin();
         } else {
             hashedPassword = PasswordHasher::computeHash(password, passwordSalt);
             doHashedLogin();
@@ -309,6 +413,14 @@ void RemoteClient::loginResponse(const Response &response)
         setStatus(StatusLoggedIn);
         emit userInfoChanged(resp.user_info());
 
+        if (passwordNeedsMigration) {
+            // The account still used the legacy password format; upgrade it to scrypt.
+            doSubmitPasswordVerifier();
+        } else if (!derivedVerifier.isEmpty()) {
+            emit sigPasswordVerifierReady(derivedVerifier);
+            derivedVerifier.clear();
+        }
+
         QList<ServerInfo_User> buddyList;
         for (int i = resp.buddy_list_size() - 1; i >= 0; --i) {
             buddyList.append(resp.buddy_list(i));
@@ -565,6 +677,8 @@ void RemoteClient::doDisconnectFromServer()
         websocket->close();
     }
     socket->close();
+    derivedVerifier.clear();
+    pendingVerifier.clear();
 }
 
 void RemoteClient::ping()
@@ -736,6 +850,12 @@ void RemoteClient::submitForgotPasswordResetResponse(const Response &response)
 {
     if (response.response_code() == Response::RespOk) {
         emit sigForgotPasswordSuccess();
+        // Persist only a real scrypt verifier; a legacy hash must not be stored under
+        // the password key, where it would break future challenge-response logins.
+        if (!derivedVerifier.isEmpty()) {
+            emit sigPasswordVerifierReady(derivedVerifier);
+            derivedVerifier.clear();
+        }
     } else {
         emit sigForgotPasswordError();
     }

libcockatrice_network/libcockatrice/network/client/remote/remote_client.h

@@ -66,6 +66,11 @@ signals:
                                                   unsigned int port,
                                                   const QString &_userName,
                                                   const QString &_email);
+    //! \brief Emitted once a scrypt verifier for the given account is known and
+    //! can be persisted instead of the plaintext password. The receiving side
+    //! should only store it for the connection it is currently negotiating, so
+    //! the hostname and user name are intentionally not part of the signal.
+    void sigPasswordVerifierReady(const QString &verifier);
 private slots:
     void slotConnected();
     void readData();
@@ -92,6 +97,8 @@ private slots:
     void doLogin();
     void doHashedLogin();
     Command_Login generateCommandLogin();
+    void doSubmitPasswordVerifier();
+    void submitPasswordVerifierResponse(const Response &response);
     void doDisconnectFromServer();
     void doActivateToServer(const QString &_token);
     void doRequestForgotPasswordToServer(const QString &hostname, unsigned int port, const QString &_userName);
@@ -123,6 +130,14 @@ private:
     QString lastHostname;
     unsigned int lastPort;
     QString hashedPassword;
+    bool passwordNeedsMigration;
+    //! \brief A previously stored "$scrypt$..." verifier used to authenticate
+    //! without the plaintext password.
+    QString storedVerifier;
+    //! \brief Verifier derived during the current login, persisted after success.
+    QString derivedVerifier;
+    //! \brief Verifier sent for migration, persisted once the server accepts it.
+    QString pendingVerifier;
 
     QString getSrvClientID(const QString &_hostname);
     bool newMissingFeatureFound(const QString &_serversMissingFeatures);
@@ -161,6 +176,12 @@ public:
     }
     void
     connectToServer(const QString &hostname, unsigned int port, const QString &_userName, const QString &_password);
+    //! \brief Provide a stored "$scrypt$..." verifier so the client can
+    //! authenticate without the plaintext password.
+    void setStoredVerifier(const QString &verifier)
+    {
+        storedVerifier = verifier;
+    }
     void registerToServer(const QString &hostname,
                           unsigned int port,
                           const QString &_userName,

libcockatrice_network/libcockatrice/network/server/remote/server.h

@@ -87,6 +87,11 @@ public:
     {
         return QMap<QString, bool>();
     }
+    /** @brief True when only challenge-response logins are accepted (strict mode). */
+    virtual bool requiresChallengeResponseAuth() const
+    {
+        return false;
+    }
     void addClient(Server_ProtocolHandler *player);
     void removeClient(Server_ProtocolHandler *player);
     QList<QString> getOnlineModeratorList() const;

libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h

@@ -53,6 +53,14 @@ public:
     {
         return {};
     }
+    virtual QString getUserPasswordData(const QString & /* user */)
+    {
+        return {};
+    }
+    virtual bool submitPasswordVerifier(const QString & /* user */, const QString & /* passwordVerifier */)
+    {
+        return false;
+    }
     virtual QMap<QString, ServerInfo_User> getBuddyList(const QString & /* name */)
     {
         return QMap<QString, ServerInfo_User>();

libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp

@@ -1,5 +1,8 @@
 #include "server_protocolhandler.h"
 
+// Challenge-response nonces are valid for at most one minute from issuance.
+static constexpr qint64 kAuthNonceLifetimeSeconds = 60;
+
 #include "game/game_config.h"
 #include "game/server_abstract_participant.h"
 #include "game/server_game.h"
@@ -72,6 +75,28 @@ Server_ProtocolHandler::~Server_ProtocolHandler()
 {
 }
 
+void Server_ProtocolHandler::setAuthNonce(const QByteArray &nonce, const QString &userName)
+{
+    authNonce = nonce;
+    authNonceUser = userName;
+    authNonceCreated = QDateTime::currentDateTimeUtc();
+}
+
+bool Server_ProtocolHandler::isAuthNonceValid(const QByteArray &nonce, const QString &userName) const
+{
+    // secsTo is signed, so a wall-clock step backwards (NTP correction, VM resume)
+    // must not make the elapsed time negative and re-validate an old nonce.
+    const qint64 elapsed = authNonceCreated.secsTo(QDateTime::currentDateTimeUtc());
+    return !authNonce.isEmpty() && authNonce == nonce && authNonceUser == userName && authNonceCreated.isValid() &&
+           elapsed >= 0 && elapsed < kAuthNonceLifetimeSeconds;
+}
+
+void Server_ProtocolHandler::clearAuthNonce()
+{
+    authNonce.clear();
+    authNonceUser.clear();
+}
+
 // This function must only be called from the thread this object lives in.
 // Except when the server is shutting down.
 // The thread must not hold any server locks when calling this (e.g. clientsLock, roomsLock).
@@ -568,6 +593,20 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd
         return Response::RespContextError;
     }
 
+    // In strict mode only challenge-response logins are accepted, but only for
+    // registered SQL accounts: guests (no DB row) and non-SQL authentication
+    // methods have no stored verifier to challenge against. userExists is false
+    // for both (it requires AuthenticationSql), so a single check covers them.
+    const bool isRegisteredAccount = databaseInterface != nullptr && databaseInterface->userExists(userName);
+    if (server->requiresChallengeResponseAuth() && isRegisteredAccount &&
+        (!cmd.has_hashed_password() || cmd.hashed_password().rfind("$challenge$", 0) != 0)) {
+        auto *re = new Response_Login;
+        re->set_denied_reason_str("Client upgrade required");
+        re->add_missing_features("challenge_response_auth");
+        rc.setResponseExtension(re);
+        return Response::RespClientUpdateRequired;
+    }
+
     // check client feature set against server feature set
     FeatureSet features;
     QMap<QString, bool> receivedClientFeatures;

libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.h

@@ -4,6 +4,8 @@
 #include "server.h"
 #include "server_abstractuserinterface.h"
 
+#include <QByteArray>
+#include <QDateTime>
 #include <QList>
 #include <QMap>
 #include <QObject>
@@ -47,6 +49,9 @@ protected:
     bool acceptsUserListChanges;
     bool acceptsRoomListChanges;
     bool idleClientWarningSent;
+    QByteArray authNonce;
+    QString authNonceUser;
+    QDateTime authNonceCreated;
     virtual void logDebugMessage(const QString & /* message */)
     {
     }
@@ -124,6 +129,13 @@ public:
         return databaseInterface;
     }
 
+    /** @brief Store a fresh challenge nonce bound to @p userName for the next challenge-response login attempt. */
+    void setAuthNonce(const QByteArray &nonce, const QString &userName);
+    /** @brief True if nonce matches the pending one, was issued for @p userName, and is less than 60 seconds old. */
+    bool isAuthNonceValid(const QByteArray &nonce, const QString &userName) const;
+    /** @brief Invalidate the pending nonce (single-use). */
+    void clearAuthNonce();
+
     int getLastCommandTime() const
     {
         return timeRunning - lastDataReceived;