Merge 2096c3c3cc into 3aed24de26

.github/workflows/check-builds.yaml

@@ -52,7 +52,7 @@ jobs:
           Copy-Item -Path "config.ini" -Destination "bin"
         shell: pwsh
       - name: Upload build artifact
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v2
         with:
           name: 'ubuntu22_04-bin-x64-${{ env.SHORT_SHA }}'
           path: bin
@@ -106,7 +106,7 @@ jobs:
           }
         shell: pwsh
       - name: Upload build artifact
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v2
         with:
           name: 'windows-vs2019-bin-x64-${{ env.SHORT_SHA }}'
           path: bin
@@ -127,7 +127,7 @@ jobs:
           GITDESC=$(git describe --tags)
           mkdir $GITDESC
           echo "ARTDIR=$GITDESC" >> $GITHUB_ENV
-      - uses: actions/download-artifact@v4
+      - uses: actions/download-artifact@v3
         with:
           path: ${{ env.ARTDIR }}
       - name: Upload artifacts

config.ini

@@ -17,10 +17,6 @@ acceptallcustomnames=true
 # should attempts to log into non-existent accounts
 # automatically create them?
 autocreateaccounts=true
-# list of supported authentication methods (comma-separated)
-# password = allow login type 1 with plaintext passwords
-# cookie = allow login type 2 with one-shot auth cookies
-authmethods=password
 # how often should everything be flushed to the database?
 # the default is 4 minutes
 dbsaveinterval=240

sql/migration4.sql

@@ -1,19 +0,0 @@
-/*
-    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,
-    Expires     INTEGER DEFAULT 0 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;

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,17 +153,9 @@ 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,
-    Expires     INTEGER DEFAULT 0 NOT NULL,
-    FOREIGN KEY(AccountID) REFERENCES Accounts(AccountID) ON DELETE CASCADE,
-    UNIQUE (AccountID)
-);

src/core/CNStructs.hpp

@@ -40,7 +40,6 @@
 
 // wrapper for U16toU8
 #define ARRLEN(x) (sizeof(x)/sizeof(*x))
-#define AUTOU8(x) std::string((char*)x, ARRLEN(x))
 #define AUTOU16TOU8(x) U16toU8(x, ARRLEN(x)) 
 
 // TODO: rewrite U16toU8 & U8toU16 to not use codecvt

src/db/Database.hpp

@@ -5,7 +5,7 @@
 #include <string>
 #include <vector>
 
-#define DATABASE_VERSION 5
+#define DATABASE_VERSION 4
 
 namespace Database {
 
@@ -53,10 +53,6 @@ namespace Database {
 
     void updateAccountLevel(int accountId, int accountLevel);
 
-    // return true if 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);

src/db/login.cpp

@@ -98,55 +98,6 @@ void Database::updateAccountLevel(int accountId, int accountLevel) {
     sqlite3_finalize(stmt);
 }
 
-bool Database::checkCookie(int accountId, const char *tryCookie) {
-    std::lock_guard<std::mutex> lock(dbCrit);
-
-    const char* sql_get = R"(
-        SELECT Cookie
-        FROM Auth
-        WHERE AccountID = ? AND Expires > ?;
-        )";
-
-    const char* sql_invalidate = R"(
-        UPDATE Auth
-        SET Expires = 0
-        WHERE AccountID = ?;
-        )";
-
-    sqlite3_stmt* stmt;
-
-    sqlite3_prepare_v2(db, sql_get, -1, &stmt, NULL);
-    sqlite3_bind_int(stmt, 1, accountId);
-    sqlite3_bind_int(stmt, 2, getTimestamp());
-    int rc = sqlite3_step(stmt);
-    if (rc != SQLITE_ROW) {
-        sqlite3_finalize(stmt);
-        return false;
-    }
-
-    const char *cookie = reinterpret_cast<const char*>(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 checkCookie(): " << sqlite3_errmsg(db) << std::endl;
-
-    return match;
-}
-
 void Database::updateSelected(int accountId, int slot) {
     std::lock_guard<std::mutex> lock(dbCrit);
 

src/servers/CNLoginServer.cpp

@@ -105,95 +105,57 @@ 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);
 
-    std::string userLogin;
+    /*
-    std::string userToken; // could be password or auth cookie
+     * 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)) {
         /*
          * The std::string -> char* -> std::string maneuver should remove any
          * trailing garbage after the null terminator.
          */
