jeirmeister/Cockatrice
1
0
Fork 0
mirror of https://github.com/Cockatrice/Cockatrice.git synced 2026-09-24 02:13:02 -07:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Revert "[Security] Add challenge-response auth with scrypt verifiers and stop storing plaintext passwords"

Browse source 
This reverts commit 088e932882.


Took 6 minutes
This commit is contained in:
Lukas Brübach 2026-08-25 20:52:25 +02:00
parent 088e932882
commit 47df709f37
33 changed files with 23 additions and 675 deletions
Download patch file Download diff file Expand all files Collapse all files

3
servatrice/migrations/servatrice_0035_to_0036.sql
View file

@ -1,3 +0,0 @@
ALTER TABLE `cockatrice_users` MODIFY `password_sha512` char(255) NOT NULL, ALGORITHM=INSTANT;
UPDATE cockatrice_schema_version SET version=36 WHERE version=35;

10
servatrice/servatrice.ini.example
View file

@ -101,16 +101,6 @@ password=123456
; Accept only registered users? default is false (accept unregistered users)
regonly=false
[security]
; How strictly new authentication features are enforced. Possible values:
; * legacy: only accept the legacy 1000-round SHA-512 password hashes;
; * mixed: accept both legacy hashes and challenge-response authentication (default);
; * strict: only accept challenge-response authentication from clients that support it,
; and reject plain password submissions. Legacy accounts are migrated to PBKDF2
; automatically on their next successful login.
authentication_strictness=mixed
[users]
; The minimum length a username can be

4
servatrice/servatrice.sql
View file

@ -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(35);
-- 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(255) NOT NULL,
`password_sha512` char(120) NOT NULL,
`email` varchar(255) NOT NULL,
`country` char(2) NOT NULL,
`avatar_bmp` mediumblob NOT NULL,

12
servatrice/src/servatrice.cpp
View file

@ -865,18 +865,6 @@ 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")) {

11
servatrice/src/servatrice.h
View file

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

76
servatrice/src/servatrice_database_interface.cpp
View file

