p2p: BIP324 shapable key exchange

dhruv · dhruv · commit ff91db0a2d3d · 2022-10-20T09:36:12.000-07:00
diff --git a/src/net.cpp b/src/net.cpp
@@ -115,6 +115,7 @@ static const uint64_t RANDOMIZER_ID_LOCALHOSTNONCE = 0xd93e69e2bbfa5735ULL; // S
 static const uint64_t RANDOMIZER_ID_ADDRCACHE = 0x1cf2e4ddd306dda9ULL; // SHA256("addrcache")[0:8]
 
 static constexpr uint8_t V2_MAX_MSG_TYPE_LEN = 12; // maximum length for V2 (BIP324) string message types
+static constexpr size_t V2_MAX_GARBAGE_BYTES = 4095; // maximum length for V2 (BIP324) shapable handshake
 //
 // Global state variables
 //
@@ -700,10 +701,16 @@ void CNode::InitV2P2P(const Span<const std::byte> their_ellswift, const Span<con
     if (initiating) {
         m_deserializer = std::make_unique<V2TransportDeserializer>(GetId(), v2_keys.responder_L, v2_keys.responder_P, v2_keys.rekey_salt);
         m_serializer = std::make_unique<V2TransportSerializer>(v2_keys.initiator_L, v2_keys.initiator_P, v2_keys.rekey_salt);
+        v2_sent_garbage_terminator = v2_keys.initiator_garbage_terminator;
+        v2_recv_garbage_terminator = v2_keys.responder_garbage_terminator;
     } else {
         m_deserializer = std::make_unique<V2TransportDeserializer>(GetId(), v2_keys.initiator_L, v2_keys.initiator_P, v2_keys.rekey_salt);
         m_serializer = std::make_unique<V2TransportSerializer>(v2_keys.responder_L, v2_keys.responder_P, v2_keys.rekey_salt);
+        v2_sent_garbage_terminator = v2_keys.responder_garbage_terminator;
+        v2_recv_garbage_terminator = v2_keys.initiator_garbage_terminator;
     }
+    // Both peers must keep around a copy of the garbage terminator for the BIP324 shapable handshake
+    v2_keys_derived = true;
 }
 
 void CNode::EnsureInitV2Key(bool initiating)
@@ -737,13 +744,24 @@ bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete)
             // decompose a transport agnostic CNetMessage from the deserializer
             bool reject_message{false};
             bool disconnect{false};
-            CNetMessage msg = m_deserializer->GetMessage(time, reject_message, disconnect, {});
+
+            std::vector<std::byte> aad;
+            if (PreferV2Conn() && !m_authenticated_v2_garbage) {
+                std::copy(v2_garbage_bytes_recd.begin(), v2_garbage_bytes_recd.end(), std::back_inserter(aad));
+            }
+            CNetMessage msg = m_deserializer->GetMessage(time, reject_message, disconnect, aad);
 
             if (disconnect) {
                 // v2 p2p incorrect MAC tag. Disconnect from peer.
                 return false;
             }
 
+            if (!aad.empty()) {
+                // first message to authenticate garbage
+                m_authenticated_v2_garbage = true;
+                memory_cleanse(v2_garbage_bytes_recd.data(), v2_garbage_bytes_recd.size());
+            }
+
             // inbound clients do not know whether the peer is trying to talk v1 or v2.
             // if the first message is not VERSION, we reinterpret the bytes as v2 ellswift
             if (!m_prefer_p2p_v2 && IsInboundConn() && nVersion == 0 && msg.m_type != NetMsgType::VERSION) {
@@ -785,7 +803,17 @@ int V1TransportDeserializer::readHeader(Span<const uint8_t> msg_bytes)
     memcpy(&hdrbuf[nHdrPos], msg_bytes.data(), nCopy);
     nHdrPos += nCopy;
 
-    // if header incomplete, exit
+    if (validated_magic_len < CMessageHeader::MESSAGE_START_SIZE) {
+        auto available = std::min((size_t)nHdrPos, CMessageHeader::MESSAGE_START_SIZE);
+        if (memcmp(hdrbuf.data(), m_chain_params.MessageStart(), available) != 0) {
+            LogPrint(BCLog::NET, "Header error: Wrong MessageStart %s received, peer=%d\n", HexStr(hdrbuf), m_node_id);
+            return -1;
+        } else {
+            validated_magic_len = available;
+        }
+    }
+
+    // don't have complete header yet, exit
     if (nHdrPos < CMessageHeader::HEADER_SIZE)
         return nCopy;
 
@@ -798,12 +826,6 @@ int V1TransportDeserializer::readHeader(Span<const uint8_t> msg_bytes)
         return -1;
     }
 
