crypto: require key on ChaCha20 initialization

Author: sipa

src/crypto/chacha20.cpp

@@ -40,11 +40,6 @@ void ChaCha20Aligned::SetKey(Span<const std::byte> key) noexcept
     input[11] = 0;
 }
 
-ChaCha20Aligned::ChaCha20Aligned() noexcept
-{
-    memset(input, 0, sizeof(input));
-}
-
 ChaCha20Aligned::~ChaCha20Aligned()
 {
     memory_cleanse(input, sizeof(input));

src/crypto/chacha20.h

@@ -34,7 +34,8 @@ class ChaCha20Aligned
     /** Block size (inputs/outputs to Keystream / Crypt should be multiples of this). */
     static constexpr unsigned BLOCKLEN{64};
 
-    ChaCha20Aligned() noexcept;
+    /** For safety, disallow initialization without key. */
+    ChaCha20Aligned() noexcept = delete;
 
     /** Initialize a cipher with specified 32-byte key. */
     ChaCha20Aligned(Span<const std::byte> key) noexcept;
@@ -84,7 +85,8 @@ class ChaCha20
     /** Expected key length in constructor and SetKey. */
     static constexpr unsigned KEYLEN = ChaCha20Aligned::KEYLEN;
 
-    ChaCha20() noexcept = default;
+    /** For safety, disallow initialization without key. */
+    ChaCha20() noexcept = delete;
 
     /** Initialize a cipher with specified 32-byte key. */
     ChaCha20(Span<const std::byte> key) noexcept : m_aligned(key) {}

src/random.cpp

@@ -6,6 +6,7 @@
 #include <random.h>
 
 #include <compat/cpuid.h>
+#include <crypto/chacha20.h>
 #include <crypto/sha256.h>
 #include <crypto/sha512.h>
 #include <logging.h>
@@ -606,10 +607,7 @@ void FastRandomContext::fillrand(Span<std::byte> output)
     rng.Keystream(output);
 }
 
-FastRandomContext::FastRandomContext(const uint256& seed) noexcept : requires_seed(false), bitbuf_size(0)
-{
-    rng.SetKey(MakeByteSpan(seed));
-}
+FastRandomContext::FastRandomContext(const uint256& seed) noexcept : requires_seed(false), rng(MakeByteSpan(seed)), bitbuf_size(0) {}
 
 bool Random_SanityCheck()
 {
@@ -657,13 +655,13 @@ bool Random_SanityCheck()
     return true;
 }
 
-FastRandomContext::FastRandomContext(bool fDeterministic) noexcept : requires_seed(!fDeterministic), bitbuf_size(0)
+static constexpr std::array<std::byte, ChaCha20::KEYLEN> ZERO_KEY{};
+
+FastRandomContext::FastRandomContext(bool fDeterministic) noexcept : requires_seed(!fDeterministic), rng(ZERO_KEY), bitbuf_size(0)
 {
-    if (!fDeterministic) {
-        return;
-    }
-    static constexpr std::array<std::byte, ChaCha20::KEYLEN> ZERO{};
-    rng.SetKey(ZERO);
+    // Note that despite always initializing with ZERO_KEY, requires_seed is set to true if not
+    // fDeterministic. That means the rng will be reinitialized with a secure random key upon first
+    // use.
 }
 
 FastRandomContext& FastRandomContext::operator=(FastRandomContext&& from) noexcept

src/test/fuzz/crypto_chacha20.cpp

@@ -17,11 +17,9 @@ FUZZ_TARGET(crypto_chacha20)
 {
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
 
-    ChaCha20 chacha20;
-    if (fuzzed_data_provider.ConsumeBool()) {
-        const std::vector<unsigned char> key = ConsumeFixedLengthByteVector(fuzzed_data_provider, 32);
-        chacha20 = ChaCha20{MakeByteSpan(key)};
-    }
+    const std::vector<unsigned char> key = ConsumeFixedLengthByteVector(fuzzed_data_provider, 32);
+    ChaCha20 chacha20{MakeByteSpan(key)};
+
     LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
         CallOneOf(
             fuzzed_data_provider,

src/test/fuzz/crypto_diff_fuzz_chacha20.cpp

@@ -267,20 +267,13 @@ void ECRYPT_keystream_bytes(ECRYPT_ctx* x, u8* stream, u32 bytes)
 
 FUZZ_TARGET(crypto_diff_fuzz_chacha20)
 {
-    static const unsigned char ZEROKEY[32] = {0};
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
 
-    ChaCha20 chacha20;
     ECRYPT_ctx ctx;
 
-    if (fuzzed_data_provider.ConsumeBool()) {
-        const std::vector<unsigned char> key = ConsumeFixedLengthByteVector(fuzzed_data_provider, 32);
-        chacha20 = ChaCha20{MakeByteSpan(key)};
-        ECRYPT_keysetup(&ctx, key.data(), key.size() * 8, 0);
-    } else {
-        // The default ChaCha20 constructor is equivalent to using the all-0 key.
-        ECRYPT_keysetup(&ctx, ZEROKEY, 256, 0);
-    }
+    const std::vector<unsigned char> key = ConsumeFixedLengthByteVector(fuzzed_data_provider, 32);
+    ChaCha20 chacha20{MakeByteSpan(key)};
+    ECRYPT_keysetup(&ctx, key.data(), key.size() * 8, 0);
 
     // ECRYPT_keysetup() doesn't set the counter and nonce to 0 while SetKey() does
     static const uint8_t iv[8] = {0, 0, 0, 0, 0, 0, 0, 0};