mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-09-25 02:43:02 -07:00
[Security] Add challenge-response auth with scrypt verifiers and stop storing plaintext passwords
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
This commit is contained in:
parent
45d97cb8d2
commit
fb7f7dde55
33 changed files with 676 additions and 23 deletions
|
|
@ -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,36 @@ 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 &hostname,
|
||||
const QString &userName,
|
||||
const QString &verifier)
|
||||
{
|
||||
Q_UNUSED(hostname);
|
||||
Q_UNUSED(userName);
|
||||
if (pendingSavePassword) {
|
||||
SettingsCache::instance().servers().setServerPassword(pendingSaveName, verifier);
|
||||
}
|
||||
}
|
||||
|
||||
void ConnectionController::connectToServerDirect(const QString &host,
|
||||
unsigned int port,
|
||||
const QString &playerName,
|
||||
const QString &password)
|
||||
const QString &password,
|
||||
const QString &storedVerifier,
|
||||
const QString &saveName,
|
||||
bool savePassword)
|
||||
{
|
||||
pendingSaveName = saveName;
|
||||
pendingSavePassword = savePassword;
|
||||
remoteClient->setStoredVerifier(storedVerifier);
|
||||
remoteClient->connectToServer(host, port, playerName, password);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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 &hostname, const QString &userName, 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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
@ -333,10 +340,13 @@ void DlgConnect::actOk()
|
|||
}
|
||||
|
||||
servers.addNewServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(),
|
||||
playernameEdit->text().trimmed(), passwordEdit->text(), savePasswordCheckBox->isChecked());
|
||||
playernameEdit->text().trimmed(),
|
||||
passwordEdit->text().isEmpty() ? storedVerifier : passwordEdit->text(),
|
||||
savePasswordCheckBox->isChecked());
|
||||
} else {
|
||||
servers.updateExistingServer(saveEdit->text().trimmed(), hostEdit->text().trimmed(), portEdit->text().trimmed(),
|
||||
playernameEdit->text().trimmed(), passwordEdit->text(),
|
||||
playernameEdit->text().trimmed(),
|
||||
passwordEdit->text().isEmpty() ? storedVerifier : passwordEdit->text(),
|
||||
savePasswordCheckBox->isChecked());
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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,19 @@ public:
|
|||
{
|
||||
return passwordEdit->text();
|
||||
}
|
||||
//! \brief Stored "$scrypt$..." verifier for challenge-response servers (never the plaintext password).
|
||||
[[nodiscard]] QString getStoredVerifier() const
|
||||
{
|
||||
return storedVerifier;
|
||||
}
|
||||
[[nodiscard]] QString getSaveName() const
|
||||
{
|
||||
return saveEdit->text();
|
||||
}
|
||||
[[nodiscard]] bool getSavePassword() const
|
||||
{
|
||||
return savePasswordCheckBox->isChecked();
|
||||
}
|
||||
|
||||
public slots:
|
||||
void downloadThePublicServers();
|
||||
|
|
@ -77,6 +91,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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue