aboutsummaryrefslogtreecommitdiff
path: root/NorthstarDLL
diff options
context:
space:
mode:
authorMaya <malte.hoermeyer@web.de>2022-10-28 02:17:54 +0200
committerGitHub <noreply@github.com>2022-10-28 02:17:54 +0200
commit9df904da17dcf3f419ac9a2b5a9a4c5a4d303454 (patch)
tree43eccff74f7a0dc385fc7244d6da8c3510012331 /NorthstarDLL
parent4c99e6f02d7063ccc2e6c09531abd48c5bf9c91e (diff)
downloadNorthstarLauncher-9df904da17dcf3f419ac9a2b5a9a4c5a4d303454.tar.gz
NorthstarLauncher-9df904da17dcf3f419ac9a2b5a9a4c5a4d303454.zip
Improve authentication flow (#306)v1.10.2-rc1v1.10.2
* Check Name for Invalid Chars Co-authored-by: Emma-Miler <27428383+emma-miler@users.noreply.github.com> * Remove `m_bRequireClientAuth ` and add logging * Format changes * Remove debugging code * Add return values for CBaseClient_Connect * Update serverauthentication.cpp * Format changes * Fix singleplayer * Add comment about singleplayer * Format * Update serverauthentication.cpp * Update serverauthentication.cpp * Update serverauthentication.cpp Co-authored-by: RoyalBlue1 <realEmail@veryRealURL.com> Co-authored-by: Emma-Miler <27428383+emma-miler@users.noreply.github.com>
Diffstat (limited to 'NorthstarDLL')
-rw-r--r--NorthstarDLL/hoststate.cpp3
-rw-r--r--NorthstarDLL/masterserver.h1
-rw-r--r--NorthstarDLL/serverauthentication.cpp107
-rw-r--r--NorthstarDLL/serverauthentication.h6
4 files changed, 85 insertions, 32 deletions
diff --git a/NorthstarDLL/hoststate.cpp b/NorthstarDLL/hoststate.cpp
index adbcde3b..c36af20c 100644
--- a/NorthstarDLL/hoststate.cpp
+++ b/NorthstarDLL/hoststate.cpp
@@ -42,9 +42,6 @@ void, __fastcall, (CHostState* self))
if (g_pServerAuthentication->m_bNeedLocalAuthForNewgame)
SetCurrentPlaylist("tdm");
- // don't require authentication on singleplayer startup
- g_pServerAuthentication->m_bRequireClientAuth = strncmp(g_pHostState->m_levelName, "sp_", 3);
-
ServerStartingOrChangingMap();
double dStartTime = Tier0::Plat_FloatTime();
diff --git a/NorthstarDLL/masterserver.h b/NorthstarDLL/masterserver.h
index 0bbf7c76..eb784002 100644
--- a/NorthstarDLL/masterserver.h
+++ b/NorthstarDLL/masterserver.h
@@ -93,7 +93,6 @@ class MasterServerManager
bool m_bOriginAuthWithMasterServerDone = false;
bool m_bOriginAuthWithMasterServerInProgress = false;
- bool m_bRequireClientAuth = false;
bool m_bSavingPersistentData = false;
bool m_bScriptRequestingServerList = false;
diff --git a/NorthstarDLL/serverauthentication.cpp b/NorthstarDLL/serverauthentication.cpp
index 39136704..bb4eb2e3 100644
--- a/NorthstarDLL/serverauthentication.cpp
+++ b/NorthstarDLL/serverauthentication.cpp
@@ -28,6 +28,7 @@ const char* AUTHSERVER_VERIFY_STRING = "I am a northstar server!";
// global vars
ServerAuthenticationManager* g_pServerAuthentication;
+CBaseServer__RejectConnectionType CBaseServer__RejectConnection;
void ServerAuthenticationManager::StartPlayerAuthServer()
{
@@ -114,23 +115,58 @@ void ServerAuthenticationManager::RemovePlayer(R2::CBaseClient* player)
m_PlayerAuthenticationData.erase(player);
}
-void ServerAuthenticationManager::VerifyPlayerName(R2::CBaseClient* player, char* authToken, char* name)
+bool checkIsPlayerNameValid(const char* name)
+{
+ int len = strlen(name);
+ // Restricts name to max 63 characters
+ if (len >= 64)
+ return false;
+ for (int i = 0; i < len; i++)
+ {
+ // Restricts the characters in the name to a restricted range in ASCII
+ if (static_cast<int>(name[i]) < 32 || static_cast<int>(name[i]) > 126)
+ {
+ return false;
+ }
+ }
+ return true;
+}
+
+bool ServerAuthenticationManager::VerifyPlayerName(const char* authToken, const char* name)
{
std::lock_guard<std::mutex> guard(m_AuthDataMutex);
- if (!m_RemoteAuthenticationData.empty() && m_RemoteAuthenticationData.count(std::string(authToken)))
+ if (CVar_ns_auth_allow_insecure->GetInt())
{
- RemoteAuthData authData = m_RemoteAuthenticationData[authToken];
+ spdlog::info("Allowing player with name '{}' because ns_auth_allow_insecure is enabled", name);
+ return true;
+ }
- bool nameAccepted = (!*authData.username || !strcmp(name, authData.username));
+ if (!checkIsPlayerNameValid(name))
+ {
+ spdlog::info("Rejecting player with name '{}' because the name contains forbidden characters", name);
+ return false;
+ }
+ // TODO: We should really have a better way of doing this for singleplayer
+ // Best way of doing this would be to check if server is actually in singleplayer mode, or just running a SP map in multiplayer
+ // Currently there's not an easy way of checking this, so we just disable this check if mapname starts with `sp_`
+ // This means that player names are not checked on singleplayer
+ if ((m_RemoteAuthenticationData.empty() || m_RemoteAuthenticationData.count(std::string(authToken)) == 0) &&
+ strncmp(R2::g_pHostState->m_levelName, "sp_", 3) != 0)
+ {
+ spdlog::info("Rejecting player with name '{}' because authToken '{}' was not found", name, authToken);
+ return false;
+ }
- if (!nameAccepted && !CVar_ns_auth_allow_insecure->GetInt())
- {
- // limit name length to 64 characters just in case something changes, this technically shouldn't be needed given the master
- // server gets usernames from origin but we have it just in case
- strncpy_s(name, 64, authData.username, 63);
- }
+ const RemoteAuthData& authData = m_RemoteAuthenticationData[authToken];
+
+ if (*authData.username && strncmp(name, authData.username, 64) != 0)
+ {
+ spdlog::info("Rejecting player with name '{}' because name does not match expected name '{}'", name, authData.username);
+ return false;
}
+
+ return true;
}
bool ServerAuthenticationManager::CheckDuplicateAccounts(R2::CBaseClient* player)
@@ -151,6 +187,9 @@ bool ServerAuthenticationManager::AuthenticatePlayer(R2::CBaseClient* player, ui
std::string strUid = std::to_string(uid);
std::lock_guard<std::mutex> guard(m_AuthDataMutex);
+ if (!strncmp(R2::g_pHostState->m_levelName, "sp_", 3))
+ return true;
+
// copy uuid
strcpy(player->m_UID, strUid.c_str());
@@ -244,14 +283,14 @@ uint64_t iNextPlayerUid;
// clang-format off
AUTOHOOK(CBaseServer__ConnectClient, engine.dll + 0x114430,
void*,, (
- void* server,
- void* a2,
+ void* self,
+ void* addr,
void* a3,
uint32_t a4,
uint32_t a5,
int32_t a6,
void* a7,
- void* a8,
+ char* playerName,
char* serverFilter,
void* a10,
char a11,
@@ -267,7 +306,21 @@ void*,, (
pNextPlayerToken = serverFilter;
iNextPlayerUid = uid;
- return CBaseServer__ConnectClient(server, a2, a3, a4, a5, a6, a7, a8, serverFilter, a10, a11, a12, a13, a14, uid, a16, a17);
+ spdlog::info(
+ "CBaseServer__ClientConnect attempted connection with uid {}, playerName '{}', serverFilter '{}'", uid, playerName, serverFilter);
+
+ if (!g_pServerAuthentication->VerifyPlayerName(pNextPlayerToken, playerName))
+ {
+ CBaseServer__RejectConnection(self, *((int*)self + 3), addr, "Invalid Name.\n");
+ return nullptr;
+ }
+ if (!g_pBanSystem->IsUIDAllowed(uid))
+ {
+ CBaseServer__RejectConnection(self, *((int*)self + 3), addr, "Banned From server.\n");
+ return nullptr;
+ }
+
+ return CBaseServer__ConnectClient(self, addr, a3, a4, a5, a6, a7, playerName, serverFilter, a10, a11, a12, a13, a14, uid, a16, a17);
}
// clang-format off
@@ -275,27 +328,27 @@ AUTOHOOK(CBaseClient__Connect, engine.dll + 0x101740,
bool,, (R2::CBaseClient* self, char* name, void* netchan_ptr_arg, char b_fake_player_arg, void* a5, char* Buffer, void* a7))
// clang-format on
{
- // try changing name before all else
- g_pServerAuthentication->VerifyPlayerName(self, pNextPlayerToken, name);
-
// try to auth player, dc if it fails
// we connect regardless of auth, because returning bad from this function can fuck client state p bad
bool ret = CBaseClient__Connect(self, name, netchan_ptr_arg, b_fake_player_arg, a5, Buffer, a7);
if (!ret)
return ret;
+ if (!g_pServerAuthentication->VerifyPlayerName(pNextPlayerToken, name))
+ {
+ R2::CBaseClient__Disconnect(self, 1, "Invalid Name.\n");
+ return false;
+ }
if (!g_pBanSystem->IsUIDAllowed(iNextPlayerUid))
{
- R2::CBaseClient__Disconnect(self, 1, "Banned from server");
- return ret;
+ R2::CBaseClient__Disconnect(self, 1, "Banned From server.\n");
+ return false;
+ }
+ if (!g_pServerAuthentication->AuthenticatePlayer(self, iNextPlayerUid, pNextPlayerToken))
+ {
+ R2::CBaseClient__Disconnect(self, 1, "Authentication Failed.\n");
+ return false;
}
-
- if (strlen(name) >= 64) // fix for name overflow bug
- R2::CBaseClient__Disconnect(self, 1, "Invalid name");
- else if (
- !g_pServerAuthentication->AuthenticatePlayer(self, iNextPlayerUid, pNextPlayerToken) &&
- g_pServerAuthentication->m_bRequireClientAuth)
- R2::CBaseClient__Disconnect(self, 1, "Authentication Failed");
g_pServerAuthentication->AddPlayer(self, pNextPlayerToken);
g_pServerLimits->AddPlayer(self);
@@ -391,6 +444,8 @@ ON_DLL_LOAD_RELIESON("engine.dll", ServerAuthentication, (ConCommand, ConVar), (
// patch to disable fairfight marking players as cheaters and kicking them
module.Offset(0x101012).Patch("E9 90 00");
+ CBaseServer__RejectConnection = module.Offset(0x1182E0).As<CBaseServer__RejectConnectionType>();
+
if (Tier0::CommandLine()->CheckParm("-allowdupeaccounts"))
{
// patch to allow same of multiple account
diff --git a/NorthstarDLL/serverauthentication.h b/NorthstarDLL/serverauthentication.h
index 08854ac0..65050d56 100644
--- a/NorthstarDLL/serverauthentication.h
+++ b/NorthstarDLL/serverauthentication.h
@@ -22,6 +22,9 @@ struct PlayerAuthenticationData
bool needPersistenceWriteOnLeave = true;
};
+typedef int64_t (*CBaseServer__RejectConnectionType)(void* a1, unsigned int a2, void* a3, const char* a4, ...);
+extern CBaseServer__RejectConnectionType CBaseServer__RejectConnection;
+
class ServerAuthenticationManager
{
private:
@@ -36,7 +39,6 @@ class ServerAuthenticationManager
std::mutex m_AuthDataMutex;
std::unordered_map<std::string, RemoteAuthData> m_RemoteAuthenticationData;
std::unordered_map<R2::CBaseClient*, PlayerAuthenticationData> m_PlayerAuthenticationData;
- bool m_bRequireClientAuth = true;
bool m_bAllowDuplicateAccounts = false;
bool m_bRunningPlayerAuthThread = false;
bool m_bNeedLocalAuthForNewgame = false;
@@ -49,7 +51,7 @@ class ServerAuthenticationManager
void RemovePlayer(R2::CBaseClient* player);
bool CheckDuplicateAccounts(R2::CBaseClient* player);
bool AuthenticatePlayer(R2::CBaseClient* player, uint64_t uid, char* authToken);
- void VerifyPlayerName(R2::CBaseClient* player, char* authToken, char* name);
+ bool VerifyPlayerName(const char* authToken, const char* name);
bool RemovePlayerAuthData(R2::CBaseClient* player);
void WritePersistentData(R2::CBaseClient* player);
};