-    if (login->iLoginType == (int32_t)LoginType::COOKIE) {
-        userLogin = std::string(AUTOU8(login->szCookie_TEGid).c_str());
-        userToken = std::string(AUTOU8(login->szCookie_authid).c_str());
-    } else {
         userLogin = std::string(AUTOU16TOU8(login->szID).c_str());
-        userToken = std::string(AUTOU16TOU8(login->szPassword).c_str());
+        userPassword = std::string(AUTOU16TOU8(login->szPassword).c_str());
     }
 
-    // check username regex
+    // the client inserts a "\n" in the password if you press enter key in the middle of the password
-    if (!CNLoginServer::isUsernameGood(userLogin)) {
+    // (not at the start or the end of the password field)
-        // send a custom error message
+    if (int(userPassword.find("\n")) > 0)
-        INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
+        userPassword.erase(userPassword.find("\n"), 1);
-        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 = 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);
-    }
-
-    // we only interpret the token as a cookie if cookie login was used and it's allowed.
-    // otherwise we interpret it as a password, and this maintains compatibility with
-    // the auto-login trick used on older clients
-    bool isCookieAuth = login->iLoginType == (int32_t)LoginType::COOKIE
-                        && CNLoginServer::isLoginTypeAllowed(LoginType::COOKIE);
-
-    // password login checks
-    if (!isCookieAuth) {
-        // bail if password auth isn't allowed
-        if (!CNLoginServer::isLoginTypeAllowed(LoginType::PASSWORD)) {
-            // send a custom error message
-            INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
-            std::string text = "Password login disabled\n";
-            text += "This server has disabled logging in with plaintext passwords.\n";
-            text += "Please contact an admin for assistance.";
-            U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
-            msg.iDuringTime = 12;
-            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 regex
-        if (!CNLoginServer::isPasswordGood(userToken)) {
+    if (!CNLoginServer::isLoginDataGood(userLogin, userPassword)) {
         // send a custom error message
         INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
-            std::string text = "Invalid password\n";
+        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";          
         U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
-            msg.iDuringTime = 10;
+        msg.iDuringTime = 15;
         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) {
-        // don't auto-create an account if it's a cookie auth for whatever reason
+        if (settings::AUTOCREATEACCOUNTS)
-        if (settings::AUTOCREATEACCOUNTS && !isCookieAuth)
+            return newAccount(sock, userLogin, userPassword, login->iClientVerC);
-            return newAccount(sock, userLogin, userToken, login->iClientVerC);
 
         return loginFail(LoginError::ID_DOESNT_EXIST, userLogin, sock);
     }
 
-    if (isCookieAuth) {
+    if (!CNLoginServer::isPasswordCorrect(findUser.Password, userPassword))
-        const char *cookie = userToken.c_str();
-        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, userToken))
-            return loginFail(LoginError::ID_AND_PASSWORD_DO_NOT_MATCH, userLogin, sock);
-    }
 
     // is the account banned
     if (findUser.BannedUntil > getTimestamp()) {
@@ -659,14 +621,11 @@ bool CNLoginServer::exitDuplicate(int accountId) {
     return false;
 }
 
-bool CNLoginServer::isUsernameGood(std::string login) {
+bool CNLoginServer::isLoginDataGood(std::string login, std::string password) {
-    const std::regex loginRegex("[a-zA-Z0-9_-]{4,32}");
+    std::regex loginRegex("[a-zA-Z0-9_-]{4,32}");
-    return (std::regex_match(login, loginRegex));
+    std::regex passwordRegex("[a-zA-Z0-9!@#$%^&*()_+]{8,32}");
-}
 
-bool CNLoginServer::isPasswordGood(std::string password) {
+    return (std::regex_match(login, loginRegex) && std::regex_match(password, passwordRegex));
-    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) {
@@ -679,17 +638,4 @@ bool CNLoginServer::isCharacterNameGood(std::string Firstname, std::string Lastn
     std::regex lastnamecheck(R"(((?! )(?!\.)[a-zA-Z0-9]*\.{0,1}(?!\.+ +)[a-zA-Z0-9]* {0,1}(?! +))*$)");
     return (std::regex_match(Firstname, firstnamecheck) && std::regex_match(Lastname, lastnamecheck));
 }
-
-bool CNLoginServer::isLoginTypeAllowed(LoginType loginType) {
-    // the config file specifies "comma-separated" but tbh we don't care
-    switch (loginType) {
-    case LoginType::PASSWORD:
-        return settings::AUTHMETHODS.find("password") != std::string::npos;
-    case LoginType::COOKIE:
-        return settings::AUTHMETHODS.find("cookie") != std::string::npos;
-    default:
-        break;
-    }
-    return false;
-}
 #pragma endregion

src/servers/CNLoginServer.hpp

@@ -23,11 +23,6 @@ enum class LoginError {
     UPDATED_EUALA_REQUIRED = 9
 };
 
-enum class LoginType {
-    PASSWORD = 1,
-    COOKIE = 2
-};
-
 // WARNING: THERE CAN ONLY BE ONE OF THESE SERVERS AT A TIME!!!!!! TODO: change loginSessions & packet handlers to be non-static
 class CNLoginServer : public CNServer {
 private:
@@ -44,12 +39,10 @@ private:
     static void changeName(CNSocket* sock, CNPacketData* data);
     static void duplicateExit(CNSocket* sock, CNPacketData* data);
 
-    static bool isUsernameGood(std::string login);
+    static bool isLoginDataGood(std::string login, std::string password);
-    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);
-    static bool isLoginTypeAllowed(LoginType loginType);
     static void newAccount(CNSocket* sock, std::string userLogin, std::string userPassword, int32_t clientVerC);
     // returns true if success
     static bool exitDuplicate(int accountId);

src/settings.cpp

@@ -13,7 +13,6 @@ bool settings::SANDBOX = true;
 int settings::LOGINPORT = 23000;
 bool settings::APPROVEALLNAMES = true;
 bool settings::AUTOCREATEACCOUNTS = true;
-std::string settings::AUTHMETHODS = "password";
 int settings::DBSAVEINTERVAL = 240;
 
 int settings::SHARDPORT = 23001;
@@ -88,7 +87,6 @@ void settings::init() {
     LOGINPORT = reader.GetInteger("login", "port", LOGINPORT);
     APPROVEALLNAMES = reader.GetBoolean("login", "acceptallcustomnames", APPROVEALLNAMES);
     AUTOCREATEACCOUNTS = reader.GetBoolean("login", "autocreateaccounts", AUTOCREATEACCOUNTS);
-    AUTHMETHODS = reader.Get("login", "authmethods", AUTHMETHODS);
     DBSAVEINTERVAL = reader.GetInteger("login", "dbsaveinterval", DBSAVEINTERVAL);
     SHARDPORT = reader.GetInteger("shard", "port", SHARDPORT);
     SHARDSERVERIP = reader.Get("shard", "ip", SHARDSERVERIP);

src/settings.hpp

@@ -9,7 +9,6 @@ namespace settings {
     extern int LOGINPORT;
     extern bool APPROVEALLNAMES;
     extern bool AUTOCREATEACCOUNTS;
-    extern std::string AUTHMETHODS;
     extern int DBSAVEINTERVAL;
     extern int SHARDPORT;
     extern std::string SHARDSERVERIP;