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)