Skip to content

Commit 4d0553e

Browse files
yungcomputerchairyungcomputerchair
committed
Refactor login packet handler for more flexible auth
1 parent 1543dac commit 4d0553e

File tree

4 files changed

+157
-98
lines changed

4 files changed

+157
-98
lines changed

‎config.ini

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ acceptallcustomnames=true
1818
# automatically create them?
1919
autocreateaccounts=true
2020
# list of supported authentication methods (comma-separated)
21-
# password = allow login type 1 with plaintext passwords
22-
# cookie = allow login type 2 with one-shot auth cookies
21+
# password = allow logging in with plaintext passwords
22+
# cookie = allow logging in with one-shot auth cookies
2323
authmethods=password
2424
# how often should everything be flushed to the database?
2525
# the default is 4 minutes

‎src/db/login.cpp

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,16 @@
22

33
#include "bcrypt/BCrypt.hpp"
44

5+
static int timingSafeStrcmp(const char* a, const char* b) {
6+
int diff = 0;
7+
while (*a && *b) {
8+
diff |= *a++ ^ *b++;
9+
}
10+
diff |= *a;
11+
diff |= *b;
12+
return diff;
13+
}
14+
515
void Database::findAccount(Account* account, std::string login) {
616
std::lock_guard<std::mutex> lock(dbCrit);
717

@@ -130,19 +140,21 @@ bool Database::checkCookie(int accountId, const char *tryCookie) {
130140
return false;
131141
}
132142

133-
/*
134-
* since cookies are immediately invalidated, we don't need to be concerned about
135-
* timing-related side channel attacks, so strcmp is fine here
136-
*/
137-
bool match = (strcmp(cookie, tryCookie) == 0);
143+
bool match = (timingSafeStrcmp(cookie, tryCookie) == 0);
138144
sqlite3_finalize(stmt);
139145

140-
sqlite3_prepare_v2(db, sql_invalidate, -1, &stmt, NULL);
141-
sqlite3_bind_int(stmt, 1, accountId);
142-
rc = sqlite3_step(stmt);
143-
sqlite3_finalize(stmt);
144-
if (rc != SQLITE_DONE)
145-
std::cout << "[WARN] Database fail on checkCookie(): " << sqlite3_errmsg(db) << std::endl;
146+
/*
147+
* Only invalidate the cookie if it was correct. This prevents
148+
* replay attacks without enabling DOS attacks on accounts.
149+
*/
150+
if (match) {
151+
sqlite3_prepare_v2(db, sql_invalidate, -1, &stmt, NULL);
152+
sqlite3_bind_int(stmt, 1, accountId);
153+
rc = sqlite3_step(stmt);
154+
sqlite3_finalize(stmt);
155+
if (rc != SQLITE_DONE)
156+
std::cout << "[WARN] Database fail on checkCookie(): " << sqlite3_errmsg(db) << std::endl;
157+
}
146158

147159
return match;
148160
}

‎src/servers/CNLoginServer.cpp

