diff --git a/.ci/Arch/Dockerfile b/.ci/Arch/Dockerfile index b08e568f3..f37315262 100644 --- a/.ci/Arch/Dockerfile +++ b/.ci/Arch/Dockerfile @@ -8,7 +8,6 @@ RUN pacman --sync --refresh --sysupgrade --needed --noconfirm \ gtest \ mariadb-libs \ ninja \ - openssl \ protobuf \ qt6-base \ qt6-declarative \ diff --git a/.ci/Debian12/Dockerfile b/.ci/Debian12/Dockerfile index e3df94ab5..0fa227d6f 100644 --- a/.ci/Debian12/Dockerfile +++ b/.ci/Debian12/Dockerfile @@ -15,7 +15,6 @@ RUN apt-get update && \ libprotobuf-dev \ libqt6multimedia6 \ libqt6sql6-mysql \ - libssl-dev \ ninja-build \ protobuf-compiler \ qt6-image-formats-plugins \ diff --git a/.ci/Debian13/Dockerfile b/.ci/Debian13/Dockerfile index 60e490c98..13e8b35c7 100644 --- a/.ci/Debian13/Dockerfile +++ b/.ci/Debian13/Dockerfile @@ -16,7 +16,6 @@ RUN apt-get update && \ libprotobuf-dev \ libqt6multimedia6 \ libqt6sql6-mysql \ - libssl-dev \ ninja-build \ protobuf-compiler \ qt6-image-formats-plugins \ diff --git a/.ci/Fedora43/Dockerfile b/.ci/Fedora43/Dockerfile index 4005bbf67..68e894543 100644 --- a/.ci/Fedora43/Dockerfile +++ b/.ci/Fedora43/Dockerfile @@ -7,7 +7,6 @@ RUN dnf install -y \ git \ mariadb-devel \ ninja-build \ - openssl-devel \ protobuf-devel \ qt6-{qtdeclarative,qtshadertools,qttools,qtsvg,qtmultimedia,qtwebsockets}-devel \ qt6-qtimageformats \ diff --git a/.ci/Fedora44/Dockerfile b/.ci/Fedora44/Dockerfile index e0224cdc6..ffd7c1b9b 100644 --- a/.ci/Fedora44/Dockerfile +++ b/.ci/Fedora44/Dockerfile @@ -7,7 +7,6 @@ RUN dnf install -y \ git \ mariadb-devel \ ninja-build \ - openssl-devel \ protobuf-devel \ qt6-{qtdeclarative,qtshadertools,qttools,qtsvg,qtmultimedia,qtwebsockets}-devel \ qt6-qtimageformats \ diff --git a/.ci/Servatrice_Debian12/Dockerfile b/.ci/Servatrice_Debian12/Dockerfile index 321aa7c0f..21f6a036e 100644 --- a/.ci/Servatrice_Debian12/Dockerfile +++ b/.ci/Servatrice_Debian12/Dockerfile @@ -12,7 +12,6 @@ RUN apt-get update && \ libmariadb-dev-compat \ libprotobuf-dev \ libqt6sql6-mysql \ - libssl-dev \ ninja-build \ protobuf-compiler \ qt6-tools-dev \ diff --git a/.ci/Ubuntu24.04/Dockerfile b/.ci/Ubuntu24.04/Dockerfile index 10adc5e64..12320c276 100644 --- a/.ci/Ubuntu24.04/Dockerfile +++ b/.ci/Ubuntu24.04/Dockerfile @@ -15,7 +15,6 @@ RUN apt-get update && \ libprotobuf-dev \ libqt6multimedia6 \ libqt6sql6-mysql \ - libssl-dev \ ninja-build \ protobuf-compiler \ qt6-image-formats-plugins \ diff --git a/.ci/Ubuntu26.04/Dockerfile b/.ci/Ubuntu26.04/Dockerfile index 1b6cf825f..ce3d9cd6c 100644 --- a/.ci/Ubuntu26.04/Dockerfile +++ b/.ci/Ubuntu26.04/Dockerfile @@ -16,7 +16,6 @@ RUN apt-get update && \ libprotobuf-dev \ libqt6multimedia6 \ libqt6sql6-mysql \ - libssl-dev \ ninja-build \ protobuf-compiler \ qt6-image-formats-plugins \ diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 75fbc59f1..e895e2220 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -40,7 +40,7 @@ jobs: steps: - name: "Checkout repository" - uses: actions/checkout@v7 + uses: actions/checkout@v6 - name: "Initialize CodeQL" uses: github/codeql-action/init@v4 diff --git a/.github/workflows/docker-release.yml b/.github/workflows/docker-release.yml index 255e8b045..df4fe233c 100644 --- a/.github/workflows/docker-release.yml +++ b/.github/workflows/docker-release.yml @@ -127,7 +127,7 @@ jobs: steps: - name: "Download digests" - uses: actions/download-artifact@v8 + uses: actions/download-artifact@v7 with: path: ${{ runner.temp }}/digests pattern: digest-* diff --git a/CMakeLists.txt b/CMakeLists.txt index 4006ead2f..bac46c2bc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -239,6 +239,11 @@ if(WIN32) find_package(OpenSSL REQUIRED) if(OPENSSL_FOUND) include_directories(${OPENSSL_INCLUDE_DIRS}) + else() + message( + WARNING + "Could not find OpenSSL runtime libraries. They are not required for compiling, but needs to be available at runtime." + ) endif() endif() diff --git a/Dockerfile b/Dockerfile index 7d3deb5fb..382309d47 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,7 +14,6 @@ RUN apt-get update \ libmariadb-dev-compat \ libprotobuf-dev \ libqt6sql6-mysql \ - libssl-dev \ qt6-websockets-dev \ protobuf-compiler \ qt6-tools-dev \ @@ -43,7 +42,6 @@ RUN apt-get update \ libprotobuf32t64 \ libqt6sql6-mysql \ libqt6websockets6 \ - libssl3 \ && apt-get clean \ && rm -rf /var/lib/apt/lists/* 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 bc9fa0679..c368e46c0 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp @@ -102,8 +102,12 @@ void ConnectionController::connectToServer() } } -void ConnectionController::onPasswordVerifierReady(const QString &verifier) +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); } 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 5e4784513..c5dc87f99 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.h +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.h @@ -87,7 +87,7 @@ private slots: void onPromptForgotPasswordChallenge(); // Persists the derived scrypt verifier after a successful challenge-response login - void onPasswordVerifierReady(const QString &verifier); + void onPasswordVerifierReady(const QString &hostname, const QString &userName, const QString &verifier); private: void wireClientSignals(); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp index 794cabe21..9eb074f53 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp @@ -333,11 +333,6 @@ void DlgConnect::actOk() { ServersSettings &servers = SettingsCache::instance().servers(); - // Never write a newly typed plaintext password to disk when a verifier is already stored: - // the typed value is used for this connection and a fresh verifier is persisted after a - // successful login. Without a stored verifier we keep the previous (plaintext legacy) behavior. - const QString passwordToSave = storedVerifier.isEmpty() ? passwordEdit->text() : storedVerifier; - if (newHostButton->isChecked()) { if (saveEdit->text().isEmpty()) { QMessageBox::critical(this, tr("Connection Warning"), tr("You need to name your new connection profile.")); @@ -345,10 +340,13 @@ void DlgConnect::actOk() } servers.addNewServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(), - playernameEdit->text().trimmed(), passwordToSave, 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(), passwordToSave, + playernameEdit->text().trimmed(), + passwordEdit->text().isEmpty() ? storedVerifier : passwordEdit->text(), savePasswordCheckBox->isChecked()); } diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.cpp index a4a31d78d..198fa259b 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.cpp @@ -1,17 +1,9 @@ #include "dlg_convert_deck_to_cod_format.h" -#include "../../../client/settings/cache_settings.h" -#include "../../deck_loader/deck_loader.h" - #include #include -#include -#include -#include #include -#include #include -#include DialogConvertDeckToCodFormat::DialogConvertDeckToCodFormat(QWidget *parent) : QDialog(parent) { @@ -46,71 +38,3 @@ bool DialogConvertDeckToCodFormat::dontAskAgain() const { return dontAskAgainCheckbox->isChecked(); } - -namespace -{ - -bool confirmOverwriteIfExists(QWidget *parent, const QString &filePath) -{ - QFileInfo fileInfo(filePath); - QString newFileName = QDir::toNativeSeparators(fileInfo.path() + "/" + fileInfo.completeBaseName() + ".cod"); - - if (QFile::exists(newFileName)) { - QMessageBox::StandardButton reply = - QMessageBox::question(parent, QObject::tr("Overwrite Existing File?"), - QObject::tr("A .cod version of this deck already exists. Overwrite it?"), - QMessageBox::Yes | QMessageBox::No); - return reply == QMessageBox::Yes; - } - return true; // Safe to proceed -} - -} // namespace - -bool DialogConvertDeckToCodFormat::promptIfRequired(QWidget *parent, - const QString &filePath, - const std::function &convert) -{ - if (DeckFileFormat::getFormatFromName(filePath) == DeckFileFormat::Cockatrice) { - return true; - } - - // Retrieve saved preference if the prompt is disabled - if (!SettingsCache::instance().visualDeckStorage().getVisualDeckStoragePromptForConversion()) { - if (!SettingsCache::instance().visualDeckStorage().getVisualDeckStorageAlwaysConvert()) { - return false; - } - - if (!confirmOverwriteIfExists(parent, filePath)) { - return false; - } - - return convert(); - } - - // Show the dialog to the user - DialogConvertDeckToCodFormat conversionDialog(parent); - if (conversionDialog.exec() != QDialog::Accepted) { - SettingsCache::instance().visualDeckStorage().setVisualDeckStoragePromptForConversion( - !conversionDialog.dontAskAgain()); - SettingsCache::instance().visualDeckStorage().setVisualDeckStorageAlwaysConvert(false); - - return false; - } - - // Try to convert file - if (!confirmOverwriteIfExists(parent, filePath)) { - return false; - } - - if (!convert()) { - return false; - } - - if (conversionDialog.dontAskAgain()) { - SettingsCache::instance().visualDeckStorage().setVisualDeckStoragePromptForConversion(false); - SettingsCache::instance().visualDeckStorage().setVisualDeckStorageAlwaysConvert(true); - } - - return true; -} diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.h b/cockatrice/src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.h index 526582135..6642ad8c6 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.h @@ -13,9 +13,6 @@ #include #include #include -#include - -class QWidget; class DialogConvertDeckToCodFormat : public QDialog { @@ -27,21 +24,6 @@ public: [[nodiscard]] bool dontAskAgain() const; - /** - * @brief Checks whether the deck file at \a filePath can store tags. - * - * If the file is not a .cod deck, prompts the user for conversion to the - * Cockatrice format, honoring the saved "always convert / don't ask again" - * preference. On acceptance \a convert is called to perform the conversion. - * - * @param parent The widget to parent the prompt to. - * @param filePath The path of the deck file to check. - * @param convert Called to convert the deck once the user agrees. - * @return true if tags can be stored (no conversion needed, or the conversion - * was performed), false if the user declined to convert. - */ - static bool promptIfRequired(QWidget *parent, const QString &filePath, const std::function &convert); - private: QVBoxLayout *layout; QLabel *label; diff --git a/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp b/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp index fb610e814..f6f34a6a5 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp @@ -525,13 +525,6 @@ void UserInfoPopup::rebuildActionButtons(const ServerInfo_User &userInfo, bool o connect(games, &QPushButton::clicked, this, [this, name] { emit showGamesRequested(name); }); add(games); - // ── Invite (only while the inviter has a joinable game for this user) ──── - if (!isSelf && online && gameInviteAvailable && gameInviteAvailable(name)) { - auto *invite = makeBtn(tr("Invite"), tr("Invite to your game"), actionArea, theme); - connect(invite, &QPushButton::clicked, this, [this, name] { emit inviteRequested(name); }); - add(invite); - } - // ── Buddy / ignore (registered users only) ──────────────────────────────── if (!isSelf && isReg) { if (isBuddy) { diff --git a/cockatrice/src/interface/widgets/server/user/user_info_popup.h b/cockatrice/src/interface/widgets/server/user/user_info_popup.h index ed7320fba..02cc2b44e 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_popup.h +++ b/cockatrice/src/interface/widgets/server/user/user_info_popup.h @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -150,17 +149,6 @@ public: /** Re-pulls the avatar/card art for the currently shown user (e.g. after it loads). */ void refreshHeader(); - /** - * Sets a predicate evaluated on every action-button rebuild. It receives - * the name of the user the popup currently shows; when it returns true an - * "Invite" button is shown. The popup itself never resolves the invite - * link, it just forwards the request. - */ - void setGameInviteAvailable(std::function available) - { - gameInviteAvailable = std::move(available); - } - signals: void mouseEnteredPopup(); void mouseLeftPopup(); @@ -171,7 +159,6 @@ signals: // ── Action signals — connect to UserContextMenu::exec*() ────────────────── void chatRequested(const QString &userName); - void inviteRequested(const QString &userName); void detailsRequested(const QString &userName); void showGamesRequested(const QString &userName); void addBuddyRequested(const QString &userName); @@ -213,7 +200,6 @@ private: QString currentUser; ServerInfo_User currentUserInfo; bool currentOnline = false; - std::function gameInviteAvailable; UserInfoHeaderWidget *header; QWidget *actionArea; ///< rebuilt per user diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp index a8c99c979..2cacfc4f9 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp @@ -345,11 +345,6 @@ UserListWidget::UserListWidget(TabSupervisor *_tabSupervisor, &cardArtProvider->cache(), &cardArtParamsMap, window()); // parented to main window so it floats above siblings - // The invite availability is scoped to the room this list belongs to, - // and gated on the room's buddy-only setting for the hovered user. - userInfoPopup->setGameInviteAvailable( - [this](const QString &userName) { return userContextMenu->hasGameInviteLink(userName); }); - userInfoPopup->hide(); userInfoPopup->setWindowOpacity(0.0); userInfoPopup->installEventFilter(this); @@ -667,8 +662,6 @@ void UserListWidget::connectPopupSignals() // Wire all action signals to UserContextMenu::exec*() connect(userInfoPopup, &UserInfoPopup::chatRequested, userContextMenu, &UserContextMenu::execChat); - connect(userInfoPopup, &UserInfoPopup::inviteRequested, this, - [this](const QString &userName) { userContextMenu->execInvite(userName); }); connect(userInfoPopup, &UserInfoPopup::detailsRequested, userContextMenu, &UserContextMenu::execDetails); connect(userInfoPopup, &UserInfoPopup::showGamesRequested, userContextMenu, &UserContextMenu::execShowGames); connect(userInfoPopup, &UserInfoPopup::addBuddyRequested, userContextMenu, &UserContextMenu::execAddToBuddy); diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.h b/cockatrice/src/interface/widgets/server/user/user_list_widget.h index 412271160..7531ef925 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.h +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.h @@ -22,7 +22,6 @@ #include #include #include -#include #include class QTreeWidget; diff --git a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp index b0dac3e7c..f96c139b3 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp @@ -1091,8 +1091,7 @@ QList TabSupervisor::getGameInviteLinksForRoom(int roomId) con // The inviter may be in several games of the same room (hosting one and // spectating another, for example). Return every game so the caller can // let the user choose which one to invite to. - for (auto it = gameTabs.cbegin(); it != gameTabs.cend(); ++it) { - TabGame *tab = it.value(); + for (TabGame *tab : gameTabs) { GameMetaInfo *metaInfo = tab->getGame()->getGameMetaInfo(); if (metaInfo->proto().room_id() != roomId) { continue; diff --git a/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_widget.cpp b/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_widget.cpp index 876fbf6ad..04dcdf7f2 100644 --- a/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_widget.cpp +++ b/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_widget.cpp @@ -10,6 +10,8 @@ #include "../visual_deck_storage_widget.h" #include "deck_preview_deck_tags_display_widget.h" +#include +#include #include #include #include @@ -497,6 +499,21 @@ void DeckPreviewWidget::actDeleteFile() // The folder widget removes this preview once the row is gone. } +static bool confirmOverwriteIfExists(QWidget *parent, const QString &filePath) +{ + QFileInfo fileInfo(filePath); + QString newFileName = QDir::toNativeSeparators(fileInfo.path() + "/" + fileInfo.completeBaseName() + ".cod"); + + if (QFile::exists(newFileName)) { + QMessageBox::StandardButton reply = + QMessageBox::question(parent, QObject::tr("Overwrite Existing File?"), + QObject::tr("A .cod version of this deck already exists. Overwrite it?"), + QMessageBox::Yes | QMessageBox::No); + return reply == QMessageBox::Yes; + } + return true; // Safe to proceed +} + /** * Checks if the deck's file format supports tags. * If not, then prompt the user for file conversion. @@ -504,8 +521,45 @@ void DeckPreviewWidget::actDeleteFile() */ bool DeckPreviewWidget::promptFileConversionIfRequired() { - return DialogConvertDeckToCodFormat::promptIfRequired(this, filePath, [this] { + if (DeckFileFormat::getFormatFromName(filePath) == DeckFileFormat::Cockatrice) { + return true; + } + + // Retrieve saved preference if the prompt is disabled + if (!SettingsCache::instance().visualDeckStorage().getVisualDeckStoragePromptForConversion()) { + if (!SettingsCache::instance().visualDeckStorage().getVisualDeckStorageAlwaysConvert()) { + return false; + } + + if (!confirmOverwriteIfExists(this, filePath)) { + return false; + } + model->convertToCockatriceFormat(row()); return true; - }); + } + + // Show the dialog to the user + DialogConvertDeckToCodFormat conversionDialog(this); + if (conversionDialog.exec() != QDialog::Accepted) { + SettingsCache::instance().visualDeckStorage().setVisualDeckStoragePromptForConversion( + !conversionDialog.dontAskAgain()); + SettingsCache::instance().visualDeckStorage().setVisualDeckStorageAlwaysConvert(false); + + return false; + } + + // Try to convert file + if (!confirmOverwriteIfExists(this, filePath)) { + return false; + } + + model->convertToCockatriceFormat(row()); + + if (conversionDialog.dontAskAgain()) { + SettingsCache::instance().visualDeckStorage().setVisualDeckStoragePromptForConversion(false); + SettingsCache::instance().visualDeckStorage().setVisualDeckStorageAlwaysConvert(true); + } + + return true; } diff --git a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp index c0c142891..8204b6842 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp @@ -135,9 +135,6 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica cmdForgotPasswordReset.set_token(token.toStdString()); if (!password.isEmpty() && serverSupportsChallengeResponse) { hashedPassword = PasswordHasher::generatePasswordVerifier(password); - // Only this branch yields a challenge-response verifier worth persisting; the - // legacy-hash and plaintext branches below must not be saved under the password key. - derivedVerifier = hashedPassword; cmdForgotPasswordReset.set_hashed_new_password(hashedPassword.toStdString()); } else if (!password.isEmpty() && serverSupportsPasswordHash) { auto passwordSalt = PasswordHasher::generateRandomSalt(); @@ -287,7 +284,7 @@ void RemoteClient::submitPasswordVerifierResponse(const Response &response) if (response.response_code() == Response::RespOk) { qCDebug(RemoteClientLog) << "Password verifier migrated successfully"; if (!pendingVerifier.isEmpty()) { - emit sigPasswordVerifierReady(pendingVerifier); + emit sigPasswordVerifierReady(lastHostname, userName, pendingVerifier); pendingVerifier.clear(); } } else { @@ -322,22 +319,10 @@ void RemoteClient::passwordSaltResponse(const Response &response) } key = PasswordHasher::computeHash(password, passwordSalt).toUtf8(); } else if (!password.isEmpty()) { - // A hostile server must not be able to make us run or allocate for - // unreasonable scrypt parameters. const int n = resp.has_n() ? resp.n() : SCRYPT_N; const int r = resp.has_r() ? resp.r() : SCRYPT_R; const int p = resp.has_p() ? resp.p() : SCRYPT_P; - if (!PasswordHasher::costParamsAreSane(n, r, p)) { - emit loginError(Response::RespClientUpdateRequired, - QStringLiteral("The server requested unreasonable scrypt cost parameters."), 0, {}); - return; - } key = PasswordHasher::deriveKey(password, QByteArray::fromBase64(passwordSalt.toUtf8()), n, r, p); - if (key.isEmpty()) { - emit loginError(Response::RespClientUpdateRequired, QStringLiteral("Unable to derive verifier."), 0, - {}); - return; - } derivedVerifier = QString("$scrypt$%1$%2$%3$%4$%5") .arg(n) .arg(r) @@ -388,7 +373,7 @@ void RemoteClient::loginResponse(const Response &response) // The account still used the legacy password format; upgrade it to scrypt. doSubmitPasswordVerifier(); } else if (!derivedVerifier.isEmpty()) { - emit sigPasswordVerifierReady(derivedVerifier); + emit sigPasswordVerifierReady(lastHostname, userName, derivedVerifier); derivedVerifier.clear(); } @@ -812,11 +797,8 @@ void RemoteClient::submitForgotPasswordResetResponse(const Response &response) { if (response.response_code() == Response::RespOk) { emit sigForgotPasswordSuccess(); - // Persist only a real scrypt verifier; a legacy hash must not be stored under - // the password key, where it would break future challenge-response logins. - if (!derivedVerifier.isEmpty()) { - emit sigPasswordVerifierReady(derivedVerifier); - derivedVerifier.clear(); + 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 941eaba25..ca467ae2f 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.h +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.h @@ -55,10 +55,8 @@ signals: const QString &_userName, const QString &_email); //! \brief Emitted once a scrypt verifier for the given account is known and - //! can be persisted instead of the plaintext password. The receiving side - //! should only store it for the connection it is currently negotiating, so - //! the hostname and user name are intentionally not part of the signal. - void sigPasswordVerifierReady(const QString &verifier); + //! 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(); diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto index d7d7ca54a..c06ac9516 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto @@ -232,6 +232,6 @@ message Command_SubmitPasswordVerifier { extend SessionCommand { optional Command_SubmitPasswordVerifier ext = 1026; } - // Full verifier string to store, e.g. "$scrypt$32768$8$1$$" + // 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 4c578b4e4..17cc77a22 100644 --- a/libcockatrice_rng/libcockatrice/rng/rng_sfmt.cpp +++ b/libcockatrice_rng/libcockatrice/rng/rng_sfmt.cpp @@ -1,5 +1,6 @@ #include "rng_sfmt.h" +#include #include #include #include @@ -10,6 +11,12 @@ #define UINT64_MAX (~(uint64_t)0) #endif +RNG_SFMT::RNG_SFMT(QObject *parent) : RNG_Abstract(parent) +{ + // initialize the random number generator with a 32bit integer seed (timestamp) + 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 diff --git a/libcockatrice_rng/libcockatrice/rng/rng_sfmt.h b/libcockatrice_rng/libcockatrice/rng/rng_sfmt.h index a180dad99..b12401799 100644 --- a/libcockatrice_rng/libcockatrice/rng/rng_sfmt.h +++ b/libcockatrice_rng/libcockatrice/rng/rng_sfmt.h @@ -36,6 +36,7 @@ private: unsigned int cdf(unsigned int min, unsigned int max); 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_utility/libcockatrice/utility/passwordhasher.cpp b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp index ffde16a04..d4c361f1e 100644 --- a/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp +++ b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp @@ -66,18 +66,11 @@ QByteArray PasswordHasher::deriveKey(const QString &password, const QByteArray & if (EVP_PBE_scrypt(passwordUtf8.constData(), passwordUtf8.size(), reinterpret_cast(salt.constData()), salt.size(), n, r, p, maxmem, reinterpret_cast(key.data()), key.size()) != 1) { - return QByteArray(); + qFatal("PasswordHasher::deriveKey: EVP_PBE_scrypt failed"); } return key; } -bool PasswordHasher::costParamsAreSane(int n, int r, int p) -{ - // Bounds adopted during review: n in [1024, 2**20] and a power of two, r in [1, 32], - // p in [1, 16]. Anything else is rejected before we allocate or derive for it. - return n >= 1024 && n <= (1 << 20) && (n & (n - 1)) == 0 && r >= 1 && r <= 32 && p >= 1 && p <= 16; -} - QString PasswordHasher::generatePasswordVerifier(const QString &password) { const QByteArray salt = CryptoUtil::randomBytes(SCRYPT_SALT_LENGTH); @@ -100,15 +93,15 @@ PasswordVerifier PasswordHasher::parsePasswordVerifier(const QString &stored) bool ok = false; const int n = parts.at(2).toInt(&ok); - if (!ok) { + if (!ok || n <= 0) { return result; } const int r = parts.at(3).toInt(&ok); - if (!ok) { + if (!ok || r <= 0) { return result; } const int p = parts.at(4).toInt(&ok); - if (!ok || !costParamsAreSane(n, r, p)) { + if (!ok || p <= 0) { return result; } @@ -133,20 +126,6 @@ bool PasswordHasher::isLegacyFormat(const QString &stored) return !stored.startsWith("$"); } -bool PasswordHasher::verifyPassword(const QString &password, const QString &storedPasswordData) -{ - if (isLegacyFormat(storedPasswordData)) { - return storedPasswordData == computeHash(password, storedPasswordData.left(16)); - } - - const PasswordVerifier verifier = parsePasswordVerifier(storedPasswordData); - if (!verifier.isValid) { - return false; - } - const QByteArray derived = deriveKey(password, verifier.salt, verifier.n, verifier.r, verifier.p); - return !derived.isEmpty() && constantTimeEquals(derived, verifier.verifier); -} - QByteArray PasswordHasher::computeResponse(const QByteArray &key, const QByteArray &nonce) { QByteArray response(EVP_MAX_MD_SIZE, '\0'); diff --git a/libcockatrice_utility/libcockatrice/utility/passwordhasher.h b/libcockatrice_utility/libcockatrice/utility/passwordhasher.h index a00146b6e..cb3c9be5d 100644 --- a/libcockatrice_utility/libcockatrice/utility/passwordhasher.h +++ b/libcockatrice_utility/libcockatrice/utility/passwordhasher.h @@ -36,18 +36,14 @@ public: static QString generateRandomSalt(const int len = 16); static QString generateActivationToken(); - /** @brief Derive the scrypt verifier for the given password, salt and cost parameters. Empty on failure. */ + /** @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 True if the scrypt cost parameters are acceptable for server and client use. */ - static bool costParamsAreSane(int n, int r, int p); /** @brief Build a "$scrypt$$$

$$" string with a fresh random salt. */ static QString generatePasswordVerifier(const QString &password); /** @brief Parse a stored "$scrypt$..." string into its components. */ static PasswordVerifier parsePasswordVerifier(const QString &stored); /** @brief True if the stored value is not in the scrypt format (legacy salt+hash). */ static bool isLegacyFormat(const QString &stored); - /** @brief True if the password matches the stored credential, whether legacy salt+hash or scrypt. */ - static bool verifyPassword(const QString &password, const QString &storedPasswordData); /** @brief HMAC-SHA256 of nonce keyed with the password verifier, used for challenge-response logins. */ static QByteArray computeResponse(const QByteArray &key, const QByteArray &nonce); /** @brief Constant-time byte comparison. */ diff --git a/servatrice/migrations/servatrice_0036_to_0037.sql b/servatrice/migrations/servatrice_0036_to_0037.sql index de3e09987..6f83cbec4 100644 --- a/servatrice/migrations/servatrice_0036_to_0037.sql +++ b/servatrice/migrations/servatrice_0036_to_0037.sql @@ -1,8 +1,5 @@ -- Servatrice db migration from version 36 to version 37 --- The column must hold "$scrypt$$$

$$" (up to ~255 chars) and arbitrary --- legacy base64 hashes, so it grows beyond the old 120-char size. varchar(255) is used as the --- column type is promotion-safe and avoids the row-format change ALGORITHM=INSTANT cannot do. -ALTER TABLE `cockatrice_users` MODIFY `password_sha512` varchar(255) NOT NULL; +ALTER TABLE `cockatrice_users` MODIFY `password_sha512` char(255) NOT NULL, ALGORITHM=INSTANT; UPDATE cockatrice_schema_version SET version=37 WHERE version=36; \ No newline at end of file diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index f7e1feed9..11aa24494 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -103,6 +103,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 @@ -350,22 +360,6 @@ max_users_websocket=500 ; Maximum number of users that can connect from the same IP address; useful to avoid bots, default is 4 max_users_per_address=4 -; How strictly new authentication features are enforced. Possible values: -; * legacy: accounts that still use the legacy 1000-round SHA-512 hash keep logging in with it -; (they are served the legacy salt, never a challenge-response nonce) and are never -; auto-migrated to scrypt. Already-migrated scrypt rows keep logging in via -; challenge-response. New credentials may use either format; -; * mixed: accept both legacy hashes and challenge-response authentication (default); -; * strict: only accept challenge-response authentication from clients that support it, -; and reject plain password submissions. Legacy accounts are migrated to scrypt -; automatically on their next successful login. -; -; Challenge-response verifiers are scrypt (RFC 7914, N=32768, r=8, p=1) stored as -; "$scrypt$$$

$$". The stored verifier is password-equivalent: -; plaintext passwords never reach the client configuration and never go over the wire, but -; a database dump yields credentials that can answer a login challenge directly. -authentication_strictness=mixed - ; You may want to allow an unlimited number of users from a trusted source. This setting can contain a ; comma-separed list of IP addresses which will allow an unlimited number of connections from each of the ; IP addresses listed (ignoring the max_users_per_address). Default is "127.0.0.1,::1"; example: "192.73.233.244,81.4.100.74" diff --git a/servatrice/servatrice.sql b/servatrice/servatrice.sql index 0bee10cd0..43851d016 100644 --- a/servatrice/servatrice.sql +++ b/servatrice/servatrice.sql @@ -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` varchar(255) 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/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index caa3478c8..ac45382a8 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -374,12 +374,6 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot if (PasswordHasher::isLegacyFormat(correctPasswordSha512)) { key = correctPasswordSha512.toUtf8(); } else { - // Design note: the stored scrypt verifier IS the challenge-response key, so a - // database dump yields credentials that can answer a login challenge directly. - // We deliberately accept this trade: it removes plaintext passwords from the - // client config and from the wire, but does not protect against DB compromise. - // A password-equivalent proof scheme (e.g. SRP-6a/OPAQUE) would be the proper - // escalation and is out of scope here. const PasswordVerifier verifier = PasswordHasher::parsePasswordVerifier(correctPasswordSha512); if (!verifier.isValid) { return NotLoggedIn; @@ -637,8 +631,7 @@ bool Servatrice_DatabaseInterface::submitPasswordVerifier(const QString &user, c qCWarning(DatabaseInterfaceLog) << "Failed to submit password verifier for user" << user << query->lastError(); return false; } - // The guard makes a re-migration a no-op; only report success when a row was actually updated. - return query->numRowsAffected() > 0; + return true; } int Servatrice_DatabaseInterface::getUserIdInDB(const QString &name) @@ -1222,13 +1215,13 @@ bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user, return false; } - const QString storedPassword = passwordQuery->value(0).toString(); - // oldPasswordNeedsHash means the client sent the old password in plaintext. Verify it - // against whatever is stored: legacy salt+hash rows or already-migrated scrypt rows - // (which must NOT be re-hashed with a salt torn out of the "$scrypt$..." string). - const bool oldPasswordMatches = oldPasswordNeedsHash ? PasswordHasher::verifyPassword(oldPassword, storedPassword) - : (oldPassword == storedPassword); - if (!oldPasswordMatches) { + const QString correctPasswordSha512 = passwordQuery->value(0).toString(); + QString oldPasswordSha512 = oldPassword; + if (oldPasswordNeedsHash) { + QString salt = correctPasswordSha512.left(16); + oldPasswordSha512 = PasswordHasher::computeHash(oldPassword, salt); + } + if (correctPasswordSha512 != oldPasswordSha512) { return false; } diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 36a3e5766..dc71c8e18 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -140,13 +140,11 @@ bool AbstractServerSocketInterface::initSession() identEvent.set_server_version(VERSION_STRING); identEvent.set_protocol_version(protocolVersion); if (servatrice->getAuthenticationMethod() == Servatrice::AuthenticationSql) { - // Challenge-response is advertised in every strictness mode: legacy accounts keep - // logging in with the legacy hash, but already-migrated scrypt rows are always - // served challenge-response (authentication_strictness only governs NEW credentials). - Event_ServerIdentification::ServerOptions serverOptions = - static_cast( - Event_ServerIdentification::SupportsPasswordHash | - Event_ServerIdentification::SupportsChallengeResponseAuth); + 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); @@ -267,6 +265,7 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedSessionComm return cmdReportDetails(cmd.GetExtension(Command_ReportDetails::ext), rc); case SessionCommand::SUBMIT_PASSWORD_VERIFIER: return cmdSubmitPasswordVerifier(cmd.GetExtension(Command_SubmitPasswordVerifier::ext), rc); + break; default: return Response::RespFunctionNotAllowed; } @@ -2471,8 +2470,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRegisterAccount(const C password = QString::fromStdString(cmd.hashed_password()); } - // Reject credential formats the configured authentication strictness does not accept. - if (!acceptsCredentialFormat(passwordNeedsHash, password)) { + // In strict mode only scrypt verifiers are accepted for new accounts. + if (servatrice->requiresChallengeResponseAuth() && + (passwordNeedsHash || PasswordHasher::isLegacyFormat(password))) { return Response::RespClientUpdateRequired; } @@ -2522,21 +2522,6 @@ bool AbstractServerSocketInterface::tooManyRegistrationAttempts(const QString &i return false; } -bool AbstractServerSocketInterface::acceptsCredentialFormat(bool passwordNeedsHash, const QString &password) const -{ - // "scryptFormat" means the client sent a derived verifier rather than a password to hash ourselves. - const bool scryptFormat = !passwordNeedsHash && !PasswordHasher::isLegacyFormat(password); - - // The strictness mode governs how existing legacy accounts are served, not which new-credential - // formats are tolerated: a legacy-mode server must still accept scrypt verifiers, because clients - // derive them whenever challenge-response is advertised (and it must be, so already-migrated - // scrypt rows keep logging in). strict is the only mode that rejects legacy formats. - if (servatrice->getAuthenticationStrictness() == Servatrice::AuthenticationStrict) { - return scryptFormat; - } - return true; // legacy and mixed accept either format -} - Response::ResponseCode AbstractServerSocketInterface::cmdActivateAccount(const Command_Activate &cmd, ResponseContainer & /*rc*/) { @@ -2871,8 +2856,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdAccountPassword(const C newPassword = QString::fromStdString(cmd.hashed_new_password()); } - // Reject new credential formats the configured authentication strictness does not accept. - if (!acceptsCredentialFormat(newPasswordNeedsHash, newPassword)) { + // In strict mode only scrypt verifiers are accepted. + if (servatrice->requiresChallengeResponseAuth() && + (newPasswordNeedsHash || PasswordHasher::isLegacyFormat(newPassword))) { return Response::RespClientUpdateRequired; } @@ -3013,8 +2999,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdForgotPasswordReset(con password = QString::fromStdString(cmd.hashed_new_password()); } - // Reject new credential formats the configured authentication strictness does not accept. - if (!acceptsCredentialFormat(passwordNeedsHash, password)) { + // In strict mode only scrypt verifiers are accepted. + if (servatrice->requiresChallengeResponseAuth() && + (passwordNeedsHash || PasswordHasher::isLegacyFormat(password))) { return Response::RespClientUpdateRequired; } @@ -3089,17 +3076,10 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con } 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); - // Legacy rows get a challenge-response nonce only outside legacy mode (there the client - // logs in with the legacy hash and the account is migrated). In legacy mode the row is - // served the legacy salt, since legacy mode only governs what NEW credentials are accepted. - if (servatrice->getAuthenticationStrictness() != Servatrice::AuthenticationLegacy) { - const QByteArray nonce = CryptoUtil::randomBytes(32); - setAuthNonce(nonce); - re->set_nonce(nonce.constData(), nonce.size()); - } } else { const PasswordVerifier verifier = PasswordHasher::parsePasswordVerifier(storedPasswordData); if (!verifier.isValid) { @@ -3111,7 +3091,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con re->set_r(verifier.r); re->set_p(verifier.p); re->set_needs_migration(false); - // scrypt rows are served challenge-response in every mode so migrated accounts never lock out. + } + + if (challengeResponseEnabled) { const QByteArray nonce = CryptoUtil::randomBytes(32); setAuthNonce(nonce); re->set_nonce(nonce.constData(), nonce.size()); @@ -3223,22 +3205,12 @@ AbstractServerSocketInterface::cmdSubmitPasswordVerifier(const Command_SubmitPas return Response::RespLoginNeeded; } - // Limit to the size of the database column (password_sha512 varchar(255)). - constexpr int MAX_PASSWORD_VERIFIER_LENGTH = 255; const QString passwordVerifier = QString::fromStdString(cmd.password_verifier()); - if (passwordVerifier.isEmpty() || passwordVerifier.length() > MAX_PASSWORD_VERIFIER_LENGTH || + if (passwordVerifier.isEmpty() || passwordVerifier.length() > MAX_NAME_LENGTH || PasswordHasher::isLegacyFormat(passwordVerifier)) { return Response::RespContextError; } - // Reject unparseable or hostile cost parameters before they reach the database. - const PasswordVerifier parsedVerifier = PasswordHasher::parsePasswordVerifier(passwordVerifier); - if (!parsedVerifier.isValid) { - qCWarning(AbstractServerSocketInterfaceLog) - << "Rejecting password verifier submission with invalid or insane cost parameters"; - return Response::RespContextError; - } - if (!sqlInterface->submitPasswordVerifier(QString::fromStdString(userInfo->name()), passwordVerifier)) { return Response::RespContextError; } diff --git a/servatrice/src/serversocketinterface.h b/servatrice/src/serversocketinterface.h index b393d2f17..6890d9e9f 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -80,7 +80,6 @@ signals: protected: void logDebugMessage(const QString &message) override; bool tooManyRegistrationAttempts(const QString &ipAddress); - bool acceptsCredentialFormat(bool passwordNeedsHash, const QString &password) const; virtual void writeToSocket(QByteArray &data) = 0; virtual void flushSocket() = 0; diff --git a/tests/password_hash_test.cpp b/tests/password_hash_test.cpp index 727677ee5..c1b5b22a9 100644 --- a/tests/password_hash_test.cpp +++ b/tests/password_hash_test.cpp @@ -104,69 +104,6 @@ TEST(PasswordHashTest, ConstantTimeEquals) ASSERT_FALSE(PasswordHasher::constantTimeEquals(QByteArray("short"), QByteArray("longer"))); } -TEST(PasswordHashTest, CostParamsAreSane) -{ - // Accept the recommended interactive parameters and the RFC 7914 test vector's. - ASSERT_TRUE(PasswordHasher::costParamsAreSane(SCRYPT_N, SCRYPT_R, SCRYPT_P)); - ASSERT_TRUE(PasswordHasher::costParamsAreSane(1024, 8, 16)); - - // n must be in [1024, 2**20] and a power of two. - ASSERT_FALSE(PasswordHasher::costParamsAreSane(512, 8, 1)); - ASSERT_FALSE(PasswordHasher::costParamsAreSane(1 << 21, 8, 1)); - ASSERT_FALSE(PasswordHasher::costParamsAreSane(1025, 8, 1)); - ASSERT_FALSE(PasswordHasher::costParamsAreSane(0, 8, 1)); - ASSERT_FALSE(PasswordHasher::costParamsAreSane(-1024, 8, 1)); - - // r in [1, 32], p in [1, 16]. - ASSERT_FALSE(PasswordHasher::costParamsAreSane(1024, 0, 1)); - ASSERT_FALSE(PasswordHasher::costParamsAreSane(1024, 33, 1)); - ASSERT_FALSE(PasswordHasher::costParamsAreSane(1024, 8, 0)); - ASSERT_FALSE(PasswordHasher::costParamsAreSane(1024, 8, 17)); -} - -TEST(PasswordHashTest, ParsePasswordVerifierRejectsHostileCostParams) -{ - // 16 bytes of salt and 64 bytes of verifier, base64 encoded. - const QString saltB64 = QLatin1String("c2FsdHNhbHRzYWx0c2FsdA=="); - const QString verifierB64 = QString(QByteArray(SCRYPT_VERIFIER_LENGTH, '\x42').toBase64()); - - ASSERT_TRUE( - PasswordHasher::parsePasswordVerifier(QString("$scrypt$1024$8$1$%1$%2").arg(saltB64).arg(verifierB64)).isValid); - - // n not a power of two, below 1024, or above 2**20. - ASSERT_FALSE( - PasswordHasher::parsePasswordVerifier(QString("$scrypt$1025$8$1$%1$%2").arg(saltB64).arg(verifierB64)).isValid); - ASSERT_FALSE( - PasswordHasher::parsePasswordVerifier(QString("$scrypt$512$8$1$%1$%2").arg(saltB64).arg(verifierB64)).isValid); - ASSERT_FALSE( - PasswordHasher::parsePasswordVerifier(QString("$scrypt$1073741824$8$1$%1$%2").arg(saltB64).arg(verifierB64)) - .isValid); - - // r and p out of range. - ASSERT_FALSE(PasswordHasher::parsePasswordVerifier(QString("$scrypt$1024$33$1$%1$%2").arg(saltB64).arg(verifierB64)) - .isValid); - ASSERT_FALSE(PasswordHasher::parsePasswordVerifier(QString("$scrypt$1024$8$17$%1$%2").arg(saltB64).arg(verifierB64)) - .isValid); -} - -TEST(PasswordHashTest, VerifyPasswordLegacyRow) -{ - const QString salt = PasswordHasher::generateRandomSalt(); - const QString legacyStored = PasswordHasher::computeHash("correct horse", salt); - ASSERT_TRUE(PasswordHasher::verifyPassword("correct horse", legacyStored)); - ASSERT_FALSE(PasswordHasher::verifyPassword("battery staple", legacyStored)); -} - -TEST(PasswordHashTest, VerifyPasswordScryptRow) -{ - const QString scryptStored = PasswordHasher::generatePasswordVerifier("correct horse"); - // Regression for the changeUserPassword bug that re-hashed the old password with - // a 16-char salt torn out of the "$scrypt$..." string, which could never match. - ASSERT_TRUE(PasswordHasher::verifyPassword("correct horse", scryptStored)); - ASSERT_FALSE(PasswordHasher::verifyPassword("battery staple", scryptStored)); - ASSERT_FALSE(PasswordHasher::verifyPassword("correct horse", "garbage")); -} - } // namespace int main(int argc, char **argv)