From fa84a073ceb84d8cebb51f57629bd5711292c83f Mon Sep 17 00:00:00 2001 From: BobTheBob9 Date: Tue, 6 Dec 2022 20:47:50 +0000 Subject: generally cleanup authentication code and fix client state issues with rejection from local server --- NorthstarDLL/NorthstarDLL.vcxproj | 1 + NorthstarDLL/NorthstarDLL.vcxproj.filters | 7 +- NorthstarDLL/hoststate.cpp | 5 + NorthstarDLL/r2engine.h | 17 ++ NorthstarDLL/rejectconnectionfixes.cpp | 32 ++++ NorthstarDLL/serverauthentication.cpp | 251 ++++++++++++++---------------- NorthstarDLL/serverauthentication.h | 24 +-- 7 files changed, 190 insertions(+), 147 deletions(-) create mode 100644 NorthstarDLL/rejectconnectionfixes.cpp diff --git a/NorthstarDLL/NorthstarDLL.vcxproj b/NorthstarDLL/NorthstarDLL.vcxproj index 57ffb9ad..5e329f1f 100644 --- a/NorthstarDLL/NorthstarDLL.vcxproj +++ b/NorthstarDLL/NorthstarDLL.vcxproj @@ -621,6 +621,7 @@ + diff --git a/NorthstarDLL/NorthstarDLL.vcxproj.filters b/NorthstarDLL/NorthstarDLL.vcxproj.filters index 164d80b8..14f8c265 100644 --- a/NorthstarDLL/NorthstarDLL.vcxproj.filters +++ b/NorthstarDLL/NorthstarDLL.vcxproj.filters @@ -1490,7 +1490,7 @@ Header Files\Squirrel - + Header Files @@ -1499,7 +1499,7 @@ Header Files\Console - + Header Files\Squirrel @@ -1730,6 +1730,9 @@ Source Files\Console + + Source Files\Client + diff --git a/NorthstarDLL/hoststate.cpp b/NorthstarDLL/hoststate.cpp index 24e4bf63..54ef67a5 100644 --- a/NorthstarDLL/hoststate.cpp +++ b/NorthstarDLL/hoststate.cpp @@ -26,7 +26,12 @@ void ServerStartingOrChangingMap() // net_data_block_enabled is required for sp, force it if we're on an sp map // sucks for security but just how it be if (!strncmp(g_pHostState->m_levelName, "sp_", 3)) + { g_pCVar->FindVar("net_data_block_enabled")->SetValue(true); + g_pServerAuthentication->m_bStartingLocalSPGame = true; + } + else + g_pServerAuthentication->m_bStartingLocalSPGame = false; } // clang-format off diff --git a/NorthstarDLL/r2engine.h b/NorthstarDLL/r2engine.h index 2614b4cc..bb399f60 100644 --- a/NorthstarDLL/r2engine.h +++ b/NorthstarDLL/r2engine.h @@ -157,12 +157,29 @@ namespace R2 READY_REMOTE }; + enum class eSignonState : int + { + NONE = 0, // no state yet; about to connect + CHALLENGE = 1, // client challenging server; all OOB packets + CONNECTED = 2, // client is connected to server; netchans ready + NEW = 3, // just got serverinfo and string tables + PRESPAWN = 4, // received signon buffers + GETTINGDATA = 5, // respawn-defined signonstate, assumedly this is for persistence + SPAWN = 6, // ready to receive entity packets + FIRSTSNAP = 7, // another respawn-defined one + FULL = 8, // we are fully connected; first non-delta packet received + CHANGELEVEL = 9, // server is changing level; please wait + }; + // clang-format off OFFSET_STRUCT(CBaseClient) { STRUCT_SIZE(0x2D728) FIELD(0x16, char m_Name[64]) FIELD(0x258, KeyValues* m_ConVars) + FIELD(0x2A0, eSignonState m_Signon) + FIELD(0x358, char m_ClanTag[16]) + FIELD(0x484, bool m_bFakePlayer) FIELD(0x4A0, ePersistenceReady m_iPersistenceReady) FIELD(0x4FA, char m_PersistenceBuffer[PERSISTENCE_MAX_SIZE]) FIELD(0xF500, char m_UID[32]) diff --git a/NorthstarDLL/rejectconnectionfixes.cpp b/NorthstarDLL/rejectconnectionfixes.cpp new file mode 100644 index 00000000..6e08195f --- /dev/null +++ b/NorthstarDLL/rejectconnectionfixes.cpp @@ -0,0 +1,32 @@ +#include "pch.h" +#include "hoststate.h" +#include "r2engine.h" + +AUTOHOOK_INIT() + +// this is called from when our connection is rejected, this is the only case we're hooking this for +AUTOHOOK(COM_ExplainDisconnection, engine.dll + 0x1342F0, +void,, (bool a1, const char* fmt, ...)) +{ + va_list va; + va_start(va, fmt); + char buf[4096]; + vsnprintf_s(buf, 4096, fmt, va); + va_end(va); + + // slightly hacky comparison, but patching the function that calls this for reject would be worse + if (!strncmp(fmt, "Connection rejected: ", 21)) + { + // when COM_ExplainDisconnection is called from engine.dll + 19ff1c for connection rejected, it doesn't + // call Host_Disconnect, which properly shuts down listen server + // not doing this gets our client in a pretty weird state so we need to shut it down manually here + R2::g_pHostState->m_iNextState = R2::HostState_t::HS_GAME_SHUTDOWN; + } + + return COM_ExplainDisconnection(a1, buf); +} + +ON_DLL_LOAD_CLIENT("engine.dll", RejectConnectionFixes, (CModule module)) +{ + AUTOHOOK_DISPATCH() +} diff --git a/NorthstarDLL/serverauthentication.cpp b/NorthstarDLL/serverauthentication.cpp index 350d8521..ea5decd9 100644 --- a/NorthstarDLL/serverauthentication.cpp +++ b/NorthstarDLL/serverauthentication.cpp @@ -100,152 +100,139 @@ void ServerAuthenticationManager::StopPlayerAuthServer() m_PlayerAuthServer.stop(); } -void ServerAuthenticationManager::AddPlayer(R2::CBaseClient* player, const char* pToken) +void ServerAuthenticationManager::AddPlayer(R2::CBaseClient* pPlayer, const char* pToken) { PlayerAuthenticationData additionalData; - additionalData.pdataSize = m_RemoteAuthenticationData[pToken].pdataSize; - additionalData.usingLocalPdata = player->m_iPersistenceReady == R2::ePersistenceReady::READY_INSECURE; - m_PlayerAuthenticationData.insert(std::make_pair(player, additionalData)); -} + auto remoteAuthData = m_RemoteAuthenticationData.find(pToken); + if (remoteAuthData != m_RemoteAuthenticationData.end()) + additionalData.pdataSize = remoteAuthData->second.pdataSize; + else + additionalData.pdataSize = R2::PERSISTENCE_MAX_SIZE; -void ServerAuthenticationManager::RemovePlayer(R2::CBaseClient* player) -{ - if (m_PlayerAuthenticationData.count(player)) - m_PlayerAuthenticationData.erase(player); + additionalData.usingLocalPdata = pPlayer->m_iPersistenceReady == R2::ePersistenceReady::READY_INSECURE; + + m_PlayerAuthenticationData.insert(std::make_pair(pPlayer, additionalData)); } -bool checkIsPlayerNameValid(const char* name) +void ServerAuthenticationManager::RemovePlayer(R2::CBaseClient* pPlayer) { - 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(name[i]) < 32 || static_cast(name[i]) > 126) - { - return false; - } - } - return true; + if (m_PlayerAuthenticationData.count(pPlayer)) + m_PlayerAuthenticationData.erase(pPlayer); } -bool ServerAuthenticationManager::VerifyPlayerName(const char* authToken, const char* name) +bool ServerAuthenticationManager::VerifyPlayerName(const char* pAuthToken, const char* pName, char pOutVerifiedName[64]) { std::lock_guard guard(m_AuthDataMutex); - if (CVar_ns_auth_allow_insecure->GetInt()) - { - spdlog::info("Allowing player with name '{}' because ns_auth_allow_insecure is enabled", name); - return true; - } - - if (!checkIsPlayerNameValid(name)) - { - spdlog::info("Rejecting player with name '{}' because the name contains forbidden characters", name); + // always use name from masterserver if available + // use of strncpy_s here should verify that this is always nullterminated within valid buffer size + auto authData = m_RemoteAuthenticationData.find(pAuthToken); + if (authData != m_RemoteAuthenticationData.end() && *authData->second.username) + strncpy_s(pOutVerifiedName, 64, authData->second.username, 63); + else + strncpy_s(pOutVerifiedName, 64, pName, 63); + + // now, check that whatever name we have is actually valid + // first, make sure it's >1 char + if (!*pOutVerifiedName) 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; - } - const RemoteAuthData& authData = m_RemoteAuthenticationData[authToken]; - - if (*authData.username && strncmp(name, authData.username, 64) != 0) + // next, make sure it's within a valid range of ascii characters + for (int i = 0; pOutVerifiedName[i]; i++) { - spdlog::info("Rejecting player with name '{}' because name does not match expected name '{}'", name, authData.username); - return false; + if (pOutVerifiedName[i] < 32 || pOutVerifiedName[i] > 126) + return false; } return true; } -bool ServerAuthenticationManager::CheckDuplicateAccounts(R2::CBaseClient* player) +bool ServerAuthenticationManager::IsDuplicateAccount(R2::CBaseClient* pPlayer, const char* pPlayerUid) { if (m_bAllowDuplicateAccounts) - return true; + return false; bool bHasUidPlayer = false; for (int i = 0; i < R2::GetMaxPlayers(); i++) - if (&R2::g_pClientArray[i] != player && !strcmp(R2::g_pClientArray[i].m_UID, player->m_UID)) - return false; + if (&R2::g_pClientArray[i] != pPlayer && !strcmp(pPlayerUid, R2::g_pClientArray[i].m_UID)) + return true; - return true; + return false; } -bool ServerAuthenticationManager::AuthenticatePlayer(R2::CBaseClient* player, uint64_t uid, char* authToken) +bool ServerAuthenticationManager::CheckAuthentication(R2::CBaseClient* pPlayer, uint64_t iUid, char* pAuthToken) { - std::string strUid = std::to_string(uid); - std::lock_guard guard(m_AuthDataMutex); + std::string sUid = std::to_string(iUid); - if (!strncmp(R2::g_pHostState->m_levelName, "sp_", 3)) + // check whether this player's authentication is valid but don't actually write anything to the player, we'll do that later + // if we don't need auth this is valid + if (Cvar_ns_auth_allow_insecure->GetBool()) return true; - // copy uuid - strcpy(player->m_UID, strUid.c_str()); + // local server that doesn't need auth (probably sp) and local player + if (m_bStartingLocalSPGame && !strcmp(sUid.c_str(), R2::g_pLocalPlayerUserID)) + return true; - bool authFail = true; - if (!m_RemoteAuthenticationData.empty() && m_RemoteAuthenticationData.count(std::string(authToken))) - { - if (!CheckDuplicateAccounts(player)) - return false; + // don't allow duplicate accounts + if (IsDuplicateAccount(pPlayer, sUid.c_str())) + return false; - // use stored auth data - RemoteAuthData authData = m_RemoteAuthenticationData[authToken]; + std::lock_guard guard(m_AuthDataMutex); + auto authData = m_RemoteAuthenticationData.find(pAuthToken); + if (authData != m_RemoteAuthenticationData.end() && !strcmp(sUid.c_str(), authData->second.uid)) + return true; - if (!strcmp(strUid.c_str(), authData.uid)) // connecting client's uid is the same as auth's uid - { - // if we're resetting let script handle the reset - if (!m_bForceResetLocalPlayerPersistence || strcmp(authData.uid, R2::g_pLocalPlayerUserID)) - { - // copy pdata into buffer - memcpy(player->m_PersistenceBuffer, authData.pdata, authData.pdataSize); - } - - // set persistent data as ready - player->m_iPersistenceReady = R2::ePersistenceReady::READY_REMOTE; - authFail = false; - } - } + return false; +} + +void ServerAuthenticationManager::AuthenticatePlayer(R2::CBaseClient* pPlayer, uint64_t iUid, char* pAuthToken) +{ + // for bot players, generate a new uid + if (pPlayer->m_bFakePlayer) + iUid = 0; // is this a good way of doing things :clueless: + + std::string sUid = std::to_string(iUid); - if (authFail) + // copy uuid + strcpy(pPlayer->m_UID, sUid.c_str()); + + std::lock_guard guard(m_AuthDataMutex); + auto authData = m_RemoteAuthenticationData.find(pAuthToken); + if (authData != m_RemoteAuthenticationData.end()) { - if (CVar_ns_auth_allow_insecure->GetBool()) + // if we're resetting let script handle the reset with InitPersistentData() on connect + if (!m_bForceResetLocalPlayerPersistence || strcmp(sUid.c_str(), R2::g_pLocalPlayerUserID)) { - // set persistent data as ready - // note: actual placeholder persistent data is populated in script with InitPersistentData() - player->m_iPersistenceReady = R2::ePersistenceReady::READY_INSECURE; - return true; + // copy pdata into buffer + memcpy(pPlayer->m_PersistenceBuffer, authData->second.pdata, authData->second.pdataSize); } - else - return false; - } - return true; // auth successful, client stays on + // set persistent data as ready + pPlayer->m_iPersistenceReady = R2::ePersistenceReady::READY_REMOTE; + } + // we probably allow insecure at this point, but make sure not to write anyway if not insecure + else if (Cvar_ns_auth_allow_insecure->GetBool() || pPlayer->m_bFakePlayer) + { + // set persistent data as ready + // note: actual placeholder persistent data is populated in script with InitPersistentData() + pPlayer->m_iPersistenceReady = R2::ePersistenceReady::READY_INSECURE; + } } -bool ServerAuthenticationManager::RemovePlayerAuthData(R2::CBaseClient* player) +bool ServerAuthenticationManager::RemovePlayerAuthData(R2::CBaseClient* pPlayer) { if (!Cvar_ns_erase_auth_info->GetBool()) // keep auth data forever return false; // hack for special case where we're on a local server, so we erase our own newly created auth data on disconnect - if (m_bNeedLocalAuthForNewgame && !strcmp(player->m_UID, R2::g_pLocalPlayerUserID)) + if (m_bNeedLocalAuthForNewgame && !strcmp(pPlayer->m_UID, R2::g_pLocalPlayerUserID)) return false; // we don't have our auth token at this point, so lookup authdata by uid for (auto& auth : m_RemoteAuthenticationData) { - if (!strcmp(player->m_UID, auth.second.uid)) + if (!strcmp(pPlayer->m_UID, auth.second.uid)) { // pretty sure this is fine, since we don't iterate after the erase // i think if we iterated after it'd be undefined behaviour tho @@ -260,14 +247,14 @@ bool ServerAuthenticationManager::RemovePlayerAuthData(R2::CBaseClient* player) return false; } -void ServerAuthenticationManager::WritePersistentData(R2::CBaseClient* player) +void ServerAuthenticationManager::WritePersistentData(R2::CBaseClient* pPlayer) { - if (player->m_iPersistenceReady == R2::ePersistenceReady::READY_REMOTE) + if (pPlayer->m_iPersistenceReady == R2::ePersistenceReady::READY_REMOTE) { g_pMasterServerManager->WritePlayerPersistentData( - player->m_UID, (const char*)player->m_PersistenceBuffer, m_PlayerAuthenticationData[player].pdataSize); + pPlayer->m_UID, (const char*)pPlayer->m_PersistenceBuffer, m_PlayerAuthenticationData[pPlayer].pdataSize); } - else if (CVar_ns_auth_allow_insecure_write->GetBool()) + else if (Cvar_ns_auth_allow_insecure_write->GetBool()) { // todo: write pdata to disk here } @@ -306,54 +293,50 @@ void*,, ( pNextPlayerToken = serverFilter; iNextPlayerUid = uid; - 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); } +ConVar* Cvar_ns_allowuserclantags; + // clang-format off 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)) +bool,, (R2::CBaseClient* self, char* pName, void* pNetChannel, char bFakePlayer, void* a5, char pDisconnectReason[256], void* a7)) // clang-format on { - // 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; + const char* pAuthenticationFailure = nullptr; + char pVerifiedName[64]; - if (!g_pServerAuthentication->VerifyPlayerName(pNextPlayerToken, name)) + if (!bFakePlayer) { - R2::CBaseClient__Disconnect(self, 1, "Invalid Name.\n"); - return false; + if (!g_pServerAuthentication->VerifyPlayerName(pNextPlayerToken, pName, pVerifiedName)) + pAuthenticationFailure = "Invalid Name."; + else if (!g_pBanSystem->IsUIDAllowed(iNextPlayerUid)) + pAuthenticationFailure = "Banned From server."; + else if (!g_pServerAuthentication->CheckAuthentication(self, iNextPlayerUid, pNextPlayerToken)) + pAuthenticationFailure = "Authentication Failed."; } - if (!g_pBanSystem->IsUIDAllowed(iNextPlayerUid)) + else // need to copy name for bots still + strncpy_s(pVerifiedName, pName, 63); + + if (pAuthenticationFailure) { - R2::CBaseClient__Disconnect(self, 1, "Banned From server.\n"); + spdlog::info("{}'s (uid {}) connection was rejected: \"{}\"", pName, self->m_UID, pAuthenticationFailure); + + strncpy_s(pDisconnectReason, 256, pAuthenticationFailure, 255); return false; } - if (!g_pServerAuthentication->AuthenticatePlayer(self, iNextPlayerUid, pNextPlayerToken)) - { - R2::CBaseClient__Disconnect(self, 1, "Authentication Failed.\n"); + + // try to actually connect the player + if (!CBaseClient__Connect(self, pVerifiedName, pNetChannel, bFakePlayer, a5, pDisconnectReason, a7)) return false; - } + + // we already know this player's authentication data is legit, actually write it to them now + g_pServerAuthentication->AuthenticatePlayer(self, iNextPlayerUid, pNextPlayerToken); g_pServerAuthentication->AddPlayer(self, pNextPlayerToken); g_pServerLimits->AddPlayer(self); - return ret; + return true; } // clang-format off @@ -395,14 +378,10 @@ void,, (R2::CBaseClient* self, uint32_t unknownButAlways1, const char* pReason, // dcing, write persistent data if (g_pServerAuthentication->m_PlayerAuthenticationData[self].needPersistenceWriteOnLeave) g_pServerAuthentication->WritePersistentData(self); - g_pServerAuthentication->RemovePlayerAuthData(self); // won't do anything 99% of the time, but just in case - g_pServerAuthentication->RemovePlayer(self); - g_pServerLimits->RemovePlayer(self); - } + memset(self->m_PersistenceBuffer, 0, g_pServerAuthentication->m_PlayerAuthenticationData[self].pdataSize); + g_pServerAuthentication->RemovePlayerAuthData(self); // won't do anything 99% of the time, but just in case - else if (g_pServerAuthentication->m_RemoteAuthenticationData.count(self->m_Name)) - { g_pServerAuthentication->RemovePlayer(self); g_pServerLimits->RemovePlayer(self); } @@ -433,9 +412,9 @@ ON_DLL_LOAD_RELIESON("engine.dll", ServerAuthentication, (ConCommand, ConVar), ( g_pServerAuthentication->Cvar_ns_player_auth_port = new ConVar("ns_player_auth_port", "8081", FCVAR_GAMEDLL, ""); g_pServerAuthentication->Cvar_ns_erase_auth_info = new ConVar("ns_erase_auth_info", "1", FCVAR_GAMEDLL, "Whether auth info should be erased from this server on disconnect or crash"); - g_pServerAuthentication->CVar_ns_auth_allow_insecure = + g_pServerAuthentication->Cvar_ns_auth_allow_insecure = new ConVar("ns_auth_allow_insecure", "0", FCVAR_GAMEDLL, "Whether this server will allow unauthenicated players to connect"); - g_pServerAuthentication->CVar_ns_auth_allow_insecure_write = new ConVar( + g_pServerAuthentication->Cvar_ns_auth_allow_insecure_write = new ConVar( "ns_auth_allow_insecure_write", "0", FCVAR_GAMEDLL, diff --git a/NorthstarDLL/serverauthentication.h b/NorthstarDLL/serverauthentication.h index 65050d56..33154118 100644 --- a/NorthstarDLL/serverauthentication.h +++ b/NorthstarDLL/serverauthentication.h @@ -33,27 +33,33 @@ class ServerAuthenticationManager public: ConVar* Cvar_ns_player_auth_port; ConVar* Cvar_ns_erase_auth_info; - ConVar* CVar_ns_auth_allow_insecure; - ConVar* CVar_ns_auth_allow_insecure_write; + ConVar* Cvar_ns_auth_allow_insecure; + ConVar* Cvar_ns_auth_allow_insecure_write; std::mutex m_AuthDataMutex; std::unordered_map m_RemoteAuthenticationData; std::unordered_map m_PlayerAuthenticationData; + bool m_bAllowDuplicateAccounts = false; bool m_bRunningPlayerAuthThread = false; bool m_bNeedLocalAuthForNewgame = false; bool m_bForceResetLocalPlayerPersistence = false; + bool m_bStartingLocalSPGame = false; public: void StartPlayerAuthServer(); void StopPlayerAuthServer(); - void AddPlayer(R2::CBaseClient* player, const char* pToken); - void RemovePlayer(R2::CBaseClient* player); - bool CheckDuplicateAccounts(R2::CBaseClient* player); - bool AuthenticatePlayer(R2::CBaseClient* player, uint64_t uid, char* authToken); - bool VerifyPlayerName(const char* authToken, const char* name); - bool RemovePlayerAuthData(R2::CBaseClient* player); - void WritePersistentData(R2::CBaseClient* player); + + void AddPlayer(R2::CBaseClient* pPlayer, const char* pAuthToken); + void RemovePlayer(R2::CBaseClient* pPlayer); + + bool VerifyPlayerName(const char* pAuthToken, const char* pName, char pOutVerifiedName[64]); + bool IsDuplicateAccount(R2::CBaseClient* pPlayer, const char* pUid); + bool CheckAuthentication(R2::CBaseClient* pPlayer, uint64_t iUid, char* pAuthToken); + + void AuthenticatePlayer(R2::CBaseClient* pPlayer, uint64_t iUid, char* pAuthToken); + bool RemovePlayerAuthData(R2::CBaseClient* pPlayer); + void WritePersistentData(R2::CBaseClient* pPlayer); }; extern ServerAuthenticationManager* g_pServerAuthentication; -- cgit v1.2.3