[Security] Refuse KDF downgrade; trim profile name and warn on silent credential drop

Challenge-response authentication: the client derives a scrypt verifier
(RFC 7914, EVP_PBE_scrypt, N=32768, r=8, p=1) and authenticates with
HMAC-SHA256(key, nonce), so neither the password nor its hash is
transmitted. The stored format becomes
"$scrypt$<n>$<r>$<p>$<salt>$<verifier>" and Response_PasswordSalt
now carries the cost parameters. Strict servers only accept scrypt
verifiers; legacy accounts are migrated after a successful login.

Fix #344 for challenge-response servers: a saved profile stores the
derived verifier under the password key instead of the plaintext password.
The connect dialog loads it without revealing it, autoconnect passes it
through, the change-password dialog no longer prefills the old password
field with it, and the client only persists the verifier when
"Save password" is checked.

Took 3 minutes

Took 1 minute

Took 10 seconds

Took 7 minutes

.ci/Arch/Dockerfile

@@ -8,6 +8,7 @@ RUN pacman --sync --refresh --sysupgrade --needed --noconfirm \
         gtest \
         mariadb-libs \
         ninja \
+        openssl \
         protobuf \
         qt6-base \
         qt6-declarative \

.ci/Debian12/Dockerfile

@@ -15,6 +15,7 @@ RUN apt-get update && \
         libprotobuf-dev \
         libqt6multimedia6 \
         libqt6sql6-mysql \
+        libssl-dev \
         ninja-build \
         protobuf-compiler \
         qt6-image-formats-plugins \

.ci/Debian13/Dockerfile

@@ -16,6 +16,7 @@ RUN apt-get update && \
         libprotobuf-dev \
         libqt6multimedia6 \
         libqt6sql6-mysql \
+        libssl-dev \
         ninja-build \
         protobuf-compiler \
         qt6-image-formats-plugins \

.ci/Fedora43/Dockerfile

@@ -7,6 +7,7 @@ RUN dnf install -y \
         git \
         mariadb-devel \
         ninja-build \
+        openssl-devel \
         protobuf-devel \
         qt6-{qtdeclarative,qtshadertools,qttools,qtsvg,qtmultimedia,qtwebsockets}-devel \
         qt6-qtimageformats \

.ci/Fedora44/Dockerfile

@@ -7,6 +7,7 @@ RUN dnf install -y \
         git \
         mariadb-devel \
         ninja-build \
+        openssl-devel \
         protobuf-devel \
         qt6-{qtdeclarative,qtshadertools,qttools,qtsvg,qtmultimedia,qtwebsockets}-devel \
         qt6-qtimageformats \

.ci/Servatrice_Debian12/Dockerfile

@@ -12,6 +12,7 @@ RUN apt-get update && \
         libmariadb-dev-compat \
         libprotobuf-dev \
         libqt6sql6-mysql \
+        libssl-dev \
         ninja-build \
         protobuf-compiler \
         qt6-tools-dev \

.ci/Ubuntu24.04/Dockerfile

@@ -15,6 +15,7 @@ RUN apt-get update && \
         libprotobuf-dev \
         libqt6multimedia6 \
         libqt6sql6-mysql \
+        libssl-dev \
         ninja-build \
         protobuf-compiler \
         qt6-image-formats-plugins \

.ci/Ubuntu26.04/Dockerfile

@@ -16,6 +16,7 @@ RUN apt-get update && \
         libprotobuf-dev \
         libqt6multimedia6 \
         libqt6sql6-mysql \
+        libssl-dev \
         ninja-build \
         protobuf-compiler \
         qt6-image-formats-plugins \

CMakeLists.txt

@@ -5,8 +5,8 @@
 # This file sets all the variables shared between the projects
 # like the installation path, compilation flags etc..
 
-# cmake 3.16 is required if using qt6
-cmake_minimum_required(VERSION 3.10)
+# 3.16 required for Qt6 and target_precompile_headers()
+cmake_minimum_required(VERSION 3.16)
 
 # Use compiler cache (ccache)
 option(USE_CCACHE "Cache the build results with ccache" OFF)
@@ -184,6 +184,9 @@ elseif(CMAKE_COMPILER_IS_GNUCXX)
       set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} ${FLAG}")
     endif()
   endforeach()
+
+  # Reduce compiler I/O by using pipes between stages instead of temp files
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pipe")
 else()
   # other: osx/llvm, bsd/llvm
   set(CMAKE_CXX_FLAGS_RELEASE "-O2")
@@ -192,6 +195,9 @@ else()
   else()
     set(CMAKE_CXX_FLAGS_DEBUG "-g -O0 -Wall -Wextra")
   endif()
+
+  # Reduce compiler I/O by using pipes between stages instead of temp files
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pipe")
 endif()
 
 # GNU systems need to define the Mersenne exponent for the RNG to compile w/o warning
@@ -239,11 +245,6 @@ 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()
 

Dockerfile

@@ -14,6 +14,7 @@ RUN apt-get update \
       libmariadb-dev-compat \
       libprotobuf-dev \
       libqt6sql6-mysql \
+      libssl-dev \
       qt6-websockets-dev \
       protobuf-compiler \
       qt6-tools-dev \
@@ -42,6 +43,7 @@ RUN apt-get update \
       libprotobuf32t64 \
       libqt6sql6-mysql \
       libqt6websockets6 \
+      libssl3 \
     && apt-get clean \
     && rm -rf /var/lib/apt/lists/*
 

cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp

@@ -83,6 +83,9 @@ void ConnectionController::wireClientSignals()
 
     connect(remoteClient, &RemoteClient::sigPromptForForgotPasswordChallenge, this,
             &ConnectionController::onPromptForgotPasswordChallenge);
+
+    connect(remoteClient, &RemoteClient::sigPasswordVerifierReady, this,
+            &ConnectionController::onPasswordVerifierReady);
 }
 
 void ConnectionController::connectToServer()
@@ -91,16 +94,32 @@ void ConnectionController::connectToServer()
     connect(dlgConnect, &DlgConnect::sigStartForgotPasswordRequest, this, &ConnectionController::forgotPasswordRequest);
 
     if (dlgConnect->exec()) {
+        pendingSaveName = dlgConnect->getSaveName();
+        pendingSavePassword = dlgConnect->getSavePassword();
+        remoteClient->setStoredVerifier(dlgConnect->getStoredVerifier());
         remoteClient->connectToServer(dlgConnect->getHost(), static_cast<unsigned int>(dlgConnect->getPort()),
                                       dlgConnect->getPlayerName(), dlgConnect->getPassword());
     }
 }
 
+void ConnectionController::onPasswordVerifierReady(const QString &verifier)
+{
+    if (pendingSavePassword) {
+        SettingsCache::instance().servers().setServerPassword(pendingSaveName, verifier);
+    }
+}
+
 void ConnectionController::connectToServerDirect(const QString &host,
                                                  unsigned int port,
                                                  const QString &playerName,
-                                                 const QString &password)
+                                                 const QString &password,
+                                                 const QString &storedVerifier,
+                                                 const QString &saveName,
+                                                 bool savePassword)
 {
+    pendingSaveName = saveName;
+    pendingSavePassword = savePassword;
+    remoteClient->setStoredVerifier(storedVerifier);
     remoteClient->connectToServer(host, port, playerName, password);
 }
 

cockatrice/src/client/network/connection_controller/remote_connection_controller.h

@@ -35,8 +35,13 @@ public:
     void registerToServer();
     void forgotPasswordRequest();
     void connectToServer();
-    void
-    connectToServerDirect(const QString &host, unsigned int port, const QString &playerName, const QString &password);
+    void connectToServerDirect(const QString &host,
+                               unsigned int port,
+                               const QString &playerName,
+                               const QString &password,
+                               const QString &storedVerifier = QString(),
+                               const QString &saveName = QString(),
+                               bool savePassword = false);
     void disconnectFromServer();
 
     void refreshWindowTitle()
@@ -81,6 +86,9 @@ private slots:
     void onPromptForgotPasswordReset();
     void onPromptForgotPasswordChallenge();
 
+    // Persists the derived scrypt verifier after a successful challenge-response login
+    void onPasswordVerifierReady(const QString &verifier);
+
 private:
     void wireClientSignals();
     void updateWindowTitle();
@@ -97,6 +105,10 @@ private:
 
     // Kept as a member so the forgot-password signal can be wired to it
     DlgConnect *dlgConnect{nullptr};
+
+    // Captured from the connect dialog when a connection is initiated
+    QString pendingSaveName;
+    bool pendingSavePassword{false};
 };
 
 #endif // COCKATRICE_REMOTE_CONNECTION_CONTROLLER_H

cockatrice/src/game_graphics/game_scene.cpp

@@ -44,11 +44,16 @@ GameScene::GameScene(PhasesToolbar *_phasesToolbar, QObject *parent)
 
 GameScene::~GameScene()
 {
-    // Sever all incoming connections (animated item destroy-tracking) before the
-    // members below are destroyed: the base QGraphicsScene destructor destroys the
-    // remaining items, and their destroyed() signals must not reach slots that
-    // reference members that no longer exist.
-    QObject::disconnect(nullptr, nullptr, this, nullptr);
+    // Sever all destroyed->removeAnimatedItem connections before the members below
+    // are destroyed: the base QGraphicsScene destructor destroys the remaining items,
+    // and their destroyed() signals must not reach slots that reference members that
+    // no longer exist. The connection handle overload is used because the string-based
+    // disconnect(nullptr, nullptr, this, nullptr) is invalid (the sender must never be
+    // nullptr) and would otherwise fail to sever these pointer-to-member connections.
+    for (auto it = animationItemConnections.constBegin(); it != animationItemConnections.constEnd(); ++it) {
+        QObject::disconnect(*it);
+    }
+    animationItemConnections.clear();
 
     delete animationTimer;
     animationTimer = nullptr;
@@ -777,8 +782,15 @@ void GameScene::registerAnimationItem(IAnimatedItem *item)
     if (!object) {
         return;
     }
-    if (!animatedItems.contains(object)) {
-        connect(object, &QObject::destroyed, this, &GameScene::removeAnimatedItem);
+    // Guard against duplicate connections using the connection map, not
+    // animatedItems: the animation timer removes entries from animatedItems when an
+    // animation completes, but the destroyed->removeAnimatedItem connection must
+    // persist until the object is destroyed. Relying on animatedItems here would let
+    // a re-registered item (e.g. a life counter that flashes repeatedly) accumulate
+    // duplicate destroyed connections, the older ones of which would survive teardown.
+    if (!animationItemConnections.contains(object)) {
+        animationItemConnections.insert(object,
+                                        connect(object, &QObject::destroyed, this, &GameScene::removeAnimatedItem));
     }
     animatedItems.insert(object, item);
     if (animationTimer && !animationTimer->isActive()) {
@@ -797,6 +809,7 @@ void GameScene::unregisterAnimationItem(IAnimatedItem *item)
 void GameScene::removeAnimatedItem(QObject *item)
 {
     animatedItems.remove(item);
+    animationItemConnections.remove(item);
     if (animationTimer && animatedItems.isEmpty()) {
         animationTimer->stop();
     }

cockatrice/src/game_graphics/game_scene.h

@@ -54,9 +54,11 @@ private:
     QPointer<CardItem> hoveredCard;                     ///< Currently hovered card
     QBasicTimer *animationTimer;                        ///< Timer for scene animations
     QHash<QObject *, IAnimatedItem *> animatedItems;    ///< Items currently animating
-    int playerRotation;                                 ///< Rotation offset for player layout
-    bool rearranging = false;                           ///< Guard against re-entrant rearrange
-    bool needsReArrange = false;                        ///< Pending rearrange requested during a pass
+    QHash<QObject *, QMetaObject::Connection>
+        animationItemConnections; ///< destroyed->removeAnimatedItem handles per animated item
+    int playerRotation;           ///< Rotation offset for player layout
+    bool rearranging = false;     ///< Guard against re-entrant rearrange
+    bool needsReArrange = false;  ///< Pending rearrange requested during a pass
 
     /**
      * @brief Updates which card is currently hovered based on scene coordinates.

cockatrice/src/game_graphics/zones/hand_zone.cpp

@@ -41,7 +41,8 @@ void HandZone::handleDropEvent(const QList<CardDragItem *> &dragItems,
             }
         }
     } else {
-        x = calcDropIndexFromY(dropPoint.y());
+        bool sameZone = startZone == getLogic();
+        x = calcDropIndexFromY(dropPoint.y(), !sameZone);
     }
 
     Command_MoveCard cmd;

cockatrice/src/game_graphics/zones/select_zone.cpp

@@ -83,7 +83,7 @@ SelectZone::StackLayoutParams SelectZone::buildStackParams(qreal minOffset) cons
     return {cardCount, boundingRect().height(), cardHeight, offset, minOffset};
 }
 
-int SelectZone::calcDropIndexFromY(qreal dropY, qreal minOffset) const
+int SelectZone::calcDropIndexFromY(qreal dropY, bool allowCountExpand, qreal minOffset) const
 {
     const auto &cards = getLogic()->getCards();
     if (cards.isEmpty()) {
@@ -94,7 +94,8 @@ int SelectZone::calcDropIndexFromY(qreal dropY, qreal minOffset) const
     if (effectiveOffset <= 0.0) {
         return 0;
     }
-    return qBound(0, qRound((dropY - start) / effectiveOffset), params.cardCount - 1);
+    int max = allowCountExpand ? params.cardCount : params.cardCount - 1;
+    return qBound(0, qRound((dropY - start) / effectiveOffset), max);
 }
 
 void SelectZone::restoreStaleEscapedCards()

cockatrice/src/game_graphics/zones/select_zone.h

@@ -104,8 +104,12 @@ protected:
     /**
      * @brief Computes the card index at a given y-coordinate within the zone's vertical layout.
      * Returns 0 if the zone has no cards or the offset is zero.
+     *
+     * @param dropY The y-coordinate that the card was dropped at
+     * @param allowCountExpand If false, clamps the index at the number of cards minus 1
+     * @param minOffset Minimum offset to preserve
      */
