mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-10-27 18:10:47 -05:00
Some checks failed
CodeQL / Analyze (cpp) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
Build Desktop / Configure (push) Has been cancelled
Build Desktop / Debian 13 (push) Has been cancelled
Build Desktop / Debian 12 (push) Has been cancelled
Build Desktop / Fedora 44 (push) Has been cancelled
Build Desktop / Fedora 43 (push) Has been cancelled
Build Desktop / Servatrice_Debian 12 (push) Has been cancelled
Build Desktop / Ubuntu 24.04 (push) Has been cancelled
Build Desktop / Ubuntu 26.04 (push) Has been cancelled
Build Desktop / Arch (push) Has been cancelled
Build Desktop / Ubuntu 26.04 cover (push) Has been cancelled
Build Desktop / macOS 13 Intel (push) Has been cancelled
Build Desktop / macOS 14 (push) Has been cancelled
Build Desktop / macOS 15 (push) Has been cancelled
Build Desktop / macOS 26 Debug (push) Has been cancelled
Build Desktop / Windows 10 (push) Has been cancelled
Build Desktop / Prune caches (push) Has been cancelled
Build Docker / Servatrice (arm) (push) Has been cancelled
Build Docker / Servatrice (x86) (push) Has been cancelled
Build Docker / Publish multi-platform Servatrice image (push) Has been cancelled
* [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 * [Security] Harden credential format acceptance and challenge nonce validation * [Security] Refuse KDF downgrade; trim profile name and warn on silent credential drop * [Security] Add SupportsAll ServerOptions enumerator to fix enum validation assert The server advertised both SupportsPasswordHash (1) and SupportsChallengeResponseAuth (2) via a bitwise-OR that produced the value 3, which no enumerator declared. Protobuf's proto2 enum validation rejects the value in _internal_set_server_options, crashing servatrice on the debug assertion. Add a SupportsAll = 3 enumerator with an identical wire value so the bitmask reads unchanged while the value is now a documented, valid member of the enum. * [Security] Normalize username when issuing challenge nonce cmdRequestPasswordSalt bound the challenge nonce to the raw, un-simplified username while cmdLogin verifies it against the simplified name. A user who typed leading or trailing whitespace in the username box would be silently rejected during challenge-response login even with the correct password. Simplify at the nonce-issuing site so both commands agree on the normalized name. Also drive-by formatting: collapse the now single-expression serverOptions initializer onto one line. * [Security] Keep an encrypted legacy password backup for old-client logins Migrating an account to scrypt stranded pre-challenge clients, which can only present the legacy salt+hash: after migration the server served them a scrypt salt they cannot use, so the only recovery was a password reset. When security/legacy_backup_key is configured (64 hex chars, openssl rand -hex 32), submitPasswordVerifier now seals the legacy credential with AES-256-GCM under that key before overwriting the row, storing it in password_legacy_backup with the time it was last used. Old clients keep working: cmdRequestPasswordSalt serves the backup's 16-char salt in password_salt so they can derive the legacy hash they were always able to compute, while new clients derive the verifier from the new scrypt_salt field, which always carries the verifier salt. The non-challenge login path accepts the plaintext or legacy hash against the decrypted backup and refreshes its window; a migrated row with no usable backup fails closed. Backups age out so a database dump taken after the window closes never carries a crackable legacy hash: getUsableLegacyBackup purges on first use past security/legacy_backup_ttl_days (default 90, clamped 1..3650), and a six-hourly sweep drops backups idle accounts never touch. changeUserPassword clears the backup since a new credential invalidates it. Without a configured key nothing is sealed or purged, so a temporarily missing key cannot destroy existing backups. The AAD is the simplified user name on both sides, sealing the blob to its row so it cannot be moved between accounts. Sealing failures degrade to the previous lockout behaviour rather than blocking migration. The columns extend the unmerged 0036->0037 migration, so the schema version stays 37. * [Security] Address review: proto compat, plaintext verify, AAD case, strict scope Fix the two blockers and follow-ups from the 2026-10-16 review: - Advertise challenge-response in a new supports_challenge_response field instead of the ServerOptions enum and drop SupportsAll: proto2 enums are closed, so already-released clients parsed server_options = 3 as NoOptions and fell back to plaintext passwords. server_options stays exactly SupportsPasswordHash on the wire; old clients ignore the new field. - Verify a plaintext password directly against a migrated scrypt row with verifyPassword() so email changes (cmdAccountEdit) and old clients sending password keep working without a legacy backup key. Pre-hashed values are still checked against the backup, and its TTL window is refreshed only when the backup is what actually matched. - Use the stored row name as the AES-GCM AAD on both seal and open so a login whose case differs from the stored name (utf8mb4_unicode_ci) still opens its backup. - Require challenge-response in strict mode only for registered SQL accounts (userExists gate), leaving guest/unregistered logins and none/password auth methods unaffected. - Document in servatrice.ini.example that a captured legacy hash can take over a not-yet-migrated account and that strict only constrains registered SQL accounts. Schema version is 38. * [Security] Raise legacy backup TTL default to 180 days Gives operators and slow-to-upgrade users a longer downgrade window for pre-challenge clients before unused encrypted legacy password backups age out. * [Security] Fix servatrice.cpp formatting per clang-format-18 * [Security] Add missing forward declaration for Command_SubmitPasswordVerifier The cmdSubmitPasswordVerifier declaration references the proto-generated type, which otherwise does not name a type in this header and breaks every C++ build. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: BruebachL <bruebachl@cockatrice.example>
249 lines
10 KiB
C++
249 lines
10 KiB
C++
#include "gtest/gtest.h"
|
|
#include <QChar>
|
|
#include <QString>
|
|
#include <QStringList>
|
|
#include <cstring>
|
|
#include <libcockatrice/utility/cryptoutil.h>
|
|
#include <libcockatrice/utility/passwordhasher.h>
|
|
#include <string>
|
|
|
|
namespace
|
|
{
|
|
|
|
TEST(PasswordHashTest, RegressionTest)
|
|
{
|
|
QString salt = "saltsaltsaltsalt";
|
|
QString password = "password";
|
|
QString expected = "vmKoWv975yf+WT2QCXhW48JNzZ2ghGxdgNvuKLBU0h7s6AQHSG72J6QO4ZswuSeqvBbAXbmgJSRBaSJrgc55WA==";
|
|
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"));
|
|
}
|
|
|
|
TEST(CryptoUtilTest, SecretRoundTrip)
|
|
{
|
|
const QByteArray key = CryptoUtil::randomBytes(32);
|
|
const QByteArray plaintext = PasswordHasher::computeHash("correct horse", "saltsaltsaltsalt").toUtf8();
|
|
const QByteArray aad = "alice";
|
|
|
|
const QString sealed = CryptoUtil::encryptSecret(plaintext, key, aad);
|
|
ASSERT_FALSE(sealed.isEmpty());
|
|
ASSERT_TRUE(sealed.startsWith("enc1:"));
|
|
ASSERT_EQ(CryptoUtil::decryptSecret(sealed, key, aad), plaintext);
|
|
}
|
|
|
|
TEST(CryptoUtilTest, SealIsNonDeterministic)
|
|
{
|
|
const QByteArray key = CryptoUtil::randomBytes(32);
|
|
const QByteArray plaintext = "same secret";
|
|
// Fresh random nonce per seal: identical inputs must not produce identical blobs.
|
|
ASSERT_NE(CryptoUtil::encryptSecret(plaintext, key), CryptoUtil::encryptSecret(plaintext, key));
|
|
}
|
|
|
|
TEST(CryptoUtilTest, RejectsWrongKey)
|
|
{
|
|
const QByteArray key = CryptoUtil::randomBytes(32);
|
|
const QByteArray otherKey = CryptoUtil::randomBytes(32);
|
|
const QString sealed = CryptoUtil::encryptSecret("secret", key, "alice");
|
|
ASSERT_FALSE(sealed.isEmpty());
|
|
ASSERT_TRUE(CryptoUtil::decryptSecret(sealed, otherKey, "alice").isEmpty());
|
|
}
|
|
|
|
TEST(CryptoUtilTest, RejectsWrongAad)
|
|
{
|
|
const QByteArray key = CryptoUtil::randomBytes(32);
|
|
const QString sealed = CryptoUtil::encryptSecret("secret", key, "alice");
|
|
ASSERT_FALSE(sealed.isEmpty());
|
|
ASSERT_TRUE(CryptoUtil::decryptSecret(sealed, key, "bob").isEmpty());
|
|
ASSERT_TRUE(CryptoUtil::decryptSecret(sealed, key).isEmpty());
|
|
}
|
|
|
|
TEST(CryptoUtilTest, RejectsTampering)
|
|
{
|
|
const QByteArray key = CryptoUtil::randomBytes(32);
|
|
const QString sealed = CryptoUtil::encryptSecret("secret", key);
|
|
ASSERT_FALSE(sealed.isEmpty());
|
|
|
|
// Flip a character inside the ciphertext segment (keep it valid base64).
|
|
QStringList parts = sealed.split(':');
|
|
ASSERT_EQ(parts.size(), 3);
|
|
QByteArray ciphertext = QByteArray::fromBase64(parts.at(2).toUtf8());
|
|
ciphertext[0] = static_cast<char>(ciphertext.at(0) ^ 0x01);
|
|
parts[2] = QString(ciphertext.toBase64());
|
|
ASSERT_TRUE(CryptoUtil::decryptSecret(parts.join(':'), key).isEmpty());
|
|
}
|
|
|
|
TEST(CryptoUtilTest, RejectsMalformedInput)
|
|
{
|
|
const QByteArray key = CryptoUtil::randomBytes(32);
|
|
ASSERT_TRUE(CryptoUtil::decryptSecret("garbage", key).isEmpty());
|
|
ASSERT_TRUE(CryptoUtil::decryptSecret("enc1:notbase64!!:also-not!", key).isEmpty());
|
|
ASSERT_TRUE(CryptoUtil::decryptSecret("enc1:AAAAAAAAAAAAAAAA:AAAA", key).isEmpty());
|
|
ASSERT_TRUE(CryptoUtil::decryptSecret("", key).isEmpty());
|
|
// A short or oversized key must never reach the cipher.
|
|
ASSERT_TRUE(
|
|
CryptoUtil::decryptSecret("enc1:AAAAAAAAAAAAAAAA:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", CryptoUtil::randomBytes(16))
|
|
.isEmpty());
|
|
ASSERT_TRUE(CryptoUtil::encryptSecret("secret", CryptoUtil::randomBytes(16)).isEmpty());
|
|
}
|
|
|
|
} // namespace
|
|
|
|
int main(int argc, char **argv)
|
|
{
|
|
::testing::InitGoogleTest(&argc, argv);
|
|
return RUN_ALL_TESTS();
|
|
}
|