@ -354,41 +354,6 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot
qCWarning(DatabaseInterfaceLog) << "Login denied: user not active";
return UserIsInactive;
}
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)) {
return NotLoggedIn;
}
QByteArray key;
if (PasswordHasher::isLegacyFormat(correctPasswordSha512)) {
key = correctPasswordSha512.toUtf8();
} else {
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));
@ -587,47 +552,6 @@ 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;
}
return true;
}
int Servatrice_DatabaseInterface::getUserIdInDB(const QString &name)
{
if (server->getAuthenticationMethod() == Servatrice::AuthenticationSql) {

4
servatrice/src/servatrice_database_interface.h
View file

@ -10,7 +10,7 @@
#include <server.h>
#include <server_database_interface.h>
#define DATABASE_SCHEMA_VERSION 36
#define DATABASE_SCHEMA_VERSION 35
class Servatrice;
@ -62,8 +62,6 @@ 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;

82
servatrice/src/serversocketinterface.cpp
View file

@ -83,8 +83,6 @@
#include <libcockatrice/protocol/pb/serverinfo_deckstorage.pb.h>
#include <libcockatrice/protocol/pb/serverinfo_replay.pb.h>
#include <libcockatrice/protocol/pb/serverinfo_user.pb.h>
#include <libcockatrice/utility/cryptoutil.h>
#include <libcockatrice/utility/passwordhasher.h>
#include <libcockatrice/utility/string_limits.h>
#include <server_response_containers.h>
#include <server_room.h>
@ -115,12 +113,7 @@ bool AbstractServerSocketInterface::initSession()
identEvent.set_server_version(VERSION_STRING);
identEvent.set_protocol_version(protocolVersion);
if (servatrice->getAuthenticationMethod() == Servatrice::AuthenticationSql) {
Event_ServerIdentification::ServerOptions serverOptions = Event_ServerIdentification::SupportsPasswordHash;
if (servatrice->getAuthenticationStrictness() != Servatrice::AuthenticationLegacy) {
serverOptions = static_cast<Event_ServerIdentification::ServerOptions>(
serverOptions | Event_ServerIdentification::SupportsChallengeResponseAuth);
}
identEvent.set_server_options(serverOptions);
identEvent.set_server_options(Event_ServerIdentification::SupportsPasswordHash);
}
SessionEvent *identSe = prepareSessionEvent(identEvent);
sendProtocolItem(*identSe);
@ -230,9 +223,6 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedSessionComm
case SessionCommand::REQUEST_PASSWORD_SALT:
return cmdRequestPasswordSalt(cmd.GetExtension(Command_RequestPasswordSalt::ext), rc);
break;
case SessionCommand::SUBMIT_PASSWORD_VERIFIER:
return cmdSubmitPasswordVerifier(cmd.GetExtension(Command_SubmitPasswordVerifier::ext), rc);
break;
default:
return Response::RespFunctionNotAllowed;
}
@ -1406,12 +1396,6 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRegisterAccount(const C
password = QString::fromStdString(cmd.hashed_password());
}
// In strict mode only scrypt verifiers are accepted for new accounts.
if (servatrice->requiresChallengeResponseAuth() &&
(passwordNeedsHash || PasswordHasher::isLegacyFormat(password))) {
return Response::RespClientUpdateRequired;
}
bool requireEmailActivation = settingsCache->value("registration/requireemailactivation", true).toBool();
bool regSucceeded = sqlInterface->registerUser(userName, realName, password, passwordNeedsHash, parsedEmailAddress,
country, !requireEmailActivation);
@ -1792,12 +1776,6 @@ Response::ResponseCode AbstractServerSocketInterface::cmdAccountPassword(const C
newPassword = QString::fromStdString(cmd.hashed_new_password());
}
// In strict mode only scrypt verifiers are accepted.
if (servatrice->requiresChallengeResponseAuth() &&
(newPasswordNeedsHash || PasswordHasher::isLegacyFormat(newPassword))) {
return Response::RespClientUpdateRequired;
}
QString userName = QString::fromStdString(userInfo->name());
if (!databaseInterface->changeUserPassword(userName, oldPassword, true, newPassword, newPasswordNeedsHash)) {
return Response::RespWrongPassword;
@ -1933,12 +1911,6 @@ Response::ResponseCode AbstractServerSocketInterface::cmdForgotPasswordReset(con
password = QString::fromStdString(cmd.hashed_new_password());
}
// In strict mode only scrypt verifiers are accepted.
if (servatrice->requiresChallengeResponseAuth() &&
(passwordNeedsHash || PasswordHasher::isLegacyFormat(password))) {
return Response::RespClientUpdateRequired;
}
if (sqlInterface->changeUserPassword(nameFromStdString(cmd.user_name()), password, passwordNeedsHash)) {
if (servatrice->getEnableForgotPasswordAudit()) {
sqlInterface->addAuditRecord(userName.simplified(), this->getAddress(), clientId.simplified(),
@ -1998,8 +1970,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con
ResponseContainer &rc)
{
const QString userName = nameFromStdString(cmd.user_name());
const QString storedPasswordData = sqlInterface->getUserPasswordData(userName);
if (storedPasswordData.isEmpty()) {
QString passwordSalt = sqlInterface->getUserSalt(userName);
if (passwordSalt.isEmpty()) {
if (server->getRegOnlyServerEnabled()) {
return Response::RespRegistrationRequired;
} else {
@ -2007,58 +1979,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRequestPasswordSalt(con
return Response::RespOk;
}
}
auto *re = new Response_PasswordSalt;
const bool challengeResponseEnabled = servatrice->getAuthenticationStrictness() != Servatrice::AuthenticationLegacy;
if (PasswordHasher::isLegacyFormat(storedPasswordData)) {
re->set_password_salt(storedPasswordData.left(16).toStdString());
re->set_needs_migration(true);
} 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);
}
if (challengeResponseEnabled) {
const QByteArray nonce = CryptoUtil::randomBytes(32);
setAuthNonce(nonce);
re->set_nonce(nonce.constData(), nonce.size());
}
re->set_password_salt(passwordSalt.toStdString());
rc.setResponseExtension(re);
return Response::RespOk;
}
Response::ResponseCode
AbstractServerSocketInterface::cmdSubmitPasswordVerifier(const Command_SubmitPasswordVerifier &cmd,
ResponseContainer & /*rc*/)
{
if (authState != PasswordRight) {
return Response::RespLoginNeeded;
}
const QString passwordVerifier = QString::fromStdString(cmd.password_verifier());
if (passwordVerifier.isEmpty() || passwordVerifier.length() > MAX_NAME_LENGTH ||
PasswordHasher::isLegacyFormat(passwordVerifier)) {
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.

1
servatrice/src/serversocketinterface.h
View file

@ -122,7 +122,6 @@ private:
Response::ResponseCode cmdForgotPasswordChallenge(const Command_ForgotPasswordChallenge &cmd,
ResponseContainer &rc);
Response::ResponseCode cmdRequestPasswordSalt(const Command_RequestPasswordSalt &cmd, ResponseContainer &rc);
Response::ResponseCode cmdSubmitPasswordVerifier(const Command_SubmitPasswordVerifier &cmd, ResponseContainer &rc);
Response::ResponseCode processExtendedSessionCommand(int cmdType, const SessionCommand &cmd, ResponseContainer &rc);
Response::ResponseCode
processExtendedModeratorCommand(int cmdType, const ModeratorCommand &cmd, ResponseContainer &rc);