From c3fad1f29df093e8fd03d70eb43f25ee9d531bf7 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 16 Aug 2023 13:21:35 -0400 Subject: [PATCH 01/10] net: add have_next_message argument to Transport::GetBytesToSend() Before this commit, there are only two possibly outcomes for the "more" prediction in Transport::GetBytesToSend(): * true: the transport itself has more to send, so the answer is certainly yes. * false: the transport has nothing further to send, but if vSendMsg has more message(s) left, that still will result in more wire bytes after the next SetMessageToSend(). For the BIP324 v2 transport, there will arguably be a third state: * definitely not: the transport has nothing further to send, but even if vSendMsg has more messages left, they can't be sent (right now). This happens before the handshake is complete. To implement this, we move the entire decision logic to the Transport, by adding a boolean to GetBytesToSend(), called have_next_message, which informs the transport whether more messages are available. The return values are still true and false, but they mean "definitely yes" and "definitely no", rather than "yes" and "maybe". --- src/net.cpp | 43 ++++++++++++------- src/net.h | 37 ++++++++++++---- src/test/denialofservice_tests.cpp | 4 +- src/test/fuzz/p2p_transport_serialization.cpp | 34 ++++++++++----- src/test/util/net.cpp | 4 +- 5 files changed, 85 insertions(+), 37 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 4addca09822..eaa99e66017 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -867,20 +867,22 @@ bool V1Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept return true; } -Transport::BytesToSend V1Transport::GetBytesToSend() const noexcept +Transport::BytesToSend V1Transport::GetBytesToSend(bool have_next_message) const noexcept { AssertLockNotHeld(m_send_mutex); LOCK(m_send_mutex); if (m_sending_header) { return {Span{m_header_to_send}.subspan(m_bytes_sent), - // We have more to send after the header if the message has payload. - !m_message_to_send.data.empty(), + // We have more to send after the header if the message has payload, or if there + // is a next message after that. + have_next_message || !m_message_to_send.data.empty(), m_message_to_send.m_type }; } else { return {Span{m_message_to_send.data}.subspan(m_bytes_sent), - // We never have more to send after this message's payload. - false, + // We only have more to send after this message's payload if there is another + // message. + have_next_message, m_message_to_send.m_type }; } @@ -916,6 +918,7 @@ std::pair CConnman::SocketSendData(CNode& node) const auto it = node.vSendMsg.begin(); size_t nSentSize = 0; bool data_left{false}; //!< second return value (whether unsent data remains) + std::optional expected_more; while (true) { if (it != node.vSendMsg.end()) { @@ -928,7 +931,12 @@ std::pair CConnman::SocketSendData(CNode& node) const ++it; } } - const auto& [data, more, msg_type] = node.m_transport->GetBytesToSend(); + const auto& [data, more, msg_type] = node.m_transport->GetBytesToSend(it != node.vSendMsg.end()); + // We rely on the 'more' value returned by GetBytesToSend to correctly predict whether more + // bytes are still to be sent, to correctly set the MSG_MORE flag. As a sanity check, + // verify that the previously returned 'more' was correct. + if (expected_more.has_value()) Assume(!data.empty() == *expected_more); + expected_more = more; data_left = !data.empty(); // will be overwritten on next loop if all of data gets sent int nBytes = 0; if (!data.empty()) { @@ -941,9 +949,7 @@ std::pair CConnman::SocketSendData(CNode& node) const } int flags = MSG_NOSIGNAL | MSG_DONTWAIT; #ifdef MSG_MORE - // We have more to send if either the transport itself has more, or if we have more - // messages to send. - if (more || it != node.vSendMsg.end()) { + if (more) { flags |= MSG_MORE; } #endif @@ -1323,9 +1329,10 @@ Sock::EventsPerSock CConnman::GenerateWaitSockets(Span nodes) { LOCK(pnode->cs_vSend); // Sending is possible if either there are bytes to send right now, or if there will be - // once a potential message from vSendMsg is handed to the transport. - const auto& [to_send, _more, _msg_type] = pnode->m_transport->GetBytesToSend(); - select_send = !to_send.empty() || !pnode->vSendMsg.empty(); + // once a potential message from vSendMsg is handed to the transport. GetBytesToSend + // determines both of these in a single call. + const auto& [to_send, more, _msg_type] = pnode->m_transport->GetBytesToSend(!pnode->vSendMsg.empty()); + select_send = !to_send.empty() || more; } if (!select_recv && !select_send) continue; @@ -3007,7 +3014,10 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) size_t nBytesSent = 0; { LOCK(pnode->cs_vSend); - const auto& [to_send, _more, _msg_type] = pnode->m_transport->GetBytesToSend(); + // Check if the transport still has unsent bytes, and indicate to it that we're about to + // give it a message to send. + const auto& [to_send, more, _msg_type] = + pnode->m_transport->GetBytesToSend(/*have_next_message=*/true); const bool queue_was_empty{to_send.empty() && pnode->vSendMsg.empty()}; // Update memory usage of send buffer. @@ -3016,10 +3026,13 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) // Move message to vSendMsg queue. pnode->vSendMsg.push_back(std::move(msg)); - // If there was nothing to send before, attempt "optimistic write": + // If there was nothing to send before, and there is now (predicted by the "more" value + // returned by the GetBytesToSend call above), attempt "optimistic write": // because the poll/select loop may pause for SELECT_TIMEOUT_MILLISECONDS before actually // doing a send, try sending from the calling thread if the queue was empty before. - if (queue_was_empty) { + // With a V1Transport, more will always be true here, because adding a message always + // results in sendable bytes there. + if (queue_was_empty && more) { std::tie(nBytesSent, std::ignore) = SocketSendData(*pnode); } } diff --git a/src/net.h b/src/net.h index 60a15fea556..1507ff7384e 100644 --- a/src/net.h +++ b/src/net.h @@ -308,19 +308,40 @@ class Transport { const std::string& /*m_type*/ >; - /** Get bytes to send on the wire. + /** Get bytes to send on the wire, if any, along with other information about it. * * As a const function, it does not modify the transport's observable state, and is thus safe * to be called multiple times. * - * The bytes returned by this function act as a stream which can only be appended to. This - * means that with the exception of MarkBytesSent, operations on the transport can only append - * to what is being returned. + * @param[in] have_next_message If true, the "more" return value reports whether more will + * be sendable after a SetMessageToSend call. It is set by the caller when they know + * they have another message ready to send, and only care about what happens + * after that. The have_next_message argument only affects this "more" return value + * and nothing else. * - * Note that m_type and to_send refer to data that is internal to the transport, and calling - * any non-const function on this object may invalidate them. + * Effectively, there are three possible outcomes about whether there are more bytes + * to send: + * - Yes: the transport itself has more bytes to send later. For example, for + * V1Transport this happens during the sending of the header of a + * message, when there is a non-empty payload that follows. + * - No: the transport itself has no more bytes to send, but will have bytes to + * send if handed a message through SetMessageToSend. In V1Transport this + * happens when sending the payload of a message. + * - Blocked: the transport itself has no more bytes to send, and is also incapable + * of sending anything more at all now, if it were handed another + * message to send. + * + * The boolean 'more' is true for Yes, false for Blocked, and have_next_message + * controls what is returned for No. + * + * @return a BytesToSend object. The to_send member returned acts as a stream which is only + * ever appended to. This means that with the exception of MarkBytesSent (which pops + * bytes off the front of later to_sends), operations on the transport can only append + * to what is being returned. Also note that m_type and to_send refer to data that is + * internal to the transport, and calling any non-const function on this object may + * invalidate them. */ - virtual BytesToSend GetBytesToSend() const noexcept = 0; + virtual BytesToSend GetBytesToSend(bool have_next_message) const noexcept = 0; /** Report how many bytes returned by the last GetBytesToSend() have been sent. * @@ -416,7 +437,7 @@ class V1Transport final : public Transport CNetMessage GetReceivedMessage(std::chrono::microseconds time, bool& reject_message) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); bool SetMessageToSend(CSerializedNetMsg& msg) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); - BytesToSend GetBytesToSend() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); + BytesToSend GetBytesToSend(bool have_next_message) const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); void MarkBytesSent(size_t bytes_sent) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); size_t GetSendMemoryUsage() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); }; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 7f5d587cf6f..8c1182b5e13 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { LOCK(dummyNode1.cs_vSend); - const auto& [to_send, _more, _msg_type] = dummyNode1.m_transport->GetBytesToSend(); + const auto& [to_send, _more, _msg_type] = dummyNode1.m_transport->GetBytesToSend(false); BOOST_CHECK(!to_send.empty()); } connman.FlushSendBuffer(dummyNode1); @@ -97,7 +97,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders { LOCK(dummyNode1.cs_vSend); - const auto& [to_send, _more, _msg_type] = dummyNode1.m_transport->GetBytesToSend(); + const auto& [to_send, _more, _msg_type] = dummyNode1.m_transport->GetBytesToSend(false); BOOST_CHECK(!to_send.empty()); } // Wait 3 more minutes diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index 2fa5de50088..468bb789ed9 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -92,7 +92,7 @@ FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serial assert(queued); std::optional known_more; while (true) { - const auto& [to_send, more, _msg_type] = send_transport.GetBytesToSend(); + const auto& [to_send, more, _msg_type] = send_transport.GetBytesToSend(false); if (known_more) assert(!to_send.empty() == *known_more); if (to_send.empty()) break; send_transport.MarkBytesSent(to_send.size()); @@ -124,11 +124,13 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa // Vectors with bytes last returned by GetBytesToSend() on transport[i]. std::array, 2> to_send; - // Last returned 'more' values (if still relevant) by transport[i]->GetBytesToSend(). - std::array, 2> last_more; + // Last returned 'more' values (if still relevant) by transport[i]->GetBytesToSend(), for + // both have_next_message false and true. + std::array, 2> last_more, last_more_next; - // Whether more bytes to be sent are expected on transport[i]. - std::array, 2> expect_more; + // Whether more bytes to be sent are expected on transport[i], before and after + // SetMessageToSend(). + std::array, 2> expect_more, expect_more_next; // Function to consume a message type. auto msg_type_fn = [&]() { @@ -177,18 +179,27 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa // Wrapper around transport[i]->GetBytesToSend() that performs sanity checks. auto bytes_to_send_fn = [&](int side) -> Transport::BytesToSend { - const auto& [bytes, more, msg_type] = transports[side]->GetBytesToSend(); + // Invoke GetBytesToSend twice (for have_next_message = {false, true}). This function does + // not modify state (it's const), and only the "more" return value should differ between + // the calls. + const auto& [bytes, more_nonext, msg_type] = transports[side]->GetBytesToSend(false); + const auto& [bytes_next, more_next, msg_type_next] = transports[side]->GetBytesToSend(true); // Compare with expected more. if (expect_more[side].has_value()) assert(!bytes.empty() == *expect_more[side]); + // Verify consistency between the two results. + assert(bytes == bytes_next); + assert(msg_type == msg_type_next); + if (more_nonext) assert(more_next); // Compare with previously reported output. assert(to_send[side].size() <= bytes.size()); assert(to_send[side] == Span{bytes}.first(to_send[side].size())); to_send[side].resize(bytes.size()); std::copy(bytes.begin(), bytes.end(), to_send[side].begin()); - // Remember 'more' result. - last_more[side] = {more}; + // Remember 'more' results. + last_more[side] = {more_nonext}; + last_more_next[side] = {more_next}; // Return. - return {bytes, more, msg_type}; + return {bytes, more_nonext, msg_type}; }; // Function to make side send a new message. @@ -199,7 +210,8 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa CSerializedNetMsg msg = next_msg[side].Copy(); bool queued = transports[side]->SetMessageToSend(msg); // Update expected more data. - expect_more[side] = std::nullopt; + expect_more[side] = expect_more_next[side]; + expect_more_next[side] = std::nullopt; // Verify consistency of GetBytesToSend after SetMessageToSend bytes_to_send_fn(/*side=*/side); if (queued) { @@ -223,6 +235,7 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa // If all to-be-sent bytes were sent, move last_more data to expect_more data. if (send_now == bytes.size()) { expect_more[side] = last_more[side]; + expect_more_next[side] = last_more_next[side]; } // Remove the bytes from the last reported to-be-sent vector. assert(to_send[side].size() >= send_now); @@ -251,6 +264,7 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa // Clear cached expected 'more' information: if certainly no more data was to be sent // before, receiving bytes makes this uncertain. if (expect_more[!side] == false) expect_more[!side] = std::nullopt; + if (expect_more_next[!side] == false) expect_more_next[!side] = std::nullopt; // Verify consistency of GetBytesToSend after ReceivedBytes bytes_to_send_fn(/*side=*/!side); bool progress = to_recv.size() < old_len; diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index 5696f8d13c2..dc64c0b4c1b 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -78,7 +78,7 @@ void ConnmanTestMsg::FlushSendBuffer(CNode& node) const node.vSendMsg.clear(); node.m_send_memusage = 0; while (true) { - const auto& [to_send, _more, _msg_type] = node.m_transport->GetBytesToSend(); + const auto& [to_send, _more, _msg_type] = node.m_transport->GetBytesToSend(false); if (to_send.empty()) break; node.m_transport->MarkBytesSent(to_send.size()); } @@ -90,7 +90,7 @@ bool ConnmanTestMsg::ReceiveMsgFrom(CNode& node, CSerializedNetMsg&& ser_msg) co assert(queued); bool complete{false}; while (true) { - const auto& [to_send, _more, _msg_type] = node.m_transport->GetBytesToSend(); + const auto& [to_send, _more, _msg_type] = node.m_transport->GetBytesToSend(false); if (to_send.empty()) break; NodeReceiveMsgBytes(node, to_send, complete); node.m_transport->MarkBytesSent(to_send.size()); From 5f4b2c6d79e81ee0445752ad558fcc17831f4b2f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 30 Aug 2023 16:10:58 -0400 Subject: [PATCH 02/10] net: remove unused Transport::SetReceiveVersion --- src/net.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/net.h b/src/net.h index 1507ff7384e..6f989aa1754 100644 --- a/src/net.h +++ b/src/net.h @@ -266,8 +266,6 @@ class Transport { /** Returns true if the current message is complete (so GetReceivedMessage can be called). */ virtual bool ReceivedMessageComplete() const = 0; - /** Set the deserialization context version for objects returned by GetReceivedMessage. */ - virtual void SetReceiveVersion(int version) = 0; /** Feed wire bytes to the transport. * @@ -413,14 +411,6 @@ class V1Transport final : public Transport return WITH_LOCK(m_recv_mutex, return CompleteInternal()); } - void SetReceiveVersion(int nVersionIn) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) - { - AssertLockNotHeld(m_recv_mutex); - LOCK(m_recv_mutex); - hdrbuf.SetVersion(nVersionIn); - vRecv.SetVersion(nVersionIn); - } - bool ReceivedBytes(Span& msg_bytes) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) { AssertLockNotHeld(m_recv_mutex); From dc2d7eb810ef95b06620f334c198687579916435 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 31 Aug 2023 10:00:05 -0400 Subject: [PATCH 03/10] crypto: Spanify EllSwiftPubKey constructor --- src/pubkey.cpp | 6 ++++++ src/pubkey.h | 3 +-- src/test/bip324_tests.cpp | 10 ++-------- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/pubkey.cpp b/src/pubkey.cpp index 4866feed67a..05808e4c227 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -336,6 +336,12 @@ bool CPubKey::Derive(CPubKey& pubkeyChild, ChainCode &ccChild, unsigned int nChi return true; } +EllSwiftPubKey::EllSwiftPubKey(Span ellswift) noexcept +{ + assert(ellswift.size() == SIZE); + std::copy(ellswift.begin(), ellswift.end(), m_pubkey.begin()); +} + CPubKey EllSwiftPubKey::Decode() const { secp256k1_pubkey pubkey; diff --git a/src/pubkey.h b/src/pubkey.h index 00defa25a01..274779f9a47 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -303,8 +303,7 @@ struct EllSwiftPubKey EllSwiftPubKey() noexcept = default; /** Construct a new ellswift public key from a given serialization. */ - EllSwiftPubKey(const std::array& ellswift) : - m_pubkey(ellswift) {} + EllSwiftPubKey(Span ellswift) noexcept; /** Decode to normal compressed CPubKey (for debugging purposes). */ CPubKey Decode() const; diff --git a/src/test/bip324_tests.cpp b/src/test/bip324_tests.cpp index 04472611ec6..1ed7e23bcf9 100644 --- a/src/test/bip324_tests.cpp +++ b/src/test/bip324_tests.cpp @@ -38,14 +38,8 @@ void TestBIP324PacketVector( { // Convert input from hex to char/byte vectors/arrays. const auto in_priv_ours = ParseHex(in_priv_ours_hex); - const auto in_ellswift_ours_vec = ParseHex(in_ellswift_ours_hex); - assert(in_ellswift_ours_vec.size() == 64); - std::array in_ellswift_ours; - std::copy(in_ellswift_ours_vec.begin(), in_ellswift_ours_vec.end(), in_ellswift_ours.begin()); - const auto in_ellswift_theirs_vec = ParseHex(in_ellswift_theirs_hex); - assert(in_ellswift_theirs_vec.size() == 64); - std::array in_ellswift_theirs; - std::copy(in_ellswift_theirs_vec.begin(), in_ellswift_theirs_vec.end(), in_ellswift_theirs.begin()); + const auto in_ellswift_ours = ParseHex(in_ellswift_ours_hex); + const auto in_ellswift_theirs = ParseHex(in_ellswift_theirs_hex); const auto in_contents = ParseHex(in_contents_hex); const auto in_aad = ParseHex(in_aad_hex); const auto mid_send_garbage = ParseHex(mid_send_garbage_hex); From 13a7f01557272db652b3f333af3f06af6897253f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 27 Jul 2023 15:10:34 -0400 Subject: [PATCH 04/10] net: add V2Transport class with subset of BIP324 functionality This introduces a V2Transport with a basic subset of BIP324 functionality: * no ability to send garbage (but receiving is supported) * no ability to send decoy packets (but receiving them is supported) * no support for short message id encoding (neither encoding or decoding) * no waiting until 12 non-V1 bytes have been received * (and thus) no detection of V1 connections on the responder side (on the sender side, detecting V1 is not supported either, but that needs to be dealt with at a higher layer, by reconnecting) --- src/net.cpp | 427 +++++++++++++++++- src/net.h | 181 +++++++- src/test/fuzz/p2p_transport_serialization.cpp | 25 + 3 files changed, 629 insertions(+), 4 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index eaa99e66017..f5425bf50e0 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -913,6 +913,427 @@ size_t V1Transport::GetSendMemoryUsage() const noexcept return m_message_to_send.GetMemoryUsage(); } +V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept : + m_cipher{}, m_initiating{initiating}, m_nodeid{nodeid}, + m_recv_type{type_in}, m_recv_version{version_in}, + m_recv_state{RecvState::KEY}, + m_send_state{SendState::AWAITING_KEY} +{ + // Initialize the send buffer with ellswift pubkey. + m_send_buffer.resize(EllSwiftPubKey::size()); + std::copy(std::begin(m_cipher.GetOurPubKey()), std::end(m_cipher.GetOurPubKey()), MakeWritableByteSpan(m_send_buffer).begin()); +} + +V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32) noexcept : + m_cipher{key, ent32}, m_initiating{initiating}, m_nodeid{nodeid}, + m_recv_type{type_in}, m_recv_version{version_in}, + m_recv_state{RecvState::KEY}, + m_send_state{SendState::AWAITING_KEY} +{ + // Initialize the send buffer with ellswift pubkey. + m_send_buffer.resize(EllSwiftPubKey::size()); + std::copy(std::begin(m_cipher.GetOurPubKey()), std::end(m_cipher.GetOurPubKey()), MakeWritableByteSpan(m_send_buffer).begin()); +} + +void V2Transport::SetReceiveState(RecvState recv_state) noexcept +{ + AssertLockHeld(m_recv_mutex); + // Enforce allowed state transitions. + switch (m_recv_state) { + case RecvState::KEY: + Assume(recv_state == RecvState::GARB_GARBTERM); + break; + case RecvState::GARB_GARBTERM: + Assume(recv_state == RecvState::GARBAUTH); + break; + case RecvState::GARBAUTH: + Assume(recv_state == RecvState::VERSION); + break; + case RecvState::VERSION: + Assume(recv_state == RecvState::APP); + break; + case RecvState::APP: + Assume(recv_state == RecvState::APP_READY); + break; + case RecvState::APP_READY: + Assume(recv_state == RecvState::APP); + break; + } + // Change state. + m_recv_state = recv_state; +} + +void V2Transport::SetSendState(SendState send_state) noexcept +{ + AssertLockHeld(m_send_mutex); + // Enforce allowed state transitions. + switch (m_send_state) { + case SendState::AWAITING_KEY: + Assume(send_state == SendState::READY); + break; + case SendState::READY: + Assume(false); // Final state + break; + } + // Change state. + m_send_state = send_state; +} + +bool V2Transport::ReceivedMessageComplete() const noexcept +{ + AssertLockNotHeld(m_recv_mutex); + LOCK(m_recv_mutex); + return m_recv_state == RecvState::APP_READY; +} + +void V2Transport::ProcessReceivedKeyBytes() noexcept +{ + AssertLockHeld(m_recv_mutex); + AssertLockNotHeld(m_send_mutex); + Assume(m_recv_state == RecvState::KEY); + Assume(m_recv_buffer.size() <= EllSwiftPubKey::size()); + if (m_recv_buffer.size() == EllSwiftPubKey::size()) { + // Other side's key has been fully received, and can now be Diffie-Hellman combined with + // our key to initialize the encryption ciphers. + + // Initialize the ciphers. + EllSwiftPubKey ellswift(MakeByteSpan(m_recv_buffer)); + LOCK(m_send_mutex); + m_cipher.Initialize(ellswift, m_initiating); + + // Switch receiver state to GARB_GARBTERM. + SetReceiveState(RecvState::GARB_GARBTERM); + m_recv_buffer.clear(); + + // Switch sender state to READY. + SetSendState(SendState::READY); + + // Append the garbage terminator to the send buffer. + 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. + m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION); + m_cipher.Encrypt( + /*contents=*/{}, + /*aad=*/{}, /* empty garbage for now */ + /*ignore=*/false, + /*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION)); + + // Construct version packet in the send buffer. + m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION + VERSION_CONTENTS.size()); + m_cipher.Encrypt( + /*contents=*/VERSION_CONTENTS, + /*aad=*/{}, + /*ignore=*/false, + /*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION + VERSION_CONTENTS.size())); + } else { + // We still have to receive more key bytes. + } +} + +bool V2Transport::ProcessReceivedGarbageBytes() noexcept +{ + AssertLockHeld(m_recv_mutex); + Assume(m_recv_state == RecvState::GARB_GARBTERM); + Assume(m_recv_buffer.size() <= MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN); + if (m_recv_buffer.size() >= BIP324Cipher::GARBAGE_TERMINATOR_LEN) { + if (MakeByteSpan(m_recv_buffer).last(BIP324Cipher::GARBAGE_TERMINATOR_LEN) == m_cipher.GetReceiveGarbageTerminator()) { + // Garbage terminator received. Switch to receiving garbage authentication packet. + m_recv_garbage = std::move(m_recv_buffer); + m_recv_garbage.resize(m_recv_garbage.size() - BIP324Cipher::GARBAGE_TERMINATOR_LEN); + m_recv_buffer.clear(); + SetReceiveState(RecvState::GARBAUTH); + } else if (m_recv_buffer.size() == MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN) { + // We've reached the maximum length for garbage + garbage terminator, and the + // terminator still does not match. Abort. + LogPrint(BCLog::NET, "V2 transport error: missing garbage terminator, peer=%d\n", m_nodeid); + return false; + } else { + // We still need to receive more garbage and/or garbage terminator bytes. + } + } else { + // We have less than GARBAGE_TERMINATOR_LEN (16) bytes, so we certainly need to receive + // more first. + } + return true; +} + +bool V2Transport::ProcessReceivedPacketBytes() noexcept +{ + AssertLockHeld(m_recv_mutex); + Assume(m_recv_state == RecvState::GARBAUTH || m_recv_state == RecvState::VERSION || + m_recv_state == RecvState::APP); + + // The maximum permitted contents length for a packet, consisting of: + // - 0x00 byte: indicating long message type encoding + // - 12 bytes of message type + // - payload + static constexpr size_t MAX_CONTENTS_LEN = + 1 + CMessageHeader::COMMAND_SIZE + + std::min(MAX_SIZE, MAX_PROTOCOL_MESSAGE_LENGTH); + + if (m_recv_buffer.size() == BIP324Cipher::LENGTH_LEN) { + // Length descriptor received. + m_recv_len = m_cipher.DecryptLength(MakeByteSpan(m_recv_buffer)); + if (m_recv_len > MAX_CONTENTS_LEN) { + LogPrint(BCLog::NET, "V2 transport error: packet too large (%u bytes), peer=%d\n", m_recv_len, m_nodeid); + return false; + } + } else if (m_recv_buffer.size() > BIP324Cipher::LENGTH_LEN && m_recv_buffer.size() == m_recv_len + BIP324Cipher::EXPANSION) { + // Ciphertext received, decrypt it into m_recv_decode_buffer. + // Note that it is impossible to reach this branch without hitting the branch above first, + // as GetMaxBytesToProcess only allows up to LENGTH_LEN into the buffer before that point. + m_recv_decode_buffer.resize(m_recv_len); + bool ignore{false}; + Span aad; + if (m_recv_state == RecvState::GARBAUTH) aad = MakeByteSpan(m_recv_garbage); + bool ret = m_cipher.Decrypt( + /*input=*/MakeByteSpan(m_recv_buffer).subspan(BIP324Cipher::LENGTH_LEN), + /*aad=*/aad, + /*ignore=*/ignore, + /*contents=*/MakeWritableByteSpan(m_recv_decode_buffer)); + if (!ret) { + LogPrint(BCLog::NET, "V2 transport error: packet decryption failure (%u bytes), peer=%d\n", m_recv_len, m_nodeid); + return false; + } + // Feed the last 4 bytes of the Poly1305 authentication tag (and its timing) into our RNG. + RandAddEvent(ReadLE32(m_recv_buffer.data() + m_recv_buffer.size() - 4)); + + // At this point we have a valid packet decrypted into m_recv_decode_buffer. Depending on + // the current state, decide what to do with it. + switch (m_recv_state) { + case RecvState::GARBAUTH: + // Ignore flag does not matter for garbage authentication. Any valid packet functions + // as authentication. Receive and process the version packet next. + SetReceiveState(RecvState::VERSION); + m_recv_garbage = {}; + break; + case RecvState::VERSION: + if (!ignore) { + // Version message received; transition to application phase. The contents is + // ignored, but can be used for future extensions. + SetReceiveState(RecvState::APP); + } + break; + case RecvState::APP: + if (!ignore) { + // Application message decrypted correctly. It can be extracted using GetMessage(). + SetReceiveState(RecvState::APP_READY); + } + break; + default: + // Any other state is invalid (this function should not have been called). + Assume(false); + } + // Wipe the receive buffer where the next packet will be received into. + m_recv_buffer = {}; + // In all but APP_READY state, we can wipe the decoded contents. + if (m_recv_state != RecvState::APP_READY) m_recv_decode_buffer = {}; + } else { + // We either have less than 3 bytes, so we don't know the packet's length yet, or more + // than 3 bytes but less than the packet's full ciphertext. Wait until those arrive. + } + return true; +} + +size_t V2Transport::GetMaxBytesToProcess() noexcept +{ + AssertLockHeld(m_recv_mutex); + switch (m_recv_state) { + case RecvState::KEY: + // During the KEY state, we only allow the 64-byte key into the receive buffer. + Assume(m_recv_buffer.size() <= EllSwiftPubKey::size()); + // As long as we have not received the other side's public key, don't receive more than + // that (64 bytes), as garbage follows, and locating the garbage terminator requires the + // key exchange first. + return EllSwiftPubKey::size() - m_recv_buffer.size(); + case RecvState::GARB_GARBTERM: + // Process garbage bytes one by one (because terminator may appear anywhere). + return 1; + case RecvState::GARBAUTH: + case RecvState::VERSION: + case RecvState::APP: + // These three states all involve decoding a packet. Process the length descriptor first, + // so that we know where the current packet ends (and we don't process bytes from the next + // packet or decoy yet). Then, process the ciphertext bytes of the current packet. + if (m_recv_buffer.size() < BIP324Cipher::LENGTH_LEN) { + return BIP324Cipher::LENGTH_LEN - m_recv_buffer.size(); + } else { + // Note that BIP324Cipher::EXPANSION is the total difference between contents size + // and encoded packet size, which includes the 3 bytes due to the packet length. + // When transitioning from receiving the packet length to receiving its ciphertext, + // the encrypted packet length is left in the receive buffer. + return BIP324Cipher::EXPANSION + m_recv_len - m_recv_buffer.size(); + } + case RecvState::APP_READY: + // No bytes can be processed until GetMessage() is called. + return 0; + } + Assume(false); // unreachable + return 0; +} + +bool V2Transport::ReceivedBytes(Span& msg_bytes) noexcept +{ + AssertLockNotHeld(m_recv_mutex); + LOCK(m_recv_mutex); + // Process the provided bytes in msg_bytes in a loop. In each iteration a nonzero number of + // bytes (decided by GetMaxBytesToProcess) are taken from the beginning om msg_bytes, and + // appended to m_recv_buffer. Then, depending on the receiver state, one of the + // ProcessReceived*Bytes functions is called to process the bytes in that buffer. + while (!msg_bytes.empty()) { + // Decide how many bytes to copy from msg_bytes to m_recv_buffer. + size_t max_read = GetMaxBytesToProcess(); + // Can't read more than provided input. + max_read = std::min(msg_bytes.size(), max_read); + // Copy data to buffer. + m_recv_buffer.insert(m_recv_buffer.end(), UCharCast(msg_bytes.data()), UCharCast(msg_bytes.data() + max_read)); + msg_bytes = msg_bytes.subspan(max_read); + + // Process data in the buffer. + switch (m_recv_state) { + case RecvState::KEY: + ProcessReceivedKeyBytes(); + break; + + case RecvState::GARB_GARBTERM: + if (!ProcessReceivedGarbageBytes()) return false; + break; + + case RecvState::GARBAUTH: + case RecvState::VERSION: + case RecvState::APP: + if (!ProcessReceivedPacketBytes()) return false; + break; + + case RecvState::APP_READY: + return true; + } + // Make sure we have made progress before continuing. + Assume(max_read > 0); + } + + return true; +} + +std::optional V2Transport::GetMessageType(Span& contents) noexcept +{ + if (contents.size() == 0) return std::nullopt; // Empty contents + uint8_t first_byte = contents[0]; + contents = contents.subspan(1); // Strip first byte. + + if (first_byte != 0) return std::nullopt; // TODO: implement short encoding + + if (contents.size() < CMessageHeader::COMMAND_SIZE) { + return std::nullopt; // Long encoding needs 12 message type bytes. + } + + size_t msg_type_len{0}; + while (msg_type_len < CMessageHeader::COMMAND_SIZE && contents[msg_type_len] != 0) { + // Verify that message type bytes before the first 0x00 are in range. + if (contents[msg_type_len] < ' ' || contents[msg_type_len] > 0x7F) { + return {}; + } + ++msg_type_len; + } + std::string ret{reinterpret_cast(contents.data()), msg_type_len}; + while (msg_type_len < CMessageHeader::COMMAND_SIZE) { + // Verify that message type bytes after the first 0x00 are also 0x00. + if (contents[msg_type_len] != 0) return {}; + ++msg_type_len; + } + // Strip message type bytes of contents. + contents = contents.subspan(CMessageHeader::COMMAND_SIZE); + return {std::move(ret)}; +} + +CNetMessage V2Transport::GetReceivedMessage(std::chrono::microseconds time, bool& reject_message) noexcept +{ + AssertLockNotHeld(m_recv_mutex); + LOCK(m_recv_mutex); + Assume(m_recv_state == RecvState::APP_READY); + Span contents{m_recv_decode_buffer}; + auto msg_type = GetMessageType(contents); + CDataStream ret(m_recv_type, m_recv_version); + CNetMessage msg{std::move(ret)}; + // Note that BIP324Cipher::EXPANSION also includes the length descriptor size. + msg.m_raw_message_size = m_recv_decode_buffer.size() + BIP324Cipher::EXPANSION; + if (msg_type) { + reject_message = false; + msg.m_type = std::move(*msg_type); + msg.m_time = time; + msg.m_message_size = contents.size(); + msg.m_recv.resize(contents.size()); + std::copy(contents.begin(), contents.end(), UCharCast(msg.m_recv.data())); + } else { + LogPrint(BCLog::NET, "V2 transport error: invalid message type (%u bytes contents), peer=%d\n", m_recv_decode_buffer.size(), m_nodeid); + reject_message = true; + } + m_recv_decode_buffer = {}; + SetReceiveState(RecvState::APP); + + return msg; +} + +bool V2Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept +{ + AssertLockNotHeld(m_send_mutex); + LOCK(m_send_mutex); + // We only allow adding a new message to be sent when in the READY state (so the packet cipher + // is available) and the send buffer is empty. This limits the number of messages in the send + // buffer to just one, and leaves the responsibility for queueing them up to the caller. + if (!(m_send_state == SendState::READY && m_send_buffer.empty())) return false; + // Construct contents (encoding message type + payload). + // Initialize with zeroes, and then write the message type string starting at offset 1. + // This means contents[0] and the unused positions in contents[1..13] remain 0x00. + std::vector contents(1 + CMessageHeader::COMMAND_SIZE + msg.data.size(), 0); + std::copy(msg.m_type.begin(), msg.m_type.end(), contents.data() + 1); + std::copy(msg.data.begin(), msg.data.end(), contents.begin() + 1 + CMessageHeader::COMMAND_SIZE); + // Construct ciphertext in send buffer. + m_send_buffer.resize(contents.size() + BIP324Cipher::EXPANSION); + m_cipher.Encrypt(MakeByteSpan(contents), {}, false, MakeWritableByteSpan(m_send_buffer)); + m_send_type = msg.m_type; + // Release memory + msg.data = {}; + return true; +} + +Transport::BytesToSend V2Transport::GetBytesToSend(bool have_next_message) const noexcept +{ + AssertLockNotHeld(m_send_mutex); + LOCK(m_send_mutex); + Assume(m_send_pos <= m_send_buffer.size()); + return { + Span{m_send_buffer}.subspan(m_send_pos), + // We only have more to send after the current m_send_buffer if there is a (next) + // message to be sent, and we're capable of sending packets. */ + have_next_message && m_send_state == SendState::READY, + m_send_type + }; +} + +void V2Transport::MarkBytesSent(size_t bytes_sent) noexcept +{ + AssertLockNotHeld(m_send_mutex); + LOCK(m_send_mutex); + m_send_pos += bytes_sent; + Assume(m_send_pos <= m_send_buffer.size()); + if (m_send_pos == m_send_buffer.size()) { + m_send_pos = 0; + m_send_buffer = {}; + } +} + +size_t V2Transport::GetSendMemoryUsage() const noexcept +{ + AssertLockNotHeld(m_send_mutex); + LOCK(m_send_mutex); + return sizeof(m_send_buffer) + memusage::DynamicUsage(m_send_buffer); +} + std::pair CConnman::SocketSendData(CNode& node) const { auto it = node.vSendMsg.begin(); @@ -923,7 +1344,8 @@ std::pair CConnman::SocketSendData(CNode& node) const while (true) { if (it != node.vSendMsg.end()) { // If possible, move one message from the send queue to the transport. This fails when - // there is an existing message still being sent. + // there is an existing message still being sent, or (for v2 transports) when the + // handshake has not yet completed. size_t memusage = it->GetMemoryUsage(); if (node.m_transport->SetMessageToSend(*it)) { // Update memory usage of send buffer (as *it will be deleted). @@ -3031,7 +3453,8 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) // because the poll/select loop may pause for SELECT_TIMEOUT_MILLISECONDS before actually // doing a send, try sending from the calling thread if the queue was empty before. // With a V1Transport, more will always be true here, because adding a message always - // results in sendable bytes there. + // results in sendable bytes there, but with V2Transport this is not the case (it may + // still be in the handshake). if (queue_was_empty && more) { std::tie(nBytesSent, std::ignore) = SocketSendData(*pnode); } diff --git a/src/net.h b/src/net.h index 6f989aa1754..27d141bc6e6 100644 --- a/src/net.h +++ b/src/net.h @@ -6,6 +6,7 @@ #ifndef BITCOIN_NET_H #define BITCOIN_NET_H +#include #include #include #include @@ -298,7 +299,8 @@ class Transport { * - Span to_send: span of bytes to be sent over the wire (possibly empty). * - bool more: whether there will be more bytes to be sent after the ones in to_send are * all sent (as signaled by MarkBytesSent()). - * - const std::string& m_type: message type on behalf of which this is being sent. + * - const std::string& m_type: message type on behalf of which this is being sent + * ("" for bytes that are not on behalf of any message). */ using BytesToSend = std::tuple< Span /*to_send*/, @@ -327,7 +329,9 @@ class Transport { * happens when sending the payload of a message. * - Blocked: the transport itself has no more bytes to send, and is also incapable * of sending anything more at all now, if it were handed another - * message to send. + * message to send. This occurs in V2Transport before the handshake is + * complete, as the encryption ciphers are not set up for sending + * messages before that point. * * The boolean 'more' is true for Yes, false for Blocked, and have_next_message * controls what is returned for No. @@ -432,6 +436,179 @@ class V1Transport final : public Transport size_t GetSendMemoryUsage() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); }; +class V2Transport final : public Transport +{ +private: + /** Contents of the version packet to send. BIP324 stipulates that senders should leave this + * empty, and receivers should ignore it. Future extensions can change what is sent as long as + * an empty version packet contents is interpreted as no extensions supported. */ + static constexpr std::array VERSION_CONTENTS = {}; + + // The sender side and receiver side of V2Transport are state machines that are transitioned + // through, based on what has been received. The receive state corresponds to the contents of, + // and bytes received to, the receive buffer. The send state controls what can be appended to + // the send buffer. + + /** State type that defines the current contents of the receive buffer and/or how the next + * received bytes added to it will be interpreted. + * + * Diagram: + * + * start /---------\ + * | | | + * v v | + * KEY -> GARB_GARBTERM -> GARBAUTH -> VERSION -> APP -> APP_READY + */ + enum class RecvState : uint8_t { + /** Public key. + * + * This is the initial state, during which the other side's public key is + * received. When that information arrives, the ciphers get initialized and the state + * becomes GARB_GARBTERM. */ + KEY, + + /** Garbage and garbage terminator. + * + * Whenever a byte is received, the last 16 bytes are compared with the expected garbage + * terminator. When that happens, the state becomes GARBAUTH. If no matching terminator is + * received in 4111 bytes (4095 for the maximum garbage length, and 16 bytes for the + * terminator), the connection aborts. */ + GARB_GARBTERM, + + /** Garbage authentication packet. + * + * A packet is received, and decrypted/verified with AAD set to the garbage received during + * the GARB_GARBTERM state. If that succeeds, the state becomes VERSION. If it fails the + * connection aborts. */ + GARBAUTH, + + /** Version packet. + * + * A packet is received, and decrypted/verified. If that succeeds, the state becomes APP, + * and the decrypted contents is interpreted as version negotiation (currently, that means + * ignoring it, but it can be used for negotiating future extensions). If it fails, the + * connection aborts. */ + VERSION, + + /** Application packet. + * + * A packet is received, and decrypted/verified. If that succeeds, the state becomes + * APP_READY and the decrypted contents is kept in m_recv_decode_buffer until it is + * retrieved as a message by GetMessage(). */ + APP, + + /** Nothing (an application packet is available for GetMessage()). + * + * Nothing can be received in this state. When the message is retrieved by GetMessage, + * the state becomes APP again. */ + APP_READY, + }; + + /** State type that controls the sender side. + * + * Diagram: + * + * start + * | + * v + * AWAITING_KEY -> READY + */ + enum class SendState : uint8_t { + /** Waiting for the other side's public key. + * + * This is the initial state. 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. */ + AWAITING_KEY, + + /** 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. */ + READY, + }; + + /** Cipher state. */ + BIP324Cipher m_cipher; + /** Whether we are the initiator side. */ + const bool m_initiating; + /** NodeId (for debug logging). */ + const NodeId m_nodeid; + + /** Lock for receiver-side fields. */ + mutable Mutex m_recv_mutex ACQUIRED_BEFORE(m_send_mutex); + /** In {GARBAUTH, VERSION, APP}, the decrypted packet length, if m_recv_buffer.size() >= + * BIP324Cipher::LENGTH_LEN. Unspecified otherwise. */ + uint32_t m_recv_len GUARDED_BY(m_recv_mutex) {0}; + /** Receive buffer; meaning is determined by m_recv_state. */ + std::vector m_recv_buffer GUARDED_BY(m_recv_mutex); + /** During GARBAUTH, the garbage received during GARB_GARBTERM. */ + std::vector m_recv_garbage GUARDED_BY(m_recv_mutex); + /** Buffer to put decrypted contents in, for converting to CNetMessage. */ + std::vector m_recv_decode_buffer GUARDED_BY(m_recv_mutex); + /** Deserialization type. */ + const int m_recv_type; + /** Deserialization version number. */ + const int m_recv_version; + /** Current receiver state. */ + RecvState m_recv_state GUARDED_BY(m_recv_mutex); + + /** Lock for sending-side fields. If both sending and receiving fields are accessed, + * m_recv_mutex must be acquired before m_send_mutex. */ + mutable Mutex m_send_mutex ACQUIRED_AFTER(m_recv_mutex); + /** The send buffer; meaning is determined by m_send_state. */ + 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}; + /** Type of the message being sent. */ + std::string m_send_type GUARDED_BY(m_send_mutex); + /** Current sender state. */ + SendState m_send_state GUARDED_BY(m_send_mutex); + + /** Change the receive state. */ + void SetReceiveState(RecvState recv_state) noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex); + /** Change the send state. */ + void SetSendState(SendState send_state) noexcept EXCLUSIVE_LOCKS_REQUIRED(m_send_mutex); + /** Given a packet's contents, find the message type (if valid), and strip it from contents. */ + 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); + /** Process bytes in m_recv_buffer, while in KEY state. */ + void ProcessReceivedKeyBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex); + /** Process bytes in m_recv_buffer, while in GARB_GARBTERM state. */ + bool ProcessReceivedGarbageBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex); + /** Process bytes in m_recv_buffer, while in GARBAUTH/VERSION/APP state. */ + bool ProcessReceivedPacketBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex); + +public: + static constexpr uint32_t MAX_GARBAGE_LEN = 4095; + + /** Construct a V2 transport with securely generated random keys. + * + * @param[in] nodeid the node's NodeId (only for debug log output). + * @param[in] initiating whether we are the initiator side. + * @param[in] type_in the serialization type of returned CNetMessages. + * @param[in] version_in the serialization version of returned CNetMessages. + */ + 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; + + // Receive side functions. + bool ReceivedMessageComplete() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); + bool ReceivedBytes(Span& msg_bytes) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex, !m_send_mutex); + CNetMessage GetReceivedMessage(std::chrono::microseconds time, bool& reject_message) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); + + // Send side functions. + bool SetMessageToSend(CSerializedNetMsg& msg) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); + BytesToSend GetBytesToSend(bool have_next_message) const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); + void MarkBytesSent(size_t bytes_sent) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); + size_t GetSendMemoryUsage() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex); +}; + struct CNodeOptions { NetPermissionFlags permission_flags = NetPermissionFlags::None; diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index 468bb789ed9..f9454eab695 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -25,6 +25,7 @@ std::vector g_all_messages; void initialize_p2p_transport_serialization() { + ECC_Start(); SelectParams(ChainType::REGTEST); g_all_messages = getAllNetMessageTypes(); std::sort(g_all_messages.begin(), g_all_messages.end()); @@ -334,6 +335,19 @@ std::unique_ptr MakeV1Transport(NodeId nodeid) noexcept return std::make_unique(nodeid, SER_NETWORK, INIT_PROTO_VERSION); } +template +std::unique_ptr MakeV2Transport(NodeId nodeid, bool initiator, RNG& rng, FuzzedDataProvider& provider) +{ + // Retrieve key + auto key = ConsumePrivateKey(provider); + if (!key.IsValid()) return {}; + // Retrieve entropy + auto ent = provider.ConsumeBytes(32); + ent.resize(32); + + return std::make_unique(nodeid, initiator, SER_NETWORK, INIT_PROTO_VERSION, key, ent); +} + } // namespace FUZZ_TARGET(p2p_transport_bidirectional, .init = initialize_p2p_transport_serialization) @@ -346,3 +360,14 @@ FUZZ_TARGET(p2p_transport_bidirectional, .init = initialize_p2p_transport_serial if (!t1 || !t2) return; SimulationTest(*t1, *t2, rng, provider); } + +FUZZ_TARGET(p2p_transport_bidirectional_v2, .init = initialize_p2p_transport_serialization) +{ + // Test with two V2 transports talking to each other. + FuzzedDataProvider provider{buffer.data(), buffer.size()}; + XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral()); + auto t1 = MakeV2Transport(NodeId{0}, true, rng, provider); + auto t2 = MakeV2Transport(NodeId{1}, false, rng, provider); + if (!t1 || !t2) return; + SimulationTest(*t1, *t2, rng, provider); +} From 8da8642062fa2c7aa2f49995b832c3d0897e37ed Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 29 Aug 2023 22:37:18 -0400 Subject: [PATCH 05/10] net: make V2Transport auto-detect incoming V1 and fall back to it --- src/net.cpp | 100 ++++++++++++++++-- src/net.h | 67 +++++++++--- src/test/fuzz/p2p_transport_serialization.cpp | 11 ++ 3 files changed, 158 insertions(+), 20 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index f5425bf50e0..dbdbbf9d4e9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -915,9 +915,9 @@ size_t V1Transport::GetSendMemoryUsage() const noexcept V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept : m_cipher{}, m_initiating{initiating}, m_nodeid{nodeid}, - m_recv_type{type_in}, m_recv_version{version_in}, - m_recv_state{RecvState::KEY}, - m_send_state{SendState::AWAITING_KEY} + 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()); @@ -926,9 +926,9 @@ V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int versio V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32) noexcept : m_cipher{key, ent32}, m_initiating{initiating}, m_nodeid{nodeid}, - m_recv_type{type_in}, m_recv_version{version_in}, - m_recv_state{RecvState::KEY}, - m_send_state{SendState::AWAITING_KEY} + 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()); @@ -940,6 +940,9 @@ void V2Transport::SetReceiveState(RecvState recv_state) noexcept AssertLockHeld(m_recv_mutex); // Enforce allowed state transitions. switch (m_recv_state) { + case RecvState::KEY_MAYBE_V1: + Assume(recv_state == RecvState::KEY || recv_state == RecvState::V1); + break; case RecvState::KEY: Assume(recv_state == RecvState::GARB_GARBTERM); break; @@ -958,6 +961,9 @@ void V2Transport::SetReceiveState(RecvState recv_state) noexcept case RecvState::APP_READY: Assume(recv_state == RecvState::APP); break; + case RecvState::V1: + Assume(false); // V1 state cannot be left + break; } // Change state. m_recv_state = recv_state; @@ -968,11 +974,15 @@ void V2Transport::SetSendState(SendState send_state) noexcept AssertLockHeld(m_send_mutex); // Enforce allowed state transitions. switch (m_send_state) { + case SendState::MAYBE_V1: + Assume(send_state == SendState::V1 || send_state == SendState::AWAITING_KEY); + break; case SendState::AWAITING_KEY: Assume(send_state == SendState::READY); break; case SendState::READY: - Assume(false); // Final state + case SendState::V1: + Assume(false); // Final states break; } // Change state. @@ -983,9 +993,48 @@ bool V2Transport::ReceivedMessageComplete() const noexcept { AssertLockNotHeld(m_recv_mutex); LOCK(m_recv_mutex); + if (m_recv_state == RecvState::V1) return m_v1_fallback.ReceivedMessageComplete(); + return m_recv_state == RecvState::APP_READY; } +void V2Transport::ProcessReceivedMaybeV1Bytes() noexcept +{ + AssertLockHeld(m_recv_mutex); + AssertLockNotHeld(m_send_mutex); + Assume(m_recv_state == RecvState::KEY_MAYBE_V1); + // We still have to determine if this is a v1 or v2 connection. The bytes being received could + // be the beginning of either a v1 packet (network magic + "version\x00"), or of a v2 public + // key. BIP324 specifies that a mismatch with this 12-byte string should trigger sending of the + // key. + std::array v1_prefix = {0, 0, 0, 0, 'v', 'e', 'r', 's', 'i', 'o', 'n', 0}; + std::copy(std::begin(Params().MessageStart()), std::end(Params().MessageStart()), v1_prefix.begin()); + Assume(m_recv_buffer.size() <= v1_prefix.size()); + 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). + LOCK(m_send_mutex); + SetSendState(SendState::AWAITING_KEY); + } 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); + Span feedback{m_recv_buffer}; + // Feed already received bytes to v1 transport. It should always accept these, because it's + // less than the size of a v1 header, and these are the first bytes fed to m_v1_fallback. + bool ret = m_v1_fallback.ReceivedBytes(feedback); + Assume(feedback.empty()); + Assume(ret); + SetReceiveState(RecvState::V1); + SetSendState(SendState::V1); + // Reset v2 transport buffers to save memory. + m_recv_buffer = {}; + m_send_buffer = {}; + } else { + // We have not received enough to distinguish v1 from v2 yet. Wait until more bytes come. + } +} + void V2Transport::ProcessReceivedKeyBytes() noexcept { AssertLockHeld(m_recv_mutex); @@ -1143,6 +1192,15 @@ size_t V2Transport::GetMaxBytesToProcess() noexcept { AssertLockHeld(m_recv_mutex); switch (m_recv_state) { + case RecvState::KEY_MAYBE_V1: + // During the KEY_MAYBE_V1 state we do not allow more than the length of v1 prefix into the + // receive buffer. + Assume(m_recv_buffer.size() <= V1_PREFIX_LEN); + // As long as we're not sure if this is a v1 or v2 connection, don't receive more than what + // is strictly necessary to distinguish the two (12 bytes). If we permitted more than + // the v1 header size (24 bytes), we may not be able to feed the already-received bytes + // back into the m_v1_fallback V1 transport. + return V1_PREFIX_LEN - m_recv_buffer.size(); case RecvState::KEY: // During the KEY state, we only allow the 64-byte key into the receive buffer. Assume(m_recv_buffer.size() <= EllSwiftPubKey::size()); @@ -1171,6 +1229,10 @@ size_t V2Transport::GetMaxBytesToProcess() noexcept case RecvState::APP_READY: // No bytes can be processed until GetMessage() is called. return 0; + case RecvState::V1: + // Not allowed (must be dealt with by the caller). + Assume(false); + return 0; } Assume(false); // unreachable return 0; @@ -1180,6 +1242,8 @@ bool V2Transport::ReceivedBytes(Span& msg_bytes) noexcept { AssertLockNotHeld(m_recv_mutex); LOCK(m_recv_mutex); + if (m_recv_state == RecvState::V1) return m_v1_fallback.ReceivedBytes(msg_bytes); + // Process the provided bytes in msg_bytes in a loop. In each iteration a nonzero number of // bytes (decided by GetMaxBytesToProcess) are taken from the beginning om msg_bytes, and // appended to m_recv_buffer. Then, depending on the receiver state, one of the @@ -1195,6 +1259,11 @@ bool V2Transport::ReceivedBytes(Span& msg_bytes) noexcept // Process data in the buffer. switch (m_recv_state) { + case RecvState::KEY_MAYBE_V1: + ProcessReceivedMaybeV1Bytes(); + if (m_recv_state == RecvState::V1) return true; + break; + case RecvState::KEY: ProcessReceivedKeyBytes(); break; @@ -1211,6 +1280,11 @@ bool V2Transport::ReceivedBytes(Span& msg_bytes) noexcept case RecvState::APP_READY: return true; + + case RecvState::V1: + // We should have bailed out before. + Assume(false); + break; } // Make sure we have made progress before continuing. Assume(max_read > 0); @@ -1254,6 +1328,8 @@ CNetMessage V2Transport::GetReceivedMessage(std::chrono::microseconds time, bool { AssertLockNotHeld(m_recv_mutex); LOCK(m_recv_mutex); + if (m_recv_state == RecvState::V1) return m_v1_fallback.GetReceivedMessage(time, reject_message); + Assume(m_recv_state == RecvState::APP_READY); Span contents{m_recv_decode_buffer}; auto msg_type = GetMessageType(contents); @@ -1282,6 +1358,7 @@ bool V2Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept { AssertLockNotHeld(m_send_mutex); LOCK(m_send_mutex); + if (m_send_state == SendState::V1) return m_v1_fallback.SetMessageToSend(msg); // We only allow adding a new message to be sent when in the READY state (so the packet cipher // is available) and the send buffer is empty. This limits the number of messages in the send // buffer to just one, and leaves the responsibility for queueing them up to the caller. @@ -1305,6 +1382,11 @@ Transport::BytesToSend V2Transport::GetBytesToSend(bool have_next_message) const { AssertLockNotHeld(m_send_mutex); 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}; Assume(m_send_pos <= m_send_buffer.size()); return { Span{m_send_buffer}.subspan(m_send_pos), @@ -1319,6 +1401,8 @@ void V2Transport::MarkBytesSent(size_t bytes_sent) noexcept { AssertLockNotHeld(m_send_mutex); LOCK(m_send_mutex); + if (m_send_state == SendState::V1) return m_v1_fallback.MarkBytesSent(bytes_sent); + m_send_pos += bytes_sent; Assume(m_send_pos <= m_send_buffer.size()); if (m_send_pos == m_send_buffer.size()) { @@ -1331,6 +1415,8 @@ size_t V2Transport::GetSendMemoryUsage() const noexcept { AssertLockNotHeld(m_send_mutex); LOCK(m_send_mutex); + if (m_send_state == SendState::V1) return m_v1_fallback.GetSendMemoryUsage(); + return sizeof(m_send_buffer) + memusage::DynamicUsage(m_send_buffer); } diff --git a/src/net.h b/src/net.h index 27d141bc6e6..9194a677b1d 100644 --- a/src/net.h +++ b/src/net.h @@ -444,25 +444,40 @@ class V2Transport final : public Transport * an empty version packet contents is interpreted as no extensions supported. */ static constexpr std::array VERSION_CONTENTS = {}; + /** The length of the V1 prefix to match bytes initially received by responders with to + * determine if their peer is speaking V1 or V2. */ + static constexpr size_t V1_PREFIX_LEN = 12; + // The sender side and receiver side of V2Transport are state machines that are transitioned // through, based on what has been received. The receive state corresponds to the contents of, // and bytes received to, the receive buffer. The send state controls what can be appended to - // the send buffer. + // the send buffer and what can be sent from it. /** State type that defines the current contents of the receive buffer and/or how the next * received bytes added to it will be interpreted. * * Diagram: * - * start /---------\ - * | | | - * v v | - * KEY -> GARB_GARBTERM -> GARBAUTH -> VERSION -> APP -> APP_READY + * start(responder) + * | + * | start(initiator) /---------\ + * | | | | + * v v v | + * KEY_MAYBE_V1 -> KEY -> GARB_GARBTERM -> GARBAUTH -> VERSION -> APP -> APP_READY + * | + * \-------> V1 */ enum class RecvState : uint8_t { + /** (Responder only) either v2 public key or v1 header. + * + * This is the initial state for responders, before data has been received to distinguish + * v1 from v2 connections. When that happens, the state becomes either KEY (for v2) or V1 + * (for v1). */ + KEY_MAYBE_V1, + /** Public key. * - * This is the initial state, during which the other side's public key is + * This is the initial state for initiators, during which the other side's public key is * received. When that information arrives, the ciphers get initialized and the state * becomes GARB_GARBTERM. */ KEY, @@ -502,23 +517,40 @@ class V2Transport final : public Transport * Nothing can be received in this state. When the message is retrieved by GetMessage, * the state becomes APP again. */ APP_READY, + + /** Nothing (this transport is using v1 fallback). + * + * All receive operations are redirected to m_v1_fallback. */ + V1, }; /** State type that controls the sender side. * * Diagram: * - * start - * | - * v - * AWAITING_KEY -> READY + * start(responder) + * | + * | start(initiator) + * | | + * v v + * MAYBE_V1 -> AWAITING_KEY -> READY + * | + * \-----> V1 */ 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 + * 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. 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 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. @@ -528,6 +560,11 @@ class V2Transport final : public Transport * 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. + * + * All send operations are redirected to m_v1_fallback. */ + V1, }; /** Cipher state. */ @@ -536,6 +573,8 @@ class V2Transport final : public Transport const bool m_initiating; /** NodeId (for debug logging). */ const NodeId m_nodeid; + /** Encapsulate a V1Transport to fall back to. */ + V1Transport m_v1_fallback; /** Lock for receiver-side fields. */ mutable Mutex m_recv_mutex ACQUIRED_BEFORE(m_send_mutex); @@ -575,6 +614,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); + /** 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. */ void ProcessReceivedKeyBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex); /** Process bytes in m_recv_buffer, while in GARB_GARBTERM state. */ diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index f9454eab695..3984bfd26e1 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -371,3 +371,14 @@ FUZZ_TARGET(p2p_transport_bidirectional_v2, .init = initialize_p2p_transport_ser if (!t1 || !t2) return; SimulationTest(*t1, *t2, rng, provider); } + +FUZZ_TARGET(p2p_transport_bidirectional_v1v2, .init = initialize_p2p_transport_serialization) +{ + // Test with a V1 initiator talking to a V2 responder. + FuzzedDataProvider provider{buffer.data(), buffer.size()}; + XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral()); + auto t1 = MakeV1Transport(NodeId{0}); + auto t2 = MakeV2Transport(NodeId{1}, false, rng, provider); + if (!t1 || !t2) return; + SimulationTest(*t1, *t2, rng, provider); +} From 0be752d9f8ca27320bc3e82498c7640fabd7e8de Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 30 Jul 2023 21:25:10 -0400 Subject: [PATCH 06/10] net: add short message encoding/decoding support to V2Transport --- src/net.cpp | 97 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 91 insertions(+), 6 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index dbdbbf9d4e9..25f390961b9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -913,6 +913,74 @@ size_t V1Transport::GetSendMemoryUsage() const noexcept return m_message_to_send.GetMemoryUsage(); } +namespace { + +/** List of short messages as defined in BIP324, in order. + * + * Only message types that are actually implemented in this codebase need to be listed, as other + * messages get ignored anyway - whether we know how to decode them or not. + */ +const std::array V2_MESSAGE_IDS = { + "", // 12 bytes follow encoding the message type like in V1 + NetMsgType::ADDR, + NetMsgType::BLOCK, + NetMsgType::BLOCKTXN, + NetMsgType::CMPCTBLOCK, + NetMsgType::FEEFILTER, + NetMsgType::FILTERADD, + NetMsgType::FILTERCLEAR, + NetMsgType::FILTERLOAD, + NetMsgType::GETBLOCKS, + NetMsgType::GETBLOCKTXN, + NetMsgType::GETDATA, + NetMsgType::GETHEADERS, + NetMsgType::HEADERS, + NetMsgType::INV, + NetMsgType::MEMPOOL, + NetMsgType::MERKLEBLOCK, + NetMsgType::NOTFOUND, + NetMsgType::PING, + NetMsgType::PONG, + NetMsgType::SENDCMPCT, + NetMsgType::TX, + NetMsgType::GETCFILTERS, + NetMsgType::CFILTER, + NetMsgType::GETCFHEADERS, + NetMsgType::CFHEADERS, + NetMsgType::GETCFCHECKPT, + NetMsgType::CFCHECKPT, + NetMsgType::ADDRV2, + // Unimplemented message types that are assigned in BIP324: + "", + "", + "", + "" +}; + +class V2MessageMap +{ + std::unordered_map m_map; + +public: + V2MessageMap() noexcept + { + for (size_t i = 1; i < std::size(V2_MESSAGE_IDS); ++i) { + m_map.emplace(V2_MESSAGE_IDS[i], i); + } + } + + std::optional operator()(const std::string& message_name) const noexcept + { + auto it = m_map.find(message_name); + if (it == m_map.end()) return std::nullopt; + return it->second; + } +}; + +const V2MessageMap V2_MESSAGE_MAP; + +} // namespace + 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}, @@ -1299,7 +1367,16 @@ std::optional V2Transport::GetMessageType(Span& cont uint8_t first_byte = contents[0]; contents = contents.subspan(1); // Strip first byte. - if (first_byte != 0) return std::nullopt; // TODO: implement short encoding + if (first_byte != 0) { + // Short (1 byte) encoding. + if (first_byte < std::size(V2_MESSAGE_IDS)) { + // Valid short message id. + return V2_MESSAGE_IDS[first_byte]; + } else { + // Unknown short message id. + return std::nullopt; + } + } if (contents.size() < CMessageHeader::COMMAND_SIZE) { return std::nullopt; // Long encoding needs 12 message type bytes. @@ -1364,11 +1441,19 @@ bool V2Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept // buffer to just one, and leaves the responsibility for queueing them up to the caller. if (!(m_send_state == SendState::READY && m_send_buffer.empty())) return false; // Construct contents (encoding message type + payload). - // Initialize with zeroes, and then write the message type string starting at offset 1. - // This means contents[0] and the unused positions in contents[1..13] remain 0x00. - std::vector contents(1 + CMessageHeader::COMMAND_SIZE + msg.data.size(), 0); - std::copy(msg.m_type.begin(), msg.m_type.end(), contents.data() + 1); - std::copy(msg.data.begin(), msg.data.end(), contents.begin() + 1 + CMessageHeader::COMMAND_SIZE); + std::vector contents; + auto short_message_id = V2_MESSAGE_MAP(msg.m_type); + if (short_message_id) { + contents.resize(1 + msg.data.size()); + contents[0] = *short_message_id; + std::copy(msg.data.begin(), msg.data.end(), contents.begin() + 1); + } else { + // Initialize with zeroes, and then write the message type string starting at offset 1. + // This means contents[0] and the unused positions in contents[1..13] remain 0x00. + contents.resize(1 + CMessageHeader::COMMAND_SIZE + msg.data.size(), 0); + std::copy(msg.m_type.begin(), msg.m_type.end(), contents.data() + 1); + std::copy(msg.data.begin(), msg.data.end(), contents.begin() + 1 + CMessageHeader::COMMAND_SIZE); + } // Construct ciphertext in send buffer. m_send_buffer.resize(contents.size() + BIP324Cipher::EXPANSION); m_cipher.Encrypt(MakeByteSpan(contents), {}, false, MakeWritableByteSpan(m_send_buffer)); From 3ffa5fb49ee4a6d9502aa957093bd94058630282 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 30 Jul 2023 11:43:10 -0400 Subject: [PATCH 07/10] 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(-) diff --git a/src/net.cpp b/src/net.cpp index 25f390961b9..e75089c8e9c 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 9194a677b1d..90f5c4b388c 100644 --- a/src/net.h +++ b/src/net.h @@ -556,9 +556,9 @@ class V2Transport final : public Transport /** 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 @@ class V2Transport final : public Transport */ 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 3984bfd26e1..6e3ef2a707b 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 From 297c8889975a18258d6cc39b1ec1e94fed6630fb Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 30 Jul 2023 10:51:12 -0400 Subject: [PATCH 08/10] net: make V2Transport preallocate receive buffer space --- src/net.cpp | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/net.cpp b/src/net.cpp index e75089c8e9c..bd3ba3f6f8e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1317,6 +1317,9 @@ size_t V2Transport::GetMaxBytesToProcess() noexcept bool V2Transport::ReceivedBytes(Span& msg_bytes) noexcept { AssertLockNotHeld(m_recv_mutex); + /** How many bytes to allocate in the receive buffer at most above what is received so far. */ + static constexpr size_t MAX_RESERVE_AHEAD = 256 * 1024; + LOCK(m_recv_mutex); if (m_recv_state == RecvState::V1) return m_v1_fallback.ReceivedBytes(msg_bytes); @@ -1327,6 +1330,40 @@ bool V2Transport::ReceivedBytes(Span& msg_bytes) noexcept while (!msg_bytes.empty()) { // Decide how many bytes to copy from msg_bytes to m_recv_buffer. size_t max_read = GetMaxBytesToProcess(); + + // Reserve space in the buffer if there is not enough. + if (m_recv_buffer.size() + std::min(msg_bytes.size(), max_read) > m_recv_buffer.capacity()) { + switch (m_recv_state) { + case RecvState::KEY_MAYBE_V1: + case RecvState::KEY: + case RecvState::GARB_GARBTERM: + // During the initial states (key/garbage), allocate once to fit the maximum (4111 + // bytes). + m_recv_buffer.reserve(MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN); + break; + case RecvState::GARBAUTH: + case RecvState::VERSION: + case RecvState::APP: { + // During states where a packet is being received, as much as is expected but never + // more than MAX_RESERVE_AHEAD bytes in addition to what is received so far. + // This means attackers that want to cause us to waste allocated memory are limited + // to MAX_RESERVE_AHEAD above the largest allowed message contents size, and to + // MAX_RESERVE_AHEAD more than they've actually sent us. + size_t alloc_add = std::min(max_read, msg_bytes.size() + MAX_RESERVE_AHEAD); + m_recv_buffer.reserve(m_recv_buffer.size() + alloc_add); + break; + } + case RecvState::APP_READY: + // The buffer is empty in this state. + Assume(m_recv_buffer.empty()); + break; + case RecvState::V1: + // Should have bailed out above. + Assume(false); + break; + } + } + // Can't read more than provided input. max_read = std::min(msg_bytes.size(), max_read); // Copy data to buffer. From 91e1ef8684997fb4b3e8b64ef3935a936445066b Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 27 Aug 2023 21:08:56 -0400 Subject: [PATCH 09/10] test: add unit tests for V2Transport --- src/test/net_tests.cpp | 502 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 502 insertions(+) diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 295cb78b361..feaa0aef61f 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -1005,4 +1006,505 @@ BOOST_AUTO_TEST_CASE(advertise_local_address) RemoveLocal(addr_cjdns); } +namespace { + +/** 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 + * side (like decoy packets). + */ +class V2TransportTester +{ + V2Transport m_transport; //!< V2Transport being tested + BIP324Cipher m_cipher; //!< Cipher to help with the other side + bool m_test_initiator; //!< Whether m_transport is the initiator (true) or responder (false) + + std::vector m_sent_garbage; //!< The garbage we've sent to m_transport. + std::vector m_to_send; //!< Bytes we have queued up to send to m_transport. + std::vector m_received; //!< Bytes we have received from m_transport. + std::deque m_msg_to_send; //!< Messages to be sent *by* m_transport to us. + +public: + /** 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_test_initiator(test_initiator) {} + + /** Data type returned by Interact: + * + * - std::nullopt: transport error occurred + * - otherwise: a vector of + * - std::nullopt: invalid message received + * - otherwise: a CNetMessage retrieved + */ + using InteractResult = std::optional>>; + + /** Send/receive scheduled/available bytes and messages. + * + * This is the only function that interacts with the transport being tested; everything else is + * scheduling things done by Interact(), or processing things learned by it. + */ + InteractResult Interact() + { + std::vector> ret; + while (true) { + bool progress{false}; + // Send bytes from m_to_send to the transport. + if (!m_to_send.empty()) { + Span to_send = Span{m_to_send}.first(1 + InsecureRandRange(m_to_send.size())); + size_t old_len = to_send.size(); + if (!m_transport.ReceivedBytes(to_send)) { + return std::nullopt; // transport error occurred + } + if (old_len != to_send.size()) { + progress = true; + m_to_send.erase(m_to_send.begin(), m_to_send.begin() + (old_len - to_send.size())); + } + } + // Retrieve messages received by the transport. + if (m_transport.ReceivedMessageComplete() && (!progress || InsecureRandBool())) { + bool reject{false}; + auto msg = m_transport.GetReceivedMessage({}, reject); + if (reject) { + ret.push_back(std::nullopt); + } else { + ret.push_back(std::move(msg)); + } + progress = true; + } + // Enqueue a message to be sent by the transport to us. + if (!m_msg_to_send.empty() && (!progress || InsecureRandBool())) { + if (m_transport.SetMessageToSend(m_msg_to_send.front())) { + m_msg_to_send.pop_front(); + progress = true; + } + } + // Receive bytes from the transport. + const auto& [recv_bytes, _more, _msg_type] = m_transport.GetBytesToSend(!m_msg_to_send.empty()); + if (!recv_bytes.empty() && (!progress || InsecureRandBool())) { + size_t to_receive = 1 + InsecureRandRange(recv_bytes.size()); + m_received.insert(m_received.end(), recv_bytes.begin(), recv_bytes.begin() + to_receive); + progress = true; + m_transport.MarkBytesSent(to_receive); + } + if (!progress) break; + } + return ret; + } + + /** Expose the cipher. */ + BIP324Cipher& GetCipher() { return m_cipher; } + + /** Schedule bytes to be sent to the transport. */ + void Send(Span data) + { + m_to_send.insert(m_to_send.end(), data.begin(), data.end()); + } + + /** Schedule bytes to be sent to the transport. */ + void Send(Span data) { Send(MakeUCharSpan(data)); } + + /** Schedule our ellswift key to be sent to the transport. */ + void SendKey() { Send(m_cipher.GetOurPubKey()); } + + /** Schedule specified garbage to be sent to the transport. */ + void SendGarbage(Span garbage) + { + // Remember the specified garbage (so we can use it for constructing the garbage + // authentication packet). + m_sent_garbage.assign(garbage.begin(), garbage.end()); + // Schedule it for sending. + Send(m_sent_garbage); + } + + /** Schedule garbage (of specified length) to be sent to the transport. */ + void SendGarbage(size_t garbage_len) + { + // Generate random garbage and send it. + SendGarbage(g_insecure_rand_ctx.randbytes(garbage_len)); + } + + /** Schedule garbage (with valid random length) to be sent to the transport. */ + void SendGarbage() + { + SendGarbage(InsecureRandRange(V2Transport::MAX_GARBAGE_LEN + 1)); + } + + /** Schedule a message to be sent to us by the transport. */ + void AddMessage(std::string m_type, std::vector payload) + { + CSerializedNetMsg msg; + msg.m_type = std::move(m_type); + msg.data = std::move(payload); + m_msg_to_send.push_back(std::move(msg)); + } + + /** Expect ellswift key to have been received from transport and process it. + * + * Many other V2TransportTester functions cannot be called until after ReceiveKey() has been + * called, as no encryption keys are set up before that point. + */ + void ReceiveKey() + { + // When processing a key, enough bytes need to have been received already. + BOOST_REQUIRE(m_received.size() >= EllSwiftPubKey::size()); + // Initialize the cipher using it (acting as the opposite side of the tested transport). + m_cipher.Initialize(MakeByteSpan(m_received).first(EllSwiftPubKey::size()), !m_test_initiator); + // Strip the processed bytes off the front of the receive buffer. + m_received.erase(m_received.begin(), m_received.begin() + EllSwiftPubKey::size()); + } + + /** Schedule an encrypted packet with specified content/aad/ignore to be sent to transport + * (only after ReceiveKey). */ + void SendPacket(Span content, Span aad = {}, bool ignore = false) + { + // Use cipher to construct ciphertext. + std::vector ciphertext; + ciphertext.resize(content.size() + BIP324Cipher::EXPANSION); + m_cipher.Encrypt( + /*contents=*/MakeByteSpan(content), + /*aad=*/MakeByteSpan(aad), + /*ignore=*/ignore, + /*output=*/ciphertext); + // Schedule it for sending. + Send(ciphertext); + } + + /** Schedule garbage terminator and authentication packet to be sent to the transport (only + * after ReceiveKey). */ + void SendGarbageTermAuth(size_t garb_auth_data_len = 0, bool garb_auth_ignore = false) + { + // Generate random data to include in the garbage authentication packet (ignored by peer). + auto garb_auth_data = g_insecure_rand_ctx.randbytes(garb_auth_data_len); + // Schedule the garbage terminator to be sent. + Send(m_cipher.GetSendGarbageTerminator()); + // Schedule the garbage authentication packet to be sent. + SendPacket(/*content=*/garb_auth_data, /*aad=*/m_sent_garbage, /*ignore=*/garb_auth_ignore); + } + + /** Schedule version packet to be sent to the transport (only after ReceiveKey). */ + void SendVersion(Span version_data = {}, bool vers_ignore = false) + { + SendPacket(/*content=*/version_data, /*aad=*/{}, /*ignore=*/vers_ignore); + } + + /** Expect a packet to have been received from transport, process it, and return its contents + * (only after ReceiveKey). By default, decoys are skipped. */ + std::vector ReceivePacket(Span aad = {}, bool skip_decoy = true) + { + std::vector contents; + // Loop as long as there are ignored packets that are to be skipped. + while (true) { + // When processing a packet, at least enough bytes for its length descriptor must be received. + BOOST_REQUIRE(m_received.size() >= BIP324Cipher::LENGTH_LEN); + // Decrypt the content length. + size_t size = m_cipher.DecryptLength(MakeByteSpan(Span{m_received}.first(BIP324Cipher::LENGTH_LEN))); + // Check that the full packet is in the receive buffer. + BOOST_REQUIRE(m_received.size() >= size + BIP324Cipher::EXPANSION); + // Decrypt the packet contents. + contents.resize(size); + bool ignore{false}; + bool ret = m_cipher.Decrypt( + /*input=*/MakeByteSpan( + Span{m_received}.first(size + BIP324Cipher::EXPANSION).subspan(BIP324Cipher::LENGTH_LEN)), + /*aad=*/aad, + /*ignore=*/ignore, + /*contents=*/MakeWritableByteSpan(contents)); + BOOST_CHECK(ret); + // Strip the processed packet's bytes off the front of the receive buffer. + m_received.erase(m_received.begin(), m_received.begin() + size + BIP324Cipher::EXPANSION); + // Stop if the ignore bit is not set on this packet, or if we choose to not honor it. + if (!ignore || !skip_decoy) break; + } + return contents; + } + + /** Expect garbage, garbage terminator, and garbage auth packet to have been received, and + * process them (only after ReceiveKey). */ + void ReceiveGarbage() + { + // Figure out the garbage length. + size_t garblen; + for (garblen = 0; garblen <= V2Transport::MAX_GARBAGE_LEN; ++garblen) { + BOOST_REQUIRE(m_received.size() >= garblen + BIP324Cipher::GARBAGE_TERMINATOR_LEN); + auto term_span = MakeByteSpan(Span{m_received}.subspan(garblen, BIP324Cipher::GARBAGE_TERMINATOR_LEN)); + if (term_span == m_cipher.GetReceiveGarbageTerminator()) break; + } + // Copy the garbage to a buffer. + std::vector garbage(m_received.begin(), m_received.begin() + garblen); + // Strip garbage + garbage terminator off the front of the receive buffer. + m_received.erase(m_received.begin(), m_received.begin() + garblen + BIP324Cipher::GARBAGE_TERMINATOR_LEN); + // Process the expected garbage authentication packet. Such a packet still functions as one + // even when its ignore bit is set to true, so we do not skip decoy packets here. + ReceivePacket(/*aad=*/MakeByteSpan(garbage), /*skip_decoy=*/false); + } + + /** Expect version packet to have been received, and process it (only after ReceiveKey). */ + void ReceiveVersion() + { + auto contents = ReceivePacket(); + // Version packets from real BIP324 peers are expected to be empty, despite the fact that + // this class supports *sending* non-empty version packets (to test that BIP324 peers + // correctly ignore version packet contents). + BOOST_CHECK(contents.empty()); + } + + /** Expect application packet to have been received, with specified short id and payload. + * (only after ReceiveKey). */ + void ReceiveMessage(uint8_t short_id, Span payload) + { + auto ret = ReceivePacket(); + BOOST_CHECK(ret.size() == payload.size() + 1); + BOOST_CHECK(ret[0] == short_id); + BOOST_CHECK(Span{ret}.subspan(1) == payload); + } + + /** Expect application packet to have been received, with specified 12-char message type and + * payload (only after ReceiveKey). */ + void ReceiveMessage(const std::string& m_type, Span payload) + { + auto ret = ReceivePacket(); + BOOST_REQUIRE(ret.size() == payload.size() + 1 + CMessageHeader::COMMAND_SIZE); + BOOST_CHECK(ret[0] == 0); + for (unsigned i = 0; i < 12; ++i) { + if (i < m_type.size()) { + BOOST_CHECK(ret[1 + i] == m_type[i]); + } else { + BOOST_CHECK(ret[1 + i] == 0); + } + } + BOOST_CHECK(Span{ret}.subspan(1 + CMessageHeader::COMMAND_SIZE) == payload); + } + + /** Schedule an encrypted packet with specified message type and payload to be sent to + * transport (only after ReceiveKey). */ + void SendMessage(std::string mtype, Span payload) + { + // Construct contents consisting of 0x00 + 12-byte message type + payload. + std::vector contents(1 + CMessageHeader::COMMAND_SIZE + payload.size()); + std::copy(mtype.begin(), mtype.end(), reinterpret_cast(contents.data() + 1)); + std::copy(payload.begin(), payload.end(), contents.begin() + 1 + CMessageHeader::COMMAND_SIZE); + // Send a packet with that as contents. + SendPacket(contents); + } + + /** Schedule an encrypted packet with specified short message id and payload to be sent to + * transport (only after ReceiveKey). */ + void SendMessage(uint8_t short_id, Span payload) + { + // Construct contents consisting of short_id + payload. + std::vector contents(1 + payload.size()); + contents[0] = short_id; + std::copy(payload.begin(), payload.end(), contents.begin() + 1); + // Send a packet with that as contents. + SendPacket(contents); + } + + /** Introduce a bit error in the data scheduled to be sent. */ + void Damage() + { + m_to_send[InsecureRandRange(m_to_send.size())] ^= (uint8_t{1} << InsecureRandRange(8)); + } +}; + +} // namespace + +BOOST_AUTO_TEST_CASE(v2transport_test) +{ + // A mostly normal scenario, testing a transport in initiator mode. + for (int i = 0; i < 10; ++i) { + V2TransportTester tester(true); + auto ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.SendKey(); + tester.SendGarbage(); + tester.ReceiveKey(); + tester.SendGarbageTermAuth(); + tester.SendVersion(); + ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.ReceiveGarbage(); + tester.ReceiveVersion(); + auto msg_data_1 = g_insecure_rand_ctx.randbytes(InsecureRandRange(100000)); + auto msg_data_2 = g_insecure_rand_ctx.randbytes(InsecureRandRange(1000)); + tester.SendMessage(uint8_t(4), msg_data_1); // cmpctblock short id + tester.SendMessage(0, {}); // Invalidly encoded message + tester.SendMessage("tx", msg_data_2); // 12-character encoded message type + ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->size() == 3); + BOOST_CHECK((*ret)[0] && (*ret)[0]->m_type == "cmpctblock" && Span{(*ret)[0]->m_recv} == MakeByteSpan(msg_data_1)); + BOOST_CHECK(!(*ret)[1]); + BOOST_CHECK((*ret)[2] && (*ret)[2]->m_type == "tx" && Span{(*ret)[2]->m_recv} == MakeByteSpan(msg_data_2)); + + // Then send a message with a bit error, expecting failure. + tester.SendMessage("bad", msg_data_1); + tester.Damage(); + ret = tester.Interact(); + BOOST_CHECK(!ret); + } + + // Normal scenario, with a transport in responder node. + for (int i = 0; i < 10; ++i) { + V2TransportTester tester(false); + tester.SendKey(); + tester.SendGarbage(); + auto ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.ReceiveKey(); + tester.SendGarbageTermAuth(); + tester.SendVersion(); + ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.ReceiveGarbage(); + tester.ReceiveVersion(); + auto msg_data_1 = g_insecure_rand_ctx.randbytes(InsecureRandRange(100000)); + auto msg_data_2 = g_insecure_rand_ctx.randbytes(InsecureRandRange(1000)); + tester.SendMessage(uint8_t(14), msg_data_1); // inv short id + tester.SendMessage(uint8_t(19), msg_data_2); // pong short id + ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->size() == 2); + BOOST_CHECK((*ret)[0] && (*ret)[0]->m_type == "inv" && Span{(*ret)[0]->m_recv} == MakeByteSpan(msg_data_1)); + BOOST_CHECK((*ret)[1] && (*ret)[1]->m_type == "pong" && Span{(*ret)[1]->m_recv} == MakeByteSpan(msg_data_2)); + + // Then send a too-large message. + auto msg_data_3 = g_insecure_rand_ctx.randbytes(4005000); + tester.SendMessage(uint8_t(11), msg_data_3); // getdata short id + ret = tester.Interact(); + BOOST_CHECK(!ret); + } + + // Various valid but unusual scenarios. + for (int i = 0; i < 50; ++i) { + /** Whether an initiator or responder is being tested. */ + bool initiator = InsecureRandBool(); + /** Use either 0 bytes or the maximum possible (4095 bytes) garbage length. */ + size_t garb_len = InsecureRandBool() ? 0 : V2Transport::MAX_GARBAGE_LEN; + /** Sometimes, use non-empty contents in the garbage authentication packet (which is to be ignored). */ + size_t garb_auth_data_len = InsecureRandBool() ? 0 : InsecureRandRange(100000); + /** Whether to set the ignore bit on the garbage authentication packet (it still functions as garbage authentication). */ + bool garb_ignore = InsecureRandBool(); + /** How many decoy packets to send before the version packet. */ + unsigned num_ignore_version = InsecureRandRange(10); + /** What data to send in the version packet (ignored by BIP324 peers, but reserved for future extensions). */ + auto ver_data = g_insecure_rand_ctx.randbytes(InsecureRandBool() ? 0 : InsecureRandRange(1000)); + /** Whether to immediately send key and garbage out (required for responders, optional otherwise). */ + bool send_immediately = !initiator || InsecureRandBool(); + /** How many decoy packets to send before the first and second real message. */ + unsigned num_decoys_1 = InsecureRandRange(1000), num_decoys_2 = InsecureRandRange(1000); + V2TransportTester tester(initiator); + if (send_immediately) { + tester.SendKey(); + tester.SendGarbage(garb_len); + } + auto ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + if (!send_immediately) { + tester.SendKey(); + tester.SendGarbage(garb_len); + } + tester.ReceiveKey(); + tester.SendGarbageTermAuth(garb_auth_data_len, garb_ignore); + for (unsigned v = 0; v < num_ignore_version; ++v) { + size_t ver_ign_data_len = InsecureRandBool() ? 0 : InsecureRandRange(1000); + auto ver_ign_data = g_insecure_rand_ctx.randbytes(ver_ign_data_len); + tester.SendVersion(ver_ign_data, true); + } + tester.SendVersion(ver_data, false); + ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.ReceiveGarbage(); + tester.ReceiveVersion(); + for (unsigned d = 0; d < num_decoys_1; ++d) { + auto decoy_data = g_insecure_rand_ctx.randbytes(InsecureRandRange(1000)); + tester.SendPacket(/*content=*/decoy_data, /*aad=*/{}, /*ignore=*/true); + } + auto msg_data_1 = g_insecure_rand_ctx.randbytes(InsecureRandRange(4000000)); + tester.SendMessage(uint8_t(28), msg_data_1); + for (unsigned d = 0; d < num_decoys_2; ++d) { + auto decoy_data = g_insecure_rand_ctx.randbytes(InsecureRandRange(1000)); + tester.SendPacket(/*content=*/decoy_data, /*aad=*/{}, /*ignore=*/true); + } + auto msg_data_2 = g_insecure_rand_ctx.randbytes(InsecureRandRange(1000)); + tester.SendMessage(uint8_t(13), msg_data_2); // headers short id + // Send invalidly-encoded message + tester.SendMessage(std::string("blocktxn\x00\x00\x00a", CMessageHeader::COMMAND_SIZE), {}); + tester.SendMessage("foobar", {}); // test receiving unknown message type + tester.AddMessage("barfoo", {}); // test sending unknown message type + ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->size() == 4); + BOOST_CHECK((*ret)[0] && (*ret)[0]->m_type == "addrv2" && Span{(*ret)[0]->m_recv} == MakeByteSpan(msg_data_1)); + BOOST_CHECK((*ret)[1] && (*ret)[1]->m_type == "headers" && Span{(*ret)[1]->m_recv} == MakeByteSpan(msg_data_2)); + BOOST_CHECK(!(*ret)[2]); + BOOST_CHECK((*ret)[3] && (*ret)[3]->m_type == "foobar" && (*ret)[3]->m_recv.empty()); + tester.ReceiveMessage("barfoo", {}); + } + + // Too long garbage (initiator). + { + V2TransportTester tester(true); + auto ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.SendKey(); + tester.SendGarbage(V2Transport::MAX_GARBAGE_LEN + 1); + tester.ReceiveKey(); + tester.SendGarbageTermAuth(); + ret = tester.Interact(); + BOOST_CHECK(!ret); + } + + // Too long garbage (responder). + { + V2TransportTester tester(false); + tester.SendKey(); + tester.SendGarbage(V2Transport::MAX_GARBAGE_LEN + 1); + auto ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.ReceiveKey(); + tester.SendGarbageTermAuth(); + ret = tester.Interact(); + BOOST_CHECK(!ret); + } + + // Send garbage that includes the first 15 garbage terminator bytes somewhere. + { + V2TransportTester tester(true); + auto ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.SendKey(); + tester.ReceiveKey(); + /** The number of random garbage bytes before the included first 15 bytes of terminator. */ + size_t len_before = InsecureRandRange(V2Transport::MAX_GARBAGE_LEN - 16 + 1); + /** The number of random garbage bytes after it. */ + size_t len_after = InsecureRandRange(V2Transport::MAX_GARBAGE_LEN - 16 - len_before + 1); + // Construct len_before + 16 + len_after random bytes. + auto garbage = g_insecure_rand_ctx.randbytes(len_before + 16 + len_after); + // Replace the designed 16 bytes in the middle with the to-be-sent garbage terminator. + auto garb_term = MakeUCharSpan(tester.GetCipher().GetSendGarbageTerminator()); + std::copy(garb_term.begin(), garb_term.begin() + 16, garbage.begin() + len_before); + // Introduce a bit error in the last byte of that copied garbage terminator, making only + // the first 15 of them match. + garbage[len_before + 15] ^= (uint8_t(1) << InsecureRandRange(8)); + tester.SendGarbage(garbage); + tester.SendGarbageTermAuth(); + tester.SendVersion(); + ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.ReceiveGarbage(); + tester.ReceiveVersion(); + auto msg_data_1 = g_insecure_rand_ctx.randbytes(4000000); // test that receiving 4M payload works + auto msg_data_2 = g_insecure_rand_ctx.randbytes(4000000); // test that sending 4M payload works + tester.SendMessage(uint8_t(InsecureRandRange(223) + 33), {}); // unknown short id + tester.SendMessage(uint8_t(2), msg_data_1); // "block" short id + tester.AddMessage("blocktxn", msg_data_2); // schedule blocktxn to be sent to us + ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->size() == 2); + BOOST_CHECK(!(*ret)[0]); + BOOST_CHECK((*ret)[1] && (*ret)[1]->m_type == "block" && Span{(*ret)[1]->m_recv} == MakeByteSpan(msg_data_1)); + tester.ReceiveMessage(uint8_t(3), msg_data_2); // "blocktxn" short id + } +} + BOOST_AUTO_TEST_SUITE_END() From db9888feec48c6220a2fcf92865503bbbdab02a4 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 5 Sep 2023 23:38:15 -0400 Subject: [PATCH 10/10] net: detect wrong-network V1 talking to V2Transport --- src/net.cpp | 22 ++++++++++++++++++++-- src/net.h | 2 +- src/test/net_tests.cpp | 25 +++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index bd3ba3f6f8e..3955005dfa6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1109,12 +1109,29 @@ void V2Transport::ProcessReceivedMaybeV1Bytes() noexcept } } -void V2Transport::ProcessReceivedKeyBytes() noexcept +bool V2Transport::ProcessReceivedKeyBytes() noexcept { AssertLockHeld(m_recv_mutex); AssertLockNotHeld(m_send_mutex); Assume(m_recv_state == RecvState::KEY); Assume(m_recv_buffer.size() <= EllSwiftPubKey::size()); + + // As a special exception, if bytes 4-16 of the key on a responder connection match the + // corresponding bytes of a V1 version message, but bytes 0-4 don't match the network magic + // (if they did, we'd have switched to V1 state already), assume this is a peer from + // another network, and disconnect them. They will almost certainly disconnect us too when + // they receive our uniformly random key and garbage, but detecting this case specially + // means we can log it. + static constexpr std::array MATCH = {'v', 'e', 'r', 's', 'i', 'o', 'n', 0, 0, 0, 0, 0}; + static constexpr size_t OFFSET = sizeof(CMessageHeader::MessageStartChars); + if (!m_initiating && m_recv_buffer.size() >= OFFSET + MATCH.size()) { + if (std::equal(MATCH.begin(), MATCH.end(), m_recv_buffer.begin() + OFFSET)) { + LogPrint(BCLog::NET, "V2 transport error: V1 peer with wrong MessageStart %s\n", + HexStr(Span(m_recv_buffer).first(OFFSET))); + return false; + } + } + if (m_recv_buffer.size() == EllSwiftPubKey::size()) { // Other side's key has been fully received, and can now be Diffie-Hellman combined with // our key to initialize the encryption ciphers. @@ -1157,6 +1174,7 @@ void V2Transport::ProcessReceivedKeyBytes() noexcept } else { // We still have to receive more key bytes. } + return true; } bool V2Transport::ProcessReceivedGarbageBytes() noexcept @@ -1378,7 +1396,7 @@ bool V2Transport::ReceivedBytes(Span& msg_bytes) noexcept break; case RecvState::KEY: - ProcessReceivedKeyBytes(); + if (!ProcessReceivedKeyBytes()) return false; break; case RecvState::GARB_GARBTERM: diff --git a/src/net.h b/src/net.h index 90f5c4b388c..cf7a240202e 100644 --- a/src/net.h +++ b/src/net.h @@ -617,7 +617,7 @@ class V2Transport final : public Transport /** 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. */ - void ProcessReceivedKeyBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex); + bool ProcessReceivedKeyBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex); /** Process bytes in m_recv_buffer, while in GARB_GARBTERM state. */ bool ProcessReceivedGarbageBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex); /** Process bytes in m_recv_buffer, while in GARBAUTH/VERSION/APP state. */ diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index feaa0aef61f..900e311d225 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1104,6 +1104,15 @@ class V2TransportTester m_to_send.insert(m_to_send.end(), data.begin(), data.end()); } + /** Send V1 version message header to the transport. */ + void SendV1Version(const CMessageHeader::MessageStartChars& magic) + { + CMessageHeader hdr(magic, "version", 126 + InsecureRandRange(11)); + CDataStream ser(SER_NETWORK, CLIENT_VERSION); + ser << hdr; + m_to_send.insert(m_to_send.end(), UCharCast(ser.data()), UCharCast(ser.data() + ser.size())); + } + /** Schedule bytes to be sent to the transport. */ void Send(Span data) { Send(MakeUCharSpan(data)); } @@ -1505,6 +1514,22 @@ BOOST_AUTO_TEST_CASE(v2transport_test) BOOST_CHECK((*ret)[1] && (*ret)[1]->m_type == "block" && Span{(*ret)[1]->m_recv} == MakeByteSpan(msg_data_1)); tester.ReceiveMessage(uint8_t(3), msg_data_2); // "blocktxn" short id } + + // Send correct network's V1 header + { + V2TransportTester tester(false); + tester.SendV1Version(Params().MessageStart()); + auto ret = tester.Interact(); + BOOST_CHECK(ret); + } + + // Send wrong network's V1 header + { + V2TransportTester tester(false); + tester.SendV1Version(CChainParams::Main()->MessageStart()); + auto ret = tester.Interact(); + BOOST_CHECK(!ret); + } } BOOST_AUTO_TEST_SUITE_END()