Fix broken 'SV_IsClientBanned' implementation

SV_IsClientBanned is now checked after the creating of CClient, and thus 'CServer::AuthClient' has been inlined into 'CServer::ConnectClient'. Since we have to request the master server to check whether or not this particular client is globally banned, we have to detach it into a separate thread, and process the results back into the main thread once finished. This takes too much time, so we connect the client anyways and if the master server marked it as 'banned', we disconnect it right away.

r5dev/engine/server/server.cpp

@@ -79,12 +79,17 @@ int CServer::GetNumClients(void) const
 }
 
 //---------------------------------------------------------------------------------
-// Purpose: client to server authentication
+// Purpose: Initializes a CSVClient for a new net connection. This will only be called
-// Input  : *pChallenge - 
+//			once for a player each game, not once for each level change.
-// Output : true if user isn't banned, false otherwise
+// Input  : *pServer - 
+//			*pInpacket - 
+// Output : pointer to client instance on success, nullptr on failure
 //---------------------------------------------------------------------------------
-bool CServer::AuthClient(user_creds_s* pChallenge)
+CClient* CServer::ConnectClient(CServer* pServer, user_creds_s* pChallenge)
 {
+	if (pServer->m_State < server_state_t::ss_active)
+		return nullptr;
+
 	char* pszPersonaName = pChallenge->personaName;
 	uint64_t nNucleusID = pChallenge->personaId;
 
@@ -98,47 +103,33 @@ bool CServer::AuthClient(user_creds_s* pChallenge)
 	// Only proceed connection if the client's name is valid and UTF-8 encoded.
 	if (!VALID_CHARSTAR(pszPersonaName) || !IsValidUTF8(pszPersonaName) || !IsValidPersonaName(pszPersonaName))
 	{
-		RejectConnection(m_Socket, &pChallenge->netAdr, "#Valve_Reject_Invalid_Name");
+		pServer->RejectConnection(pServer->m_Socket, &pChallenge->netAdr, "#Valve_Reject_Invalid_Name");
 		if (bEnableLogging)
 			Warning(eDLL_T::SERVER, "Connection rejected for '%s' ('%llu' has an invalid name!)\n", pszAddresBuffer, nNucleusID);
 
-		return false;
+		return nullptr;
 	}
 
 	if (g_pBanSystem->IsBanListValid())
 	{
 		if (g_pBanSystem->IsBanned(pszAddresBuffer, nNucleusID))
 		{
-			RejectConnection(m_Socket, &pChallenge->netAdr, "#Valve_Reject_Banned");
+			pServer->RejectConnection(pServer->m_Socket, &pChallenge->netAdr, "#Valve_Reject_Banned");
 			if (bEnableLogging)
 				Warning(eDLL_T::SERVER, "Connection rejected for '%s' ('%llu' is banned from this server!)\n", pszAddresBuffer, nNucleusID);
 
-			return false;
+			return nullptr;
 		}
 	}
 
-	if (sv_globalBanlist->GetBool())
+	CClient* pClient = v_CServer_ConnectClient(pServer, pChallenge);
+
+	if (pClient && sv_globalBanlist->GetBool())
 	{
-		std::thread th(SV_IsClientBanned, string(pszAddresBuffer), nNucleusID, string(pszPersonaName));
+		std::thread th(SV_IsClientBanned, pClient, string(pszAddresBuffer), nNucleusID, string(pszPersonaName));
 		th.detach();
 	}
 
-	return true;
-}
-
-//---------------------------------------------------------------------------------
-// Purpose: Initializes a CSVClient for a new net connection. This will only be called
-//			once for a player each game, not once for each level change.
-// Input  : *pServer - 
-//			*pInpacket - 
-// Output : pointer to client instance on success, nullptr on failure
-//---------------------------------------------------------------------------------
-CClient* CServer::ConnectClient(CServer* pServer, user_creds_s* pChallenge)
-{
-	if (pServer->m_State < server_state_t::ss_active || !pServer->AuthClient(pChallenge))
-		return nullptr;
-
-	CClient* pClient = v_CServer_ConnectClient(pServer, pChallenge);
 	return pClient;
 }
 

r5dev/engine/server/server.h

@@ -42,7 +42,6 @@ public:
 	bool IsActive(void) const { return m_State >= server_state_t::ss_active; }
 	bool IsLoading(void) const { return m_State == server_state_t::ss_loading; }
 	bool IsDedicated(void) const { return m_bIsDedicated; }
