diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 56a243085c2d..45834049b46f 100644 --- a/src/httprpc.cpp +++ b/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 raw_salt; + std::array raw_salt{}; GetStrongRandBytes(raw_salt); std::string salt = HexStr(raw_salt); diff --git a/src/key.cpp b/src/key.cpp index f17a286750e4..c7ab2fe3b0bc 100644 --- a/src/key.cpp +++ b/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 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 CKey::CreateMuSig2Nonce(MuSig2SecNonce& secnonce, const uin // Generate randomness for nonce uint256 rand; - GetStrongRandBytes(rand); + GetStrongRandBytes(MakeWritableByteSpan(rand)); // Generate nonce secp256k1_musig_pubnonce pubnonce; diff --git a/src/key.h b/src/key.h index f35f3cc01562..e16a53bb0831 100644 --- a/src/key.h +++ b/src/key.h @@ -50,7 +50,7 @@ class CKey private: /** Internal data container for private key material. */ - using KeyType = std::array; + using KeyType = std::array; //! 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 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) { 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(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(); } diff --git a/src/random.cpp b/src/random.cpp index aeaf6baa8e8b..baca22cf0629 100644 --- a/src/random.cpp +++ b/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 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 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 g_used_g_prng{false}; // Only accessed from tests void GetRandBytes(std::span 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 bytes) noexcept +void GetStrongRandBytes(std::span 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(); } diff --git a/src/random.h b/src/random.h index 330c10a36b88..9070d4eb7434 100644 --- a/src/random.h +++ b/src/random.h @@ -126,7 +126,7 @@ void GetRandBytes(std::span bytes) noexcept; * * Thread-safe. */ -void GetStrongRandBytes(std::span bytes) noexcept; +void GetStrongRandBytes(std::span bytes) noexcept; /* ============================= RANDOM NUMBER GENERATION CLASSES ============================= diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index acced716f0e6..8641aafefa1d 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -15,6 +15,8 @@ #include #include +#include +#include #include #include @@ -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 zeros_uc{}; + CKey invalid_uc; + invalid_uc.Set(zeros_uc.begin(), zeros_uc.end(), /*fCompressedIn=*/true); + BOOST_CHECK(!invalid_uc.IsValid()); + + std::array 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); diff --git a/src/test/random_tests.cpp b/src/test/random_tests.cpp index 213465fded16..0c9bb25f6a12 100644 --- a/src/test/random_tests.cpp +++ b/src/test/random_tests.cpp @@ -10,8 +10,12 @@ #include +#include #include +#include +#include #include +#include 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 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 diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 56c676c189f1..3eb975887330 100644 --- a/src/wallet/wallet.cpp +++ b/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;