diff --git a/source/network/NetClient.cpp b/source/network/NetClient.cpp index 7fe2875e84..8140054f8e 100644 --- a/source/network/NetClient.cpp +++ b/source/network/NetClient.cpp @@ -66,10 +66,14 @@ constexpr u32 NETWORK_BAD_PING = DEFAULT_TURN_LENGTH * COMMAND_DELAY_MP / 2; CNetClient *g_NetClient = NULL; -CNetClient::CNetClient(CGame* game, const CStrW& username, const CStr& hostJID) : +CNetClient::CNetClient(CGame* game, const CStrW& username, const CStr& hostJID, + std::string hashedPassword) : m_UserName{username}, m_HostJID{hostJID}, - m_Game{game} + m_Game{game}, + // Hash on top with the user's name, to make sure not all + // hashing data is in control of the host. + m_Password{HashCryptographically(std::move(hashedPassword), m_UserName.ToUTF8())} { m_Game->SetTurnManager(NULL); // delete the old local turn manager so we don't accidentally use it @@ -142,13 +146,6 @@ CNetClient::~CNetClient() DestroyConnection(); } -void CNetClient::SetGamePassword(const CStr& hashedPassword) -{ - // Hash on top with the user's name, to make sure not all - // hashing data is in control of the host. - m_Password = HashCryptographically(hashedPassword, m_UserName.ToUTF8()); -} - void CNetClient::SetControllerSecret(const std::string& secret) { m_ControllerSecret = secret; diff --git a/source/network/NetClient.h b/source/network/NetClient.h index 70ab481bfc..001fb072de 100644 --- a/source/network/NetClient.h +++ b/source/network/NetClient.h @@ -78,7 +78,8 @@ public: * The user's name will be displayed to all players. * The JID of the host is needed for the secure lobby authentication. */ - CNetClient(CGame* game, const CStrW& username = L"anonymous", const CStr& hostJID = {}); + CNetClient(CGame* game, const CStrW& username = L"anonymous", const CStr& hostJID = {}, + std::string hashedPassword = {}); virtual ~CNetClient(); @@ -86,12 +87,6 @@ public: bool IsController() const { return m_IsController; } - /** - * Set the game password. - * Must be called after SetUserName, as that is used to hash further. - */ - void SetGamePassword(const CStr& hashedPassword); - /** * Returns the GUID of the local client. * Used for distinguishing observers. diff --git a/source/network/NetServer.cpp b/source/network/NetServer.cpp index 63ceba6647..fab22881f9 100644 --- a/source/network/NetServer.cpp +++ b/source/network/NetServer.cpp @@ -111,9 +111,11 @@ static CStr DebugName(CNetServerSession* session) * See https://gitea.wildfiregames.com/0ad/0ad/issues/654 */ -CNetServerWorker::CNetServerWorker(const bool continueSavedGame, const bool useLobbyAuth) : +CNetServerWorker::CNetServerWorker(const bool continueSavedGame, const bool useLobbyAuth, + std::string password) : m_ContinuesSavedGame{continueSavedGame}, - m_LobbyAuth{useLobbyAuth} + m_LobbyAuth{useLobbyAuth}, + m_Password{std::move(password)} { } @@ -152,11 +154,6 @@ CNetServerWorker::~CNetServerWorker() delete m_ServerTurnManager; } -void CNetServerWorker::SetPassword(const CStr& hashedPassword) -{ - m_Password = hashedPassword; -} - void CNetServerWorker::SetControllerSecret(const std::string& secret) { @@ -1692,9 +1689,10 @@ void CNetServerWorker::SendHolePunchingMessage(const CStr& ipStr, u16 port) -CNetServer::CNetServer(const bool continueSavedGame, const bool useLobbyAuth) : - m_Worker{new CNetServerWorker{continueSavedGame, useLobbyAuth}}, - m_LobbyAuth{useLobbyAuth} +CNetServer::CNetServer(const bool continueSavedGame, const bool useLobbyAuth, std::string password) : + m_Worker{new CNetServerWorker{continueSavedGame, useLobbyAuth, password}}, + m_LobbyAuth{useLobbyAuth}, + m_Password{std::move(password)} { } @@ -1761,13 +1759,6 @@ bool CNetServer::IsBanned(const std::string& username) const return it != m_FailedAttempts.end() && it->second >= FAILED_PASSWORD_TRIES_BEFORE_BAN; } -void CNetServer::SetPassword(const CStr& password) -{ - m_Password = password; - std::lock_guard lock(m_Worker->m_WorkerMutex); - m_Worker->SetPassword(password); -} - void CNetServer::SetControllerSecret(const std::string& secret) { std::lock_guard lock(m_Worker->m_WorkerMutex); diff --git a/source/network/NetServer.h b/source/network/NetServer.h index 95eb51203d..5740d9006b 100644 --- a/source/network/NetServer.h +++ b/source/network/NetServer.h @@ -112,7 +112,7 @@ class CNetServer { NONCOPYABLE(CNetServer); public: - CNetServer(const bool isSavedGame, const bool useLobbyAuth = false); + CNetServer(const bool isSavedGame, const bool useLobbyAuth = false, std::string password = {}); ~CNetServer(); /** @@ -179,8 +179,6 @@ public: */ bool IsBanned(const std::string& username) const; - void SetPassword(const CStr& password); - void SetControllerSecret(const std::string& secret); private: @@ -230,13 +228,11 @@ public: private: friend class CNetServer; - CNetServerWorker(const bool continuesSavedGame, const bool useLobbyAuth); + CNetServerWorker(const bool continuesSavedGame, const bool useLobbyAuth, std::string password); ~CNetServerWorker(); bool CheckPassword(const std::string& password, const std::string& salt) const; - void SetPassword(const CStr& hashedPassword); - void SetControllerSecret(const std::string& secret); /** diff --git a/source/network/scripting/JSInterface_Network.cpp b/source/network/scripting/JSInterface_Network.cpp index e35aaf47b7..0feb48ffbc 100644 --- a/source/network/scripting/JSInterface_Network.cpp +++ b/source/network/scripting/JSInterface_Network.cpp @@ -83,8 +83,29 @@ void StartNetworkHost(const CStrW& playerName, const u16 serverPort, const CStr& ENSURE(!g_Game); // 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. - bool hasLobby = !!g_XmppClient; - g_NetServer = new CNetServer(continueSavedGame, hasLobby); + const bool hasLobby = !!g_XmppClient; + const std::string hostJID{hasLobby ? g_XmppClient->GetJID() : ""}; + + /** + * Password security - we want 0 A.D. to protect players from malicious hosts. We assume that clients + * might mistakenly send a personal password instead of the game password (e.g. enter their mail account's password on autopilot). + * Malicious dedicated servers might be set up to farm these failed logins and possibly obtain user credentials. + * Therefore, we hash the passwords on the client side before sending them to the server. + * This still makes the passwords potentially recoverable, but makes it much harder at scale. + * To prevent the creation of rainbow tables, hash with: + * - the host name + * - the client name (this makes rainbow tables completely unworkable unless a specific user is targeted, + * but that would require both computing the matching rainbow table _and_ for that specific user to mistype a personal password, + * at which point we assume the attacker would/could probably just rather use another means of obtaining the password). + * - the password itself + * - the engine version (so that the hashes change periodically) + * TODO: it should be possible to implement SRP or something along those lines to completely protect from this, + * but the cost/benefit ratio is probably not worth it. + */ + std::string hashedPassword = hasLobby ? + HashCryptographically(password, hostJID + password + PS_SERIALIZATION_VERSION) : ""; + + g_NetServer = new CNetServer(continueSavedGame, hasLobby, hashedPassword); if (!g_NetServer->SetupConnection(serverPort)) { @@ -105,32 +126,7 @@ void StartNetworkHost(const CStrW& playerName, const u16 serverPort, const CStr& g_NetServer->SetControllerSecret(secret); g_Game = new CGame(storeReplay); - const std::string hostJID{hasLobby ? g_XmppClient->GetJID() : ""}; - g_NetClient = new CNetClient(g_Game, playerName, hostJID); - - if (hasLobby) - { - /** - * Password security - we want 0 A.D. to protect players from malicious hosts. We assume that clients - * might mistakenly send a personal password instead of the game password (e.g. enter their mail account's password on autopilot). - * Malicious dedicated servers might be set up to farm these failed logins and possibly obtain user credentials. - * Therefore, we hash the passwords on the client side before sending them to the server. - * This still makes the passwords potentially recoverable, but makes it much harder at scale. - * To prevent the creation of rainbow tables, hash with: - * - the host name - * - the client name (this makes rainbow tables completely unworkable unless a specific user is targeted, - * but that would require both computing the matching rainbow table _and_ for that specific user to mistype a personal password, - * at which point we assume the attacker would/could probably just rather use another means of obtaining the password). - * - the password itself - * - the engine version (so that the hashes change periodically) - * TODO: it should be possible to implement SRP or something along those lines to completely protect from this, - * but the cost/benefit ratio is probably not worth it. - */ - CStr hashedPass = HashCryptographically(password, hostJID + password + PS_SERIALIZATION_VERSION); - g_NetServer->SetPassword(hashedPass); - g_NetClient->SetGamePassword(hashedPass); - } - + g_NetClient = new CNetClient(g_Game, playerName, hostJID, hashedPassword); g_NetClient->SetupServerData("127.0.0.1", serverPort); g_NetClient->SetControllerSecret(secret); @@ -174,8 +170,7 @@ void StartNetworkJoinLobby(const CStrW& playerName, const CStr& hostJID, const C CStr hashedPass = HashCryptographically(password, hostJID + password + PS_SERIALIZATION_VERSION); g_Game = new CGame(true); - g_NetClient = new CNetClient(g_Game, playerName, hostJID); - g_NetClient->SetGamePassword(hashedPass); + g_NetClient = new CNetClient(g_Game, playerName, hostJID, hashedPass); g_NetClient->SetupConnectionViaLobby(); }