From a510e8ee9593efa0b162090b9738b1b706abdba2 Mon Sep 17 00:00:00 2001 From: MHunterG <48920434+MHunterG@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:24:39 +0300 Subject: [PATCH] Let extension key look-ups find every key the keys file gives hle_extension::key() looks in CryptoKeys::named first and then in the fields PortableKit keeps its own keys in (KIRK slots, kirk.cmd1, savedata, tags). Keys PortableKit checks itself and keeps elsewhere, such as amctrl.1CD4, amctrl.1CE4, amctrl.1CF4 and amctrl.dnas.1A90 for PGD, were in neither place the look-up searched, so a module asking for them got nothing although PortableKit's own PGD code read them. The keys file reader now also keeps every key that passes its checks under its name in CryptoKeys::named, so a look-up by name finds any key the file gives, whatever structure PortableKit uses for it, including names added later. portablekit_keys_file_tests reads a keys file through the desktop app's reader, with a stand-in module that declares a name PortableKit checks (kirk.aes.5D) and one of its own, and checks that a PortableKit-checked name the module declares, one it does not (savedata.2) and kirk.cmd1 are all found by name. PortableKit's key values come from crypto_keys_builtin.cpp, compiled under other names, so the test writes none. It fails without the fix. Co-Authored-By: Claude Opus 5.5 (1M context) --- CMakeLists.txt | 23 ++++++++ apps/portablekit/src/keys_file.cpp | 3 + host/crypto_keys.hpp | 6 +- tests/builtin_keys_renamed.cpp | 6 ++ tests/keys_file_tests.cpp | 92 ++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 tests/builtin_keys_renamed.cpp create mode 100644 tests/keys_file_tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 2e4ec44..63c819d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -485,3 +485,26 @@ if(PSPRECOMP_BUILD_TESTS) set_target_properties(portablekit_io_filter_tests PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin") add_test(NAME portablekit_io_filter_tests COMMAND portablekit_io_filter_tests) endif() + +# The desktop app's keys file as extension modules look keys up in it. The +# values of PortableKit's own keys come from crypto_keys_builtin.cpp, compiled +# under other names (tests/builtin_keys_renamed.cpp) beside the app's own. +if(PSPRECOMP_BUILD_TESTS) + add_executable(portablekit_keys_file_tests + "${PORTABLEKIT_ROOT}/tests/keys_file_tests.cpp" + "${PORTABLEKIT_ROOT}/tests/builtin_keys_renamed.cpp" + "${PORTABLEKIT_ROOT}/apps/portablekit/src/keys_file.cpp" + "${PORTABLEKIT_ROOT}/apps/portablekit/src/app_home.cpp" + "${PORTABLEKIT_ROOT}/apps/portablekit/src/text_format.cpp" + "${PORTABLEKIT_ROOT}/host/app_paths.cpp" + "${PORTABLEKIT_ROOT}/host/install/user_data.cpp" + "${PORTABLEKIT_ROOT}/host/extension_keys.cpp" + "${PORTABLEKIT_ROOT}/host/profile.cpp" + "${PORTABLEKIT_ROOT}/tests/test_profile.cpp") + target_compile_features(portablekit_keys_file_tests PRIVATE cxx_std_20) + target_include_directories(portablekit_keys_file_tests PRIVATE + "${PORTABLEKIT_ROOT}/host" "${PORTABLEKIT_ROOT}/apps/portablekit/src" "${PORTABLEKIT_ROOT}/include") + target_link_libraries(portablekit_keys_file_tests PRIVATE psprecomp_core) + set_target_properties(portablekit_keys_file_tests PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin") + add_test(NAME portablekit_keys_file_tests COMMAND portablekit_keys_file_tests) +endif() diff --git a/apps/portablekit/src/keys_file.cpp b/apps/portablekit/src/keys_file.cpp index 2e62e69..66cf0f3 100644 --- a/apps/portablekit/src/keys_file.cpp +++ b/apps/portablekit/src/keys_file.cpp @@ -187,6 +187,9 @@ KeysReport read_keys_file(const std::filesystem::path &path) { report.problems.push_back(raw_name + " is not the right key: its fingerprint does not match."); continue; } + // Every key that passed is also kept by its name, so a look-up by + // name (hle_extension::key) finds it whatever structure it goes to. + report.keys.named[name] = *bytes; if (name.starts_with("kirk.aes.")) { const unsigned long slot = std::strtoul(name.substr(9).c_str(), nullptr, 16); report.keys.kirk_aes[static_cast(slot)] = to_key(*bytes); diff --git a/host/crypto_keys.hpp b/host/crypto_keys.hpp index 83d4eb8..94f9c63 100644 --- a/host/crypto_keys.hpp +++ b/host/crypto_keys.hpp @@ -30,8 +30,10 @@ struct CryptoKeys { // The AES key of KIRK command 1 (signed and encrypted blocks), which wraps // an executable's payload key. std::optional kirk_cmd1; - // Keys HLE extension modules declare (extension_keys.hpp), by their - // keys-file name in lower case, of any length. + // Every key the player's keys file gave, by its keys-file name in lower + // case and of any length: PortableKit's own (also kept in the fields + // here) and those HLE extension modules declare (extension_keys.hpp). + // hle_extension::key() looks here first. std::map> named; // Save-data keys 2 to 7, in the order they are usually published. std::map savedata; diff --git a/tests/builtin_keys_renamed.cpp b/tests/builtin_keys_renamed.cpp new file mode 100644 index 0000000..881bf82 --- /dev/null +++ b/tests/builtin_keys_renamed.cpp @@ -0,0 +1,6 @@ +// host/crypto_keys_builtin.cpp under other names, for a test that also links +// the desktop app's keys file (which defines the usual ones): the values stay +// written in exactly one place. +#define crypto_keys builtin_crypto_keys +#define keys_come_from_keys_file builtin_keys_come_from_keys_file +#include "crypto_keys_builtin.cpp" diff --git a/tests/keys_file_tests.cpp b/tests/keys_file_tests.cpp new file mode 100644 index 0000000..bd819e0 --- /dev/null +++ b/tests/keys_file_tests.cpp @@ -0,0 +1,92 @@ +// The desktop app's keys file as HLE extension modules see it: every key the +// file gives can be looked up by its name (hle_extension::key goes through +// key_value), PortableKit's own included, whether or not a module also +// declares it. The values of PortableKit's own keys come from +// host/crypto_keys_builtin.cpp, compiled here under another name, so no key +// is written in this file. + +#include "extension_keys.hpp" +#include "keys_file.hpp" + +#include +#include +#include +#include +#include + +namespace portablekit { +const CryptoKeys *builtin_crypto_keys(); // crypto_keys_builtin.cpp, renamed + +// The keys the modules of this test declare: one PortableKit checks itself, +// and one of the module's own. +const std::vector &declared_extension_keys() { + static const std::vector keys{{"kirk.aes.5d", "", 16u, "Test module"}, + {"vendor.key", "", 16u, "Test module"}}; + return keys; +} +} // namespace portablekit + +namespace { + +int g_failures = 0; + +void check(bool condition, const char *what) { + std::printf("%s %s\n", condition ? "ok " : "FAIL", what); + if (!condition) ++g_failures; +} + +std::string hex(const portablekit::Key16 &key) { + std::string text; + char digits[3]; + for (const std::uint8_t byte : key) { + std::snprintf(digits, sizeof digits, "%02X", byte); + text += digits; + } + return text; +} + +std::vector bytes(const portablekit::Key16 &key) { return {key.begin(), key.end()}; } + +} // namespace + +int main() { + const portablekit::CryptoKeys &builtin = *portablekit::builtin_crypto_keys(); + const portablekit::Key16 kirk5d = *builtin.kirk(0x5Du); + const portablekit::Key16 save2 = *builtin.savedata_key(2); + const portablekit::Key16 cmd1 = *builtin.kirk_cmd1; + + const auto path = std::filesystem::temp_directory_path() / "portablekit_keys_file_test.txt"; + { + std::ofstream out(path); + out << "# checked by PortableKit, declared by the module too\n" + << "kirk.aes.5D = " << hex(kirk5d) << "\n" + << "# checked by PortableKit, declared by nobody\n" + << "savedata.2 = " << hex(save2) << "\n" + << "KIRK.CMD1 = " << hex(cmd1) << "\n" + << "vendor.key = 000102030405060708090A0B0C0D0E0F\n" + << "unknown.name = 00\n"; + } + const portablekit::app::KeysReport report = portablekit::app::read_keys_file(path); + std::filesystem::remove(path); + + for (const std::string &problem : report.problems) std::printf(" problem: %s\n", problem.c_str()); + check(report.file_found && report.problems.empty(), "the file is read without problems"); + check(report.warnings.size() == 1u, "a name nothing reads is a warning"); + check(report.can_decrypt_executables, "PortableKit's own keys still do their work"); + + // What hle_extension::key(name) answers from (hle_extension_host.cpp). + const portablekit::CryptoKeys *keys = &report.keys; + check(portablekit::key_value(keys, "kirk.aes.5D") == bytes(kirk5d) && report.keys.named.contains("kirk.aes.5d"), + "a name PortableKit checks and a module declares is found by name"); + check(portablekit::key_value(keys, "savedata.2") == bytes(save2) && report.keys.named.contains("savedata.2"), + "a name PortableKit checks and no module declares is found by name"); + check(report.keys.named.contains("kirk.cmd1"), "every key PortableKit accepts is kept by its name"); + check(portablekit::key_value(keys, "vendor.key").value_or(std::vector{}).size() == 16u, + "a module's own key is found"); + bool listed = report.declared.size() == 2u; + for (const auto &key : report.declared) listed = listed && key.present; + check(listed, "keys status lists both declared keys as present"); + + if (g_failures != 0) std::printf("%d failed\n", g_failures); + return g_failures == 0 ? 0 : 1; +}