From a139084ad628f4273f9fd4b00dfe5bdd6f9d5951 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Fri, 16 Oct 2026 22:52:26 +0200 Subject: [PATCH] [Security] Add challenge-response auth with scrypt verifiers and stop storing plaintext passwords (#7193) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [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$$$

$$" 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 Co-authored-by: BruebachL --- .../remote_connection_controller.cpp | 21 +- .../remote_connection_controller.h | 16 +- .../interface/widgets/dialogs/dlg_connect.cpp | 18 +- .../interface/widgets/dialogs/dlg_connect.h | 18 ++ .../widgets/dialogs/dlg_edit_password.cpp | 5 +- .../widgets/server/user/user_info_box.cpp | 4 +- cockatrice/src/interface/window_main.cpp | 3 +- .../client/abstract/abstract_client.cpp | 3 +- .../network/client/abstract/abstract_client.h | 5 + .../network/client/remote/remote_client.cpp | 128 +++++++- .../network/client/remote/remote_client.h | 21 ++ .../network/server/remote/server.h | 5 + .../server/remote/server_database_interface.h | 8 + .../server/remote/server_protocolhandler.cpp | 39 +++ .../server/remote/server_protocolhandler.h | 12 + .../libcockatrice/protocol/featureset.cpp | 3 +- .../pb/event_server_identification.proto | 6 + .../protocol/pb/response_password_salt.proto | 18 ++ .../protocol/pb/session_commands.proto | 12 + .../settings/servers_settings.cpp | 13 + .../libcockatrice/settings/servers_settings.h | 2 + .../libcockatrice/utility/cryptoutil.cpp | 103 ++++++ .../libcockatrice/utility/cryptoutil.h | 9 + .../libcockatrice/utility/passwordhasher.cpp | 114 +++++++ .../libcockatrice/utility/passwordhasher.h | 44 +++ .../migrations/servatrice_0039_to_0040.sql | 17 + servatrice/servatrice.ini.example | 42 +++ servatrice/servatrice.sql | 6 +- servatrice/src/servatrice.cpp | 53 +++ servatrice/src/servatrice.h | 17 + .../src/servatrice_database_interface.cpp | 303 +++++++++++++++++- .../src/servatrice_database_interface.h | 14 +- servatrice/src/serversocketinterface.cpp | 131 +++++++- servatrice/src/serversocketinterface.h | 3 + tests/password_hash_test.cpp | 200 ++++++++++++ 35 files changed, 1385 insertions(+), 31 deletions(-) create mode 100644 servatrice/migrations/servatrice_0039_to_0040.sql diff --git a/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp b/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp index 9b985073d..037565c6a 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp @@ -95,6 +95,9 @@ void ConnectionController::wireClientSignals() connect(remoteClient, &RemoteClient::sigPromptForForgotPasswordChallenge, this, &ConnectionController::onPromptForgotPasswordChallenge); + + connect(remoteClient, &RemoteClient::sigPasswordVerifierReady, this, + &ConnectionController::onPasswordVerifierReady); } void ConnectionController::connectToServer() @@ -103,16 +106,32 @@ void ConnectionController::connectToServer() connect(dlgConnect, &DlgConnect::sigStartForgotPasswordRequest, this, &ConnectionController::forgotPasswordRequest); if (dlgConnect->exec()) { + pendingSaveName = dlgConnect->getSaveName(); + pendingSavePassword = dlgConnect->getSavePassword(); + remoteClient->setStoredVerifier(dlgConnect->getStoredVerifier()); remoteClient->connectToServer(dlgConnect->getHost(), static_cast(dlgConnect->getPort()), dlgConnect->getPlayerName(), dlgConnect->getPassword()); } } +void ConnectionController::onPasswordVerifierReady(const QString &verifier) +{ + if (pendingSavePassword) { + SettingsCache::instance().servers().setServerPassword(pendingSaveName, verifier); + } +} + void ConnectionController::connectToServerDirect(const QString &host, unsigned int port, const QString &playerName, - const QString &password) + const QString &password, + const QString &storedVerifier, + const QString &saveName, + bool savePassword) { + pendingSaveName = saveName; + pendingSavePassword = savePassword; + remoteClient->setStoredVerifier(storedVerifier); remoteClient->connectToServer(host, port, playerName, password); } diff --git a/cockatrice/src/client/network/connection_controller/remote_connection_controller.h b/cockatrice/src/client/network/connection_controller/remote_connection_controller.h index d4a832c75..150ec9e5b 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.h +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.h @@ -40,8 +40,13 @@ public: void registerToServer(); void forgotPasswordRequest(); void connectToServer(); - void - connectToServerDirect(const QString &host, unsigned int port, const QString &playerName, const QString &password); + void connectToServerDirect(const QString &host, + unsigned int port, + const QString &playerName, + const QString &password, + const QString &storedVerifier = QString(), + const QString &saveName = QString(), + bool savePassword = false); void disconnectFromServer(); void refreshWindowTitle() @@ -86,6 +91,9 @@ private slots: void onPromptForgotPasswordReset(); void onPromptForgotPasswordChallenge(); + // Persists the derived scrypt verifier after a successful challenge-response login + void onPasswordVerifierReady(const QString &verifier); + private: void wireClientSignals(); void updateWindowTitle(); @@ -102,6 +110,10 @@ private: // Kept as a member so the forgot-password signal can be wired to it DlgConnect *dlgConnect{nullptr}; + + // Captured from the connect dialog when a connection is initiated + QString pendingSaveName; + bool pendingSavePassword{false}; }; #endif // COCKATRICE_REMOTE_CONNECTION_CONTROLLER_H diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp index 7a7e31a58..43b40bf30 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp @@ -289,9 +289,15 @@ void DlgConnect::updateDisplayInfo(const QString &saveName) playernameEdit->setText(_data.at(3)); playernameEdit->setFocus(); savePasswordCheckBox->setChecked(savePasswordStatus); + storedVerifier.clear(); if (savePasswordStatus) { - passwordEdit->setText(_data.at(4)); + const QString stored = _data.at(4); + if (stored.startsWith("$")) { + storedVerifier = stored; + } else { + passwordEdit->setText(stored); + } } if (!_data.at(6).isEmpty()) { @@ -317,6 +323,7 @@ void DlgConnect::newHostSelected(bool state) portEdit->setDisabled(false); playernameEdit->clear(); passwordEdit->clear(); + storedVerifier.clear(); saveEdit->clear(); saveEdit->setPlaceholderText(tr("Unique Server Name")); saveEdit->setDisabled(false); @@ -342,6 +349,11 @@ void DlgConnect::actOk() { ServersSettings &servers = SettingsCache::instance().servers(); + // Never write a newly typed plaintext password to disk when a verifier is already stored: + // the typed value is used for this connection and a fresh verifier is persisted after a + // successful login. Without a stored verifier we keep the previous (plaintext legacy) behavior. + const QString passwordToSave = storedVerifier.isEmpty() ? passwordEdit->text() : storedVerifier; + if (newHostButton->isChecked()) { if (saveEdit->text().isEmpty()) { QMessageBox::critical(this, tr("Connection Warning"), tr("You need to name your new connection profile.")); @@ -349,10 +361,10 @@ void DlgConnect::actOk() } servers.addNewServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(), - playernameEdit->text().trimmed(), passwordEdit->text(), savePasswordCheckBox->isChecked()); + playernameEdit->text().trimmed(), passwordToSave, savePasswordCheckBox->isChecked()); } else { servers.updateExistingServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(), - playernameEdit->text().trimmed(), passwordEdit->text(), + playernameEdit->text().trimmed(), passwordToSave, savePasswordCheckBox->isChecked()); } diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_connect.h b/cockatrice/src/interface/widgets/dialogs/dlg_connect.h index 10cb5366a..bef5f136e 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.h @@ -7,6 +7,7 @@ #ifndef DLG_CONNECT_H #define DLG_CONNECT_H +#include #include #include #include @@ -51,6 +52,22 @@ public: { return passwordEdit->text(); } + //! \brief Stored "$scrypt$..." verifier for challenge-response servers (never the plaintext password). + [[nodiscard]] QString getStoredVerifier() const + { + return storedVerifier; + } + [[nodiscard]] QString getSaveName() const + { + // Trim so the profile lookup key always matches the name DlgConnect::actOk + // stored (it saves under saveEdit->text().trimmed()); otherwise a trailing + // space here misses the index and silently drops the saved verifier. + return saveEdit->text().trimmed(); + } + [[nodiscard]] bool getSavePassword() const + { + return savePasswordCheckBox->isChecked(); + } public slots: void downloadThePublicServers(); @@ -81,6 +98,7 @@ private: QPushButton *btnConnect, *btnForgotPassword, *btnRefreshServers, *btnDeleteServer; QMap> savedHostList; HandlePublicServers *hps; + QString storedVerifier; const QString placeHolderText = tr("Downloading..."); }; #endif diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_edit_password.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_edit_password.cpp index 59f80c79c..979a4519c 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_edit_password.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_edit_password.cpp @@ -23,7 +23,10 @@ DlgEditPassword::DlgEditPassword(QWidget *parent) : QDialog(parent) auto &servers = SettingsCache::instance().servers(); if (servers.getSavePassword()) { - oldPasswordEdit->setText(servers.getPassword()); + const QString stored = servers.getPassword(); + if (!stored.startsWith("$")) { + oldPasswordEdit->setText(stored); + } } oldPasswordLabel->setBuddy(oldPasswordEdit); diff --git a/cockatrice/src/interface/widgets/server/user/user_info_box.cpp b/cockatrice/src/interface/widgets/server/user/user_info_box.cpp index ff3d9a224..60bc97edc 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_box.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_info_box.cpp @@ -301,7 +301,9 @@ void UserInfoBox::changePassword(const QString &oldPassword, const QString &newP { Command_AccountPassword cmd; cmd.set_old_password(oldPassword.toStdString()); - if (client->getServerSupportsPasswordHash()) { + if (client->getServerSupportsChallengeResponse()) { + cmd.set_hashed_new_password(PasswordHasher::generatePasswordVerifier(newPassword).toStdString()); + } else if (client->getServerSupportsPasswordHash()) { auto passwordSalt = PasswordHasher::generateRandomSalt(); QString hashedPassword = PasswordHasher::computeHash(newPassword, passwordSalt); cmd.set_hashed_new_password(hashedPassword.toStdString()); diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index 76092e765..d23ce254c 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -1000,7 +1000,8 @@ void MainWindow::attemptStartupAutoConnect() qCInfo(WindowMainStartupAutoconnectLog) << "Attempting auto-connect..."; DlgConnect dlg(this); connectionController->connectToServerDirect(dlg.getHost(), static_cast(dlg.getPort()), - dlg.getPlayerName(), dlg.getPassword()); + dlg.getPlayerName(), dlg.getPassword(), dlg.getStoredVerifier(), + dlg.getSaveName(), dlg.getSavePassword()); } } diff --git a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp index d66653fb1..0c523d776 100644 --- a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp @@ -41,7 +41,8 @@ #include AbstractClient::AbstractClient(QObject *parent) - : QObject(parent), nextCmdId(0), status(StatusDisconnected), serverSupportsPasswordHash(false) + : QObject(parent), nextCmdId(0), status(StatusDisconnected), serverSupportsPasswordHash(false), + serverSupportsChallengeResponse(false) { qRegisterMetaType("QVariant"); qRegisterMetaType("CommandContainer"); diff --git a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h index 442e96eb2..7efd67972 100644 --- a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h +++ b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h @@ -133,6 +133,7 @@ protected: QMap 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; diff --git a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp index feda9e3f4..cbf7dbda2 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/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 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(); } diff --git a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.h b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.h index 4d8c24fd2..dd91c4c6d 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.h +++ b/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, diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server.h b/libcockatrice_network/libcockatrice/network/server/remote/server.h index 8e4e1a8f1..9e05750ac 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server.h @@ -87,6 +87,11 @@ public: { return QMap(); } + /** @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 getOnlineModeratorList() const; diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h b/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h index 3fef26904..936c48880 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h +++ b/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 getBuddyList(const QString & /* name */) { return QMap(); diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp index f75e2d3e3..eb2b2fc1c 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp +++ b/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 receivedClientFeatures; diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.h b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.h index 65dab99ed..c0a1ffe9a 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.h @@ -4,6 +4,8 @@ #include "server.h" #include "server_abstractuserinterface.h" +#include +#include #include #include #include @@ -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; diff --git a/libcockatrice_protocol/libcockatrice/protocol/featureset.cpp b/libcockatrice_protocol/libcockatrice/protocol/featureset.cpp index 0f8fd4210..b0d67c00d 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/featureset.cpp +++ b/libcockatrice_protocol/libcockatrice/protocol/featureset.cpp @@ -26,7 +26,8 @@ void FeatureSet::initalizeFeatureList(QMap &_featureList) _featureList.insert("idle_client", false); _featureList.insert("forgot_password", false); _featureList.insert("websocket", false); - // featureList.insert("hashed_password_login", false); + _featureList.insert("hashed_password_login", false); + _featureList.insert("challenge_response_auth", false); // These are temp to force users onto a newer client _featureList.insert("2.7.0_min_version", false); _featureList.insert("2.8.0_min_version", false); diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/event_server_identification.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/event_server_identification.proto index 987ab20d1..1f1775063 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/event_server_identification.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/event_server_identification.proto @@ -13,4 +13,10 @@ message Event_ServerIdentification { optional string server_version = 2; optional uint32 protocol_version = 3; optional ServerOptions server_options = 4 [default = NoOptions]; + // Advertises challenge-response authentication separately from server_options. + // server_options must stay exactly SupportsPasswordHash on the wire: it is a + // proto2 enum (closed), so already-released clients compiled without any new + // enumerator parse an unknown value as NoOptions and fall back to sending + // the plaintext password. New clients check this field instead. + optional bool supports_challenge_response = 5; } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto index 3fc228530..aeb3eade2 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto @@ -5,5 +5,23 @@ message Response_PasswordSalt { extend Response { optional Response_PasswordSalt ext = 1017; } + // Salt for the legacy password hash. Pre-challenge clients derive and send + // that hash from it. For migrated accounts holding a fresh legacy backup + // this carries the backup's salt so those clients can still log in; clients + // that speak challenge-response must use scrypt_salt instead when present. optional string password_salt = 1; + // scrypt cost parameters for the verifier. Absent/zero for legacy accounts. + optional int32 n = 2; + optional int32 r = 3; + optional int32 p = 4; + // Server-generated challenge. When present the client must authenticate + // with a challenge-response instead of transmitting the password hash. + optional bytes nonce = 5; + // True when the account still uses the legacy password format and should + // be migrated to the scrypt format after a successful login. + optional bool needs_migration = 6; + // Salt of the stored scrypt verifier. Present for migrated accounts; + // challenge-response clients derive the verifier from this rather than + // from password_salt, which may carry a legacy backup salt. + optional string scrypt_salt = 7; } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto index 4b7fe9c85..10aefe7b5 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto @@ -36,6 +36,7 @@ message SessionCommand { DECK_DOWNLOAD_PUBLIC = 1031; DECK_SHARE_LIST_MINE = 1032; DECK_SHARE_REMOVE = 1033; + SUBMIT_PASSWORD_VERIFIER = 1034; REPLAY_LIST = 1100; REPLAY_DOWNLOAD = 1101; REPLAY_MODIFY_MATCH = 1102; @@ -231,3 +232,14 @@ message Command_SetCardArtParams { optional double vertical_offset = 5; optional double zoom = 6; } + +// Client uploads the new password verifier to migrate a legacy account +// after a successful challenge-response login. Idempotent; only applies +// to accounts still using the legacy password format. +message Command_SubmitPasswordVerifier { + extend SessionCommand { + optional Command_SubmitPasswordVerifier ext = 1034; + } + // Full verifier string to store, e.g. "$scrypt$32768$8$1$$" + required string password_verifier = 1; +} diff --git a/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp b/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp index c5c4b2592..4595da2a2 100644 --- a/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp @@ -155,6 +155,19 @@ void ServersSettings::setFPPlayerName(QString playerName) setValue(playerName, "fpPlayerName"); } +void ServersSettings::setServerPassword(const QString &saveName, const QString &password) +{ + const int index = getPrevioushostindex(saveName); + if (index >= 0) { + setValue(password, QString("password%1").arg(index), "server", "server_details"); + } else { + // A credential write that silently no-ops is hard to diagnose from a + // bug report; surface a mismatched profile name instead. + qCWarning(ServersSettingsLog) << "setServerPassword() could not find profile:" << saveName + << "- password not saved"; + } +} + QString ServersSettings::getFPPlayerName(QString defaultName) const { QVariant name = getValue("fpPlayerName"); diff --git a/libcockatrice_settings/libcockatrice/settings/servers_settings.h b/libcockatrice_settings/libcockatrice/settings/servers_settings.h index 40648baa9..38bb02876 100644 --- a/libcockatrice_settings/libcockatrice/settings/servers_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/servers_settings.h @@ -51,6 +51,8 @@ public: void setFPHostName(QString hostname); void setFPPort(QString port); void setFPPlayerName(QString playerName); + //! \brief Store a password (or a "$scrypt$..." verifier) for the given saved server. + void setServerPassword(const QString &saveName, const QString &password); void addNewServer(const QString &saveName, const QString &serv, const QString &port, diff --git a/libcockatrice_utility/libcockatrice/utility/cryptoutil.cpp b/libcockatrice_utility/libcockatrice/utility/cryptoutil.cpp index fb59022de..871ee1357 100644 --- a/libcockatrice_utility/libcockatrice/utility/cryptoutil.cpp +++ b/libcockatrice_utility/libcockatrice/utility/cryptoutil.cpp @@ -1,8 +1,16 @@ #include "cryptoutil.h" +#include +#include #include #include +// Sealed secret layout: "enc1::". +static constexpr int SECRET_KEY_LENGTH = 32; +static constexpr int SECRET_NONCE_LENGTH = 12; +static constexpr int SECRET_TAG_LENGTH = 16; +static const char *SECRET_FORMAT_PREFIX = "enc1"; + namespace CryptoUtil { QByteArray randomBytes(int count) @@ -23,4 +31,99 @@ quint64 randomUInt64() } return value; } + +QString encryptSecret(const QByteArray &plaintext, const QByteArray &key, const QByteArray &aad) +{ + if (key.size() != SECRET_KEY_LENGTH) { + return {}; + } + const QByteArray nonce = randomBytes(SECRET_NONCE_LENGTH); + QByteArray ciphertext(plaintext.size() + SECRET_TAG_LENGTH, Qt::Uninitialized); + QByteArray tag(SECRET_TAG_LENGTH, Qt::Uninitialized); + + EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new(); + if (ctx == nullptr) { + return {}; + } + int written = 0; + int finalWritten = 0; + bool ok = EVP_EncryptInit_ex(ctx, EVP_aes_256_gcm(), nullptr, nullptr, nullptr) == 1 && + EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, nonce.size(), nullptr) == 1 && + EVP_EncryptInit_ex(ctx, nullptr, nullptr, reinterpret_cast(key.constData()), + reinterpret_cast(nonce.constData())) == 1; + if (ok && !aad.isEmpty()) { + ok = EVP_EncryptUpdate(ctx, nullptr, &written, reinterpret_cast(aad.constData()), + aad.size()) == 1; + } + if (ok) { + ok = EVP_EncryptUpdate(ctx, reinterpret_cast(ciphertext.data()), &written, + reinterpret_cast(plaintext.constData()), plaintext.size()) == 1; + } + if (ok) { + ok = EVP_EncryptFinal_ex(ctx, reinterpret_cast(ciphertext.data() + written), &finalWritten) == + 1 && + EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, SECRET_TAG_LENGTH, tag.data()) == 1; + } + EVP_CIPHER_CTX_free(ctx); + if (!ok) { + return {}; + } + ciphertext.truncate(written + finalWritten); + ciphertext.append(tag); + + return QString::fromLatin1(SECRET_FORMAT_PREFIX) + QLatin1Char(':') + nonce.toBase64() + QLatin1Char(':') + + ciphertext.toBase64(); +} + +QByteArray decryptSecret(const QString &sealed, const QByteArray &key, const QByteArray &aad) +{ + if (key.size() != SECRET_KEY_LENGTH) { + return {}; + } + const QStringList parts = sealed.split(QLatin1Char(':')); + if (parts.size() != 3 || parts.at(0) != QLatin1String(SECRET_FORMAT_PREFIX)) { + return {}; + } + const QByteArray nonce = QByteArray::fromBase64(parts.at(1).toUtf8()); + const QByteArray ciphertext = QByteArray::fromBase64(parts.at(2).toUtf8()); + if (nonce.size() != SECRET_NONCE_LENGTH || ciphertext.size() < SECRET_TAG_LENGTH) { + return {}; + } + const int plaintextLength = ciphertext.size() - SECRET_TAG_LENGTH; + QByteArray plaintext(qMax(plaintextLength, 1), Qt::Uninitialized); + QByteArray tag = ciphertext.right(SECRET_TAG_LENGTH); + const QByteArray body = ciphertext.left(plaintextLength); + + EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new(); + if (ctx == nullptr) { + return {}; + } + int written = 0; + int finalWritten = 0; + bool ok = EVP_DecryptInit_ex(ctx, EVP_aes_256_gcm(), nullptr, nullptr, nullptr) == 1 && + EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, nonce.size(), nullptr) == 1 && + EVP_DecryptInit_ex(ctx, nullptr, nullptr, reinterpret_cast(key.constData()), + reinterpret_cast(nonce.constData())) == 1; + if (ok && !aad.isEmpty()) { + ok = EVP_DecryptUpdate(ctx, nullptr, &written, reinterpret_cast(aad.constData()), + aad.size()) == 1; + } + if (ok) { + // The tag must be set before the final call, which authenticates it; a + // mismatch makes EVP_DecryptFinal_ex fail and the secret stays opaque. + ok = EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, tag.size(), tag.data()) == 1 && + EVP_DecryptUpdate(ctx, reinterpret_cast(plaintext.data()), &written, + reinterpret_cast(body.constData()), body.size()) == 1; + } + if (ok) { + ok = + EVP_DecryptFinal_ex(ctx, reinterpret_cast(plaintext.data() + written), &finalWritten) == 1; + } + EVP_CIPHER_CTX_free(ctx); + if (!ok) { + return {}; + } + plaintext.truncate(written + finalWritten); + return plaintext; +} } // namespace CryptoUtil diff --git a/libcockatrice_utility/libcockatrice/utility/cryptoutil.h b/libcockatrice_utility/libcockatrice/utility/cryptoutil.h index dba9dc37d..d12200bc7 100644 --- a/libcockatrice_utility/libcockatrice/utility/cryptoutil.h +++ b/libcockatrice_utility/libcockatrice/utility/cryptoutil.h @@ -2,12 +2,21 @@ #define CRYPTOUTIL_H #include +#include #include namespace CryptoUtil { QByteArray randomBytes(int count); quint64 randomUInt64(); +/** @brief Seal a secret with AES-256-GCM as "enc1::". + * Returns an empty string unless @p key is a valid 32-byte key and the cipher + * completes. @p aad is authenticated but not stored (bind the ciphertext to its + * context, e.g. the user name it belongs to). */ +QString encryptSecret(const QByteArray &plaintext, const QByteArray &key, const QByteArray &aad = {}); +/** @brief Open a string produced by encryptSecret(). Returns an empty array on any + * failure: unknown format, malformed base64, wrong key, wrong aad, or tampering. */ +QByteArray decryptSecret(const QString &sealed, const QByteArray &key, const QByteArray &aad = {}); } // namespace CryptoUtil #endif diff --git a/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp index b4a412fa6..a7106aadc 100644 --- a/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp +++ b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp @@ -4,6 +4,9 @@ #include #include #include +#include +#include +#include QString PasswordHasher::computeHash(const QString &password, const QString &salt) { @@ -54,3 +57,114 @@ QString PasswordHasher::generateActivationToken() { return QString(CryptoUtil::randomBytes(16).toBase64().left(16)); } + +QByteArray PasswordHasher::deriveKey(const QString &password, const QByteArray &salt, int n, int r, int p) +{ + QByteArray key(SCRYPT_VERIFIER_LENGTH, '\0'); + const QByteArray passwordUtf8 = password.toUtf8(); + // EVP_PBE_scrypt aborts unless maxmem covers the required working memory, + // which is roughly 128 * n * r bytes (plus the small Salsa20/8 block array). + const auto maxmem = static_cast(128) * n * r + static_cast(128) * r * p + 4096; + if (EVP_PBE_scrypt(passwordUtf8.constData(), passwordUtf8.size(), + reinterpret_cast(salt.constData()), salt.size(), n, r, p, maxmem, + reinterpret_cast(key.data()), key.size()) != 1) { + return QByteArray(); + } + return key; +} + +bool PasswordHasher::costParamsAreSane(int n, int r, int p) +{ + // Bounds adopted during review: n in [1024, 2**20] and a power of two, r in [1, 32], + // p in [1, 16]. Anything else is rejected before we allocate or derive for it. + return n >= 1024 && n <= (1 << 20) && (n & (n - 1)) == 0 && r >= 1 && r <= 32 && p >= 1 && p <= 16; +} + +QString PasswordHasher::generatePasswordVerifier(const QString &password) +{ + const QByteArray salt = CryptoUtil::randomBytes(SCRYPT_SALT_LENGTH); + const QByteArray verifier = deriveKey(password, salt, SCRYPT_N, SCRYPT_R, SCRYPT_P); + return QString("$scrypt$%1$%2$%3$%4$%5") + .arg(SCRYPT_N) + .arg(SCRYPT_R) + .arg(SCRYPT_P) + .arg(QString(salt.toBase64())) + .arg(QString(verifier.toBase64())); +} + +PasswordVerifier PasswordHasher::parsePasswordVerifier(const QString &stored) +{ + PasswordVerifier result; + const QStringList parts = stored.split("$"); + if (parts.size() != 7 || parts.at(1) != "scrypt") { + return result; + } + + bool ok = false; + const int n = parts.at(2).toInt(&ok); + if (!ok) { + return result; + } + const int r = parts.at(3).toInt(&ok); + if (!ok) { + return result; + } + const int p = parts.at(4).toInt(&ok); + if (!ok || !costParamsAreSane(n, r, p)) { + return result; + } + + const QByteArray salt = QByteArray::fromBase64(parts.at(5).toUtf8()); + const QByteArray verifier = QByteArray::fromBase64(parts.at(6).toUtf8()); + if (salt.isEmpty() || verifier.size() != SCRYPT_VERIFIER_LENGTH) { + return result; + } + + result.format = PasswordFormat::Scrypt; + result.n = n; + result.r = r; + result.p = p; + result.salt = salt; + result.verifier = verifier; + result.isValid = true; + return result; +} + +bool PasswordHasher::isLegacyFormat(const QString &stored) +{ + return !stored.startsWith("$"); +} + +bool PasswordHasher::verifyPassword(const QString &password, const QString &storedPasswordData) +{ + if (isLegacyFormat(storedPasswordData)) { + return storedPasswordData == computeHash(password, storedPasswordData.left(16)); + } + + const PasswordVerifier verifier = parsePasswordVerifier(storedPasswordData); + if (!verifier.isValid) { + return false; + } + const QByteArray derived = deriveKey(password, verifier.salt, verifier.n, verifier.r, verifier.p); + return !derived.isEmpty() && constantTimeEquals(derived, verifier.verifier); +} + +QByteArray PasswordHasher::computeResponse(const QByteArray &key, const QByteArray &nonce) +{ + QByteArray response(EVP_MAX_MD_SIZE, '\0'); + unsigned int responseLength = 0; + if (HMAC(EVP_sha256(), key.constData(), key.size(), reinterpret_cast(nonce.constData()), + nonce.size(), reinterpret_cast(response.data()), &responseLength) == nullptr) { + qFatal("PasswordHasher::computeResponse: HMAC failed"); + } + response.resize(responseLength); + return response; +} + +bool PasswordHasher::constantTimeEquals(const QByteArray &a, const QByteArray &b) +{ + if (a.size() != b.size()) { + return false; + } + return CRYPTO_memcmp(a.constData(), b.constData(), a.size()) == 0; +} diff --git a/libcockatrice_utility/libcockatrice/utility/passwordhasher.h b/libcockatrice_utility/libcockatrice/utility/passwordhasher.h index 8ad83f089..f46d2f21a 100644 --- a/libcockatrice_utility/libcockatrice/utility/passwordhasher.h +++ b/libcockatrice_utility/libcockatrice/utility/passwordhasher.h @@ -1,14 +1,58 @@ #ifndef PASSWORDHASHER_H #define PASSWORDHASHER_H +#include +#include #include +// scrypt cost parameters used for newly created password verifiers. These match +// the RFC 7914 recommended parameters for interactive use. +constexpr int SCRYPT_N = 32768; +constexpr int SCRYPT_R = 8; +constexpr int SCRYPT_P = 1; +constexpr int SCRYPT_SALT_LENGTH = 16; +constexpr int SCRYPT_VERIFIER_LENGTH = 64; + +enum class PasswordFormat +{ + None = 0, + Scrypt +}; + +struct PasswordVerifier +{ + PasswordFormat format = PasswordFormat::None; + int n = 0; + int r = 0; + int p = 0; + QByteArray salt; + QByteArray verifier; + bool isValid = false; +}; + class PasswordHasher { public: static QString computeHash(const QString &password, const QString &salt); static QString generateRandomSalt(const int len = 16); static QString generateActivationToken(); + + /** @brief Derive the scrypt verifier for the given password, salt and cost parameters. Empty on failure. */ + static QByteArray deriveKey(const QString &password, const QByteArray &salt, int n, int r, int p); + /** @brief True if the scrypt cost parameters are acceptable for server and client use. */ + static bool costParamsAreSane(int n, int r, int p); + /** @brief Build a "$scrypt$$$

$$" string with a fresh random salt. */ + static QString generatePasswordVerifier(const QString &password); + /** @brief Parse a stored "$scrypt$..." string into its components. */ + static PasswordVerifier parsePasswordVerifier(const QString &stored); + /** @brief True if the stored value is not in the scrypt format (legacy salt+hash). */ + static bool isLegacyFormat(const QString &stored); + /** @brief True if the password matches the stored credential, whether legacy salt+hash or scrypt. */ + static bool verifyPassword(const QString &password, const QString &storedPasswordData); + /** @brief HMAC-SHA256 of nonce keyed with the password verifier, used for challenge-response logins. */ + static QByteArray computeResponse(const QByteArray &key, const QByteArray &nonce); + /** @brief Constant-time byte comparison. */ + static bool constantTimeEquals(const QByteArray &a, const QByteArray &b); }; #endif diff --git a/servatrice/migrations/servatrice_0039_to_0040.sql b/servatrice/migrations/servatrice_0039_to_0040.sql new file mode 100644 index 000000000..8298d03f9 --- /dev/null +++ b/servatrice/migrations/servatrice_0039_to_0040.sql @@ -0,0 +1,17 @@ +-- Servatrice db migration from version 39 to version 40 + +-- The column must hold "$scrypt$$$

$$" (up to ~255 chars) and arbitrary +-- legacy base64 hashes, so it grows beyond the old 120-char size. varchar(255) is used because +-- widening a CHAR requires a table rebuild, which ALGORITHM=INSTANT cannot perform — dropping the +-- clause lets the server pick a suitable algorithm (and any row-format change is avoided anyway). +ALTER TABLE `cockatrice_users` MODIFY `password_sha512` varchar(255) NOT NULL; + +-- Encrypted downgrade backup: lets pre-challenge clients keep logging in after the +-- account migrates to scrypt, without storing the fast-crackable legacy hash in the +-- clear. Sealed with security/legacy_backup_key and purged after +-- security/legacy_backup_ttl_days without use (see password_legacy_backup_used_at). +ALTER TABLE `cockatrice_users` + ADD COLUMN `password_legacy_backup` varchar(255) NOT NULL DEFAULT '', + ADD COLUMN `password_legacy_backup_used_at` datetime DEFAULT NULL; + +UPDATE cockatrice_schema_version SET version=40 WHERE version=39; diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index 90207aa70..68478810b 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -420,6 +420,48 @@ max_forgot_password_requests_per_ip=3 ; Length in seconds of the forgot-password window. Default is 3600 (1 hour). forgot_password_window_seconds=3600 +; How strictly new authentication features are enforced. Possible values: +; * legacy: accounts that still use the legacy 1000-round SHA-512 hash keep logging in with it +; (they are served the legacy salt, never a challenge-response nonce) and are never +; auto-migrated to scrypt. Already-migrated scrypt rows keep logging in via +; challenge-response. New credentials may use either format; +; * mixed: accept both legacy hashes and challenge-response authentication (default); +; * strict: only accept challenge-response authentication from clients that support it, +; and reject plain password submissions. Legacy accounts are migrated to scrypt +; automatically on their next successful login. Strict only constrains +; registered SQL accounts: guest/unregistered logins and servers with +; authentication/method set to none or password are unaffected, since +; they have no stored verifier to challenge against. +; +; Note: anyone holding only the legacy salt+hash of a not-yet-migrated account +; (e.g. from a wire capture of an old client, or an old database dump) can log in +; with it and then set a brand new password of their choosing. Before +; challenge-response a captured hash yielded a session, but changing the password +; still required the plaintext old password. There is no way around this without +; a plaintext proof at migration time, so not-yet-migrated rows should be treated +; accordingly. +; +; Challenge-response verifiers are scrypt (RFC 7914, N=32768, r=8, p=1) stored as +; "$scrypt$$$

$$". The stored verifier is password-equivalent: +; plaintext passwords never reach the client configuration and never go over the wire, but +; a database dump yields credentials that can answer a login challenge directly. +authentication_strictness=mixed + +; Optional key that seals the legacy password backup (64 hex characters). +; Migrating an account to scrypt normally strands old clients, which can only present +; the legacy hash. With a key configured, the server keeps that hash encrypted next to +; the scrypt verifier, so old clients can still log in during the window below without +; the database holding a fast-crackable hash in the clear. +; Generate with: openssl rand -hex 32 +; If unset, migrated accounts have no downgrade window: old clients must use a password +; reset to get back in. Rotating the key invalidates existing backups (they become +; unusable immediately; expired ones are still purged). +legacy_backup_key= + +; Days a legacy password backup may go unused before it is purged from the database. +; Logging in with an old client refreshes the window; backups that are never touched +; age out. Default 180. +legacy_backup_ttl_days=180 ; You may want to allow an unlimited number of users from a trusted source. This setting can contain a ; comma-separed list of IP addresses which will allow an unlimited number of connections from each of the diff --git a/servatrice/servatrice.sql b/servatrice/servatrice.sql index 438b32ccd..e942e4e91 100644 --- a/servatrice/servatrice.sql +++ b/servatrice/servatrice.sql @@ -20,7 +20,7 @@ CREATE TABLE IF NOT EXISTS `cockatrice_schema_version` ( PRIMARY KEY (`version`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci; -INSERT INTO cockatrice_schema_version VALUES(39); +INSERT INTO cockatrice_schema_version VALUES(40); -- users and user data tables CREATE TABLE IF NOT EXISTS `cockatrice_users` ( @@ -31,7 +31,9 @@ CREATE TABLE IF NOT EXISTS `cockatrice_users` ( `admin` tinyint(1) NOT NULL, `name` varchar(35) NOT NULL, `realname` varchar(255) NOT NULL, - `password_sha512` char(120) NOT NULL, + `password_sha512` varchar(255) NOT NULL, + `password_legacy_backup` varchar(255) NOT NULL DEFAULT '', + `password_legacy_backup_used_at` datetime DEFAULT NULL, `email` varchar(255) NOT NULL, `country` char(2) NOT NULL, `avatar_bmp` mediumblob NOT NULL, diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index a03444bf5..a306de523 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -76,6 +77,10 @@ inline Q_LOGGING_CATEGORY(ServatriceLog, "servatrice"); +// Expired legacy password backups are also purged on first use, but idle accounts +// need this sweep so credentials that are never touched still age out of the database. +static constexpr qint64 kLegacyBackupSweepIntervalMs = 6 * 60 * 60 * 1000; + Servatrice_GameServer::Servatrice_GameServer(Servatrice *_server, int _numberPools, const QSqlDatabase &_sqlDatabase, @@ -613,6 +618,10 @@ bool Servatrice::initServer() deckShareCleanupClock->start(deckShareCleanupInterval); } + legacyBackupSweepClock = new QTimer(this); + connect(legacyBackupSweepClock, &QTimer::timeout, this, &Servatrice::sweepLegacyBackups); + legacyBackupSweepClock->start(kLegacyBackupSweepIntervalMs); + // SOCKET SERVER if (getNumberOfTCPPools() > 0) { gameServer = @@ -1002,6 +1011,14 @@ SessionEvent *Servatrice::getLoginSessionEvent() const return nullptr; } +void Servatrice::sweepLegacyBackups() +{ + if (!servatriceDatabaseInterface->checkSql()) { + return; + } + servatriceDatabaseInterface->purgeExpiredLegacyBackups(); +} + void Servatrice::scheduleShutdown(const QString &reason, int minutes) { shutdownStateMutex.lock(); @@ -1194,6 +1211,42 @@ QString Servatrice::getRequiredFeatures() const return settingsCache->value("server/requiredfeatures", "").toString(); } +Servatrice::AuthenticationStrictness Servatrice::getAuthenticationStrictness() const +{ + const QString strictness = settingsCache->value("security/authentication_strictness", "mixed").toString(); + if (strictness == "strict") { + return AuthenticationStrict; + } + if (strictness == "legacy") { + return AuthenticationLegacy; + } + return AuthenticationMixed; +} + +QByteArray Servatrice::getLegacyBackupKey() const +{ + const QString keyHex = settingsCache->value("security/legacy_backup_key").toString().trimmed(); + if (keyHex.isEmpty()) { + return {}; + } + static const QRegularExpression validHexKey(QStringLiteral("^[0-9a-fA-F]{64}$")); + if (!validHexKey.match(keyHex).hasMatch()) { + qDebug() << "security/legacy_backup_key is not 64 hex characters;" << "legacy password backups are disabled"; + return {}; + } + return QByteArray::fromHex(keyHex.toUtf8()); +} + +int Servatrice::getLegacyBackupTtlDays() const +{ + bool ok = false; + const int days = settingsCache->value("security/legacy_backup_ttl_days", 180).toInt(&ok); + if (!ok || days < 1 || days > 3650) { + return 180; + } + return days; +} + QString Servatrice::getDBTypeString() const { if (QProcessEnvironment::systemEnvironment().contains("DATABASE_URL")) { diff --git a/servatrice/src/servatrice.h b/servatrice/src/servatrice.h index d5414f810..119adc8b2 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -155,8 +155,15 @@ public: AuthenticationSql, AuthenticationPassword }; + enum AuthenticationStrictness + { + AuthenticationLegacy, + AuthenticationMixed, + AuthenticationStrict + }; private slots: void statusUpdate(); + void sweepLegacyBackups(); void shutdownTimeout(); void cleanupExpiredDeckShares(); @@ -173,6 +180,7 @@ private: DatabaseType databaseType; QTimer *pingClock, *statusUpdateClock; QTimer *deckShareCleanupClock; + QTimer *legacyBackupSweepClock; Servatrice_GameServer *gameServer; Servatrice_WebsocketGameServer *websocketGameServer; Servatrice_IslServer *islServer; @@ -251,6 +259,14 @@ public: { return serverRequiredFeatureList; } + bool requiresChallengeResponseAuth() const override + { + return getAuthenticationStrictness() == AuthenticationStrict; + } + /** @brief Key sealing legacy password backups, or empty when they are disabled. */ + QByteArray getLegacyBackupKey() const; + /** @brief Days a legacy password backup may go unused before it is purged. */ + int getLegacyBackupTtlDays() const; QString getServerName() const; QString getLoginMessage() const override { @@ -272,6 +288,7 @@ public: { return authenticationMethod; } + AuthenticationStrictness getAuthenticationStrictness() const; bool permitUnregisteredUsers() const override { return authenticationMethod != AuthenticationNone; diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index d7912cfe6..04f6047fa 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -44,6 +45,17 @@ inline Q_LOGGING_CATEGORY(DatabaseInterfaceLog, "database_interface"); +// Oldest password_legacy_backup_used_at that is still inside the TTL. The value is +// compared against (and stored as) a UTC "yyyy-MM-dd HH:mm:ss" string, which sorts +// lexicographically, so no timezone-aware parsing is needed on either side. +static QString legacyBackupExpiryCutoff(int ttlDays) +{ + return QDateTime::currentDateTimeUtc().addDays(-ttlDays).toString(QStringLiteral("yyyy-MM-dd HH:mm:ss")); +} + +// Width of the password_legacy_backup column; a sealed backup must fit inside it. +static constexpr int MAX_LEGACY_BACKUP_LENGTH = 255; + Servatrice_DatabaseInterface::Servatrice_DatabaseInterface(int _instanceId, Servatrice *_server) : instanceId(_instanceId), sqlDatabase(QSqlDatabase()), server(_server) { @@ -429,13 +441,104 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot qCWarning(DatabaseInterfaceLog) << "Login denied: user not active"; return UserIsInactive; } + + // Fail closed on an absent stored credential: an empty key would + // otherwise authenticate anyone who can compute HMAC("", nonce). + if (correctPasswordSha512.isEmpty()) { + qCWarning(DatabaseInterfaceLog) << "Login denied: empty stored credential"; + return NotLoggedIn; + } + + if (password.startsWith("$challenge$")) { + // Challenge-response login: verify HMAC(stored_key, nonce) without + // ever transmitting the stored credential or password hash. + const QStringList parts = password.split("$"); + if (parts.size() != 4) { + return NotLoggedIn; + } + const QByteArray nonce = QByteArray::fromBase64(parts.at(2).toUtf8()); + const QByteArray response = QByteArray::fromBase64(parts.at(3).toUtf8()); + if (nonce.isEmpty() || response.isEmpty() || !handler->isAuthNonceValid(nonce, user)) { + return NotLoggedIn; + } + + QByteArray key; + if (PasswordHasher::isLegacyFormat(correctPasswordSha512)) { + key = correctPasswordSha512.toUtf8(); + } else { + // Design note: the stored scrypt verifier IS the challenge-response key, so a + // database dump yields credentials that can answer a login challenge directly. + // We deliberately accept this trade: it removes plaintext passwords from the + // client config and from the wire, but does not protect against DB compromise. + // A password-equivalent proof scheme (e.g. SRP-6a/OPAQUE) would be the proper + // escalation and is out of scope here. + const PasswordVerifier verifier = PasswordHasher::parsePasswordVerifier(correctPasswordSha512); + if (!verifier.isValid) { + return NotLoggedIn; + } + key = verifier.verifier; + } + + const QByteArray expected = PasswordHasher::computeResponse(key, nonce); + handler->clearAuthNonce(); + if (PasswordHasher::constantTimeEquals(expected, response)) { + qCDebug(DatabaseInterfaceLog) << "Login accepted: challenge-response password right"; + return PasswordRight; + } + qCDebug(DatabaseInterfaceLog) << "Login denied: challenge-response password wrong"; + return NotLoggedIn; + } + + // A migrated (scrypt) row distinguishes how the credential arrived: + // a plaintext password (passwordNeedsHash) is verified directly against + // the scrypt verifier, so email changes (cmdAccountEdit) and old clients + // that still send `password` keep working without a legacy backup key. + // A pre-hashed legacy value is accepted against the encrypted legacy + // backup while that backup is fresh. Without one there is no downgrade + // path. The backup window is refreshed only when the backup is what + // actually matched, so a plaintext proof never keeps the + // fast-crackable hash alive. + if (!PasswordHasher::isLegacyFormat(correctPasswordSha512)) { + if (passwordNeedsHash) { + // Cost is bounded by costParamsAreSane inside parsePasswordVerifier. + if (PasswordHasher::verifyPassword(password, correctPasswordSha512)) { + if (forceChange) { + qCDebug(DatabaseInterfaceLog) << "Login accepted but password change required"; + return PasswordChangeRequired; + } + qCDebug(DatabaseInterfaceLog) << "Login accepted: password right"; + return PasswordRight; + } + qCDebug(DatabaseInterfaceLog) << "Login denied: password wrong"; + return NotLoggedIn; + } + const QString backupCredential = getUsableLegacyBackup(user); + if (backupCredential.isEmpty()) { + qCDebug(DatabaseInterfaceLog) + << "Login denied: migrated account without a usable legacy backup"; + return NotLoggedIn; + } + if (PasswordHasher::constantTimeEquals(backupCredential.toUtf8(), password.toUtf8())) { + touchLegacyBackup(user); + if (forceChange) { + qCDebug(DatabaseInterfaceLog) << "Login accepted but password change required"; + return PasswordChangeRequired; + } + qCDebug(DatabaseInterfaceLog) << "Login accepted: password right"; + return PasswordRight; + } + qCDebug(DatabaseInterfaceLog) << "Login denied: password wrong"; + return NotLoggedIn; + } + + QString storedCredential = correctPasswordSha512; QString hashedPassword; if (passwordNeedsHash) { - hashedPassword = PasswordHasher::computeHash(password, correctPasswordSha512.left(16)); + hashedPassword = PasswordHasher::computeHash(password, storedCredential.left(16)); } else { hashedPassword = password; } - if (correctPasswordSha512 == hashedPassword) { + if (PasswordHasher::constantTimeEquals(storedCredential.toUtf8(), hashedPassword.toUtf8())) { if (forceChange) { qCDebug(DatabaseInterfaceLog) << "Login accepted but password change required"; return PasswordChangeRequired; @@ -631,6 +734,185 @@ QString Servatrice_DatabaseInterface::getUserSalt(const QString &user) return {}; } +QString Servatrice_DatabaseInterface::getUserPasswordData(const QString &user) +{ + if (server->getAuthenticationMethod() != Servatrice::AuthenticationSql) { + return {}; + } + + checkSql(); + + QSqlQuery *query = prepareQuery("SELECT password_sha512 FROM {prefix}_users WHERE name = :name"); + query->bindValue(":name", user); + if (!execSqlQuery(query)) { + return {}; + } + + if (!query->next()) { + return {}; + } + + return query->value(0).toString(); +} + +bool Servatrice_DatabaseInterface::submitPasswordVerifier(const QString &user, const QString &passwordVerifier) +{ + if (server->getAuthenticationMethod() != Servatrice::AuthenticationSql) { + return false; + } + + checkSql(); + + // Read the credential that is about to be replaced: while it is still in the + // legacy format it becomes the encrypted downgrade backup, sealed before the + // row is overwritten with the scrypt verifier. + QSqlQuery *currentQuery = prepareQuery("SELECT name, password_sha512 FROM {prefix}_users WHERE name = :name"); + currentQuery->bindValue(":name", user); + if (!execSqlQuery(currentQuery) || !currentQuery->next()) { + return false; + } + // Use the stored row name (not the caller-supplied one) as the AAD: name is + // utf8mb4_unicode_ci, so `alice` logs in as `Alice` and the SELECT still finds + // the row while a client-typed name would fail the GCM tag check on open. + const QString storedName = currentQuery->value(0).toString(); + const QString currentCredential = currentQuery->value(1).toString(); + + QString sealedBackup; + QDateTime backupUsedAt; + if (PasswordHasher::isLegacyFormat(currentCredential)) { + const QByteArray key = server->getLegacyBackupKey(); + if (!key.isEmpty()) { + const QByteArray aad = storedName.simplified().toUtf8(); + sealedBackup = CryptoUtil::encryptSecret(currentCredential.toUtf8(), key, aad); + if (sealedBackup.isEmpty() || sealedBackup.length() > MAX_LEGACY_BACKUP_LENGTH) { + qCWarning(DatabaseInterfaceLog) + << "Failed to seal legacy backup for user" << user << "- account migrates without a downgrade path"; + sealedBackup.clear(); + } else { + backupUsedAt = QDateTime::currentDateTimeUtc(); + } + } + } + + // Only migrate accounts that still use the credential this backup was taken from; + // the guard makes a re-migration (or a raced second submission) a no-op. + QSqlQuery *query = prepareQuery("update {prefix}_users set password_sha512 = :verifier, " + "password_legacy_backup = :backup, password_legacy_backup_used_at = :used_at " + "where name = :user and password_sha512 = :expected and password_sha512 not like " + "'$%'"); + query->bindValue(":verifier", passwordVerifier); + query->bindValue(":backup", sealedBackup); + query->bindValue(":used_at", backupUsedAt.isValid() + ? QVariant(backupUsedAt.toString(QStringLiteral("yyyy-MM-dd HH:mm:ss"))) + : QVariant()); + query->bindValue(":user", user); + query->bindValue(":expected", currentCredential); + if (!execSqlQuery(query)) { + qCWarning(DatabaseInterfaceLog) << "Failed to submit password verifier for user" << user << query->lastError(); + return false; + } + // The guard makes a re-migration a no-op; only report success when a row was actually updated. + return query->numRowsAffected() > 0; +} + +QString Servatrice_DatabaseInterface::getUsableLegacyBackup(const QString &user) +{ + if (server->getAuthenticationMethod() != Servatrice::AuthenticationSql) { + return {}; + } + + // Without a configured key the backup cannot be sealed or opened; do not touch + // (and never purge on this path) so a temporarily missing key cannot destroy it. + const QByteArray key = server->getLegacyBackupKey(); + if (key.isEmpty()) { + return {}; + } + + checkSql(); + + QSqlQuery *query = prepareQuery( + "SELECT name, password_legacy_backup, password_legacy_backup_used_at FROM {prefix}_users WHERE name = :name"); + query->bindValue(":name", user); + if (!execSqlQuery(query) || !query->next()) { + return {}; + } + + // Open with the stored row name as the AAD: sealing used the canonical DB name + // while the caller passes the client-typed name, and the two differ whenever + // the login case does not match the stored case (utf8mb4_unicode_ci). + const QString storedName = query->value(0).toString(); + const QString sealed = query->value(1).toString(); + const QString usedAt = query->value(2).toString(); + if (sealed.isEmpty()) { + return {}; + } + + // Age out: a backup unused for the whole TTL is purged on sight, so a database + // dump taken after the window closes never carries a crackable legacy hash. + if (usedAt.isEmpty() || usedAt < legacyBackupExpiryCutoff(server->getLegacyBackupTtlDays())) { + qCDebug(DatabaseInterfaceLog) << "Purging expired legacy backup for user" << user; + clearLegacyBackup(user); + return {}; + } + + const QByteArray legacyCredential = CryptoUtil::decryptSecret(sealed, key, storedName.simplified().toUtf8()); + if (legacyCredential.isEmpty()) { + qCWarning(DatabaseInterfaceLog) << "Legacy backup for user" << user << "failed to decrypt" + << "- check security/legacy_backup_key"; + return {}; + } + return QString::fromUtf8(legacyCredential); +} + +void Servatrice_DatabaseInterface::touchLegacyBackup(const QString &user) +{ + if (!checkSql()) { + return; + } + + QSqlQuery *query = prepareQuery("UPDATE {prefix}_users SET password_legacy_backup_used_at = UTC_TIMESTAMP() " + "WHERE name = :name AND password_legacy_backup <> ''"); + query->bindValue(":name", user); + execSqlQuery(query); +} + +void Servatrice_DatabaseInterface::clearLegacyBackup(const QString &user) +{ + if (!checkSql()) { + return; + } + + QSqlQuery *query = prepareQuery("UPDATE {prefix}_users SET password_legacy_backup = '', " + "password_legacy_backup_used_at = NULL WHERE name = :name"); + query->bindValue(":name", user); + execSqlQuery(query); +} + +void Servatrice_DatabaseInterface::purgeExpiredLegacyBackups() +{ + if (server->getAuthenticationMethod() != Servatrice::AuthenticationSql) { + return; + } + + if (!checkSql()) { + return; + } + + QSqlQuery *query = prepareQuery("UPDATE {prefix}_users SET password_legacy_backup = '', " + "password_legacy_backup_used_at = NULL " + "WHERE password_legacy_backup <> '' AND " + "(password_legacy_backup_used_at IS NULL OR password_legacy_backup_used_at < " + ":cutoff)"); + query->bindValue(":cutoff", legacyBackupExpiryCutoff(server->getLegacyBackupTtlDays())); + if (!execSqlQuery(query)) { + qCWarning(DatabaseInterfaceLog) << "Legacy backup sweep failed:" << query->lastError(); + return; + } + if (query->numRowsAffected() > 0) { + qCDebug(DatabaseInterfaceLog) << "Purged" << query->numRowsAffected() << "expired legacy password backups"; + } +} + int Servatrice_DatabaseInterface::getUserIdInDB(const QString &name) { if (server->getAuthenticationMethod() == Servatrice::AuthenticationSql) { @@ -1349,7 +1631,10 @@ bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user, passwordSha512 = PasswordHasher::computeHash(password, PasswordHasher::generateRandomSalt()); } + // The backup authenticates the credential it was taken from; a new credential + // invalidates it (the old password is dead, and the new one has no legacy form). QSqlQuery *passwordQuery = prepareQuery("update {prefix}_users set password_sha512=:password, " + "password_legacy_backup='', password_legacy_backup_used_at=NULL, " "passwordLastChangedDate = NOW() where name = :name"); passwordQuery->bindValue(":password", passwordSha512); passwordQuery->bindValue(":name", user); @@ -1402,13 +1687,13 @@ bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user, return false; } - const QString correctPasswordSha512 = passwordQuery->value(0).toString(); - QString oldPasswordSha512 = oldPassword; - if (oldPasswordNeedsHash) { - QString salt = correctPasswordSha512.left(16); - oldPasswordSha512 = PasswordHasher::computeHash(oldPassword, salt); - } - if (correctPasswordSha512 != oldPasswordSha512) { + const QString storedPassword = passwordQuery->value(0).toString(); + // oldPasswordNeedsHash means the client sent the old password in plaintext. Verify it + // against whatever is stored: legacy salt+hash rows or already-migrated scrypt rows + // (which must NOT be re-hashed with a salt torn out of the "$scrypt$..." string). + const bool oldPasswordMatches = oldPasswordNeedsHash ? PasswordHasher::verifyPassword(oldPassword, storedPassword) + : (oldPassword == storedPassword); + if (!oldPasswordMatches) { return false; } diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index eccd0d943..822accada 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -21,7 +21,7 @@ class QSqlQuery; -#define DATABASE_SCHEMA_VERSION 39 +#define DATABASE_SCHEMA_VERSION 40 class Servatrice; @@ -96,6 +96,18 @@ public: bool activeUserExists(const QString &user) override; bool userExists(const QString &user) override; QString getUserSalt(const QString &user) override; + QString getUserPasswordData(const QString &user) override; + bool submitPasswordVerifier(const QString &user, const QString &passwordVerifier) override; + /** @brief Decrypt the account's legacy password backup when it exists, is decryptable + * with the configured key and has not aged out; purges it when it has. Returns the + * legacy credential in the stored "salt + hash" format, or an empty string. */ + QString getUsableLegacyBackup(const QString &user); + /** @brief Refresh the age window of the account's legacy password backup. */ + void touchLegacyBackup(const QString &user); + /** @brief Drop the account's legacy password backup. */ + void clearLegacyBackup(const QString &user); + /** @brief Drop every legacy password backup that has not been used within the TTL. */ + void purgeExpiredLegacyBackups(); int getUserIdInDB(const QString &name); QMap getBuddyList(const QString &name) override; QMap getIgnoreList(const QString &name) override; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index b4d12599d..b24126d93 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -161,6 +161,7 @@ #include #include #include +#include #include #include #include @@ -204,7 +205,15 @@ bool AbstractServerSocketInterface::initSession() identEvent.set_server_version(VERSION_STRING); identEvent.set_protocol_version(protocolVersion); if (servatrice->getAuthenticationMethod() == Servatrice::AuthenticationSql) { + // Challenge-response is advertised in every strictness mode: legacy accounts keep + // logging in with the legacy hash, but already-migrated scrypt rows are always + // served challenge-response (authentication_strictness only governs NEW credentials). + // server_options stays exactly SupportsPasswordHash on the wire: already-released + // clients parse an unknown proto2 enum value as NoOptions and would fall back to + // plaintext passwords. Challenge support is advertised in the separate + // supports_challenge_response field, which old clients simply ignore. identEvent.set_server_options(Event_ServerIdentification::SupportsPasswordHash); + identEvent.set_supports_challenge_response(true); } SessionEvent *identSe = prepareSessionEvent(identEvent); sendProtocolItem(*identSe); @@ -409,6 +418,8 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedSessionComm return cmdReportAddComment(cmd.GetExtension(Command_ReportAddComment::ext), rc); case SessionCommand::REPORT_DETAILS: return cmdReportDetails(cmd.GetExtension(Command_ReportDetails::ext), rc); + case SessionCommand::SUBMIT_PASSWORD_VERIFIER: + return cmdSubmitPasswordVerifier(cmd.GetExtension(Command_SubmitPasswordVerifier::ext), rc); default: return Response::RespFunctionNotAllowed; } @@ -3298,6 +3309,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRegisterAccount(const C password = QString::fromStdString(cmd.hashed_password()); } + // Reject credential formats the configured authentication strictness does not accept. + if (!acceptsCredentialFormat(passwordNeedsHash, password)) { + return Response::RespClientUpdateRequired; + } + bool requireEmailActivation = settingsCache->value("registration/requireemailactivation", true).toBool(); bool regSucceeded = sqlInterface->registerUser(userName, realName, password, passwordNeedsHash, parsedEmailAddress, country, !requireEmailActivation); @@ -3342,6 +3358,30 @@ bool AbstractServerSocketInterface::tooManyRegistrationAttempts(const QString &i return servatrice->recordRegistrationAttempt(ipAddress); } +bool AbstractServerSocketInterface::acceptsCredentialFormat(bool passwordNeedsHash, const QString &password) const +{ + // An empty credential must never reach the database: it would be accepted + // as a legacy format and stored as '' (fail-open on login, see the empty + // stored-credential guard in Servatrice_DatabaseInterface). + if (password.isEmpty()) { + return false; + } + // "scryptFormat" means the client sent a derived verifier rather than a + // password to hash ourselves. parsePasswordVerifier enforces the sane-cost + // clamp, so nothing starting with '$' reaches the database unparsed. + const bool scryptFormat = !passwordNeedsHash && PasswordHasher::parsePasswordVerifier(password).isValid; + + // The strictness mode governs how existing legacy accounts are served, not which new-credential + // formats are tolerated: a legacy-mode server must still accept scrypt verifiers, because clients + // derive them whenever challenge-response is advertised (and it must be, so already-migrated + // scrypt rows keep logging in). strict is the only mode that rejects legacy formats. + if (servatrice->getAuthenticationStrictness() == Servatrice::AuthenticationStrict) { + return scryptFormat; + } + // legacy and mixed accept a valid scrypt verifier or a genuine legacy salt+hash. + return scryptFormat || PasswordHasher::isLegacyFormat(password); +} + Response::ResponseCode AbstractServerSocketInterface::cmdActivateAccount(const Command_Activate &cmd, ResponseContainer & /*rc*/) { @@ -3656,6 +3696,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdAccountPassword(const C newPassword = QString::fromStdString(cmd.hashed_new_password()); } + // Reject new credential formats the configured authentication strictness does not accept. + if (!acceptsCredentialFormat(newPasswordNeedsHash, newPassword)) { + return Response::RespClientUpdateRequired; + } + QString userName = QString::fromStdString(userInfo->name()); if (!databaseInterface->changeUserPassword(userName, oldPassword, true, newPassword, newPasswordNeedsHash)) { return Response::RespWrongPassword; @@ -3802,6 +3847,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdForgotPasswordReset(con password = QString::fromStdString(cmd.hashed_new_password()); } + // Reject new credential formats the configured authentication strictness does not accept. + if (!acceptsCredentialFormat(passwordNeedsHash, password)) { + return Response::RespClientUpdateRequired; + } + if (sqlInterface->changeUserPassword(nameFromStdString(cmd.user_name()), password, passwordNeedsHash)) { if (servatrice->getEnableForgotPasswordAudit()) { sqlInterface->addAuditRecord(userName.simplified(), this->getAddress(), clientId.simplified(), @@ -3869,9 +3919,11 @@ AbstractServerSocketInterface::cmdForgotPasswordChallenge(const Command_ForgotPa Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(const Command_RequestPasswordSalt &cmd, ResponseContainer &rc) { - const QString userName = nameFromStdString(cmd.user_name()); - QString passwordSalt = sqlInterface->getUserSalt(userName); - if (passwordSalt.isEmpty()) { + // Simplified exactly like cmdLogin, so the nonce bound to userName here validates + // against the same normalized name during the login's challenge-response check. + const QString userName = nameFromStdString(cmd.user_name()).simplified(); + const QString storedPasswordData = sqlInterface->getUserPasswordData(userName); + if (storedPasswordData.isEmpty()) { if (server->getRegOnlyServerEnabled()) { return Response::RespRegistrationRequired; } else { @@ -3879,8 +3931,46 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con return Response::RespOk; } } + auto *re = new Response_PasswordSalt; - re->set_password_salt(passwordSalt.toStdString()); + if (PasswordHasher::isLegacyFormat(storedPasswordData)) { + re->set_password_salt(storedPasswordData.left(16).toStdString()); + re->set_needs_migration(true); + // Legacy rows get a challenge-response nonce only outside legacy mode (there the client + // logs in with the legacy hash and the account is migrated). In legacy mode the row is + // served the legacy salt, since legacy mode only governs what NEW credentials are accepted. + if (servatrice->getAuthenticationStrictness() != Servatrice::AuthenticationLegacy) { + const QByteArray nonce = CryptoUtil::randomBytes(32); + setAuthNonce(nonce, userName); + re->set_nonce(nonce.constData(), nonce.size()); + } + } else { + const PasswordVerifier verifier = PasswordHasher::parsePasswordVerifier(storedPasswordData); + if (!verifier.isValid) { + delete re; + return Response::RespContextError; + } + const QString scryptSalt = QString(verifier.salt.toBase64()); + re->set_password_salt(scryptSalt.toStdString()); + re->set_scrypt_salt(scryptSalt.toStdString()); + re->set_n(verifier.n); + re->set_r(verifier.r); + re->set_p(verifier.p); + re->set_needs_migration(false); + // Pre-challenge clients derive the legacy hash from password_salt. While a fresh + // encrypted legacy backup exists, serve its salt there so they can still log in; + // challenge-response clients must use scrypt_salt, which always carries the + // verifier salt. + const QString legacyBackup = sqlInterface->getUsableLegacyBackup(userName); + if (!legacyBackup.isEmpty()) { + re->set_password_salt(legacyBackup.left(16).toStdString()); + } + // scrypt rows are served challenge-response in every mode so migrated accounts never lock out. + const QByteArray nonce = CryptoUtil::randomBytes(32); + setAuthNonce(nonce, userName); + re->set_nonce(nonce.constData(), nonce.size()); + } + rc.setResponseExtension(re); return Response::RespOk; } @@ -3978,6 +4068,39 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReport(const Command_Re return Response::RespOk; } +Response::ResponseCode +AbstractServerSocketInterface::cmdSubmitPasswordVerifier(const Command_SubmitPasswordVerifier &cmd, + ResponseContainer & /*rc*/) +{ + if (authState != PasswordRight) { + return Response::RespLoginNeeded; + } + + // Limit to the size of the database column (password_sha512 varchar(255)). + constexpr int MAX_PASSWORD_VERIFIER_LENGTH = 255; + const QString passwordVerifier = QString::fromStdString(cmd.password_verifier()); + if (passwordVerifier.isEmpty() || passwordVerifier.length() > MAX_PASSWORD_VERIFIER_LENGTH || + PasswordHasher::isLegacyFormat(passwordVerifier)) { + return Response::RespContextError; + } + + // Reject unparseable or hostile cost parameters before they reach the database. + const PasswordVerifier parsedVerifier = PasswordHasher::parsePasswordVerifier(passwordVerifier); + if (!parsedVerifier.isValid) { + qCWarning(AbstractServerSocketInterfaceLog) + << "Rejecting password verifier submission with invalid or insane cost parameters"; + return Response::RespContextError; + } + + if (!sqlInterface->submitPasswordVerifier(QString::fromStdString(userInfo->name()), passwordVerifier)) { + return Response::RespContextError; + } + + qCDebug(AbstractServerSocketInterfaceLog) + << "Password verifier migrated for user" << QString::fromStdString(userInfo->name()); + return Response::RespOk; +} + // ADMIN FUNCTIONS. // Permission is checked by the calling function. diff --git a/servatrice/src/serversocketinterface.h b/servatrice/src/serversocketinterface.h index 586bff831..dd9d3f2bb 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -103,6 +103,7 @@ class Command_ReportUserInfo; class Command_RequestPasswordSalt; class Command_ResetUserPassword; class Command_SetCardArtParams; +class Command_SubmitPasswordVerifier; class Command_UpdateAdminNotes; class Command_ViewLogHistory; class Command_WarnUser; @@ -124,6 +125,7 @@ protected: void logDebugMessage(const QString &message) override; bool tooManyRegistrationAttempts(const QString &ipAddress); void processCommandContainer(const CommandContainer &cont) override; + bool acceptsCredentialFormat(bool passwordNeedsHash, const QString &password) const; virtual void writeToSocket(QByteArray &data) = 0; virtual void flushSocket() = 0; @@ -205,6 +207,7 @@ private: Response::ResponseCode cmdReportDetails(const Command_ReportDetails &cmd, ResponseContainer &rc); Response::ResponseCode cmdReportAddComment(const Command_ReportAddComment &cmd, ResponseContainer &rc); Response::ResponseCode cmdReplayDownloadByGameId(const Command_ReplayDownloadByGameId &cmd, ResponseContainer &rc); + Response::ResponseCode cmdSubmitPasswordVerifier(const Command_SubmitPasswordVerifier &cmd, ResponseContainer &rc); Response::ResponseCode processExtendedSessionCommand(int cmdType, const SessionCommand &cmd, ResponseContainer &rc) override; Response::ResponseCode diff --git a/tests/password_hash_test.cpp b/tests/password_hash_test.cpp index 2a504b81c..99f2e1400 100644 --- a/tests/password_hash_test.cpp +++ b/tests/password_hash_test.cpp @@ -1,7 +1,9 @@ #include "gtest/gtest.h" #include #include +#include #include +#include #include #include @@ -39,6 +41,204 @@ TEST(PasswordHashTest, TokenHasExpectedLength) const QString token = PasswordHasher::generateActivationToken(); ASSERT_EQ(token.size(), 16); } + +TEST(PasswordHashTest, DeriveKeyMatchesKnownVector) +{ + // RFC 7914 scrypt test vector, P="password", S="NaCl", N=1024, r=8, p=16 + const QByteArray expected = QByteArray::fromHex("fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b" + "3731622eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cb" + "dfa2cc0640"); + const QByteArray derived = PasswordHasher::deriveKey("password", QByteArray("NaCl"), 1024, 8, 16); + ASSERT_EQ(derived.toHex(), expected.toHex()); +} + +TEST(PasswordHashTest, PasswordVerifierRoundTrip) +{ + const QString stored = PasswordHasher::generatePasswordVerifier("hunter2"); + ASSERT_FALSE(stored.isEmpty()); + ASSERT_TRUE(stored.startsWith("$scrypt$")); + + const PasswordVerifier parsed = PasswordHasher::parsePasswordVerifier(stored); + ASSERT_TRUE(parsed.isValid); + ASSERT_EQ(parsed.format, PasswordFormat::Scrypt); + ASSERT_EQ(parsed.n, SCRYPT_N); + ASSERT_EQ(parsed.r, SCRYPT_R); + ASSERT_EQ(parsed.p, SCRYPT_P); + ASSERT_EQ(parsed.salt.size(), SCRYPT_SALT_LENGTH); + ASSERT_EQ(parsed.verifier.size(), SCRYPT_VERIFIER_LENGTH); +} + +TEST(PasswordHashTest, PasswordVerifierInvalidInput) +{ + ASSERT_FALSE(PasswordHasher::parsePasswordVerifier("garbage").isValid); + ASSERT_FALSE(PasswordHasher::parsePasswordVerifier("$scrypt$not-an-int$8$1$AAAA$BBBB").isValid); + ASSERT_FALSE(PasswordHasher::parsePasswordVerifier("$scrypt$1024$8$1$AAAA$too-short").isValid); + ASSERT_FALSE(PasswordHasher::parsePasswordVerifier("$pbkdf2-sha512$1000$AAAA$BBBB").isValid); +} + +TEST(PasswordHashTest, LegacyFormatDetection) +{ + ASSERT_TRUE(PasswordHasher::isLegacyFormat("salt+hash")); + ASSERT_FALSE(PasswordHasher::isLegacyFormat(PasswordHasher::generatePasswordVerifier("password"))); +} + +TEST(PasswordHashTest, DeriveKeyDependsOnCostParameters) +{ + const QByteArray keyA = PasswordHasher::deriveKey("password", QByteArray("NaCl"), 1024, 8, 16); + const QByteArray keyB = PasswordHasher::deriveKey("password", QByteArray("NaCl"), 2048, 8, 16); + const QByteArray keyC = PasswordHasher::deriveKey("password", QByteArray("NaCl"), 1024, 8, 1); + ASSERT_NE(keyA, keyB); + ASSERT_NE(keyA, keyC); +} + +TEST(PasswordHashTest, ComputeResponseIsDeterministic) +{ + const QByteArray nonce = QByteArray("a nonce value"); + const QByteArray key = QByteArray("the verifier bytes"); + const QByteArray r1 = PasswordHasher::computeResponse(key, nonce); + const QByteArray r2 = PasswordHasher::computeResponse(key, nonce); + const QByteArray r3 = PasswordHasher::computeResponse(QByteArray("a different key"), nonce); + ASSERT_EQ(r1, r2); + ASSERT_NE(r1, r3); +} + +TEST(PasswordHashTest, ConstantTimeEquals) +{ + ASSERT_TRUE(PasswordHasher::constantTimeEquals(QByteArray("same"), QByteArray("same"))); + ASSERT_FALSE(PasswordHasher::constantTimeEquals(QByteArray("same"), QByteArray("diff"))); + ASSERT_FALSE(PasswordHasher::constantTimeEquals(QByteArray("short"), QByteArray("longer"))); +} + +TEST(PasswordHashTest, CostParamsAreSane) +{ + // Accept the recommended interactive parameters and the RFC 7914 test vector's. + ASSERT_TRUE(PasswordHasher::costParamsAreSane(SCRYPT_N, SCRYPT_R, SCRYPT_P)); + ASSERT_TRUE(PasswordHasher::costParamsAreSane(1024, 8, 16)); + + // n must be in [1024, 2**20] and a power of two. + ASSERT_FALSE(PasswordHasher::costParamsAreSane(512, 8, 1)); + ASSERT_FALSE(PasswordHasher::costParamsAreSane(1 << 21, 8, 1)); + ASSERT_FALSE(PasswordHasher::costParamsAreSane(1025, 8, 1)); + ASSERT_FALSE(PasswordHasher::costParamsAreSane(0, 8, 1)); + ASSERT_FALSE(PasswordHasher::costParamsAreSane(-1024, 8, 1)); + + // r in [1, 32], p in [1, 16]. + ASSERT_FALSE(PasswordHasher::costParamsAreSane(1024, 0, 1)); + ASSERT_FALSE(PasswordHasher::costParamsAreSane(1024, 33, 1)); + ASSERT_FALSE(PasswordHasher::costParamsAreSane(1024, 8, 0)); + ASSERT_FALSE(PasswordHasher::costParamsAreSane(1024, 8, 17)); +} + +TEST(PasswordHashTest, ParsePasswordVerifierRejectsHostileCostParams) +{ + // 16 bytes of salt and 64 bytes of verifier, base64 encoded. + const QString saltB64 = QLatin1String("c2FsdHNhbHRzYWx0c2FsdA=="); + const QString verifierB64 = QString(QByteArray(SCRYPT_VERIFIER_LENGTH, '\x42').toBase64()); + + ASSERT_TRUE( + PasswordHasher::parsePasswordVerifier(QString("$scrypt$1024$8$1$%1$%2").arg(saltB64).arg(verifierB64)).isValid); + + // n not a power of two, below 1024, or above 2**20. + ASSERT_FALSE( + PasswordHasher::parsePasswordVerifier(QString("$scrypt$1025$8$1$%1$%2").arg(saltB64).arg(verifierB64)).isValid); + ASSERT_FALSE( + PasswordHasher::parsePasswordVerifier(QString("$scrypt$512$8$1$%1$%2").arg(saltB64).arg(verifierB64)).isValid); + ASSERT_FALSE( + PasswordHasher::parsePasswordVerifier(QString("$scrypt$1073741824$8$1$%1$%2").arg(saltB64).arg(verifierB64)) + .isValid); + + // r and p out of range. + ASSERT_FALSE(PasswordHasher::parsePasswordVerifier(QString("$scrypt$1024$33$1$%1$%2").arg(saltB64).arg(verifierB64)) + .isValid); + ASSERT_FALSE(PasswordHasher::parsePasswordVerifier(QString("$scrypt$1024$8$17$%1$%2").arg(saltB64).arg(verifierB64)) + .isValid); +} + +TEST(PasswordHashTest, VerifyPasswordLegacyRow) +{ + const QString salt = PasswordHasher::generateRandomSalt(); + const QString legacyStored = PasswordHasher::computeHash("correct horse", salt); + ASSERT_TRUE(PasswordHasher::verifyPassword("correct horse", legacyStored)); + ASSERT_FALSE(PasswordHasher::verifyPassword("battery staple", legacyStored)); +} + +TEST(PasswordHashTest, VerifyPasswordScryptRow) +{ + const QString scryptStored = PasswordHasher::generatePasswordVerifier("correct horse"); + // Regression for the changeUserPassword bug that re-hashed the old password with + // a 16-char salt torn out of the "$scrypt$..." string, which could never match. + ASSERT_TRUE(PasswordHasher::verifyPassword("correct horse", scryptStored)); + ASSERT_FALSE(PasswordHasher::verifyPassword("battery staple", scryptStored)); + ASSERT_FALSE(PasswordHasher::verifyPassword("correct horse", "garbage")); +} + +TEST(CryptoUtilTest, SecretRoundTrip) +{ + const QByteArray key = CryptoUtil::randomBytes(32); + const QByteArray plaintext = PasswordHasher::computeHash("correct horse", "saltsaltsaltsalt").toUtf8(); + const QByteArray aad = "alice"; + + const QString sealed = CryptoUtil::encryptSecret(plaintext, key, aad); + ASSERT_FALSE(sealed.isEmpty()); + ASSERT_TRUE(sealed.startsWith("enc1:")); + ASSERT_EQ(CryptoUtil::decryptSecret(sealed, key, aad), plaintext); +} + +TEST(CryptoUtilTest, SealIsNonDeterministic) +{ + const QByteArray key = CryptoUtil::randomBytes(32); + const QByteArray plaintext = "same secret"; + // Fresh random nonce per seal: identical inputs must not produce identical blobs. + ASSERT_NE(CryptoUtil::encryptSecret(plaintext, key), CryptoUtil::encryptSecret(plaintext, key)); +} + +TEST(CryptoUtilTest, RejectsWrongKey) +{ + const QByteArray key = CryptoUtil::randomBytes(32); + const QByteArray otherKey = CryptoUtil::randomBytes(32); + const QString sealed = CryptoUtil::encryptSecret("secret", key, "alice"); + ASSERT_FALSE(sealed.isEmpty()); + ASSERT_TRUE(CryptoUtil::decryptSecret(sealed, otherKey, "alice").isEmpty()); +} + +TEST(CryptoUtilTest, RejectsWrongAad) +{ + const QByteArray key = CryptoUtil::randomBytes(32); + const QString sealed = CryptoUtil::encryptSecret("secret", key, "alice"); + ASSERT_FALSE(sealed.isEmpty()); + ASSERT_TRUE(CryptoUtil::decryptSecret(sealed, key, "bob").isEmpty()); + ASSERT_TRUE(CryptoUtil::decryptSecret(sealed, key).isEmpty()); +} + +TEST(CryptoUtilTest, RejectsTampering) +{ + const QByteArray key = CryptoUtil::randomBytes(32); + const QString sealed = CryptoUtil::encryptSecret("secret", key); + ASSERT_FALSE(sealed.isEmpty()); + + // Flip a character inside the ciphertext segment (keep it valid base64). + QStringList parts = sealed.split(':'); + ASSERT_EQ(parts.size(), 3); + QByteArray ciphertext = QByteArray::fromBase64(parts.at(2).toUtf8()); + ciphertext[0] = static_cast(ciphertext.at(0) ^ 0x01); + parts[2] = QString(ciphertext.toBase64()); + ASSERT_TRUE(CryptoUtil::decryptSecret(parts.join(':'), key).isEmpty()); +} + +TEST(CryptoUtilTest, RejectsMalformedInput) +{ + const QByteArray key = CryptoUtil::randomBytes(32); + ASSERT_TRUE(CryptoUtil::decryptSecret("garbage", key).isEmpty()); + ASSERT_TRUE(CryptoUtil::decryptSecret("enc1:notbase64!!:also-not!", key).isEmpty()); + ASSERT_TRUE(CryptoUtil::decryptSecret("enc1:AAAAAAAAAAAAAAAA:AAAA", key).isEmpty()); + ASSERT_TRUE(CryptoUtil::decryptSecret("", key).isEmpty()); + // A short or oversized key must never reach the cipher. + ASSERT_TRUE( + CryptoUtil::decryptSecret("enc1:AAAAAAAAAAAAAAAA:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", CryptoUtil::randomBytes(16)) + .isEmpty()); + ASSERT_TRUE(CryptoUtil::encryptSecret("secret", CryptoUtil::randomBytes(16)).isEmpty()); +} + } // namespace int main(int argc, char **argv)