-    int calcDropIndexFromY(qreal dropY, qreal minOffset = 0.0) const;
+    int calcDropIndexFromY(qreal dropY, bool allowCountExpand, qreal minOffset = 0.0) const;
 
     /**
      * @brief Positions cards vertically with alternating left/right x-offsets.

cockatrice/src/game_graphics/zones/stack_zone.cpp

@@ -57,18 +57,14 @@ void StackZone::handleDropEvent(const QList<CardDragItem *> &dragItems,
         return;
     }
 
-    const auto &cards = getLogic()->getCards();
-    int index;
-    if (startZone == getLogic()) {
-        // Reordering within the zone: use drop position
-        index = calcDropIndexFromY(dropPoint.y(), MIN_CARD_VISIBLE);
+    bool sameZone = startZone == getLogic();
+    int index = calcDropIndexFromY(dropPoint.y(), !sameZone, MIN_CARD_VISIBLE);
+    if (sameZone) {
         // Same-zone no-op: don't move a card onto itself
+        const auto &cards = getLogic()->getCards();
         if (!cards.isEmpty() && cards.at(index)->getId() == dragItems.at(0)->getId()) {
             return;
         }
-    } else {
-        // Coming from another zone: append at end (top of stack, rendered on top)
-        index = static_cast<int>(cards.size());
     }
 
     Command_MoveCard cmd;

cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp

@@ -273,9 +273,15 @@ void DlgConnect::updateDisplayInfo(const QString &saveName)
     playernameEdit->setText(_data.at(3));
     playernameEdit->setFocus();
     savePasswordCheckBox->setChecked(savePasswordStatus);
+    storedVerifier.clear();
 
     if (savePasswordStatus) {
-        passwordEdit->setText(_data.at(4));
+        const QString stored = _data.at(4);
+        if (stored.startsWith("$")) {
+            storedVerifier = stored;
+        } else {
+            passwordEdit->setText(stored);
+        }
     }
 
     if (!_data.at(6).isEmpty()) {
@@ -301,6 +307,7 @@ void DlgConnect::newHostSelected(bool state)
         portEdit->setDisabled(false);
         playernameEdit->clear();
         passwordEdit->clear();
+        storedVerifier.clear();
         saveEdit->clear();
         saveEdit->setPlaceholderText(tr("Unique Server Name"));
         saveEdit->setDisabled(false);
@@ -326,6 +333,11 @@ void DlgConnect::actOk()
 {
     ServersSettings &servers = SettingsCache::instance().servers();
 
+    // Never write a newly typed plaintext password to disk when a verifier is already stored:
+    // the typed value is used for this connection and a fresh verifier is persisted after a
+    // successful login. Without a stored verifier we keep the previous (plaintext legacy) behavior.
+    const QString passwordToSave = storedVerifier.isEmpty() ? passwordEdit->text() : storedVerifier;
+
     if (newHostButton->isChecked()) {
         if (saveEdit->text().isEmpty()) {
             QMessageBox::critical(this, tr("Connection Warning"), tr("You need to name your new connection profile."));
@@ -333,10 +345,10 @@ void DlgConnect::actOk()
         }
 
         servers.addNewServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(),
-                             playernameEdit->text().trimmed(), passwordEdit->text(), savePasswordCheckBox->isChecked());
+                             playernameEdit->text().trimmed(), passwordToSave, savePasswordCheckBox->isChecked());
     } else {
         servers.updateExistingServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(),
-                                     playernameEdit->text().trimmed(), passwordEdit->text(),
+                                     playernameEdit->text().trimmed(), passwordToSave,
                                      savePasswordCheckBox->isChecked());
     }
 

cockatrice/src/interface/widgets/dialogs/dlg_connect.h

@@ -10,6 +10,7 @@
 #include "../interface/widgets/server/handle_public_servers.h"
 #include "../interface/widgets/server/user/user_info_connection.h"
 
+#include <QCheckBox>
 #include <QDialog>
 #include <QLineEdit>
 #include <libcockatrice/utility/macros.h>
@@ -47,6 +48,22 @@ public:
     {
         return passwordEdit->text();
     }
+    //! \brief Stored "$scrypt$..." verifier for challenge-response servers (never the plaintext password).
+    [[nodiscard]] QString getStoredVerifier() const
+    {
+        return storedVerifier;
+    }
+    [[nodiscard]] QString getSaveName() const
+    {
+        // Trim so the profile lookup key always matches the name DlgConnect::actOk
+        // stored (it saves under saveEdit->text().trimmed()); otherwise a trailing
+        // space here misses the index and silently drops the saved verifier.
+        return saveEdit->text().trimmed();
+    }
+    [[nodiscard]] bool getSavePassword() const
+    {
+        return savePasswordCheckBox->isChecked();
+    }
 
 public slots:
     void downloadThePublicServers();
@@ -77,6 +94,7 @@ private:
     QPushButton *btnConnect, *btnForgotPassword, *btnRefreshServers, *btnDeleteServer;
     QMap<QString, std::pair<QString, UserConnection_Information>> savedHostList;
     HandlePublicServers *hps;
+    QString storedVerifier;
     const QString placeHolderText = tr("Downloading...");
 };
 #endif

cockatrice/src/interface/widgets/dialogs/dlg_edit_password.cpp

@@ -18,7 +18,10 @@ DlgEditPassword::DlgEditPassword(QWidget *parent) : QDialog(parent)
 
     auto &servers = SettingsCache::instance().servers();
     if (servers.getSavePassword()) {
-        oldPasswordEdit->setText(servers.getPassword());
+        const QString stored = servers.getPassword();
+        if (!stored.startsWith("$")) {
+            oldPasswordEdit->setText(stored);
+        }
     }
 
     oldPasswordLabel->setBuddy(oldPasswordEdit);

cockatrice/src/interface/widgets/server/user/user_info_box.cpp

@@ -283,7 +283,9 @@ void UserInfoBox::changePassword(const QString &oldPassword, const QString &newP
 {
     Command_AccountPassword cmd;
     cmd.set_old_password(oldPassword.toStdString());
-    if (client->getServerSupportsPasswordHash()) {
+    if (client->getServerSupportsChallengeResponse()) {
+        cmd.set_hashed_new_password(PasswordHasher::generatePasswordVerifier(newPassword).toStdString());
+    } else if (client->getServerSupportsPasswordHash()) {
         auto passwordSalt = PasswordHasher::generateRandomSalt();
         QString hashedPassword = PasswordHasher::computeHash(newPassword, passwordSalt);
         cmd.set_hashed_new_password(hashedPassword.toStdString());

cockatrice/src/interface/widgets/settings_page/appearance_settings_page.cpp

@@ -154,6 +154,48 @@ AppearanceSettingsPage::AppearanceSettingsPage()
     homeTabGroupBox = new QGroupBox;
     homeTabGroupBox->setLayout(homeTabGrid);
 
+    // Playmat settings
+    playmatVisibilityCombo.addItem(tr("Show all playmats"), PlaymatVisibilityAll);
+    playmatVisibilityCombo.addItem(tr("Show own playmat only"), PlaymatVisibilityOwnOnly);
+    playmatVisibilityCombo.addItem(tr("Don't use playmats"), PlaymatVisibilityNone);
+    int visIdx = playmatVisibilityCombo.findData(settings.userInterface().getPlaymatVisibility());
+    if (visIdx >= 0) {
+        playmatVisibilityCombo.setCurrentIndex(visIdx);
+    }
+    connect(&playmatVisibilityCombo, qOverload<int>(&QComboBox::currentIndexChanged), this, [this](int index) {
+        SettingsCache::instance().userInterface().setPlaymatVisibility(playmatVisibilityCombo.itemData(index).toInt());
+    });
+    playmatVisibilityLabel.setBuddy(&playmatVisibilityCombo);
+
+    // Playmat mode: Override / Fallback / Deck-only
+    playmatModeCombo.addItem(tr("Override deck playmat"), PlaymatModeOverrideDeck);
+    playmatModeCombo.addItem(tr("Fallback if deck has none"), PlaymatModeFallback);
+    playmatModeCombo.addItem(tr("Deck only, ignore collection"), PlaymatModeDeckOnly);
+    int modeIdx = playmatModeCombo.findData(settings.userInterface().getPlaymatMode());
+    if (modeIdx >= 0) {
+        playmatModeCombo.setCurrentIndex(modeIdx);
+    }
+    connect(&playmatModeCombo, qOverload<int>(&QComboBox::currentIndexChanged), this, [this](int index) {
+        SettingsCache::instance().userInterface().setPlaymatMode(playmatModeCombo.itemData(index).toInt());
+    });
+    playmatModeLabel.setBuddy(&playmatModeCombo);
+
+    // User-level playmat settings: fallback collection.
+    connect(&playmatDefaultEditButton, &QPushButton::clicked, this,
+            &AppearanceSettingsPage::openPlaymatCollectionDialog);
+
+    auto *playmatGrid = new QGridLayout;
+    playmatGrid->addWidget(&playmatVisibilityLabel, 0, 0, 1, 1);
+    playmatGrid->addWidget(&playmatVisibilityCombo, 0, 1, 1, 1);
+    playmatGrid->addWidget(&playmatModeLabel, 1, 0, 1, 1);
+    playmatGrid->addWidget(&playmatModeCombo, 1, 1, 1, 1);
+    playmatGrid->addWidget(&playmatDefaultLabel, 2, 0, 1, 1);
+    playmatGrid->addWidget(&playmatDefaultEditButton, 2, 1, 1, 1);
+
+    playmatGroupBox = new QGroupBox;
+    playmatGroupBox->setLayout(playmatGrid);
+
+    // Styling settings
     styleUserListCheckBox.setChecked(settings.appearance().getStyleUserList());
     connect(&styleUserListCheckBox, &QCheckBox::QT_STATE_CHANGED, &settings.appearance(),
             &AppearanceSettings::setStyleUserList);
@@ -259,7 +301,6 @@ AppearanceSettingsPage::AppearanceSettingsPage()
     cardLayoutGroupBox->setLayout(cardLayoutGrid);
 
     // Card counter colors
-
     auto *cardCounterColorsLayout = new QGridLayout;
     cardCounterColorsLayout->setColumnStretch(1, 1);
     cardCounterColorsLayout->setColumnStretch(3, 1);
@@ -339,47 +380,6 @@ AppearanceSettingsPage::AppearanceSettingsPage()
     tableGroupBox = new QGroupBox;
     tableGroupBox->setLayout(tableGrid);
 
-    // Playmat settings
-    playmatVisibilityCombo.addItem(tr("Show all playmats"), PlaymatVisibilityAll);
-    playmatVisibilityCombo.addItem(tr("Show own playmat only"), PlaymatVisibilityOwnOnly);
-    playmatVisibilityCombo.addItem(tr("Don't use playmats"), PlaymatVisibilityNone);
-    int visIdx = playmatVisibilityCombo.findData(settings.userInterface().getPlaymatVisibility());
-    if (visIdx >= 0) {
-        playmatVisibilityCombo.setCurrentIndex(visIdx);
-    }
-    connect(&playmatVisibilityCombo, qOverload<int>(&QComboBox::currentIndexChanged), this, [this](int index) {
-        SettingsCache::instance().userInterface().setPlaymatVisibility(playmatVisibilityCombo.itemData(index).toInt());
-    });
-    playmatVisibilityLabel.setBuddy(&playmatVisibilityCombo);
-
-    // Playmat mode: Override / Fallback / Deck-only
-    playmatModeCombo.addItem(tr("Override deck playmat"), PlaymatModeOverrideDeck);
-    playmatModeCombo.addItem(tr("Fallback if deck has none"), PlaymatModeFallback);
-    playmatModeCombo.addItem(tr("Deck only, ignore collection"), PlaymatModeDeckOnly);
-    int modeIdx = playmatModeCombo.findData(settings.userInterface().getPlaymatMode());
-    if (modeIdx >= 0) {
-        playmatModeCombo.setCurrentIndex(modeIdx);
-    }
-    connect(&playmatModeCombo, qOverload<int>(&QComboBox::currentIndexChanged), this, [this](int index) {
-        SettingsCache::instance().userInterface().setPlaymatMode(playmatModeCombo.itemData(index).toInt());
-    });
-    playmatModeLabel.setBuddy(&playmatModeCombo);
-
-    // User-level playmat settings: fallback collection.
-    connect(&playmatDefaultEditButton, &QPushButton::clicked, this,
-            &AppearanceSettingsPage::openPlaymatCollectionDialog);
-
-    auto *playmatGrid = new QGridLayout;
-    playmatGrid->addWidget(&playmatVisibilityLabel, 0, 0, 1, 1);
-    playmatGrid->addWidget(&playmatVisibilityCombo, 0, 1, 1, 1);
-    playmatGrid->addWidget(&playmatModeLabel, 1, 0, 1, 1);
-    playmatGrid->addWidget(&playmatModeCombo, 1, 1, 1, 1);
-    playmatGrid->addWidget(&playmatDefaultLabel, 2, 0, 1, 1);
-    playmatGrid->addWidget(&playmatDefaultEditButton, 2, 1, 1, 1);
-
-    playmatGroupBox = new QGroupBox;
-    playmatGroupBox->setLayout(playmatGrid);
-
     // putting it all together
     auto *mainLayout = new QVBoxLayout;
     mainLayout->addWidget(themeGroupBox);
@@ -512,6 +512,12 @@ void AppearanceSettingsPage::retranslateUi()
     homeTabButtonColorSourceBox.setToolTip(
         tr("Automatic: extract from background if present, otherwise use theme default"));
 
+    playmatGroupBox->setTitle(tr("Playmat settings"));
+    playmatVisibilityLabel.setText(tr("Playmat visibility:"));
+    playmatModeLabel.setText(tr("Default collection behavior:"));
+    playmatDefaultLabel.setText(tr("Default playmat collection:"));
+    playmatDefaultEditButton.setText(tr("Edit..."));
+
     stylingGroupBox->setTitle(tr("Styling settings"));
     styleUserListCheckBox.setText(tr("Style user list"));
 
@@ -554,9 +560,4 @@ void AppearanceSettingsPage::retranslateUi()
     tableGroupBox->setTitle(tr("Table grid layout"));
     invertVerticalCoordinateCheckBox.setText(tr("Invert vertical coordinate"));
     minPlayersForMultiColumnLayoutLabel.setText(tr("Minimum player count for multi-column layout:"));
-    playmatGroupBox->setTitle(tr("Playmat settings"));
-    playmatVisibilityLabel.setText(tr("Playmat visibility:"));
-    playmatModeLabel.setText(tr("Default collection behavior:"));
-    playmatDefaultLabel.setText(tr("Default playmat collection:"));
-    playmatDefaultEditButton.setText(tr("Edit..."));
 }

cockatrice/src/interface/widgets/settings_page/appearance_settings_page.h

@@ -44,46 +44,55 @@ private:
     QLabel homeTabButtonColorSourceLabel;
     QComboBox homeTabButtonColorSourceBox;
 
-    QCheckBox styleUserListCheckBox;
-    QCheckBox showShortcutsCheckBox;
-    QCheckBox showGameSelectorFilterToolbarCheckBox;
-    QLabel minPlayersForMultiColumnLayoutLabel;
-    QLabel maxFontSizeForCardsLabel;
-    QCheckBox overrideAllCardArtWithPersonalPreferenceCheckBox;
-    QCheckBox bumpSetsWithCardsInDeckToTopCheckBox;
-    QCheckBox displayCardNamesCheckBox;
-    QCheckBox autoRotateSidewaysLayoutCardsCheckBox;
-    QCheckBox cardScalingCheckBox;
-    QCheckBox roundCardCornersCheckBox;
-    QLabel verticalCardOverlapPercentLabel;
-    QSpinBox verticalCardOverlapPercentBox;
-    QLabel cardViewInitialRowsMaxLabel;
-    QSpinBox cardViewInitialRowsMaxBox;
-    QLabel cardViewExpandedRowsMaxLabel;
-    QSpinBox cardViewExpandedRowsMaxBox;
-    QCheckBox horizontalHandCheckBox;
-    QCheckBox leftJustifiedHandCheckBox;
-    QCheckBox invertVerticalCoordinateCheckBox;
     QLabel playmatVisibilityLabel;
     QComboBox playmatVisibilityCombo;
     QLabel playmatModeLabel;
     QComboBox playmatModeCombo;
     QLabel playmatDefaultLabel;
     QPushButton playmatDefaultEditButton;
+
+    QCheckBox styleUserListCheckBox;
+
+    QCheckBox showShortcutsCheckBox;
+    QCheckBox showGameSelectorFilterToolbarCheckBox;
+
+    QCheckBox overrideAllCardArtWithPersonalPreferenceCheckBox;
+    QCheckBox bumpSetsWithCardsInDeckToTopCheckBox;
+
+    QCheckBox displayCardNamesCheckBox;
+    QCheckBox autoRotateSidewaysLayoutCardsCheckBox;
+    QCheckBox cardScalingCheckBox;
+    QCheckBox roundCardCornersCheckBox;
+    QLabel maxFontSizeForCardsLabel;
+    QSpinBox maxFontSizeForCardsEdit;
+
+    QLabel verticalCardOverlapPercentLabel;
+    QSpinBox verticalCardOverlapPercentBox;
+    QLabel cardViewInitialRowsMaxLabel;
+    QSpinBox cardViewInitialRowsMaxBox;
+    QLabel cardViewExpandedRowsMaxLabel;
+    QSpinBox cardViewExpandedRowsMaxBox;
+
+    QList<QLabel *> cardCounterNames;
+
+    QCheckBox horizontalHandCheckBox;
+    QCheckBox leftJustifiedHandCheckBox;
+
+    QCheckBox invertVerticalCoordinateCheckBox;
+    QLabel minPlayersForMultiColumnLayoutLabel;
+    QSpinBox minPlayersForMultiColumnLayoutEdit;
+
     QGroupBox *themeGroupBox;
     QGroupBox *homeTabGroupBox;
+    QGroupBox *playmatGroupBox;
     QGroupBox *stylingGroupBox;
     QGroupBox *menuGroupBox;
     QGroupBox *printingsGroupBox;
     QGroupBox *cardsGroupBox;
     QGroupBox *cardLayoutGroupBox;
-    QGroupBox *handGroupBox;
-    QGroupBox *playmatGroupBox;
-    QGroupBox *tableGroupBox;
     QGroupBox *cardCountersGroupBox;
-    QList<QLabel *> cardCounterNames;
-    QSpinBox minPlayersForMultiColumnLayoutEdit;
-    QSpinBox maxFontSizeForCardsEdit;
+    QGroupBox *handGroupBox;
+    QGroupBox *tableGroupBox;
 
 public:
     AppearanceSettingsPage();

cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp

@@ -425,29 +425,28 @@ void GeneralSettingsPage::updateStartupServerControlsVisibility()
 
 void GeneralSettingsPage::retranslateUi()
 {
+    const auto &settings = SettingsCache::instance();
+
     languageGroupBox->setTitle(tr("Language settings"));
     languageLabel.setText(tr("Language:"));
-
-    versionGroupBox->setTitle(tr("Version settings"));
-    cardDatabaseGroupBox->setTitle(tr("Card database"));
-    startupGroupBox->setTitle(tr("Startup settings"));
-
-    if (SettingsCache::instance().getIsPortableBuild()) {
-        pathsGroupBox->setTitle(tr("Paths (editing disabled in portable mode)"));
-    } else {
-        pathsGroupBox->setTitle(tr("Paths"));
-    }
     advertiseTranslationPageLabel.setText(
         QString("<a href='%1'>%2</a>").arg(WIKI_TRANSLATION_FAQ).arg(tr("How to help with translations")));
-    deckPathLabel.setText(tr("Decks directory:"));
-    filtersPathLabel.setText(tr("Filters directory:"));
-    replaysPathLabel.setText(tr("Replays directory:"));
-    picsPathLabel.setText(tr("Pictures directory:"));
-    cardDatabasePathLabel.setText(tr("Card database:"));
-    customCardDatabasePathLabel.setText(tr("Custom database directory:"));
-    tokenDatabasePathLabel.setText(tr("Token database:"));
+
+    versionGroupBox->setTitle(tr("Version settings"));
     updateReleaseChannelLabel.setText(tr("Update channel"));
     startupUpdateCheckCheckBox.setText(tr("Check for client updates on startup"));
+    updateNotificationCheckBox.setText(tr("Notify if a feature supported by the server is missing in my client"));
+    newVersionOracleCheckBox.setText(tr("Automatically run Oracle when running a new version of Cockatrice"));
+
+    // We can't change the strings after they're put into the QComboBox, so this is our workaround
+    int oldIndex = updateReleaseChannelBox.currentIndex();
+    updateReleaseChannelBox.clear();
+    for (ReleaseChannel *chan : settings.getUpdateReleaseChannels()) {
+        updateReleaseChannelBox.addItem(tr(chan->getName().toUtf8()));
+    }
+    updateReleaseChannelBox.setCurrentIndex(oldIndex);
+
+    cardDatabaseGroupBox->setTitle(tr("Card database"));
     startupCardUpdateCheckBehaviorLabel.setText(tr("Check for card database updates on startup"));
     startupCardUpdateCheckBehaviorSelector.setItemText(startupCardUpdateCheckBehaviorIndexNone, tr("Don't check"));
     startupCardUpdateCheckBehaviorSelector.setItemText(startupCardUpdateCheckBehaviorIndexPrompt,
@@ -456,8 +455,13 @@ void GeneralSettingsPage::retranslateUi()
                                                        tr("Always update in the background"));
     cardUpdateCheckIntervalLabel.setText(tr("Check for card database updates every"));
     cardUpdateCheckIntervalSpinBox.setSuffix(tr(" days"));
-    updateNotificationCheckBox.setText(tr("Notify if a feature supported by the server is missing in my client"));
-    newVersionOracleCheckBox.setText(tr("Automatically run Oracle when running a new version of Cockatrice"));
+
+    QDate lastCheckDate = settings.updates().getLastCardUpdateCheck();
+    int daysAgo = lastCheckDate.daysTo(QDate::currentDate());
+    lastCardUpdateCheckDateLabel.setText(
+        tr("Last update check on %1 (%2 days ago)").arg(lastCheckDate.toString()).arg(daysAgo));
+
+    startupGroupBox->setTitle(tr("Startup settings"));
     showTipsOnStartup.setText(tr("Show tips on startup"));
     startupTabLabel.setText(tr("Startup tab:"));
     startupTabSelector.setItemText(StartupTab::StartupTabHome, tr("Home"));
@@ -473,21 +477,18 @@ void GeneralSettingsPage::retranslateUi()
     startupServerLabel.setText(tr("Server:"));
     startupRoomLabel.setText(tr("Room:"));
     startupRoomNameEdit->setPlaceholderText(tr("Room name"));
-    resetAllPathsButton->setText(tr("Reset all paths"));
 
-    const auto &settings = SettingsCache::instance();
-
-    QDate lastCheckDate = settings.updates().getLastCardUpdateCheck();
-    int daysAgo = lastCheckDate.daysTo(QDate::currentDate());
-
-    lastCardUpdateCheckDateLabel.setText(
-        tr("Last update check on %1 (%2 days ago)").arg(lastCheckDate.toString()).arg(daysAgo));
-
-    // We can't change the strings after they're put into the QComboBox, so this is our workaround
-    int oldIndex = updateReleaseChannelBox.currentIndex();
-    updateReleaseChannelBox.clear();
-    for (ReleaseChannel *chan : settings.getUpdateReleaseChannels()) {
-        updateReleaseChannelBox.addItem(tr(chan->getName().toUtf8()));
+    if (settings.getIsPortableBuild()) {
+        pathsGroupBox->setTitle(tr("Paths (editing disabled in portable mode)"));
+    } else {
+        pathsGroupBox->setTitle(tr("Paths"));
     }
-    updateReleaseChannelBox.setCurrentIndex(oldIndex);
-}
+    deckPathLabel.setText(tr("Decks directory:"));
+    filtersPathLabel.setText(tr("Filters directory:"));
+    replaysPathLabel.setText(tr("Replays directory:"));
+    picsPathLabel.setText(tr("Pictures directory:"));
+    cardDatabasePathLabel.setText(tr("Card database:"));
+    customCardDatabasePathLabel.setText(tr("Custom database directory:"));
+    tokenDatabasePathLabel.setText(tr("Token database:"));
+    resetAllPathsButton->setText(tr("Reset all paths"));
+}

cockatrice/src/interface/widgets/settings_page/general_settings_page.h

@@ -42,6 +42,37 @@ private:
     QGroupBox *startupGroupBox;
     QGroupBox *pathsGroupBox;
 
+    QLabel languageLabel;
+    QComboBox languageBox;
+    QLabel advertiseTranslationPageLabel;
+
+    QLabel updateReleaseChannelLabel;
+    QComboBox updateReleaseChannelBox;
+    QCheckBox startupUpdateCheckCheckBox;
+    QCheckBox updateNotificationCheckBox;
+    QCheckBox newVersionOracleCheckBox;
+
+    QLabel startupCardUpdateCheckBehaviorLabel;
+    QComboBox startupCardUpdateCheckBehaviorSelector;
+    QLabel cardUpdateCheckIntervalLabel;
+    QSpinBox cardUpdateCheckIntervalSpinBox;
+    QLabel lastCardUpdateCheckDateLabel;
+
+    QCheckBox showTipsOnStartup;
+    QLabel startupTabLabel;
+    QComboBox startupTabSelector;
+    QLabel startupServerLabel;
+    QComboBox startupServerSelector;
+    QLabel startupRoomLabel;
+    QLineEdit *startupRoomNameEdit;
+
+    QLabel deckPathLabel;
+    QLabel filtersPathLabel;
+    QLabel replaysPathLabel;
+    QLabel picsPathLabel;
+    QLabel cardDatabasePathLabel;
+    QLabel customCardDatabasePathLabel;
+    QLabel tokenDatabasePathLabel;
     QLineEdit *deckPathEdit;
     QLineEdit *filtersPathEdit;
     QLineEdit *replaysPathEdit;
@@ -51,33 +82,6 @@ private:
     QLineEdit *tokenDatabasePathEdit;
     QPushButton *resetAllPathsButton;
     QLabel *allPathsResetLabel;
-    QComboBox languageBox;
-    QCheckBox startupUpdateCheckCheckBox;
-    QLabel startupCardUpdateCheckBehaviorLabel;
-    QComboBox startupCardUpdateCheckBehaviorSelector;
-    QLabel cardUpdateCheckIntervalLabel;
-    QSpinBox cardUpdateCheckIntervalSpinBox;
-    QLabel lastCardUpdateCheckDateLabel;
-    QCheckBox updateNotificationCheckBox;
-    QCheckBox newVersionOracleCheckBox;
-    QComboBox updateReleaseChannelBox;
-    QLabel languageLabel;
-    QLabel deckPathLabel;
-    QLabel filtersPathLabel;
-    QLabel replaysPathLabel;
-    QLabel picsPathLabel;
-    QLabel cardDatabasePathLabel;
-    QLabel customCardDatabasePathLabel;
-    QLabel tokenDatabasePathLabel;
-    QLabel updateReleaseChannelLabel;
-    QLabel advertiseTranslationPageLabel;
-    QCheckBox showTipsOnStartup;
-    QLabel startupTabLabel;
-    QComboBox startupTabSelector;
-    QLabel startupServerLabel;
-    QComboBox startupServerSelector;
-    QLabel startupRoomLabel;
-    QLineEdit *startupRoomNameEdit;
 };
 
 #endif // COCKATRICE_GENERAL_SETTINGS_PAGE_H

cockatrice/src/interface/widgets/settings_page/user_interface_settings_page.cpp

@@ -20,26 +20,7 @@ enum visualDeckStoragePromptForConversionIndex
 
 UserInterfaceSettingsPage::UserInterfaceSettingsPage()
 {
-    // general settings and notification settings
-    notificationsEnabledCheckBox.setChecked(SettingsCache::instance().userInterface().getNotificationsEnabled());
-    connect(&notificationsEnabledCheckBox, &QCheckBox::QT_STATE_CHANGED, &SettingsCache::instance().userInterface(),
-            &InterfaceSettings::setNotificationsEnabled);
-    connect(&notificationsEnabledCheckBox, &QCheckBox::QT_STATE_CHANGED, this,
-            &UserInterfaceSettingsPage::setNotificationEnabled);
-
-    specNotificationsEnabledCheckBox.setChecked(
-        SettingsCache::instance().userInterface().getSpectatorNotificationsEnabled());
-    specNotificationsEnabledCheckBox.setEnabled(SettingsCache::instance().userInterface().getNotificationsEnabled());
-    connect(&specNotificationsEnabledCheckBox, &QCheckBox::QT_STATE_CHANGED, &SettingsCache::instance().userInterface(),
-            &InterfaceSettings::setSpectatorNotificationsEnabled);
-
-    buddyConnectNotificationsEnabledCheckBox.setChecked(
-        SettingsCache::instance().userInterface().getBuddyConnectNotificationsEnabled());
-    buddyConnectNotificationsEnabledCheckBox.setEnabled(
-        SettingsCache::instance().userInterface().getNotificationsEnabled());
-    connect(&buddyConnectNotificationsEnabledCheckBox, &QCheckBox::QT_STATE_CHANGED,
-            &SettingsCache::instance().userInterface(), &InterfaceSettings::setBuddyConnectNotificationsEnabled);
-
+    // general settings
     doubleClickToPlayCheckBox.setChecked(SettingsCache::instance().userInterface().getDoubleClickToPlay());
     connect(&doubleClickToPlayCheckBox, &QCheckBox::QT_STATE_CHANGED, &SettingsCache::instance().userInterface(),
             &InterfaceSettings::setDoubleClickToPlay);
@@ -103,6 +84,26 @@ UserInterfaceSettingsPage::UserInterfaceSettingsPage()
     generalGroupBox = new QGroupBox;
     generalGroupBox->setLayout(generalGrid);
 
+    // notification settings
+    notificationsEnabledCheckBox.setChecked(SettingsCache::instance().userInterface().getNotificationsEnabled());
+    connect(&notificationsEnabledCheckBox, &QCheckBox::QT_STATE_CHANGED, &SettingsCache::instance().userInterface(),
+            &InterfaceSettings::setNotificationsEnabled);
+    connect(&notificationsEnabledCheckBox, &QCheckBox::QT_STATE_CHANGED, this,
+            &UserInterfaceSettingsPage::setNotificationEnabled);
+
+    specNotificationsEnabledCheckBox.setChecked(
+        SettingsCache::instance().userInterface().getSpectatorNotificationsEnabled());
+    specNotificationsEnabledCheckBox.setEnabled(SettingsCache::instance().userInterface().getNotificationsEnabled());
+    connect(&specNotificationsEnabledCheckBox, &QCheckBox::QT_STATE_CHANGED, &SettingsCache::instance().userInterface(),
+            &InterfaceSettings::setSpectatorNotificationsEnabled);
+
+    buddyConnectNotificationsEnabledCheckBox.setChecked(
+        SettingsCache::instance().userInterface().getBuddyConnectNotificationsEnabled());
+    buddyConnectNotificationsEnabledCheckBox.setEnabled(
+        SettingsCache::instance().userInterface().getNotificationsEnabled());
+    connect(&buddyConnectNotificationsEnabledCheckBox, &QCheckBox::QT_STATE_CHANGED,
+            &SettingsCache::instance().userInterface(), &InterfaceSettings::setBuddyConnectNotificationsEnabled);
+
     auto *notificationsGrid = new QGridLayout;
     notificationsGrid->addWidget(&notificationsEnabledCheckBox, 0, 0);
     notificationsGrid->addWidget(&specNotificationsEnabledCheckBox, 1, 0);
@@ -355,6 +356,7 @@ void UserInterfaceSettingsPage::retranslateUi()
     notificationsEnabledCheckBox.setText(tr("Enable notifications in taskbar"));
     specNotificationsEnabledCheckBox.setText(tr("Notify in the taskbar for game events while you are spectating"));
     buddyConnectNotificationsEnabledCheckBox.setText(tr("Notify in the taskbar when users in your buddy list connect"));
+
     animationGroupBox->setTitle(tr("Animation settings"));
     enableAllAnimationsButton.setText(tr("&Enable all animations"));
     disableAllAnimationsButton.setText(tr("&Disable all animations"));
@@ -362,6 +364,7 @@ void UserInterfaceSettingsPage::retranslateUi()
     arrowDrawAnimationCheckBox.setText(tr("&Arrow draw animation"));
     lifeCounterAnimationsCheckBox.setText(tr("Life counter flash"));
     battlefieldFlashCheckBox.setText(tr("Battlefield flash on damage"));
+
     deckEditorGroupBox->setTitle(tr("Deck editor/storage settings"));
     openDeckInNewTabCheckBox.setText(tr("Open deck in new tab by default"));
     visualDeckStorageInGameCheckBox.setText(tr("Use visual deck storage in game lobby"));
@@ -397,8 +400,8 @@ void UserInterfaceSettingsPage::retranslateUi()
         0, CommanderBracketNames::CommanderSpellbookBracketNames);
     commanderSpellbookIntegrationBracketNamingSelector.setItemText(
         1, CommanderBracketNames::OfficialCommanderBracketNames);
-
     commanderSpellbookIntegrationUseOfficialBracketNamesExplainer.setToolTip(CommanderBracketNames::Explainer);
+
     replayGroupBox->setTitle(tr("Replay settings"));
     rewindBufferingMsLabel.setText(tr("Buffer time for backwards skip via shortcut:"));
     rewindBufferingMsBox.setSuffix(" ms");

cockatrice/src/interface/widgets/settings_page/user_interface_settings_page.h

@@ -23,9 +23,6 @@ private slots:
     void updateCommanderSpellbookUiState();
 
 private:
-    QCheckBox notificationsEnabledCheckBox;
-    QCheckBox specNotificationsEnabledCheckBox;
-    QCheckBox buddyConnectNotificationsEnabledCheckBox;
     QCheckBox doubleClickToPlayCheckBox;
     QCheckBox clickPlaysAllSelectedCheckBox;
     QCheckBox playToStackCheckBox;
@@ -37,12 +34,18 @@ private:
     QCheckBox showTotalSelectionCountCheckBox;
     QCheckBox useTearOffMenusCheckBox;
     QCheckBox keepGameChatFocusCheckBox;
+
+    QCheckBox notificationsEnabledCheckBox;
+    QCheckBox specNotificationsEnabledCheckBox;
+    QCheckBox buddyConnectNotificationsEnabledCheckBox;
+
     QPushButton enableAllAnimationsButton;
     QPushButton disableAllAnimationsButton;
     QCheckBox tapAnimationCheckBox;
     QCheckBox arrowDrawAnimationCheckBox;
     QCheckBox lifeCounterAnimationsCheckBox;
     QCheckBox battlefieldFlashCheckBox;
+
     QCheckBox openDeckInNewTabCheckBox;
     QLabel visualDeckStoragePromptForConversionLabel;
     QComboBox visualDeckStoragePromptForConversionSelector;
@@ -57,8 +60,10 @@ private:
     QLabel commanderSpellbookIntegrationUseOfficialBracketNamesLabel;
     QToolButton commanderSpellbookIntegrationUseOfficialBracketNamesExplainer;
     QComboBox commanderSpellbookIntegrationBracketNamingSelector;
+
     QLabel rewindBufferingMsLabel;
     QSpinBox rewindBufferingMsBox;
+
     QGroupBox *generalGroupBox;
     QGroupBox *notificationsGroupBox;
     QGroupBox *animationGroupBox;

cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp

@@ -116,9 +116,10 @@ void CloseButton::paintEvent(QPaintEvent * /*event*/)
 }
 
 TabSupervisor::TabSupervisor(AbstractClient *_client, QMenu *tabsMenu, QWidget *parent)
