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 4e425fb66..b0b5a7b03 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp @@ -81,6 +81,9 @@ void ConnectionController::wireClientSignals() connect(remoteClient, &RemoteClient::sigPromptForForgotPasswordChallenge, this, &ConnectionController::onPromptForgotPasswordChallenge); + + connect(remoteClient, &RemoteClient::sigPasswordVerifierReady, this, + &ConnectionController::onPasswordVerifierReady); } void ConnectionController::connectToServer() @@ -89,16 +92,36 @@ 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 &hostname, + const QString &userName, + const QString &verifier) +{ + Q_UNUSED(hostname); + Q_UNUSED(userName); + 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 7486bc81a..d6f7ba262 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.h +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.h @@ -35,8 +35,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() @@ -77,6 +82,9 @@ private slots: void onPromptForgotPasswordReset(); void onPromptForgotPasswordChallenge(); + // Persists the derived scrypt verifier after a successful challenge-response login + void onPasswordVerifierReady(const QString &hostname, const QString &userName, const QString &verifier); + private: void wireClientSignals(); void updateWindowTitle(); @@ -93,6 +101,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 f1e7a8ba1..1a4aa714e 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp @@ -272,9 +272,15 @@ void DlgConnect::updateDisplayInfo(const QString &saveName) portEdit->setText(_data.at(2)); playernameEdit->setText(_data.at(3)); 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()) { @@ -300,6 +306,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); @@ -332,10 +339,13 @@ void DlgConnect::actOk() } servers.addNewServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(), - playernameEdit->text().trimmed(), passwordEdit->text(), savePasswordCheckBox->isChecked()); + playernameEdit->text().trimmed(), + passwordEdit->text().isEmpty() ? storedVerifier : passwordEdit->text(), + savePasswordCheckBox->isChecked()); } else { servers.updateExistingServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(), - playernameEdit->text().trimmed(), passwordEdit->text(), + playernameEdit->text().trimmed(), + passwordEdit->text().isEmpty() ? storedVerifier : passwordEdit->text(), savePasswordCheckBox->isChecked()); } diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_connect.h b/cockatrice/src/interface/widgets/dialogs/dlg_connect.h index 083dad0ad..456c5af93 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.h @@ -10,6 +10,7 @@ #include "../interface/widgets/server/handle_public_servers.h" #include "../interface/widgets/server/user/user_info_connection.h" +#include #include #include #include @@ -47,6 +48,19 @@ 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 + { + return saveEdit->text(); + } + [[nodiscard]] bool getSavePassword() const + { + return savePasswordCheckBox->isChecked(); + } public slots: void downloadThePublicServers(); @@ -77,6 +91,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 4310c03fc..9bb6c4dd1 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_edit_password.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_edit_password.cpp @@ -18,7 +18,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 416cd42e3..883a7e111 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_box.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_info_box.cpp @@ -283,7 +283,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 21d847e63..8d50fcb5f 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -747,8 +747,9 @@ void MainWindow::changeEvent(QEvent *event) !SettingsCache::instance().debug().getLocalGameOnStartup()) { qCInfo(WindowMainStartupAutoconnectLog) << "Attempting auto-connect..."; DlgConnect dlg(this); - connectionController->connectToServerDirect(dlg.getHost(), static_cast(dlg.getPort()), - dlg.getPlayerName(), dlg.getPassword()); + connectionController->connectToServerDirect( + dlg.getHost(), static_cast(dlg.getPort()), dlg.getPlayerName(), dlg.getPassword(), + dlg.getStoredVerifier(), dlg.getSaveName(), dlg.getSavePassword()); } } } diff --git a/cockatrice/src/main.cpp b/cockatrice/src/main.cpp index dbfd2b6b7..062fda142 100644 --- a/cockatrice/src/main.cpp +++ b/cockatrice/src/main.cpp @@ -44,6 +44,7 @@ #include #include #include +#include QTranslator *translator, *qtTranslator; RNG_Abstract *rng; @@ -241,7 +242,7 @@ int main(int argc, char *argv[]) Logger::getInstance().logToFile(true); } - rng = new RNG_SFMT; + rng = new RNG_SFMT(CryptoUtil::randomUInt64()); themeManager = new ThemeManager; soundEngine = new SoundEngine; diff --git a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp index 916f4351b..81a7884fc 100644 --- a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp @@ -21,7 +21,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 2eb7e3356..bdbc2f47b 100644 --- a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h +++ b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h @@ -94,6 +94,7 @@ protected: QMap pendingCommands; QString userName, password, email, country, realName, token; bool serverSupportsPasswordHash; + bool serverSupportsChallengeResponse; void setStatus(ClientStatus _status); int getNewCmdId() { @@ -117,6 +118,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 7e20f2722..88687c4c4 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp @@ -27,7 +27,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(); @@ -114,6 +115,8 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica return; } serverSupportsPasswordHash = event.server_options() & Event_ServerIdentification::SupportsPasswordHash; + serverSupportsChallengeResponse = + event.server_options() & Event_ServerIdentification::SupportsChallengeResponseAuth; if (getStatus() == StatusRequestingForgotPassword) { Command_ForgotPasswordRequest cmdForgotPasswordRequest; @@ -130,7 +133,10 @@ 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); + 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()); @@ -158,7 +164,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()); @@ -223,7 +232,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 @@ -257,6 +268,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(lastHostname, userName, pendingVerifier); + pendingVerifier.clear(); + } + } else { + qCWarning(RemoteClientLog) << "Failed to migrate password verifier:" << response.response_code(); + } +} + void RemoteClient::processConnectionClosedEvent(const Event_ConnectionClosed & /*event*/) { doDisconnectFromServer(); @@ -269,7 +304,47 @@ 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()); + 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()) { + 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; + key = PasswordHasher::deriveKey(password, QByteArray::fromBase64(passwordSalt.toUtf8()), n, r, p); + derivedVerifier = QString("$scrypt$%1$%2$%3$%4$%5") + .arg(n) + .arg(r) + .arg(p) + .arg(passwordSalt) + .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(); @@ -294,6 +369,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(lastHostname, userName, derivedVerifier); + derivedVerifier.clear(); + } + QList buddyList; for (int i = resp.buddy_list_size() - 1; i >= 0; --i) { buddyList.append(resp.buddy_list(i)); @@ -549,6 +632,8 @@ void RemoteClient::doDisconnectFromServer() websocket->close(); } socket->close(); + derivedVerifier.clear(); + pendingVerifier.clear(); } void RemoteClient::ping() @@ -711,6 +796,9 @@ void RemoteClient::submitForgotPasswordResetResponse(const Response &response) { if (response.response_code() == Response::RespOk) { emit sigForgotPasswordSuccess(); + if (!hashedPassword.isEmpty()) { + emit sigPasswordVerifierReady(lastHostname, userName, hashedPassword); + } } 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 289fdc5d0..44d297089 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.h +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.h @@ -54,6 +54,9 @@ 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. + void sigPasswordVerifierReady(const QString &hostname, const QString &userName, const QString &verifier); private slots: void slotConnected(); void readData(); @@ -80,6 +83,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); @@ -111,6 +116,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); @@ -133,6 +146,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 2fca46593..6a8bdbb72 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server.h @@ -84,6 +84,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 b43dbde42..4721d2fa7 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h @@ -40,6 +40,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 c441da781..6f1113789 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp @@ -43,6 +43,22 @@ Server_ProtocolHandler::~Server_ProtocolHandler() { } +void Server_ProtocolHandler::setAuthNonce(const QByteArray &nonce) +{ + authNonce = nonce; + authNonceCreated = QDateTime::currentDateTimeUtc(); +} + +bool Server_ProtocolHandler::isAuthNonceValid(const QByteArray &nonce) const +{ + return !authNonce.isEmpty() && authNonce == nonce && authNonceCreated.secsTo(QDateTime::currentDateTimeUtc()) < 60; +} + +void Server_ProtocolHandler::clearAuthNonce() +{ + authNonce.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). @@ -485,6 +501,16 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd return Response::RespContextError; } + // In strict mode only challenge-response logins are accepted. + if (server->requiresChallengeResponseAuth() && + (!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 0d05b91c8..c4917e845 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 @@ -55,6 +57,8 @@ protected: bool acceptsUserListChanges; bool acceptsRoomListChanges; bool idleClientWarningSent; + QByteArray authNonce; + QDateTime authNonceCreated; virtual void logDebugMessage(const QString & /* message */) { } @@ -124,6 +128,13 @@ public: return databaseInterface; } + /** @brief Store a fresh challenge nonce for the next challenge-response login attempt. */ + void setAuthNonce(const QByteArray &nonce); + /** @brief True if nonce matches the pending one and was issued less than 60 seconds ago. */ + bool isAuthNonceValid(const QByteArray &nonce) 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 3e687ef56..439c68748 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/featureset.cpp +++ b/libcockatrice_protocol/libcockatrice/protocol/featureset.cpp @@ -25,7 +25,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..371ae1e95 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/event_server_identification.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/event_server_identification.proto @@ -8,6 +8,7 @@ message Event_ServerIdentification { enum ServerOptions { NoOptions = 0; SupportsPasswordHash = 1; + SupportsChallengeResponseAuth = 2; } optional string server_name = 1; optional string server_version = 2; diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto index 3fc228530..e6d036794 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto @@ -6,4 +6,14 @@ message Response_PasswordSalt { optional Response_PasswordSalt ext = 1017; } optional string password_salt = 1; + // scrypt cost parameters for password_salt. 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; } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto index 9d207c711..6c7f73ccd 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto @@ -28,6 +28,7 @@ message SessionCommand { FORGOT_PASSWORD_CHALLENGE = 1023; REQUEST_PASSWORD_SALT = 1024; SET_CARD_ART_PARAMS = 1025; + SUBMIT_PASSWORD_VERIFIER = 1026; REPLAY_LIST = 1100; REPLAY_DOWNLOAD = 1101; REPLAY_MODIFY_MATCH = 1102; @@ -218,3 +219,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 = 1026; + } + // Full verifier string to store, e.g. "$pbkdf2-sha512$210000$$" + required string password_verifier = 1; +} diff --git a/libcockatrice_rng/libcockatrice/rng/rng_sfmt.cpp b/libcockatrice_rng/libcockatrice/rng/rng_sfmt.cpp index 5b38deb3f..17cc77a22 100644 --- a/libcockatrice_rng/libcockatrice/rng/rng_sfmt.cpp +++ b/libcockatrice_rng/libcockatrice/rng/rng_sfmt.cpp @@ -17,6 +17,13 @@ RNG_SFMT::RNG_SFMT(QObject *parent) : RNG_Abstract(parent) sfmt_init_gen_rand(&sfmt, QDateTime::currentDateTime().toSecsSinceEpoch()); } +RNG_SFMT::RNG_SFMT(uint64_t seed, QObject *parent) : RNG_Abstract(parent) +{ + // initialize the random number generator with a 64bit seed, e.g. from a CSPRNG + uint32_t seedArray[2] = {static_cast(seed), static_cast(seed >> 32)}; + sfmt_init_by_array(&sfmt, seedArray, 2); +} + /** * This method is the rand() equivalent which calls the cdf with proper bounds. * diff --git a/libcockatrice_rng/libcockatrice/rng/rng_sfmt.h b/libcockatrice_rng/libcockatrice/rng/rng_sfmt.h index 7e9f53df3..b12401799 100644 --- a/libcockatrice_rng/libcockatrice/rng/rng_sfmt.h +++ b/libcockatrice_rng/libcockatrice/rng/rng_sfmt.h @@ -37,6 +37,7 @@ private: public: explicit RNG_SFMT(QObject *parent = nullptr); + explicit RNG_SFMT(uint64_t seed, QObject *parent = nullptr); unsigned int rand(int min, int max) override; }; diff --git a/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp b/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp index d9b98e036..2f0c45564 100644 --- a/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp @@ -146,6 +146,14 @@ 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"); + } +} + 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 40fa996fb..9ffecb295 100644 --- a/libcockatrice_settings/libcockatrice/settings/servers_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/servers_settings.h @@ -46,6 +46,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/CMakeLists.txt b/libcockatrice_utility/CMakeLists.txt index 2d34cad31..594749623 100644 --- a/libcockatrice_utility/CMakeLists.txt +++ b/libcockatrice_utility/CMakeLists.txt @@ -5,12 +5,13 @@ set(CMAKE_AUTOMOC ON) set(CMAKE_AUTOUIC ON) set(CMAKE_AUTORCC ON) -set(UTILITY_SOURCES libcockatrice/utility/expression.cpp libcockatrice/utility/levenshtein.cpp - libcockatrice/utility/passwordhasher.cpp +set(UTILITY_SOURCES libcockatrice/utility/cryptoutil.cpp libcockatrice/utility/expression.cpp + libcockatrice/utility/levenshtein.cpp libcockatrice/utility/passwordhasher.cpp ) set(UTILITY_HEADERS libcockatrice/utility/color.h + libcockatrice/utility/cryptoutil.h libcockatrice/utility/expression.h libcockatrice/utility/levenshtein.h libcockatrice/utility/macros.h @@ -27,7 +28,9 @@ add_library(libcockatrice_utility STATIC ${UTILITY_SOURCES} ${UTILITY_HEADERS}) target_include_directories(libcockatrice_utility PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) -target_link_libraries(libcockatrice_utility PUBLIC libcockatrice_rng ${QT_CORE_MODULE}) +find_package(OpenSSL REQUIRED) + +target_link_libraries(libcockatrice_utility PUBLIC libcockatrice_rng OpenSSL::Crypto ${QT_CORE_MODULE}) set(ORACLE_LIBS) diff --git a/libcockatrice_utility/libcockatrice/utility/cryptoutil.cpp b/libcockatrice_utility/libcockatrice/utility/cryptoutil.cpp new file mode 100644 index 000000000..416ef261b --- /dev/null +++ b/libcockatrice_utility/libcockatrice/utility/cryptoutil.cpp @@ -0,0 +1,25 @@ +#include "cryptoutil.h" + +#include + +namespace CryptoUtil +{ +QByteArray randomBytes(int count) +{ + QByteArray bytes(count, '\0'); + if (RAND_bytes(reinterpret_cast(bytes.data()), count) != 1) { + // Randomness failure is fatal: never fall back to a predictable source. + qFatal("CryptoUtil::randomBytes: RAND_bytes failed"); + } + return bytes; +} + +quint64 randomUInt64() +{ + quint64 value; + if (RAND_bytes(reinterpret_cast(&value), sizeof(value)) != 1) { + qFatal("CryptoUtil::randomUInt64: RAND_bytes failed"); + } + return value; +} +} // namespace CryptoUtil diff --git a/libcockatrice_utility/libcockatrice/utility/cryptoutil.h b/libcockatrice_utility/libcockatrice/utility/cryptoutil.h new file mode 100644 index 000000000..dba9dc37d --- /dev/null +++ b/libcockatrice_utility/libcockatrice/utility/cryptoutil.h @@ -0,0 +1,13 @@ +#ifndef CRYPTOUTIL_H +#define CRYPTOUTIL_H + +#include +#include + +namespace CryptoUtil +{ +QByteArray randomBytes(int count); +quint64 randomUInt64(); +} // namespace CryptoUtil + +#endif diff --git a/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp index c40c5f94f..d4c361f1e 100644 --- a/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp +++ b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp @@ -1,7 +1,10 @@ #include "passwordhasher.h" #include -#include +#include +#include +#include +#include QString PasswordHasher::computeHash(const QString &password, const QString &salt) { @@ -21,12 +24,28 @@ QString PasswordHasher::generateRandomSalt(const int len) static const char alphanum[] = "0123456789" "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"; + const int size = sizeof(alphanum) - 1; + + // Two bytes per character, corrected for modulo bias via rejection sampling. + const int bucketSize = 65536 / size; + const int limit = bucketSize * size; QString ret; - int size = sizeof(alphanum) - 1; - + ret.reserve(len); + QByteArray random = CryptoUtil::randomBytes(len * 2); + int bytesUsed = 0; for (int i = 0; i < len; ++i) { - ret.append(alphanum[rng->rand(0, size)]); + unsigned int value; + do { + if (bytesUsed >= random.size()) { + random = CryptoUtil::randomBytes(len * 2); + bytesUsed = 0; + } + value = static_cast(static_cast(random.at(bytesUsed))) << 8 | + static_cast(static_cast(random.at(bytesUsed + 1))); + bytesUsed += 2; + } while (value >= limit); + ret.append(alphanum[value / bucketSize]); } return ret; @@ -34,5 +53,95 @@ QString PasswordHasher::generateRandomSalt(const int len) QString PasswordHasher::generateActivationToken() { - return QCryptographicHash::hash(generateRandomSalt().toUtf8(), QCryptographicHash::Md5).toBase64().left(16); + 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) { + qFatal("PasswordHasher::deriveKey: EVP_PBE_scrypt failed"); + } + return key; +} + +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 || n <= 0) { + return result; + } + const int r = parts.at(3).toInt(&ok); + if (!ok || r <= 0) { + return result; + } + const int p = parts.at(4).toInt(&ok); + if (!ok || p <= 0) { + 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("$"); +} + +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 811ecef15..cb3c9be5d 100644 --- a/libcockatrice_utility/libcockatrice/utility/passwordhasher.h +++ b/libcockatrice_utility/libcockatrice/utility/passwordhasher.h @@ -1,14 +1,53 @@ #ifndef PASSWORDHASHER_H #define PASSWORDHASHER_H +#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. */ + static QByteArray deriveKey(const QString &password, const QByteArray &salt, 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 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_0035_to_0036.sql b/servatrice/migrations/servatrice_0035_to_0036.sql new file mode 100644 index 000000000..a088cd4c2 --- /dev/null +++ b/servatrice/migrations/servatrice_0035_to_0036.sql @@ -0,0 +1,3 @@ +ALTER TABLE `cockatrice_users` MODIFY `password_sha512` char(255) NOT NULL, ALGORITHM=INSTANT; + +UPDATE cockatrice_schema_version SET version=36 WHERE version=35; diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index fac743c39..a531c0872 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -101,6 +101,16 @@ password=123456 ; Accept only registered users? default is false (accept unregistered users) regonly=false +[security] + +; How strictly new authentication features are enforced. Possible values: +; * legacy: only accept the legacy 1000-round SHA-512 password hashes; +; * 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 PBKDF2 +; automatically on their next successful login. +authentication_strictness=mixed + [users] ; The minimum length a username can be diff --git a/servatrice/servatrice.sql b/servatrice/servatrice.sql index 7f530063c..d40b35ad6 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(35); +INSERT INTO cockatrice_schema_version VALUES(36); -- users and user data tables CREATE TABLE IF NOT EXISTS `cockatrice_users` ( @@ -28,7 +28,7 @@ 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` char(255) NOT NULL, `email` varchar(255) NOT NULL, `country` char(2) NOT NULL, `avatar_bmp` mediumblob NOT NULL, diff --git a/servatrice/src/main.cpp b/servatrice/src/main.cpp index 9e7fe38d9..13bf95a82 100644 --- a/servatrice/src/main.cpp +++ b/servatrice/src/main.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include RNG_Abstract *rng; @@ -169,7 +170,7 @@ int main(int argc, char *argv[]) signalhandler = new SignalHandler(); - rng = new RNG_SFMT; + rng = new RNG_SFMT(CryptoUtil::randomUInt64()); std::cerr << "Servatrice " << VERSION_STRING << " starting." << std::endl; std::cerr << "-------------------------" << std::endl; diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index aa50e068a..736b13b05 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -865,6 +865,18 @@ 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; +} + QString Servatrice::getDBTypeString() const { if (QProcessEnvironment::systemEnvironment().contains("DATABASE_URL")) { diff --git a/servatrice/src/servatrice.h b/servatrice/src/servatrice.h index 62fb382cb..119a500b9 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -137,6 +137,12 @@ public: AuthenticationSql, AuthenticationPassword }; + enum AuthenticationStrictness + { + AuthenticationLegacy, + AuthenticationMixed, + AuthenticationStrict + }; private slots: void statusUpdate(); void shutdownTimeout(); @@ -210,6 +216,10 @@ public: { return serverRequiredFeatureList; } + bool requiresChallengeResponseAuth() const override + { + return getAuthenticationStrictness() == AuthenticationStrict; + } QString getServerName() const; QString getLoginMessage() const override { @@ -229,6 +239,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 d5e1f13ef..d36d61015 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -354,6 +354,41 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot qCWarning(DatabaseInterfaceLog) << "Login denied: user not active"; return UserIsInactive; } + + 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)) { + return NotLoggedIn; + } + + QByteArray key; + if (PasswordHasher::isLegacyFormat(correctPasswordSha512)) { + key = correctPasswordSha512.toUtf8(); + } else { + 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; + } + QString hashedPassword; if (passwordNeedsHash) { hashedPassword = PasswordHasher::computeHash(password, correctPasswordSha512.left(16)); @@ -552,6 +587,47 @@ 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(); + + // Only migrate accounts that still use the legacy format; the query is a no-op otherwise. + QSqlQuery *query = prepareQuery( + "update {prefix}_users set password_sha512 = :verifier where name = :user and password_sha512 not like '$%'"); + query->bindValue(":verifier", passwordVerifier); + query->bindValue(":user", user); + if (!execSqlQuery(query)) { + qCWarning(DatabaseInterfaceLog) << "Failed to submit password verifier for user" << user << query->lastError(); + return false; + } + return true; +} + int Servatrice_DatabaseInterface::getUserIdInDB(const QString &name) { if (server->getAuthenticationMethod() == Servatrice::AuthenticationSql) { diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index 1e3501ec7..300ca0263 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -10,7 +10,7 @@ #include #include -#define DATABASE_SCHEMA_VERSION 35 +#define DATABASE_SCHEMA_VERSION 36 class Servatrice; @@ -62,6 +62,8 @@ 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; 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 6ceebfca9..f68d4e579 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -83,6 +83,8 @@ #include #include #include +#include +#include #include #include #include @@ -113,7 +115,12 @@ bool AbstractServerSocketInterface::initSession() identEvent.set_server_version(VERSION_STRING); identEvent.set_protocol_version(protocolVersion); if (servatrice->getAuthenticationMethod() == Servatrice::AuthenticationSql) { - identEvent.set_server_options(Event_ServerIdentification::SupportsPasswordHash); + Event_ServerIdentification::ServerOptions serverOptions = Event_ServerIdentification::SupportsPasswordHash; + if (servatrice->getAuthenticationStrictness() != Servatrice::AuthenticationLegacy) { + serverOptions = static_cast( + serverOptions | Event_ServerIdentification::SupportsChallengeResponseAuth); + } + identEvent.set_server_options(serverOptions); } SessionEvent *identSe = prepareSessionEvent(identEvent); sendProtocolItem(*identSe); @@ -223,6 +230,9 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedSessionComm case SessionCommand::REQUEST_PASSWORD_SALT: return cmdRequestPasswordSalt(cmd.GetExtension(Command_RequestPasswordSalt::ext), rc); break; + case SessionCommand::SUBMIT_PASSWORD_VERIFIER: + return cmdSubmitPasswordVerifier(cmd.GetExtension(Command_SubmitPasswordVerifier::ext), rc); + break; default: return Response::RespFunctionNotAllowed; } @@ -1392,6 +1402,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRegisterAccount(const C password = QString::fromStdString(cmd.hashed_password()); } + // In strict mode only scrypt verifiers are accepted for new accounts. + if (servatrice->requiresChallengeResponseAuth() && + (passwordNeedsHash || PasswordHasher::isLegacyFormat(password))) { + return Response::RespClientUpdateRequired; + } + bool requireEmailActivation = settingsCache->value("registration/requireemailactivation", true).toBool(); bool regSucceeded = sqlInterface->registerUser(userName, realName, password, passwordNeedsHash, parsedEmailAddress, country, !requireEmailActivation); @@ -1772,6 +1788,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdAccountPassword(const C newPassword = QString::fromStdString(cmd.hashed_new_password()); } + // In strict mode only scrypt verifiers are accepted. + if (servatrice->requiresChallengeResponseAuth() && + (newPasswordNeedsHash || PasswordHasher::isLegacyFormat(newPassword))) { + return Response::RespClientUpdateRequired; + } + QString userName = QString::fromStdString(userInfo->name()); if (!databaseInterface->changeUserPassword(userName, oldPassword, true, newPassword, newPasswordNeedsHash)) { return Response::RespWrongPassword; @@ -1907,6 +1929,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdForgotPasswordReset(con password = QString::fromStdString(cmd.hashed_new_password()); } + // In strict mode only scrypt verifiers are accepted. + if (servatrice->requiresChallengeResponseAuth() && + (passwordNeedsHash || PasswordHasher::isLegacyFormat(password))) { + return Response::RespClientUpdateRequired; + } + if (sqlInterface->changeUserPassword(nameFromStdString(cmd.user_name()), password, passwordNeedsHash)) { if (servatrice->getEnableForgotPasswordAudit()) { sqlInterface->addAuditRecord(userName.simplified(), this->getAddress(), clientId.simplified(), @@ -1966,8 +1994,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con ResponseContainer &rc) { const QString userName = nameFromStdString(cmd.user_name()); - QString passwordSalt = sqlInterface->getUserSalt(userName); - if (passwordSalt.isEmpty()) { + const QString storedPasswordData = sqlInterface->getUserPasswordData(userName); + if (storedPasswordData.isEmpty()) { if (server->getRegOnlyServerEnabled()) { return Response::RespRegistrationRequired; } else { @@ -1975,12 +2003,58 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con return Response::RespOk; } } + auto *re = new Response_PasswordSalt; - re->set_password_salt(passwordSalt.toStdString()); + const bool challengeResponseEnabled = servatrice->getAuthenticationStrictness() != Servatrice::AuthenticationLegacy; + if (PasswordHasher::isLegacyFormat(storedPasswordData)) { + re->set_password_salt(storedPasswordData.left(16).toStdString()); + re->set_needs_migration(true); + } else { + const PasswordVerifier verifier = PasswordHasher::parsePasswordVerifier(storedPasswordData); + if (!verifier.isValid) { + delete re; + return Response::RespContextError; + } + re->set_password_salt(QString(verifier.salt.toBase64()).toStdString()); + re->set_n(verifier.n); + re->set_r(verifier.r); + re->set_p(verifier.p); + re->set_needs_migration(false); + } + + if (challengeResponseEnabled) { + const QByteArray nonce = CryptoUtil::randomBytes(32); + setAuthNonce(nonce); + re->set_nonce(nonce.constData(), nonce.size()); + } + rc.setResponseExtension(re); return Response::RespOk; } +Response::ResponseCode +AbstractServerSocketInterface::cmdSubmitPasswordVerifier(const Command_SubmitPasswordVerifier &cmd, + ResponseContainer & /*rc*/) +{ + if (authState != PasswordRight) { + return Response::RespLoginNeeded; + } + + const QString passwordVerifier = QString::fromStdString(cmd.password_verifier()); + if (passwordVerifier.isEmpty() || passwordVerifier.length() > MAX_NAME_LENGTH || + PasswordHasher::isLegacyFormat(passwordVerifier)) { + 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 0d66ae78f..693fda8fe 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -122,6 +122,7 @@ private: Response::ResponseCode cmdForgotPasswordChallenge(const Command_ForgotPasswordChallenge &cmd, ResponseContainer &rc); Response::ResponseCode cmdRequestPasswordSalt(const Command_RequestPasswordSalt &cmd, ResponseContainer &rc); + Response::ResponseCode cmdSubmitPasswordVerifier(const Command_SubmitPasswordVerifier &cmd, ResponseContainer &rc); Response::ResponseCode processExtendedSessionCommand(int cmdType, const SessionCommand &cmd, ResponseContainer &rc); Response::ResponseCode processExtendedModeratorCommand(int cmdType, const ModeratorCommand &cmd, ResponseContainer &rc); diff --git a/tests/password_hash_test.cpp b/tests/password_hash_test.cpp index 38d9b6315..c1b5b22a9 100644 --- a/tests/password_hash_test.cpp +++ b/tests/password_hash_test.cpp @@ -1,25 +1,9 @@ #include "gtest/gtest.h" -#include -#include +#include #include -RNG_Abstract *rng; - namespace { -class PasswordHashTest : public ::testing::Test -{ -protected: - void SetUp() override - { - rng = new RNG_SFMT; - } - - void TearDown() override - { - delete rng; - } -}; TEST(PasswordHashTest, RegressionTest) { @@ -29,6 +13,97 @@ TEST(PasswordHashTest, RegressionTest) QString hash = PasswordHasher::computeHash(password, salt); ASSERT_EQ(hash, salt + expected) << "The computed hash value remains the same"; } + +TEST(PasswordHashTest, SaltUsesAlphanumericCharset) +{ + static const char alphanum[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + const QString salt = PasswordHasher::generateRandomSalt(); + ASSERT_EQ(salt.size(), 16); + for (const QChar &c : salt) { + ASSERT_NE(strchr(alphanum, c.toLatin1()), nullptr); + } +} + +TEST(PasswordHashTest, SaltsAreUnique) +{ + const QString salt1 = PasswordHasher::generateRandomSalt(); + const QString salt2 = PasswordHasher::generateRandomSalt(); + ASSERT_NE(salt1, salt2); +} + +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"))); +} + } // namespace int main(int argc, char **argv)