key: migrate CKey key data to std::byte

src/httprpc.cpp

@@ -275,7 +275,7 @@ static bool InitRPCAuthentication()
     // If there is a plaintext credential, hash it with a random salt before storage.
     if (!user.empty() || !pass.empty()) {
         // Generate a random 16 byte hex salt.
-        std::array<unsigned char, 16> raw_salt;
+        std::array<std::byte, 16> raw_salt{};
         GetStrongRandBytes(raw_salt);
         std::string salt = HexStr(raw_salt);
 

src/key.cpp

@@ -155,15 +155,15 @@ int ec_seckey_export_der(const secp256k1_context *ctx, unsigned char *seckey, si
     return 1;
 }
 
-bool CKey::Check(const unsigned char *vch) {
-    return secp256k1_ec_seckey_verify(secp256k1_context_static, vch);
-}
+bool CKey::Check(const unsigned char* vch) { return secp256k1_ec_seckey_verify(secp256k1_context_static, vch); }
+
+bool CKey::Check(const std::byte* vch) { return Check(UCharCast(vch)); }
 
 void CKey::MakeNewKey(bool fCompressedIn) {
     MakeKeyData();
     do {
         GetStrongRandBytes(*keydata);
-    } while (!Check(keydata->data()));
+    } while (!Check(begin()));
     fCompressed = fCompressedIn;
 }
 
@@ -317,7 +317,7 @@ EllSwiftPubKey CKey::EllSwiftCreate(std::span<const std::byte> ent32) const
 
     auto success = secp256k1_ellswift_create(secp256k1_context_sign,
                                              UCharCast(encoded_pubkey.data()),
-                                             keydata->data(),
+                                             UCharCast(begin()),
                                              UCharCast(ent32.data()));
 
     // Should always succeed for valid keys (asserted above).
@@ -336,7 +336,7 @@ ECDHSecret CKey::ComputeBIP324ECDHSecret(const EllSwiftPubKey& their_ellswift, c
                                           UCharCast(output.data()),
                                           UCharCast(initiating ? our_ellswift.data() : their_ellswift.data()),
                                           UCharCast(initiating ? their_ellswift.data() : our_ellswift.data()),
-                                          keydata->data(),
+                                          UCharCast(begin()),
                                           initiating ? 0 : 1,
                                           secp256k1_ellswift_xdh_hash_function_bip324,
                                           nullptr);
