From e8479cd6e5276effcd802ee1bea9067e533196a3 Mon Sep 17 00:00:00 2001 From: Aaron Kimbrell Date: Tue, 10 Nov 2026 12:54:33 -0600 Subject: [PATCH] fix: generate session keys from the system entropy source The key came from the shared game random engine, which is seeded from the start time. It now comes from std::random_device through a small helper. The key stays a 32 bit number, so the packets do not change. The master and world servers no longer write the key or its hash to the log. Co-Authored-By: Claude Fable 5.2 --- dCommon/GeneralUtils.h | 17 +++++++++++ dMasterServer/MasterServer.cpp | 2 +- dNet/AuthPackets.cpp | 2 +- dWorldServer/WorldServer.cpp | 2 +- tests/dCommonTests/CMakeLists.txt | 1 + tests/dCommonTests/TestGeneralUtils.cpp | 35 ++++++++++++++++++++++ tests/dCommonTests/dCommonDependencies.cpp | 3 ++ 7 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 tests/dCommonTests/TestGeneralUtils.cpp diff --git a/dCommon/GeneralUtils.h b/dCommon/GeneralUtils.h index 3c2a6c950..3c345241e 100644 --- a/dCommon/GeneralUtils.h +++ b/dCommon/GeneralUtils.h @@ -372,6 +372,23 @@ namespace GeneralUtils { return GenerateRandomNumber(std::numeric_limits::min(), std::numeric_limits::max()); } + /** + * Generates a number from the operating system's entropy source instead of the shared game engine. + * Use this for values that must not be predictable from earlier ones, such as session keys. + */ + template + inline T GenerateUnpredictableNumber() { + static_assert(std::is_integral_v && std::is_unsigned_v, "Not an unsigned integral type"); + + std::random_device device; + T value = 0; + for (uint32_t bytesFilled = 0; bytesFilled < sizeof(T); bytesFilled += sizeof(uint32_t)) { + // Two half shifts, a full 32 bit shift is not valid on a 32 bit type + value = static_cast(static_cast(value << 16) << 16) | static_cast(device()); + } + return value; + } + // https://www.quora.com/How-do-you-round-to-specific-increments-like-0-5-in-C // Rounds to the nearest floating point value specified. template , int> = 0> diff --git a/dMasterServer/MasterServer.cpp b/dMasterServer/MasterServer.cpp index ab1abb679..b797f6f5c 100644 --- a/dMasterServer/MasterServer.cpp +++ b/dMasterServer/MasterServer.cpp @@ -847,7 +847,7 @@ namespace { } activeSessions.insert(std::make_pair(sessionKey, username.string)); - LOG("Got sessionKey %i for user %s", sessionKey, username.string.c_str()); + LOG("Got a session key for user %s", username.string.c_str()); } void OnRequestSessionKey(const RequestSessionKey& request, const SystemAddress& sysAddr) { diff --git a/dNet/AuthPackets.cpp b/dNet/AuthPackets.cpp index 199fe9014..1549f1884 100644 --- a/dNet/AuthPackets.cpp +++ b/dNet/AuthPackets.cpp @@ -308,7 +308,7 @@ void AuthPackets::SendLoginResponse(dServer* server, const SystemAddress& sysAdd GeneralUtils::TryParse(Game::config->GetValue("version_minor")).value_or(ClientVersion::minor); // The user key - uint32_t sessionKey = GeneralUtils::GenerateRandomNumber(); + uint32_t sessionKey = GeneralUtils::GenerateUnpredictableNumber(); std::string userHash = std::to_string(sessionKey); userHash = md5(userHash); loginResponse.userKey = LUWString(userHash); diff --git a/dWorldServer/WorldServer.cpp b/dWorldServer/WorldServer.cpp index cd8111967..9d777336f 100644 --- a/dWorldServer/WorldServer.cpp +++ b/dWorldServer/WorldServer.cpp @@ -882,7 +882,7 @@ namespace { //Verify it: if (userHash != it->second.hash) { - LOG("SOMEONE IS TRYING TO HACK? SESSION KEY MISMATCH: ours: %s != master: %s", userHash.c_str(), it->second.hash.c_str()); + LOG("Session key mismatch for user %s", username.GetAsString().c_str()); Game::server->Disconnect(it->second.sysAddr, eServerDisconnectIdentifiers::INVALID_SESSION_KEY); return; } else { diff --git a/tests/dCommonTests/CMakeLists.txt b/tests/dCommonTests/CMakeLists.txt index 25cb2adf5..061904fbd 100644 --- a/tests/dCommonTests/CMakeLists.txt +++ b/tests/dCommonTests/CMakeLists.txt @@ -10,6 +10,7 @@ set(DCOMMONTEST_SOURCES "TestLDFFormat.cpp" "TestNiPoint3.cpp" "TestEncoding.cpp" + "TestGeneralUtils.cpp" "TestLUString.cpp" "TestLUWString.cpp" "TestBitStreamUtils.cpp" diff --git a/tests/dCommonTests/TestGeneralUtils.cpp b/tests/dCommonTests/TestGeneralUtils.cpp new file mode 100644 index 000000000..682c77fa8 --- /dev/null +++ b/tests/dCommonTests/TestGeneralUtils.cpp @@ -0,0 +1,35 @@ +#include + +#include +#include + +#include "Game.h" +#include "GeneralUtils.h" + +TEST(GeneralUtilsTest, UnpredictableNumberLeavesTheGameEngineAlone) { + Game::randomEngine = std::mt19937(1234); + std::mt19937 expected(1234); + + GeneralUtils::GenerateUnpredictableNumber(); + GeneralUtils::GenerateUnpredictableNumber(); + + EXPECT_EQ(Game::randomEngine(), expected()); +} + +TEST(GeneralUtilsTest, UnpredictableNumberDoesNotFollowTheGameEngineSeed) { + std::set seen; + for (uint32_t i = 0; i < 16; i++) { + // The same seed every time would give the same number every time from the game engine + Game::randomEngine = std::mt19937(1234); + seen.insert(GeneralUtils::GenerateUnpredictableNumber()); + } + + EXPECT_EQ(seen.size(), 16); +} + +TEST(GeneralUtilsTest, UnpredictableNumberFillsEveryByte) { + uint64_t seenBits = 0; + for (uint32_t i = 0; i < 64; i++) seenBits |= GeneralUtils::GenerateUnpredictableNumber(); + + EXPECT_EQ(seenBits, 0xFFFFFFFFFFFFFFFFULL); +} diff --git a/tests/dCommonTests/dCommonDependencies.cpp b/tests/dCommonTests/dCommonDependencies.cpp index 9081dab7f..b6800c58f 100644 --- a/tests/dCommonTests/dCommonDependencies.cpp +++ b/tests/dCommonTests/dCommonDependencies.cpp @@ -1,9 +1,12 @@ #include "Game.h" +#include + class Logger; class dConfig; namespace Game { Logger* logger; dConfig* config = nullptr; // Permissions.cpp reads the permission levels from it + std::mt19937 randomEngine; } // namespace Game