From d22d5d925c000bf25ad2410ca66c4c21eea75004 Mon Sep 17 00:00:00 2001
From: stratospher <44024636+stratospher@users.noreply.github.com>
Date: Sun, 13 Aug 2023 11:55:46 +0530
Subject: [PATCH 1/2] crypto: BIP324 ciphersuite follow-up

follow-up to #28008.
* move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests
outside of the loop to be reused every time
* use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()`
* comment for initiator in `BIP324Cipher::Initialize()`
* systematically damage ciphertext with bit positions in bip324_tests
* use 4095 max bytes for aad in bip324 fuzz test
---
 src/bip324.h                    | 1 +
 src/crypto/chacha20poly1305.cpp | 2 +-
 src/test/bip324_tests.cpp       | 4 ++--
 src/test/crypto_tests.cpp       | 3 +--
 src/test/fuzz/bip324.cpp        | 4 ++--
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/bip324.h b/src/bip324.h
index 8d025c2ee37..ecab299af5e 100644
--- a/src/bip324.h
+++ b/src/bip324.h
@@ -54,6 +54,7 @@ public:
 
     /** Initialize when the other side's public key is received. Can only be called once.
      *
+     * initiator is set to true if we are the initiator establishing the v2 P2P connection.
      * self_decrypt is only for testing, and swaps encryption/decryption keys, so that encryption
      * and decryption can be tested without knowing the other side's private key.
      */
diff --git a/src/crypto/chacha20poly1305.cpp b/src/crypto/chacha20poly1305.cpp
index c936dd22659..2636ebe2b2a 100644
--- a/src/crypto/chacha20poly1305.cpp
+++ b/src/crypto/chacha20poly1305.cpp
@@ -95,7 +95,7 @@ bool AEADChaCha20Poly1305::Decrypt(Span<const std::byte> cipher, Span<const std:
     m_chacha20.Seek64(nonce, 0);
     std::byte expected_tag[EXPANSION];
     ComputeTag(m_chacha20, aad, cipher.first(cipher.size() - EXPANSION), expected_tag);
-    if (timingsafe_bcmp(UCharCast(expected_tag), UCharCast(cipher.data() + cipher.size() - EXPANSION), EXPANSION)) return false;
+    if (timingsafe_bcmp(UCharCast(expected_tag), UCharCast(cipher.last(EXPANSION).data()), EXPANSION)) return false;
 
     // Decrypt (starting at block 1).
     m_chacha20.Crypt(UCharCast(cipher.data()), UCharCast(plain1.data()), plain1.size());
diff --git a/src/test/bip324_tests.cpp b/src/test/bip324_tests.cpp
index ccb9e59e589..0bc22496e3f 100644
--- a/src/test/bip324_tests.cpp
+++ b/src/test/bip324_tests.cpp
@@ -131,10 +131,10 @@ void TestBIP324PacketVector(
         // Decrypt length
         auto to_decrypt = ciphertext;
         if (error >= 2 && error <= 9) {
-            to_decrypt[InsecureRandRange(to_decrypt.size())] ^= std::byte(1U << InsecureRandRange(8));
+            to_decrypt[InsecureRandRange(to_decrypt.size())] ^= std::byte(1U << (error - 2));
         }
 
-        // Decrypt length and resize ciphertext to accomodate.
+        // Decrypt length and resize ciphertext to accommodate.
         uint32_t dec_len = dec_cipher.DecryptLength(MakeByteSpan(to_decrypt).first(cipher.LENGTH_LEN));
         to_decrypt.resize(dec_len + cipher.EXPANSION);
 
diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp
index 6663c914a9a..6fbe74a680a 100644
--- a/src/test/crypto_tests.cpp
+++ b/src/test/crypto_tests.cpp
@@ -300,11 +300,11 @@ static void TestFSChaCha20Poly1305(const std::string& plain_hex, const std::stri
     for (int it = 0; it < 10; ++it) {
         // During it==0 we use the single-plain Encrypt/Decrypt; others use a split at prefix.
         size_t prefix = it ? InsecureRandRange(plain.size() + 1) : plain.size();
+        std::byte dummy_tag[FSChaCha20Poly1305::EXPANSION] = {{}};
 
         // Do msg_idx dummy encryptions to seek to the correct packet.
         FSChaCha20Poly1305 enc_aead{key, 224};
         for (uint64_t i = 0; i < msg_idx; ++i) {
-            std::byte dummy_tag[FSChaCha20Poly1305::EXPANSION] = {{}};
             enc_aead.Encrypt(Span{dummy_tag}.first(0), Span{dummy_tag}.first(0), dummy_tag);
         }
 
@@ -319,7 +319,6 @@ static void TestFSChaCha20Poly1305(const std::string& plain_hex, const std::stri
         // Do msg_idx dummy decryptions to seek to the correct packet.
         FSChaCha20Poly1305 dec_aead{key, 224};
         for (uint64_t i = 0; i < msg_idx; ++i) {
-            std::byte dummy_tag[FSChaCha20Poly1305::EXPANSION] = {{}};
             dec_aead.Decrypt(dummy_tag, Span{dummy_tag}.first(0), Span{dummy_tag}.first(0));
         }
 
diff --git a/src/test/fuzz/bip324.cpp b/src/test/fuzz/bip324.cpp
index 359de6c66a3..8282261c52a 100644
--- a/src/test/fuzz/bip324.cpp
+++ b/src/test/fuzz/bip324.cpp
@@ -75,13 +75,13 @@ FUZZ_TARGET(bip324_cipher_roundtrip, .init=Initialize)
         // - Bit 0: whether the ignore bit is set in message
         // - Bit 1: whether the responder (0) or initiator (1) sends
         // - Bit 2: whether this ciphertext will be corrupted (making it the last sent one)
-        // - Bit 3-4: controls the maximum aad length (max 511 bytes)
+        // - Bit 3-4: controls the maximum aad length (max 4095 bytes)
         // - Bit 5-7: controls the maximum content length (max 16383 bytes, for performance reasons)
         unsigned mode = provider.ConsumeIntegral<uint8_t>();
         bool ignore = mode & 1;
         bool from_init = mode & 2;
         bool damage = mode & 4;
-        unsigned aad_length_bits = 3 * ((mode >> 3) & 3);
+        unsigned aad_length_bits = 4 * ((mode >> 3) & 3);
         unsigned aad_length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << aad_length_bits) - 1);
         unsigned length_bits = 2 * ((mode >> 5) & 7);
         unsigned length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << length_bits) - 1);

From 93cb8f03807dcd3297b7fe18b6c901a8d2a01596 Mon Sep 17 00:00:00 2001
From: stratospher <44024636+stratospher@users.noreply.github.com>
Date: Mon, 14 Aug 2023 17:01:07 +0530
Subject: [PATCH 2/2] refactor: add missing headers for BIP324 ciphersuite

---
 src/bip324.cpp                  | 5 +++++
 src/bip324.h                    | 1 +
 src/crypto/chacha20poly1305.cpp | 2 --
 src/crypto/chacha20poly1305.h   | 1 -
 src/test/bip324_tests.cpp       | 4 +++-
 src/test/fuzz/bip324.cpp        | 1 -
 6 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/bip324.cpp b/src/bip324.cpp
index 7ed99e5585d..314e756829f 100644
--- a/src/bip324.cpp
+++ b/src/bip324.cpp
@@ -8,14 +8,19 @@
 #include <crypto/chacha20.h>
 #include <crypto/chacha20poly1305.h>
 #include <crypto/hkdf_sha256_32.h>
+#include <key.h>
+#include <pubkey.h>
 #include <random.h>
 #include <span.h>
 #include <support/cleanse.h>
+#include <uint256.h>
 
 #include <algorithm>
 #include <assert.h>
 #include <cstdint>
 #include <cstddef>
+#include <iterator>
+#include <string>
 
 BIP324Cipher::BIP324Cipher() noexcept
 {
diff --git a/src/bip324.h b/src/bip324.h
index ecab299af5e..0238c479c08 100644
--- a/src/bip324.h
+++ b/src/bip324.h
@@ -5,6 +5,7 @@
 #ifndef BITCOIN_BIP324_H
 #define BITCOIN_BIP324_H
 
+#include <array>
 #include <cstddef>
 #include <optional>
 
diff --git a/src/crypto/chacha20poly1305.cpp b/src/crypto/chacha20poly1305.cpp
index 2636ebe2b2a..26161641bb3 100644
--- a/src/crypto/chacha20poly1305.cpp
+++ b/src/crypto/chacha20poly1305.cpp
@@ -11,9 +11,7 @@
 #include <support/cleanse.h>
 
 #include <assert.h>
-#include <cstdint>
 #include <cstddef>
-#include <iterator>
 
 AEADChaCha20Poly1305::AEADChaCha20Poly1305(Span<const std::byte> key) noexcept : m_chacha20(UCharCast(key.data()))
 {
diff --git a/src/crypto/chacha20poly1305.h b/src/crypto/chacha20poly1305.h
index dc8fb1cc13f..a847c258ef1 100644
--- a/src/crypto/chacha20poly1305.h
+++ b/src/crypto/chacha20poly1305.h
@@ -6,7 +6,6 @@
 #define BITCOIN_CRYPTO_CHACHA20POLY1305_H
 
 #include <cstddef>
-#include <cstdlib>
 #include <stdint.h>
 
 #include <crypto/chacha20.h>
diff --git a/src/test/bip324_tests.cpp b/src/test/bip324_tests.cpp
index 0bc22496e3f..04472611ec6 100644
--- a/src/test/bip324_tests.cpp
+++ b/src/test/bip324_tests.cpp
@@ -6,13 +6,15 @@
 #include <chainparams.h>
 #include <key.h>
 #include <pubkey.h>
+#include <span.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <util/strencodings.h>
 
 #include <array>
-#include <vector>
 #include <cstddef>
+#include <cstdint>
+#include <vector>
 
 #include <boost/test/unit_test.hpp>
 
diff --git a/src/test/fuzz/bip324.cpp b/src/test/fuzz/bip324.cpp
index 8282261c52a..98ac10e364f 100644
--- a/src/test/fuzz/bip324.cpp
+++ b/src/test/fuzz/bip324.cpp
@@ -11,7 +11,6 @@
 #include <test/util/xoroshiro128plusplus.h>
 
 #include <cstdint>
-#include <tuple>
 #include <vector>
 
 namespace {