From e20846a88d5e23b3bb2b8254eef58990e70cb781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Wed, 2 Sep 2026 20:12:45 +0200 Subject: [PATCH] [Security] Refuse KDF downgrade; trim profile name and warn on silent credential drop --- .../src/interface/widgets/dialogs/dlg_connect.h | 5 ++++- .../network/client/remote/remote_client.cpp | 11 +++++++++++ .../libcockatrice/settings/servers_settings.cpp | 5 +++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_connect.h b/cockatrice/src/interface/widgets/dialogs/dlg_connect.h index 456c5af93..a1d6fcb23 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.h @@ -55,7 +55,10 @@ public: } [[nodiscard]] QString getSaveName() const { - return saveEdit->text(); + // 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 { diff --git a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp index c0c142891..04a58ae2d 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp @@ -311,6 +311,17 @@ void RemoteClient::passwordSaltResponse(const Response &response) 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 diff --git a/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp b/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp index 8ae63fc81..d98fe270c 100644 --- a/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/servers_settings.cpp @@ -151,6 +151,11 @@ void ServersSettings::setServerPassword(const QString &saveName, const QString & 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"; } }