diff --git a/src/bip324.cpp b/src/bip324.cpp index 314e756829f..f579a25193a 100644 --- a/src/bip324.cpp +++ b/src/bip324.cpp @@ -22,13 +22,6 @@ #include #include -BIP324Cipher::BIP324Cipher() noexcept -{ - m_key.MakeNewKey(true); - uint256 entropy = GetRandHash(); - m_our_pubkey = m_key.EllSwiftCreate(MakeByteSpan(entropy)); -} - BIP324Cipher::BIP324Cipher(const CKey& key, Span ent32) noexcept : m_key(key) { diff --git a/src/bip324.h b/src/bip324.h index 0238c479c08..28e7c411eaa 100644 --- a/src/bip324.h +++ b/src/bip324.h @@ -41,8 +41,8 @@ class BIP324Cipher std::array m_recv_garbage_terminator; public: - /** Initialize a BIP324 cipher with securely generated random keys. */ - BIP324Cipher() noexcept; + /** No default constructor; keys must be provided to create a BIP324Cipher. */ + BIP324Cipher() = delete; /** Initialize a BIP324 cipher with specified key and encoding entropy (testing only). */ BIP324Cipher(const CKey& key, Span ent32) noexcept; diff --git a/src/net.cpp b/src/net.cpp index 3955005dfa6..af2855932de 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -979,36 +979,56 @@ class V2MessageMap const V2MessageMap V2_MESSAGE_MAP; -} // namespace +CKey GenerateRandomKey() noexcept +{ + CKey key; + key.MakeNewKey(/*fCompressed=*/true); + return key; +} -V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept : - m_cipher{}, 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} +std::vector GenerateRandomGarbage() noexcept { - // Construct garbage (including its length) using a FastRandomContext. + std::vector ret; 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); + ret.resize(rng.randrange(V2Transport::MAX_GARBAGE_LEN + 1)); + rng.fillrand(MakeWritableByteSpan(ret)); + return ret; +} + +} // namespace + +void V2Transport::StartSendingHandshake() noexcept +{ + AssertLockHeld(m_send_mutex); + Assume(m_send_state == SendState::AWAITING_KEY); + Assume(m_send_buffer.empty()); + // Initialize the send buffer with ellswift pubkey + provided garbage. + m_send_buffer.resize(EllSwiftPubKey::size() + m_send_garbage.size()); 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())); + std::copy(m_send_garbage.begin(), m_send_garbage.end(), m_send_buffer.begin() + EllSwiftPubKey::size()); + // We cannot wipe m_send_garbage as it will still be used to construct the garbage + // authentication packet. } -V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32, Span garbage) noexcept : +V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32, std::vector 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_garbage{std::move(garbage)}, m_send_state{initiating ? SendState::AWAITING_KEY : SendState::MAYBE_V1} { - 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()); + Assume(m_send_garbage.size() <= MAX_GARBAGE_LEN); + // Start sending immediately if we're the initiator of the connection. + if (initiating) { + LOCK(m_send_mutex); + StartSendingHandshake(); + } } +V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept : + V2Transport{nodeid, initiating, type_in, version_in, GenerateRandomKey(), + MakeByteSpan(GetRandHash()), GenerateRandomGarbage()} { } + void V2Transport::SetReceiveState(RecvState recv_state) noexcept { AssertLockHeld(m_recv_mutex); @@ -1087,9 +1107,10 @@ void V2Transport::ProcessReceivedMaybeV1Bytes() noexcept if (!std::equal(m_recv_buffer.begin(), m_recv_buffer.end(), v1_prefix.begin())) { // Mismatch with v1 prefix, so we can assume a v2 connection. SetReceiveState(RecvState::KEY); // Convert to KEY state, leaving received bytes around. - // Transition the sender to AWAITING_KEY state (if not already). + // Transition the sender to AWAITING_KEY state and start sending. LOCK(m_send_mutex); SetSendState(SendState::AWAITING_KEY); + StartSendingHandshake(); } else if (m_recv_buffer.size() == v1_prefix.size()) { // Full match with the v1 prefix, so fall back to v1 behavior. LOCK(m_send_mutex); @@ -1149,7 +1170,6 @@ bool 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(), @@ -1160,9 +1180,12 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION); m_cipher.Encrypt( /*contents=*/{}, - /*aad=*/MakeByteSpan(m_send_buffer).subspan(EllSwiftPubKey::size(), garbage_len), + /*aad=*/MakeByteSpan(m_send_garbage), /*ignore=*/false, /*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION)); + // We no longer need the garbage. + m_send_garbage.clear(); + m_send_garbage.shrink_to_fit(); // Construct version packet in the send buffer. m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION + VERSION_CONTENTS.size()); @@ -1532,9 +1555,7 @@ Transport::BytesToSend V2Transport::GetBytesToSend(bool have_next_message) const LOCK(m_send_mutex); if (m_send_state == SendState::V1) return m_v1_fallback.GetBytesToSend(have_next_message); - // We do not send anything in MAYBE_V1 state (as we don't know if the peer is v1 or v2), - // despite there being data in the send buffer in that state. - if (m_send_state == SendState::MAYBE_V1) return {{}, false, m_send_type}; + if (m_send_state == SendState::MAYBE_V1) Assume(m_send_buffer.empty()); Assume(m_send_pos <= m_send_buffer.size()); return { Span{m_send_buffer}.subspan(m_send_pos), @@ -1553,10 +1574,8 @@ void V2Transport::MarkBytesSent(size_t bytes_sent) noexcept m_send_pos += bytes_sent; Assume(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()) { + // Wipe the buffer when everything is sent. + if (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 cf7a240202e..e1d8995a8eb 100644 --- a/src/net.h +++ b/src/net.h @@ -540,25 +540,25 @@ class V2Transport final : public Transport enum class SendState : uint8_t { /** (Responder only) Not sending until v1 or v2 is detected. * - * This is the initial state for responders. The send buffer contains the public key to - * send, but nothing is sent in this state yet. When the receiver determines whether this + * This is the initial state for responders. The send buffer is empty. + * When the receiver determines whether this * is a V1 or V2 connection, the sender state becomes AWAITING_KEY (for v2) or V1 (for v1). */ MAYBE_V1, /** Waiting for the other side's public key. * - * This is the initial state for initiators. The public key is sent out. When the receiver - * receives the other side's public key and transitions to GARB_GARBTERM, the sender state - * becomes READY. */ + * This is the initial state for initiators. The public key and garbage is sent out. When + * the receiver receives the other side's public key and transitions to GARB_GARBTERM, the + * sender state becomes READY. */ AWAITING_KEY, /** Normal sending state. * * In this state, the ciphers are initialized, so packets can be sent. When this state is - * 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. */ + * entered, the garbage terminator, garbage authentication packet, and version + * packet are appended to the send buffer (in addition to the key and garbage 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. @@ -601,6 +601,8 @@ class V2Transport final : public Transport std::vector m_send_buffer GUARDED_BY(m_send_mutex); /** How many bytes from the send buffer have been sent so far. */ uint32_t m_send_pos GUARDED_BY(m_send_mutex) {0}; + /** The garbage sent, or to be sent (MAYBE_V1 and AWAITING_KEY state only). */ + std::vector m_send_garbage GUARDED_BY(m_send_mutex); /** Type of the message being sent. */ std::string m_send_type GUARDED_BY(m_send_mutex); /** Current sender state. */ @@ -614,6 +616,8 @@ class V2Transport final : public Transport static std::optional GetMessageType(Span& contents) noexcept; /** Determine how many received bytes can be processed in one go (not allowed in V1 state). */ size_t GetMaxBytesToProcess() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex); + /** Put our public key + garbage in the send buffer. */ + void StartSendingHandshake() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_send_mutex); /** Process bytes in m_recv_buffer, while in KEY_MAYBE_V1 state. */ void ProcessReceivedMaybeV1Bytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex); /** Process bytes in m_recv_buffer, while in KEY state. */ @@ -636,7 +640,7 @@ class V2Transport final : public Transport V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) 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; + V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32, std::vector 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 6e3ef2a707b..88d6e96eacb 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -366,7 +366,7 @@ std::unique_ptr MakeV2Transport(NodeId nodeid, bool initiator, RNG& r .Write(garb.data(), garb.size()) .Finalize(UCharCast(ent.data())); - return std::make_unique(nodeid, initiator, SER_NETWORK, INIT_PROTO_VERSION, key, ent, garb); + return std::make_unique(nodeid, initiator, SER_NETWORK, INIT_PROTO_VERSION, key, ent, std::move(garb)); } } // namespace diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 900e311d225..3eb7bdec62f 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1008,12 +1008,20 @@ BOOST_AUTO_TEST_CASE(advertise_local_address) namespace { +CKey GenerateRandomTestKey() noexcept +{ + CKey key; + uint256 key_data = InsecureRand256(); + key.Set(key_data.begin(), key_data.end(), true); + return key; +} + /** A class for scenario-based tests of V2Transport * * Each V2TransportTester encapsulates a V2Transport (the one being tested), and can be told to * interact with it. To do so, it also encapsulates a BIP324Cipher to act as the other side. A * second V2Transport is not used, as doing so would not permit scenarios that involve sending - * invalid data, or ones scenarios using BIP324 features that are not implemented on the sending + * invalid data, or ones using BIP324 features that are not implemented on the sending * side (like decoy packets). */ class V2TransportTester @@ -1031,6 +1039,7 @@ class V2TransportTester /** Construct a tester object. test_initiator: whether the tested transport is initiator. */ V2TransportTester(bool test_initiator) : m_transport(0, test_initiator, SER_NETWORK, INIT_PROTO_VERSION), + m_cipher{GenerateRandomTestKey(), MakeByteSpan(InsecureRand256())}, m_test_initiator(test_initiator) {} /** Data type returned by Interact: