Merge bitcoin/bitcoin#28100: crypto: more Span<std::byte> modernization & follow-ups

57cc136 crypto: make ChaCha20::SetKey wipe buffer (Pieter Wuille)
da0ec62 tests: miscellaneous hex / std::byte improvements (Pieter Wuille)
bdcbc85 fuzz: support std::byte in Consume{Fixed,Variable}LengthByteVector (Pieter Wuille)
7d1cd93 crypto: require key on ChaCha20 initialization (Pieter Wuille)
44c1176 random: simplify FastRandomContext::randbytes using fillrand (Pieter Wuille)
3da636e crypto: refactor ChaCha20 classes to use Span<std::byte> interface (Pieter Wuille)

Pull request description:

  This modernizes the ChaCha20 and ChaCha20Aligned interfaces to be `Span<std::byte>` based, and other improvements.

  * Modifies all functions and constructors of `ChaCha20` and `ChaCha20Aligned` to be `Span<std::byte>` based (aligning them with `FSChaCha20`, `AEADChaCha20Poly1305`, and `FSChaCha20Poly1305`)
  * Remove default constructors, to make sure all call sites provide a key (suggested in bitcoin/bitcoin#26153 (comment))
  * Wipe key material on rekey for security (suggested in bitcoin/bitcoin#26153 (comment))
  * Use `HexStr` on byte vectors in tests (suggested in bitcoin/bitcoin#27993 (comment))
  * Support `std::byte` vectors in `ConsumeRandomLengthByteVector` and `ConsumeFixedLengthByteVector`, and use it (suggested in bitcoin/bitcoin#27993 (comment))
  * And a few more.

  While related, I don't see this as a necessary for BIP324.

ACKs for top commit:
  stratospher:
    ACK 57cc136.
  theStack:
    re-ACK 57cc136

Tree-SHA512: 361da4ff003c8465a32eeac0983a8a6f047dbbf5b400168b409c8e3234e79d577fc854e0764389446585da3e12b964c94dd67fc0c9c1d1d092cec296121e05d4

Author: fanquake

src/bench/chacha20.cpp

@@ -14,13 +14,13 @@ static const uint64_t BUFFER_SIZE_LARGE = 1024*1024;
 
 static void CHACHA20(benchmark::Bench& bench, size_t buffersize)
 {
-    std::vector<uint8_t> key(32,0);
-    ChaCha20 ctx(key.data());
-    ctx.Seek64({0, 0}, 0);
-    std::vector<uint8_t> in(buffersize,0);
-    std::vector<uint8_t> out(buffersize,0);
+    std::vector<std::byte> key(32, {});
+    ChaCha20 ctx(key);
+    ctx.Seek({0, 0}, 0);
+    std::vector<std::byte> in(buffersize, {});
+    std::vector<std::byte> out(buffersize, {});
     bench.batch(in.size()).unit("byte").run([&] {
-        ctx.Crypt(in.data(), out.data(), in.size());
+        ctx.Crypt(in, out);
     });
 }
 

src/crypto/chacha20.cpp

@@ -8,6 +8,7 @@
 #include <crypto/common.h>
 #include <crypto/chacha20.h>
 #include <support/cleanse.h>
+#include <span.h>
 
 #include <algorithm>
 #include <string.h>
@@ -22,47 +23,47 @@ constexpr static inline uint32_t rotl32(uint32_t v, int c) { return (v << c) | (
 
 #define REPEAT10(a) do { {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; } while(0)
 
-void ChaCha20Aligned::SetKey32(const unsigned char* k)
+void ChaCha20Aligned::SetKey(Span<const std::byte> key) noexcept
 {
-    input[0] = ReadLE32(k + 0);
-    input[1] = ReadLE32(k + 4);
-    input[2] = ReadLE32(k + 8);
-    input[3] = ReadLE32(k + 12);
-    input[4] = ReadLE32(k + 16);
-    input[5] = ReadLE32(k + 20);
-    input[6] = ReadLE32(k + 24);
-    input[7] = ReadLE32(k + 28);
+    assert(key.size() == KEYLEN);
+    input[0] = ReadLE32(UCharCast(key.data() + 0));
+    input[1] = ReadLE32(UCharCast(key.data() + 4));
+    input[2] = ReadLE32(UCharCast(key.data() + 8));
+    input[3] = ReadLE32(UCharCast(key.data() + 12));
+    input[4] = ReadLE32(UCharCast(key.data() + 16));
+    input[5] = ReadLE32(UCharCast(key.data() + 20));
+    input[6] = ReadLE32(UCharCast(key.data() + 24));
+    input[7] = ReadLE32(UCharCast(key.data() + 28));
     input[8] = 0;
     input[9] = 0;
     input[10] = 0;
     input[11] = 0;
 }
 
-ChaCha20Aligned::ChaCha20Aligned()
-{
-    memset(input, 0, sizeof(input));
-}
-
 ChaCha20Aligned::~ChaCha20Aligned()
 {
     memory_cleanse(input, sizeof(input));
 }
 
-ChaCha20Aligned::ChaCha20Aligned(const unsigned char* key32)
+ChaCha20Aligned::ChaCha20Aligned(Span<const std::byte> key) noexcept
 {
-    SetKey32(key32);
+    SetKey(key);
 }
 
-void ChaCha20Aligned::Seek64(Nonce96 nonce, uint32_t block_counter)
+void ChaCha20Aligned::Seek(Nonce96 nonce, uint32_t block_counter) noexcept
 {
     input[8] = block_counter;
     input[9] = nonce.first;
     input[10] = nonce.second;
     input[11] = nonce.second >> 32;
 }
 
-inline void ChaCha20Aligned::Keystream64(unsigned char* c, size_t blocks)
+inline void ChaCha20Aligned::Keystream(Span<std::byte> output) noexcept
 {
+    unsigned char* c = UCharCast(output.data());
+    size_t blocks = output.size() / BLOCKLEN;
+    assert(blocks * BLOCKLEN == output.size());
+
     uint32_t x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13, x14, x15;
     uint32_t j4, j5, j6, j7, j8, j9, j10, j11, j12, j13, j14, j15;
 
@@ -154,12 +155,18 @@ inline void ChaCha20Aligned::Keystream64(unsigned char* c, size_t blocks)
             return;
         }
         blocks -= 1;
-        c += 64;
+        c += BLOCKLEN;
     }
 }
 
-inline void ChaCha20Aligned::Crypt64(const unsigned char* m, unsigned char* c, size_t blocks)
+inline void ChaCha20Aligned::Crypt(Span<const std::byte> in_bytes, Span<std::byte> out_bytes) noexcept
 {
+    assert(in_bytes.size() == out_bytes.size());
+    const unsigned char* m = UCharCast(in_bytes.data());
+    unsigned char* c = UCharCast(out_bytes.data());
+    size_t blocks = out_bytes.size() / BLOCKLEN;
+    assert(blocks * BLOCKLEN == out_bytes.size());
+
     uint32_t x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13, x14, x15;
     uint32_t j4, j5, j6, j7, j8, j9, j10, j11, j12, j13, j14, j15;
 
@@ -268,70 +275,75 @@ inline void ChaCha20Aligned::Crypt64(const unsigned char* m, unsigned char* c, s
             return;
         }
         blocks -= 1;
-        c += 64;
-        m += 64;
+        c += BLOCKLEN;
+        m += BLOCKLEN;
     }
 }
 
-void ChaCha20::Keystream(unsigned char* c, size_t bytes)
+void ChaCha20::Keystream(Span<std::byte> out) noexcept
 {
-    if (!bytes) return;
+    if (out.empty()) return;
     if (m_bufleft) {
-        unsigned reuse = std::min<size_t>(m_bufleft, bytes);
-        memcpy(c, m_buffer + 64 - m_bufleft, reuse);
+        unsigned reuse = std::min<size_t>(m_bufleft, out.size());
+        std::copy(m_buffer.end() - m_bufleft, m_buffer.end() - m_bufleft + reuse, out.begin());
         m_bufleft -= reuse;
-        bytes -= reuse;
-        c += reuse;
+        out = out.subspan(reuse);
     }
-    if (bytes >= 64) {
-        size_t blocks = bytes / 64;
-        m_aligned.Keystream64(c, blocks);
-        c += blocks * 64;
-        bytes -= blocks * 64;
+    if (out.size() >= m_aligned.BLOCKLEN) {
+        size_t blocks = out.size() / m_aligned.BLOCKLEN;
+        m_aligned.Keystream(out.first(blocks * m_aligned.BLOCKLEN));
+        out = out.subspan(blocks * m_aligned.BLOCKLEN);
     }
-    if (bytes) {
-        m_aligned.Keystream64(m_buffer, 1);
-        memcpy(c, m_buffer, bytes);
-        m_bufleft = 64 - bytes;
+    if (!out.empty()) {
+        m_aligned.Keystream(m_buffer);
+        std::copy(m_buffer.begin(), m_buffer.begin() + out.size(), out.begin());
+        m_bufleft = m_aligned.BLOCKLEN - out.size();
     }
 }
 
-void ChaCha20::Crypt(const unsigned char* m, unsigned char* c, size_t bytes)
+void ChaCha20::Crypt(Span<const std::byte> input, Span<std::byte> output) noexcept
 {
-    if (!bytes) return;
+    assert(input.size() == output.size());
+
+    if (!input.size()) return;
     if (m_bufleft) {
-        unsigned reuse = std::min<size_t>(m_bufleft, bytes);
+        unsigned reuse = std::min<size_t>(m_bufleft, input.size());
         for (unsigned i = 0; i < reuse; i++) {
-            c[i] = m[i] ^ m_buffer[64 - m_bufleft + i];
+            output[i] = input[i] ^ m_buffer[m_aligned.BLOCKLEN - m_bufleft + i];
         }
         m_bufleft -= reuse;
-        bytes -= reuse;
-        c += reuse;
-        m += reuse;
+        output = output.subspan(reuse);
+        input = input.subspan(reuse);
     }
-    if (bytes >= 64) {
-        size_t blocks = bytes / 64;
-        m_aligned.Crypt64(m, c, blocks);
-        c += blocks * 64;
-        m += blocks * 64;
-        bytes -= blocks * 64;
+    if (input.size() >= m_aligned.BLOCKLEN) {
+        size_t blocks = input.size() / m_aligned.BLOCKLEN;
+        m_aligned.Crypt(input.first(blocks * m_aligned.BLOCKLEN), output.first(blocks * m_aligned.BLOCKLEN));
+        output = output.subspan(blocks * m_aligned.BLOCKLEN);
+        input = input.subspan(blocks * m_aligned.BLOCKLEN);
     }
-    if (bytes) {
-        m_aligned.Keystream64(m_buffer, 1);
-        for (unsigned i = 0; i < bytes; i++) {
-            c[i] = m[i] ^ m_buffer[i];
+    if (!input.empty()) {
+        m_aligned.Keystream(m_buffer);
+        for (unsigned i = 0; i < input.size(); i++) {
+            output[i] = input[i] ^ m_buffer[i];
         }
-        m_bufleft = 64 - bytes;
+        m_bufleft = m_aligned.BLOCKLEN - input.size();
     }
 }
 
 ChaCha20::~ChaCha20()
 {
-    memory_cleanse(m_buffer, sizeof(m_buffer));
+    memory_cleanse(m_buffer.data(), m_buffer.size());
+}
+
+void ChaCha20::SetKey(Span<const std::byte> key) noexcept
+{
+    m_aligned.SetKey(key);
+    m_bufleft = 0;
+    memory_cleanse(m_buffer.data(), m_buffer.size());
 }
 
 FSChaCha20::FSChaCha20(Span<const std::byte> key, uint32_t rekey_interval) noexcept :
-    m_chacha20(UCharCast(key.data())), m_rekey_interval(rekey_interval)
+    m_chacha20(key), m_rekey_interval(rekey_interval)
 {
     assert(key.size() == KEYLEN);
 }
@@ -341,20 +353,20 @@ void FSChaCha20::Crypt(Span<const std::byte> input, Span<std::byte> output) noex
     assert(input.size() == output.size());
 
     // Invoke internal stream cipher for actual encryption/decryption.
-    m_chacha20.Crypt(UCharCast(input.data()), UCharCast(output.data()), input.size());
+    m_chacha20.Crypt(input, output);
 
     // Rekey after m_rekey_interval encryptions/decryptions.
     if (++m_chunk_counter == m_rekey_interval) {
         // Get new key from the stream cipher.
         std::byte new_key[KEYLEN];
-        m_chacha20.Keystream(UCharCast(new_key), sizeof(new_key));
+        m_chacha20.Keystream(new_key);
         // Update its key.
-        m_chacha20.SetKey32(UCharCast(new_key));
+        m_chacha20.SetKey(new_key);
         // Wipe the key (a copy remains inside m_chacha20, where it'll be wiped on the next rekey
         // or on destruction).
         memory_cleanse(new_key, sizeof(new_key));
         // Set the nonce for the new section of output.
-        m_chacha20.Seek64({0, ++m_rekey_counter}, 0);
+        m_chacha20.Seek({0, ++m_rekey_counter}, 0);
         // Reset the chunk counter.
         m_chunk_counter = 0;
     }

src/crypto/chacha20.h

@@ -28,16 +28,23 @@ class ChaCha20Aligned
     uint32_t input[12];
 
 public:
-    ChaCha20Aligned();
+    /** Expected key length in constructor and SetKey. */
+    static constexpr unsigned KEYLEN{32};
+
+    /** Block size (inputs/outputs to Keystream / Crypt should be multiples of this). */
+    static constexpr unsigned BLOCKLEN{64};
+
+    /** For safety, disallow initialization without key. */
+    ChaCha20Aligned() noexcept = delete;
 
     /** Initialize a cipher with specified 32-byte key. */
-    ChaCha20Aligned(const unsigned char* key32);
+    ChaCha20Aligned(Span<const std::byte> key) noexcept;
 
     /** Destructor to clean up private memory. */
     ~ChaCha20Aligned();
 
-    /** set 32-byte key. */
-    void SetKey32(const unsigned char* key32);
+    /** Set 32-byte key, and seek to nonce 0 and block position 0. */
+    void SetKey(Span<const std::byte> key) noexcept;
 
     /** Type for 96-bit nonces used by the Set function below.
      *
@@ -51,61 +58,63 @@ class ChaCha20Aligned
 
     /** Set the 96-bit nonce and 32-bit block counter.
      *
-     * Block_counter selects a position to seek to (to byte 64*block_counter). After 256 GiB, the
-     * block counter overflows, and nonce.first is incremented.
+     * Block_counter selects a position to seek to (to byte BLOCKLEN*block_counter). After 256 GiB,
+     * the block counter overflows, and nonce.first is incremented.
      */
-    void Seek64(Nonce96 nonce, uint32_t block_counter);
+    void Seek(Nonce96 nonce, uint32_t block_counter) noexcept;
 
-    /** outputs the keystream of size <64*blocks> into <c> */
-    void Keystream64(unsigned char* c, size_t blocks);
+    /** outputs the keystream into out, whose length must be a multiple of BLOCKLEN. */
+    void Keystream(Span<std::byte> out) noexcept;
 
-    /** enciphers the message <input> of length <64*blocks> and write the enciphered representation into <output>
-     *  Used for encryption and decryption (XOR)
+    /** en/deciphers the message <input> and write the result into <output>
+     *
+     * The size of input and output must be equal, and be a multiple of BLOCKLEN.
      */
-    void Crypt64(const unsigned char* input, unsigned char* output, size_t blocks);
+    void Crypt(Span<const std::byte> input, Span<std::byte> output) noexcept;
 };
 
 /** Unrestricted ChaCha20 cipher. */
 class ChaCha20
 {
 private:
     ChaCha20Aligned m_aligned;
-    unsigned char m_buffer[64] = {0};
+    std::array<std::byte, ChaCha20Aligned::BLOCKLEN> m_buffer;
     unsigned m_bufleft{0};
 
 public:
-    ChaCha20() = default;
+    /** Expected key length in constructor and SetKey. */
+    static constexpr unsigned KEYLEN = ChaCha20Aligned::KEYLEN;
+
+    /** For safety, disallow initialization without key. */
+    ChaCha20() noexcept = delete;
 
     /** Initialize a cipher with specified 32-byte key. */
-    ChaCha20(const unsigned char* key32) : m_aligned(key32) {}
+    ChaCha20(Span<const std::byte> key) noexcept : m_aligned(key) {}
 
     /** Destructor to clean up private memory. */
     ~ChaCha20();
 
-    /** set 32-byte key. */
-    void SetKey32(const unsigned char* key32)
-    {
-        m_aligned.SetKey32(key32);
-        m_bufleft = 0;
-    }
+    /** Set 32-byte key, and seek to nonce 0 and block position 0. */
+    void SetKey(Span<const std::byte> key) noexcept;
 
     /** 96-bit nonce type. */
     using Nonce96 = ChaCha20Aligned::Nonce96;
 
-    /** Set the 96-bit nonce and 32-bit block counter. */
-    void Seek64(Nonce96 nonce, uint32_t block_counter)
+    /** Set the 96-bit nonce and 32-bit block counter. See ChaCha20Aligned::Seek. */
+    void Seek(Nonce96 nonce, uint32_t block_counter) noexcept
     {
-        m_aligned.Seek64(nonce, block_counter);
+        m_aligned.Seek(nonce, block_counter);
         m_bufleft = 0;
     }
 
-    /** outputs the keystream of size <bytes> into <c> */
-    void Keystream(unsigned char* c, size_t bytes);
-
-    /** enciphers the message <input> of length <bytes> and write the enciphered representation into <output>
-     *  Used for encryption and decryption (XOR)
+    /** en/deciphers the message <in_bytes> and write the result into <out_bytes>
+     *
+     * The size of in_bytes and out_bytes must be equal.
      */
-    void Crypt(const unsigned char* input, unsigned char* output, size_t bytes);
+    void Crypt(Span<const std::byte> in_bytes, Span<std::byte> out_bytes) noexcept;
+
+    /** outputs the keystream to out. */
+    void Keystream(Span<std::byte> out) noexcept;
 };
 
 /** Forward-secure ChaCha20