Index: ps/trunk/source/lobby/IXmppClient.h =================================================================== --- ps/trunk/source/lobby/IXmppClient.h +++ ps/trunk/source/lobby/IXmppClient.h @@ -36,7 +36,7 @@ virtual void SendIqGetProfile(const std::string& player) = 0; virtual void SendIqGameReport(const ScriptRequest& rq, JS::HandleValue data) = 0; virtual void SendIqRegisterGame(const ScriptRequest& rq, JS::HandleValue data) = 0; - virtual void SendIqGetConnectionData(const std::string& jid, const std::string& password, bool localIP) = 0; + virtual void SendIqGetConnectionData(const std::string& jid, const std::string& password, const std::string& clientSalt, bool localIP) = 0; virtual void SendIqUnregisterGame() = 0; virtual void SendIqChangeStateGame(const std::string& nbp, const std::string& players) = 0; virtual void SendIqLobbyAuth(const std::string& to, const std::string& token) = 0; Index: ps/trunk/source/lobby/StanzaExtensions.h =================================================================== --- ps/trunk/source/lobby/StanzaExtensions.h +++ ps/trunk/source/lobby/StanzaExtensions.h @@ -63,6 +63,7 @@ glooxwrapper::string m_IsLocalIP; glooxwrapper::string m_UseSTUN; glooxwrapper::string m_Password; + glooxwrapper::string m_ClientSalt; glooxwrapper::string m_Error; }; Index: ps/trunk/source/lobby/StanzaExtensions.cpp =================================================================== --- ps/trunk/source/lobby/StanzaExtensions.cpp +++ ps/trunk/source/lobby/StanzaExtensions.cpp @@ -309,6 +309,9 @@ const glooxwrapper::Tag* pw = tag->findTag_clone("connectiondata/password"); if (pw) m_Password = pw->cdata(); + const glooxwrapper::Tag* cs = tag->findTag_clone("connectiondata/clientsalt"); + if (cs) + m_ClientSalt = cs->cdata(); const glooxwrapper::Tag* e = tag->findTag_clone("connectiondata/error"); if (e) m_Error= e->cdata(); @@ -318,6 +321,7 @@ glooxwrapper::Tag::free(pip); glooxwrapper::Tag::free(s); glooxwrapper::Tag::free(pw); + glooxwrapper::Tag::free(cs); glooxwrapper::Tag::free(e); } @@ -348,6 +352,8 @@ t->addChild(glooxwrapper::Tag::allocate("useSTUN", m_UseSTUN)); if (!m_Password.empty()) t->addChild(glooxwrapper::Tag::allocate("password", m_Password)); + if (!m_ClientSalt.empty()) + t->addChild(glooxwrapper::Tag::allocate("clientsalt", m_ClientSalt)); if (!m_Error.empty()) t->addChild(glooxwrapper::Tag::allocate("error", m_Error)); return t; Index: ps/trunk/source/lobby/XmppClient.h =================================================================== --- ps/trunk/source/lobby/XmppClient.h +++ ps/trunk/source/lobby/XmppClient.h @@ -86,7 +86,7 @@ void SendIqGetProfile(const std::string& player); void SendIqGameReport(const ScriptRequest& rq, JS::HandleValue data); void SendIqRegisterGame(const ScriptRequest& rq, JS::HandleValue data); - void SendIqGetConnectionData(const std::string& jid, const std::string& password, bool localIP); + void SendIqGetConnectionData(const std::string& jid, const std::string& password, const std::string& clientSalt, bool localIP); void SendIqUnregisterGame(); void SendIqChangeStateGame(const std::string& nbp, const std::string& players); void SendIqLobbyAuth(const std::string& to, const std::string& token); Index: ps/trunk/source/lobby/XmppClient.cpp =================================================================== --- ps/trunk/source/lobby/XmppClient.cpp +++ ps/trunk/source/lobby/XmppClient.cpp @@ -365,12 +365,13 @@ /** * Request the Connection data (ip, port...) from the server. */ -void XmppClient::SendIqGetConnectionData(const std::string& jid, const std::string& password, bool localIP) +void XmppClient::SendIqGetConnectionData(const std::string& jid, const std::string& password, const std::string& clientSalt, bool localIP) { glooxwrapper::JID targetJID(jid); ConnectionData* connectionData = new ConnectionData(); connectionData->m_Password = password; + connectionData->m_ClientSalt = clientSalt; connectionData->m_IsLocalIP = localIP ? "1" : "0"; glooxwrapper::IQ iq(gloox::IQ::Get, targetJID, m_client->getID()); iq.addExtension(connectionData); @@ -974,7 +975,7 @@ m_client->send(response); return true; } - if (!g_NetServer->CheckPasswordAndIncrement(CStr(cd->m_Password.to_string()), iq.from().username())) + if (!g_NetServer->CheckPasswordAndIncrement(iq.from().username(), cd->m_Password.to_string(), cd->m_ClientSalt.to_string())) { glooxwrapper::IQ response(gloox::IQ::Result, iq.from(), iq.id()); ConnectionData* connectionData = new ConnectionData(); Index: ps/trunk/source/network/NetClient.h =================================================================== --- ps/trunk/source/network/NetClient.h +++ ps/trunk/source/network/NetClient.h @@ -103,6 +103,7 @@ /** * Set the game password. + * Must be called after SetUserName, as that is used to hash further. */ void SetGamePassword(const CStr& hashedPassword); @@ -126,6 +127,11 @@ bool SetupConnection(ENetHost* enetClient); /** + * Request connection information over the lobby. + */ + void SetupConnectionViaLobby(); + + /** * Connect to the remote networked server using lobby. * Push netstatus messages on failure. * @return true on success, false on connection failure Index: ps/trunk/source/network/NetClient.cpp =================================================================== --- ps/trunk/source/network/NetClient.cpp +++ ps/trunk/source/network/NetClient.cpp @@ -33,6 +33,7 @@ #include "ps/Compress.h" #include "ps/CStr.h" #include "ps/Game.h" +#include "ps/Hashing.h" #include "ps/Loader.h" #include "ps/Profile.h" #include "ps/Threading.h" @@ -184,7 +185,9 @@ void CNetClient::SetGamePassword(const CStr& hashedPassword) { - m_Password = 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) @@ -202,6 +205,11 @@ return ok; } +void CNetClient::SetupConnectionViaLobby() +{ + g_XmppClient->SendIqGetConnectionData(m_HostJID, m_Password, m_UserName.ToUTF8(), false); +} + void CNetClient::SetupServerData(CStr address, u16 port, bool stun) { ENSURE(!m_Session); @@ -268,7 +276,7 @@ // To work around that, send again a connection data request, but for internal IP this time. if (ip == m_ServerAddress) { - g_XmppClient->SendIqGetConnectionData(m_HostJID, m_Password, true); + g_XmppClient->SendIqGetConnectionData(m_HostJID, m_Password, m_UserName.ToUTF8(), true); // Return true anyways - we're on a success path here. return true; } Index: ps/trunk/source/network/NetServer.h =================================================================== --- ps/trunk/source/network/NetServer.h +++ ps/trunk/source/network/NetServer.h @@ -175,7 +175,7 @@ * when guessing password trying to get connection data from the host. * @return true iff password is valid */ - bool CheckPasswordAndIncrement(const CStr& password, const std::string& username); + bool CheckPasswordAndIncrement(const std::string& username, const std::string& password, const std::string& salt); /** * Check if user reached certain number of failed attempts. @@ -239,6 +239,8 @@ CNetServerWorker(bool useLobbyAuth, int autostartPlayers); ~CNetServerWorker(); + bool CheckPassword(const std::string& password, const std::string& salt) const; + void SetPassword(const CStr& hashedPassword); void SetControllerSecret(const std::string& secret); Index: ps/trunk/source/network/NetServer.cpp =================================================================== --- ps/trunk/source/network/NetServer.cpp +++ ps/trunk/source/network/NetServer.cpp @@ -31,6 +31,7 @@ #include "ps/CLogger.h" #include "ps/ConfigDB.h" #include "ps/GUID.h" +#include "ps/Hashing.h" #include "ps/Profile.h" #include "ps/Threading.h" #include "scriptinterface/ScriptContext.h" @@ -206,6 +207,12 @@ } +bool CNetServerWorker::CheckPassword(const std::string& password, const std::string& salt) const +{ + return HashCryptographically(m_Password, salt) == password; +} + + bool CNetServerWorker::SetupConnection(const u16 port) { ENSURE(m_State == SERVER_STATE_UNCONNECTED); @@ -999,7 +1006,8 @@ } // Check the password before anything else. - if (server.m_Password != message->m_Password) + // NB: m_Name must match the client's salt, @see CNetClient::SetGamePassword + if (!server.CheckPassword(message->m_Password, message->m_Name.ToUTF8())) { // Noisy logerror because players are not supposed to be able to get the IP, // so this might be someone targeting the host for some reason @@ -1674,10 +1682,10 @@ return StunClient::FindPublicIP(*m_Worker->m_Host, m_PublicIp, m_PublicPort); } -bool CNetServer::CheckPasswordAndIncrement(const CStr& password, const std::string& username) +bool CNetServer::CheckPasswordAndIncrement(const std::string& username, const std::string& password, const std::string& salt) { std::unordered_map::iterator it = m_FailedAttempts.find(username); - if (m_Password == password) + if (m_Worker->CheckPassword(password, salt)) { if (it != m_FailedAttempts.end()) it->second = 0; Index: ps/trunk/source/network/scripting/JSInterface_Network.cpp =================================================================== --- ps/trunk/source/network/scripting/JSInterface_Network.cpp +++ ps/trunk/source/network/scripting/JSInterface_Network.cpp @@ -28,8 +28,11 @@ #include "network/NetServer.h" #include "network/StunClient.h" #include "ps/CLogger.h" +#include "ps/CStr.h" #include "ps/Game.h" #include "ps/GUID.h" +#include "ps/Hashing.h" +#include "ps/Pyrogenesis.h" #include "ps/Util.h" #include "scriptinterface/FunctionWrapper.h" #include "scriptinterface/StructuredClone.h" @@ -59,36 +62,6 @@ return !!g_NetClient; } -CStr HashPassword(const CStr& password) -{ - if (password.empty()) - return password; - - ENSURE(sodium_init() >= 0); - const int DIGESTSIZE = crypto_hash_sha256_BYTES; - constexpr int ITERATIONS = 1737; - - cassert(DIGESTSIZE == 32); - - static const unsigned char salt_base[DIGESTSIZE] = { - 244, 243, 249, 244, 32, 33, 19, 35, 16, 11, 12, 13, 14, 15, 16, 17, - 18, 19, 20, 32, 33, 244, 224, 127, 129, 130, 140, 153, 88, 123, 234, 123 }; - - // initialize the salt buffer - unsigned char salt_buffer[DIGESTSIZE] = { 0 }; - crypto_hash_sha256_state state; - crypto_hash_sha256_init(&state); - crypto_hash_sha256_update(&state, salt_base, sizeof(salt_base)); - - crypto_hash_sha256_final(&state, salt_buffer); - - // PBKDF2 to create the buffer - unsigned char encrypted[DIGESTSIZE]; - pbkdf2(encrypted, (unsigned char*)password.c_str(), password.length(), salt_buffer, DIGESTSIZE, ITERATIONS); - return CStr(Hexify(encrypted, DIGESTSIZE)).UpperCase(); -} - - void StartNetworkHost(const ScriptRequest& rq, const CStrW& playerName, const u16 serverPort, bool useSTUN, const CStr& password) { ENSURE(!g_NetClient); @@ -125,18 +98,38 @@ // Generate a secret to identify the host client. std::string secret = ps_generate_guid(); - - // We will get hashed password from clients, so hash it once for server - CStr hashedPass = HashPassword(password); - g_NetServer->SetPassword(hashedPass); g_NetServer->SetControllerSecret(secret); g_Game = new CGame(true); g_NetClient = new CNetClient(g_Game); g_NetClient->SetUserName(playerName); + if (hasLobby) - g_NetClient->SetHostJID(g_XmppClient->GetJID()); - g_NetClient->SetGamePassword(hashedPass); + { + CStr hostJID = 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. + */ + CStr hashedPass = HashCryptographically(password, hostJID + password + engine_version); + g_NetServer->SetPassword(hashedPass); + g_NetClient->SetHostJID(hostJID); + g_NetClient->SetGamePassword(hashedPass); + } + g_NetClient->SetupServerData("127.0.0.1", serverPort, false); g_NetClient->SetControllerSecret(secret); @@ -179,13 +172,13 @@ ENSURE(!g_NetServer); ENSURE(!g_Game); - CStr hashedPass = HashPassword(password); + CStr hashedPass = HashCryptographically(password, hostJID + password + engine_version); g_Game = new CGame(true); g_NetClient = new CNetClient(g_Game); g_NetClient->SetUserName(playerName); g_NetClient->SetHostJID(hostJID); g_NetClient->SetGamePassword(hashedPass); - g_XmppClient->SendIqGetConnectionData(hostJID, hashedPass.c_str(), false); + g_NetClient->SetupConnectionViaLobby(); } void DisconnectNetworkGame() Index: ps/trunk/source/ps/Hashing.h =================================================================== --- ps/trunk/source/ps/Hashing.h +++ ps/trunk/source/ps/Hashing.h @@ -0,0 +1,32 @@ +/* Copyright (C) 2021 Wildfire Games. + * This file is part of 0 A.D. + * + * 0 A.D. is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * 0 A.D. is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with 0 A.D. If not, see . + */ + +#ifndef INCLUDED_HASHING +#define INCLUDED_HASHING + +class CStr8; + +/** + * Hash a string in a cryptographically secure manner. + * This method is intended to be 'somewhat fast' for password hashing, + * and should neither be used where a fast real-time hash is wanted, + * nor for more sensitive passwords. + * @return a hex-encoded string. + */ +CStr8 HashCryptographically(const CStr8& password, const CStr8& salt); + +#endif Index: ps/trunk/source/ps/Hashing.cpp =================================================================== --- ps/trunk/source/ps/Hashing.cpp +++ ps/trunk/source/ps/Hashing.cpp @@ -0,0 +1,59 @@ +/* Copyright (C) 2021 Wildfire Games. + * This file is part of 0 A.D. + * + * 0 A.D. is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * 0 A.D. is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with 0 A.D. If not, see . + */ +#include "precompiled.h" + +#include "ps/CStr.h" +#include "ps/Util.h" + +#include + +CStr8 HashCryptographically(const CStr8& string, const CStr8& salt) +{ + if (string.empty()) + return string; + + ENSURE(sodium_init() >= 0); + + constexpr int SALTSIZE = crypto_pwhash_SALTBYTES; + static_assert(SALTSIZE >= crypto_generichash_BYTES_MIN); + static_assert(SALTSIZE <= crypto_generichash_BYTES_MAX); + static_assert(SALTSIZE >= crypto_generichash_KEYBYTES_MIN); + static_assert(SALTSIZE <= crypto_generichash_KEYBYTES_MAX); + + // First generate a fixed-size salt from out variable-sized one (libsodium requires it). + unsigned char salt_buffer[SALTSIZE] = { + 235, 82, 29, 20, 135, 168, 184, 97, 7, 240, 48, 109, 8, 34, 158, 32, + }; + crypto_generichash_state state; + crypto_generichash_init(&state, salt_buffer, SALTSIZE, SALTSIZE); + crypto_generichash_update(&state, reinterpret_cast(salt.c_str()), salt.size()); + crypto_generichash_final(&state, salt_buffer, SALTSIZE); + + constexpr int HASHSIZE = 32; + static_assert(HASHSIZE >= crypto_pwhash_BYTES_MIN); + static_assert(HASHSIZE <= crypto_pwhash_BYTES_MAX); + + // Now that we have a fixed-length key, use that to hash the password. + unsigned char output[HASHSIZE] = { 0 }; + // For HashCryptographically, we use 'fast' parameters, corresponding to low values. + // These parameters must not change, or hashes will change, hence why the #defined values are copied. + constexpr size_t memLimit = 8192 * 4; // 4 * crypto_pwhash_argon2id_MEMLIMIT_MIN + constexpr size_t opsLimit = 2; // crypto_pwhash_argon2id_OPSLIMIT_INTERACTIVE + ENSURE(crypto_pwhash(output, HASHSIZE, string.c_str(), string.size(), salt_buffer, opsLimit, memLimit, crypto_pwhash_ALG_ARGON2ID13) == 0); + + return CStr(Hexify(output, HASHSIZE)).UpperCase(); +} Index: ps/trunk/source/ps/tests/test_Hashing.h =================================================================== --- ps/trunk/source/ps/tests/test_Hashing.h +++ ps/trunk/source/ps/tests/test_Hashing.h @@ -0,0 +1,57 @@ +/* Copyright (C) 2021 Wildfire Games. + * This file is part of 0 A.D. + * + * 0 A.D. is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * 0 A.D. is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with 0 A.D. If not, see . + */ + +#include "lib/self_test.h" + +#include "ps/Hashing.h" + +class TestHashing : public CxxTest::TestSuite +{ +public: + void test_hash_cryptographically() + { + // Simple test: these should be deterministic and no collision on these trivial cases + TS_ASSERT_EQUALS(HashCryptographically("", ""), ""); + TS_ASSERT_EQUALS(HashCryptographically("", "foo"), ""); + + TS_ASSERT_EQUALS(HashCryptographically("pass", ""), "CFD946EEBCC23A1642BD846FF54B3659765D305D352D6C590DCCE0728BAAE360"); + TS_ASSERT_EQUALS(HashCryptographically("pass", "foo"), "7E18AD648A3BE29EC513551D54AFD2505F9726EE14750BCC92F979D41A3328D3"); + TS_ASSERT_EQUALS(HashCryptographically("pass", "foofoo"), "5CCF3FCD4A285A133F19461ADE1819C838E16D9C1BED3B18276B5F5EBF57E172"); + TS_ASSERT_EQUALS(HashCryptographically("pass", "bar"), "5195DE5588213B1A07BCA48BB43050EE2C1FD99DC37E243B313D3E12A6AAFFD8"); + TS_ASSERT_EQUALS(HashCryptographically("pass", "foobar"), "E63C16BBE04E806DC54032A0D4BAABB96A99E0DA695357035E23C83A4E20E718"); + TS_ASSERT_EQUALS(HashCryptographically("pass", ""), "CFD946EEBCC23A1642BD846FF54B3659765D305D352D6C590DCCE0728BAAE360"); + + TS_ASSERT_EQUALS(HashCryptographically("passpass", ""), "68FCE509D0B68EC7142D28165E6D697E26FEB929FCC1FE70ED4D0A8C716F7E56"); + TS_ASSERT_EQUALS(HashCryptographically("passpass", "foo"), "B766D8DB7AD9D110ED6059BAC6E3667486609AE193FF62ADB4EE174AC665F6F8"); + TS_ASSERT_EQUALS(HashCryptographically("passpass", "foofoo"), "0BDD6EE3B37FB7B6B4AA24F24AD148CED9BC26793B5EDBF68598800F5F53FD77"); + TS_ASSERT_EQUALS(HashCryptographically("passpass", "bar"), "BB719CDF8E5E0505AFEEABC487BE4A2A2EE83683DEC6BFD5A08C2E6C308A51C2"); + TS_ASSERT_EQUALS(HashCryptographically("passpass", "foobar"), "6745DD30BAD7B7A78BC0DC559C684CD4A5E13AD538CDE23D75577B61943D3DC1"); + + // Test that hashing hashes works. + TS_ASSERT_EQUALS(HashCryptographically("A989A9C5BDB02DD91C038661424BE039E2AE727483A30D3F13F995D0AB6C3712", "foobar"), "9509646C4675EED47E9D49AF20456F3F08605E87CA825DD44A846F5E3E3AC02F"); + TS_ASSERT_EQUALS(HashCryptographically("D9895FDEE287DBEE19907B7329207F388B1708AC4A123CA537603E953885B20F", "foobar"), "8CE4D45113D5A682FE4B6F185C1880F83EEA6CB2F007E815DCA5BF4B8178ECD0"); + } + + void test_hash_perf_DISABLED() + { + double t = timer_Time(); + for (size_t i = 0; i < 100; ++i) + HashCryptographically("somePasswordValue", reinterpret_cast(&i)); + double total = timer_Time() - t; + printf("Time: %lfs\n", total); + } +};