-    // Check start string, network magic
-    if (memcmp(hdr.pchMessageStart, m_chain_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) {
-        LogPrint(BCLog::NET, "Header error: Wrong MessageStart %s received, peer=%d\n", HexStr(hdr.pchMessageStart), m_node_id);
-        return -1;
-    }
-
     // reject messages larger than MAX_SIZE or MAX_PROTOCOL_MESSAGE_LENGTH
     if (hdr.nMessageSize > MAX_SIZE || hdr.nMessageSize > MAX_PROTOCOL_MESSAGE_LENGTH) {
         LogPrint(BCLog::NET, "Header error: Size too large (%s, %u bytes), peer=%d\n", SanitizeString(hdr.GetCommand()), hdr.nMessageSize, m_node_id);
@@ -979,9 +1001,14 @@ CNetMessage V2TransportDeserializer::GetMessage(const std::chrono::microseconds
         vRecv.resize(m_contents_size);
         reject_message = reject_message || (BIP324HeaderFlags(BIP324_IGNORE & flags) != BIP324_NONE);
 
-        // The first message we receive is the BIP324 transport version placeholder message.
-        // Discard it for v2.0 clients.
-        if (!m_processed_version_placeholder) {
+        if (!aad.empty()) {
+            // This only happens for the first encrypted message received by an inbound client
+            // which is meant to authenticate the garbage bytes for the BIP324 shapable handshake
+            // That message is not to be passed to the p2p application layer.
+            reject_message = true;
+        } else if (!m_processed_version_placeholder) {
+            // BIP324 transport version placeholder message.
+            // Discard it for v2.0 clients.
             reject_message = true;
             m_processed_version_placeholder = true;
         }
@@ -1568,43 +1595,111 @@ void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes,
                 }
                 nBytes = pnode->m_sock->Recv(pchBuf, sizeof(pchBuf), MSG_DONTWAIT);
             }
+            uint8_t* ptr = pchBuf;
             if (nBytes > 0) {
                 bool notify = false;
                 size_t num_bytes = (size_t)nBytes;
-                if (!pnode->ReceiveMsgBytes({pchBuf, num_bytes}, notify)) {
+
+                // for a v2 outbound peer:
+                //     prior to InitV2P2P(), pnode->m_deserializer is not init and ReceiveMsgBytes() will return false
+                // for a v2 inbound peer:
+                //     prior to InitV2P2P(), pnode->m_deserializer is init to V1TransportDeserializer, ReceiveMsgBytes will fail due to a malformed header according to the v1 protocol
+                // for all v2 peers:
+                //     after InitV2P2P(), keys are derived, but m_deserializer would simply disconnect on MAC failure until
+                //          the key exchage phase ends with the garbage terminator
+                //     after garbage terminator, ReceiveMsgBytes() should work on valid v2 encrypted packets
+                if ((pnode->v2_keys_derived && !pnode->v2_garbage_terminated) ||
+                    !pnode->ReceiveMsgBytes({ptr, num_bytes}, notify)) {
                     // when we cannot understand the received bytes, disconnect if:
                     //   1. we don't support BIP324 v2, or
                     //   2. v2 key exchange is complete, we should understand the bytes, or
                     //   3. we've previously received a v1 (or v2) VERSION message from the peer
-                    //   4. the received bytes are not exactly the size of an ellswift encoding
                     if (!gArgs.GetBoolArg("-v2transport", DEFAULT_V2_TRANSPORT) ||
                         pnode->v2_key_exchange_complete ||
-                        pnode->nVersion != 0 ||
-                        num_bytes < ELLSWIFT_ENCODED_SIZE) {
+                        pnode->nVersion != 0) {
                         pnode->CloseSocketDisconnect();
                     } else {
                         pnode->EnsureInitV2Key(!pnode->IsInboundConn());
 
-                        pnode->InitV2P2P({AsBytePtr(pchBuf), ELLSWIFT_ENCODED_SIZE}, MakeByteSpan(pnode->ellswift_pubkey), !pnode->IsInboundConn());
-                        if (pnode->IsInboundConn()) {
-                            PushV2EllSwiftPubkey(pnode);
+                        if (!pnode->v2_keys_derived) {
+                            // If we're the inbound peer, upon receiving any ellswift bytes we send our ellswift key
+                            if (pnode->IsInboundConn() && pnode->peer_ellswift_buf.empty()) {
+                                PushV2EllSwiftPubkey(pnode);
+                                // We now know the peer prefers a BIP324 v2 connection
+                                pnode->m_prefer_p2p_v2 = true;
+                            }
+
+                            // Keys are not derived because we don't have the peer ellswift yet, keep buffering.
+                            auto old_ellswift_sz = pnode->peer_ellswift_buf.size();
+                            auto more_ellswift_bytes = std::min(ELLSWIFT_ENCODED_SIZE - old_ellswift_sz, num_bytes);
+                            pnode->peer_ellswift_buf.resize(old_ellswift_sz + more_ellswift_bytes);
+                            memcpy(pnode->peer_ellswift_buf.data() + old_ellswift_sz, ptr, more_ellswift_bytes);
+
+                            ptr += more_ellswift_bytes;
+                            num_bytes -= more_ellswift_bytes;
+
+                            if (pnode->peer_ellswift_buf.size() < ELLSWIFT_ENCODED_SIZE)
+                                continue;
+
+                            // At this point we have the entire peer ellswift, we can derive keys
+                            // and instantiate the v2 encryption session
+                            pnode->InitV2P2P(pnode->peer_ellswift_buf, MakeByteSpan(pnode->ellswift_pubkey), !pnode->IsInboundConn());
+
+                            // After keys are exchanged, both peers send the garbage terminator
+                            // followed by the v2 encrypted message that authenticates the garbage
+                            // which is currently empty as the mechanism is unused in bitcoin core.
+                            PushV2GarbageTerminator(pnode);
+                            CSerializedNetMsg garbage_auth_msg;
+                            PushMessage(pnode, std::move(garbage_auth_msg));
+                            // Send empty message again for transport version placeholder
+                            CSerializedNetMsg transport_version_msg;
+                            PushMessage(pnode, std::move(transport_version_msg));
                         }
 
-                        // Send empty message for transport version placeholder
-                        CSerializedNetMsg msg;
-                        PushMessage(pnode, std::move(msg));
-
-                        if (!pnode->IsInboundConn()) {
-                            // Outbound peer has completed ECDH and can start the P2P protocol
-                            m_msgproc->InitP2P(*pnode, nLocalServices);
+                        if (pnode->v2_keys_derived && !pnode->v2_garbage_terminated && num_bytes > 0) {
+                            // Keep buffering bytes until the garbage terminator
+                            auto old_size = pnode->v2_garbage_bytes_recd.size();
+                            auto new_size = old_size + num_bytes;
+
+                            // Might be better to just allocate to V2_MAX_GARBAGE_BYTES at start once the mechanism
+                            // is actually used
+                            pnode->v2_garbage_bytes_recd.resize(std::min(new_size, V2_MAX_GARBAGE_BYTES));
+                            memcpy(pnode->v2_garbage_bytes_recd.data() + old_size, ptr, (new_size - old_size));
+                            auto it = std::search(pnode->v2_garbage_bytes_recd.begin(), pnode->v2_garbage_bytes_recd.end(),
+                                                  pnode->v2_recv_garbage_terminator.begin(), pnode->v2_recv_garbage_terminator.end());
+
+                            if (it != pnode->v2_garbage_bytes_recd.end()) {
+                                // Found the terminator...
+                                auto garbage_size = it - pnode->v2_garbage_bytes_recd.begin();
+                                pnode->v2_garbage_bytes_recd.erase(it, pnode->v2_garbage_bytes_recd.end());
+                                if (garbage_size <= long{V2_MAX_GARBAGE_BYTES}) {
+                                    // ...in less than the maximum allowed bytes
+                                    auto fwd = garbage_size + pnode->v2_recv_garbage_terminator.size() - old_size;
+                                    ptr += fwd;
+                                    num_bytes -= fwd;
+                                    pnode->v2_garbage_terminated = true;
+                                } else {
+                                    // ..but more than the maximum allowed bytes were used
+                                    pnode->CloseSocketDisconnect();
+                                }
+                                memory_cleanse(pnode->v2_recv_garbage_terminator.data(), pnode->v2_recv_garbage_terminator.size());
+                            } else if (pnode->v2_garbage_bytes_recd.size() >= V2_MAX_GARBAGE_BYTES) {
+                                pnode->CloseSocketDisconnect();
+                                memory_cleanse(pnode->v2_recv_garbage_terminator.data(), pnode->v2_recv_garbage_terminator.size());
+                            }
                         }
 
-                        pnode->v2_key_exchange_complete = true;
-                        if (num_bytes > ELLSWIFT_ENCODED_SIZE &&
-                            !pnode->ReceiveMsgBytes(
-                                {pchBuf + ELLSWIFT_ENCODED_SIZE, (size_t)(nBytes - ELLSWIFT_ENCODED_SIZE)},
-                                notify)) {
-                            pnode->CloseSocketDisconnect();
+                        // after a successful ECDH and shapable handshake garbage termination, process the remaining bytes.
+                        if (pnode->v2_garbage_terminated) {
+                            if (num_bytes > 0 && !pnode->ReceiveMsgBytes({ptr, num_bytes}, notify)) {
+                                pnode->CloseSocketDisconnect();
+                            } else {
+                                pnode->v2_key_exchange_complete = true;
+                                if (!pnode->IsInboundConn()) {
+                                    // Outbound peer has completed key exchange and can start the P2P protocol
+                                    m_msgproc->InitP2P(*pnode, nLocalServices);
+                                }
+                            }
                         }
                     }
                 }
@@ -3173,6 +3268,21 @@ void CConnman::PushV2EllSwiftPubkey(CNode* pnode)
     if (nBytesSent) RecordBytesSent(nBytesSent);
 }
 
+void CConnman::PushV2GarbageTerminator(CNode* pnode)
+{
+    std::vector<unsigned char> terminator_uchars;
+    terminator_uchars.resize(pnode->v2_sent_garbage_terminator.size());
+    memcpy(terminator_uchars.data(), pnode->v2_sent_garbage_terminator.data(), terminator_uchars.size());
+    {
+        LOCK(pnode->cs_vSend);
+        pnode->nSendSize += pnode->v2_sent_garbage_terminator.size();
+
+        // We do not have to send immediately because this is followed shortly by the
+        // transport version message
+        pnode->vSendMsg.push_back(terminator_uchars);
+    }
+}
+
 bool CConnman::ForNode(NodeId id, std::function<bool(CNode* pnode)> func)
 {
     CNode* found = nullptr;
diff --git a/src/net.h b/src/net.h
@@ -289,6 +289,7 @@ class V1TransportDeserializer final : public TransportDeserializer
     CDataStream vRecv;              // received message data
     unsigned int nHdrPos;
     unsigned int nDataPos;
+    uint8_t validated_magic_len{0};
 
     const uint256& GetMessageHash() const;
     int readHeader(Span<const uint8_t> msg_bytes);
@@ -527,6 +528,8 @@ class CNode
     std::atomic_bool fPauseRecv{false};
     std::atomic_bool fPauseSend{false};
     std::atomic_bool v2_key_exchange_complete{false};
+    std::atomic_bool m_authenticated_v2_garbage{false};
+    bool v2_garbage_terminated{false};
 
     bool IsOutboundOrBlockRelayConn() const {
         switch (m_conn_type) {
@@ -638,6 +641,7 @@ class CNode
     std::atomic<std::chrono::microseconds> m_min_ping_time{std::chrono::microseconds::max()};
 
     EllSwiftPubKey ellswift_pubkey;
+    std::vector<std::byte> peer_ellswift_buf;
 
     CNode(NodeId id,
           std::shared_ptr<Sock> sock,
@@ -726,6 +730,10 @@ class CNode
 
     std::list<CNetMessage> vRecvMsg; // Used only by SocketHandler thread
     CKey v2_priv_key;
+    std::array<std::byte, BIP324_GARBAGE_TERMINATOR_LEN> v2_sent_garbage_terminator;
+    std::array<std::byte, BIP324_GARBAGE_TERMINATOR_LEN> v2_recv_garbage_terminator;
+    std::vector<std::byte> v2_garbage_bytes_recd;
+    bool v2_keys_derived{false};
 
     // Our address, as reported by the peer
     CService addrLocal GUARDED_BY(m_addr_local_mutex);
@@ -886,6 +894,7 @@ class CConnman
 
     void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
     void PushV2EllSwiftPubkey(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
+    void PushV2GarbageTerminator(CNode* pnode);
 
     using NodeFn = std::function<void(CNode*)>;
     void ForEachNode(const NodeFn& func)
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp
@@ -86,6 +86,11 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE
     if (p2p_node.PreferV2Conn()) {
         InitTestV2P2P(fuzzed_data_provider, p2p_node, connman);
         const CNetMsgMaker mm{0};
+        if (p2p_node.IsInboundConn()) {
+            // pretend to have received the garbage terminator and garbage authentication message
+            p2p_node.v2_garbage_terminated = true;
+            p2p_node.m_authenticated_v2_garbage = true;
+        }
         // receive the transport version placeholder message
         CSerializedNetMsg dummy{mm.Make(NetMsgType::VERSION)};
         (void)connman.ReceiveMsgFrom(p2p_node, dummy);
diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp
@@ -52,6 +52,11 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages)
         if (p2p_node.PreferV2Conn()) {
             InitTestV2P2P(fuzzed_data_provider, p2p_node, connman);
             const CNetMsgMaker mm{0};
+            if (p2p_node.IsInboundConn()) {
+                // pretend to have received the garbage terminator and garbage authentication message
+                p2p_node.v2_garbage_terminated = true;
+                p2p_node.m_authenticated_v2_garbage = true;
+            }
             // receive the transport version placeholder message
             CSerializedNetMsg dummy{mm.Make(NetMsgType::VERSION)};
             (void)connman.ReceiveMsgFrom(p2p_node, dummy);
diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py
@@ -62,7 +62,6 @@ def set_test_params(self):
     def run_test(self):
         self.test_buffer()
         self.test_duplicate_version_msg()
-        self.test_magic_bytes()
         self.test_checksum()
         self.test_size()
         self.test_msgtype()
@@ -101,17 +100,6 @@ def test_duplicate_version_msg(self):
             conn.send_and_ping(msg_version())
         self.nodes[0].disconnect_p2ps()
 
-    def test_magic_bytes(self):
-        self.log.info("Test message with invalid magic bytes disconnects peer")
-        conn = self.nodes[0].add_p2p_connection(P2PDataStore())
-        with self.nodes[0].assert_debug_log(['Header error: Wrong MessageStart ffffffff received']):
-            msg = conn.build_message(msg_unrecognized(str_data="d"))
-            # modify magic bytes
-            msg = b'\xff' * 4 + msg[4:]
-            conn.send_raw_message(msg)
-            conn.wait_for_disconnect(timeout=1)
-        self.nodes[0].disconnect_p2ps()
-
     def test_checksum(self):
         self.log.info("Test message with invalid checksum logs an error")
         conn = self.nodes[0].add_p2p_connection(P2PDataStore())
