From a38b14b79a821a453b692bfa23ac431a9b38659e Mon Sep 17 00:00:00 2001 From: Gent Semaj Date: Thu, 5 Sep 2024 11:00:16 -0400 Subject: [PATCH] Auth cookie support --- sql/migration4.sql | 19 ++++++++++ sql/tables.sql | 14 +++++-- src/db/Database.hpp | 6 ++- src/db/login.cpp | 47 +++++++++++++++++++++++ src/servers/CNLoginServer.cpp | 71 ++++++++++++++++++++++------------- src/servers/CNLoginServer.hpp | 3 +- 6 files changed, 128 insertions(+), 32 deletions(-) create mode 100644 sql/migration4.sql diff --git a/sql/migration4.sql b/sql/migration4.sql new file mode 100644 index 0000000..9d943d6 --- /dev/null +++ b/sql/migration4.sql @@ -0,0 +1,19 @@ +/* + It is recommended in the SQLite manual to turn off + foreign keys when making schema changes that involve them +*/ +PRAGMA foreign_keys=OFF; +BEGIN TRANSACTION; +-- New table to store auth cookies +CREATE TABLE Auth ( + AccountID INTEGER NOT NULL, + Cookie TEXT NOT NULL, + Valid INTEGER NOT NULL, + FOREIGN KEY(AccountID) REFERENCES Accounts(AccountID) ON DELETE CASCADE, + UNIQUE (AccountID) +); +-- Update DB Version +UPDATE Meta SET Value = 5 WHERE Key = 'DatabaseVersion'; +UPDATE Meta SET Value = strftime('%s', 'now') WHERE Key = 'LastMigration'; +COMMIT; +PRAGMA foreign_keys=ON; \ No newline at end of file diff --git a/sql/tables.sql b/sql/tables.sql index 564e2ac..5a89ed2 100644 --- a/sql/tables.sql +++ b/sql/tables.sql @@ -143,7 +143,7 @@ CREATE TABLE IF NOT EXISTS EmailItems ( UNIQUE (PlayerID, MsgIndex, Slot) ); -CREATE TABLE IF NOT EXISTS RaceResults( +CREATE TABLE IF NOT EXISTS RaceResults ( EPID INTEGER NOT NULL, PlayerID INTEGER NOT NULL, Score INTEGER NOT NULL, @@ -153,9 +153,17 @@ CREATE TABLE IF NOT EXISTS RaceResults( FOREIGN KEY(PlayerID) REFERENCES Players(PlayerID) ON DELETE CASCADE ); -CREATE TABLE IF NOT EXISTS RedeemedCodes( +CREATE TABLE IF NOT EXISTS RedeemedCodes ( PlayerID INTEGER NOT NULL, Code TEXT NOT NULL, FOREIGN KEY(PlayerID) REFERENCES Players(PlayerID) ON DELETE CASCADE, UNIQUE (PlayerID, Code) -) +); + +CREATE TABLE IF NOT EXISTS Auth ( + AccountID INTEGER NOT NULL, + Cookie TEXT NOT NULL, + Valid INTEGER DEFAULT 0 NOT NULL, + FOREIGN KEY(AccountID) REFERENCES Accounts(AccountID) ON DELETE CASCADE, + UNIQUE (AccountID) +); diff --git a/src/db/Database.hpp b/src/db/Database.hpp index 8883137..ee1b9d6 100644 --- a/src/db/Database.hpp +++ b/src/db/Database.hpp @@ -5,7 +5,7 @@ #include #include -#define DATABASE_VERSION 4 +#define DATABASE_VERSION 5 namespace Database { @@ -53,6 +53,10 @@ namespace Database { void updateAccountLevel(int accountId, int accountLevel); + // return true iff cookie is valid for the account. + // invalidates the stored cookie afterwards + bool checkCookie(int accountId, const char *cookie); + // interface for the /ban command bool banPlayer(int playerId, std::string& reason); bool unbanPlayer(int playerId); diff --git a/src/db/login.cpp b/src/db/login.cpp index f3493ba..1e1404a 100644 --- a/src/db/login.cpp +++ b/src/db/login.cpp @@ -98,6 +98,53 @@ void Database::updateAccountLevel(int accountId, int accountLevel) { sqlite3_finalize(stmt); } +bool Database::checkCookie(int accountId, const char *tryCookie) { + std::lock_guard lock(dbCrit); + + const char* sql_get = R"( + SELECT Cookie + FROM Auth + WHERE AccountID = ? AND Valid = 1; + )"; + + const char* sql_invalidate = R"( + UPDATE Auth + SET Valid = 0 + WHERE AccountID = ?; + )"; + + sqlite3_stmt* stmt; + + sqlite3_prepare_v2(db, sql_get, -1, &stmt, NULL); + sqlite3_bind_int(stmt, 1, accountId); + int rc = sqlite3_step(stmt); + if (rc != SQLITE_ROW) { + sqlite3_finalize(stmt); + return false; + } + + const char *cookie = reinterpret_cast(sqlite3_column_text(stmt, 0)); + if (strlen(cookie) != strlen(tryCookie)) { + sqlite3_finalize(stmt); + return false; + } + + /* since cookies are immediately invalidated, we don't need to be concerned about + * timing-related side channel attacks, so strcmp is fine here + */ + bool match = (strcmp(cookie, tryCookie) == 0); + sqlite3_finalize(stmt); + + sqlite3_prepare_v2(db, sql_invalidate, -1, &stmt, NULL); + sqlite3_bind_int(stmt, 1, accountId); + rc = sqlite3_step(stmt); + sqlite3_finalize(stmt); + if (rc != SQLITE_DONE) + std::cout << "[WARN] Database fail on consumeCookie(): " << sqlite3_errmsg(db) << std::endl; + + return match; +} + void Database::updateSelected(int accountId, int slot) { std::lock_guard lock(dbCrit); diff --git a/src/servers/CNLoginServer.cpp b/src/servers/CNLoginServer.cpp index 427e7bb..9e905e6 100644 --- a/src/servers/CNLoginServer.cpp +++ b/src/servers/CNLoginServer.cpp @@ -105,15 +105,14 @@ void loginFail(LoginError errorCode, std::string userLogin, CNSocket* sock) { void CNLoginServer::login(CNSocket* sock, CNPacketData* data) { auto login = (sP_CL2LS_REQ_LOGIN*)data->buf; - // TODO: implement better way of sending credentials - std::string userLogin((char*)login->szCookie_TEGid); - std::string userPassword((char*)login->szCookie_authid); + bool isCookieAuth = login->iLoginType == 2; - /* - * Sometimes the client sends garbage cookie data. - * Validate it as normal credentials instead of using a length check before falling back. - */ - if (!CNLoginServer::isLoginDataGood(userLogin, userPassword)) { + std::string userLogin; + std::string userPassword; + if (isCookieAuth) { + // username encoded in TEGid raw + userLogin = std::string((char*)login->szCookie_TEGid); + } else { /* * The std::string -> char* -> std::string maneuver should remove any * trailing garbage after the null terminator. @@ -122,40 +121,55 @@ void CNLoginServer::login(CNSocket* sock, CNPacketData* data) { userPassword = std::string(AUTOU16TOU8(login->szPassword).c_str()); } - // the client inserts a "\n" in the password if you press enter key in the middle of the password - // (not at the start or the end of the password field) - if (int(userPassword.find("\n")) > 0) - userPassword.erase(userPassword.find("\n"), 1); - - // check regex - if (!CNLoginServer::isLoginDataGood(userLogin, userPassword)) { + // check username regex + if (!CNLoginServer::isUsernameGood(userLogin)) { // send a custom error message INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg); - std::string text = "Invalid login or password\n"; - text += "Login has to be 4 - 32 characters long and can't contain special characters other than dash and underscore\n"; - text += "Password has to be 8 - 32 characters long"; + std::string text = "Invalid login\n"; + text += "Login has to be 4 - 32 characters long and can't contain special characters other than dash and underscore"; U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg)); - msg.iDuringTime = 15; + msg.iDuringTime = 10; + sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE); + + // we still have to send login fail to prevent softlock + return loginFail(LoginError::LOGIN_ERROR, userLogin, sock); + } + + // check password regex if not cookie auth + if (!isCookieAuth && !CNLoginServer::isPasswordGood(userPassword)) { + // send a custom error message + INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg); + std::string text = "Invalid password\n"; + text += "Password has to be 8 - 32 characters long"; + U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg)); + msg.iDuringTime = 10; sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE); // we still have to send login fail to prevent softlock return loginFail(LoginError::LOGIN_ERROR, userLogin, sock); } - Database::Account findUser = {}; Database::findAccount(&findUser, userLogin); // account was not found if (findUser.AccountID == 0) { - if (settings::AUTOCREATEACCOUNTS) + // don't auto-create an account if it's a cookie auth for whatever reason + if (settings::AUTOCREATEACCOUNTS && !isCookieAuth) return newAccount(sock, userLogin, userPassword, login->iClientVerC); return loginFail(LoginError::ID_DOESNT_EXIST, userLogin, sock); } - if (!CNLoginServer::isPasswordCorrect(findUser.Password, userPassword)) - return loginFail(LoginError::ID_AND_PASSWORD_DO_NOT_MATCH, userLogin, sock); + if (isCookieAuth) { + const char *cookie = reinterpret_cast(login->szCookie_authid); + if (!Database::checkCookie(findUser.AccountID, cookie)) + return loginFail(LoginError::ID_AND_PASSWORD_DO_NOT_MATCH, userLogin, sock); + } else { + // simple password check + if (!CNLoginServer::isPasswordCorrect(findUser.Password, userPassword)) + return loginFail(LoginError::ID_AND_PASSWORD_DO_NOT_MATCH, userLogin, sock); + } // is the account banned if (findUser.BannedUntil > getTimestamp()) { @@ -621,11 +635,14 @@ bool CNLoginServer::exitDuplicate(int accountId) { return false; } -bool CNLoginServer::isLoginDataGood(std::string login, std::string password) { - std::regex loginRegex("[a-zA-Z0-9_-]{4,32}"); - std::regex passwordRegex("[a-zA-Z0-9!@#$%^&*()_+]{8,32}"); +bool CNLoginServer::isUsernameGood(std::string login) { + const std::regex loginRegex("[a-zA-Z0-9_-]{4,32}"); + return (std::regex_match(login, loginRegex)); +} - return (std::regex_match(login, loginRegex) && std::regex_match(password, passwordRegex)); +bool CNLoginServer::isPasswordGood(std::string password) { + const std::regex passwordRegex("[a-zA-Z0-9!@#$%^&*()_+]{8,32}"); + return (std::regex_match(password, passwordRegex)); } bool CNLoginServer::isPasswordCorrect(std::string actualPassword, std::string tryPassword) { diff --git a/src/servers/CNLoginServer.hpp b/src/servers/CNLoginServer.hpp index 9b13829..000154c 100644 --- a/src/servers/CNLoginServer.hpp +++ b/src/servers/CNLoginServer.hpp @@ -39,7 +39,8 @@ private: static void changeName(CNSocket* sock, CNPacketData* data); static void duplicateExit(CNSocket* sock, CNPacketData* data); - static bool isLoginDataGood(std::string login, std::string password); + static bool isUsernameGood(std::string login); + static bool isPasswordGood(std::string password); static bool isPasswordCorrect(std::string actualPassword, std::string tryPassword); static bool isAccountInUse(int accountId); static bool isCharacterNameGood(std::string Firstname, std::string Lastname);