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

This commit is contained in:
Lukas Brübach 2026-09-02 20:12:45 +02:00 committed by GitHub
parent 46be02fbcf
commit 8ea3557696
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 20 additions and 1 deletions

View file

@ -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
{

View file

@ -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

View file

@ -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";
}
}