Merge bitcoin/bitcoin#33039: refactor,test: follow-ups to multi-byte block obfuscation

86e3a0a8cb refactor: standardize obfuscation memory alignment (Lőrinc)
13f00345c0 refactor: write `Obfuscation` object when new key is generated in dbwrapper (Lőrinc)
e5b1b7c557 refactor: rename `OBFUSCATION_KEY_KEY` (Lőrinc)
298bf95105 refactor: simplify `Obfuscation::HexKey` (Lőrinc)
2dea045425 test: make `obfuscation_serialize` more thorough (Lőrinc)
a17d8202c3 test: merge xor_roundtrip_random_chunks and xor_bytes_reference (Lőrinc)

Pull request description:

  Follow up for https://github.com/bitcoin/bitcoin/pull/31144
  Applied the remaining comments in separate commits - except for the last one where I could group them.
  Please see the commit messages for more context.

ACKs for top commit:
  achow101:
    ACK 86e3a0a8cb
  ryanofsky:
    Code review ACK 86e3a0a8cb, just tweaking key write assert as suggested
  hodlinator:
    ACK 86e3a0a8cb

Tree-SHA512: 967510a141fbb57bf9d088d92b554cf2fffc2f6aa0eab756cbae3230f53e9b04ceebcc6fea5f3383c01ad41985ecde5b5686c64a771ca9deae3497b9b88c1c8b

src/dbwrapper.cpp

@@ -249,11 +249,12 @@ CDBWrapper::CDBWrapper(const DBParams& params)
         LogInfo("Finished database compaction of %s", fs::PathToString(params.path));
     }
 
-    assert(!m_obfuscation); // Needed for unobfuscated Read()/Write() below
+    if (!Read(OBFUSCATION_KEY, m_obfuscation) && params.obfuscate && IsEmpty()) {
-    if (!Read(OBFUSCATION_KEY_KEY, m_obfuscation) && params.obfuscate && IsEmpty()) {
+        // Generate and write the new obfuscation key.
-        // Generate, write and read back the new obfuscation key, making sure we don't obfuscate the key itself
+        const Obfuscation obfuscation{FastRandomContext{}.randbytes<Obfuscation::KEY_SIZE>()};
-        Write(OBFUSCATION_KEY_KEY, FastRandomContext{}.randbytes(Obfuscation::KEY_SIZE));
+        assert(!m_obfuscation); // Make sure the key is written without obfuscation.
-        Read(OBFUSCATION_KEY_KEY, m_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());

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;

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.
+// Check optimized obfuscation with random offsets and sizes to ensure proper
-BOOST_AUTO_TEST_CASE(xor_roundtrip_random_chunks)
+// handling of key wrapping. Also verify it roundtrips.
+BOOST_AUTO_TEST_CASE(xor_random_chunks)
 {
     auto apply_random_xor_chunks{[&](std::span<std::byte> 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<Obfuscation::KEY_SIZE>() : std::array<std::byte, Obfuscation::KEY_SIZE>{}};
         const Obfuscation obfuscation{key_bytes};
         apply_random_xor_chunks(roundtrip, obfuscation);
-
+        BOOST_CHECK_EQUAL(roundtrip.size(), original.size());
-        const bool key_all_zeros{std::ranges::all_of(
+        for (size_t i{0}; i < original.size(); ++i) {
-            std::span{key_bytes}.first(std::min(write_size, Obfuscation::KEY_SIZE)), [](auto b) { return b == std::byte{0}; })};
+            BOOST_CHECK_EQUAL(roundtrip[i], original[i] ^ key_bytes[i % Obfuscation::KEY_SIZE]);
-        BOOST_CHECK(key_all_zeros ? original == roundtrip : original != roundtrip);
+        }
 
         apply_random_xor_chunks(roundtrip, obfuscation);
-        BOOST_CHECK(original == roundtrip);
+        BOOST_CHECK_EQUAL_COLLECTIONS(roundtrip.begin(), roundtrip.end(), original.begin(), original.end());
-    }
+  }
-}
-
-// 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<std::byte> target, std::span<const std::byte, Obfuscation::KEY_SIZE> 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<Obfuscation::KEY_SIZE>() : std::array<std::byte, Obfuscation::KEY_SIZE>{}};
-        const Obfuscation obfuscation{key_bytes};
-        std::vector expected{m_rng.randbytes<std::byte>(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_AUTO_TEST_CASE(obfuscation_hexkey)
@@ -84,19 +58,24 @@ BOOST_AUTO_TEST_CASE(obfuscation_hexkey)
 
 BOOST_AUTO_TEST_CASE(obfuscation_serialize)
 {
-    const Obfuscation original{m_rng.randbytes<Obfuscation::KEY_SIZE>()};
+    Obfuscation obfuscation{};
+    BOOST_CHECK(!obfuscation);
 
-    // Serialization
+    // Test loading a key.
-    DataStream ds;
+    std::vector key_in{m_rng.randbytes<std::byte>(Obfuscation::KEY_SIZE)};
-    ds << original;
+    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<std::byte> key_out;
+    DataStream ds_out;
+    ds_out << obfuscation;
+    ds_out >> key_out;
 
-    // Deserialization
+    // Make sure saved key is the same.
-    Obfuscation recovered{};
+    BOOST_CHECK_EQUAL_COLLECTIONS(key_in.begin(), key_in.end(), key_out.begin(), key_out.end());
-    ds >> recovered;
-
-    BOOST_CHECK_EQUAL(recovered.HexKey(), original.HexKey());
 }
 
 BOOST_AUTO_TEST_CASE(obfuscation_empty)

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
+            // Obfuscate until KEY_SIZE alignment boundary
-            if (const auto misalign{std::bit_cast<uintptr_t>(target.data()) % KEY_SIZE}) {
+            if (const auto misalign{reinterpret_cast<uintptr_t>(target.data()) % KEY_SIZE}) {
-                const size_t alignment{std::min(KEY_SIZE - misalign, target.size())};
+                const size_t alignment{KEY_SIZE - misalign};
                 XorWord(target.first(alignment), rot_key);
 
                 target = {std::assume_aligned<KEY_SIZE>(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<KEY_SIZE>(), rot_key);
             }
@@ -78,7 +78,7 @@ public:
 
     std::string HexKey() const
     {
-        return HexStr(std::bit_cast<std::array<uint8_t, KEY_SIZE>>(m_rotations[0]));
+        return HexStr(std::as_bytes(std::span{&m_rotations[0], 1}));
     }
 
 private: