refactor: encapsulate vector/array keys into Obfuscation

Author: l0rinc

src/bench/obfuscation.cpp

@@ -13,11 +13,11 @@ static void ObfuscationBench(benchmark::Bench& bench)
 {
     FastRandomContext frc{/*fDeterministic=*/true};
     auto data{frc.randbytes<std::byte>(1024)};
-    const auto key{frc.randbytes<Obfuscation::KEY_SIZE>()};
+    const Obfuscation obfuscation{frc.randbytes<Obfuscation::KEY_SIZE>()};
 
     size_t offset{0};
     bench.batch(data.size()).unit("byte").run([&] {
-        Obfuscation().Xor(data, key, offset++); // mutated differently each time
+        obfuscation(data, offset++); // mutated differently each time
         ankerl::nanobench::doNotOptimizeAway(data);
     });
 }

src/dbwrapper.cpp

@@ -174,7 +174,7 @@ void CDBBatch::Clear()
 void CDBBatch::WriteImpl(std::span<const std::byte> key, DataStream& ssValue)
 {
     leveldb::Slice slKey(CharCast(key.data()), key.size());
-    ssValue.Xor(dbwrapper_private::GetObfuscation(parent));
+    dbwrapper_private::GetObfuscation(parent)(ssValue);
     leveldb::Slice slValue(CharCast(ssValue.data()), ssValue.size());
     m_impl_batch->batch.Put(slKey, slValue);
 }
@@ -249,15 +249,14 @@ CDBWrapper::CDBWrapper(const DBParams& params)
         LogPrintf("Finished database compaction of %s\n", fs::PathToString(params.path));
     }
 
-    m_obfuscation = std::vector<uint8_t>(Obfuscation::KEY_SIZE, '\000'); // Needed for unobfuscated Read()/Write() below
+    assert(!m_obfuscation); // Needed for unobfuscated Read()/Write() below
     if (!Read(OBFUSCATION_KEY_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);
-        LogInfo("Wrote new obfuscation key for %s: %s", fs::PathToString(params.path), HexStr(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), HexStr(m_obfuscation));
-
+    LogInfo("Using obfuscation key for %s: %s", fs::PathToString(params.path), m_obfuscation.HexKey());
 }
 
 CDBWrapper::~CDBWrapper()
@@ -385,7 +384,7 @@ void CDBIterator::Next() { m_impl_iter->iter->Next(); }
 
 namespace dbwrapper_private {
 
-const std::vector<unsigned char>& GetObfuscation(const CDBWrapper &w)
+const Obfuscation& GetObfuscation(const CDBWrapper& w)
 {
     return w.m_obfuscation;
 }

src/dbwrapper.h

@@ -18,7 +18,6 @@
 #include <optional>
 #include <stdexcept>
 #include <string>
-#include <vector>
 
 static const size_t DBWRAPPER_PREALLOC_KEY_SIZE = 64;
 static const size_t DBWRAPPER_PREALLOC_VALUE_SIZE = 1024;
@@ -63,8 +62,7 @@ namespace dbwrapper_private {
  * Database obfuscation should be considered an implementation detail of the
  * specific database.
  */
-const std::vector<unsigned char>& GetObfuscation(const CDBWrapper &w);
-
+const Obfuscation& GetObfuscation(const CDBWrapper&);
 }; // namespace dbwrapper_private
 
 bool DestroyDB(const std::string& path_str);
@@ -166,7 +164,7 @@ class CDBIterator
     template<typename V> bool GetValue(V& value) {
         try {
             DataStream ssValue{GetValueImpl()};
-            ssValue.Xor(dbwrapper_private::GetObfuscation(parent));
+            dbwrapper_private::GetObfuscation(parent)(ssValue);
             ssValue >> value;
         } catch (const std::exception&) {
             return false;
@@ -179,16 +177,16 @@ struct LevelDBContext;
 
 class CDBWrapper
 {
-    friend const std::vector<unsigned char>& dbwrapper_private::GetObfuscation(const CDBWrapper &w);
+    friend const Obfuscation& dbwrapper_private::GetObfuscation(const CDBWrapper&);
 private:
     //! holds all leveldb-specific fields of this class
     std::unique_ptr<LevelDBContext> m_db_context;
 
     //! the name of this database
     std::string m_name;
 
-    //! a key used for optional XOR-obfuscation of the database
-    std::vector<unsigned char> m_obfuscation;
+    //! optional XOR-obfuscation of the database
+    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
@@ -223,7 +221,7 @@ class CDBWrapper
         }
         try {
             DataStream ssValue{MakeByteSpan(*strValue)};
-            ssValue.Xor(m_obfuscation);
+            m_obfuscation(ssValue);
             ssValue >> value;
         } catch (const std::exception&) {
             return false;

src/node/blockstorage.cpp

@@ -1174,7 +1174,7 @@ static auto InitBlocksdirXorKey(const BlockManager::Options& opts)
         };
     }
     LogInfo("Using obfuscation key for blocksdir *.dat files (%s): '%s'\n", fs::PathToString(opts.blocks_dir), HexStr(obfuscation));
-    return std::vector<std::byte>{obfuscation.begin(), obfuscation.end()};
+    return Obfuscation{obfuscation};
 }
 
 BlockManager::BlockManager(const util::SignalInterrupt& interrupt, Options opts)

src/node/blockstorage.h

@@ -235,7 +235,7 @@ class BlockManager
 
     const bool m_prune_mode;
 
-    const std::vector<std::byte> m_obfuscation;
+    const Obfuscation m_obfuscation;
 
     /** Dirty block index entries. */
     std::set<CBlockIndex*> m_dirty_blockindex;

src/node/mempool_persist.cpp

@@ -64,7 +64,7 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active
         if (version == MEMPOOL_DUMP_VERSION_NO_XOR_KEY) {
             file.SetObfuscation({});
         } else if (version == MEMPOOL_DUMP_VERSION) {
-            std::vector<std::byte> obfuscation(Obfuscation::KEY_SIZE);
+            Obfuscation obfuscation;
             file >> obfuscation;
             file.SetObfuscation(obfuscation);
         } else {
@@ -183,8 +183,7 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock
         file << version;
 
         if (!pool.m_opts.persist_v1_dat) {
-            std::vector<std::byte> obfuscation(Obfuscation::KEY_SIZE);
-            FastRandomContext{}.fillrand(obfuscation);
+            const Obfuscation obfuscation{FastRandomContext{}.randbytes<Obfuscation::KEY_SIZE>()};
             file << obfuscation;
             file.SetObfuscation(obfuscation);
         } else {

src/streams.cpp

@@ -10,8 +10,7 @@
 
 #include <array>
 
-AutoFile::AutoFile(std::FILE* file, std::vector<std::byte> obfuscation)
-    : m_file{file}, m_obfuscation{std::move(obfuscation)}
+AutoFile::AutoFile(std::FILE* file, const Obfuscation& obfuscation) : m_file{file}, m_obfuscation{obfuscation}
 {
     if (!IsNull()) {
         auto pos{std::ftell(m_file)};
@@ -22,12 +21,12 @@ AutoFile::AutoFile(std::FILE* file, std::vector<std::byte> obfuscation)
 std::size_t AutoFile::detail_fread(std::span<std::byte> dst)
 {
     if (!m_file) throw std::ios_base::failure("AutoFile::read: file handle is nullptr");
-    size_t ret = std::fread(dst.data(), 1, dst.size(), m_file);
-    if (!m_obfuscation.empty()) {
-        if (!m_position.has_value()) throw std::ios_base::failure("AutoFile::read: position unknown");
-        Obfuscation().Xor(dst.subspan(0, ret), m_obfuscation, *m_position);
+    const size_t ret = std::fread(dst.data(), 1, dst.size(), m_file);
+    if (m_obfuscation) {
+        if (!m_position) throw std::ios_base::failure("AutoFile::read: position unknown");
+        m_obfuscation(dst.subspan(0, ret), *m_position);
     }
-    if (m_position.has_value()) *m_position += ret;
+    if (m_position) *m_position += ret;
     return ret;
 }
 
@@ -82,7 +81,7 @@ void AutoFile::ignore(size_t nSize)
 void AutoFile::write(std::span<const std::byte> src)
 {
     if (!m_file) throw std::ios_base::failure("AutoFile::write: file handle is nullptr");
-    if (m_obfuscation.empty()) {
+    if (!m_obfuscation) {
         if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) {
             throw std::ios_base::failure("AutoFile::write: write failed");
         }
@@ -102,9 +101,9 @@ void AutoFile::write(std::span<const std::byte> src)
 void AutoFile::write_buffer(std::span<std::byte> src)
 {
     if (!m_file) throw std::ios_base::failure("AutoFile::write_buffer: file handle is nullptr");
-    if (m_obfuscation.size()) {
+    if (m_obfuscation) {
         if (!m_position) throw std::ios_base::failure("AutoFile::write_buffer: obfuscation position unknown");
-        Obfuscation().Xor(src, m_obfuscation, *m_position); // obfuscate in-place
+        m_obfuscation(src, *m_position); // obfuscate in-place
     }
     if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) {
         throw std::ios_base::failure("AutoFile::write_buffer: write failed");

src/streams.h

@@ -25,7 +25,6 @@
 #include <limits>
 #include <optional>
 #include <string>
-#include <utility>
 #include <vector>
 
 /* Minimal stream for overwriting and/or appending to an existing byte vector
@@ -245,23 +244,13 @@ class DataStream
         return (*this);
     }
 
-    template<typename T>
+    template <typename T>
     DataStream& operator>>(T&& obj)
     {
         ::Unserialize(*this, obj);
         return (*this);
     }
 
-    /**
-     * XOR the contents of this stream with a certain key.
-     *
-     * @param[in] key    The key used to XOR the data in this stream.
-     */
-    void Xor(const std::vector<unsigned char>& key)
-    {
-        Obfuscation().Xor(MakeWritableByteSpan(*this), MakeByteSpan(key));
-    }
-
     /** Compute total memory usage of this object (own memory + any dynamic memory). */
     size_t GetMemoryUsage() const noexcept;
 };
@@ -382,12 +371,12 @@ class AutoFile
 {
 protected:
     std::FILE* m_file;
-    std::vector<std::byte> m_obfuscation;
+    Obfuscation m_obfuscation;
     std::optional<int64_t> m_position;
     bool m_was_written{false};
 
 public:
-    explicit AutoFile(std::FILE* file, std::vector<std::byte> obfuscation={});
+    explicit AutoFile(std::FILE* file, const Obfuscation& obfuscation = {});
 
     ~AutoFile()
     {
@@ -435,7 +424,7 @@ class AutoFile
     bool IsNull() const { return m_file == nullptr; }
 
     /** Continue with a different XOR key */
-    void SetObfuscation(std::vector<std::byte> obfuscation) { m_obfuscation = obfuscation; }
+    void SetObfuscation(const Obfuscation& obfuscation) { m_obfuscation = obfuscation; }
 
     /** Implementation detail, only used internally. */
     std::size_t detail_fread(std::span<std::byte> dst);

src/test/dbwrapper_tests.cpp

@@ -9,21 +9,12 @@
 #include <util/string.h>
 
 #include <memory>
+#include <ranges>
 
 #include <boost/test/unit_test.hpp>
 
 using util::ToString;
 
-// Test if a string consists entirely of null characters
-static bool is_null_key(const std::vector<unsigned char>& key) {
-    bool isnull = true;
-
-    for (unsigned int i = 0; i < key.size(); i++)
-        isnull &= (key[i] == '\x00');
-
-    return isnull;
-}
-
 BOOST_FIXTURE_TEST_SUITE(dbwrapper_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(dbwrapper)
@@ -33,7 +24,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper)
         constexpr size_t CACHE_SIZE{1_MiB};
         const fs::path path{m_args.GetDataDirBase() / "dbwrapper"};
 
-        std::vector<uint8_t> obfuscation_key{};
+        Obfuscation obfuscation;
         std::vector<std::pair<uint8_t, uint256>> key_values{};
 
         // Write values
@@ -42,8 +33,8 @@ BOOST_AUTO_TEST_CASE(dbwrapper)
             BOOST_CHECK_EQUAL(obfuscate, !dbw.IsEmpty());
 
             // Ensure that we're doing real obfuscation when obfuscate=true
-            obfuscation_key = dbwrapper_private::GetObfuscation(dbw);
-            BOOST_CHECK_EQUAL(obfuscate, !is_null_key(obfuscation_key));
+            obfuscation = dbwrapper_private::GetObfuscation(dbw);
+            BOOST_CHECK_EQUAL(obfuscate, dbwrapper_private::GetObfuscation(dbw));
 
             for (uint8_t k{0}; k < 10; ++k) {
                 uint8_t key{k};
@@ -56,16 +47,16 @@ BOOST_AUTO_TEST_CASE(dbwrapper)
         // Verify that the obfuscation key is never obfuscated
         {
             CDBWrapper dbw{{.path = path, .cache_bytes = CACHE_SIZE, .obfuscate = false}};
-            BOOST_CHECK(obfuscation_key == dbwrapper_private::GetObfuscation(dbw));
+            BOOST_CHECK_EQUAL(obfuscation, dbwrapper_private::GetObfuscation(dbw));
         }
 
         // Read back the values
         {
             CDBWrapper dbw{{.path = path, .cache_bytes = CACHE_SIZE, .obfuscate = obfuscate}};
 
             // Ensure obfuscation is read back correctly
-            BOOST_CHECK(obfuscation_key == dbwrapper_private::GetObfuscation(dbw));
-            BOOST_CHECK_EQUAL(obfuscate, !is_null_key(obfuscation_key));
+            BOOST_CHECK_EQUAL(obfuscation, dbwrapper_private::GetObfuscation(dbw));
+            BOOST_CHECK_EQUAL(obfuscate, dbwrapper_private::GetObfuscation(dbw));
 
             // Verify all written values
             for (const auto& [key, expected_value] : key_values) {
@@ -89,7 +80,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_basic_data)
         bool res_bool;
 
         // Ensure that we're doing real obfuscation when obfuscate=true
-        BOOST_CHECK_EQUAL(obfuscate, !is_null_key(dbwrapper_private::GetObfuscation(dbw)));
+        BOOST_CHECK_EQUAL(obfuscate, dbwrapper_private::GetObfuscation(dbw));
 
         //Simulate block raw data - "b + block hash"
         std::string key_block = "b" + m_rng.rand256().ToString();
@@ -264,7 +255,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate)
     BOOST_CHECK_EQUAL(res2.ToString(), in.ToString());
 
     BOOST_CHECK(!odbw.IsEmpty());
-    BOOST_CHECK(is_null_key(dbwrapper_private::GetObfuscation(odbw))); // The key should be an empty string
+    BOOST_CHECK(!dbwrapper_private::GetObfuscation(odbw)); // The key should be an empty string
 
     uint256 in2 = m_rng.rand256();
     uint256 res3;
@@ -301,7 +292,7 @@ BOOST_AUTO_TEST_CASE(existing_data_reindex)
     // Check that the key/val we wrote with unobfuscated wrapper doesn't exist
     uint256 res2;
     BOOST_CHECK(!odbw.Read(key, res2));
-    BOOST_CHECK(!is_null_key(dbwrapper_private::GetObfuscation(odbw)));
+    BOOST_CHECK(dbwrapper_private::GetObfuscation(odbw));
 
     uint256 in2 = m_rng.rand256();
     uint256 res3;

src/test/fuzz/autofile.cpp

@@ -22,7 +22,7 @@ FUZZ_TARGET(autofile)
     const auto key_bytes{ConsumeFixedLengthByteVector<std::byte>(fuzzed_data_provider, Obfuscation::KEY_SIZE)};
     AutoFile auto_file{
         fuzzed_file_provider.open(),
-        key_bytes,
+        Obfuscation{std::span{key_bytes}.first<Obfuscation::KEY_SIZE>()},
     };
     LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 100)
     {