-    : QTabWidget(parent), userInfo(nullptr), client(_client), tabsMenu(tabsMenu), tabVisualDeckStorage(nullptr),
-      tabServer(nullptr), tabAccount(nullptr), tabDeckStorage(nullptr), tabReplays(nullptr), tabAdmin(nullptr),
-      tabLog(nullptr), tabReport(nullptr), tabModeration(nullptr), isLocalGame(false)
+    : QTabWidget(parent), userInfo(nullptr), client(_client), tabsMenu(tabsMenu), tabHome(nullptr),
+      tabVisualDeckStorage(nullptr), tabServer(nullptr), tabAccount(nullptr), tabDeckStorage(nullptr),
+      tabReplays(nullptr), tabAdmin(nullptr), tabCardArtRules(nullptr), tabLog(nullptr), tabReport(nullptr),
+      tabModeration(nullptr), isLocalGame(false)
 {
     setElideMode(Qt::ElideRight);
     setMovable(true);
@@ -245,6 +246,7 @@ void TabSupervisor::retranslateUi()
     aTabLog->setText(tr("Logs"));
     aTabReport->setText(tr("Report Queue"));
     aTabModeration->setText(tr("Moderation"));
+    aTabCardArtRules->setText(tr("Card Art Rules"));
 
     // tabs
     QList<Tab *> tabs;
@@ -256,6 +258,7 @@ void TabSupervisor::retranslateUi()
     tabs.append(tabLog);
     tabs.append(tabReport);
     tabs.append(tabModeration);
+    tabs.append(tabCardArtRules);
     QMapIterator<int, TabRoom *> roomIterator(roomTabs);
     while (roomIterator.hasNext()) {
         tabs.append(roomIterator.next().value());
@@ -520,7 +523,9 @@ void TabSupervisor::start(const ServerInfo_User &_userInfo)
         if (SettingsCache::instance().tabs().getTabModerationOpen()) {
             openTabModeration();
         }
-        openTabCardArtRules();
+        if (SettingsCache::instance().tabs().getTabCardArtRulesOpen()) {
+            openTabCardArtRules();
+        }
     }
 
     retranslateUi();
@@ -582,6 +587,9 @@ void TabSupervisor::stop()
         if (tabModeration) {
             tabModeration->close();
         }
+        if (tabCardArtRules) {
+            tabCardArtRules->close();
+        }
     }
 
     QList<Tab *> tabsToDelete;
