From a17d8202c36abf8a17fb8736e05f318422a3c7fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Tue, 22 Jul 2025 09:39:21 -0700 Subject: [PATCH 1/6] test: merge xor_roundtrip_random_chunks and xor_bytes_reference Instead of a separate roundtrip test and a simplified xor reference test, we can merge the two and provide the same coverage See: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211205949 Co-authored-by: Ryan Ofsky --- src/test/streams_tests.cpp | 44 ++++++++------------------------------ 1 file changed, 9 insertions(+), 35 deletions(-) diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index ce496df5a9f..44aaf4d1775 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -18,8 +18,9 @@ using namespace util::hex_literals; BOOST_FIXTURE_TEST_SUITE(streams_tests, BasicTestingSetup) -// Test that obfuscation can be properly reverted even with random chunk sizes. -BOOST_AUTO_TEST_CASE(xor_roundtrip_random_chunks) +// Check optimized obfuscation with random offsets and sizes to ensure proper +// handling of key wrapping. Also verify it roundtrips. +BOOST_AUTO_TEST_CASE(xor_random_chunks) { auto apply_random_xor_chunks{[&](std::span target, const Obfuscation& obfuscation) { for (size_t offset{0}; offset < target.size();) { @@ -37,41 +38,14 @@ BOOST_AUTO_TEST_CASE(xor_roundtrip_random_chunks) const auto key_bytes{m_rng.randbool() ? m_rng.randbytes() : std::array{}}; const Obfuscation obfuscation{key_bytes}; apply_random_xor_chunks(roundtrip, obfuscation); - - const bool key_all_zeros{std::ranges::all_of( - std::span{key_bytes}.first(std::min(write_size, Obfuscation::KEY_SIZE)), [](auto b) { return b == std::byte{0}; })}; - BOOST_CHECK(key_all_zeros ? original == roundtrip : original != roundtrip); + BOOST_CHECK_EQUAL(roundtrip.size(), original.size()); + for (size_t i{0}; i < original.size(); ++i) { + BOOST_CHECK_EQUAL(roundtrip[i], original[i] ^ key_bytes[i % Obfuscation::KEY_SIZE]); + } apply_random_xor_chunks(roundtrip, obfuscation); - BOOST_CHECK(original == roundtrip); - } -} - -// Compares optimized obfuscation against a trivial, byte-by-byte reference implementation -// with random offsets to ensure proper handling of key wrapping. -BOOST_AUTO_TEST_CASE(xor_bytes_reference) -{ - auto expected_xor{[](std::span target, std::span obfuscation, size_t key_offset) { - for (auto& b : target) { - b ^= obfuscation[key_offset++ % obfuscation.size()]; - } - }}; - - for (size_t test{0}; test < 100; ++test) { - const size_t write_size{1 + m_rng.randrange(100U)}; - const size_t key_offset{m_rng.randrange(3 * Obfuscation::KEY_SIZE)}; // Make sure the key can wrap around - const size_t write_offset{std::min(write_size, m_rng.randrange(Obfuscation::KEY_SIZE * 2))}; // Write unaligned data - - const auto key_bytes{m_rng.randbool() ? m_rng.randbytes() : std::array{}}; - const Obfuscation obfuscation{key_bytes}; - std::vector expected{m_rng.randbytes(write_size)}; - std::vector actual{expected}; - - expected_xor(std::span{expected}.subspan(write_offset), key_bytes, key_offset); - obfuscation(std::span{actual}.subspan(write_offset), key_offset); - - BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), actual.begin(), actual.end()); - } + BOOST_CHECK_EQUAL_COLLECTIONS(roundtrip.begin(), roundtrip.end(), original.begin(), original.end()); + } } BOOST_AUTO_TEST_CASE(obfuscation_hexkey) From 2dea0454254180d79464dc6afd3312b1caf369a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Tue, 22 Jul 2025 10:04:30 -0700 Subject: [PATCH 2/6] test: make `obfuscation_serialize` more thorough See: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216849672 Co-authored-by: Ryan Ofsky --- src/test/streams_tests.cpp | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 44aaf4d1775..43d06a7d0bf 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -58,19 +58,24 @@ BOOST_AUTO_TEST_CASE(obfuscation_hexkey) BOOST_AUTO_TEST_CASE(obfuscation_serialize) { - const Obfuscation original{m_rng.randbytes()}; + Obfuscation obfuscation{}; + BOOST_CHECK(!obfuscation); - // Serialization - DataStream ds; - ds << original; + // Test loading a key. + std::vector key_in{m_rng.randbytes(Obfuscation::KEY_SIZE)}; + DataStream ds_in; + ds_in << key_in; + BOOST_CHECK_EQUAL(ds_in.size(), 1 + Obfuscation::KEY_SIZE); // serialized as a vector + ds_in >> obfuscation; - BOOST_CHECK_EQUAL(ds.size(), 1 + Obfuscation::KEY_SIZE); // serialized as a vector + // Test saving the key. + std::vector key_out; + DataStream ds_out; + ds_out << obfuscation; + ds_out >> key_out; - // Deserialization - Obfuscation recovered{}; - ds >> recovered; - - BOOST_CHECK_EQUAL(recovered.HexKey(), original.HexKey()); + // Make sure saved key is the same. + BOOST_CHECK_EQUAL_COLLECTIONS(key_in.begin(), key_in.end(), key_out.begin(), key_out.end()); } BOOST_AUTO_TEST_CASE(obfuscation_empty) From 298bf9510578263a1439513729e5ff955a453437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Tue, 22 Jul 2025 10:19:25 -0700 Subject: [PATCH 3/6] refactor: simplify `Obfuscation::HexKey` See: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2215746554 Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com> --- src/util/obfuscation.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/obfuscation.h b/src/util/obfuscation.h index db7527064b4..a9de5f2f038 100644 --- a/src/util/obfuscation.h +++ b/src/util/obfuscation.h @@ -78,7 +78,7 @@ public: std::string HexKey() const { - return HexStr(std::bit_cast>(m_rotations[0])); + return HexStr(std::as_bytes(std::span{&m_rotations[0], 1})); } private: From e5b1b7c5577ee36b5bcfb6c02b92da88455411e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Tue, 22 Jul 2025 10:19:36 -0700 Subject: [PATCH 4/6] refactor: rename `OBFUSCATION_KEY_KEY` See: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216425882 Co-authored-by: Ryan Ofsky --- src/dbwrapper.cpp | 6 +++--- src/dbwrapper.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index b13699572c4..f43e07bc6bc 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -250,10 +250,10 @@ CDBWrapper::CDBWrapper(const DBParams& params) } assert(!m_obfuscation); // Needed for unobfuscated Read()/Write() below - if (!Read(OBFUSCATION_KEY_KEY, m_obfuscation) && params.obfuscate && IsEmpty()) { + if (!Read(OBFUSCATION_KEY, m_obfuscation) && params.obfuscate && IsEmpty()) { // Generate, write and read back the new obfuscation key, making sure we don't obfuscate the key itself - Write(OBFUSCATION_KEY_KEY, FastRandomContext{}.randbytes(Obfuscation::KEY_SIZE)); - Read(OBFUSCATION_KEY_KEY, m_obfuscation); + Write(OBFUSCATION_KEY, FastRandomContext{}.randbytes(Obfuscation::KEY_SIZE)); + Read(OBFUSCATION_KEY, m_obfuscation); LogInfo("Wrote new obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey()); } LogInfo("Using obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey()); diff --git a/src/dbwrapper.h b/src/dbwrapper.h index c5d49404861..b9b98bd96ad 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -189,7 +189,7 @@ private: Obfuscation m_obfuscation; //! obfuscation key storage key, null-prefixed to avoid collisions - inline static const std::string OBFUSCATION_KEY_KEY{"\000obfuscate_key", 14}; // explicit size to avoid truncation at leading \0 + inline static const std::string OBFUSCATION_KEY{"\000obfuscate_key", 14}; // explicit size to avoid truncation at leading \0 //! path to filesystem storage const fs::path m_path; From 13f00345c061a8df2fe41ff9d0a6aadfb6137fd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Tue, 22 Jul 2025 10:13:16 -0700 Subject: [PATCH 5/6] refactor: write `Obfuscation` object when new key is generated in dbwrapper See: * https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2215720251 * https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2223539466 Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com> Co-authored-by: Ryan Ofsky --- src/dbwrapper.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index f43e07bc6bc..8939ff84f8f 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -249,11 +249,12 @@ CDBWrapper::CDBWrapper(const DBParams& params) LogPrintf("Finished database compaction of %s\n", fs::PathToString(params.path)); } - assert(!m_obfuscation); // Needed for unobfuscated Read()/Write() below if (!Read(OBFUSCATION_KEY, m_obfuscation) && params.obfuscate && IsEmpty()) { - // Generate, write and read back the new obfuscation key, making sure we don't obfuscate the key itself - Write(OBFUSCATION_KEY, FastRandomContext{}.randbytes(Obfuscation::KEY_SIZE)); - Read(OBFUSCATION_KEY, m_obfuscation); + // Generate and write the new obfuscation key. + const Obfuscation obfuscation{FastRandomContext{}.randbytes()}; + assert(!m_obfuscation); // Make sure the key is written without obfuscation. + Write(OBFUSCATION_KEY, obfuscation); + m_obfuscation = obfuscation; LogInfo("Wrote new obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey()); } LogInfo("Using obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey()); From 86e3a0a8cbd30cfee98f5b4acf4ce6d0a75a3ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Tue, 22 Jul 2025 10:15:46 -0700 Subject: [PATCH 6/6] refactor: standardize obfuscation memory alignment See: * https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216962117 * https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2220277161 * https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2210851772 Co-authored-by: Russell Yanofsky --- src/util/obfuscation.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/util/obfuscation.h b/src/util/obfuscation.h index a9de5f2f038..e9a2e6093b6 100644 --- a/src/util/obfuscation.h +++ b/src/util/obfuscation.h @@ -36,21 +36,21 @@ public: KeyType rot_key{m_rotations[key_offset % KEY_SIZE]}; // Continue obfuscation from where we left off if (target.size() > KEY_SIZE) { - // Obfuscate until 64-bit alignment boundary - if (const auto misalign{std::bit_cast(target.data()) % KEY_SIZE}) { - const size_t alignment{std::min(KEY_SIZE - misalign, target.size())}; + // Obfuscate until KEY_SIZE alignment boundary + if (const auto misalign{reinterpret_cast(target.data()) % KEY_SIZE}) { + const size_t alignment{KEY_SIZE - misalign}; XorWord(target.first(alignment), rot_key); target = {std::assume_aligned(target.data() + alignment), target.size() - alignment}; rot_key = m_rotations[(key_offset + alignment) % KEY_SIZE]; } - // Aligned obfuscation in 64-byte chunks + // Aligned obfuscation in 8*KEY_SIZE chunks for (constexpr auto unroll{8}; target.size() >= KEY_SIZE * unroll; target = target.subspan(KEY_SIZE * unroll)) { for (size_t i{0}; i < unroll; ++i) { XorWord(target.subspan(i * KEY_SIZE, KEY_SIZE), rot_key); } } - // Aligned obfuscation in 64-bit chunks + // Aligned obfuscation in KEY_SIZE chunks for (; target.size() >= KEY_SIZE; target = target.subspan(KEY_SIZE)) { XorWord(target.first(), rot_key); }