From 2d9aca0b3ffb6db4f32ff718fb91c3adb6a023f6 Mon Sep 17 00:00:00 2001 From: elexis Date: Sat, 25 Aug 2018 14:34:30 +0000 Subject: [PATCH] Always require lobby authentication for lobby matches, refs #3549 / rP21520 / D897. This is due to too many oversteppings of the lobby Terms of Use following JS mods that implemented an UI for players to join lobby games with arbitrary nicknames or 'replace' / impersonate other players in lobby games. Agreed with: user1, Dunedan Code proofread by: Vladislav Minor discussions with: Imarok, Hannibal_Barca, smiley, fpre, bb, nani refs https://wildfiregames.com/forum/index.php?/topic/24722-improving-mod-security/ git-svn-id: https://svn.wildfiregames.com/public/ps/trunk@21877 3db68df2-c116-0410-a063-a993310a9797 --- binaries/data/config/default.cfg | 1 - binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js | 7 ++----- binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.xml | 10 ---------- source/lobby/scripting/JSInterface_Lobby.cpp | 13 ++++++++++++- source/lobby/scripting/JSInterface_Lobby.h | 3 ++- source/network/NetServer.cpp | 12 +++++++++++- source/network/NetServer.h | 13 +++++++++++-- source/network/scripting/JSInterface_Network.cpp | 8 +++++--- source/network/scripting/JSInterface_Network.h | 3 ++- 9 files changed, 45 insertions(+), 25 deletions(-) diff --git a/binaries/data/config/default.cfg b/binaries/data/config/default.cfg index 717e73bd65..9a09010e25 100644 --- a/binaries/data/config/default.cfg +++ b/binaries/data/config/default.cfg @@ -421,7 +421,6 @@ xpartamupp = "wfgbot23" ; Name of the server-side XMPP-account that echelon = "echelon23" ; Name of the server-side XMPP-account that manages ratings buddies = "," ; Comma separated list of playernames that the current user has marked as buddies rememberpassword = true ; Whether to store the encrypted password in the user config -secureauth = true ; Secure Lobby Authentication: This prevents the impersonation of other players. The lobby server confirms the identity of the player before they join. [lobby.columns] gamerating = false ; Show the average rating of the participating players in a column of the gamelist diff --git a/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js b/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js index 2ee2cb0197..63e6d200e0 100644 --- a/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js +++ b/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js @@ -48,7 +48,6 @@ function init(attribs) case "host": { Engine.GetGUIObjectByName("hostSTUNWrapper").hidden = !Engine.HasXmppClient(); - Engine.GetGUIObjectByName("hostLobbyAuthWrapper").hidden = !Engine.HasXmppClient(); if (Engine.HasXmppClient()) { Engine.GetGUIObjectByName("hostPlayerName").caption = attribs.name; @@ -56,7 +55,6 @@ function init(attribs) sprintf(translate("%(name)s's game"), { "name": attribs.name }); Engine.GetGUIObjectByName("useSTUN").checked = Engine.ConfigDB_GetValue("user", "lobby.stun.enabled") == "true"; - Engine.GetGUIObjectByName("useLobbyAuth").checked = Engine.ConfigDB_GetValue("user", "lobby.secureauth") == "true"; } switchSetupPage("pageHost"); @@ -269,7 +267,7 @@ function switchSetupPage(newPage) if (newPage == "pageJoin" || newPage == "pageHost") { let pageSize = multiplayerPages.size; - let halfHeight = newPage == "pageJoin" ? 130 : Engine.HasXmppClient() ? 145 : 110; + let halfHeight = newPage == "pageJoin" ? 130 : Engine.HasXmppClient() ? 125 : 110; pageSize.top = -halfHeight; pageSize.bottom = halfHeight; multiplayerPages.size = pageSize; @@ -313,10 +311,9 @@ function startHost(playername, servername, port) } } - let useLobbyAuth = Engine.HasXmppClient() && Engine.GetGUIObjectByName("useLobbyAuth").checked; try { - Engine.StartNetworkHost(playername + (g_UserRating ? " (" + g_UserRating + ")" : ""), port, playername, useLobbyAuth); + Engine.StartNetworkHost(playername + (g_UserRating ? " (" + g_UserRating + ")" : ""), port, playername); } catch (e) { diff --git a/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.xml b/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.xml index 4515dc2321..b37a17c56e 100644 --- a/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.xml +++ b/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.xml @@ -114,16 +114,6 @@ Use STUN to work around firewalls - - - - saveSettingAndWriteToUserConfig("lobby.secureauth", String(this.checked)); - - - Require Lobby Authentication - This prevents the impersonation of other players. The lobby server confirms the identity of the player before they join. - - diff --git a/source/lobby/scripting/JSInterface_Lobby.cpp b/source/lobby/scripting/JSInterface_Lobby.cpp index b932946e2f..e3a90f2ad9 100644 --- a/source/lobby/scripting/JSInterface_Lobby.cpp +++ b/source/lobby/scripting/JSInterface_Lobby.cpp @@ -22,13 +22,17 @@ #include "gui/GUIManager.h" #include "lib/utf8.h" #include "lobby/IXmppClient.h" +#include "network/NetServer.h" +#include "ps/CLogger.h" #include "ps/CStr.h" -#include "ps/Profile.h" #include "ps/Util.h" #include "scriptinterface/ScriptInterface.h" +#include "scriptinterface/ScriptVal.h" #include "third_party/encryption/pkcs5_pbkdf2.h" +#include + void JSI_Lobby::RegisterScriptFunctions(const ScriptInterface& scriptInterface) { // Lobby functions @@ -154,6 +158,13 @@ void JSI_Lobby::SendRegisterGame(ScriptInterface::CxPrivate* pCxPrivate, JS::Han if (!g_XmppClient) return; + // Prevent JS mods to register matches in the lobby that were started with lobby authentication disabled + if (!g_NetServer || !g_NetServer->UseLobbyAuth()) + { + LOGERROR("Registering games in the lobby requires lobby authentication to be enabled!"); + return; + } + g_XmppClient->SendIqRegisterGame(*(pCxPrivate->pScriptInterface), data); } diff --git a/source/lobby/scripting/JSInterface_Lobby.h b/source/lobby/scripting/JSInterface_Lobby.h index f6bd2ca804..9e5c4faf22 100644 --- a/source/lobby/scripting/JSInterface_Lobby.h +++ b/source/lobby/scripting/JSInterface_Lobby.h @@ -20,7 +20,8 @@ #include "lib/config2.h" #include "scriptinterface/ScriptInterface.h" -#include "scriptinterface/ScriptVal.h" + +#include namespace JSI_Lobby { diff --git a/source/network/NetServer.cpp b/source/network/NetServer.cpp index 3fbf328228..08d2fd8b07 100644 --- a/source/network/NetServer.cpp +++ b/source/network/NetServer.cpp @@ -26,11 +26,13 @@ #include "NetStats.h" #include "lib/external_libraries/enet.h" +#include "lib/types.h" #include "network/StunClient.h" #include "ps/CLogger.h" #include "ps/ConfigDB.h" #include "ps/GUID.h" #include "ps/Profile.h" +#include "ps/ThreadUtil.h" #include "scriptinterface/ScriptInterface.h" #include "scriptinterface/ScriptRuntime.h" #include "simulation2/Simulation2.h" @@ -43,6 +45,8 @@ #include #endif +#include + /** * Number of peers to allocate for the enet host. * Limited by ENET_PROTOCOL_MAXIMUM_PEER_ID (4096). @@ -1556,7 +1560,8 @@ void CNetServerWorker::SendHolePunchingMessage(const CStr& ipStr, u16 port) CNetServer::CNetServer(bool useLobbyAuth, int autostartPlayers) : - m_Worker(new CNetServerWorker(useLobbyAuth, autostartPlayers)) + m_Worker(new CNetServerWorker(useLobbyAuth, autostartPlayers)), + m_LobbyAuth(useLobbyAuth) { } @@ -1565,6 +1570,11 @@ CNetServer::~CNetServer() delete m_Worker; } +bool CNetServer::UseLobbyAuth() const +{ + return m_LobbyAuth; +} + bool CNetServer::SetupConnection(const u16 port) { return m_Worker->SetupConnection(port); diff --git a/source/network/NetServer.h b/source/network/NetServer.h index da6fbb6bbc..f17aebecc7 100644 --- a/source/network/NetServer.h +++ b/source/network/NetServer.h @@ -20,11 +20,13 @@ #include "NetFileTransfer.h" #include "NetHost.h" - #include "lib/config2.h" +#include "lib/types.h" #include "ps/ThreadUtil.h" #include "scriptinterface/ScriptTypes.h" +#include +#include #include class CNetServerSession; @@ -137,12 +139,15 @@ public: */ void SetTurnLength(u32 msecs); + bool UseLobbyAuth() const; + void OnLobbyAuth(const CStr& name, const CStr& token); void SendHolePunchingMessage(const CStr& ip, u16 port); private: CNetServerWorker* m_Worker; + const bool m_LobbyAuth; }; /** @@ -298,7 +303,11 @@ private: JS::PersistentRootedValue m_GameAttributes; int m_AutostartPlayers; - bool m_LobbyAuth; + + /** + * Whether this match requires lobby authentication. + */ + const bool m_LobbyAuth; ENetHost* m_Host; std::vector m_Sessions; diff --git a/source/network/scripting/JSInterface_Network.cpp b/source/network/scripting/JSInterface_Network.cpp index 8436532eb5..9b7fd7136a 100644 --- a/source/network/scripting/JSInterface_Network.cpp +++ b/source/network/scripting/JSInterface_Network.cpp @@ -21,6 +21,7 @@ #include "lib/external_libraries/enet.h" #include "lib/external_libraries/libsdl.h" +#include "lib/types.h" #include "lobby/IXmppClient.h" #include "network/NetClient.h" #include "network/NetMessage.h" @@ -50,13 +51,14 @@ JS::Value JSI_Network::FindStunEndpoint(ScriptInterface::CxPrivate* pCxPrivate, return StunClient::FindStunEndpointHost(*(pCxPrivate->pScriptInterface), port); } -void JSI_Network::StartNetworkHost(ScriptInterface::CxPrivate* pCxPrivate, const CStrW& playerName, const u16 serverPort, const CStr& hostLobbyName, bool useLobbyAuth) +void JSI_Network::StartNetworkHost(ScriptInterface::CxPrivate* pCxPrivate, const CStrW& playerName, const u16 serverPort, const CStr& hostLobbyName) { ENSURE(!g_NetClient); ENSURE(!g_NetServer); ENSURE(!g_Game); - g_NetServer = new CNetServer(useLobbyAuth); + // Always use lobby authentication for lobby matches to prevent impersonation and smurfing, in particular through mods that implemented an UI for arbitrary or other players nicknames. + g_NetServer = new CNetServer(static_cast(g_XmppClient)); if (!g_NetServer->SetupConnection(serverPort)) { pCxPrivate->pScriptInterface->ReportError("Failed to start server"); @@ -228,7 +230,7 @@ void JSI_Network::RegisterScriptFunctions(const ScriptInterface& scriptInterface scriptInterface.RegisterFunction("HasNetServer"); scriptInterface.RegisterFunction("HasNetClient"); scriptInterface.RegisterFunction("FindStunEndpoint"); - scriptInterface.RegisterFunction("StartNetworkHost"); + scriptInterface.RegisterFunction("StartNetworkHost"); scriptInterface.RegisterFunction("StartNetworkJoin"); scriptInterface.RegisterFunction("DisconnectNetworkGame"); scriptInterface.RegisterFunction("GetPlayerGUID"); diff --git a/source/network/scripting/JSInterface_Network.h b/source/network/scripting/JSInterface_Network.h index 1e74df5059..852bc47ace 100644 --- a/source/network/scripting/JSInterface_Network.h +++ b/source/network/scripting/JSInterface_Network.h @@ -18,6 +18,7 @@ #ifndef INCLUDED_JSI_NETWORK #define INCLUDED_JSI_NETWORK +#include "lib/types.h" #include "ps/CStr.h" #include "scriptinterface/ScriptInterface.h" @@ -28,7 +29,7 @@ namespace JSI_Network bool HasNetClient(ScriptInterface::CxPrivate* pCxPrivate); void StartNetworkGame(ScriptInterface::CxPrivate* pCxPrivate); void SetNetworkGameAttributes(ScriptInterface::CxPrivate* pCxPrivate, JS::HandleValue attribs1); - void StartNetworkHost(ScriptInterface::CxPrivate* pCxPrivate, const CStrW& playerName, const u16 serverPort, const CStr& hostLobbyName, bool useLobbyAuth); + void StartNetworkHost(ScriptInterface::CxPrivate* pCxPrivate, const CStrW& playerName, const u16 serverPort, const CStr& hostLobbyName); void StartNetworkJoin(ScriptInterface::CxPrivate* pCxPrivate, const CStrW& playerName, const CStr& serverAddress, u16 serverPort, bool useSTUN, const CStr& hostJID); JS::Value FindStunEndpoint(ScriptInterface::CxPrivate* pCxPrivate, int port); void DisconnectNetworkGame(ScriptInterface::CxPrivate* pCxPrivate); -- 2.11.4.GIT