-	bool AuthClient(user_creds_s* pChallenge);
 	void RejectConnection(int iSocket, netadr_t* pNetAdr, const char* szMessage);
 	static CClient* ConnectClient(CServer* pServer, user_creds_s* pChallenge);
 	static void RunFrame(CServer* pServer);

r5dev/engine/server/sv_main.cpp

@@ -15,8 +15,10 @@
 //-----------------------------------------------------------------------------
 // Purpose: checks if particular client is banned on the comp server
 //-----------------------------------------------------------------------------
-void SV_IsClientBanned(const string& svIPAddr, const uint64_t nNucleusID, const string& svPersonaName)
+void SV_IsClientBanned(CClient* pClient, const string& svIPAddr, const uint64_t nNucleusID, const string& svPersonaName)
 {
+	Assert(pClient != nullptr);
+
 	string svError;
 	bool bCompBanned = g_pMasterServer->CheckForBan(svIPAddr, nNucleusID, svPersonaName, svError);
 
@@ -24,11 +26,19 @@ void SV_IsClientBanned(const string& svIPAddr, const uint64_t nNucleusID, const
 	{
 		if (!ThreadInMainThread())
 		{
-			g_TaskScheduler->Dispatch([svError, svIPAddr, nNucleusID]
+			g_TaskScheduler->Dispatch([pClient, svError, svIPAddr, nNucleusID]
 				{
-					g_pBanSystem->KickPlayerById(svIPAddr.c_str(), svError.c_str());
+					// Make sure client isn't already disconnected,
-					Warning(eDLL_T::SERVER, "Removed client '%s' ('%llu' is banned globally!)\n",
+					// and that if there is a valid netchannel, that
-						svIPAddr.c_str(), nNucleusID);
+					// it hasn't been taken by a different client by
+					// the time this task is getting executed.
+					CNetChan* pChan = pClient->GetNetChan();
+					if (pChan && pClient->GetNucleusID() == nNucleusID)
+					{
+						pClient->Disconnect(Reputation_t::REP_MARK_BAD, svError.c_str());
+						Warning(eDLL_T::SERVER, "Removed client '%s' ('%llu' is banned globally!)\n",
+							svIPAddr.c_str(), nNucleusID);
+					}
 				}, 0);
 		}
 	}
@@ -77,15 +87,16 @@ void SV_CheckForBan(const BannedVec_t* pBannedVec /*= nullptr*/)
 		const uint64_t nNucleusID = pClient->GetNucleusID();
 
 		if (!pBannedVec)
-			bannedVec.push_back(std::make_pair(szIPAddr, nNucleusID));
+			bannedVec.push_back(std::make_pair(string(szIPAddr), nNucleusID));
 		else
 		{
 			for (auto& it : *pBannedVec)
 			{
 				if (it.second == pClient->GetNucleusID())
 				{
-					Warning(eDLL_T::SERVER, "Removing client '%s' from slot '%i' ('%llu' is banned globally!)\n", szIPAddr, c, nNucleusID);
 					pClient->Disconnect(Reputation_t::REP_MARK_BAD, "%s", it.first.c_str());
+					Warning(eDLL_T::SERVER, "Removed client '%s' from slot '%i' ('%llu' is banned globally!)\n",
+						szIPAddr, c, nNucleusID);
 				}
 			}
 		}

r5dev/engine/server/sv_main.h

@@ -3,6 +3,7 @@
 #include "networksystem/bansystem.h"
 
 ///////////////////////////////////////////////////////////////////////////////
+class CClient;
 
 /* ==== SV_MAIN ======================================================================================================================================================= */
 inline CMemory p_SV_InitGameDLL;
@@ -27,7 +28,7 @@ inline bool* s_bIsDedicated = nullptr;
 void SV_InitGameDLL();
 void SV_ShutdownGameDLL();
 bool SV_ActivateServer();
-void SV_IsClientBanned(const string& svIPAddr, const uint64_t nNucleusID, const string& svPersonaName);
+void SV_IsClientBanned(CClient* pClient, const string& svIPAddr, const uint64_t nNucleusID, const string& svPersonaName);
 void SV_CheckForBan(const BannedVec_t* pBannedVec = nullptr);
 ///////////////////////////////////////////////////////////////////////////////