@@ -365,7 +365,7 @@ std::vector<uint8_t> CKey::CreateMuSig2Nonce(MuSig2SecNonce& secnonce, const uin
 
     // Generate randomness for nonce
     uint256 rand;
-    GetStrongRandBytes(rand);
+    GetStrongRandBytes(MakeWritableByteSpan(rand));
 
     // Generate nonce
     secp256k1_musig_pubnonce pubnonce;

src/key.h

@@ -50,7 +50,7 @@ class CKey
 
 private:
     /** Internal data container for private key material. */
-    using KeyType = std::array<unsigned char, 32>;
+    using KeyType = std::array<std::byte, 32>;
 
     //! Whether the public key corresponding to this private key is (to be) compressed.
     bool fCompressed{false};
@@ -59,7 +59,8 @@ class CKey
     secure_unique_ptr<KeyType> keydata;
 
     //! Check whether the 32-byte array pointed to by vch is valid keydata.
-    bool static Check(const unsigned char* vch);
+    static bool Check(const std::byte* vch);
+    static bool Check(const unsigned char* vch);
 
     void MakeKeyData()
     {
@@ -105,9 +106,9 @@ class CKey
     {
         if (size_t(pend - pbegin) != std::tuple_size_v<KeyType>) {
             ClearKeyData();
-        } else if (Check(UCharCast(&pbegin[0]))) {
+        } else if (Check(&pbegin[0])) {
             MakeKeyData();
-            memcpy(keydata->data(), (unsigned char*)&pbegin[0], keydata->size());
+            memcpy(keydata->data(), &pbegin[0], keydata->size());
             fCompressed = fCompressedIn;
         } else {
             ClearKeyData();
@@ -116,7 +117,7 @@ class CKey
 
     //! Simple read-only vector-like interface.
     unsigned int size() const { return keydata ? keydata->size() : 0; }
-    const std::byte* data() const { return keydata ? reinterpret_cast<const std::byte*>(keydata->data()) : nullptr; }
+    const std::byte* data() const { return keydata ? keydata->data() : nullptr; }
     const std::byte* begin() const { return data(); }
     const std::byte* end() const { return data() + size(); }
 

src/random.cpp

@@ -409,9 +409,9 @@ class RNGState {
      * If always_use_real_rng is false, and MakeDeterministic has been called before, output
      * from the deterministic PRNG instead.
      */
-    bool MixExtract(unsigned char* out, size_t num, CSHA512&& hasher, bool strong_seed, bool always_use_real_rng) noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+    bool MixExtract(std::span<std::byte> out, CSHA512&& hasher, bool strong_seed, bool always_use_real_rng) noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
     {
-        assert(num <= 32);
+        assert(out.size() <= 32);
         unsigned char buf[64];
         static_assert(sizeof(buf) == CSHA512::OUTPUT_SIZE, "Buffer needs to have hasher's output size");
         bool ret;
@@ -430,15 +430,14 @@ class RNGState {
             // Handle requests for deterministic randomness.
             if (!always_use_real_rng && m_deterministic_prng.has_value()) [[unlikely]] {
                 // Overwrite the beginning of buf, which will be used for output.
-                m_deterministic_prng->Keystream(std::as_writable_bytes(std::span{buf, num}));
+                m_deterministic_prng->Keystream(std::as_writable_bytes(std::span{buf, out.size()}));
                 // Do not require strong seeding for deterministic output.
                 ret = true;
             }
         }
         // If desired, copy (up to) the first 32 bytes of the hash output as output.
-        if (num) {
-            assert(out != nullptr);
-            memcpy(out, buf, num);
+        if (!out.empty()) {
+            memcpy(out.data(), buf, out.size());
         }
         // Best effort cleanup of internal state
         hasher.Reset();
@@ -508,7 +507,7 @@ void SeedStrengthen(CSHA512& hasher, RNGState& rng, SteadyClock::duration dur) n
     // Generate 32 bytes of entropy from the RNG, and a copy of the entropy already in hasher.
     // Never use the deterministic PRNG for this, as the result is only used internally.
     unsigned char strengthen_seed[32];
-    rng.MixExtract(strengthen_seed, sizeof(strengthen_seed), CSHA512(hasher), false, /*always_use_real_rng=*/true);
+    rng.MixExtract(MakeWritableByteSpan(strengthen_seed), CSHA512(hasher), false, /*always_use_real_rng=*/true);
     // Strengthen the seed, and feed it into hasher.
     Strengthen(strengthen_seed, dur, hasher);
 }
@@ -559,12 +558,12 @@ enum class RNGLevel {
     PERIODIC, //!< Called by RandAddPeriodic()
 };
 
-void ProcRand(unsigned char* out, int num, RNGLevel level, bool always_use_real_rng) noexcept
+void ProcRand(std::span<std::byte> out, RNGLevel level, bool always_use_real_rng) noexcept
 {
     // Make sure the RNG is initialized first (as all Seed* function possibly need hwrand to be available).
     RNGState& rng = GetRNGState();
 
-    assert(num <= 32);
+    assert(out.size() <= 32);
 
     CSHA512 hasher;
     switch (level) {
@@ -580,11 +579,11 @@ void ProcRand(unsigned char* out, int num, RNGLevel level, bool always_use_real_
     }
 
     // Combine with and update state
-    if (!rng.MixExtract(out, num, std::move(hasher), false, always_use_real_rng)) {
+    if (!rng.MixExtract(out, std::move(hasher), false, always_use_real_rng)) {
         // On the first invocation, also seed with SeedStartup().
         CSHA512 startup_hasher;
         SeedStartup(startup_hasher, rng);
-        rng.MixExtract(out, num, std::move(startup_hasher), true, always_use_real_rng);
+        rng.MixExtract(out, std::move(startup_hasher), true, always_use_real_rng);
     }
 }
 
@@ -601,17 +600,17 @@ std::atomic<bool> g_used_g_prng{false}; // Only accessed from tests
 void GetRandBytes(std::span<unsigned char> bytes) noexcept
 {
     g_used_g_prng = true;
-    ProcRand(bytes.data(), bytes.size(), RNGLevel::FAST, /*always_use_real_rng=*/false);
+    ProcRand(std::as_writable_bytes(bytes), RNGLevel::FAST, /*always_use_real_rng=*/false);
 }
 
-void GetStrongRandBytes(std::span<unsigned char> bytes) noexcept
+void GetStrongRandBytes(std::span<std::byte> bytes) noexcept
 {
-    ProcRand(bytes.data(), bytes.size(), RNGLevel::SLOW, /*always_use_real_rng=*/true);
+    ProcRand(bytes, RNGLevel::SLOW, /*always_use_real_rng=*/true);
 }
 
 void RandAddPeriodic() noexcept
 {
-    ProcRand(nullptr, 0, RNGLevel::PERIODIC, /*always_use_real_rng=*/false);
+    ProcRand({}, RNGLevel::PERIODIC, /*always_use_real_rng=*/false);
 }
 
 void RandAddEvent(const uint32_t event_info) noexcept { GetRNGState().AddEvent(event_info); }
@@ -679,7 +678,7 @@ bool Random_SanityCheck()
     CSHA512 to_add;
     to_add.Write((const unsigned char*)&start, sizeof(start));
     to_add.Write((const unsigned char*)&stop, sizeof(stop));
-    GetRNGState().MixExtract(nullptr, 0, std::move(to_add), false, /*always_use_real_rng=*/true);
+    GetRNGState().MixExtract({}, std::move(to_add), false, /*always_use_real_rng=*/true);
 
     return true;
 }
@@ -696,7 +695,7 @@ FastRandomContext::FastRandomContext(bool fDeterministic) noexcept : requires_se
 void RandomInit()
 {
     // Invoke RNG code to trigger initialization (if not already performed)
-    ProcRand(nullptr, 0, RNGLevel::FAST, /*always_use_real_rng=*/true);
+    ProcRand({}, RNGLevel::FAST, /*always_use_real_rng=*/true);
 
     ReportHardwareRand();
 }

src/random.h

@@ -126,7 +126,7 @@ void GetRandBytes(std::span<unsigned char> bytes) noexcept;
  *
  * Thread-safe.
  */
-void GetStrongRandBytes(std::span<unsigned char> bytes) noexcept;
+void GetStrongRandBytes(std::span<std::byte> bytes) noexcept;
 
 
 /* ============================= RANDOM NUMBER GENERATION CLASSES =============================

src/test/key_tests.cpp

@@ -15,6 +15,8 @@
 #include <util/strencodings.h>
 #include <util/string.h>
 
+#include <array>
+#include <cstring>
 #include <string>
 #include <vector>
 
@@ -37,6 +39,34 @@ static const std::string strAddressBad = "1HV9Lc3sNHZxwj4Zk6fB38tEmBryq2cBiF";
 
 BOOST_FIXTURE_TEST_SUITE(key_tests, BasicTestingSetup)
 
+BOOST_AUTO_TEST_CASE(ckey_set_accepts_unsigned_char_and_std_byte)
+{
+    const CKey ref{DecodeSecret(strSecret1C)};
+    BOOST_REQUIRE(ref.IsValid());
+    BOOST_REQUIRE(ref.IsCompressed());
+
+    auto uc_span{MakeUCharSpan(std::span{ref.begin(), ref.end()})};
+    CKey key_uc;
+    key_uc.Set(uc_span.begin(), uc_span.end(), /*fCompressedIn=*/true);
+    BOOST_CHECK(key_uc.IsValid());
+    BOOST_CHECK(key_uc == ref);
+
+    CKey key_byte;
+    key_byte.Set(ref.begin(), ref.end(), /*fCompressedIn=*/true);
+    BOOST_CHECK(key_byte.IsValid());
+    BOOST_CHECK(key_byte == ref);
+
+    std::array<unsigned char, 32> zeros_uc{};
+    CKey invalid_uc;
+    invalid_uc.Set(zeros_uc.begin(), zeros_uc.end(), /*fCompressedIn=*/true);
+    BOOST_CHECK(!invalid_uc.IsValid());
+
+    std::array<std::byte, 32> zeros_byte{};
+    CKey invalid_byte;
+    invalid_byte.Set(zeros_byte.begin(), zeros_byte.end(), /*fCompressedIn=*/true);
+    BOOST_CHECK(!invalid_byte.IsValid());
+}
+
 BOOST_AUTO_TEST_CASE(key_test1)
 {
     CKey key1  = DecodeSecret(strSecret1);

src/test/random_tests.cpp

@@ -10,8 +10,12 @@
 
 #include <boost/test/unit_test.hpp>
 
+#include <array>
 #include <algorithm>
+#include <cstdint>
+#include <cstring>
 #include <random>
+#include <set>
 
 BOOST_FIXTURE_TEST_SUITE(random_tests, BasicTestingSetup)
 
@@ -20,6 +24,18 @@ BOOST_AUTO_TEST_CASE(osrandom_tests)
     BOOST_CHECK(Random_SanityCheck());
 }
 
+BOOST_AUTO_TEST_CASE(strongrandbytes_test)
+{
+    // Check that GetStrongRandBytes wiring is correct and returns different values on subsequent calls
+    std::set<uint64_t> nums;
+    for (int i{0}; i < 10; ++i) {
+        uint64_t n;
+        GetStrongRandBytes(MakeWritableByteSpan(std::span{&n, 1}));
+        nums.insert(n);
+    }
+    BOOST_CHECK(nums.size() > 1);
+}
+
 BOOST_AUTO_TEST_CASE(fastrandom_tests_deterministic)
 {
     // Check that deterministic FastRandomContexts are deterministic

src/wallet/wallet.cpp

@@ -800,12 +800,12 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase)
     CKeyingMaterial plain_master_key;
 
     plain_master_key.resize(WALLET_CRYPTO_KEY_SIZE);
-    GetStrongRandBytes(plain_master_key);
+    GetStrongRandBytes(MakeWritableByteSpan(plain_master_key));
 
     CMasterKey master_key;
 
     master_key.vchSalt.resize(WALLET_CRYPTO_SALT_SIZE);
-    GetStrongRandBytes(master_key.vchSalt);
+    GetStrongRandBytes(MakeWritableByteSpan(master_key.vchSalt));
 
     if (!EncryptMasterKey(strWalletPassphrase, plain_master_key, master_key)) {
         return false;