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 b0b5a7b03..4e425fb66 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp @@ -81,9 +81,6 @@ void ConnectionController::wireClientSignals() connect(remoteClient, &RemoteClient::sigPromptForForgotPasswordChallenge, this, &ConnectionController::onPromptForgotPasswordChallenge); - - connect(remoteClient, &RemoteClient::sigPasswordVerifierReady, this, - &ConnectionController::onPasswordVerifierReady); } void ConnectionController::connectToServer() @@ -92,36 +89,16 @@ 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 &storedVerifier, - const QString &saveName, - bool savePassword) + const QString &password) { - 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 d6f7ba262..7486bc81a 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.h +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.h @@ -35,13 +35,8 @@ public: void registerToServer(); void forgotPasswordRequest(); void connectToServer(); - 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 + connectToServerDirect(const QString &host, unsigned int port, const QString &playerName, const QString &password); void disconnectFromServer(); void refreshWindowTitle() @@ -82,9 +77,6 @@ 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(); @@ -101,10 +93,6 @@ 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 9eb074f53..aa8a916f8 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp @@ -273,15 +273,9 @@ void DlgConnect::updateDisplayInfo(const QString &saveName) playernameEdit->setText(_data.at(3)); playernameEdit->setFocus(); savePasswordCheckBox->setChecked(savePasswordStatus); - storedVerifier.clear(); if (savePasswordStatus) { - const QString stored = _data.at(4); - if (stored.startsWith("$")) { - storedVerifier = stored; - } else { - passwordEdit->setText(stored); - } + passwordEdit->setText(_data.at(4)); } if (!_data.at(6).isEmpty()) { @@ -307,7 +301,6 @@ 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); @@ -340,13 +333,10 @@ void DlgConnect::actOk() } servers.addNewServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(), - playernameEdit->text().trimmed(), - passwordEdit->text().isEmpty() ? storedVerifier : passwordEdit->text(), - savePasswordCheckBox->isChecked()); + playernameEdit->text().trimmed(), passwordEdit->text(), savePasswordCheckBox->isChecked()); } else { servers.updateExistingServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(), - playernameEdit->text().trimmed(), - passwordEdit->text().isEmpty() ? storedVerifier : passwordEdit->text(), + playernameEdit->text().trimmed(), 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 456c5af93..083dad0ad 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.h @@ -10,7 +10,6 @@ #include "../interface/widgets/server/handle_public_servers.h" #include "../interface/widgets/server/user/user_info_connection.h" -#include #include #include #include @@ -48,19 +47,6 @@ 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(); @@ -91,7 +77,6 @@ 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 9bb6c4dd1..4310c03fc 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_edit_password.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_edit_password.cpp @@ -18,10 +18,7 @@ DlgEditPassword::DlgEditPassword(QWidget *parent) : QDialog(parent) auto &servers = SettingsCache::instance().servers(); if (servers.getSavePassword()) { - const QString stored = servers.getPassword(); - if (!stored.startsWith("$")) { - oldPasswordEdit->setText(stored); - } + oldPasswordEdit->setText(servers.getPassword()); } 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 883a7e111..416cd42e3 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_box.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_info_box.cpp @@ -283,9 +283,7 @@ void UserInfoBox::changePassword(const QString &oldPassword, const QString &newP { Command_AccountPassword cmd; cmd.set_old_password(oldPassword.toStdString()); - if (client->getServerSupportsChallengeResponse()) { - cmd.set_hashed_new_password(PasswordHasher::generatePasswordVerifier(newPassword).toStdString()); - } else if (client->getServerSupportsPasswordHash()) { + 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 1ca672866..44e188760 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -753,9 +753,8 @@ 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(), - dlg.getStoredVerifier(), dlg.getSaveName(), dlg.getSavePassword()); + connectionController->connectToServerDirect(dlg.getHost(), static_cast(dlg.getPort()), + dlg.getPlayerName(), dlg.getPassword()); } } } diff --git a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp index 81a7884fc..916f4351b 100644 --- a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp @@ -21,8 +21,7 @@ #include AbstractClient::AbstractClient(QObject *parent) - : QObject(parent), nextCmdId(0), status(StatusDisconnected), serverSupportsPasswordHash(false), - serverSupportsChallengeResponse(false) + : QObject(parent), nextCmdId(0), status(StatusDisconnected), serverSupportsPasswordHash(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 2fb871e07..982aa6bf3 100644 --- a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h +++ b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h @@ -94,7 +94,6 @@ protected: QMap pendingCommands; QString userName, password, email, country, realName, token; bool serverSupportsPasswordHash; - bool serverSupportsChallengeResponse; void setStatus(ClientStatus _status); int getNewCmdId() { @@ -118,10 +117,6 @@ 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 88687c4c4..7e20f2722 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp @@ -27,8 +27,7 @@ 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(), - passwordNeedsMigration(false) + messageInProgress(false), handshakeStarted(false), usingWebSocket(false), messageLength(0), hashedPassword() { clearNewClientFeatures(); @@ -115,8 +114,6 @@ 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; @@ -133,10 +130,7 @@ 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() && serverSupportsChallengeResponse) { - hashedPassword = PasswordHasher::generatePasswordVerifier(password); - cmdForgotPasswordReset.set_hashed_new_password(hashedPassword.toStdString()); - } else if (!password.isEmpty() && serverSupportsPasswordHash) { + if (!password.isEmpty() && serverSupportsPasswordHash) { auto passwordSalt = PasswordHasher::generateRandomSalt(); hashedPassword = PasswordHasher::computeHash(password, passwordSalt); cmdForgotPasswordReset.set_hashed_new_password(hashedPassword.toStdString()); @@ -164,10 +158,7 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica if (getStatus() == StatusRegistering) { Command_Register cmdRegister; cmdRegister.set_user_name(userName.toStdString()); - if (!password.isEmpty() && serverSupportsChallengeResponse) { - hashedPassword = PasswordHasher::generatePasswordVerifier(password); - cmdRegister.set_hashed_password(hashedPassword.toStdString()); - } else if (!password.isEmpty() && serverSupportsPasswordHash) { + if (!password.isEmpty() && serverSupportsPasswordHash) { auto passwordSalt = PasswordHasher::generateRandomSalt(); hashedPassword = PasswordHasher::computeHash(password, passwordSalt); cmdRegister.set_hashed_password(hashedPassword.toStdString()); @@ -232,9 +223,7 @@ Command_Login RemoteClient::generateCommandLogin() void RemoteClient::doLogin() { - if ((!password.isEmpty() || !storedVerifier.isEmpty()) && serverSupportsChallengeResponse) { - doRequestPasswordSalt(); // ask salt + nonce to build the challenge response - } else if (!password.isEmpty() && serverSupportsPasswordHash) { + 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 @@ -268,30 +257,6 @@ 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(); @@ -304,47 +269,7 @@ 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(); @@ -369,14 +294,6 @@ 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)); @@ -632,8 +549,6 @@ void RemoteClient::doDisconnectFromServer() websocket->close(); } socket->close(); - derivedVerifier.clear(); - pendingVerifier.clear(); } void RemoteClient::ping() @@ -796,9 +711,6 @@ 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 ca467ae2f..862dac06e 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.h +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.h @@ -54,9 +54,6 @@ 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(); @@ -83,8 +80,6 @@ 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); @@ -116,14 +111,6 @@ 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); @@ -162,12 +149,6 @@ 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 6a8bdbb72..2fca46593 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server.h @@ -84,11 +84,6 @@ 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 4721d2fa7..b43dbde42 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h @@ -40,14 +40,6 @@ 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 1d015f160..c3686ddfa 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp @@ -43,22 +43,6 @@ 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). @@ -501,16 +485,6 @@ 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 c4917e845..0d05b91c8 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.h @@ -4,8 +4,6 @@ #include "server.h" #include "server_abstractuserinterface.h" -#include -#include #include #include #include @@ -57,8 +55,6 @@ protected: bool acceptsUserListChanges; bool acceptsRoomListChanges; bool idleClientWarningSent; - QByteArray authNonce; - QDateTime authNonceCreated; virtual void logDebugMessage(const QString & /* message */) { } @@ -128,13 +124,6 @@ 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 439c68748..3e687ef56 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/featureset.cpp +++ b/libcockatrice_protocol/libcockatrice/protocol/featureset.cpp @@ -25,8 +25,7 @@ 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("challenge_response_auth", false); + // featureList.insert("hashed_password_login", 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 371ae1e95..987ab20d1 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/event_server_identification.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/event_server_identification.proto @@ -8,7 +8,6 @@ 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 e6d036794..3fc228530 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto @@ -6,14 +6,4 @@ 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 6c7f73ccd..9d207c711 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto @@ -28,7 +28,6 @@ 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; @@ -219,14 +218,3 @@ 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_settings/libcockatrice/settings/servers_settings.cpp b/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp index 8ae63fc81..811b0c842 100644 --- a/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp @@ -146,14 +146,6 @@ 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 c4ee894c9..f9803a158 100644 --- a/libcockatrice_settings/libcockatrice/settings/servers_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/servers_settings.h @@ -46,8 +46,6 @@ 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/passwordhasher.cpp b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp index d4c361f1e..1c22fdcfa 100644 --- a/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp +++ b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp @@ -2,9 +2,6 @@ #include #include -#include -#include -#include QString PasswordHasher::computeHash(const QString &password, const QString &salt) { @@ -55,93 +52,3 @@ 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) { - 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 cb3c9be5d..811ecef15 100644 --- a/libcockatrice_utility/libcockatrice/utility/passwordhasher.h +++ b/libcockatrice_utility/libcockatrice/utility/passwordhasher.h @@ -1,53 +1,14 @@ #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 deleted file mode 100644 index a088cd4c2..000000000 --- a/servatrice/migrations/servatrice_0035_to_0036.sql +++ /dev/null @@ -1,3 +0,0 @@ -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 a531c0872..fac743c39 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -101,16 +101,6 @@ 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 d40b35ad6..7f530063c 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(36); +INSERT INTO cockatrice_schema_version VALUES(35); -- 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(255) NOT NULL, + `password_sha512` char(120) NOT 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 736b13b05..aa50e068a 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -865,18 +865,6 @@ 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 119a500b9..62fb382cb 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -137,12 +137,6 @@ public: AuthenticationSql, AuthenticationPassword }; - enum AuthenticationStrictness - { - AuthenticationLegacy, - AuthenticationMixed, - AuthenticationStrict - }; private slots: void statusUpdate(); void shutdownTimeout(); @@ -216,10 +210,6 @@ public: { return serverRequiredFeatureList; } - bool requiresChallengeResponseAuth() const override - { - return getAuthenticationStrictness() == AuthenticationStrict; - } QString getServerName() const; QString getLoginMessage() const override { @@ -239,7 +229,6 @@ 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 d36d61015..d5e1f13ef 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -354,41 +354,6 @@ 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)); @@ -587,47 +552,6 @@ 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 300ca0263..1e3501ec7 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 36 +#define DATABASE_SCHEMA_VERSION 35 class Servatrice; @@ -62,8 +62,6 @@ 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 df0f34399..842ddb4c8 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -83,8 +83,6 @@ #include #include #include -#include -#include #include #include #include @@ -115,12 +113,7 @@ bool AbstractServerSocketInterface::initSession() identEvent.set_server_version(VERSION_STRING); identEvent.set_protocol_version(protocolVersion); if (servatrice->getAuthenticationMethod() == Servatrice::AuthenticationSql) { - Event_ServerIdentification::ServerOptions serverOptions = Event_ServerIdentification::SupportsPasswordHash; - if (servatrice->getAuthenticationStrictness() != Servatrice::AuthenticationLegacy) { - serverOptions = static_cast( - serverOptions | Event_ServerIdentification::SupportsChallengeResponseAuth); - } - identEvent.set_server_options(serverOptions); + identEvent.set_server_options(Event_ServerIdentification::SupportsPasswordHash); } SessionEvent *identSe = prepareSessionEvent(identEvent); sendProtocolItem(*identSe); @@ -230,9 +223,6 @@ 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; } @@ -1406,12 +1396,6 @@ 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); @@ -1792,12 +1776,6 @@ 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; @@ -1933,12 +1911,6 @@ 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(), @@ -1998,8 +1970,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con ResponseContainer &rc) { const QString userName = nameFromStdString(cmd.user_name()); - const QString storedPasswordData = sqlInterface->getUserPasswordData(userName); - if (storedPasswordData.isEmpty()) { + QString passwordSalt = sqlInterface->getUserSalt(userName); + if (passwordSalt.isEmpty()) { if (server->getRegOnlyServerEnabled()) { return Response::RespRegistrationRequired; } else { @@ -2007,58 +1979,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con return Response::RespOk; } } - auto *re = new Response_PasswordSalt; - 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()); - } - + re->set_password_salt(passwordSalt.toStdString()); 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 693fda8fe..0d66ae78f 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -122,7 +122,6 @@ 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 c1b5b22a9..2b8f8bdb7 100644 --- a/tests/password_hash_test.cpp +++ b/tests/password_hash_test.cpp @@ -36,74 +36,6 @@ 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)