From 3ffa5fb49ee4a6d9502aa957093bd94058630282 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 30 Jul 2023 11:43:10 -0400 Subject: net: make V2Transport send uniformly random number garbage bytes --- src/net.cpp | 27 +++++++++++++++++++-------- src/net.h | 10 +++++----- src/test/fuzz/p2p_transport_serialization.cpp | 25 +++++++++++++++++++++++-- 3 files changed, 47 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index 25f390961b..e75089c8e9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -987,20 +987,26 @@ V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int versio m_recv_state{initiating ? RecvState::KEY : RecvState::KEY_MAYBE_V1}, m_send_state{initiating ? SendState::AWAITING_KEY : SendState::MAYBE_V1} { - // Initialize the send buffer with ellswift pubkey. - m_send_buffer.resize(EllSwiftPubKey::size()); + // Construct garbage (including its length) using a FastRandomContext. + FastRandomContext rng; + size_t garbage_len = rng.randrange(MAX_GARBAGE_LEN + 1); + // Initialize the send buffer with ellswift pubkey + garbage. + m_send_buffer.resize(EllSwiftPubKey::size() + garbage_len); std::copy(std::begin(m_cipher.GetOurPubKey()), std::end(m_cipher.GetOurPubKey()), MakeWritableByteSpan(m_send_buffer).begin()); + rng.fillrand(MakeWritableByteSpan(m_send_buffer).subspan(EllSwiftPubKey::size())); } -V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32) noexcept : +V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32, Span garbage) noexcept : m_cipher{key, ent32}, m_initiating{initiating}, m_nodeid{nodeid}, m_v1_fallback{nodeid, type_in, version_in}, m_recv_type{type_in}, m_recv_version{version_in}, m_recv_state{initiating ? RecvState::KEY : RecvState::KEY_MAYBE_V1}, m_send_state{initiating ? SendState::AWAITING_KEY : SendState::MAYBE_V1} { - // Initialize the send buffer with ellswift pubkey. - m_send_buffer.resize(EllSwiftPubKey::size()); + assert(garbage.size() <= MAX_GARBAGE_LEN); + // Initialize the send buffer with ellswift pubkey + provided garbage. + m_send_buffer.resize(EllSwiftPubKey::size() + garbage.size()); std::copy(std::begin(m_cipher.GetOurPubKey()), std::end(m_cipher.GetOurPubKey()), MakeWritableByteSpan(m_send_buffer).begin()); + std::copy(garbage.begin(), garbage.end(), m_send_buffer.begin() + EllSwiftPubKey::size()); } void V2Transport::SetReceiveState(RecvState recv_state) noexcept @@ -1126,16 +1132,18 @@ void V2Transport::ProcessReceivedKeyBytes() noexcept SetSendState(SendState::READY); // Append the garbage terminator to the send buffer. + size_t garbage_len = m_send_buffer.size() - EllSwiftPubKey::size(); m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::GARBAGE_TERMINATOR_LEN); std::copy(m_cipher.GetSendGarbageTerminator().begin(), m_cipher.GetSendGarbageTerminator().end(), MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::GARBAGE_TERMINATOR_LEN).begin()); - // Construct garbage authentication packet in the send buffer. + // Construct garbage authentication packet in the send buffer (using the garbage data which + // is still there). m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION); m_cipher.Encrypt( /*contents=*/{}, - /*aad=*/{}, /* empty garbage for now */ + /*aad=*/MakeByteSpan(m_send_buffer).subspan(EllSwiftPubKey::size(), garbage_len), /*ignore=*/false, /*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION)); @@ -1490,7 +1498,10 @@ void V2Transport::MarkBytesSent(size_t bytes_sent) noexcept m_send_pos += bytes_sent; Assume(m_send_pos <= m_send_buffer.size()); - if (m_send_pos == m_send_buffer.size()) { + // Only wipe the buffer when everything is sent in the READY state. In the AWAITING_KEY state + // we still need the garbage that's in the send buffer to construct the garbage authentication + // packet. + if (m_send_state == SendState::READY && m_send_pos == m_send_buffer.size()) { m_send_pos = 0; m_send_buffer = {}; } diff --git a/src/net.h b/src/net.h index 9194a677b1..90f5c4b388 100644 --- a/src/net.h +++ b/src/net.h @@ -556,9 +556,9 @@ private: /** Normal sending state. * * In this state, the ciphers are initialized, so packets can be sent. When this state is - * entered, the garbage terminator, garbage authentication packet, and version packet are - * appended to the send buffer (in addition to the key which may still be there). In this - * state a message can be provided if the send buffer is empty. */ + * entered, the garbage, garbage terminator, garbage authentication packet, and version + * packet are appended to the send buffer (in addition to the key which may still be + * there). In this state a message can be provided if the send buffer is empty. */ READY, /** This transport is using v1 fallback. @@ -635,8 +635,8 @@ public: */ V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept; - /** Construct a V2 transport with specified keys (test use only). */ - V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32) noexcept; + /** Construct a V2 transport with specified keys and garbage (test use only). */ + V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32, Span garbage) noexcept; // Receive side functions. bool ReceivedMessageComplete() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index 3984bfd26e..6e3ef2a707 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -341,11 +341,32 @@ std::unique_ptr MakeV2Transport(NodeId nodeid, bool initiator, RNG& r // Retrieve key auto key = ConsumePrivateKey(provider); if (!key.IsValid()) return {}; + // Construct garbage + size_t garb_len = provider.ConsumeIntegralInRange(0, V2Transport::MAX_GARBAGE_LEN); + std::vector garb; + if (garb_len <= 64) { + // When the garbage length is up to 64 bytes, read it directly from the fuzzer input. + garb = provider.ConsumeBytes(garb_len); + garb.resize(garb_len); + } else { + // If it's longer, generate it from the RNG. This avoids having large amounts of + // (hopefully) irrelevant data needing to be stored in the fuzzer data. + for (auto& v : garb) v = uint8_t(rng()); + } // Retrieve entropy auto ent = provider.ConsumeBytes(32); ent.resize(32); - - return std::make_unique(nodeid, initiator, SER_NETWORK, INIT_PROTO_VERSION, key, ent); + // Use as entropy SHA256(ent || garbage). This prevents a situation where the fuzzer manages to + // include the garbage terminator (which is a function of both ellswift keys) in the garbage. + // This is extremely unlikely (~2^-116) with random keys/garbage, but the fuzzer can choose + // both non-randomly and dependently. Since the entropy is hashed anyway inside the ellswift + // computation, no coverage should be lost by using a hash as entropy, and it removes the + // possibility of garbage that happens to contain what is effectively a hash of the keys. + CSHA256().Write(UCharCast(ent.data()), ent.size()) + .Write(garb.data(), garb.size()) + .Finalize(UCharCast(ent.data())); + + return std::make_unique(nodeid, initiator, SER_NETWORK, INIT_PROTO_VERSION, key, ent, garb); } } // namespace -- cgit v1.2.3