Lines changed: 120 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -109,111 +109,59 @@ void CNLoginServer::login(CNSocket* sock, CNPacketData* data) {
109109
std::string userLogin;
110110
std::string userToken; // could be password or auth cookie
111111

112+
/*
113+
* In this context, "cookie auth" just means the credentials were sent
114+
* in the szCookie fields instead of szID and szPassword.
115+
*/
116+
bool isCookieAuth = login->iLoginType == USE_COOKIE_FIELDS;
117+
112118
/*
113119
* The std::string -> char* -> std::string maneuver should remove any
114120
* trailing garbage after the null terminator.
115121
*/
116-
if (login->iLoginType == (int32_t)LoginType::COOKIE) {
122+
if (isCookieAuth) {
117123
userLogin = std::string(AUTOU8(login->szCookie_TEGid).c_str());
118124
userToken = std::string(AUTOU8(login->szCookie_authid).c_str());
119125
} else {
120126
userLogin = std::string(AUTOU16TOU8(login->szID).c_str());
121127
userToken = std::string(AUTOU16TOU8(login->szPassword).c_str());
122128
}
123129

124-
// check username regex
125-
if (!CNLoginServer::isUsernameGood(userLogin)) {
126-
// send a custom error message
127-
INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
128-
std::string text = "Invalid login\n";
129-
text += "Login has to be 4 - 32 characters long and can't contain special characters other than dash and underscore";
130-
U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
131-
msg.iDuringTime = 10;
132-
sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE);
133-
134-
// we still have to send login fail to prevent softlock
130+
if (!CNLoginServer::checkUsername(sock, userLogin)) {
135131
return loginFail(LoginError::LOGIN_ERROR, userLogin, sock);
136132
}
137133

138-
// we only interpret the token as a cookie if cookie login was used and it's allowed.
139-
// otherwise we interpret it as a password, and this maintains compatibility with
140-
// the auto-login trick used on older clients
141-
bool isCookieAuth = login->iLoginType == (int32_t)LoginType::COOKIE
142-
&& CNLoginServer::isLoginTypeAllowed(LoginType::COOKIE);
143-
144-
// password login checks
145134
if (!isCookieAuth) {
146-
// bail if password auth isn't allowed
147-
if (!CNLoginServer::isLoginTypeAllowed(LoginType::PASSWORD)) {
148-
// send a custom error message
149-
INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
150-
std::string text = "Password login disabled\n";
151-
text += "This server has disabled logging in with plaintext passwords.\n";
152-
text += "Please contact an admin for assistance.";
153-
U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
154-
msg.iDuringTime = 12;
155-
sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE);
156-
157-
// we still have to send login fail to prevent softlock
158-
return loginFail(LoginError::LOGIN_ERROR, userLogin, sock);
159-
}
160-
161-
// check regex
162-
if (!CNLoginServer::isPasswordGood(userToken)) {
163-
// send a custom error message
164-
INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
165-
std::string text = "Invalid password\n";
166-
text += "Password has to be 8 - 32 characters long";
167-
U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
168-
msg.iDuringTime = 10;
169-
sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE);
170-
171-
// we still have to send login fail to prevent softlock
135+
// password was sent in plaintext
136+
if (!CNLoginServer::checkPassword(sock, userToken)) {
172137
return loginFail(LoginError::LOGIN_ERROR, userLogin, sock);
173138
}
174139
}
175140

176141
Database::Account findUser = {};
177142
Database::findAccount(&findUser, userLogin);
178-
143+
179144
// account was not found
180145
if (findUser.AccountID == 0) {
181-
// don't auto-create an account if it's a cookie auth for whatever reason
182-
if (settings::AUTOCREATEACCOUNTS && !isCookieAuth)
146+
/*
147+
* Don't auto-create accounts if it's a cookie login.
148+
* It'll either be a bad cookie or a plaintext password sent by auto-login;
149+
* either way, we only want to allow auto-creation if the user explicitly entered their credentials.
150+
*/
151+
if (settings::AUTOCREATEACCOUNTS && !isCookieAuth) {
183152
return newAccount(sock, userLogin, userToken, login->iClientVerC);
153+
}
184154

185155
return loginFail(LoginError::ID_DOESNT_EXIST, userLogin, sock);
186156
}
187157

188-
if (isCookieAuth) {
189-
const char *cookie = userToken.c_str();
190-
if (!Database::checkCookie(findUser.AccountID, cookie))
191-
return loginFail(LoginError::ID_AND_PASSWORD_DO_NOT_MATCH, userLogin, sock);
192-
} else {
193-
// simple password check
194-
if (!CNLoginServer::isPasswordCorrect(findUser.Password, userToken))
195-
return loginFail(LoginError::ID_AND_PASSWORD_DO_NOT_MATCH, userLogin, sock);
158+
// make sure either a valid cookie or password was sent
159+
if (!CNLoginServer::checkToken(sock, findUser, userToken, isCookieAuth)) {
160+
return loginFail(LoginError::ID_AND_PASSWORD_DO_NOT_MATCH, userLogin, sock);
196161
}
197162

198-
// is the account banned
199-
if (findUser.BannedUntil > getTimestamp()) {
200-
// send a custom error message
201-
INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
202-
203-
// ceiling devision
204-
int64_t remainingDays = (findUser.BannedUntil-getTimestamp()) / 86400 + ((findUser.BannedUntil - getTimestamp()) % 86400 != 0);
205-
206-
std::string text = "Your account has been banned. \nReason: ";
207-
text += findUser.BanReason;
208-
text += "\nBan expires in " + std::to_string(remainingDays) + " day";
209-
if (remainingDays > 1)
210-
text += "s";
211-
212-
U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
213-
msg.iDuringTime = 99999999;
214-
sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE);
215-
// don't send fail packet
216-
return;
163+
if (CNLoginServer::checkBan(sock, findUser)) {
164+
return; // don't send fail packet
217165
}
218166

219167
/*
@@ -659,12 +607,12 @@ bool CNLoginServer::exitDuplicate(int accountId) {
659607
return false;
660608
}
661609

662-
bool CNLoginServer::isUsernameGood(std::string login) {
610+
bool CNLoginServer::isUsernameGood(std::string& login) {
663611
const std::regex loginRegex("[a-zA-Z0-9_-]{4,32}");
664612
return (std::regex_match(login, loginRegex));
665613
}
666614

667-
bool CNLoginServer::isPasswordGood(std::string password) {
615+
bool CNLoginServer::isPasswordGood(std::string& password) {
668616
const std::regex passwordRegex("[a-zA-Z0-9!@#$%^&*()_+]{8,32}");
669617
return (std::regex_match(password, passwordRegex));
670618
}
@@ -680,16 +628,107 @@ bool CNLoginServer::isCharacterNameGood(std::string Firstname, std::string Lastn
680628
return (std::regex_match(Firstname, firstnamecheck) && std::regex_match(Lastname, lastnamecheck));
681629
}
682630

683-
bool CNLoginServer::isLoginTypeAllowed(LoginType loginType) {
631+
bool CNLoginServer::isAuthMethodAllowed(AuthMethod authMethod) {
684632
// the config file specifies "comma-separated" but tbh we don't care
685-
switch (loginType) {
686-
case LoginType::PASSWORD:
633+
switch (authMethod) {
634+
case AuthMethod::PASSWORD:
687635
return settings::AUTHMETHODS.find("password") != std::string::npos;
688-
case LoginType::COOKIE:
636+
case AuthMethod::COOKIE:
689637
return settings::AUTHMETHODS.find("cookie") != std::string::npos;
690638
default:
691639
break;
692640
}
693641
return false;
694642
}
643+
644+
bool CNLoginServer::checkPassword(CNSocket* sock, std::string& password) {
645+
// check password auth allowed
646+
if (!CNLoginServer::isAuthMethodAllowed(AuthMethod::PASSWORD)) {
647+
// send a custom error message
648+
INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
649+
std::string text = "Password login disabled\n";
650+
text += "This server has disabled logging in with plaintext passwords.\n";
651+
text += "Please contact an admin for assistance.";
652+
U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
653+
msg.iDuringTime = 12;
654+
sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE);
655+
656+
return false;
657+
}
658+
659+
// check regex
660+
if (!CNLoginServer::isPasswordGood(password)) {
661+
// send a custom error message
662+
INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
663+
std::string text = "Invalid password\n";
664+
text += "Password has to be 8 - 32 characters long";
665+
U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
666+
msg.iDuringTime = 10;
667+
sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE);
668+
669+
return false;
670+
}
671+
672+
return true;
673+
}
674+
675+
bool CNLoginServer::checkUsername(CNSocket* sock, std::string& username) {
676+
// check username regex
677+
if (!CNLoginServer::isUsernameGood(username)) {
678+
// send a custom error message
679+
INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
680+
std::string text = "Invalid login\n";
681+
text += "Login has to be 4 - 32 characters long and can't contain special characters other than dash and underscore";
682+
U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
683+
msg.iDuringTime = 10;
684+
sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE);
685+
686+
return false;
687+
}
688+
689+
return true;
690+
}
691+
692+
bool CNLoginServer::checkToken(CNSocket* sock, Database::Account& account, std::string& token, bool isCookieAuth) {
693+
// check for valid cookie first
694+
if (isCookieAuth && CNLoginServer::isAuthMethodAllowed(AuthMethod::COOKIE)) {
695+
const char *cookie = token.c_str();
696+
if (Database::checkCookie(account.AccountID, cookie)) {
697+
return true;
698+
}
699+
}
700+
701+
// cookie check failed; check to see if it's a plaintext password sent by auto-login
702+
if (CNLoginServer::isAuthMethodAllowed(AuthMethod::PASSWORD)
703+
&& CNLoginServer::isPasswordCorrect(account.Password, token)) {
704+
return true;
705+
}
706+
707+
return false;
708+
}
709+
710+
bool CNLoginServer::checkBan(CNSocket* sock, Database::Account& account) {
711+
// check if the account is banned
712+
if (account.BannedUntil > getTimestamp()) {
713+
// send a custom error message
714+
INITSTRUCT(sP_FE2CL_GM_REP_PC_ANNOUNCE, msg);
715+
716+
// ceiling devision
717+
int64_t remainingDays = (account.BannedUntil-getTimestamp()) / 86400 + ((account.BannedUntil - getTimestamp()) % 86400 != 0);
718+
719+
std::string text = "Your account has been banned. \nReason: ";
720+
text += account.BanReason;
721+
text += "\nBan expires in " + std::to_string(remainingDays) + " day";
722+
if (remainingDays > 1)
723+
text += "s";
724+
725+
U8toU16(text, msg.szAnnounceMsg, sizeof(msg.szAnnounceMsg));
726+
msg.iDuringTime = 99999999;
727+
sock->sendPacket(msg, P_FE2CL_GM_REP_PC_ANNOUNCE);
728+
729+
return true;
730+
}
731+
732+
return false;
733+
}
695734
#pragma endregion

‎src/servers/CNLoginServer.hpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#pragma once
22

33
#include "core/Core.hpp"
4+
#include "db/Database.hpp"
45

56
#include "Player.hpp"
67

@@ -23,7 +24,9 @@ enum class LoginError {
2324
UPDATED_EUALA_REQUIRED = 9
2425
};
2526

26-
enum class LoginType {
27+
#define USE_COOKIE_FIELDS 2
28+
29+
enum class AuthMethod {
2730
PASSWORD = 1,
2831
COOKIE = 2
2932
};
@@ -44,12 +47,17 @@ class CNLoginServer : public CNServer {
4447
static void changeName(CNSocket* sock, CNPacketData* data);
4548
static void duplicateExit(CNSocket* sock, CNPacketData* data);
4649

47-
static bool isUsernameGood(std::string login);
48-
static bool isPasswordGood(std::string password);
50+
static bool isUsernameGood(std::string& login);
51+
static bool isPasswordGood(std::string& password);
4952
static bool isPasswordCorrect(std::string actualPassword, std::string tryPassword);
5053
static bool isAccountInUse(int accountId);
5154
static bool isCharacterNameGood(std::string Firstname, std::string Lastname);
52-
static bool isLoginTypeAllowed(LoginType loginType);
55+
static bool isAuthMethodAllowed(AuthMethod authMethod);
56+
static bool checkUsername(CNSocket* sock, std::string& username);
57+
static bool checkPassword(CNSocket* sock, std::string& password);
58+
static bool checkToken(CNSocket* sock, Database::Account& account, std::string& token, bool isCookieAuth);
59+
static bool checkBan(CNSocket* sock, Database::Account& account);
60+
5361
static void newAccount(CNSocket* sock, std::string userLogin, std::string userPassword, int32_t clientVerC);
5462
// returns true if success
5563
static bool exitDuplicate(int accountId);

0 commit comments

Comments
 (0)