@@ -775,6 +783,7 @@ void TabSupervisor::openTabAdmin()
 
 void TabSupervisor::actTabCardArtRules(bool checked)
 {
+    SettingsCache::instance().tabs().setTabCardArtRulesOpen(checked);
     if (checked && !tabCardArtRules) {
         openTabCardArtRules();
         setCurrentWidget(tabCardArtRules);

cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_folder_display_widget.cpp

@@ -125,9 +125,7 @@ void VisualDeckStorageFolderDisplayWidget::continueDeckPass()
         }
 
         const bool matches = index.data(VisualDeckStorageRoles::FilterMatchRole).toBool();
-        if (matches == deckPreviewWidget->isHidden()) {
-            deckPreviewWidget->setVisible(matches);
-        }
+        deckPreviewWidget->setVisible(matches);
         if (matches) {
             ++visibleDeckCount;
         }

cockatrice/src/interface/window_main.cpp

@@ -871,8 +871,9 @@ void MainWindow::changeEvent(QEvent *event)
                        !startupDestinationConnectsToServer()) {
                 qCInfo(WindowMainStartupAutoconnectLog) << "Attempting auto-connect...";
                 DlgConnect dlg(this);
-                connectionController->connectToServerDirect(dlg.getHost(), static_cast<unsigned int>(dlg.getPort()),
-                                                            dlg.getPlayerName(), dlg.getPassword());
+                connectionController->connectToServerDirect(
+                    dlg.getHost(), static_cast<unsigned int>(dlg.getPort()), dlg.getPlayerName(), dlg.getPassword(),
+                    dlg.getStoredVerifier(), dlg.getSaveName(), dlg.getSavePassword());
             }
         }
     }

cockatrice/src/main.cpp

@@ -53,6 +53,7 @@
 #include <libcockatrice/settings/interface_settings.h>
 #include <libcockatrice/settings/network_settings.h>
 #include <libcockatrice/settings/personal_settings.h>
+#include <libcockatrice/utility/cryptoutil.h>
 
 QTranslator *translator, *qtTranslator;
 RNG_Abstract *rng;
@@ -292,7 +293,7 @@ int main(int argc, char *argv[])
         }
     }
 
-    rng = new RNG_SFMT;
+    rng = new RNG_SFMT(CryptoUtil::randomUInt64());
     themeManager = new ThemeManager;
     soundEngine = new SoundEngine;
 

libcockatrice_interfaces/libcockatrice/interfaces/interface_tabs_settings_provider.h

@@ -21,6 +21,7 @@ public:
     [[nodiscard]] virtual bool getTabLogOpen() const = 0;
     [[nodiscard]] virtual bool getTabReportOpen() const = 0;
     [[nodiscard]] virtual bool getTabModerationOpen() const = 0;
+    [[nodiscard]] virtual bool getTabCardArtRulesOpen() const = 0;
 };
 
 #endif // COCKATRICE_INTERFACE_TABS_SETTINGS_PROVIDER_H

libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp

@@ -22,7 +22,8 @@
 #include <libcockatrice/protocol/pending_command.h>
 
 AbstractClient::AbstractClient(QObject *parent)
