From 1d79991ec4b4fa62df619e9352cd2f4f4c68156d Mon Sep 17 00:00:00 2001 From: Niel Lebeck Date: Sat, 27 Apr 2024 20:07:17 -0700 Subject: [PATCH] Split `SettingsHandler` into separate reader and writer classes --- Source/Core/Common/SettingsHandler.cpp | 54 ++++++------ Source/Core/Common/SettingsHandler.h | 33 ++++---- Source/Core/Core/Boot/Boot_BS2Emu.cpp | 29 +++---- Source/Core/Core/IOS/DolphinDevice.cpp | 6 +- .../Core/Core/IOS/Network/KD/NetKDRequest.cpp | 14 ++-- .../UnitTests/Common/SettingsHandlerTest.cpp | 83 +++++++++---------- 6 files changed, 107 insertions(+), 112 deletions(-) diff --git a/Source/Core/Common/SettingsHandler.cpp b/Source/Core/Common/SettingsHandler.cpp index b5c32a2099..6cc9f5a8fe 100644 --- a/Source/Core/Common/SettingsHandler.cpp +++ b/Source/Core/Common/SettingsHandler.cpp @@ -17,72 +17,72 @@ namespace Common { -SettingsHandler::SettingsHandler() : m_buffer{}, m_position{0}, m_key{INITIAL_SEED}, decoded{""} +namespace +{ +// Key used to encrypt/decrypt setting.txt contents +constexpr u32 INITIAL_SEED = 0x73B5DBFA; +} // namespace + +SettingsWriter::SettingsWriter() : m_buffer{}, m_position{0}, m_key{INITIAL_SEED} { } -SettingsHandler::SettingsHandler(const Buffer& buffer) : SettingsHandler() -{ - m_buffer = buffer; - Decrypt(); -} - -const SettingsHandler::Buffer& SettingsHandler::GetBytes() const +const SettingsBuffer& SettingsWriter::GetBytes() const { return m_buffer; } -std::string SettingsHandler::GetValue(std::string_view key) const +std::string SettingsReader::GetValue(std::string_view key) const { constexpr char delim[] = "\n"; std::string toFind = std::string(delim).append(key).append("="); - size_t found = decoded.find(toFind); + size_t found = m_decoded.find(toFind); if (found != std::string_view::npos) { - size_t delimFound = decoded.find(delim, found + toFind.length()); + size_t delimFound = m_decoded.find(delim, found + toFind.length()); if (delimFound == std::string_view::npos) - delimFound = decoded.length() - 1; - return decoded.substr(found + toFind.length(), delimFound - (found + toFind.length())); + delimFound = m_decoded.length() - 1; + return m_decoded.substr(found + toFind.length(), delimFound - (found + toFind.length())); } else { toFind = std::string(key).append("="); - found = decoded.find(toFind); + found = m_decoded.find(toFind); if (found == 0) { - size_t delimFound = decoded.find(delim, found + toFind.length()); + size_t delimFound = m_decoded.find(delim, found + toFind.length()); if (delimFound == std::string_view::npos) - delimFound = decoded.length() - 1; - return decoded.substr(found + toFind.length(), delimFound - (found + toFind.length())); + delimFound = m_decoded.length() - 1; + return m_decoded.substr(found + toFind.length(), delimFound - (found + toFind.length())); } } return ""; } -void SettingsHandler::Decrypt() +SettingsReader::SettingsReader(const SettingsBuffer& buffer) : m_decoded{""} { - while (m_position < m_buffer.size()) + u32 key = INITIAL_SEED; + for (u32 position = 0; position < buffer.size(); ++position) { - decoded.push_back((u8)(m_buffer[m_position] ^ m_key)); - m_position++; - m_key = (m_key >> 31) | (m_key << 1); + m_decoded.push_back((u8)(buffer[position] ^ key)); + key = (key >> 31) | (key << 1); } // The decoded data normally uses CRLF line endings, but occasionally // (see the comment in WriteLine), lines can be separated by CRLFLF. // To handle this, we remove every CR and treat LF as the line ending. // (We ignore empty lines.) - std::erase(decoded, '\x0d'); + std::erase(m_decoded, '\x0d'); } -void SettingsHandler::AddSetting(std::string_view key, std::string_view value) +void SettingsWriter::AddSetting(std::string_view key, std::string_view value) { WriteLine(fmt::format("{}={}\r\n", key, value)); } -void SettingsHandler::WriteLine(std::string_view str) +void SettingsWriter::WriteLine(std::string_view str) { const u32 old_position = m_position; const u32 old_key = m_key; @@ -106,7 +106,7 @@ void SettingsHandler::WriteLine(std::string_view str) } } -void SettingsHandler::WriteByte(u8 b) +void SettingsWriter::WriteByte(u8 b) { if (m_position >= m_buffer.size()) return; @@ -116,7 +116,7 @@ void SettingsHandler::WriteByte(u8 b) m_key = (m_key >> 31) | (m_key << 1); } -std::string SettingsHandler::GenerateSerialNumber() +std::string SettingsWriter::GenerateSerialNumber() { const std::time_t t = std::time(nullptr); diff --git a/Source/Core/Common/SettingsHandler.h b/Source/Core/Common/SettingsHandler.h index 0b8bc2044d..e16d1155ec 100644 --- a/Source/Core/Common/SettingsHandler.h +++ b/Source/Core/Common/SettingsHandler.h @@ -13,34 +13,35 @@ namespace Common { -class SettingsHandler +using SettingsBuffer = std::array; + +class SettingsWriter { public: - enum - { - SETTINGS_SIZE = 0x100, - // Key used to encrypt/decrypt setting.txt contents - INITIAL_SEED = 0x73B5DBFA - }; - - using Buffer = std::array; - SettingsHandler(); - explicit SettingsHandler(const Buffer& buffer); + SettingsWriter(); void AddSetting(std::string_view key, std::string_view value); - const Buffer& GetBytes() const; - std::string GetValue(std::string_view key) const; + const SettingsBuffer& GetBytes() const; static std::string GenerateSerialNumber(); private: - void Decrypt(); void WriteLine(std::string_view str); void WriteByte(u8 b); - std::array m_buffer; + SettingsBuffer m_buffer; u32 m_position, m_key; - std::string decoded; +}; + +class SettingsReader +{ +public: + explicit SettingsReader(const SettingsBuffer& buffer); + + std::string GetValue(std::string_view key) const; + +private: + std::string m_decoded; }; } // namespace Common diff --git a/Source/Core/Core/Boot/Boot_BS2Emu.cpp b/Source/Core/Core/Boot/Boot_BS2Emu.cpp index 1f331fbe54..baf2079677 100644 --- a/Source/Core/Core/Boot/Boot_BS2Emu.cpp +++ b/Source/Core/Core/Boot/Boot_BS2Emu.cpp @@ -371,12 +371,12 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con const auto fs = system.GetIOS()->GetFS(); { - Common::SettingsHandler::Buffer data; + Common::SettingsBuffer data; const auto file = fs->OpenFile(IOS::SYSMENU_UID, IOS::SYSMENU_GID, settings_file_path, IOS::HLE::FS::Mode::Read); if (file && file->Read(data.data(), data.size())) { - Common::SettingsHandler settings_reader(data); + const Common::SettingsReader settings_reader(data); serno = settings_reader.GetValue("SERNO"); model = settings_reader.GetValue("MODEL"); @@ -413,7 +413,7 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con if (Core::WantsDeterminism()) serno = "123456789"; else - serno = Common::SettingsHandler::GenerateSerialNumber(); + serno = Common::SettingsWriter::GenerateSerialNumber(); INFO_LOG_FMT(BOOT, "No previous serial number found, generated one instead: {}", serno); } else @@ -421,20 +421,21 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con INFO_LOG_FMT(BOOT, "Using serial number: {}", serno); } - Common::SettingsHandler gen; - gen.AddSetting("AREA", region_setting.area); - gen.AddSetting("MODEL", model); - gen.AddSetting("DVD", "0"); - gen.AddSetting("MPCH", "0x7FFE"); - gen.AddSetting("CODE", region_setting.code); - gen.AddSetting("SERNO", serno); - gen.AddSetting("VIDEO", region_setting.video); - gen.AddSetting("GAME", region_setting.game); + Common::SettingsWriter settings_writer; + settings_writer.AddSetting("AREA", region_setting.area); + settings_writer.AddSetting("MODEL", model); + settings_writer.AddSetting("DVD", "0"); + settings_writer.AddSetting("MPCH", "0x7FFE"); + settings_writer.AddSetting("CODE", region_setting.code); + settings_writer.AddSetting("SERNO", serno); + settings_writer.AddSetting("VIDEO", region_setting.video); + settings_writer.AddSetting("GAME", region_setting.game); constexpr IOS::HLE::FS::Mode rw_mode = IOS::HLE::FS::Mode::ReadWrite; const auto settings_file = fs->CreateAndOpenFile(IOS::SYSMENU_UID, IOS::SYSMENU_GID, settings_file_path, {rw_mode, rw_mode, rw_mode}); - if (!settings_file || !settings_file->Write(gen.GetBytes().data(), gen.GetBytes().size())) + if (!settings_file || + !settings_file->Write(settings_writer.GetBytes().data(), settings_writer.GetBytes().size())) { PanicAlertFmtT("SetupWiiMemory: Can't create setting.txt file"); return false; @@ -443,7 +444,7 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con auto& memory = system.GetMemory(); // Write the 256 byte setting.txt to memory. - memory.CopyToEmu(0x3800, gen.GetBytes().data(), gen.GetBytes().size()); + memory.CopyToEmu(0x3800, settings_writer.GetBytes().data(), settings_writer.GetBytes().size()); INFO_LOG_FMT(BOOT, "Setup Wii Memory..."); diff --git a/Source/Core/Core/IOS/DolphinDevice.cpp b/Source/Core/Core/IOS/DolphinDevice.cpp index 79a9b0529c..944bec41b2 100644 --- a/Source/Core/Core/IOS/DolphinDevice.cpp +++ b/Source/Core/Core/IOS/DolphinDevice.cpp @@ -133,13 +133,13 @@ IPCReply GetRealProductCode(Core::System& system, const IOCtlVRequest& request) if (!file) return IPCReply(IPC_ENOENT); - Common::SettingsHandler::Buffer data; + Common::SettingsBuffer data; if (!file.ReadBytes(data.data(), data.size())) return IPCReply(IPC_ENOENT); - Common::SettingsHandler gen(data); - const std::string code = gen.GetValue("CODE"); + const Common::SettingsReader settings_reader(data); + const std::string code = settings_reader.GetValue("CODE"); const size_t length = std::min(request.io_vectors[0].size, code.length()); if (length == 0) diff --git a/Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp b/Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp index 14553199d6..ddc99fa4c5 100644 --- a/Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp +++ b/Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp @@ -923,7 +923,7 @@ IPCReply NetKDRequestDevice::HandleRequestRegisterUserId(const IOS::HLE::IOCtlRe return IPCReply{IPC_SUCCESS}; } - Common::SettingsHandler::Buffer data; + Common::SettingsBuffer data; if (!file->Read(data.data(), data.size())) { WriteReturnValue(memory, NWC24::WC24_ERR_FILE_READ, request.buffer_out); @@ -931,8 +931,8 @@ IPCReply NetKDRequestDevice::HandleRequestRegisterUserId(const IOS::HLE::IOCtlRe return IPCReply{IPC_SUCCESS}; } - const Common::SettingsHandler gen{data}; - const std::string serno = gen.GetValue("SERNO"); + const Common::SettingsReader settings_reader{data}; + const std::string serno = settings_reader.GetValue("SERNO"); const std::string form_data = fmt::format("mlid=w{}&hdid={}&rgncd={}", m_config.Id(), m_ios.GetIOSC().GetDeviceId(), serno); const Common::HttpRequest::Response response = m_http.Post(m_config.GetAccountURL(), form_data); @@ -1076,12 +1076,12 @@ std::optional NetKDRequestDevice::IOCtl(const IOCtlRequest& request) const auto fs = m_ios.GetFS(); if (const auto file = fs->OpenFile(PID_KD, PID_KD, settings_file_path, FS::Mode::Read)) { - Common::SettingsHandler::Buffer data; + Common::SettingsBuffer data; if (file->Read(data.data(), data.size())) { - const Common::SettingsHandler gen{data}; - area = gen.GetValue("AREA"); - model = gen.GetValue("MODEL"); + const Common::SettingsReader settings_reader{data}; + area = settings_reader.GetValue("AREA"); + model = settings_reader.GetValue("MODEL"); } } diff --git a/Source/UnitTests/Common/SettingsHandlerTest.cpp b/Source/UnitTests/Common/SettingsHandlerTest.cpp index 360a2221a5..20b6a59b85 100644 --- a/Source/UnitTests/Common/SettingsHandlerTest.cpp +++ b/Source/UnitTests/Common/SettingsHandlerTest.cpp @@ -9,15 +9,15 @@ namespace { // The encrypted bytes corresponding to the following settings, in order: // "key" = "val" -Common::SettingsHandler::Buffer BUFFER_A{0x91, 0x91, 0x90, 0xEE, 0xD1, 0x2F, 0xF0, 0x34, 0x79}; +Common::SettingsBuffer BUFFER_A{0x91, 0x91, 0x90, 0xEE, 0xD1, 0x2F, 0xF0, 0x34, 0x79}; // The encrypted bytes corresponding to the following settings, in order: // "key1" = "val1" // "key2" = "val2" // "foo" = "bar" -Common::SettingsHandler::Buffer BUFFER_B{ - 0x91, 0x91, 0x90, 0xE2, 0x9A, 0x38, 0xFD, 0x55, 0x42, 0xEA, 0xC4, 0xF6, 0x5E, 0xF, 0xDF, 0xE7, - 0xC3, 0x0A, 0xBB, 0x9C, 0x50, 0xB1, 0x10, 0x82, 0xB4, 0x8A, 0x0D, 0xBE, 0xCD, 0x72, 0xF4}; +Common::SettingsBuffer BUFFER_B{0x91, 0x91, 0x90, 0xE2, 0x9A, 0x38, 0xFD, 0x55, 0x42, 0xEA, 0xC4, + 0xF6, 0x5E, 0xF, 0xDF, 0xE7, 0xC3, 0x0A, 0xBB, 0x9C, 0x50, 0xB1, + 0x10, 0x82, 0xB4, 0x8A, 0x0D, 0xBE, 0xCD, 0x72, 0xF4}; // The encrypted bytes corresponding to the following setting: // "\xFA" = "a" @@ -25,7 +25,7 @@ Common::SettingsHandler::Buffer BUFFER_B{ // This setting triggers the edge case fixed in PR #8704: the key's first and only byte matches the // first byte of the initial encryption key, resulting in a null encoded byte on the first attempt // to encode the line. -Common::SettingsHandler::Buffer BUFFER_C{0xF0, 0x0E, 0xD4, 0xB2, 0xAA, 0x44}; +Common::SettingsBuffer BUFFER_C{0xF0, 0x0E, 0xD4, 0xB2, 0xAA, 0x44}; // The encrypted bytes corresponding to the following setting: // "\xFA\xE9" = "a" @@ -38,76 +38,69 @@ Common::SettingsHandler::Buffer BUFFER_C{0xF0, 0x0E, 0xD4, 0xB2, 0xAA, 0x44}; // 2. The key's second byte matches the first byte of the encryption key after two // rotations, resulting in a null encoded byte on the second attempt to encode the line (with a // single LF inserted before the line). -Common::SettingsHandler::Buffer BUFFER_D{0xF0, 0xFE, 0x13, 0x3A, 0x9A, 0x2F, 0x91, 0x33}; +Common::SettingsBuffer BUFFER_D{0xF0, 0xFE, 0x13, 0x3A, 0x9A, 0x2F, 0x91, 0x33}; } // namespace -TEST(SettingsHandlerTest, EncryptSingleSetting) +TEST(SettingsWriterTest, EncryptSingleSetting) { - Common::SettingsHandler handler; - handler.AddSetting("key", "val"); - Common::SettingsHandler::Buffer buffer = handler.GetBytes(); + Common::SettingsWriter writer; + writer.AddSetting("key", "val"); + Common::SettingsBuffer buffer = writer.GetBytes(); EXPECT_TRUE(std::ranges::equal(buffer, BUFFER_A)); } -TEST(SettingsHandlerTest, DecryptSingleSetting) +TEST(SettingsReaderTest, DecryptSingleSetting) { - Common::SettingsHandler handler(BUFFER_A); - EXPECT_EQ(handler.GetValue("key"), "val"); + const Common::SettingsReader reader(BUFFER_A); + EXPECT_EQ(reader.GetValue("key"), "val"); } -TEST(SettingsHandlerTest, EncryptMultipleSettings) +TEST(SettingsWriterTest, EncryptMultipleSettings) { - Common::SettingsHandler handler; - handler.AddSetting("key1", "val1"); - handler.AddSetting("key2", "val2"); - handler.AddSetting("foo", "bar"); - Common::SettingsHandler::Buffer buffer = handler.GetBytes(); + Common::SettingsWriter writer; + writer.AddSetting("key1", "val1"); + writer.AddSetting("key2", "val2"); + writer.AddSetting("foo", "bar"); + Common::SettingsBuffer buffer = writer.GetBytes(); EXPECT_TRUE(std::ranges::equal(buffer, BUFFER_B)); } -TEST(SettingsHandlerTest, DecryptMultipleSettings) +TEST(SettingsReaderTest, DecryptMultipleSettings) { - Common::SettingsHandler handler(BUFFER_B); - EXPECT_EQ(handler.GetValue("key1"), "val1"); - EXPECT_EQ(handler.GetValue("key2"), "val2"); - EXPECT_EQ(handler.GetValue("foo"), "bar"); + const Common::SettingsReader reader(BUFFER_B); + EXPECT_EQ(reader.GetValue("key1"), "val1"); + EXPECT_EQ(reader.GetValue("key2"), "val2"); + EXPECT_EQ(reader.GetValue("foo"), "bar"); } -TEST(SettingsHandlerTest, GetValueOnSameInstance) +TEST(SettingsWriterTest, EncryptAddsLFOnNullChar) { - Common::SettingsHandler handler; - handler.AddSetting("key", "val"); - EXPECT_EQ(handler.GetValue("key"), ""); -} - -TEST(SettingsHandlerTest, EncryptAddsLFOnNullChar) -{ - Common::SettingsHandler handler; - handler.AddSetting("\xFA", "a"); - Common::SettingsHandler::Buffer buffer = handler.GetBytes(); + Common::SettingsWriter writer; + writer.AddSetting("\xFA", "a"); + Common::SettingsBuffer buffer = writer.GetBytes(); EXPECT_TRUE(std::ranges::equal(buffer, BUFFER_C)); } -TEST(SettingsHandlerTest, EncryptAddsLFOnNullCharTwice) +TEST(SettingsWriterTest, EncryptAddsLFOnNullCharTwice) { - Common::SettingsHandler handler; - handler.AddSetting("\xFA\xE9", "a"); - Common::SettingsHandler::Buffer buffer = handler.GetBytes(); + Common::SettingsWriter writer; + writer.AddSetting("\xFA\xE9", "a"); + Common::SettingsBuffer buffer = writer.GetBytes(); EXPECT_TRUE(std::ranges::equal(buffer, BUFFER_D)); } -TEST(SettingsHandlerTest, DecryptSingleAddedLF) +TEST(SettingsReaderTest, DecryptSingleAddedLF) { - Common::SettingsHandler handler(BUFFER_C); - EXPECT_EQ(handler.GetValue("\xFA"), "a"); + const Common::SettingsReader reader(BUFFER_C); + EXPECT_EQ(reader.GetValue("\xFA"), "a"); } -TEST(SettingsHandlerTest, DecryptTwoAddedLFs) +TEST(SettingsReaderTest, DecryptTwoAddedLFs) { - Common::SettingsHandler handler(BUFFER_D); - EXPECT_EQ(handler.GetValue("\xFA\xE9"), "a"); + const Common::SettingsReader reader(BUFFER_D); + EXPECT_EQ(reader.GetValue("\xFA\xE9"), "a"); }