-    : QObject(parent), nextCmdId(0), status(StatusDisconnected), serverSupportsPasswordHash(false)
+    : QObject(parent), nextCmdId(0), status(StatusDisconnected), serverSupportsPasswordHash(false),
+      serverSupportsChallengeResponse(false)
 {
     qRegisterMetaType<QVariant>("QVariant");
     qRegisterMetaType<CommandContainer>("CommandContainer");

libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h

@@ -117,6 +117,7 @@ protected:
     QMap<int, PendingCommand *> pendingCommands;
     QString userName, password, email, country, realName, token;
     bool serverSupportsPasswordHash;
+    bool serverSupportsChallengeResponse;
     void setStatus(ClientStatus _status);
     int getNewCmdId()
     {
@@ -150,6 +151,10 @@ public:
     {
         return serverSupportsPasswordHash;
     }
+    bool getServerSupportsChallengeResponse() const
+    {
+        return serverSupportsChallengeResponse;
+    }
     const QString &getUserName() const
     {
         return userName;

libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp

@@ -27,7 +27,8 @@ static const unsigned int protocolVersion = 14;
 
 RemoteClient::RemoteClient(QObject *parent, INetworkSettingsProvider *_networkSettingsProvider)
     : AbstractClient(parent), networkSettingsProvider(_networkSettingsProvider), timeRunning(0), lastDataReceived(0),
-      messageInProgress(false), handshakeStarted(false), usingWebSocket(false), messageLength(0), hashedPassword()
+      messageInProgress(false), handshakeStarted(false), usingWebSocket(false), messageLength(0), hashedPassword(),
+      passwordNeedsMigration(false)
 {
 
     clearNewClientFeatures();
@@ -114,6 +115,8 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica
         return;
     }
     serverSupportsPasswordHash = event.server_options() & Event_ServerIdentification::SupportsPasswordHash;
+    serverSupportsChallengeResponse =
+        event.server_options() & Event_ServerIdentification::SupportsChallengeResponseAuth;
 
     if (getStatus() == StatusRequestingForgotPassword) {
         Command_ForgotPasswordRequest cmdForgotPasswordRequest;
@@ -130,7 +133,13 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica
         cmdForgotPasswordReset.set_user_name(userName.toStdString());
         cmdForgotPasswordReset.set_clientid(getSrvClientID(lastHostname).toStdString());
         cmdForgotPasswordReset.set_token(token.toStdString());
-        if (!password.isEmpty() && serverSupportsPasswordHash) {
+        if (!password.isEmpty() && serverSupportsChallengeResponse) {
+            hashedPassword = PasswordHasher::generatePasswordVerifier(password);
+            // Only this branch yields a challenge-response verifier worth persisting; the
+            // legacy-hash and plaintext branches below must not be saved under the password key.
+            derivedVerifier = hashedPassword;
+            cmdForgotPasswordReset.set_hashed_new_password(hashedPassword.toStdString());
+        } else if (!password.isEmpty() && serverSupportsPasswordHash) {
             auto passwordSalt = PasswordHasher::generateRandomSalt();
             hashedPassword = PasswordHasher::computeHash(password, passwordSalt);
             cmdForgotPasswordReset.set_hashed_new_password(hashedPassword.toStdString());
@@ -158,7 +167,10 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica
     if (getStatus() == StatusRegistering) {
         Command_Register cmdRegister;
         cmdRegister.set_user_name(userName.toStdString());
-        if (!password.isEmpty() && serverSupportsPasswordHash) {
+        if (!password.isEmpty() && serverSupportsChallengeResponse) {
+            hashedPassword = PasswordHasher::generatePasswordVerifier(password);
+            cmdRegister.set_hashed_password(hashedPassword.toStdString());
+        } else if (!password.isEmpty() && serverSupportsPasswordHash) {
             auto passwordSalt = PasswordHasher::generateRandomSalt();
             hashedPassword = PasswordHasher::computeHash(password, passwordSalt);
             cmdRegister.set_hashed_password(hashedPassword.toStdString());
@@ -223,7 +235,9 @@ Command_Login RemoteClient::generateCommandLogin()
 
 void RemoteClient::doLogin()
 {
-    if (!password.isEmpty() && serverSupportsPasswordHash) {
+    if ((!password.isEmpty() || !storedVerifier.isEmpty()) && serverSupportsChallengeResponse) {
+        doRequestPasswordSalt(); // ask salt + nonce to build the challenge response
+    } else if (!password.isEmpty() && serverSupportsPasswordHash) {
         //! \todo Store and log in using stored hashed password.
         if (hashedPassword.isEmpty()) {
             doRequestPasswordSalt(); // ask salt to create hashedPassword, then log in
@@ -257,6 +271,30 @@ void RemoteClient::doHashedLogin()
     sendCommand(pend);
 }
 
+void RemoteClient::doSubmitPasswordVerifier()
+{
+    pendingVerifier = PasswordHasher::generatePasswordVerifier(password);
+    Command_SubmitPasswordVerifier cmdSubmitVerifier;
+    cmdSubmitVerifier.set_password_verifier(pendingVerifier.toStdString());
+
+    PendingCommand *pend = prepareSessionCommand(cmdSubmitVerifier);
+    connect(pend, &PendingCommand::finished, this, &RemoteClient::submitPasswordVerifierResponse);
+    sendCommand(pend);
+}
+
+void RemoteClient::submitPasswordVerifierResponse(const Response &response)
+{
+    if (response.response_code() == Response::RespOk) {
+        qCDebug(RemoteClientLog) << "Password verifier migrated successfully";
+        if (!pendingVerifier.isEmpty()) {
+            emit sigPasswordVerifierReady(pendingVerifier);
+            pendingVerifier.clear();
+        }
+    } else {
+        qCWarning(RemoteClientLog) << "Failed to migrate password verifier:" << response.response_code();
+    }
+}
+
 void RemoteClient::processConnectionClosedEvent(const Event_ConnectionClosed & /*event*/)
 {
     doDisconnectFromServer();
@@ -269,7 +307,70 @@ void RemoteClient::passwordSaltResponse(const Response &response)
         auto passwordSalt = QString::fromStdString(resp.password_salt());
         if (passwordSalt.isEmpty()) { // the server does not recognize the user but allows them to enter unregistered
             password.clear();         // the password will not be used
+            storedVerifier.clear();
             doLogin();
+        } else if (serverSupportsChallengeResponse && resp.has_nonce()) {
+            const QByteArray nonce = QByteArray::fromStdString(resp.nonce());
+            // needs_migration is a server-controlled flag over an unauthenticated
+            // transport (plain TCP by default). If we already hold a scrypt
+            // verifier for this account, a server telling us to fall back to the
+            // 1000-round legacy hash is a downgrade of the KDF — refuse it rather
+            // than hand an attacker a weakly-keyed HMAC they can attack offline.
+            if (resp.needs_migration() && PasswordHasher::parsePasswordVerifier(storedVerifier).isValid) {
+                emit loginError(Response::RespClientUpdateRequired,
+                                QStringLiteral("Server asked to downgrade authentication for a migrated account."), 0,
+                                {});
+                return;
+            }
+            QByteArray key;
+            if (resp.needs_migration()) {
+                // The account still uses the legacy format; the legacy full hash
+                // is only derivable from the plaintext password.
+                if (password.isEmpty()) {
+                    emit loginError(Response::RespClientUpdateRequired,
+                                    QStringLiteral("This account must be logged in with its password once."), 0, {});
+                    return;
+                }
+                key = PasswordHasher::computeHash(password, passwordSalt).toUtf8();
+            } else if (!password.isEmpty()) {
+                // A hostile server must not be able to make us run or allocate for
+                // unreasonable scrypt parameters.
+                const int n = resp.has_n() ? resp.n() : SCRYPT_N;
+                const int r = resp.has_r() ? resp.r() : SCRYPT_R;
+                const int p = resp.has_p() ? resp.p() : SCRYPT_P;
+                if (!PasswordHasher::costParamsAreSane(n, r, p)) {
+                    emit loginError(Response::RespClientUpdateRequired,
+                                    QStringLiteral("The server requested unreasonable scrypt cost parameters."), 0, {});
+                    return;
+                }
+                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)
+                                      .arg(p)
+                                      .arg(passwordSalt)
+                                      .arg(QString(key.toBase64()));
+            } else if (!storedVerifier.isEmpty()) {
+                const PasswordVerifier verifier = PasswordHasher::parsePasswordVerifier(storedVerifier);
+                if (!verifier.isValid) {
+                    emit loginError(Response::RespClientUpdateRequired, QStringLiteral("Stored verifier is invalid."),
+                                    0, {});
+                    return;
+                }
+                key = verifier.verifier;
+            } else {
+                emit loginError(Response::RespLoginNeeded, {}, 0, {});
+                return;
+            }
+            passwordNeedsMigration = resp.needs_migration();
+            const QByteArray responseBytes = PasswordHasher::computeResponse(key, nonce);
+            hashedPassword = "$challenge$" + QString(nonce.toBase64()) + "$" + QString(responseBytes.toBase64());
+            doHashedLogin();
         } else {
             hashedPassword = PasswordHasher::computeHash(password, passwordSalt);
             doHashedLogin();
@@ -294,6 +395,14 @@ void RemoteClient::loginResponse(const Response &response)
         setStatus(StatusLoggedIn);
         emit userInfoChanged(resp.user_info());
 
+        if (passwordNeedsMigration) {
+            // The account still used the legacy password format; upgrade it to scrypt.
+            doSubmitPasswordVerifier();
+        } else if (!derivedVerifier.isEmpty()) {
+            emit sigPasswordVerifierReady(derivedVerifier);
+            derivedVerifier.clear();
+        }
+
         QList<ServerInfo_User> buddyList;
         for (int i = resp.buddy_list_size() - 1; i >= 0; --i) {
             buddyList.append(resp.buddy_list(i));
@@ -550,6 +659,8 @@ void RemoteClient::doDisconnectFromServer()
         websocket->close();
     }
     socket->close();
+    derivedVerifier.clear();
+    pendingVerifier.clear();
 }
 
 void RemoteClient::ping()
@@ -712,6 +823,12 @@ void RemoteClient::submitForgotPasswordResetResponse(const Response &response)
 {
     if (response.response_code() == Response::RespOk) {
         emit sigForgotPasswordSuccess();
+        // Persist only a real scrypt verifier; a legacy hash must not be stored under
+        // the password key, where it would break future challenge-response logins.
+        if (!derivedVerifier.isEmpty()) {
+            emit sigPasswordVerifierReady(derivedVerifier);
+            derivedVerifier.clear();
+        }
     } else {
         emit sigForgotPasswordError();
     }

libcockatrice_network/libcockatrice/network/client/remote/remote_client.h

@@ -54,6 +54,11 @@ signals:
                                                   unsigned int port,
                                                   const QString &_userName,
                                                   const QString &_email);
+    //! \brief Emitted once a scrypt verifier for the given account is known and
+    //! can be persisted instead of the plaintext password. The receiving side
+    //! should only store it for the connection it is currently negotiating, so
+    //! the hostname and user name are intentionally not part of the signal.
+    void sigPasswordVerifierReady(const QString &verifier);
 private slots:
     void slotConnected();
     void readData();
@@ -80,6 +85,8 @@ private slots:
     void doLogin();
     void doHashedLogin();
     Command_Login generateCommandLogin();
+    void doSubmitPasswordVerifier();
+    void submitPasswordVerifierResponse(const Response &response);
     void doDisconnectFromServer();
     void doActivateToServer(const QString &_token);
     void doRequestForgotPasswordToServer(const QString &hostname, unsigned int port, const QString &_userName);
@@ -111,6 +118,14 @@ private:
     QString lastHostname;
     unsigned int lastPort;
     QString hashedPassword;
+    bool passwordNeedsMigration;
+    //! \brief A previously stored "$scrypt$..." verifier used to authenticate
+    //! without the plaintext password.
+    QString storedVerifier;
+    //! \brief Verifier derived during the current login, persisted after success.
+    QString derivedVerifier;
+    //! \brief Verifier sent for migration, persisted once the server accepts it.
+    QString pendingVerifier;
 
     QString getSrvClientID(const QString &_hostname);
     bool newMissingFeatureFound(const QString &_serversMissingFeatures);
@@ -149,6 +164,12 @@ public:
     }
     void
     connectToServer(const QString &hostname, unsigned int port, const QString &_userName, const QString &_password);
+    //! \brief Provide a stored "$scrypt$..." verifier so the client can
+    //! authenticate without the plaintext password.
+    void setStoredVerifier(const QString &verifier)
+    {
+        storedVerifier = verifier;
+    }
     void registerToServer(const QString &hostname,
                           unsigned int port,
                           const QString &_userName,

libcockatrice_network/libcockatrice/network/server/remote/server.h

@@ -85,6 +85,11 @@ public:
     {
         return QMap<QString, bool>();
     }
+    /** @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<QString> getOnlineModeratorList() const;

libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h

@@ -40,6 +40,14 @@ public:
     {
         return {};
     }
+    virtual QString getUserPasswordData(const QString & /* user */)
+    {
+        return {};
+    }
+    virtual bool submitPasswordVerifier(const QString & /* user */, const QString & /* passwordVerifier */)
+    {
+        return false;
+    }
     virtual QMap<QString, ServerInfo_User> getBuddyList(const QString & /* name */)
     {
         return QMap<QString, ServerInfo_User>();

libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp

@@ -1,5 +1,8 @@
 #include "server_protocolhandler.h"
 
+// Challenge-response nonces are valid for at most one minute from issuance.
+static constexpr qint64 kAuthNonceLifetimeSeconds = 60;
+
 #include "game/game_config.h"
 #include "game/server_game.h"
 #include "game/server_player.h"
@@ -44,6 +47,28 @@ Server_ProtocolHandler::~Server_ProtocolHandler()
 {
 }
 
+void Server_ProtocolHandler::setAuthNonce(const QByteArray &nonce, const QString &userName)
+{
+    authNonce = nonce;
+    authNonceUser = userName;
+    authNonceCreated = QDateTime::currentDateTimeUtc();
+}
+
+bool Server_ProtocolHandler::isAuthNonceValid(const QByteArray &nonce, const QString &userName) const
+{
+    // secsTo is signed, so a wall-clock step backwards (NTP correction, VM resume)
+    // must not make the elapsed time negative and re-validate an old nonce.
+    const qint64 elapsed = authNonceCreated.secsTo(QDateTime::currentDateTimeUtc());
+    return !authNonce.isEmpty() && authNonce == nonce && authNonceUser == userName && authNonceCreated.isValid() &&
+           elapsed >= 0 && elapsed < kAuthNonceLifetimeSeconds;
+}
+
+void Server_ProtocolHandler::clearAuthNonce()
+{
+    authNonce.clear();
+    authNonceUser.clear();
+}
+
 // This function must only be called from the thread this object lives in.
 // Except when the server is shutting down.
 // The thread must not hold any server locks when calling this (e.g. clientsLock, roomsLock).
@@ -507,6 +532,16 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd
         return Response::RespContextError;
     }
 
+    // In strict mode only challenge-response logins are accepted.
+    if (server->requiresChallengeResponseAuth() &&
+        (!cmd.has_hashed_password() || cmd.hashed_password().rfind("$challenge$", 0) != 0)) {
+        auto *re = new Response_Login;
+        re->set_denied_reason_str("Client upgrade required");
+        re->add_missing_features("challenge_response_auth");
+        rc.setResponseExtension(re);
+        return Response::RespClientUpdateRequired;
+    }
+
     // check client feature set against server feature set
     FeatureSet features;
     QMap<QString, bool> receivedClientFeatures;

libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.h

@@ -4,6 +4,8 @@
 #include "server.h"
 #include "server_abstractuserinterface.h"
 
+#include <QByteArray>
+#include <QDateTime>
 #include <QObject>
 #include <libcockatrice/protocol/pb/response.pb.h>
 #include <libcockatrice/protocol/pb/server_message.pb.h>
@@ -55,6 +57,9 @@ protected:
     bool acceptsUserListChanges;
     bool acceptsRoomListChanges;
     bool idleClientWarningSent;
+    QByteArray authNonce;
+    QString authNonceUser;
+    QDateTime authNonceCreated;
     virtual void logDebugMessage(const QString & /* message */)
     {
     }
@@ -124,6 +129,13 @@ public:
         return databaseInterface;
     }
 
+    /** @brief Store a fresh challenge nonce bound to @p userName for the next challenge-response login attempt. */
+    void setAuthNonce(const QByteArray &nonce, const QString &userName);
+    /** @brief True if nonce matches the pending one, was issued for @p userName, and is less than 60 seconds old. */
+    bool isAuthNonceValid(const QByteArray &nonce, const QString &userName) const;
+    /** @brief Invalidate the pending nonce (single-use). */
+    void clearAuthNonce();
+
     int getLastCommandTime() const
     {
         return timeRunning - lastDataReceived;

libcockatrice_protocol/libcockatrice/protocol/featureset.cpp

@@ -25,7 +25,8 @@ void FeatureSet::initalizeFeatureList(QMap<QString, bool> &_featureList)
     _featureList.insert("idle_client", false);
     _featureList.insert("forgot_password", false);
     _featureList.insert("websocket", false);
-    // featureList.insert("hashed_password_login", false);
+    _featureList.insert("hashed_password_login", false);
+    _featureList.insert("challenge_response_auth", false);
     // These are temp to force users onto a newer client
     _featureList.insert("2.7.0_min_version", false);
     _featureList.insert("2.8.0_min_version", false);

libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt

@@ -90,7 +90,6 @@ set(PROTO_FILES
     event_game_log_notice.proto
     event_game_say.proto
     event_game_state_changed.proto
-    event_game_state_changed.proto
     event_join.proto
     event_join_room.proto
     event_kicked.proto

libcockatrice_protocol/libcockatrice/protocol/pb/event_server_identification.proto

@@ -8,6 +8,7 @@ message Event_ServerIdentification {
     enum ServerOptions {
         NoOptions = 0;
         SupportsPasswordHash = 1;
+        SupportsChallengeResponseAuth = 2;
     }
     optional string server_name = 1;
     optional string server_version = 2;

libcockatrice_protocol/libcockatrice/protocol/pb/response_password_salt.proto

@@ -6,4 +6,14 @@ message Response_PasswordSalt {
         optional Response_PasswordSalt ext = 1017;
     }
     optional string password_salt = 1;
+    // scrypt cost parameters for password_salt. Absent/zero for legacy accounts.
+    optional int32 n = 2;
+    optional int32 r = 3;
+    optional int32 p = 4;
+    // Server-generated challenge. When present the client must authenticate
+    // with a challenge-response instead of transmitting the password hash.
+    optional bytes nonce = 5;
+    // True when the account still uses the legacy password format and should
+    // be migrated to the scrypt format after a successful login.
+    optional bool needs_migration = 6;
 }

libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto

@@ -28,6 +28,7 @@ message SessionCommand {
         FORGOT_PASSWORD_CHALLENGE = 1023;
         REQUEST_PASSWORD_SALT = 1024;
         SET_CARD_ART_PARAMS = 1025;
+        SUBMIT_PASSWORD_VERIFIER = 1026;
         REPLAY_LIST = 1100;
         REPLAY_DOWNLOAD = 1101;
         REPLAY_MODIFY_MATCH = 1102;
@@ -223,3 +224,14 @@ message Command_SetCardArtParams {
     optional double vertical_offset = 5;
     optional double zoom = 6;
 }
+
+// Client uploads the new password verifier to migrate a legacy account
+// after a successful challenge-response login. Idempotent; only applies
+// to accounts still using the legacy password format.
+message Command_SubmitPasswordVerifier {
+    extend SessionCommand {
+        optional Command_SubmitPasswordVerifier ext = 1026;
+    }
+    // Full verifier string to store, e.g. "$scrypt$32768$8$1$<salt>$<verifier>"
+    required string password_verifier = 1;
+}

libcockatrice_rng/libcockatrice/rng/rng_sfmt.cpp

@@ -1,6 +1,5 @@
 #include "rng_sfmt.h"
 
-#include <QDateTime>
 #include <algorithm>
 #include <climits>
 #include <stdexcept>
@@ -11,10 +10,11 @@
 #define UINT64_MAX (~(uint64_t)0)
 #endif
 
-RNG_SFMT::RNG_SFMT(QObject *parent) : RNG_Abstract(parent)
+RNG_SFMT::RNG_SFMT(uint64_t seed, QObject *parent) : RNG_Abstract(parent)
 {
-    // initialize the random number generator with a 32bit integer seed (timestamp)
-    sfmt_init_gen_rand(&sfmt, QDateTime::currentDateTime().toSecsSinceEpoch());
+    // initialize the random number generator with a 64bit seed, e.g. from a CSPRNG
+    uint32_t seedArray[2] = {static_cast<uint32_t>(seed), static_cast<uint32_t>(seed >> 32)};
+    sfmt_init_by_array(&sfmt, seedArray, 2);
 }
 
 /**

libcockatrice_rng/libcockatrice/rng/rng_sfmt.h

@@ -36,7 +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;
 };
 

libcockatrice_settings/libcockatrice/settings/servers_settings.cpp

@@ -146,6 +146,19 @@ void ServersSettings::setFPPlayerName(QString playerName)
     setValue(playerName, "fpPlayerName");
 }
 
+void ServersSettings::setServerPassword(const QString &saveName, const QString &password)
+{
+    const int index = getPrevioushostindex(saveName);
+    if (index >= 0) {
+        setValue(password, QString("password%1").arg(index), "server", "server_details");
+    } else {
+        // A credential write that silently no-ops is hard to diagnose from a
+        // bug report; surface a mismatched profile name instead.
+        qCWarning(ServersSettingsLog) << "setServerPassword() could not find profile:" << saveName
+                                      << "- password not saved";
+    }
+}
+
 QString ServersSettings::getFPPlayerName(QString defaultName) const
 {
     QVariant name = getValue("fpPlayerName");

libcockatrice_settings/libcockatrice/settings/servers_settings.h

@@ -46,6 +46,8 @@ public:
     void setFPHostName(QString hostname);
     void setFPPort(QString port);
     void setFPPlayerName(QString playerName);
+    //! \brief Store a password (or a "$scrypt$..." verifier) for the given saved server.
+    void setServerPassword(const QString &saveName, const QString &password);
     void addNewServer(const QString &saveName,
                       const QString &serv,
                       const QString &port,

libcockatrice_settings/libcockatrice/settings/tabs_settings.cpp

@@ -106,6 +106,11 @@ bool TabsSettings::getTabModerationOpen() const
     return getValue("moderation", QString(), QString(), false).toBool();
 }
 
+bool TabsSettings::getTabCardArtRulesOpen() const
+{
+    return getValue("cardArtRules", QString(), QString(), false).toBool();
+}
+
 void TabsSettings::setTabVisualDeckStorageOpen(bool value)
 {
     setValue(value, "visualDeckStorage");
@@ -150,3 +155,8 @@ void TabsSettings::setTabModerationOpen(bool value)
 {
     setValue(value, "moderation");
 }
+
+void TabsSettings::setTabCardArtRulesOpen(bool value)
+{
+    setValue(value, "cardArtRules");
+}

libcockatrice_settings/libcockatrice/settings/tabs_settings.h

@@ -43,6 +43,7 @@ public:
     [[nodiscard]] bool getTabLogOpen() const override;
     [[nodiscard]] bool getTabReportOpen() const override;
     [[nodiscard]] bool getTabModerationOpen() const override;
+    [[nodiscard]] bool getTabCardArtRulesOpen() const override;
 
     void setStartupTabIndex(int value);
     void setStartupServerHost(const QString &host);
@@ -57,6 +58,7 @@ public:
     void setTabLogOpen(bool value);
     void setTabReportOpen(bool value);
     void setTabModerationOpen(bool value);
+    void setTabCardArtRulesOpen(bool value);
 
 signals:
     void startupTabIndexChanged(int index);

libcockatrice_utility/CMakeLists.txt

@@ -6,13 +6,15 @@ set(CMAKE_AUTOUIC ON)
 set(CMAKE_AUTORCC ON)
 
 set(UTILITY_SOURCES
-    libcockatrice/utility/expression.cpp libcockatrice/utility/levenshtein.cpp libcockatrice/utility/passwordhasher.cpp
-    libcockatrice/utility/server_rate_limiter.cpp libcockatrice/utility/warning_categories.cpp
+    libcockatrice/utility/cryptoutil.cpp libcockatrice/utility/expression.cpp libcockatrice/utility/levenshtein.cpp
+    libcockatrice/utility/passwordhasher.cpp libcockatrice/utility/server_rate_limiter.cpp
+    libcockatrice/utility/warning_categories.cpp
 )
 
 set(UTILITY_HEADERS
     libcockatrice/utility/card_ref.h
     libcockatrice/utility/color.h
+    libcockatrice/utility/cryptoutil.h
     libcockatrice/utility/expression.h
     libcockatrice/utility/levenshtein.h
     libcockatrice/utility/macros.h
@@ -32,7 +34,9 @@ add_library(libcockatrice_utility STATIC ${UTILITY_SOURCES} ${UTILITY_HEADERS})
 
 target_include_directories(libcockatrice_utility PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
 
-target_link_libraries(libcockatrice_utility PUBLIC libcockatrice_rng ${QT_CORE_MODULE})
+find_package(OpenSSL REQUIRED)
+
+target_link_libraries(libcockatrice_utility PUBLIC libcockatrice_rng OpenSSL::Crypto ${QT_CORE_MODULE})
 
 set(ORACLE_LIBS)
 

libcockatrice_utility/libcockatrice/utility/cryptoutil.cpp

@@ -0,0 +1,25 @@
+#include "cryptoutil.h"
+
+#include <openssl/rand.h>
+
+namespace CryptoUtil
+{
+QByteArray randomBytes(int count)
+{
+    QByteArray bytes(count, '\0');
+    if (RAND_bytes(reinterpret_cast<unsigned char *>(bytes.data()), count) != 1) {
+        // Randomness failure is fatal: never fall back to a predictable source.
+        qFatal("CryptoUtil::randomBytes: RAND_bytes failed");
+    }
+    return bytes;
+}
+
+quint64 randomUInt64()
+{
+    quint64 value;
+    if (RAND_bytes(reinterpret_cast<unsigned char *>(&value), sizeof(value)) != 1) {
+        qFatal("CryptoUtil::randomUInt64: RAND_bytes failed");
+    }
+    return value;
+}
+} // namespace CryptoUtil

libcockatrice_utility/libcockatrice/utility/cryptoutil.h

@@ -0,0 +1,13 @@
+#ifndef CRYPTOUTIL_H
+#define CRYPTOUTIL_H
+
+#include <QByteArray>
+#include <QtGlobal>
+
+namespace CryptoUtil
+{
+QByteArray randomBytes(int count);
+quint64 randomUInt64();
+} // namespace CryptoUtil
+
+#endif

libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp

@@ -1,7 +1,10 @@
 #include "passwordhasher.h"
 
 #include <QCryptographicHash>
-#include <libcockatrice/rng/rng_sfmt.h>
+#include <libcockatrice/utility/cryptoutil.h>
+#include <openssl/crypto.h>
+#include <openssl/evp.h>
+#include <openssl/hmac.h>
 
 QString PasswordHasher::computeHash(const QString &password, const QString &salt)
 {
@@ -21,12 +24,28 @@ QString PasswordHasher::generateRandomSalt(const int len)
     static const char alphanum[] = "0123456789"
                                    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
                                    "abcdefghijklmnopqrstuvwxyz";
+    const int size = sizeof(alphanum) - 1;
+
+    // Two bytes per character, corrected for modulo bias via rejection sampling.
+    const int bucketSize = 65536 / size;
+    const int limit = bucketSize * size;
 
     QString ret;
-    int size = sizeof(alphanum) - 1;
-
+    ret.reserve(len);
+    QByteArray random = CryptoUtil::randomBytes(len * 2);
+    int bytesUsed = 0;
     for (int i = 0; i < len; ++i) {
-        ret.append(alphanum[rng->rand(0, size)]);
+        unsigned int value;
+        do {
+            if (bytesUsed >= random.size()) {
+                random = CryptoUtil::randomBytes(len * 2);
+                bytesUsed = 0;
+            }
+            value = static_cast<unsigned int>(static_cast<unsigned char>(random.at(bytesUsed))) << 8 |
+                    static_cast<unsigned int>(static_cast<unsigned char>(random.at(bytesUsed + 1)));
+            bytesUsed += 2;
+        } while (value >= limit);
+        ret.append(alphanum[value / bucketSize]);
     }
 
     return ret;
@@ -34,5 +53,116 @@ QString PasswordHasher::generateRandomSalt(const int len)
 
 QString PasswordHasher::generateActivationToken()
 {
-    return QCryptographicHash::hash(generateRandomSalt().toUtf8(), QCryptographicHash::Md5).toBase64().left(16);
+    return QString(CryptoUtil::randomBytes(16).toBase64().left(16));
+}
+
+QByteArray PasswordHasher::deriveKey(const QString &password, const QByteArray &salt, int n, int r, int p)
+{
+    QByteArray key(SCRYPT_VERIFIER_LENGTH, '\0');
+    const QByteArray passwordUtf8 = password.toUtf8();
+    // EVP_PBE_scrypt aborts unless maxmem covers the required working memory,
+    // which is roughly 128 * n * r bytes (plus the small Salsa20/8 block array).
+    const auto maxmem = static_cast<quint64>(128) * n * r + static_cast<quint64>(128) * r * p + 4096;
+    if (EVP_PBE_scrypt(passwordUtf8.constData(), passwordUtf8.size(),
+                       reinterpret_cast<const unsigned char *>(salt.constData()), salt.size(), n, r, p, maxmem,
+                       reinterpret_cast<unsigned char *>(key.data()), key.size()) != 1) {
+        return QByteArray();
+    }
+    return key;
+}
+
+bool PasswordHasher::costParamsAreSane(int n, int r, int p)
+{
+    // Bounds adopted during review: n in [1024, 2**20] and a power of two, r in [1, 32],
+    // p in [1, 16]. Anything else is rejected before we allocate or derive for it.
+    return n >= 1024 && n <= (1 << 20) && (n & (n - 1)) == 0 && r >= 1 && r <= 32 && p >= 1 && p <= 16;
+}
+
+QString PasswordHasher::generatePasswordVerifier(const QString &password)
+{
+    const QByteArray salt = CryptoUtil::randomBytes(SCRYPT_SALT_LENGTH);
+    const QByteArray verifier = deriveKey(password, salt, SCRYPT_N, SCRYPT_R, SCRYPT_P);
+    return QString("$scrypt$%1$%2$%3$%4$%5")
+        .arg(SCRYPT_N)
+        .arg(SCRYPT_R)
+        .arg(SCRYPT_P)
+        .arg(QString(salt.toBase64()))
+        .arg(QString(verifier.toBase64()));
+}
+
+PasswordVerifier PasswordHasher::parsePasswordVerifier(const QString &stored)
+{
+    PasswordVerifier result;
+    const QStringList parts = stored.split("$");
+    if (parts.size() != 7 || parts.at(1) != "scrypt") {
+        return result;
+    }
+
+    bool ok = false;
+    const int n = parts.at(2).toInt(&ok);
+    if (!ok) {
+        return result;
+    }
+    const int r = parts.at(3).toInt(&ok);
+    if (!ok) {
+        return result;
+    }
+    const int p = parts.at(4).toInt(&ok);
+    if (!ok || !costParamsAreSane(n, r, p)) {
+        return result;
+    }
+
+    const QByteArray salt = QByteArray::fromBase64(parts.at(5).toUtf8());
+    const QByteArray verifier = QByteArray::fromBase64(parts.at(6).toUtf8());
+    if (salt.isEmpty() || verifier.size() != SCRYPT_VERIFIER_LENGTH) {
+        return result;
+    }
+
+    result.format = PasswordFormat::Scrypt;
+    result.n = n;
+    result.r = r;
+    result.p = p;
+    result.salt = salt;
+    result.verifier = verifier;
+    result.isValid = true;
+    return result;
+}
+
+bool PasswordHasher::isLegacyFormat(const QString &stored)
+{
+    return !stored.startsWith("$");
+}
+
+bool PasswordHasher::verifyPassword(const QString &password, const QString &storedPasswordData)
+{
+    if (isLegacyFormat(storedPasswordData)) {
+        return storedPasswordData == computeHash(password, storedPasswordData.left(16));
+    }
+
+    const PasswordVerifier verifier = parsePasswordVerifier(storedPasswordData);
+    if (!verifier.isValid) {
+        return false;
+    }
+    const QByteArray derived = deriveKey(password, verifier.salt, verifier.n, verifier.r, verifier.p);
+    return !derived.isEmpty() && constantTimeEquals(derived, verifier.verifier);
+}
+
+QByteArray PasswordHasher::computeResponse(const QByteArray &key, const QByteArray &nonce)
+{
+    QByteArray response(EVP_MAX_MD_SIZE, '\0');
+    unsigned int responseLength = 0;
+    if (HMAC(EVP_sha256(), key.constData(), key.size(), reinterpret_cast<const unsigned char *>(nonce.constData()),
+             nonce.size(), reinterpret_cast<unsigned char *>(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;
 }

libcockatrice_utility/libcockatrice/utility/passwordhasher.h

@@ -1,14 +1,57 @@
 #ifndef PASSWORDHASHER_H
 #define PASSWORDHASHER_H
 
+#include <QByteArray>
 #include <QObject>
 
+// scrypt cost parameters used for newly created password verifiers. These match
+// the RFC 7914 recommended parameters for interactive use.
+constexpr int SCRYPT_N = 32768;
+constexpr int SCRYPT_R = 8;
+constexpr int SCRYPT_P = 1;
+constexpr int SCRYPT_SALT_LENGTH = 16;
+constexpr int SCRYPT_VERIFIER_LENGTH = 64;
+
+enum class PasswordFormat
+{
+    None = 0,
+    Scrypt
+};
+
+struct PasswordVerifier
+{
+    PasswordFormat format = PasswordFormat::None;
+    int n = 0;
+    int r = 0;
+    int p = 0;
+    QByteArray salt;
+    QByteArray verifier;
+    bool isValid = false;
+};
+
 class PasswordHasher
 {
 public:
     static QString computeHash(const QString &password, const QString &salt);
     static QString generateRandomSalt(const int len = 16);
     static QString generateActivationToken();
+
+    /** @brief Derive the scrypt verifier for the given password, salt and cost parameters. Empty on failure. */
+    static QByteArray deriveKey(const QString &password, const QByteArray &salt, int n, int r, int p);
+    /** @brief True if the scrypt cost parameters are acceptable for server and client use. */
+    static bool costParamsAreSane(int n, int r, int p);
+    /** @brief Build a "$scrypt$<n>$<r>$<p>$<salt>$<verifier>" string with a fresh random salt. */
+    static QString generatePasswordVerifier(const QString &password);
+    /** @brief Parse a stored "$scrypt$..." string into its components. */
+    static PasswordVerifier parsePasswordVerifier(const QString &stored);
+    /** @brief True if the stored value is not in the scrypt format (legacy salt+hash). */
+    static bool isLegacyFormat(const QString &stored);
+    /** @brief True if the password matches the stored credential, whether legacy salt+hash or scrypt. */
+    static bool verifyPassword(const QString &password, const QString &storedPasswordData);
+    /** @brief HMAC-SHA256 of nonce keyed with the password verifier, used for challenge-response logins. */
+    static QByteArray computeResponse(const QByteArray &key, const QByteArray &nonce);
+    /** @brief Constant-time byte comparison. */
+    static bool constantTimeEquals(const QByteArray &a, const QByteArray &b);
 };
 
 #endif

servatrice/migrations/servatrice_0036_to_0037.sql

@@ -0,0 +1,9 @@
+-- Servatrice db migration from version 36 to version 37
+
+-- The column must hold "$scrypt$<n>$<r>$<p>$<salt>$<verifier>" (up to ~255 chars) and arbitrary
+-- legacy base64 hashes, so it grows beyond the old 120-char size. varchar(255) is used because
+-- widening a CHAR requires a table rebuild, which ALGORITHM=INSTANT cannot perform — dropping the
+-- clause lets the server pick a suitable algorithm (and any row-format change is avoided anyway).
+ALTER TABLE `cockatrice_users` MODIFY `password_sha512` varchar(255) NOT NULL;
+
+UPDATE cockatrice_schema_version SET version=37 WHERE version=36;

servatrice/servatrice.ini.example

@@ -350,6 +350,22 @@ 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$<n>$<r>$<p>$<salt>$<verifier>". 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"

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(37);
 
 -- users and user data tables
 CREATE TABLE IF NOT EXISTS `cockatrice_users` (
@@ -28,7 +28,7 @@ CREATE TABLE IF NOT EXISTS `cockatrice_users` (
   `admin` tinyint(1) NOT NULL,
   `name` varchar(35) NOT NULL,
   `realname` varchar(255) NOT NULL,
-  `password_sha512` char(120) NOT NULL,
+  `password_sha512` varchar(255) NOT NULL,
   `email` varchar(255) NOT NULL,
   `country` char(2) NOT NULL,
   `avatar_bmp` mediumblob NOT NULL,

servatrice/src/main.cpp

@@ -33,6 +33,7 @@
 #include <QtGlobal>
 #include <iostream>
 #include <libcockatrice/rng/rng_sfmt.h>
+#include <libcockatrice/utility/cryptoutil.h>
 #include <libcockatrice/utility/passwordhasher.h>
 
 RNG_Abstract *rng;
@@ -169,7 +170,7 @@ int main(int argc, char *argv[])
 
     signalhandler = new SignalHandler();
 
-    rng = new RNG_SFMT;
+    rng = new RNG_SFMT(CryptoUtil::randomUInt64());
 
     std::cerr << "Servatrice " << VERSION_STRING << " starting." << std::endl;
     std::cerr << "-------------------------" << std::endl;

servatrice/src/servatrice.cpp

@@ -900,6 +900,18 @@ QString Servatrice::getRequiredFeatures() const
     return settingsCache->value("server/requiredfeatures", "").toString();
 }
 
+Servatrice::AuthenticationStrictness Servatrice::getAuthenticationStrictness() const
+{
+    const QString strictness = settingsCache->value("security/authentication_strictness", "mixed").toString();
+    if (strictness == "strict") {
+        return AuthenticationStrict;
+    }
+    if (strictness == "legacy") {
+        return AuthenticationLegacy;
+    }
+    return AuthenticationMixed;
+}
+
 QString Servatrice::getDBTypeString() const
 {
     if (QProcessEnvironment::systemEnvironment().contains("DATABASE_URL")) {

servatrice/src/servatrice.h

@@ -140,6 +140,12 @@ public:
         AuthenticationSql,
         AuthenticationPassword
     };
+    enum AuthenticationStrictness
+    {
+        AuthenticationLegacy,
+        AuthenticationMixed,
+        AuthenticationStrict
+    };
 private slots:
     void statusUpdate();
     void shutdownTimeout();
@@ -219,6 +225,10 @@ public:
     {
         return serverRequiredFeatureList;
     }
+    bool requiresChallengeResponseAuth() const override
+    {
+        return getAuthenticationStrictness() == AuthenticationStrict;
+    }
     QString getServerName() const;
     QString getLoginMessage() const override
     {
@@ -240,6 +250,7 @@ public:
     {
         return authenticationMethod;
     }
+    AuthenticationStrictness getAuthenticationStrictness() const;
     bool permitUnregisteredUsers() const override
     {
         return authenticationMethod != AuthenticationNone;

servatrice/src/servatrice_database_interface.cpp

@@ -356,6 +356,54 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot
                     qCWarning(DatabaseInterfaceLog) << "Login denied: user not active";
                     return UserIsInactive;
                 }
+
+                // Fail closed on an absent stored credential: an empty key would
+                // otherwise authenticate anyone who can compute HMAC("", nonce).
+                if (correctPasswordSha512.isEmpty()) {
+                    qCWarning(DatabaseInterfaceLog) << "Login denied: empty stored credential";
+                    return NotLoggedIn;
+                }
+
+                if (password.startsWith("$challenge$")) {
+                    // Challenge-response login: verify HMAC(stored_key, nonce) without
+                    // ever transmitting the stored credential or password hash.
+                    const QStringList parts = password.split("$");
+                    if (parts.size() != 4) {
+                        return NotLoggedIn;
+                    }
+                    const QByteArray nonce = QByteArray::fromBase64(parts.at(2).toUtf8());
+                    const QByteArray response = QByteArray::fromBase64(parts.at(3).toUtf8());
+                    if (nonce.isEmpty() || response.isEmpty() || !handler->isAuthNonceValid(nonce, user)) {
+                        return NotLoggedIn;
+                    }
+
+                    QByteArray key;
+                    if (PasswordHasher::isLegacyFormat(correctPasswordSha512)) {
+                        key = correctPasswordSha512.toUtf8();
+                    } else {
+                        // Design note: the stored scrypt verifier IS the challenge-response key, so a
+                        // database dump yields credentials that can answer a login challenge directly.
+                        // We deliberately accept this trade: it removes plaintext passwords from the
+                        // client config and from the wire, but does not protect against DB compromise.
+                        // A password-equivalent proof scheme (e.g. SRP-6a/OPAQUE) would be the proper
+                        // escalation and is out of scope here.
+                        const PasswordVerifier verifier = PasswordHasher::parsePasswordVerifier(correctPasswordSha512);
+                        if (!verifier.isValid) {
+                            return NotLoggedIn;
+                        }
+                        key = verifier.verifier;
+                    }
+
+                    const QByteArray expected = PasswordHasher::computeResponse(key, nonce);
+                    handler->clearAuthNonce();
+                    if (PasswordHasher::constantTimeEquals(expected, response)) {
+                        qCDebug(DatabaseInterfaceLog) << "Login accepted: challenge-response password right";
+                        return PasswordRight;
+                    }
+                    qCDebug(DatabaseInterfaceLog) << "Login denied: challenge-response password wrong";
+                    return NotLoggedIn;
+                }
+
                 QString hashedPassword;
                 if (passwordNeedsHash) {
                     hashedPassword = PasswordHasher::computeHash(password, correctPasswordSha512.left(16));
@@ -558,6 +606,48 @@ 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;
+    }
+    // The guard makes a re-migration a no-op; only report success when a row was actually updated.
+    return query->numRowsAffected() > 0;
+}
+
 int Servatrice_DatabaseInterface::getUserIdInDB(const QString &name)
 {
     if (server->getAuthenticationMethod() == Servatrice::AuthenticationSql) {
@@ -1139,13 +1229,13 @@ bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user,
         return false;
     }
 
-    const QString correctPasswordSha512 = passwordQuery->value(0).toString();
-    QString oldPasswordSha512 = oldPassword;
-    if (oldPasswordNeedsHash) {
-        QString salt = correctPasswordSha512.left(16);
-        oldPasswordSha512 = PasswordHasher::computeHash(oldPassword, salt);
-    }
-    if (correctPasswordSha512 != oldPasswordSha512) {
+    const QString storedPassword = passwordQuery->value(0).toString();
+    // oldPasswordNeedsHash means the client sent the old password in plaintext. Verify it
+    // against whatever is stored: legacy salt+hash rows or already-migrated scrypt rows
+    // (which must NOT be re-hashed with a salt torn out of the "$scrypt$..." string).
+    const bool oldPasswordMatches = oldPasswordNeedsHash ? PasswordHasher::verifyPassword(oldPassword, storedPassword)
+                                                         : (oldPassword == storedPassword);
+    if (!oldPasswordMatches) {
         return false;
     }
 

servatrice/src/servatrice_database_interface.h

@@ -13,7 +13,7 @@
 #include <server.h>
 #include <server_database_interface.h>
 
-#define DATABASE_SCHEMA_VERSION 36
+#define DATABASE_SCHEMA_VERSION 37
 
 class Servatrice;
 
@@ -65,6 +65,8 @@ public:
     bool activeUserExists(const QString &user) override;
     bool userExists(const QString &user) override;
     QString getUserSalt(const QString &user) override;
+    QString getUserPasswordData(const QString &user) override;
+    bool submitPasswordVerifier(const QString &user, const QString &passwordVerifier) override;
     int getUserIdInDB(const QString &name);
     QMap<QString, ServerInfo_User> getBuddyList(const QString &name) override;
     QMap<QString, ServerInfo_User> getIgnoreList(const QString &name) override;

servatrice/src/serversocketinterface.cpp

@@ -107,6 +107,7 @@
 #include <libcockatrice/protocol/pb/serverinfo_user.pb.h>
 #include <libcockatrice/protocol/pb/serverinfo_user_alt.pb.h>
 #include <libcockatrice/protocol/pb/serverinfo_user_session.pb.h>
+#include <libcockatrice/utility/cryptoutil.h>
 #include <libcockatrice/utility/passwordhasher.h>
 #include <libcockatrice/utility/string_limits.h>
 #include <libcockatrice/utility/warning_categories.h>
@@ -139,7 +140,14 @@ bool AbstractServerSocketInterface::initSession()
     identEvent.set_server_version(VERSION_STRING);
     identEvent.set_protocol_version(protocolVersion);
     if (servatrice->getAuthenticationMethod() == Servatrice::AuthenticationSql) {
-        identEvent.set_server_options(Event_ServerIdentification::SupportsPasswordHash);
+        // 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::ServerOptions>(
+                Event_ServerIdentification::SupportsPasswordHash |
+                Event_ServerIdentification::SupportsChallengeResponseAuth);
+        identEvent.set_server_options(serverOptions);
     }
     SessionEvent *identSe = prepareSessionEvent(identEvent);
     sendProtocolItem(*identSe);
@@ -257,6 +265,8 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedSessionComm
             return cmdReportAddComment(cmd.GetExtension(Command_ReportAddComment::ext), rc);
         case SessionCommand::REPORT_DETAILS:
             return cmdReportDetails(cmd.GetExtension(Command_ReportDetails::ext), rc);
+        case SessionCommand::SUBMIT_PASSWORD_VERIFIER:
+            return cmdSubmitPasswordVerifier(cmd.GetExtension(Command_SubmitPasswordVerifier::ext), rc);
         default:
             return Response::RespFunctionNotAllowed;
     }
@@ -2461,6 +2471,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRegisterAccount(const C
         password = QString::fromStdString(cmd.hashed_password());
     }
 
+    // Reject credential formats the configured authentication strictness does not accept.
+    if (!acceptsCredentialFormat(passwordNeedsHash, password)) {
+        return Response::RespClientUpdateRequired;
+    }
+
     bool requireEmailActivation = settingsCache->value("registration/requireemailactivation", true).toBool();
     bool regSucceeded = sqlInterface->registerUser(userName, realName, password, passwordNeedsHash, parsedEmailAddress,
                                                    country, !requireEmailActivation);
@@ -2507,6 +2522,30 @@ bool AbstractServerSocketInterface::tooManyRegistrationAttempts(const QString &i
     return false;
 }
 
+bool AbstractServerSocketInterface::acceptsCredentialFormat(bool passwordNeedsHash, const QString &password) const
+{
+    // An empty credential must never reach the database: it would be accepted
+    // as a legacy format and stored as '' (fail-open on login, see the empty
+    // stored-credential guard in Servatrice_DatabaseInterface).
+    if (password.isEmpty()) {
+        return false;
+    }
+    // "scryptFormat" means the client sent a derived verifier rather than a
+    // password to hash ourselves. parsePasswordVerifier enforces the sane-cost
+    // clamp, so nothing starting with '$' reaches the database unparsed.
+    const bool scryptFormat = !passwordNeedsHash && PasswordHasher::parsePasswordVerifier(password).isValid;
+
+    // The strictness mode governs how existing legacy accounts are served, not which new-credential
+    // formats are tolerated: a legacy-mode server must still accept scrypt verifiers, because clients
+    // derive them whenever challenge-response is advertised (and it must be, so already-migrated
+    // scrypt rows keep logging in). strict is the only mode that rejects legacy formats.
+    if (servatrice->getAuthenticationStrictness() == Servatrice::AuthenticationStrict) {
+        return scryptFormat;
+    }
+    // legacy and mixed accept a valid scrypt verifier or a genuine legacy salt+hash.
+    return scryptFormat || PasswordHasher::isLegacyFormat(password);
+}
+
 Response::ResponseCode AbstractServerSocketInterface::cmdActivateAccount(const Command_Activate &cmd,
                                                                          ResponseContainer & /*rc*/)
 {
@@ -2841,6 +2880,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdAccountPassword(const C
         newPassword = QString::fromStdString(cmd.hashed_new_password());
     }
 
+    // Reject new credential formats the configured authentication strictness does not accept.
+    if (!acceptsCredentialFormat(newPasswordNeedsHash, newPassword)) {
+        return Response::RespClientUpdateRequired;
+    }
+
     QString userName = QString::fromStdString(userInfo->name());
     if (!databaseInterface->changeUserPassword(userName, oldPassword, true, newPassword, newPasswordNeedsHash)) {
         return Response::RespWrongPassword;
@@ -2978,6 +3022,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdForgotPasswordReset(con
         password = QString::fromStdString(cmd.hashed_new_password());
     }
 
+    // Reject new credential formats the configured authentication strictness does not accept.
+    if (!acceptsCredentialFormat(passwordNeedsHash, password)) {
+        return Response::RespClientUpdateRequired;
+    }
+
     if (sqlInterface->changeUserPassword(nameFromStdString(cmd.user_name()), password, passwordNeedsHash)) {
         if (servatrice->getEnableForgotPasswordAudit()) {
             sqlInterface->addAuditRecord(userName.simplified(), this->getAddress(), clientId.simplified(),
@@ -3038,8 +3087,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con
                                                                              ResponseContainer &rc)
 {
     const QString userName = nameFromStdString(cmd.user_name());
-    QString passwordSalt = sqlInterface->getUserSalt(userName);
-    if (passwordSalt.isEmpty()) {
+    const QString storedPasswordData = sqlInterface->getUserPasswordData(userName);
+    if (storedPasswordData.isEmpty()) {
         if (server->getRegOnlyServerEnabled()) {
             return Response::RespRegistrationRequired;
         } else {
@@ -3047,8 +3096,36 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con
             return Response::RespOk;
         }
     }
+
     auto *re = new Response_PasswordSalt;
-    re->set_password_salt(passwordSalt.toStdString());
+    if (PasswordHasher::isLegacyFormat(storedPasswordData)) {
+        re->set_password_salt(storedPasswordData.left(16).toStdString());
+        re->set_needs_migration(true);
+        // Legacy rows get a challenge-response nonce only outside legacy mode (there the client
+        // logs in with the legacy hash and the account is migrated). In legacy mode the row is
+        // served the legacy salt, since legacy mode only governs what NEW credentials are accepted.
+        if (servatrice->getAuthenticationStrictness() != Servatrice::AuthenticationLegacy) {
+            const QByteArray nonce = CryptoUtil::randomBytes(32);
+            setAuthNonce(nonce, userName);
+            re->set_nonce(nonce.constData(), nonce.size());
+        }
+    } else {
+        const PasswordVerifier verifier = PasswordHasher::parsePasswordVerifier(storedPasswordData);
+        if (!verifier.isValid) {
+            delete re;
+            return Response::RespContextError;
+        }
+        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);
+        // scrypt rows are served challenge-response in every mode so migrated accounts never lock out.
+        const QByteArray nonce = CryptoUtil::randomBytes(32);
+        setAuthNonce(nonce, userName);
+        re->set_nonce(nonce.constData(), nonce.size());
+    }
+
     rc.setResponseExtension(re);
     return Response::RespOk;
 }
@@ -3147,6 +3224,39 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReport(const Command_Re
     return Response::RespOk;
 }
 
+Response::ResponseCode
+AbstractServerSocketInterface::cmdSubmitPasswordVerifier(const Command_SubmitPasswordVerifier &cmd,
+                                                         ResponseContainer & /*rc*/)
+{
+    if (authState != PasswordRight) {
+        return Response::RespLoginNeeded;
+    }
+
+    // Limit to the size of the database column (password_sha512 varchar(255)).
+    constexpr int MAX_PASSWORD_VERIFIER_LENGTH = 255;
+    const QString passwordVerifier = QString::fromStdString(cmd.password_verifier());
+    if (passwordVerifier.isEmpty() || passwordVerifier.length() > MAX_PASSWORD_VERIFIER_LENGTH ||
+        PasswordHasher::isLegacyFormat(passwordVerifier)) {
+        return Response::RespContextError;
+    }
+
+    // Reject unparseable or hostile cost parameters before they reach the database.
+    const PasswordVerifier parsedVerifier = PasswordHasher::parsePasswordVerifier(passwordVerifier);
+    if (!parsedVerifier.isValid) {
+        qCWarning(AbstractServerSocketInterfaceLog)
+            << "Rejecting password verifier submission with invalid or insane cost parameters";
+        return Response::RespContextError;
+    }
+
+    if (!sqlInterface->submitPasswordVerifier(QString::fromStdString(userInfo->name()), passwordVerifier)) {
+        return Response::RespContextError;
+    }
+
+    qCDebug(AbstractServerSocketInterfaceLog)
+        << "Password verifier migrated for user" << QString::fromStdString(userInfo->name());
+    return Response::RespOk;
+}
+
 // ADMIN FUNCTIONS.
 // Permission is checked by the calling function.
 

servatrice/src/serversocketinterface.h

@@ -80,6 +80,7 @@ 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;
@@ -145,6 +146,7 @@ private:
     Response::ResponseCode cmdReportDetails(const Command_ReportDetails &cmd, ResponseContainer &rc);
     Response::ResponseCode cmdReportAddComment(const Command_ReportAddComment &cmd, ResponseContainer &rc);
     Response::ResponseCode cmdReplayDownloadByGameId(const Command_ReplayDownloadByGameId &cmd, ResponseContainer &rc);
+    Response::ResponseCode cmdSubmitPasswordVerifier(const Command_SubmitPasswordVerifier &cmd, ResponseContainer &rc);
     Response::ResponseCode
     processExtendedSessionCommand(int cmdType, const SessionCommand &cmd, ResponseContainer &rc) override;
     Response::ResponseCode

tests/password_hash_test.cpp

@@ -1,25 +1,9 @@
 #include "gtest/gtest.h"
-#include <libcockatrice/rng/rng_abstract.h>
-#include <libcockatrice/rng/rng_sfmt.h>
+#include <cstring>
 #include <libcockatrice/utility/passwordhasher.h>
 
-RNG_Abstract *rng;
-
 namespace
 {
-class PasswordHashTest : public ::testing::Test
-{
-protected:
-    void SetUp() override
-    {
-        rng = new RNG_SFMT;
-    }
-
-    void TearDown() override
-    {
-        delete rng;
-    }
-};
 
 TEST(PasswordHashTest, RegressionTest)
 {
@@ -29,6 +13,160 @@ TEST(PasswordHashTest, RegressionTest)
     QString hash = PasswordHasher::computeHash(password, salt);
     ASSERT_EQ(hash, salt + expected) << "The computed hash value remains the same";
 }
+
+TEST(PasswordHashTest, SaltUsesAlphanumericCharset)
+{
+    static const char alphanum[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
+    const QString salt = PasswordHasher::generateRandomSalt();
+    ASSERT_EQ(salt.size(), 16);
+    for (const QChar &c : salt) {
+        ASSERT_NE(strchr(alphanum, c.toLatin1()), nullptr);
+    }
+}
+
+TEST(PasswordHashTest, SaltsAreUnique)
+{
+    const QString salt1 = PasswordHasher::generateRandomSalt();
+    const QString salt2 = PasswordHasher::generateRandomSalt();
+    ASSERT_NE(salt1, salt2);
+}
+
+TEST(PasswordHashTest, TokenHasExpectedLength)
+{
+    const QString token = PasswordHasher::generateActivationToken();
+    ASSERT_EQ(token.size(), 16);
+}
+
+TEST(PasswordHashTest, DeriveKeyMatchesKnownVector)
+{
+    // RFC 7914 scrypt test vector, P="password", S="NaCl", N=1024, r=8, p=16
+    const QByteArray expected = QByteArray::fromHex("fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b"
+                                                    "3731622eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cb"
+                                                    "dfa2cc0640");
+    const QByteArray derived = PasswordHasher::deriveKey("password", QByteArray("NaCl"), 1024, 8, 16);
+    ASSERT_EQ(derived.toHex(), expected.toHex());
+}
+
+TEST(PasswordHashTest, PasswordVerifierRoundTrip)
+{
+    const QString stored = PasswordHasher::generatePasswordVerifier("hunter2");
+    ASSERT_FALSE(stored.isEmpty());
+    ASSERT_TRUE(stored.startsWith("$scrypt$"));
+
+    const PasswordVerifier parsed = PasswordHasher::parsePasswordVerifier(stored);
+    ASSERT_TRUE(parsed.isValid);
+    ASSERT_EQ(parsed.format, PasswordFormat::Scrypt);
+    ASSERT_EQ(parsed.n, SCRYPT_N);
+    ASSERT_EQ(parsed.r, SCRYPT_R);
+    ASSERT_EQ(parsed.p, SCRYPT_P);
+    ASSERT_EQ(parsed.salt.size(), SCRYPT_SALT_LENGTH);
+    ASSERT_EQ(parsed.verifier.size(), SCRYPT_VERIFIER_LENGTH);
+}
+
+TEST(PasswordHashTest, PasswordVerifierInvalidInput)
+{
+    ASSERT_FALSE(PasswordHasher::parsePasswordVerifier("garbage").isValid);
+    ASSERT_FALSE(PasswordHasher::parsePasswordVerifier("$scrypt$not-an-int$8$1$AAAA$BBBB").isValid);
+    ASSERT_FALSE(PasswordHasher::parsePasswordVerifier("$scrypt$1024$8$1$AAAA$too-short").isValid);
+    ASSERT_FALSE(PasswordHasher::parsePasswordVerifier("$pbkdf2-sha512$1000$AAAA$BBBB").isValid);
+}
+
+TEST(PasswordHashTest, LegacyFormatDetection)
+{
+    ASSERT_TRUE(PasswordHasher::isLegacyFormat("salt+hash"));
+    ASSERT_FALSE(PasswordHasher::isLegacyFormat(PasswordHasher::generatePasswordVerifier("password")));
+}
+
+TEST(PasswordHashTest, DeriveKeyDependsOnCostParameters)
+{
+    const QByteArray keyA = PasswordHasher::deriveKey("password", QByteArray("NaCl"), 1024, 8, 16);
+    const QByteArray keyB = PasswordHasher::deriveKey("password", QByteArray("NaCl"), 2048, 8, 16);
+    const QByteArray keyC = PasswordHasher::deriveKey("password", QByteArray("NaCl"), 1024, 8, 1);
+    ASSERT_NE(keyA, keyB);
+    ASSERT_NE(keyA, keyC);
+}
+
+TEST(PasswordHashTest, ComputeResponseIsDeterministic)
+{
+    const QByteArray nonce = QByteArray("a nonce value");
+    const QByteArray key = QByteArray("the verifier bytes");
+    const QByteArray r1 = PasswordHasher::computeResponse(key, nonce);
+    const QByteArray r2 = PasswordHasher::computeResponse(key, nonce);
+    const QByteArray r3 = PasswordHasher::computeResponse(QByteArray("a different key"), nonce);
+    ASSERT_EQ(r1, r2);
+    ASSERT_NE(r1, r3);
+}
+
+TEST(PasswordHashTest, ConstantTimeEquals)
+{
+    ASSERT_TRUE(PasswordHasher::constantTimeEquals(QByteArray("same"), QByteArray("same")));
+    ASSERT_FALSE(PasswordHasher::constantTimeEquals(QByteArray("same"), QByteArray("diff")));
+    ASSERT_FALSE(PasswordHasher::constantTimeEquals(QByteArray("short"), QByteArray("longer")));
+}
+
+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)

tests/settings/settings_defaults_test.cpp

@@ -238,6 +238,12 @@ TEST_F(SettingsDefaultsTest, Tabs_ModerationOpen_Default)
     ASSERT_EQ(s.getTabModerationOpen(), false);
 }
 
+TEST_F(SettingsDefaultsTest, Tabs_CardArtRulesOpen_Default)
+{
+    TabsSettings s(settingsPath, nullptr);
+    ASSERT_EQ(s.getTabCardArtRulesOpen(), false);
+}
+
 // --- ChatSettings ---
 
 TEST_F(SettingsDefaultsTest, Chat_Mention_Default)