From 95c90838a02b53ce4fea28c4dcda4b09067a7f38 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Mon, 5 Oct 2026 12:55:51 -0400 Subject: [PATCH 01/12] net: record a first-cause-wins close cause Add NodeCloseCause and CNode::RequestDisconnect(cause). It records the cause with a compare-exchange from NONE before setting fDisconnect, so the first cause wins and later or concurrent calls, including cleanup, cannot relabel it. CloseSocketDisconnect(cause) goes through it. The socket sites record PEER_EOF, PEER_RESET and SEND_ERROR, InactivityCheck() and the ping timeout record TIMEOUT, and every other writer in net.cpp and net_processing.cpp records LOCAL by calling RequestDisconnect() instead of setting fDisconnect directly. A node that reaches DisconnectNodes() with no cause is finalized there as LOCAL. fDisconnect stays for its readers, and no protocol behavior changes. The close_cause_first_wins test drives each site through CConnman and PeerManager: EOF, reset, send error, inactivity, receive-parse failure, operator disconnect, network off, eviction, shutdown and the ping timeout. It also checks that repeated and racing calls keep the first cause and that no node is deleted without one. Work item F1 of #206 (#184 part 5). Co-Authored-By: Claude Opus 5.5 --- src/net.cpp | 32 ++++-- src/net.h | 40 +++++++- src/net_processing.cpp | 88 ++++++++-------- src/test/net_tests.cpp | 221 +++++++++++++++++++++++++++++++++++++++++ src/test/util/net.h | 13 +++ 5 files changed, 337 insertions(+), 57 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 2c330ce0dc..a597651b8f 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -557,9 +557,19 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo return nullptr; } -void CNode::CloseSocketDisconnect() +void CNode::RequestDisconnect(NodeCloseCause cause) noexcept { + Assume(cause != NodeCloseCause::NONE); + // Compare-exchange from NONE so the first cause wins, and record it before + // setting fDisconnect so a thread that sees fDisconnect also sees the cause. + NodeCloseCause expected{NodeCloseCause::NONE}; + m_close_cause.compare_exchange_strong(expected, cause); fDisconnect = true; +} + +void CNode::CloseSocketDisconnect(NodeCloseCause cause) +{ + RequestDisconnect(cause); LOCK(m_sock_mutex); if (m_sock) { LogDebug(BCLog::NET, "Resetting socket for peer=%d%s", GetId(), LogIP(fLogIPs)); @@ -1950,7 +1960,7 @@ std::pair CConnman::SocketSendData(CNode& node) const int nErr = WSAGetLastError(); if (nErr != WSAEWOULDBLOCK && nErr != WSAEMSGSIZE && nErr != WSAEINTR && nErr != WSAEINPROGRESS) { LogDebug(BCLog::NET, "socket send error, %s: %s\n", node.DisconnectMsg(fLogIPs), NetworkErrorString(nErr)); - node.CloseSocketDisconnect(); + node.CloseSocketDisconnect(NodeCloseCause::SEND_ERROR); } } break; @@ -2016,7 +2026,7 @@ bool CConnman::AttemptToEvictConnection() pnode->ConnectionTypeAsString().c_str(), pnode->ConnectedThroughNetwork(), Ticks(pnode->m_connected)); - pnode->fDisconnect = true; + pnode->RequestDisconnect(); return true; } } @@ -2210,7 +2220,7 @@ void CConnman::DisconnectNodes() for (CNode* pnode : m_nodes) { if (!pnode->fDisconnect) { LogDebug(BCLog::NET, "Network not active, %s\n", pnode->DisconnectMsg(fLogIPs)); - pnode->fDisconnect = true; + pnode->RequestDisconnect(); } } } @@ -2241,7 +2251,7 @@ void CConnman::DisconnectNodes() // release outbound grant (if any) pnode->grantOutbound.Release(); - // close socket and cleanup + // close socket and cleanup; a node with no recorded close cause counts as LOCAL pnode->CloseSocketDisconnect(); // update connection count by network @@ -2484,7 +2494,7 @@ void CConnman::SocketHandlerConnected(const std::vector& nodes, if (!pnode->fDisconnect) { LogDebug(BCLog::NET, "socket closed, %s\n", pnode->DisconnectMsg(fLogIPs)); } - pnode->CloseSocketDisconnect(); + pnode->CloseSocketDisconnect(NodeCloseCause::PEER_EOF); } else if (nBytes < 0) { @@ -2495,12 +2505,12 @@ void CConnman::SocketHandlerConnected(const std::vector& nodes, if (!pnode->fDisconnect) { LogDebug(BCLog::NET, "socket recv error, %s: %s\n", pnode->DisconnectMsg(fLogIPs), NetworkErrorString(nErr)); } - pnode->CloseSocketDisconnect(); + pnode->CloseSocketDisconnect(NodeCloseCause::PEER_RESET); } } } - if (InactivityCheck(*pnode)) pnode->fDisconnect = true; + if (InactivityCheck(*pnode)) pnode->RequestDisconnect(NodeCloseCause::TIMEOUT); } } @@ -3944,7 +3954,7 @@ bool CConnman::DisconnectNode(const std::string& strNode) LOCK(m_nodes_mutex); if (CNode* pnode = FindNode(strNode)) { LogDebug(BCLog::NET, "disconnect by address%s match, %s", (fLogIPs ? strprintf("=%s", strNode) : ""), pnode->DisconnectMsg(fLogIPs)); - pnode->fDisconnect = true; + pnode->RequestDisconnect(); return true; } return false; @@ -3957,7 +3967,7 @@ bool CConnman::DisconnectNode(const CSubNet& subnet) for (CNode* pnode : m_nodes) { if (subnet.Match(pnode->addr)) { LogDebug(BCLog::NET, "disconnect by subnet%s match, %s", (fLogIPs ? strprintf("=%s", subnet.ToString()) : ""), pnode->DisconnectMsg(fLogIPs)); - pnode->fDisconnect = true; + pnode->RequestDisconnect(); disconnected = true; } } @@ -3975,7 +3985,7 @@ bool CConnman::DisconnectNode(NodeId id) for(CNode* pnode : m_nodes) { if (id == pnode->GetId()) { LogDebug(BCLog::NET, "disconnect by id, %s", pnode->DisconnectMsg(fLogIPs)); - pnode->fDisconnect = true; + pnode->RequestDisconnect(); return true; } } diff --git a/src/net.h b/src/net.h index 8f061ed795..ccc208d03d 100644 --- a/src/net.h +++ b/src/net.h @@ -749,6 +749,27 @@ class V2Transport final : public Transport void DiscardHybridSecretForTesting() noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex, !m_send_mutex); }; +/** + * Why a connection closed. The first recorded cause wins (see + * CNode::RequestDisconnect()), so cleanup that runs after a peer closed the + * connection never relabels it. + */ +enum class NodeCloseCause : uint8_t { + //! No cause recorded. A node that reaches DisconnectNodes() without one + //! (fDisconnect set directly) is finalized there as LOCAL. + NONE = 0, + //! Our own decision: operator, eviction, feeler, protocol policy, network off or shutdown. + LOCAL = 1, + //! recv() returned 0: the peer closed the connection. + PEER_EOF = 2, + //! recv() failed with a non-transient error, such as a connection reset. + PEER_RESET = 3, + //! send() failed with a non-transient error. + SEND_ERROR = 4, + //! The peer stopped responding: InactivityCheck() or the ping timeout. + TIMEOUT = 5, +}; + struct CNodeOptions { NetPermissionFlags permission_flags = NetPermissionFlags::None; @@ -818,7 +839,7 @@ class CNode /** fSuccessfullyConnected is set to true on receiving VERACK from the peer. */ std::atomic_bool fSuccessfullyConnected{false}; // Setting fDisconnect to true will cause the node to be disconnected the - // next time DisconnectNodes() runs + // next time DisconnectNodes() runs. Set it with RequestDisconnect(). std::atomic_bool fDisconnect{false}; CountingSemaphoreGrant<> grantOutbound; std::atomic nRefCount{0}; @@ -1033,7 +1054,19 @@ class CNode nRefCount--; } - void CloseSocketDisconnect() EXCLUSIVE_LOCKS_REQUIRED(!m_sock_mutex); + /** + * Mark the node for disconnection and record why. Only the first cause is + * kept: later or concurrent calls, including cleanup, leave it unchanged. + * The cause is recorded before fDisconnect is set, so a thread that sees + * fDisconnect also sees the cause. + */ + void RequestDisconnect(NodeCloseCause cause = NodeCloseCause::LOCAL) noexcept; + + /** RequestDisconnect(cause), then close the socket. */ + void CloseSocketDisconnect(NodeCloseCause cause = NodeCloseCause::LOCAL) EXCLUSIVE_LOCKS_REQUIRED(!m_sock_mutex); + + /** The first recorded close cause, or NONE if none has been recorded. */ + NodeCloseCause GetCloseCause() const noexcept { return m_close_cause.load(); } void CopyStats(CNodeStats& stats) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv); @@ -1066,6 +1099,9 @@ class CNode const uint64_t nLocalHostNonce; std::atomic m_greatest_common_version{INIT_PROTO_VERSION}; + /** Written only by RequestDisconnect(), which keeps the first cause. */ + std::atomic m_close_cause{NodeCloseCause::NONE}; + const size_t m_recv_flood_size; std::list vRecvMsg; // Used only by SocketHandler thread diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 047d470352..dfdfa2e0d4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2397,7 +2397,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& !pfrom.HasPermission(NetPermissionFlags::Download) // nodes with the download permission may exceed target ) { LogDebug(BCLog::NET, "historical block serving limit reached, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } tip = m_chainman.ActiveChain().Tip(); @@ -2407,7 +2407,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& )) { LogDebug(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold, %s\n", pfrom.DisconnectMsg(fLogIPs)); //disconnect node and prevent it from stalling (would otherwise wait for the missing block) - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } // Pruned nodes may have deleted the block, so check whether @@ -2439,7 +2439,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } else { LogError("Cannot load block from disk, %s\n", pfrom.DisconnectMsg(fLogIPs)); } - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } MakeAndPushMessage(pfrom, NetMsgType::BLOCK, std::span{block_data}); @@ -2453,7 +2453,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } else { LogError("Cannot load block from disk, %s\n", pfrom.DisconnectMsg(fLogIPs)); } - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } pblock = pblockRead; @@ -2974,7 +2974,7 @@ void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, Peer& peer // as an anti-DoS measure. if (pfrom.IsOutboundOrBlockRelayConn()) { LogInfo("outbound peer headers chain has insufficient work, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); } } } @@ -3305,7 +3305,7 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, if (!supported_filter_type) { LogDebug(BCLog::NET, "peer requested unsupported block filter type: %d, %s\n", static_cast(filter_type), node.DisconnectMsg(fLogIPs)); - node.fDisconnect = true; + node.RequestDisconnect(); return false; } @@ -3317,7 +3317,7 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, if (!stop_index || !BlockRequestAllowed(stop_index)) { LogDebug(BCLog::NET, "peer requested invalid block hash: %s, %s\n", stop_hash.ToString(), node.DisconnectMsg(fLogIPs)); - node.fDisconnect = true; + node.RequestDisconnect(); return false; } } @@ -3327,13 +3327,13 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, LogDebug(BCLog::NET, "peer sent invalid getcfilters/getcfheaders with " "start height %d and stop height %d, %s\n", start_height, stop_height, node.DisconnectMsg(fLogIPs)); - node.fDisconnect = true; + node.RequestDisconnect(); return false; } if (stop_height - start_height >= max_height_diff) { LogDebug(BCLog::NET, "peer requested too many cfilters/cfheaders: %d / %d, %s\n", stop_height - start_height + 1, max_height_diff, node.DisconnectMsg(fLogIPs)); - node.fDisconnect = true; + node.RequestDisconnect(); return false; } @@ -3630,7 +3630,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, nServices, GetDesirableServiceFlags(nServices), pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } if ((pfrom.m_conn_type == ConnectionType::OUTBOUND_FULL_RELAY || @@ -3640,13 +3640,13 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, LogDebug(BCLog::NET, "peer advertises NODE_WITNESS_PRUNED in full-validation mode (%08x), %s\n", nServices, pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } if (pfrom.m_is_archive_connection && (nServices & NODE_WITNESS_PRUNED)) { LogPrintf("Archive peer advertises NODE_WITNESS_PRUNED, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } if (pfrom.m_is_archive_connection && !HasArchiveServiceFlags(nServices)) { @@ -3654,14 +3654,14 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, nServices, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_ARCHIVE), pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } if (nVersion < MIN_PEER_PROTO_VERSION) { // disconnect from peers older than this proto version LogDebug(BCLog::NET, "peer using obsolete version %i, %s\n", nVersion, pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } @@ -3687,7 +3687,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (pfrom.IsInboundConn() && !m_connman.CheckIncomingNonce(nNonce)) { LogPrintf("connected to self at %s, disconnecting\n", pfrom.addr.ToStringAddrPort()); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } @@ -3839,7 +3839,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Feeler connections exist only to verify if address is online. if (pfrom.IsFeelerConn()) { LogDebug(BCLog::NET, "feeler connection completed, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); } return; } @@ -3943,7 +3943,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (pfrom.fSuccessfullyConnected) { // Disconnect peers that send a wtxidrelay message after VERACK. LogDebug(BCLog::NET, "wtxidrelay received after verack, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } if (pfrom.GetCommonVersion() >= WTXID_RELAY_VERSION) { @@ -3965,7 +3965,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (pfrom.fSuccessfullyConnected) { // Disconnect peers that send a SENDADDRV2 message after VERACK. LogDebug(BCLog::NET, "sendaddrv2 received after verack, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } peer->m_wants_addrv2 = true; @@ -3983,14 +3983,14 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (pfrom.fSuccessfullyConnected) { LogDebug(BCLog::NET, "sendtxrcncl received after verack, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } // Peer must not offer us reconciliations if we specified no tx relay support in VERSION. if (RejectIncomingTxs(pfrom)) { LogDebug(BCLog::NET, "sendtxrcncl received to which we indicated no tx relay, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } @@ -4000,7 +4000,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const auto* tx_relay = peer->GetTxRelay(); if (!tx_relay || !WITH_LOCK(tx_relay->m_bloom_filter_mutex, return tx_relay->m_relay_txs)) { LogDebug(BCLog::NET, "sendtxrcncl received which indicated no tx relay to us, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } @@ -4018,11 +4018,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, break; case ReconciliationRegisterResult::ALREADY_REGISTERED: LogDebug(BCLog::NET, "txreconciliation protocol violation (sendtxrcncl received from already registered peer), %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; case ReconciliationRegisterResult::PROTOCOL_VIOLATION: LogDebug(BCLog::NET, "txreconciliation protocol violation, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } return; @@ -4125,7 +4125,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // AddrFetch: Require multiple addresses to avoid disconnecting on self-announcements if (pfrom.IsAddrFetchConn() && vAddr.size() > 1) { LogDebug(BCLog::NET, "addrfetch connection completed, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); } return; } @@ -4175,7 +4175,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } else if (inv.IsGenTxMsg()) { if (reject_tx_invs) { LogDebug(BCLog::NET, "transaction (%s) inv sent in violation of protocol, %s\n", inv.hash.ToString(), pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } const GenTxid gtxid = ToGenTxid(inv); @@ -4252,7 +4252,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (locator.vHave.size() > MAX_LOCATOR_SZ) { LogDebug(BCLog::NET, "getblocks locator size %lld > %d, %s\n", locator.vHave.size(), MAX_LOCATOR_SZ, pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } @@ -4374,7 +4374,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (locator.vHave.size() > MAX_LOCATOR_SZ) { LogDebug(BCLog::NET, "getheaders locator size %lld > %d, %s\n", locator.vHave.size(), MAX_LOCATOR_SZ, pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } @@ -4450,7 +4450,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (msg_type == NetMsgType::TX) { if (RejectIncomingTxs(pfrom)) { LogDebug(BCLog::NET, "transaction sent in violation of protocol, %s", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } @@ -4937,7 +4937,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (!pfrom.HasPermission(NetPermissionFlags::NoBan)) { LogDebug(BCLog::NET, "mempool request with bloom filters disabled, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); } return; } @@ -4947,7 +4947,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (!pfrom.HasPermission(NetPermissionFlags::NoBan)) { LogDebug(BCLog::NET, "mempool request with bandwidth limit reached, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); } return; } @@ -5037,7 +5037,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (msg_type == NetMsgType::FILTERLOAD) { if (!(peer->m_our_services & NODE_BLOOM)) { LogDebug(BCLog::NET, "filterload received despite not offering bloom services, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } CBloomFilter filter; @@ -5062,7 +5062,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (msg_type == NetMsgType::FILTERADD) { if (!(peer->m_our_services & NODE_BLOOM)) { LogDebug(BCLog::NET, "filteradd received despite not offering bloom services, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } std::vector vData; @@ -5090,7 +5090,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (msg_type == NetMsgType::FILTERCLEAR) { if (!(peer->m_our_services & NODE_BLOOM)) { LogDebug(BCLog::NET, "filterclear received despite not offering bloom services, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); return; } auto tx_relay = peer->GetTxRelay(); @@ -5183,7 +5183,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (!(old_services & NODE_WITNESS_PRUNED) && pfrom.m_is_archive_connection) { LogPrintf("Archive peer implies NODE_WITNESS_PRUNED via NOTFOUND, %s\n", pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); } RemoveBlockRequest(block_hash, pfrom.GetId()); continue; @@ -5191,7 +5191,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, LogInfo("Peer sent NOTFOUND for in-flight block %s, %s\n", block_hash.ToString(), pfrom.DisconnectMsg(fLogIPs)); - pfrom.fDisconnect = true; + pfrom.RequestDisconnect(); } m_txdownloadman.ReceivedNotFound(pfrom.GetId(), tx_invs); return; @@ -5230,7 +5230,7 @@ bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer) // all peers on the same local address) LogDebug(BCLog::NET, "Warning: disconnecting but not discouraging %s peer %d!\n", pnode.m_inbound_onion ? "inbound onion" : "local", peer.m_id); - pnode.fDisconnect = true; + pnode.RequestDisconnect(); return true; } @@ -5359,7 +5359,7 @@ void PeerManagerImpl::ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seco if (state.m_chain_sync.m_sent_getheaders) { // They've run out of time to catch up! LogInfo("Outbound peer has old chain, best known block = %s, %s\n", state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "", pto.DisconnectMsg(fLogIPs)); - pto.fDisconnect = true; + pto.RequestDisconnect(); } else { assert(state.m_chain_sync.m_work_header); // Here, we assume that the getheaders message goes out, @@ -5417,7 +5417,7 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) CNodeState *node_state = State(pnode->GetId()); if (node_state == nullptr || (now - pnode->m_connected >= MINIMUM_CONNECT_TIME && node_state->vBlocksInFlight.empty())) { - pnode->fDisconnect = true; + pnode->RequestDisconnect(); LogDebug(BCLog::NET, "disconnecting extra block-relay-only peer=%d (last block received at time %d)\n", pnode->GetId(), count_seconds(pnode->m_last_block_time)); return true; @@ -5470,7 +5470,7 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) CNodeState &state = *State(pnode->GetId()); if (now - pnode->m_connected > MINIMUM_CONNECT_TIME && state.vBlocksInFlight.empty()) { LogDebug(BCLog::NET, "disconnecting extra outbound peer=%d (last block announcement received at time %d)\n", pnode->GetId(), oldest_block_announcement); - pnode->fDisconnect = true; + pnode->RequestDisconnect(); return true; } else { LogDebug(BCLog::NET, "keeping outbound peer=%d chosen for eviction (connect time: %d, blocks_in_flight: %d)\n", @@ -5526,7 +5526,7 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic // The ping timeout is using mocktime. To disable the check during // testing, increase -peertimeout. LogDebug(BCLog::NET, "ping timeout: %fs, %s", 0.000001 * count_microseconds(now - peer.m_ping_start.load()), node_to.DisconnectMsg(fLogIPs)); - node_to.fDisconnect = true; + node_to.RequestDisconnect(NodeCloseCause::TIMEOUT); return; } @@ -5755,7 +5755,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (pto->IsAddrFetchConn() && current_time - pto->m_connected > 10 * AVG_ADDRESS_BROADCAST_INTERVAL) { LogDebug(BCLog::NET, "addrfetch connection timeout, %s\n", pto->DisconnectMsg(fLogIPs)); - pto->fDisconnect = true; + pto->RequestDisconnect(); return true; } @@ -6103,7 +6103,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // the download window should be much larger than the to-be-downloaded set of blocks, so disconnection // should only happen during initial block download. LogInfo("Peer is stalling block download, %s\n", pto->DisconnectMsg(fLogIPs)); - pto->fDisconnect = true; + pto->RequestDisconnect(); // Increase timeout for the next peer so that we don't disconnect multiple peers if our own // bandwidth is insufficient. const auto new_timeout = std::min(2 * stalling_timeout, BLOCK_STALLING_TIMEOUT_MAX); @@ -6123,7 +6123,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) const auto block_download_timeout_interval = std::max(std::chrono::seconds{consensusParams.nPowTargetSpacing}, BLOCK_DOWNLOAD_TIMEOUT_INTERVAL_MIN); if (current_time > state.m_downloading_since + block_download_timeout_interval * (BLOCK_DOWNLOAD_TIMEOUT_BASE + BLOCK_DOWNLOAD_TIMEOUT_PER_PEER * nOtherPeersWithValidatedDownloads)) { LogInfo("Timeout downloading block %s, %s\n", queuedBlock.pindex->GetBlockHash().ToString(), pto->DisconnectMsg(fLogIPs)); - pto->fDisconnect = true; + pto->RequestDisconnect(); return true; } } @@ -6139,7 +6139,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // problems if we can't get any outbound peers. if (!pto->HasPermission(NetPermissionFlags::NoBan)) { LogInfo("Timeout downloading headers, %s\n", pto->DisconnectMsg(fLogIPs)); - pto->fDisconnect = true; + pto->RequestDisconnect(); return true; } else { LogInfo("Timeout downloading headers from noban peer, not %s\n", pto->DisconnectMsg(fLogIPs)); diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 5f0c37ea2a..ebcb9adfb0 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -34,11 +35,16 @@ #include #include +#include +#include #include #include +#include +#include #include #include #include +#include #include using namespace std::literals; @@ -179,6 +185,221 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) BOOST_CHECK_EQUAL(pnode4->ConnectedThroughNetwork(), Network::NET_ONION); } +namespace { +//! A socket whose Send() and Recv() fail as if the peer reset the connection. +class ResetSock : public ZeroSock +{ +public: + ssize_t Send(const void*, size_t, int) const override { return Fail(); } + ssize_t Recv(void*, size_t, int) const override { return Fail(); } + +private: + static ssize_t Fail() + { +#ifdef WIN32 + WSASetLastError(WSAECONNRESET); +#else + errno = ECONNRESET; +#endif + return -1; + } + + ResetSock& operator=(Sock&&) override + { + assert(false && "Move of Sock into ResetSock not allowed."); + return *this; + } +}; + +//! Message processing that only records each node's close cause when CConnman deletes it. +class CloseCauseRecorder final : public NetEventsInterface +{ +public: + std::map m_finalized; + + void InitializeNode(const CNode&, ServiceFlags) override {} + void FinalizeNode(const CNode& node) override { m_finalized.emplace(node.GetId(), node.GetCloseCause()); } + bool HasAllDesirableServiceFlags(ServiceFlags) const override { return true; } + bool HasUndesirableServiceFlags(ServiceFlags) const override { return false; } + bool ProcessMessages(CNode*, std::atomic&) override EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) { return false; } + bool SendMessages(CNode*) override EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) { return false; } +}; +} // namespace + +BOOST_AUTO_TEST_CASE(close_cause_first_wins) +{ + constexpr std::array causes{NodeCloseCause::PEER_EOF, NodeCloseCause::PEER_RESET, NodeCloseCause::SEND_ERROR, + NodeCloseCause::TIMEOUT, NodeCloseCause::LOCAL}; + NodeId id{0}; + const auto new_node{[&](std::shared_ptr sock, ConnectionType conn_type = ConnectionType::OUTBOUND_FULL_RELAY) { + return new CNode{id++, + std::move(sock), + CAddress{LookupNumeric("1.2.3.4", 8333), NODE_NONE}, + /*nKeyedNetGroupIn=*/0, + /*nLocalHostNonceIn=*/0, + CAddress{}, + /*addrNameIn=*/"", + conn_type, + /*inbound_onion=*/false}; + }}; + + // Each cause is recorded, and repeated cleanup keeps it. + for (const auto cause : causes) { + const std::unique_ptr node{new_node(nullptr)}; + BOOST_CHECK(node->GetCloseCause() == NodeCloseCause::NONE); + BOOST_CHECK(!node->fDisconnect); + node->RequestDisconnect(cause); + BOOST_CHECK(node->GetCloseCause() == cause); + BOOST_CHECK(node->fDisconnect); + for (const auto later : causes) { + node->RequestDisconnect(later); + node->CloseSocketDisconnect(later); + } + node->CloseSocketDisconnect(); + BOOST_CHECK(node->GetCloseCause() == cause); + } + + // Racing writers, like the socket, message handler and RPC threads, all + // see the same cause once their own call returns. + for (int round{0}; round < 50; ++round) { + const std::unique_ptr node{new_node(nullptr)}; + std::atomic start{false}; + std::array seen{}; + std::vector threads; + for (size_t i{0}; i < causes.size(); ++i) { + threads.emplace_back([&, i] { + while (!start) std::this_thread::yield(); + node->CloseSocketDisconnect(causes[i]); + node->RequestDisconnect(); + seen[i] = node->GetCloseCause(); + }); + } + start = true; + for (auto& thread : threads) thread.join(); + const NodeCloseCause winner{node->GetCloseCause()}; + BOOST_CHECK(winner != NodeCloseCause::NONE); + for (const auto cause : seen) BOOST_CHECK(cause == winner); + } + + // Each disconnect site records its cause. + CloseCauseRecorder events; + ConnmanTestMsg connman{0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()}; + connman.SetMsgProc(&events); + connman.SetPeerConnectTimeout(60s); + const auto now{GetTime()}; + SetMockTime(now); + + const std::shared_ptr eof_sock{std::make_shared("")}; + const std::shared_ptr reset_sock{std::make_shared()}; + CNode* eof_node{new_node(eof_sock)}; + CNode* reset_node{new_node(reset_sock)}; + CNode* send_node{new_node(std::make_shared())}; + CNode* timeout_node{new_node(std::make_shared())}; + CNode* local_node{new_node(std::make_shared())}; + CNode* unexplained_node{new_node(std::make_shared())}; + // Zeros never start with the network magic, so the transport rejects them. + const std::shared_ptr garbage_sock{std::make_shared()}; + CNode* garbage_node{new_node(garbage_sock)}; + CNode* network_off_node{new_node(std::make_shared())}; + const std::vector nodes{eof_node, reset_node, send_node, timeout_node, local_node, unexplained_node, garbage_node, network_off_node}; + for (CNode* node : nodes) { + node->AddRef(); // the reference m_nodes holds, as in ConnectNode() + node->AddRef(); // keeps the node alive through DisconnectNodes() below + connman.AddTestNode(*node); + } + + // Our own decision, as from the RPC thread. + BOOST_CHECK(connman.DisconnectNode(local_node->GetId())); + BOOST_CHECK(local_node->GetCloseCause() == NodeCloseCause::LOCAL); + // A failed optimistic send, as from the message handler thread. + connman.PushMessage(send_node, NetMsg::Make(NetMsgType::VERACK)); + BOOST_CHECK(send_node->GetCloseCause() == NodeCloseCause::SEND_ERROR); + // Setting fDisconnect directly leaves the cause unknown. + unexplained_node->fDisconnect = true; + + // One socket handler pass after the inactivity timeout, so the timeout + // check runs for every node with an open socket after it was read. + SetMockTime(now + 61s); + Sock::EventsPerSock events_per_sock; + for (const auto& sock : {eof_sock, reset_sock, garbage_sock}) { + events_per_sock.emplace(sock, Sock::Events{Sock::RECV}).first->second.occurred = Sock::RECV; + } + connman.SocketHandlerConnectedPublic({eof_node, reset_node, send_node, timeout_node, local_node, garbage_node}, events_per_sock); + BOOST_CHECK(eof_node->GetCloseCause() == NodeCloseCause::PEER_EOF); + BOOST_CHECK(reset_node->GetCloseCause() == NodeCloseCause::PEER_RESET); + BOOST_CHECK(send_node->GetCloseCause() == NodeCloseCause::SEND_ERROR); + BOOST_CHECK(timeout_node->GetCloseCause() == NodeCloseCause::TIMEOUT); + BOOST_CHECK(local_node->GetCloseCause() == NodeCloseCause::LOCAL); + BOOST_CHECK(garbage_node->GetCloseCause() == NodeCloseCause::LOCAL); + BOOST_CHECK(unexplained_node->GetCloseCause() == NodeCloseCause::NONE); + BOOST_CHECK(network_off_node->GetCloseCause() == NodeCloseCause::NONE); + + // With the network off, DisconnectNodes() disconnects the remaining node + // as LOCAL. Finalization counts the unexplained disconnect as LOCAL and + // keeps every recorded cause. + connman.SetNetworkActive(false); + connman.DisconnectNodesPublic(); + connman.SetNetworkActive(true); + BOOST_CHECK(connman.TestNodes().empty()); + BOOST_CHECK(eof_node->GetCloseCause() == NodeCloseCause::PEER_EOF); + BOOST_CHECK(reset_node->GetCloseCause() == NodeCloseCause::PEER_RESET); + BOOST_CHECK(send_node->GetCloseCause() == NodeCloseCause::SEND_ERROR); + BOOST_CHECK(timeout_node->GetCloseCause() == NodeCloseCause::TIMEOUT); + BOOST_CHECK(local_node->GetCloseCause() == NodeCloseCause::LOCAL); + BOOST_CHECK(garbage_node->GetCloseCause() == NodeCloseCause::LOCAL); + BOOST_CHECK(unexplained_node->GetCloseCause() == NodeCloseCause::LOCAL); + BOOST_CHECK(network_off_node->GetCloseCause() == NodeCloseCause::LOCAL); + + for (CNode* node : nodes) node->Release(); + connman.DisconnectNodesPublic(); // deletes the released nodes + + // Eviction records LOCAL. Enough inbound candidates that some survive the + // protections in SelectNodeToEvict(); outbound connections are never evicted. + std::vector inbound_nodes; + for (int i{0}; i < 40; ++i) { + inbound_nodes.push_back(new_node(nullptr, ConnectionType::INBOUND)); + } + CNode* shutdown_node{new_node(std::make_shared())}; + for (CNode* node : inbound_nodes) { + node->AddRef(); + connman.AddTestNode(*node); + } + shutdown_node->AddRef(); + connman.AddTestNode(*shutdown_node); + BOOST_CHECK(connman.AttemptToEvictConnectionPublic()); + std::vector evicted; + std::copy_if(inbound_nodes.begin(), inbound_nodes.end(), std::back_inserter(evicted), [](const CNode* node) { return node->fDisconnect.load(); }); + BOOST_CHECK_EQUAL(evicted.size(), 1U); + for (const CNode* node : evicted) BOOST_CHECK(node->GetCloseCause() == NodeCloseCause::LOCAL); + + // Shutdown deletes the remaining nodes and records LOCAL for any without a cause. + const NodeId shutdown_id{shutdown_node->GetId()}; + connman.Stop(); + BOOST_CHECK(connman.TestNodes().empty()); + BOOST_CHECK(events.m_finalized.at(shutdown_id) == NodeCloseCause::LOCAL); + for (const auto& [finalized_id, cause] : events.m_finalized) BOOST_CHECK(cause != NodeCloseCause::NONE); + + // The ping timeout records TIMEOUT, from the message handler thread. + { + LOCK(NetEventsInterface::g_msgproc_mutex); + auto& fixture_connman{static_cast(*m_node.connman)}; + const std::unique_ptr ping_node{new_node(nullptr, ConnectionType::INBOUND)}; + // The handshake ends by sending the first ping. + fixture_connman.Handshake(*ping_node, + /*successfully_connected=*/true, + /*remote_services=*/ServiceFlags(NODE_NETWORK | NODE_WITNESS), + /*local_services=*/ServiceFlags(NODE_NETWORK | NODE_WITNESS), + /*version=*/PROTOCOL_VERSION, + /*relay_txs=*/true); + BOOST_CHECK(!ping_node->fDisconnect); + SetMockTime(GetTime() + TIMEOUT_INTERVAL + 1s); + m_node.peerman->SendMessages(ping_node.get()); + BOOST_CHECK(ping_node->GetCloseCause() == NodeCloseCause::TIMEOUT); + m_node.peerman->FinalizeNode(*ping_node); + } + SetMockTime(0s); +} + BOOST_AUTO_TEST_CASE(cnetaddr_basic) { CNetAddr addr; diff --git a/src/test/util/net.h b/src/test/util/net.h index 3af8dfba13..f578a111b4 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -93,6 +93,19 @@ struct ConnmanTestMsg : public CConnman { CNode* ConnectNodePublic(PeerManager& peerman, const char* pszDest, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); + + void SocketHandlerConnectedPublic(const std::vector& nodes, const Sock::EventsPerSock& events_per_sock) + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc) + { + SocketHandlerConnected(nodes, events_per_sock); + } + + void DisconnectNodesPublic() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !m_reconnections_mutex) + { + DisconnectNodes(); + } + + bool AttemptToEvictConnectionPublic() { return AttemptToEvictConnection(); } }; constexpr ServiceFlags ALL_SERVICE_FLAGS[]{ From 2506e368d5eacb4535956ea77a7107fa0c27bd21 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Mon, 5 Oct 2026 15:13:56 -0400 Subject: [PATCH 02/12] net: attribute PQ outcomes per direction with bounded failure rings Count every hybrid negotiation outcome exactly once, per direction, and keep the recent non-success outcomes as evidence. This is the data source for the fallback (F3), load shedding (F4) and the RPC (F6). CConnman::ObservePQ() snapshots the transport under its own lock, releases it, and only then takes m_pq_mutex; nothing calls into the transport, socket, peer manager or node enumeration while holding it. It runs after ReceiveMsgBytes() and once more, finalizing, in DisconnectNodes() before the disconnected pool keeps the node. Each CNode records the events already accounted, so an event seen after receive, at disconnect and in cleanup counts once. Finalization derives closed_after_switch (outbound, switched but unconfirmed, closed by EOF, reset or timeout) and abandoned (an offer queued, no authenticated version) from the F1 close cause, marks the node final and wipes the transport's pending secrets. With the network off it wipes but records nothing, since every close is then our own. - Transport gains GetPQSnapshot() and ClearPQSecrets(), with no-op defaults for V1Transport. The snapshot gains the legacy reason (no_features or parse_error) and the transport's own reason for an abandoned offer (garbage_terminator, version_length, version_tag), set at V2Transport's existing failure sites. - CNode keeps a typed PQEndpointKey captured at creation through CNodeOptions::pq_endpoint: the resolved CService with its port, the normalized hostname and port of a name-proxy destination (never the proxy's address), or an inbound peer's remote address and source port. CNodeOptions::pq carries the negotiation mode to the transport; CConnman's mode stays off until part 6 adds the option. - Counters count events: switched at key installation, even if the confirmation fails; each failure once; abandoned, closed_after_switch and the shed placeholder per direction. - Two rings of at most 256 entries, inbound and outbound (manual included), each with its own sequence and dropped count, so inbound noise can't evict outbound evidence. Reasons come from the fixed vocabulary only. - instance_id and since identify the process's counting period, and GetPQTransportStats() returns an owning copy. - One "v2 pq:" line per outcome, with fixed fields. Outbound failures log at info level, each from its own call site; inbound outcomes, switched, legacy_peer and abandoned are net debug, from separate call sites so inbound noise can't rate-limit outbound lines. net_pq_tests covers once-only observation (including racing observers), endpoint identity, the ring bounds, switched-then-failed, and every abandoned and closed_after_switch cause; a confirmed handshake that the peer later closes, or that times out, counts nothing. It checks the exact text and level of each outcome's log line in both directions (info for outbound failures, net debug otherwise), and the status each failure reports (off for a local fault before commit, pending while a failed connection closes). pq_cipher_state_closes checks that an inconsistent cipher state closes in both roles, the initiator before it commits included, and counts once as internal_error/cipher_state. pq_finalization_wipes checks, for every close cause (EOF, reset, timeout, local, send error, network off, transport failure) and both roles, that the pending secrets survive the socket close and are gone once the connection is finalized, before the node is deleted. pq_name_proxy_endpoint drives ConnectNode() through a name proxy (a mocked SOCKS5 server) and checks that the endpoint key is the normalized destination and port, never the proxy. ProxySettingsRestorerForTesting, a test-only netbase guard, saves and restores the proxy settings so that no later test sees them. Production construction stays PQ-off. Work item F2 of #206 (#184 part 5). Co-Authored-By: Claude Opus 5.5 --- src/bip324_pq.h | 19 + src/net.cpp | 318 +++++++++++- src/net.h | 175 ++++++- src/netbase.cpp | 16 + src/netbase.h | 18 + src/test/CMakeLists.txt | 1 + src/test/net_pq_tests.cpp | 1016 +++++++++++++++++++++++++++++++++++++ src/test/util/net.h | 23 +- 8 files changed, 1575 insertions(+), 11 deletions(-) create mode 100644 src/test/net_pq_tests.cpp diff --git a/src/bip324_pq.h b/src/bip324_pq.h index d8f0f8cf8a..73462f0d4d 100644 --- a/src/bip324_pq.h +++ b/src/bip324_pq.h @@ -70,6 +70,13 @@ enum class PQFailure : uint8_t { CIPHER_STATE_INTERNAL = 11, }; +/** Why the peer counts as a legacy peer: its contents carried no own record, or did not parse. */ +enum class PQLegacyReason : uint8_t { NONE = 0, NO_FEATURES = 1, PARSE_ERROR = 2 }; + +/** Why the transport closed before the peer's version packet authenticated (the transport's own + * reasons for an abandoned offer). The first one recorded is kept. */ +enum class PQAbort : uint8_t { NONE = 0, GARBAGE_TERMINATOR = 1, VERSION_LENGTH = 2, VERSION_TAG = 3 }; + /** Entropy for key generation and encapsulation, 32 bytes per call. */ struct PQRandomSource { using Fill32 = void (*)(void*, std::span) noexcept; @@ -118,6 +125,9 @@ class PQHandshake //! The peer's key confirmation verified. bool confirmed{false}; PQFailure failure{PQFailure::NONE}; + //! Set when the negotiation ended with a legacy peer. + PQLegacyReason legacy{PQLegacyReason::NONE}; + PQAbort abort{PQAbort::NONE}; }; /** @@ -180,6 +190,15 @@ class PQHandshake void SetVersionReceived() noexcept { m_snapshot.version_received = true; } void SetSwitched() noexcept { m_snapshot.switched = true; } void SetConfirmed() noexcept { m_snapshot.confirmed = true; } + /** The peer's contents parsed as kind, without an own record: a legacy peer. */ + void SetLegacy(ParseKind kind) noexcept + { + m_snapshot.legacy = kind == ParseKind::INVALID_GRAMMAR ? PQLegacyReason::PARSE_ERROR : PQLegacyReason::NO_FEATURES; + } + void SetAbort(PQAbort abort) noexcept + { + if (m_snapshot.abort == PQAbort::NONE) m_snapshot.abort = abort; + } /** Record a failure unless one is recorded already, and wipe the secrets. */ void SetFailure(PQFailure failure) noexcept; Snapshot GetSnapshot() const noexcept { return m_snapshot; } diff --git a/src/net.cpp b/src/net.cpp index a597651b8f..9f3e5e2c9d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -462,6 +463,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo std::unique_ptr i2p_transient_session; for (auto& target_addr: connect_to) { + // What the hybrid negotiation is accounted to: the destination, never a proxy. + PQEndpointKey pq_endpoint{CService{target_addr}}; if (target_addr.IsValid()) { const bool use_proxy{GetProxy(target_addr.GetNetwork(), proxy)}; bool proxyConnectionFailed = false; @@ -512,6 +515,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo std::string host; uint16_t port{default_port}; SplitHostPort(std::string(pszDest), port, host); + pq_endpoint = MakePQNameEndpoint(host, port); bool proxyConnectionFailed; sock = ConnectThroughProxy(proxy, host, port, proxyConnectionFailed); } @@ -545,6 +549,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo .recv_flood_size = nReceiveFloodSize, .use_v2transport = use_v2transport, .is_archive_connection = is_archive_connection, + .pq = {.mode = m_pq_mode}, + .pq_endpoint = std::move(pq_endpoint), }); pnode->AddRef(); @@ -1240,6 +1246,7 @@ bool V2Transport::ProcessReceivedGarbageBytes() noexcept // We've reached the maximum length for garbage + garbage terminator, and the // terminator still does not match. Abort. LogDebug(BCLog::NET, "V2 transport error: missing garbage terminator, peer=%d\n", m_nodeid); + m_pq.SetAbort(PQAbort::GARBAGE_TERMINATOR); return false; } else { // We still need to receive more garbage and/or garbage terminator bytes. @@ -1277,6 +1284,7 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept } if (m_recv_len > MAX_CONTENTS_LEN) { LogDebug(BCLog::NET, "V2 transport error: packet too large (%u bytes), peer=%d\n", m_recv_len, m_nodeid); + if (m_recv_state == RecvState::VERSION) m_pq.SetAbort(PQAbort::VERSION_LENGTH); return false; } } else if (m_recv_buffer.size() > BIP324Cipher::LENGTH_LEN && m_recv_buffer.size() == m_recv_len + BIP324Cipher::EXPANSION) { @@ -1294,6 +1302,7 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept /*contents=*/MakeWritableByteSpan(m_recv_decode_buffer)); if (!ret) { LogDebug(BCLog::NET, "V2 transport error: packet decryption failure (%u bytes), peer=%d\n", m_recv_len, m_nodeid); + if (m_recv_state == RecvState::VERSION) m_pq.SetAbort(PQAbort::VERSION_TAG); return false; } // We have decrypted a valid packet with the AAD we expected, so clear the expected AAD. @@ -1383,6 +1392,7 @@ bool V2Transport::ProcessVersionContents(std::span contents) no // keys; an initiator now sends its empty version packet. const auto parsed{PQHandshake::ParseContents(contents)}; if (parsed.kind != PQHandshake::ParseKind::OWN_RECORD) { + m_pq.SetLegacy(parsed.kind); ClearHybridSecrets(); SetSendState(SendState::READY); if (m_initiating) AppendVersionPacket(VERSION_CONTENTS); @@ -1897,6 +1907,14 @@ void V2Transport::DiscardHybridSecretForTesting() noexcept m_cipher.DiscardHybridSecret(); } +void V2Transport::ClearPQSecrets() noexcept +{ + AssertLockNotHeld(m_recv_mutex); + AssertLockNotHeld(m_send_mutex); + LOCK2(m_recv_mutex, m_send_mutex); + ClearHybridSecrets(); +} + std::pair CConnman::SocketSendData(CNode& node) const { auto it = node.vSendMsg.begin(); @@ -2146,6 +2164,9 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, .prefer_evict = discouraged, .recv_flood_size = nReceiveFloodSize, .use_v2transport = use_v2transport, + .pq = {.mode = m_pq_mode}, + // The actual remote address and source port. + .pq_endpoint = addr, }); pnode->AddRef(); m_msgproc->InitializeNode(*pnode, local_services); @@ -2202,6 +2223,291 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ return true; } +PQNameEndpoint MakePQNameEndpoint(std::string_view hostname, uint16_t port) +{ + // DNS names are case-insensitive, and "example.com." is the fully qualified "example.com". + if (hostname.ends_with('.')) hostname.remove_suffix(1); + return {.hostname = ToLower(hostname), .port = port}; +} + +std::string_view PQOutcomeString(PQOutcome outcome) noexcept +{ + switch (outcome) { + case PQOutcome::SWITCHED: return "switched"; + case PQOutcome::CONFIRMED: return "confirmed"; + case PQOutcome::LEGACY_PEER: return "legacy_peer"; + case PQOutcome::MALFORMED_RECORD: return "malformed_record"; + case PQOutcome::FIRST_PACKET_FAILED: return "first_packet_failed"; + case PQOutcome::ABANDONED: return "abandoned"; + case PQOutcome::CLOSED_AFTER_SWITCH: return "closed_after_switch"; + case PQOutcome::FALLBACK: return "fallback"; + case PQOutcome::INTERNAL_ERROR: return "internal_error"; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + +namespace { +/** The index of a direction in the hybrid negotiation counters and failure rings. */ +size_t PQDirection(bool inbound) { return inbound ? 0 : 1; } + +/** The outcome a negotiation failure counts as. */ +PQOutcome PQFailureOutcome(PQFailure failure) noexcept +{ + switch (failure) { + case PQFailure::EK_LENGTH: + case PQFailure::EK_MODULUS: + case PQFailure::CT_LENGTH: + return PQOutcome::MALFORMED_RECORD; + case PQFailure::CONFIRM_LENGTH: + case PQFailure::CONFIRM_TAG: + case PQFailure::CONFIRM_NOT_DECOY: + return PQOutcome::FIRST_PACKET_FAILED; + case PQFailure::KEYGEN_INTERNAL: + case PQFailure::CHECK_EK_INTERNAL: + case PQFailure::ENCAPS_INTERNAL: + case PQFailure::DECAPS_INTERNAL: + case PQFailure::CIPHER_STATE_INTERNAL: + return PQOutcome::INTERNAL_ERROR; + case PQFailure::NONE: + break; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + +/** The reason word of a negotiation failure. */ +std::string_view PQFailureReason(PQFailure failure) noexcept +{ + switch (failure) { + case PQFailure::EK_LENGTH: return "ek_length"; + case PQFailure::EK_MODULUS: return "ek_modulus"; + case PQFailure::CT_LENGTH: return "ct_length"; + case PQFailure::CONFIRM_LENGTH: return "length"; + case PQFailure::CONFIRM_TAG: return "tag"; + case PQFailure::CONFIRM_NOT_DECOY: return "not_decoy"; + case PQFailure::KEYGEN_INTERNAL: return "keygen"; + case PQFailure::CHECK_EK_INTERNAL: return "check_public_key"; + case PQFailure::ENCAPS_INTERNAL: return "encaps"; + case PQFailure::DECAPS_INTERNAL: return "decaps"; + case PQFailure::CIPHER_STATE_INTERNAL: return "cipher_state"; + case PQFailure::NONE: break; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + +/** The reason word of a close cause. A node without one is finalized as LOCAL. */ +std::string_view PQCloseCauseReason(NodeCloseCause cause) noexcept +{ + switch (cause) { + case NodeCloseCause::PEER_EOF: return "eof"; + case NodeCloseCause::PEER_RESET: return "reset"; + case NodeCloseCause::TIMEOUT: return "timeout"; + case NodeCloseCause::SEND_ERROR: return "send_error"; + case NodeCloseCause::NONE: + case NodeCloseCause::LOCAL: return "local"; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + +/** The reason word of the transport closing before the peer's version arrived. */ +std::string_view PQAbortReason(PQAbort abort) noexcept +{ + switch (abort) { + case PQAbort::GARBAGE_TERMINATOR: return "garbage_terminator"; + case PQAbort::VERSION_LENGTH: return "version_length"; + case PQAbort::VERSION_TAG: return "version_tag"; + case PQAbort::NONE: break; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + +/** The counter of an outcome, or nullptr for a success, which isn't counted. */ +std::atomic* PQCounter(PQCounters& counters, PQOutcome outcome) noexcept +{ + switch (outcome) { + case PQOutcome::SWITCHED: return &counters.switched; + case PQOutcome::CONFIRMED: return nullptr; + case PQOutcome::LEGACY_PEER: return &counters.legacy_peer; + case PQOutcome::MALFORMED_RECORD: return &counters.malformed_record; + case PQOutcome::FIRST_PACKET_FAILED: return &counters.first_packet_failed; + case PQOutcome::ABANDONED: return &counters.abandoned; + case PQOutcome::CLOSED_AFTER_SWITCH: return &counters.closed_after_switch; + case PQOutcome::FALLBACK: return &counters.fallback; + case PQOutcome::INTERNAL_ERROR: return &counters.internal_error; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + +PQCounts LoadPQCounts(const PQCounters& counters) noexcept +{ + return { + .switched = counters.switched.load(), + .legacy_peer = counters.legacy_peer.load(), + .malformed_record = counters.malformed_record.load(), + .first_packet_failed = counters.first_packet_failed.load(), + .abandoned = counters.abandoned.load(), + .internal_error = counters.internal_error.load(), + .closed_after_switch = counters.closed_after_switch.load(), + .fallback = counters.fallback.load(), + .shed = counters.shed.load(), + }; +} +} // namespace + +void CConnman::ObservePQ(CNode& node, bool finalizing) +{ + AssertLockNotHeld(m_pq_mutex); + // The transport takes and releases its own lock. + const PQHandshake::Snapshot snapshot{node.m_transport->GetPQSnapshot()}; + // Nothing to account yet, as on every connection without the negotiation. + if (!finalizing && !snapshot.switched && snapshot.legacy == PQLegacyReason::NONE && snapshot.failure == PQFailure::NONE) return; + const NodeCloseCause cause{node.GetCloseCause()}; + const bool record{fNetworkActive}; + std::vector lines; + { + LOCK(m_pq_mutex); + if (node.m_pq_finalized) return; + if (record) lines = AccountPQ(node, snapshot, cause, finalizing, Now()); + if (finalizing) node.m_pq_finalized = true; + } + if (finalizing) node.m_transport->ClearPQSecrets(); + for (const PQLogLine& line : lines) LogPQOutcome(node, line); +} + +std::vector CConnman::AccountPQ(CNode& node, const PQHandshake::Snapshot& snapshot, NodeCloseCause cause, + bool finalizing, NodeSeconds now) +{ + AssertLockHeld(m_pq_mutex); + const bool inbound{node.IsInboundConn()}; + std::vector lines; + // Each event counts once per connection, however often it is observed. + const auto account{[&](PQOutcome outcome, std::string_view reason) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex) { + const uint16_t bit{static_cast(1U << static_cast(outcome))}; + if (node.m_pq_accounted_events & bit) return; + node.m_pq_accounted_events |= bit; + if (std::atomic* counter{PQCounter(m_pq_counters[PQDirection(inbound)], outcome)}) ++*counter; + if (outcome == PQOutcome::CONFIRMED) return; + if (outcome != PQOutcome::SWITCHED) { + AddPQFailure({.sequence = 0, + .time = now, + .endpoint = node.m_pq_endpoint, + .inbound = inbound, + .connection_type = node.m_conn_type, + .peer_id = node.GetId(), + .outcome = outcome, + .reason = std::string{reason}}); + } + lines.push_back({outcome, reason}); + }}; + + // switched counts the key installation, even if the confirmation fails later. + if (snapshot.switched) account(PQOutcome::SWITCHED, {}); + if (snapshot.confirmed) account(PQOutcome::CONFIRMED, {}); + if (snapshot.legacy != PQLegacyReason::NONE) { + account(PQOutcome::LEGACY_PEER, snapshot.legacy == PQLegacyReason::PARSE_ERROR ? "parse_error" : "no_features"); + } + if (snapshot.failure != PQFailure::NONE) account(PQFailureOutcome(snapshot.failure), PQFailureReason(snapshot.failure)); + if (finalizing && snapshot.failure == PQFailure::NONE) { + // We switched, and the peer closed or stopped responding before its confirmation + // verified. Our own closes and send errors are not the peer's doing. + if (!inbound && snapshot.switched && !snapshot.confirmed && + (cause == NodeCloseCause::PEER_EOF || cause == NodeCloseCause::PEER_RESET || cause == NodeCloseCause::TIMEOUT)) { + account(PQOutcome::CLOSED_AFTER_SWITCH, PQCloseCauseReason(cause)); + } + // Our offer was queued, but no authenticated version arrived before the close. An offer + // that failed internally is internal_error only. + if (snapshot.offer == PQOfferState::SENT && !snapshot.version_received) { + account(PQOutcome::ABANDONED, snapshot.abort != PQAbort::NONE ? PQAbortReason(snapshot.abort) : PQCloseCauseReason(cause)); + } + } + return lines; +} + +void CConnman::AddPQFailure(PQFailureEntry entry) +{ + AssertLockHeld(m_pq_mutex); + PQFailureRing& ring{m_pq_failures[PQDirection(entry.inbound)]}; + entry.sequence = ++ring.last_sequence; + ring.entries.push_back(std::move(entry)); + if (ring.entries.size() > PQ_FAILURE_RING_SIZE) { + ring.entries.pop_front(); + ++ring.dropped; + } +} + +void CConnman::LogPQOutcome(const CNode& node, const PQLogLine& line) const +{ + const auto backends{mlkem::GetBackendNames()}; + const std::string_view word{PQOutcomeString(line.outcome)}; + const std::string conn_type{node.ConnectionTypeAsString()}; + const std::string peeraddr{node.LogIP(fLogIPs)}; + if (node.IsInboundConn()) { + // Outsiders can cause every inbound outcome at will, so these are debug lines. + switch (line.outcome) { + case PQOutcome::SWITCHED: + LogDebug(BCLog::NET, "v2 pq: switched role=responder conn_type=%s peer=%d%s", conn_type, node.GetId(), peeraddr); + return; + case PQOutcome::LEGACY_PEER: + case PQOutcome::ABANDONED: + LogDebug(BCLog::NET, "v2 pq: %s reason=%s role=responder conn_type=%s peer=%d%s", word, line.reason, conn_type, node.GetId(), peeraddr); + return; + case PQOutcome::MALFORMED_RECORD: + case PQOutcome::FIRST_PACKET_FAILED: + case PQOutcome::INTERNAL_ERROR: + LogDebug(BCLog::NET, "v2 pq: %s reason=%s role=responder conn_type=%s peer=%d%s arith=%s keccak=%s", + word, line.reason, conn_type, node.GetId(), peeraddr, backends.arith, backends.keccak); + return; + case PQOutcome::CONFIRMED: + case PQOutcome::CLOSED_AFTER_SWITCH: + case PQOutcome::FALLBACK: + return; + } // no default case, so the compiler can warn about missing cases + return; + } + // Outbound failures are logged at info level, each from its own call site. + switch (line.outcome) { + case PQOutcome::SWITCHED: + LogDebug(BCLog::NET, "v2 pq: switched role=initiator conn_type=%s peer=%d%s", conn_type, node.GetId(), peeraddr); + return; + case PQOutcome::LEGACY_PEER: + case PQOutcome::ABANDONED: + LogDebug(BCLog::NET, "v2 pq: %s reason=%s role=initiator conn_type=%s peer=%d%s", word, line.reason, conn_type, node.GetId(), peeraddr); + return; + case PQOutcome::MALFORMED_RECORD: + LogInfo("v2 pq: malformed_record reason=%s role=initiator conn_type=%s peer=%d%s arith=%s keccak=%s", + line.reason, conn_type, node.GetId(), peeraddr, backends.arith, backends.keccak); + return; + case PQOutcome::FIRST_PACKET_FAILED: + LogInfo("v2 pq: first_packet_failed reason=%s role=initiator conn_type=%s peer=%d%s arith=%s keccak=%s", + line.reason, conn_type, node.GetId(), peeraddr, backends.arith, backends.keccak); + return; + case PQOutcome::CLOSED_AFTER_SWITCH: + LogInfo("v2 pq: closed_after_switch cause=%s role=initiator conn_type=%s peer=%d%s arith=%s keccak=%s", + line.reason, conn_type, node.GetId(), peeraddr, backends.arith, backends.keccak); + return; + case PQOutcome::INTERNAL_ERROR: + LogInfo("v2 pq: internal_error reason=%s role=initiator conn_type=%s peer=%d%s arith=%s keccak=%s", + line.reason, conn_type, node.GetId(), peeraddr, backends.arith, backends.keccak); + return; + case PQOutcome::CONFIRMED: + case PQOutcome::FALLBACK: + return; + } // no default case, so the compiler can warn about missing cases +} + +PQTransportStats CConnman::GetPQTransportStats() const +{ + AssertLockNotHeld(m_pq_mutex); + LOCK(m_pq_mutex); + return { + .instance_id = m_pq_instance_id, + .since = m_pq_since, + .inbound = LoadPQCounts(m_pq_counters[PQDirection(/*inbound=*/true)]), + .outbound = LoadPQCounts(m_pq_counters[PQDirection(/*inbound=*/false)]), + .inbound_failures = m_pq_failures[PQDirection(/*inbound=*/true)], + .outbound_failures = m_pq_failures[PQDirection(/*inbound=*/false)], + }; +} + void CConnman::DisconnectNodes() { AssertLockNotHeld(m_nodes_mutex); @@ -2254,6 +2560,10 @@ void CConnman::DisconnectNodes() // close socket and cleanup; a node with no recorded close cause counts as LOCAL pnode->CloseSocketDisconnect(); + // account the hybrid negotiation's close outcome and wipe its pending secrets + // before the disconnected pool keeps the node + ObservePQ(*pnode, /*finalizing=*/true); + // update connection count by network if (pnode->IsManualOrFullOutboundConn()) --m_network_conn_counts[pnode->addr.GetNetwork()]; @@ -2482,6 +2792,7 @@ void CConnman::SocketHandlerConnected(const std::vector& nodes, ); pnode->CloseSocketDisconnect(); } + ObservePQ(*pnode, /*finalizing=*/false); RecordBytesRecv(nBytes); if (notify) { pnode->MarkReceivedMsgsForProcessing(); @@ -4097,10 +4408,10 @@ ServiceFlags CConnman::GetLocalServices() const return m_local_services; } -static std::unique_ptr MakeTransport(NodeId id, bool use_v2transport, bool inbound) noexcept +static std::unique_ptr MakeTransport(NodeId id, bool use_v2transport, bool inbound, const V2PQOptions& pq) noexcept { if (use_v2transport) { - return std::make_unique(id, /*initiating=*/!inbound); + return std::make_unique(id, /*initiating=*/!inbound, pq); } else { return std::make_unique(id); } @@ -4116,7 +4427,7 @@ CNode::CNode(NodeId idIn, ConnectionType conn_type_in, bool inbound_onion, CNodeOptions&& node_opts) - : m_transport{MakeTransport(idIn, node_opts.use_v2transport, conn_type_in == ConnectionType::INBOUND)}, + : m_transport{MakeTransport(idIn, node_opts.use_v2transport, conn_type_in == ConnectionType::INBOUND, node_opts.pq)}, m_permission_flags{node_opts.permission_flags}, m_sock{sock}, m_connected{GetTime()}, @@ -4129,6 +4440,7 @@ CNode::CNode(NodeId idIn, nKeyedNetGroup{nKeyedNetGroupIn}, m_conn_type{conn_type_in}, m_is_archive_connection{node_opts.is_archive_connection}, + m_pq_endpoint{node_opts.pq_endpoint ? std::move(*node_opts.pq_endpoint) : PQEndpointKey{CService{addrIn}}}, id{idIn}, nLocalHostNonce{nLocalHostNonceIn}, m_recv_flood_size{node_opts.recv_flood_size}, diff --git a/src/net.h b/src/net.h index ccc208d03d..5b6c47da7c 100644 --- a/src/net.h +++ b/src/net.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -44,8 +45,11 @@ #include #include #include +#include +#include #include #include +#include #include class AddrMan; @@ -371,6 +375,12 @@ class Transport { /** Whether upon disconnections, a reconnect with V1 is warranted. */ virtual bool ShouldReconnectV1() const noexcept = 0; + + /** The hybrid negotiation's progress and first failure (nothing, without a negotiation). */ + virtual PQHandshake::Snapshot GetPQSnapshot() const noexcept { return {}; } + + /** Wipe the hybrid negotiation's pending secrets; the current keys stay in use. */ + virtual void ClearPQSecrets() noexcept {} }; class V1Transport final : public Transport @@ -739,8 +749,8 @@ class V2Transport final : public Transport // Miscellaneous functions. bool ShouldReconnectV1() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex, !m_send_mutex); Info GetInfo() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); - /** The hybrid negotiation's progress and first failure. */ - PQHandshake::Snapshot GetPQSnapshot() const noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); + PQHandshake::Snapshot GetPQSnapshot() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); + void ClearPQSecrets() noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex, !m_send_mutex); /** Test only: whether the decapsulation key or the retained ECDH secret is still held. */ bool HoldsHybridSecretsForTesting() const noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); @@ -770,6 +780,101 @@ enum class NodeCloseCause : uint8_t { TIMEOUT = 5, }; +/** A name-proxy destination: the SOCKS destination hostname and the effective port. */ +struct PQNameEndpoint { + std::string hostname; + uint16_t port{0}; + friend bool operator==(const PQNameEndpoint&, const PQNameEndpoint&) = default; + friend auto operator<=>(const PQNameEndpoint&, const PQNameEndpoint&) = default; +}; + +/** + * The endpoint a connection's hybrid negotiation outcomes are accounted to. A resolved + * destination, and an inbound peer, is its CService: the address, of any network, and the port + * (for an inbound peer, its source port). A name-proxy destination, never resolved locally, is + * its normalized hostname and port; never the proxy's own address. + */ +using PQEndpointKey = std::variant; + +/** The endpoint of a name-proxy destination: hostname in lowercase ASCII, one trailing root dot removed. */ +PQNameEndpoint MakePQNameEndpoint(std::string_view hostname, uint16_t port); + +/** A hybrid negotiation event. Each one's word (PQOutcomeString) is its counter key and log word. */ +enum class PQOutcome : uint8_t { + SWITCHED = 0, + //! The peer's key confirmation verified: a success, not counted (switched counted it). + CONFIRMED = 1, + LEGACY_PEER = 2, + MALFORMED_RECORD = 3, + FIRST_PACKET_FAILED = 4, + //! A responder's offer was queued, but no authenticated version arrived before the close. + ABANDONED = 5, + //! Outbound: the peer closed (or timed out) after we switched, before its confirmation. + CLOSED_AFTER_SWITCH = 6, + //! Outbound: an endpoint entered the fallback set. + FALLBACK = 7, + INTERNAL_ERROR = 8, +}; + +std::string_view PQOutcomeString(PQOutcome outcome) noexcept; + +/** Hybrid negotiation event counts of one direction. They count events, not exclusive final + * states: a switch whose confirmation then fails counts switched and first_packet_failed. */ +template +struct PQCountsBase { + T switched{0}; + T legacy_peer{0}; + T malformed_record{0}; + T first_packet_failed{0}; + //! Inbound only: only responders offer. + T abandoned{0}; + T internal_error{0}; + //! Outbound only. + T closed_after_switch{0}; + T fallback{0}; + //! Inbound only: offers not made because of load shedding. + T shed{0}; +}; +using PQCounters = PQCountsBase>; +using PQCounts = PQCountsBase; + +/** A non-success hybrid negotiation outcome, as kept in a failure ring. */ +struct PQFailureEntry { + uint64_t sequence; + NodeSeconds time; + PQEndpointKey endpoint; + bool inbound; + ConnectionType connection_type; + NodeId peer_id; + PQOutcome outcome; + //! Assigned only from the fixed reason vocabulary, never from peer bytes. + std::string reason; +}; + +/** The most recent failures of one direction, oldest first, with their own sequence. */ +struct PQFailureRing { + //! The sequence of the newest entry ever added (the first is 1); 0 if none. + uint64_t last_sequence{0}; + //! How many entries were evicted to keep the bound. + uint64_t dropped{0}; + std::deque entries; +}; + +/** At most this many entries in each failure ring. */ +inline constexpr size_t PQ_FAILURE_RING_SIZE{256}; + +/** An owning copy of the hybrid transport statistics, without atomics. */ +struct PQTransportStats { + //! Random per process start; counters, rings and history count from since. + uint256 instance_id; + NodeSeconds since; + PQCounts inbound; + PQCounts outbound; + PQFailureRing inbound_failures; + //! Outbound connections, manual ones included. + PQFailureRing outbound_failures; +}; + struct CNodeOptions { NetPermissionFlags permission_flags = NetPermissionFlags::None; @@ -778,6 +883,10 @@ struct CNodeOptions size_t recv_flood_size{DEFAULT_MAXRECEIVEBUFFER * 1000}; bool use_v2transport = false; bool is_archive_connection = false; + //! The hybrid negotiation of a v2 transport; off unless set. + V2PQOptions pq{}; + //! The endpoint key of the connection; the peer's address and port if unset. + std::optional pq_endpoint{}; }; /** Information about a peer */ @@ -850,6 +959,13 @@ class CNode const ConnectionType m_conn_type; const bool m_is_archive_connection{false}; + //! The endpoint this connection's hybrid negotiation outcomes are accounted to. + const PQEndpointKey m_pq_endpoint; + + //! The PQOutcome events already accounted for this connection (a bit per outcome), and + //! whether its accounting is final. Accessed only by CConnman, under its m_pq_mutex. + uint16_t m_pq_accounted_events{0}; + bool m_pq_finalized{false}; /** Move all messages from the received queue to the processing queue. */ void MarkReceivedMsgsForProcessing() @@ -1409,6 +1525,9 @@ class CConnman uint64_t GetTotalBytesRecv() const; uint64_t GetTotalBytesSent() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); + /** An owning copy of the hybrid transport counters and failure rings. */ + PQTransportStats GetPQTransportStats() const EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); + /** Get a unique deterministic randomizer. */ CSipHasher GetDeterministicRandomizer(uint64_t id) const; @@ -1462,7 +1581,7 @@ class CConnman const CService& addr_bind, const CService& addr); - void DisconnectNodes() EXCLUSIVE_LOCKS_REQUIRED(!m_reconnections_mutex, !m_nodes_mutex); + void DisconnectNodes() EXCLUSIVE_LOCKS_REQUIRED(!m_reconnections_mutex, !m_nodes_mutex, !m_pq_mutex); void NotifyNumConnectionsChanged(); /** Return true if the peer is inactive and should be disconnected. */ bool InactivityCheck(const CNode& node) const; @@ -1477,7 +1596,7 @@ class CConnman /** * Check connected and listening sockets for IO readiness and process them accordingly. */ - void SocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); + void SocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_pq_mutex); /** * Do the read/write for connected sockets that are ready for IO. @@ -1486,7 +1605,7 @@ class CConnman */ void SocketHandlerConnected(const std::vector& nodes, const Sock::EventsPerSock& events_per_sock) - EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_pq_mutex); /** * Accept incoming connections, one from each read-ready listening socket. @@ -1494,7 +1613,7 @@ class CConnman */ void SocketHandlerListening(const Sock::EventsPerSock& events_per_sock); - void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex); + void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex, !m_pq_mutex); void ThreadDNSAddressSeed() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex); uint64_t CalculateKeyedNetGroup(const CNetAddr& ad) const; @@ -1820,6 +1939,50 @@ class CConnman std::vector m_nodes_copy; }; + /** One hybrid negotiation event of a connection, to log once m_pq_mutex is released. */ + struct PQLogLine { + PQOutcome outcome; + std::string_view reason; + }; + + /** + * Account the hybrid negotiation events of node that are new since its last observation. It + * runs after the node received bytes, and once more, finalizing, before DisconnectNodes() + * hands the node to the disconnected pool: that derives closed_after_switch and abandoned + * from the close cause, marks the node finalized so no later observation counts anything, + * and wipes the transport's pending secrets. With the network off, every close is our own + * decision, so nothing is recorded, but finalization still wipes. + * + * The transport is snapshotted under its own lock, which is released before m_pq_mutex is + * taken; nothing calls into the transport, socket, peer manager or node enumeration while + * holding m_pq_mutex. + */ + void ObservePQ(CNode& node, bool finalizing) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); + + /** Count each event of snapshot not yet accounted for node, once, and add a ring entry for + * each non-success outcome. Returns the lines to log. */ + std::vector AccountPQ(CNode& node, const PQHandshake::Snapshot& snapshot, NodeCloseCause cause, + bool finalizing, NodeSeconds now) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); + + /** Add a non-success outcome to the ring of its direction. */ + void AddPQFailure(PQFailureEntry entry) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); + + /** Log one event of node: inbound and outbound lines come from separate call sites, because + * the logger rate-limits per call site. */ + void LogPQOutcome(const CNode& node, const PQLogLine& line) const; + + /** The hybrid negotiation mode of new v2 connections. Off until the option sets it (part 6); + * set before the connection threads start. */ + PQMode m_pq_mode{PQMode::OFF}; + + mutable Mutex m_pq_mutex; + //! Event counters, inbound [0] and outbound [1]. Written under m_pq_mutex. + std::array m_pq_counters; + //! Failure rings, inbound [0] and outbound [1]: inbound noise can't evict outbound evidence. + std::array m_pq_failures GUARDED_BY(m_pq_mutex); + const uint256 m_pq_instance_id{GetRandHash()}; + const NodeSeconds m_pq_since{Now()}; + const CChainParams& m_params; friend struct ConnmanTestMsg; diff --git a/src/netbase.cpp b/src/netbase.cpp index d5f21b4f4d..84b8c8a355 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -16,10 +16,12 @@ #include #include +#include #include #include #include #include +#include #include #include @@ -790,6 +792,20 @@ bool GetNameProxy(Proxy &nameProxyOut) { return true; } +ProxySettingsRestorerForTesting::ProxySettingsRestorerForTesting() +{ + LOCK(g_proxyinfo_mutex); + std::copy(std::begin(proxyInfo), std::end(proxyInfo), m_proxies.begin()); + m_name_proxy = nameProxy; +} + +ProxySettingsRestorerForTesting::~ProxySettingsRestorerForTesting() +{ + LOCK(g_proxyinfo_mutex); + std::copy(m_proxies.begin(), m_proxies.end(), std::begin(proxyInfo)); + nameProxy = m_name_proxy; +} + bool HaveNameProxy() { LOCK(g_proxyinfo_mutex); return nameProxy.IsValid(); diff --git a/src/netbase.h b/src/netbase.h index 18f6522790..9396ed7801 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -219,6 +220,23 @@ bool SetNameProxy(const Proxy &addrProxy); bool HaveNameProxy(); bool GetNameProxy(Proxy &nameProxyOut); +/** + * Test only: saves every proxy setting (each network's proxy and the name proxy) when created, + * and restores them when destroyed. The settings can't be unset otherwise, so a test that sets a + * proxy uses this to keep it from leaking into later tests. + */ +class ProxySettingsRestorerForTesting +{ + std::array m_proxies; + Proxy m_name_proxy; + +public: + ProxySettingsRestorerForTesting(); + ~ProxySettingsRestorerForTesting(); + ProxySettingsRestorerForTesting(const ProxySettingsRestorerForTesting&) = delete; + ProxySettingsRestorerForTesting& operator=(const ProxySettingsRestorerForTesting&) = delete; +}; + using DNSLookupFn = std::function(const std::string&, bool)>; extern DNSLookupFn g_dns_lookup; diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 673e436286..bf2fcebf52 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -72,6 +72,7 @@ add_executable(test_bitcoin multisig_tests.cpp net_peer_connection_tests.cpp net_peer_eviction_tests.cpp + net_pq_tests.cpp net_tests.cpp netbase_tests.cpp node_init_tests.cpp diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp new file mode 100644 index 0000000000..12a69367d9 --- /dev/null +++ b/src/test/net_pq_tests.cpp @@ -0,0 +1,1016 @@ +// Copyright (c) 2026-present The qbit core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://opensource.org/license/mit. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace std::literals; + +namespace { + +//! Messages are never processed: the tests drive the connection manager directly. +class NoMessages final : public NetEventsInterface +{ +public: + void InitializeNode(const CNode&, ServiceFlags) override {} + void FinalizeNode(const CNode&) override {} + bool HasAllDesirableServiceFlags(ServiceFlags) const override { return true; } + bool HasUndesirableServiceFlags(ServiceFlags) const override { return false; } + bool ProcessMessages(CNode*, std::atomic&) override EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) { return false; } + bool SendMessages(CNode*) override EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) { return false; } +}; + +//! A DynSock whose Recv() or Send() can fail as if the peer reset the connection. +class FailingSock : public DynSock +{ +public: + using DynSock::DynSock; + + std::atomic m_fail_recv{false}; + std::atomic m_fail_send{false}; + + ssize_t Recv(void* buf, size_t len, int flags) const override + { + return m_fail_recv ? Fail() : DynSock::Recv(buf, len, flags); + } + ssize_t Send(const void* buf, size_t len, int flags) const override + { + return m_fail_send ? Fail() : DynSock::Send(buf, len, flags); + } + +private: + FailingSock& operator=(Sock&&) override + { + assert(false && "Move of Sock into FailingSock not allowed."); + return *this; + } + + static ssize_t Fail() + { +#ifdef WIN32 + WSASetLastError(WSAECONNRESET); +#else + errno = ECONNRESET; +#endif + return -1; + } +}; + +//! One connection under test: a CNode on a FailingSock, whose pipes are the peer's side. +struct Link { + std::shared_ptr pipes; + std::shared_ptr sock; + CNode* node; +}; + +//! The far side of a v2 connection, built by hand: it sends no garbage and any version contents. +class RawPeer +{ + BIP324Cipher m_cipher; + const bool m_initiator; + +public: + explicit RawPeer(bool initiator) : m_cipher{GenerateRandomKey(), MakeByteSpan(GetRandHash())}, m_initiator{initiator} {} + + std::vector Key() const + { + const EllSwiftPubKey& key{m_cipher.GetOurPubKey()}; + const auto bytes{MakeUCharSpan(std::span{key.data(), key.size()})}; + return {bytes.begin(), bytes.end()}; + } + + void Initialize(std::span their_key) + { + BOOST_REQUIRE_EQUAL(their_key.size(), EllSwiftPubKey::size()); + m_cipher.Initialize(EllSwiftPubKey{MakeByteSpan(their_key)}, m_initiator); + } + + //! Our garbage terminator, then a version packet with contents (empty garbage as AAD). + std::vector TerminatorAndVersion(std::span contents, bool damage_tag = false) + { + std::vector out(BIP324Cipher::GARBAGE_TERMINATOR_LEN + BIP324Cipher::EXPANSION + contents.size()); + std::ranges::copy(MakeUCharSpan(m_cipher.GetSendGarbageTerminator()), out.begin()); + m_cipher.Encrypt(contents, /*aad=*/{}, /*ignore=*/false, MakeWritableByteSpan(out).subspan(BIP324Cipher::GARBAGE_TERMINATOR_LEN)); + if (damage_tag) out.back() ^= 0x01; + return out; + } +}; + +/** A record CompactSize(1 + payload size) || header || payload. */ +std::vector MakeRecord(uint8_t header, size_t payload_size) +{ + DataStream stream; + WriteCompactSize(stream, 1 + payload_size); + stream << header; + stream.write(std::vector(payload_size, std::byte{0x42})); + return {stream.begin(), stream.end()}; +} + +/** The level a line is logged at, as #184's logging table sets it. */ +enum class LineLevel { NET_DEBUG, INFO, WARNING }; + +/** + * Matches a log line that ends with one of texts, logged at level. The logger prints a level + * prefix right before the message: "[net] " for a net debug line, "[warning] " for a warning, + * nothing for an info line. The test setup turns on -logsourcelocations, so without a prefix the + * bracket right before the message is the function signature. + */ +std::function AtLevel(std::vector texts, LineLevel level) +{ + BOOST_REQUIRE(LogInstance().m_log_sourcelocations); + return [texts, level](const std::string* line) { + if (line == nullptr) return true; + return std::ranges::any_of(texts, [&](const std::string& text) { + if (!line->ends_with(text + "\n")) return false; + const std::string_view head{std::string_view{*line}.substr(0, line->size() - text.size() - 1)}; + const size_t open{head.rfind('[')}; + if (open == std::string_view::npos || !head.ends_with("] ")) return false; + const std::string_view bracket{head.substr(open)}; + switch (level) { + case LineLevel::NET_DEBUG: return bracket == "[net] "; + case LineLevel::WARNING: return bracket == "[warning] "; + case LineLevel::INFO: return bracket.find('(') != std::string_view::npos; + } + return false; + }); + }; +} + +/** Expect a log line that is exactly text, logged at level. */ +DebugLogHelper ExpectLine(std::string text, LineLevel level) { return DebugLogHelper{text, AtLevel({text}, level)}; } + +/** The backends every failure line names. */ +std::string Backends() +{ + const auto backends{mlkem::GetBackendNames()}; + return strprintf(" arith=%s keccak=%s", backends.arith, backends.keccak); +} + +struct PQNetSetup : public RegTestingSetup { + NoMessages m_events; + ConnmanTestMsg m_connman{0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()}; + NodeId m_next_id{0}; + + PQNetSetup() + { + CConnman::Options options; + options.m_max_automatic_connections = DEFAULT_MAX_PEER_CONNECTIONS; + m_connman.Init(options); + m_connman.SetMsgProc(&m_events); + m_connman.SetPeerConnectTimeout(60s); + m_connman.SetPQMode(PQMode::NEGOTIATE); + SetMockTime(GetTime()); + } + + ~PQNetSetup() + { + m_connman.Stop(); + SetMockTime(0s); + } + + /** A connection the connection manager holds, as ConnectNode() or the accept path would add it. */ + Link Add(ConnectionType conn_type, PQMode mode = PQMode::NEGOTIATE, std::optional endpoint = std::nullopt, + const CService& addr = LookupNumeric("1.2.3.4", 8333)) + { + auto pipes{std::make_shared()}; + auto sock{std::make_shared(pipes, std::make_shared())}; + CNode* node{new CNode{m_next_id++, sock, CAddress{addr, NODE_NONE}, /*nKeyedNetGroupIn=*/0, /*nLocalHostNonceIn=*/0, + CAddress{}, /*addrNameIn=*/"", conn_type, /*inbound_onion=*/false, + CNodeOptions{.use_v2transport = true, .pq = {.mode = mode}, .pq_endpoint = std::move(endpoint)}}}; + node->AddRef(); // the reference m_nodes holds + m_connman.AddTestNode(*node); + return {pipes, sock, node}; + } + + /** Two socket handler passes over links: each node sends what it has queued, receives once, + * then sends what that queued. */ + void Pass(const std::vector& links) + { + Sock::EventsPerSock events; + std::vector nodes; + for (const Link& link : links) { + events.emplace(link.sock, Sock::Events{Sock::RECV | Sock::SEND}).first->second.occurred = Sock::RECV | Sock::SEND; + nodes.push_back(link.node); + } + m_connman.SocketHandlerConnectedPublic(nodes, events); + m_connman.SocketHandlerConnectedPublic(nodes, events); + } + void Pass(const Link& link) { Pass(std::vector{link}); } + + /** Everything the node sent so far. */ + static std::vector Sent(const Link& link) + { + std::vector out; + uint8_t buf[4096]; + ssize_t read; + while ((read = link.pipes->send.GetBytes(buf, sizeof(buf))) > 0) out.insert(out.end(), buf, buf + read); + return out; + } + + /** The node's key, at the start of what it sent; the rest is dropped. */ + static std::vector NodeKey(const Link& link) + { + std::vector sent{Sent(link)}; + BOOST_REQUIRE(sent.size() >= EllSwiftPubKey::size()); + sent.resize(EllSwiftPubKey::size()); + return sent; + } + + static void Receive(const Link& link, std::span bytes) { link.pipes->recv.PushBytes(bytes.data(), bytes.size()); } + + /** What peer has to send. */ + static std::vector PeerBytes(Transport& peer) + { + std::vector out; + while (true) { + const auto [bytes, more, msg_type] = peer.GetBytesToSend(/*have_next_message=*/false); + if (bytes.empty()) return out; + out.insert(out.end(), bytes.begin(), bytes.end()); + peer.MarkBytesSent(bytes.size()); + } + } + + /** Queue what peer has to send for the node. */ + static void PeerToNode(const Link& link, Transport& peer) { Receive(link, PeerBytes(peer)); } + + /** Feed peer what the node sent; false if peer failed. */ + static bool NodeToPeer(const Link& link, Transport& peer) + { + const std::vector sent{Sent(link)}; + std::span bytes{sent}; + while (!bytes.empty()) { + if (!peer.ReceivedBytes(bytes)) return false; + if (peer.ReceivedMessageComplete()) { + bool reject{false}; + peer.GetReceivedMessage({}, reject); + } + } + return true; + } + + /** Run socket handler passes, moving bytes both ways. A peer that fails gets nothing more, but + * what it queued before still reaches the node. */ + void Exchange(const Link& link, Transport& peer) + { + bool peer_failed{false}; + for (int round{0}; round < 10; ++round) { + PeerToNode(link, peer); + Pass(link); + if (peer_failed) { + Sent(link); + } else { + peer_failed = !NodeToPeer(link, peer); + } + } + } + + /** Remove the disconnected nodes; those with a reference left stay in the disconnected pool. */ + void Disconnect() { m_connman.DisconnectNodesPublic(); } + + PQTransportStats Stats() const { return m_connman.GetPQTransportStats(); } +}; + +/** Our outbound node's peer: a responder. */ +V2Transport Responder(V2PQOptions pq = {.mode = PQMode::NEGOTIATE}) { return {/*nodeid=*/1000, /*initiating=*/false, pq}; } +/** Our inbound node's peer: an initiator. */ +V2Transport Initiator(V2PQOptions pq = {.mode = PQMode::NEGOTIATE}) { return {/*nodeid=*/1000, /*initiating=*/true, pq}; } + +/** 64 bytes an initiator could send as its key: any 64 bytes are a valid ElligatorSwift key. */ +std::vector InitiatorKey() { return FastRandomContext{}.randbytes(EllSwiftPubKey::size()); } + +} // namespace + +BOOST_FIXTURE_TEST_SUITE(net_pq_tests, PQNetSetup) + +BOOST_AUTO_TEST_CASE(pq_endpoint_identity) +{ + // Name-proxy destinations: lowercase ASCII, one trailing root dot removed, the port kept. + BOOST_CHECK(MakePQNameEndpoint("Example.COM.", 8444) == (PQNameEndpoint{"example.com", 8444})); + BOOST_CHECK(MakePQNameEndpoint("example.com", 8444) == MakePQNameEndpoint("EXAMPLE.com.", 8444)); + BOOST_CHECK(MakePQNameEndpoint("example.com..", 8444) == (PQNameEndpoint{"example.com.", 8444})); + BOOST_CHECK(MakePQNameEndpoint("example.com", 8444) != MakePQNameEndpoint("example.com", 8445)); + BOOST_CHECK(MakePQNameEndpoint("example.com", 8444) != MakePQNameEndpoint("example.org", 8444)); + BOOST_CHECK(MakePQNameEndpoint("\xC3\x84" "B.example", 1) == (PQNameEndpoint{"\xC3\x84" "b.example", 1})); // ASCII only + + // Resolved destinations keep the address and the port. + const CService ipv4{LookupNumeric("1.2.3.4", 8333)}; + const PQEndpointKey a{ipv4}; + BOOST_CHECK(a != PQEndpointKey{LookupNumeric("1.2.3.4", 8334)}); + BOOST_CHECK(a != PQEndpointKey{LookupNumeric("2001:db8::1", 8333)}); + BOOST_CHECK(a == PQEndpointKey{LookupNumeric("::ffff:1.2.3.4", 8333)}); // the same IPv4 address + // Private addresses share a network class, NET_UNROUTABLE, but stay distinct endpoints. + BOOST_CHECK(PQEndpointKey{LookupNumeric("10.0.0.1", 8333)} != PQEndpointKey{LookupNumeric("10.0.0.2", 8333)}); + // A name never aliases an address, even one that spells the same. + BOOST_CHECK(a != PQEndpointKey{MakePQNameEndpoint("1.2.3.4", 8333)}); + + // A map keyed by endpoint orders them strictly. + std::map keys; + for (const PQEndpointKey& key : {a, PQEndpointKey{LookupNumeric("1.2.3.4", 8334)}, PQEndpointKey{MakePQNameEndpoint("A.example", 1)}, + PQEndpointKey{MakePQNameEndpoint("a.example.", 1)}, PQEndpointKey{MakePQNameEndpoint("b.example", 1)}}) { + ++keys[key]; + } + BOOST_CHECK_EQUAL(keys.size(), 4U); + + // A node without a captured endpoint uses its address and port. + Link plain{Add(ConnectionType::OUTBOUND_FULL_RELAY)}; + BOOST_CHECK(plain.node->m_pq_endpoint == a); + Link named{Add(ConnectionType::MANUAL, PQMode::NEGOTIATE, MakePQNameEndpoint("Seed.Example.", 18444), CService{})}; + BOOST_CHECK(named.node->m_pq_endpoint == PQEndpointKey{(PQNameEndpoint{"seed.example", 18444})}); + + // Outbound construction captures the resolved destination and port. + const auto create_sock_orig{CreateSock}; + CreateSock = [](int, int, int) -> std::unique_ptr { return std::make_unique(); }; + { + const std::unique_ptr outbound{m_connman.ConnectNodeOnly("5.6.7.8:18555", ConnectionType::MANUAL, /*use_v2transport=*/true)}; + BOOST_REQUIRE(outbound); + BOOST_CHECK(outbound->m_pq_endpoint == PQEndpointKey{LookupNumeric("5.6.7.8", 18555)}); + const std::unique_ptr ipv6{m_connman.ConnectNodeOnly("[2a01:4f8::2]:18555", ConnectionType::MANUAL, /*use_v2transport=*/true)}; + BOOST_REQUIRE(ipv6); + BOOST_CHECK(ipv6->m_pq_endpoint == PQEndpointKey{LookupNumeric("2a01:4f8::2", 18555)}); + } + CreateSock = create_sock_orig; + + // Inbound uses the actual remote address and source port. + m_connman.CreateNodeFromAcceptedSocketPublic(std::make_unique(), LookupNumeric("127.0.0.1", 18444), LookupNumeric("9.8.7.6", 51234)); + const std::vector nodes{m_connman.TestNodes()}; + BOOST_REQUIRE(!nodes.empty()); + BOOST_CHECK(nodes.back()->IsInboundConn()); + BOOST_CHECK(nodes.back()->m_pq_endpoint == PQEndpointKey{LookupNumeric("9.8.7.6", 51234)}); +} + +BOOST_AUTO_TEST_CASE(proxy_settings_restorer) +{ + // The guard puts back exactly what was set before it, nested or not. + BOOST_REQUIRE(!HaveNameProxy()); + Proxy proxy; + BOOST_REQUIRE(!GetProxy(NET_IPV4, proxy)); + { + ProxySettingsRestorerForTesting outer; + BOOST_REQUIRE(SetNameProxy(Proxy{LookupNumeric("127.0.0.1", 9050)})); + { + ProxySettingsRestorerForTesting inner; + BOOST_REQUIRE(SetNameProxy(Proxy{LookupNumeric("127.0.0.2", 9050)})); + BOOST_REQUIRE(SetProxy(NET_IPV4, Proxy{LookupNumeric("127.0.0.3", 9050)})); + } + BOOST_REQUIRE(GetNameProxy(proxy)); + BOOST_CHECK(proxy.proxy == LookupNumeric("127.0.0.1", 9050)); + BOOST_CHECK(!GetProxy(NET_IPV4, proxy)); + } + BOOST_CHECK(!HaveNameProxy()); + BOOST_CHECK(!GetProxy(NET_IPV4, proxy)); +} + +BOOST_AUTO_TEST_CASE(pq_name_proxy_endpoint) +{ + // Through a name proxy, a destination by name is never resolved locally: ConnectNode() asks + // the proxy for it, and the endpoint key is the normalized SOCKS destination and the + // effective port, never the proxy's own address. + const CService proxy{LookupNumeric("127.0.0.9", 9050)}; + const auto create_sock_orig{CreateSock}; + // Every CConnman's Interrupt(), such as an earlier test's at teardown, interrupts SOCKS5. + const bool socks5_interrupted{g_socks5_interrupt}; + g_socks5_interrupt.reset(); + { + ProxySettingsRestorerForTesting restore_proxies; + BOOST_REQUIRE(SetNameProxy(Proxy{proxy})); + // The proxy accepts without authentication, then reports the connection made. + CreateSock = [](int, int, int) -> std::unique_ptr { + return std::make_unique(std::string{"\x05\x00" "\x05\x00\x00\x01" "\x00\x00\x00\x00" "\x00\x00", 12}); + }; + for (const auto& [dest, key] : {std::pair{"Seed.Example.COM.:18555", PQNameEndpoint{"seed.example.com", 18555}}, + std::pair{"seed.example.com", PQNameEndpoint{"seed.example.com", Params().GetDefaultPort()}}, + std::pair{"other.example:18555", PQNameEndpoint{"other.example", 18555}}}) { + const std::unique_ptr node{m_connman.ConnectNodeOnly(dest, ConnectionType::MANUAL, /*use_v2transport=*/true)}; + BOOST_REQUIRE(node); + BOOST_CHECK(node->m_pq_endpoint == PQEndpointKey{key}); + BOOST_CHECK(node->m_pq_endpoint != PQEndpointKey{proxy}); + } + CreateSock = create_sock_orig; + } + if (socks5_interrupted) g_socks5_interrupt(); + // No later test sees the name proxy. + BOOST_CHECK(!HaveNameProxy()); +} + +BOOST_AUTO_TEST_CASE(pq_observation_once) +{ + // A hybrid handshake: switched once, however often it is observed. + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY)}; + link.node->AddRef(); // keep the node through DisconnectNodes() + V2Transport peer{Responder()}; + Exchange(link, peer); + BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().confirmed); + BOOST_CHECK(link.node->m_transport->GetInfo().transport_pq_status == PQStatus::HYBRID); + for (int i{0}; i < 3; ++i) m_connman.ObservePQPublic(*link.node, /*finalizing=*/false); + BOOST_CHECK_EQUAL(Stats().outbound.switched, 1U); + + // A switch whose confirmation fails, seen after receive, at disconnect and in cleanup. + Link failing{Add(ConnectionType::OUTBOUND_FULL_RELAY)}; + failing.node->AddRef(); + V2Transport bad_peer{Responder({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true})}; + Exchange(failing, bad_peer); + BOOST_REQUIRE(failing.node->fDisconnect); + m_connman.ObservePQPublic(*failing.node, /*finalizing=*/false); + PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.outbound.switched, 2U); + BOOST_CHECK_EQUAL(stats.outbound.first_packet_failed, 1U); + BOOST_CHECK_EQUAL(stats.outbound_failures.entries.size(), 1U); + + // Racing observers, as from the socket handler and cleanup, still count each event once. + std::vector threads; + for (int i{0}; i < 4; ++i) { + threads.emplace_back([&, i] { m_connman.ObservePQPublic(*failing.node, /*finalizing=*/i % 2 == 0); }); + } + for (auto& thread : threads) thread.join(); + Disconnect(); + for (const Link& l : {link, failing}) { + m_connman.ObservePQPublic(*l.node, /*finalizing=*/false); + m_connman.ObservePQPublic(*l.node, /*finalizing=*/true); + BOOST_CHECK(l.node->m_pq_finalized); + } + stats = Stats(); + BOOST_CHECK_EQUAL(stats.outbound.switched, 2U); + BOOST_CHECK_EQUAL(stats.outbound.first_packet_failed, 1U); + BOOST_CHECK_EQUAL(stats.outbound.closed_after_switch, 0U); // our own close after the failure + BOOST_CHECK_EQUAL(stats.outbound_failures.entries.size(), 1U); + BOOST_CHECK_EQUAL(stats.outbound_failures.last_sequence, 1U); + BOOST_CHECK_EQUAL(stats.inbound.switched + stats.inbound.first_packet_failed, 0U); + BOOST_CHECK(stats.inbound_failures.entries.empty()); + for (const Link& l : {link, failing}) l.node->Release(); + Disconnect(); + + // Instance identity: random per connection manager, counted from its creation. + ConnmanTestMsg other{0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()}; + BOOST_CHECK(other.GetPQTransportStats().instance_id != stats.instance_id); + BOOST_CHECK(stats.since <= Now()); + BOOST_CHECK_EQUAL(other.GetPQTransportStats().outbound.switched, 0U); +} + +BOOST_AUTO_TEST_CASE(pq_ring_bounds) +{ + // One outbound failure: the responder offers no features. + Link outbound{Add(ConnectionType::MANUAL)}; + V2Transport legacy{Responder({})}; + Exchange(outbound, legacy); + BOOST_CHECK_EQUAL(Stats().outbound.legacy_peer, 1U); + + // 300 inbound failures: each offer is abandoned when the peer closes. + std::vector inbound; + for (int i{0}; i < 300; ++i) { + inbound.push_back(Add(ConnectionType::INBOUND, PQMode::NEGOTIATE, std::nullopt, LookupNumeric("10.0.0.1", 40000 + i))); + Receive(inbound.back(), InitiatorKey()); + } + Pass(inbound); + for (const Link& link : inbound) { + BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().offer == PQOfferState::SENT); + link.pipes->recv.Eof(); + } + Pass(inbound); + Disconnect(); + + const PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.inbound.abandoned, 300U); + BOOST_CHECK_EQUAL(stats.inbound_failures.entries.size(), PQ_FAILURE_RING_SIZE); + BOOST_CHECK_EQUAL(stats.inbound_failures.dropped, 44U); + BOOST_CHECK_EQUAL(stats.inbound_failures.last_sequence, 300U); + for (size_t i{0}; i < stats.inbound_failures.entries.size(); ++i) { + const PQFailureEntry& entry{stats.inbound_failures.entries[i]}; + BOOST_CHECK_EQUAL(entry.sequence, 45 + i); + BOOST_CHECK(entry.inbound); + BOOST_CHECK(entry.outcome == PQOutcome::ABANDONED); + BOOST_CHECK_EQUAL(entry.reason, "eof"); + } + BOOST_CHECK(stats.inbound_failures.entries.back().endpoint == PQEndpointKey{LookupNumeric("10.0.0.1", 40299)}); + + // The outbound entry survives, with its own sequence. + BOOST_REQUIRE_EQUAL(stats.outbound_failures.entries.size(), 1U); + const PQFailureEntry& entry{stats.outbound_failures.entries.front()}; + BOOST_CHECK_EQUAL(entry.sequence, 1U); + BOOST_CHECK_EQUAL(stats.outbound_failures.last_sequence, 1U); + BOOST_CHECK_EQUAL(stats.outbound_failures.dropped, 0U); + BOOST_CHECK(entry.outcome == PQOutcome::LEGACY_PEER); + BOOST_CHECK_EQUAL(entry.reason, "no_features"); + BOOST_CHECK(!entry.inbound); + BOOST_CHECK(entry.connection_type == ConnectionType::MANUAL); + BOOST_CHECK_EQUAL(entry.peer_id, outbound.node->GetId()); + BOOST_CHECK(entry.endpoint == PQEndpointKey{LookupNumeric("1.2.3.4", 8333)}); + BOOST_CHECK(entry.time == Now()); +} + +BOOST_AUTO_TEST_CASE(pq_switched_then_failed) +{ + for (const bool inbound : {false, true}) { + Link link{Add(inbound ? ConnectionType::INBOUND : ConnectionType::OUTBOUND_FULL_RELAY)}; + V2Transport peer{inbound ? Initiator({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true}) : + Responder({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true})}; + Exchange(link, peer); + const PQHandshake::Snapshot snapshot{link.node->m_transport->GetPQSnapshot()}; + BOOST_CHECK(snapshot.switched); + BOOST_CHECK(snapshot.failure == PQFailure::CONFIRM_LENGTH || snapshot.failure == PQFailure::CONFIRM_TAG); + BOOST_CHECK(link.node->fDisconnect); + Disconnect(); + const PQTransportStats stats{Stats()}; + const PQCounts& counts{inbound ? stats.inbound : stats.outbound}; + BOOST_CHECK_EQUAL(counts.switched, 1U); + BOOST_CHECK_EQUAL(counts.first_packet_failed, 1U); + BOOST_CHECK_EQUAL(counts.closed_after_switch + counts.abandoned + counts.malformed_record, 0U); + const PQFailureRing& ring{inbound ? stats.inbound_failures : stats.outbound_failures}; + BOOST_REQUIRE_EQUAL(ring.entries.size(), 1U); + BOOST_CHECK(ring.entries.front().outcome == PQOutcome::FIRST_PACKET_FAILED); + BOOST_CHECK(ring.entries.front().reason == "length" || ring.entries.front().reason == "tag"); + } +} + +BOOST_AUTO_TEST_CASE(pq_abandoned) +{ + // An offer is queued, and the connection closes before an authenticated version arrives. + struct Case { + std::string reason; + std::function close; + }; + const std::vector cases{ + {"eof", [](PQNetSetup&, const Link& link, RawPeer&) { link.pipes->recv.Eof(); }}, + {"reset", [](PQNetSetup&, const Link& link, RawPeer&) { link.sock->m_fail_recv = true; }}, + {"timeout", [](PQNetSetup&, const Link&, RawPeer&) { SetMockTime(GetTime() + 61s); }}, + {"local", [](PQNetSetup& setup, const Link& link, RawPeer&) { BOOST_CHECK(setup.m_connman.DisconnectNode(link.node->GetId())); }}, + // The peer's version packet doesn't authenticate. + {"version_tag", [](PQNetSetup&, const Link& link, RawPeer& peer) { + Receive(link, peer.TerminatorAndVersion({}, /*damage_tag=*/true)); + }}, + // The peer's version packet is longer than any packet may be. + {"version_length", [](PQNetSetup&, const Link& link, RawPeer& peer) { + const auto packet{peer.TerminatorAndVersion(std::vector(MAX_PROTOCOL_MESSAGE_LENGTH + 14))}; + Receive(link, std::span{packet}.first(BIP324Cipher::GARBAGE_TERMINATOR_LEN + BIP324Cipher::LENGTH_LEN)); + }}, + // No garbage terminator in the longest garbage allowed. + {"garbage_terminator", [](PQNetSetup&, const Link& link, RawPeer&) { + Receive(link, std::vector(V2Transport::MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN, 0)); + }}, + }; + for (const Case& test : cases) { + BOOST_TEST_MESSAGE("abandoned: " + test.reason); + // The peer sends its key, so the node offers, and then not the version it is asked for. + Link link{Add(ConnectionType::INBOUND, PQMode::NEGOTIATE, std::nullopt, LookupNumeric("10.0.0.2", 50000))}; + RawPeer peer{/*initiator=*/true}; + Receive(link, peer.Key()); + Pass(link); + BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().offer == PQOfferState::SENT); + peer.Initialize(NodeKey(link)); + test.close(*this, link, peer); + for (int i{0}; i < 4; ++i) Pass(link); + BOOST_CHECK(link.node->fDisconnect); + const auto line{ExpectLine(strprintf("v2 pq: abandoned reason=%s role=responder conn_type=inbound peer=%d", test.reason, link.node->GetId()), + LineLevel::NET_DEBUG)}; + Disconnect(); + } + // A failed send of the queued offer. + { + Link link{Add(ConnectionType::INBOUND)}; + Receive(link, InitiatorKey()); + link.sock->m_fail_send = true; + Pass(link); // receives the key and queues the offer + Pass(link); // the send fails + BOOST_CHECK(link.node->GetCloseCause() == NodeCloseCause::SEND_ERROR); + Disconnect(); + } + + const PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.inbound.abandoned, cases.size() + 1); + BOOST_CHECK_EQUAL(stats.inbound.switched, 0U); + BOOST_REQUIRE_EQUAL(stats.inbound_failures.entries.size(), cases.size() + 1); + for (size_t i{0}; i < cases.size(); ++i) BOOST_CHECK_EQUAL(stats.inbound_failures.entries[i].reason, cases[i].reason); + BOOST_CHECK_EQUAL(stats.inbound_failures.entries.back().reason, "send_error"); + BOOST_CHECK(stats.inbound_failures.entries.front().endpoint == PQEndpointKey{LookupNumeric("10.0.0.2", 50000)}); + BOOST_CHECK(stats.outbound_failures.entries.empty()); + + // An offer the peer answered is not abandoned, however the connection ends. + Link answered{Add(ConnectionType::INBOUND)}; + V2Transport peer{Initiator({})}; + Exchange(answered, peer); + BOOST_CHECK(answered.node->m_transport->GetPQSnapshot().legacy == PQLegacyReason::NO_FEATURES); + answered.pipes->recv.Eof(); + Pass(answered); + Disconnect(); + BOOST_CHECK_EQUAL(Stats().inbound.abandoned, cases.size() + 1); + BOOST_CHECK_EQUAL(Stats().inbound.legacy_peer, 1U); +} + +BOOST_AUTO_TEST_CASE(pq_closed_after_switch) +{ + // We switched, and the peer closed or stopped responding before its confirmation arrived. + for (const std::string cause : {"eof", "reset", "timeout", "local", "send_error"}) { + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY)}; + V2Transport peer{Responder()}; + Pass(link); // our key and garbage + BOOST_REQUIRE(NodeToPeer(link, peer)); + PeerToNode(link, peer); // the peer's key, garbage, terminator and offer + if (cause == "send_error") link.sock->m_fail_send = true; + Pass(link); + BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().switched); + if (cause == "eof") link.pipes->recv.Eof(); + if (cause == "reset") link.sock->m_fail_recv = true; + if (cause == "timeout") SetMockTime(GetTime() + 61s); + if (cause == "local") link.node->RequestDisconnect(); + Pass(link); + Pass(link); + BOOST_REQUIRE(link.node->fDisconnect); + const bool counted{cause == "eof" || cause == "reset" || cause == "timeout"}; + // Count the lines rather than require one, so that a missing line fails the check instead + // of throwing from a destructor. + const std::string text{strprintf("v2 pq: closed_after_switch cause=%s role=initiator conn_type=outbound-full-relay peer=%d%s", + cause, link.node->GetId(), Backends())}; + const auto expected{AtLevel({text}, LineLevel::INFO)}; + int lines{0}, expected_lines{0}; + const uint64_t before{Stats().outbound.closed_after_switch}; + { + DebugLogHelper watch{"v2 pq: closed_after_switch", [&](const std::string* line) { + if (line) { + ++lines; + if (expected(line)) ++expected_lines; + } + return false; + }}; + Disconnect(); + } + BOOST_CHECK_EQUAL(lines, counted ? 1 : 0); + BOOST_CHECK_EQUAL(expected_lines, counted ? 1 : 0); + BOOST_CHECK_EQUAL(Stats().outbound.closed_after_switch, before + (counted ? 1 : 0)); + } + const PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.outbound.switched, 5U); + BOOST_CHECK_EQUAL(stats.outbound.closed_after_switch, 3U); + BOOST_REQUIRE_EQUAL(stats.outbound_failures.entries.size(), 3U); + BOOST_CHECK_EQUAL(stats.outbound_failures.entries[2].reason, "timeout"); + + // An inbound switch closed by the peer is not counted: closed_after_switch is outbound only. + Link inbound{Add(ConnectionType::INBOUND)}; + V2Transport peer{Initiator()}; + PeerToNode(inbound, peer); + Pass(inbound); + BOOST_REQUIRE(NodeToPeer(inbound, peer)); + const std::vector accept{PeerBytes(peer)}; // and the confirmation, held back + Receive(inbound, std::span{accept}.first(accept.size() - PQ_CONFIRMATION_BYTES)); + Pass(inbound); + BOOST_REQUIRE(inbound.node->m_transport->GetPQSnapshot().switched); + BOOST_REQUIRE(!inbound.node->m_transport->GetPQSnapshot().confirmed); + inbound.pipes->recv.Eof(); + Pass(inbound); + Disconnect(); + BOOST_CHECK_EQUAL(Stats().inbound.switched, 1U); + BOOST_CHECK_EQUAL(Stats().inbound.closed_after_switch + Stats().inbound.abandoned, 0U); + + // After a confirmed handshake, a peer close or a timeout is an ordinary close: nothing is + // counted, the ring gets no entry, and nothing is logged. + int lines{0}; + { + DebugLogHelper watch{"v2 pq: closed_after_switch", [&](const std::string* line) { + if (line) ++lines; + return false; + }}; + for (const NodeCloseCause cause : {NodeCloseCause::PEER_EOF, NodeCloseCause::PEER_RESET, NodeCloseCause::TIMEOUT}) { + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::NEGOTIATE, LookupNumeric("1.2.3.6", 8333))}; + V2Transport responder{Responder()}; + Exchange(link, responder); + BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().confirmed); + if (cause == NodeCloseCause::PEER_EOF) link.pipes->recv.Eof(); + if (cause == NodeCloseCause::PEER_RESET) link.sock->m_fail_recv = true; + if (cause == NodeCloseCause::TIMEOUT) SetMockTime(GetTime() + 61s); + Pass(link); + BOOST_REQUIRE(link.node->GetCloseCause() == cause); + Disconnect(); + } + } + BOOST_CHECK_EQUAL(lines, 0); + BOOST_CHECK_EQUAL(Stats().outbound.switched, stats.outbound.switched + 3); + BOOST_CHECK_EQUAL(Stats().outbound.closed_after_switch, stats.outbound.closed_after_switch); + BOOST_CHECK_EQUAL(Stats().outbound_failures.last_sequence, stats.outbound_failures.last_sequence); +} + +BOOST_AUTO_TEST_CASE(pq_log_lines) +{ + // Outbound failures are info lines; inbound lines and outbound successes are net debug lines. + const auto line{[](std::string_view text, const Link& link, bool failure) { + return ExpectLine(strprintf("v2 pq: %s conn_type=%s peer=%d%s", text, link.node->ConnectionTypeAsString(), link.node->GetId(), + failure ? Backends() : ""), + failure && !link.node->IsInboundConn() ? LineLevel::INFO : LineLevel::NET_DEBUG); + }}; + + // Outbound: a hybrid handshake, a legacy peer, malformed records, a parse error. + { + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY)}; + V2Transport peer{Responder()}; + const auto expect{line("switched role=initiator", link, false)}; + Exchange(link, peer); + } + { + Link link{Add(ConnectionType::BLOCK_RELAY)}; + V2Transport peer{Responder({})}; + const auto expect{line("legacy_peer reason=no_features role=initiator", link, false)}; + Exchange(link, peer); + } + const auto raw_responder{[&](const Link& link, std::span contents) { + Pass(link); + RawPeer peer{/*initiator=*/false}; + peer.Initialize(NodeKey(link)); + Receive(link, peer.Key()); + Receive(link, peer.TerminatorAndVersion(contents)); + Pass(link); + }}; + { + Link link{Add(ConnectionType::MANUAL)}; + const auto expect{line("legacy_peer reason=parse_error role=initiator", link, false)}; + raw_responder(link, std::vector{std::byte{0x05}, std::byte{0x01}}); + } + { + Link link{Add(ConnectionType::MANUAL)}; + const auto expect{line("malformed_record reason=ek_length role=initiator", link, true)}; + raw_responder(link, MakeRecord(PQ_MLKEM1024, 100)); + BOOST_CHECK(link.node->fDisconnect); + // A closing connection reports pending until it is removed. + BOOST_CHECK(link.node->m_transport->GetInfo().transport_pq_status == PQStatus::PENDING); + } + // With mismatched keys, the confirmation's length decrypts to a random value, rarely to 0. + const auto confirm_failed{[](const Link& link, std::string_view role) { + std::vector texts; + for (const std::string_view reason : {"length", "tag"}) { + texts.push_back(strprintf("v2 pq: first_packet_failed reason=%s role=%s conn_type=%s peer=%d%s", reason, role, + link.node->ConnectionTypeAsString(), link.node->GetId(), Backends())); + } + return DebugLogHelper{"v2 pq: first_packet_failed reason=", AtLevel(texts, role == "initiator" ? LineLevel::INFO : LineLevel::NET_DEBUG)}; + }}; + { + Link link{Add(ConnectionType::FEELER)}; + const auto expect{confirm_failed(link, "initiator")}; + V2Transport peer{Responder({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true})}; + Exchange(link, peer); + } + { + // A local fault before we committed: the connection continues as plain v2, status off. + Link link{Add(ConnectionType::ADDR_FETCH)}; + const auto expect{line("internal_error reason=encaps role=initiator", link, true)}; + V2Transport peer{Responder()}; + mlkem::InjectResultForTesting inject{mlkem::Operation::ENCAPS, mlkem::upstream::ERR_FAIL}; + Exchange(link, peer); + BOOST_CHECK(!link.node->fDisconnect); + BOOST_CHECK(link.node->m_transport->GetInfo().transport_pq_status == PQStatus::OFF); + } + + // Inbound: the same outcomes, at debug level from their own call sites. + { + Link link{Add(ConnectionType::INBOUND)}; + V2Transport peer{Initiator()}; + const auto expect{line("switched role=responder", link, false)}; + Exchange(link, peer); + BOOST_CHECK(link.node->m_transport->GetPQSnapshot().confirmed); + } + { + Link link{Add(ConnectionType::INBOUND)}; + V2Transport peer{Initiator({})}; + const auto expect{line("legacy_peer reason=no_features role=responder", link, false)}; + Exchange(link, peer); + } + const auto raw_initiator{[&](const Link& link, std::span contents) { + RawPeer peer{/*initiator=*/true}; + Receive(link, peer.Key()); + Pass(link); + peer.Initialize(NodeKey(link)); + Receive(link, peer.TerminatorAndVersion(contents)); + Pass(link); + }}; + { + Link link{Add(ConnectionType::INBOUND)}; + const auto expect{line("legacy_peer reason=parse_error role=responder", link, false)}; + raw_initiator(link, std::vector{std::byte{0x05}, std::byte{0x01}}); + } + { + Link link{Add(ConnectionType::INBOUND)}; + const auto expect{line("malformed_record reason=ct_length role=responder", link, true)}; + raw_initiator(link, MakeRecord(PQ_MLKEM1024, 100)); + } + { + Link link{Add(ConnectionType::INBOUND)}; + const auto expect{confirm_failed(link, "responder")}; + V2Transport peer{Initiator({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true})}; + Exchange(link, peer); + } + { + Link link{Add(ConnectionType::INBOUND)}; + const auto expect{line("internal_error reason=keygen role=responder", link, true)}; + mlkem::InjectResultForTesting inject{mlkem::Operation::KEYGEN, mlkem::upstream::ERR_FAIL}; + Receive(link, InitiatorKey()); + Pass(link); + // No offer, so the connection continues as plain v2, status off. + BOOST_CHECK(!link.node->fDisconnect); + BOOST_CHECK(link.node->m_transport->GetInfo().transport_pq_status == PQStatus::OFF); + } + { + Link link{Add(ConnectionType::INBOUND)}; + const auto expect{line("internal_error reason=decaps role=responder", link, true)}; + V2Transport peer{Initiator()}; + mlkem::InjectResultForTesting inject{mlkem::Operation::DECAPS, mlkem::upstream::ERR_FAIL}; + Exchange(link, peer); + // The initiator has committed, so this local fault closes, and reports pending until the + // connection is removed. Counting reads the failure, never the status. + BOOST_CHECK(link.node->fDisconnect); + BOOST_CHECK(link.node->m_transport->GetInfo().transport_pq_status == PQStatus::PENDING); + } + + // -logips adds the peer's address. + { + Link link{Add(ConnectionType::MANUAL)}; + V2Transport peer{Responder()}; + const bool log_ips{fLogIPs}; + fLogIPs = true; + const auto expect{ExpectLine(strprintf("v2 pq: switched role=initiator conn_type=manual peer=%d peeraddr=1.2.3.4:8333", link.node->GetId()), + LineLevel::NET_DEBUG)}; + Exchange(link, peer); + fLogIPs = log_ips; + } + Disconnect(); + + const PQTransportStats stats{Stats()}; + for (const PQCounts& counts : {stats.inbound, stats.outbound}) { + BOOST_CHECK_EQUAL(counts.legacy_peer, 2U); + BOOST_CHECK_EQUAL(counts.malformed_record, 1U); + BOOST_CHECK_EQUAL(counts.first_packet_failed, 1U); + } + BOOST_CHECK_EQUAL(stats.outbound.switched, 3U); // hybrid, corrupted, -logips + BOOST_CHECK_EQUAL(stats.outbound.internal_error, 1U); + BOOST_CHECK_EQUAL(stats.inbound.switched, 2U); // hybrid, corrupted + BOOST_CHECK_EQUAL(stats.inbound.internal_error, 2U); +} + +BOOST_AUTO_TEST_CASE(pq_cipher_state_closes) +{ + // An inconsistent cipher state is a local fault that closes the connection in either role, + // an initiator that hasn't committed yet included. It counts once, as internal_error with + // reason cipher_state: an inbound offer that was queued is not also abandoned. + // DiscardHybridSecretForTesting() induces it; the failing check is an Assume, so only + // release builds can exercise it. + if constexpr (!G_FUZZING_BUILD && !G_ABORT_ON_FAILED_ASSUME) { + const PQEndpointKey endpoint{LookupNumeric("1.2.3.5", 8333)}; + for (int i{0}; i < 3; ++i) { + // Outbound: the keys are initialized, and the responder's offer arrives after the discard. + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::NEGOTIATE, endpoint)}; + auto& transport{dynamic_cast(*link.node->m_transport)}; + V2Transport peer{Responder()}; + Pass(link); + BOOST_REQUIRE(NodeToPeer(link, peer)); + const std::vector bytes{PeerBytes(peer)}; + Receive(link, std::span{bytes}.first(EllSwiftPubKey::size())); + Pass(link); + BOOST_REQUIRE(transport.HoldsHybridSecretsForTesting()); + transport.DiscardHybridSecretForTesting(); + const std::string text{strprintf("v2 pq: internal_error reason=cipher_state role=initiator conn_type=outbound-full-relay peer=%d%s", + link.node->GetId(), Backends())}; + DebugLogHelper expect{text, AtLevel({text}, LineLevel::INFO)}; + Receive(link, std::span{bytes}.subspan(EllSwiftPubKey::size())); + Pass(link); + BOOST_CHECK(link.node->fDisconnect); + BOOST_CHECK(transport.GetPQSnapshot().failure == PQFailure::CIPHER_STATE_INTERNAL); + BOOST_CHECK(transport.GetInfo().transport_pq_status == PQStatus::PENDING); + Disconnect(); + } + { + // Inbound: the offer is queued, and the initiator's accept arrives after the discard. + Link link{Add(ConnectionType::INBOUND)}; + auto& transport{dynamic_cast(*link.node->m_transport)}; + V2Transport peer{Initiator()}; + PeerToNode(link, peer); + Pass(link); + BOOST_REQUIRE(transport.GetPQSnapshot().offer == PQOfferState::SENT); + BOOST_REQUIRE(NodeToPeer(link, peer)); + transport.DiscardHybridSecretForTesting(); + const std::string text{strprintf("v2 pq: internal_error reason=cipher_state role=responder conn_type=inbound peer=%d%s", + link.node->GetId(), Backends())}; + DebugLogHelper expect{text, AtLevel({text}, LineLevel::NET_DEBUG)}; + PeerToNode(link, peer); + Pass(link); + BOOST_CHECK(link.node->fDisconnect); + BOOST_CHECK(transport.GetPQSnapshot().failure == PQFailure::CIPHER_STATE_INTERNAL); + Disconnect(); + } + const PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.outbound.internal_error, 3U); + BOOST_CHECK_EQUAL(stats.inbound.internal_error, 1U); + BOOST_CHECK_EQUAL(stats.inbound.abandoned + stats.outbound.closed_after_switch + stats.outbound.switched, 0U); + BOOST_REQUIRE_EQUAL(stats.inbound_failures.entries.size(), 1U); + BOOST_CHECK_EQUAL(stats.inbound_failures.entries.front().reason, "cipher_state"); + BOOST_REQUIRE_EQUAL(stats.outbound_failures.entries.size(), 3U); + for (const PQFailureEntry& entry : stats.outbound_failures.entries) { + BOOST_CHECK(entry.outcome == PQOutcome::INTERNAL_ERROR); + BOOST_CHECK_EQUAL(entry.reason, "cipher_state"); + } + } +} + +BOOST_AUTO_TEST_CASE(pq_finalization_wipes) +{ + // Every way a connection can close while hybrid secrets are pending: an inbound offer's + // decapsulation key and either role's retained ECDH secret. Finalization wipes them before + // the disconnected pool releases the node, not only when the node is deleted. + enum class Close { PEER_EOF, PEER_RESET, TIMEOUT, LOCAL, SEND_ERROR, NETWORK_OFF, TRANSPORT }; + for (const bool inbound : {true, false}) { + for (const Close close : {Close::PEER_EOF, Close::PEER_RESET, Close::TIMEOUT, Close::LOCAL, Close::SEND_ERROR, + Close::NETWORK_OFF, Close::TRANSPORT}) { + Link link{Add(inbound ? ConnectionType::INBOUND : ConnectionType::OUTBOUND_FULL_RELAY)}; + link.node->AddRef(); // keep the node in the disconnected pool + const auto& transport{dynamic_cast(*link.node->m_transport)}; + if (!inbound) Pass(link); // our key and garbage + // The peer's key: an inbound node queues its offer, an outbound one its terminator. + Receive(link, InitiatorKey()); + if (close == Close::SEND_ERROR) link.sock->m_fail_send = true; + Pass(link); + BOOST_CHECK_EQUAL(transport.GetPQSnapshot().offer == PQOfferState::SENT, inbound); + if (close != Close::SEND_ERROR) BOOST_REQUIRE(transport.HoldsHybridSecretsForTesting()); + switch (close) { + case Close::PEER_EOF: link.pipes->recv.Eof(); break; + case Close::PEER_RESET: link.sock->m_fail_recv = true; break; + case Close::TIMEOUT: SetMockTime(GetTime() + 61s); break; + case Close::LOCAL: link.node->RequestDisconnect(); break; + case Close::SEND_ERROR: break; + case Close::NETWORK_OFF: m_connman.SetNetworkActive(false); break; + case Close::TRANSPORT: // garbage without a terminator; the transport wipes as it fails + Receive(link, std::vector(V2Transport::MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN, 0)); + break; + } + Pass(link); + // A peer or socket close leaves the secrets to finalization. + if (close != Close::TRANSPORT && close != Close::NETWORK_OFF) { + BOOST_REQUIRE(link.node->fDisconnect); + BOOST_CHECK(transport.HoldsHybridSecretsForTesting()); + } + const uint64_t abandoned{Stats().inbound.abandoned}; + Disconnect(); + m_connman.SetNetworkActive(true); + BOOST_CHECK(link.node->m_pq_finalized); + BOOST_CHECK(!transport.HoldsHybridSecretsForTesting()); + // With the network off, every close is ours: nothing is recorded. + BOOST_CHECK_EQUAL(Stats().inbound.abandoned, abandoned + (inbound && close != Close::NETWORK_OFF ? 1 : 0)); + link.node->Release(); + Disconnect(); + } + } + + // The wipe is effective: a peer's accept that arrives later can no longer switch the keys. + // The transport then fails an Assume, so only release builds can exercise it. + if constexpr (!G_FUZZING_BUILD && !G_ABORT_ON_FAILED_ASSUME) { + Link link{Add(ConnectionType::INBOUND)}; + link.node->AddRef(); + V2Transport peer{Initiator()}; + PeerToNode(link, peer); + Pass(link); + BOOST_REQUIRE(NodeToPeer(link, peer)); // the peer now holds its accept for us + link.node->RequestDisconnect(); + Disconnect(); + const std::vector bytes{PeerBytes(peer)}; + std::span span{bytes}; + BOOST_CHECK(!link.node->m_transport->ReceivedBytes(span)); + const PQHandshake::Snapshot snapshot{link.node->m_transport->GetPQSnapshot()}; + BOOST_CHECK(snapshot.version_received); + BOOST_CHECK(!snapshot.switched); + BOOST_CHECK(snapshot.failure == PQFailure::CIPHER_STATE_INTERNAL); + // Nothing is counted after finalization. + m_connman.ObservePQPublic(*link.node, /*finalizing=*/false); + BOOST_CHECK_EQUAL(Stats().inbound.internal_error, 0U); + link.node->Release(); + Disconnect(); + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/net.h b/src/test/util/net.h index f578a111b4..a1ecd814c7 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -95,16 +95,35 @@ struct ConnmanTestMsg : public CConnman { EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); void SocketHandlerConnectedPublic(const std::vector& nodes, const Sock::EventsPerSock& events_per_sock) - EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc) + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_pq_mutex) { SocketHandlerConnected(nodes, events_per_sock); } - void DisconnectNodesPublic() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !m_reconnections_mutex) + void DisconnectNodesPublic() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !m_reconnections_mutex, !m_pq_mutex) { DisconnectNodes(); } + void SetPQMode(PQMode mode) { m_pq_mode = mode; } + + void ObservePQPublic(CNode& node, bool finalizing) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex) + { + ObservePQ(node, finalizing); + } + + /** ConnectNode() without adding the node to the connection manager; the caller owns it. */ + CNode* ConnectNodeOnly(const char* dest, ConnectionType conn_type, bool use_v2transport) + EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex) + { + return ConnectNode(CAddress{}, dest, /*fCountFailure=*/false, conn_type, use_v2transport); + } + + void CreateNodeFromAcceptedSocketPublic(std::unique_ptr&& sock, const CService& addr_bind, const CService& addr) + { + CreateNodeFromAcceptedSocket(std::move(sock), NetPermissionFlags::None, addr_bind, addr); + } + bool AttemptToEvictConnectionPublic() { return AttemptToEvictConnection(); } }; From e528f74ed9f1e3e0700aa7d5f74a9eb82907af91 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Mon, 5 Oct 2026 16:11:51 -0400 Subject: [PATCH 03/12] net: apply bounded per-endpoint fallback after repeated PQ failures Keep upgraded nodes connected when the hybrid negotiation with one endpoint keeps failing, without letting an outsider downgrade connections cheaply: a single failure is telemetry only. - Counted failures are outbound only, per endpoint key: malformed_record, first_packet_failed, and closed_after_switch (eof, reset or timeout). Our own closes, send errors, legacy outcomes and inbound failures are neutral: they neither advance nor reset a streak. - The third consecutive counted failure puts the endpoint in the fallback set for 1 h, then 4 h, then 24 h for each later entry (PQ_FALLBACK_WINDOWS). At expiry the streak restarts at 0 and the escalation stays. A failure from a connection already running during a window neither extends it nor enters again. - A confirmed hybrid handshake moves the endpoint into a separate success set, never evicted until restart, and drops its history. The check and the insert share m_pq_mutex with the counters and rings, so a late failure can't reinsert a protected endpoint. - The history holds at most 1,000 endpoints, evicting the oldest insertion first; insertion orders are unique, so equal times can't tie. It is memory only and uses the mockable NodeClock. - ConnectNode() asks IsPQFallback(endpoint, now): a fallback connection runs PQMode::FALLBACK, plain v2 that neither parses nor validates the offer. No retry loop is added: the next attempt the connection logic makes asks again, existing connections never renegotiate, and inbound connections never fall back. - Entering counts the outbound fallback counter once, adds a ring entry and logs one warning, "v2 pq: fallback cause= until=" with the usual fields. - When 8 distinct endpoints have counted failures and none has succeeded since startup, it warns once that this node's own ML-KEM code may be at fault, naming the active backends. It names no remedy: this tree has no option that forces portable code yet. - GetPQTransportStats() gains the fallback set and the failure streaks for part 6. net_pq_tests covers the threshold for each counted variant with neutral closes in between, the escalating windows under mock time (including new connections getting FALLBACK and existing ones not), success protection against a late failure and 1,001 endpoints of churn, oldest-insertion eviction with tied times and insertion order opposite to key order, the local-fault warning (once, naming the backends, native or portable; never after a success), and a fallback connection completing as plain v2 against a malformed offer. pq_closed_after_switch now uses an endpoint per connection, so that its three counted failures don't enter fallback, and pq_cipher_state_closes checks that a local fault never counts toward the fallback. Work item F3 of #206 (#184 part 5). Co-Authored-By: Claude Opus 5.5 --- src/net.cpp | 140 ++++++++++++- src/net.h | 115 +++++++++-- src/test/net_pq_tests.cpp | 409 +++++++++++++++++++++++++++++++++++++- src/test/util/net.h | 4 +- 4 files changed, 636 insertions(+), 32 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 9f3e5e2c9d..40f484f02e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -524,6 +524,12 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo continue; } + // Repeated hybrid failures to this endpoint put it in the fallback set: plain v2, without + // parsing or validating the peer's offer. The next attempt, whenever the connection logic + // makes one, asks again. + PQMode pq_mode{m_pq_mode}; + if (pq_mode == PQMode::NEGOTIATE && use_v2transport && IsPQFallback(pq_endpoint, Now())) pq_mode = PQMode::FALLBACK; + NetPermissionFlags permission_flags = NetPermissionFlags::None; std::vector whitelist_permissions = conn_type == ConnectionType::MANUAL ? vWhitelistedRangeOutgoing : std::vector{}; AddWhitelistPermissionFlags(permission_flags, target_addr, whitelist_permissions); @@ -549,7 +555,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo .recv_flood_size = nReceiveFloodSize, .use_v2transport = use_v2transport, .is_archive_connection = is_archive_connection, - .pq = {.mode = m_pq_mode}, + .pq = {.mode = pq_mode}, .pq_endpoint = std::move(pq_endpoint), }); pnode->AddRef(); @@ -2337,6 +2343,22 @@ std::atomic* PQCounter(PQCounters& counters, PQOutcome outcome) noexce assert(false); } +/** The window an endpoint enters next: escalating per completed window, then the last one. */ +std::chrono::seconds PQFallbackWindow(const PQEndpointHistory& history) noexcept +{ + return PQ_FALLBACK_WINDOWS[std::min(history.completed_windows, PQ_FALLBACK_WINDOWS.size() - 1)]; +} + +/** End an expired fallback window: the streak restarts at 0, and the escalation level stays. */ +void ExpirePQFallback(PQEndpointHistory& history, NodeSeconds now) noexcept +{ + if (!history.expires || now < *history.expires) return; + history.entered.reset(); + history.expires.reset(); + history.streak = 0; + history.completed_windows = static_cast(std::min(history.completed_windows + 1, PQ_FALLBACK_WINDOWS.size())); +} + PQCounts LoadPQCounts(const PQCounters& counters) noexcept { return { @@ -2362,7 +2384,7 @@ void CConnman::ObservePQ(CNode& node, bool finalizing) if (!finalizing && !snapshot.switched && snapshot.legacy == PQLegacyReason::NONE && snapshot.failure == PQFailure::NONE) return; const NodeCloseCause cause{node.GetCloseCause()}; const bool record{fNetworkActive}; - std::vector lines; + PQLogLines lines; { LOCK(m_pq_mutex); if (node.m_pq_finalized) return; @@ -2370,22 +2392,31 @@ void CConnman::ObservePQ(CNode& node, bool finalizing) if (finalizing) node.m_pq_finalized = true; } if (finalizing) node.m_transport->ClearPQSecrets(); - for (const PQLogLine& line : lines) LogPQOutcome(node, line); + for (const PQLogLine& line : lines.outcomes) LogPQOutcome(node, line); + if (lines.local_fault) LogPQLocalFault(); } -std::vector CConnman::AccountPQ(CNode& node, const PQHandshake::Snapshot& snapshot, NodeCloseCause cause, - bool finalizing, NodeSeconds now) +CConnman::PQLogLines CConnman::AccountPQ(CNode& node, const PQHandshake::Snapshot& snapshot, NodeCloseCause cause, + bool finalizing, NodeSeconds now) { AssertLockHeld(m_pq_mutex); const bool inbound{node.IsInboundConn()}; - std::vector lines; + PQLogLines lines; // Each event counts once per connection, however often it is observed. const auto account{[&](PQOutcome outcome, std::string_view reason) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex) { const uint16_t bit{static_cast(1U << static_cast(outcome))}; if (node.m_pq_accounted_events & bit) return; node.m_pq_accounted_events |= bit; if (std::atomic* counter{PQCounter(m_pq_counters[PQDirection(inbound)], outcome)}) ++*counter; - if (outcome == PQOutcome::CONFIRMED) return; + if (outcome == PQOutcome::CONFIRMED) { + // A confirmed hybrid handshake protects an outbound endpoint until restart: no + // failure, however late, can count against it again. + if (!inbound) { + m_pq_successful_endpoints.insert(node.m_pq_endpoint); + m_pq_history.erase(node.m_pq_endpoint); + } + return; + } if (outcome != PQOutcome::SWITCHED) { AddPQFailure({.sequence = 0, .time = now, @@ -2396,7 +2427,14 @@ std::vector CConnman::AccountPQ(CNode& node, const PQHandsh .outcome = outcome, .reason = std::string{reason}}); } - lines.push_back({outcome, reason}); + lines.outcomes.push_back({outcome, reason}); + // Only failures the peer side caused count toward the fallback, and only outbound. + if (!inbound && (outcome == PQOutcome::MALFORMED_RECORD || outcome == PQOutcome::FIRST_PACKET_FAILED || + outcome == PQOutcome::CLOSED_AFTER_SWITCH)) { + if (const auto until{CountPQFailure(node, outcome, reason, now, lines.local_fault)}) { + lines.outcomes.push_back({PQOutcome::FALLBACK, PQOutcomeString(outcome), until}); + } + } }}; // switched counts the key installation, even if the confirmation fails later. @@ -2422,6 +2460,62 @@ std::vector CConnman::AccountPQ(CNode& node, const PQHandsh return lines; } +std::optional CConnman::CountPQFailure(const CNode& node, PQOutcome outcome, std::string_view reason, NodeSeconds now, + bool& local_fault) +{ + AssertLockHeld(m_pq_mutex); + const PQEndpointKey& endpoint{node.m_pq_endpoint}; + if (m_pq_successful_endpoints.contains(endpoint)) return std::nullopt; + + // Many endpoints failing, and none succeeding, points at this node's own ML-KEM code. + if (m_pq_failed_endpoints_for_warning.size() < PQ_LOCAL_FAULT_THRESHOLD) m_pq_failed_endpoints_for_warning.insert(endpoint); + if (!m_pq_local_fault_warned && m_pq_failed_endpoints_for_warning.size() >= PQ_LOCAL_FAULT_THRESHOLD && + m_pq_successful_endpoints.empty()) { + m_pq_local_fault_warned = true; + local_fault = true; + } + + auto it{m_pq_history.find(endpoint)}; + if (it == m_pq_history.end()) { + if (m_pq_history.size() >= PQ_MAX_ENDPOINT_HISTORY) { + m_pq_history.erase(std::ranges::min_element(m_pq_history, {}, [](const auto& entry) { return entry.second.insertion_order; })); + } + it = m_pq_history.emplace(endpoint, PQEndpointHistory{.insertion_order = m_pq_history_insertions++}).first; + } + PQEndpointHistory& history{it->second}; + ExpirePQFallback(history, now); + // A connection that was already running when the window began can't extend it. + if (history.expires) return std::nullopt; + ++history.streak; + history.last_failure = now; + history.cause = outcome; + history.reason = reason; + if (history.streak < PQ_FAILURE_THRESHOLD) return std::nullopt; + + history.entered = now; + history.expires = now + PQFallbackWindow(history); + ++m_pq_counters[PQDirection(/*inbound=*/false)].fallback; + AddPQFailure({.sequence = 0, + .time = now, + .endpoint = endpoint, + .inbound = false, + .connection_type = node.m_conn_type, + .peer_id = node.GetId(), + .outcome = PQOutcome::FALLBACK, + .reason = std::string{PQOutcomeString(outcome)}}); + return history.expires; +} + +bool CConnman::IsPQFallback(const PQEndpointKey& endpoint, NodeSeconds now) +{ + AssertLockNotHeld(m_pq_mutex); + LOCK(m_pq_mutex); + const auto it{m_pq_history.find(endpoint)}; + if (it == m_pq_history.end()) return false; + ExpirePQFallback(it->second, now); + return it->second.expires.has_value(); +} + void CConnman::AddPQFailure(PQFailureEntry entry) { AssertLockHeld(m_pq_mutex); @@ -2488,24 +2582,50 @@ void CConnman::LogPQOutcome(const CNode& node, const PQLogLine& line) const LogInfo("v2 pq: internal_error reason=%s role=initiator conn_type=%s peer=%d%s arith=%s keccak=%s", line.reason, conn_type, node.GetId(), peeraddr, backends.arith, backends.keccak); return; - case PQOutcome::CONFIRMED: case PQOutcome::FALLBACK: + LogWarning("v2 pq: fallback cause=%s until=%s role=initiator conn_type=%s peer=%d%s arith=%s keccak=%s", + line.reason, FormatISO8601DateTime(TicksSinceEpoch(line.until.value_or(NodeSeconds{}))), + conn_type, node.GetId(), peeraddr, backends.arith, backends.keccak); + return; + case PQOutcome::CONFIRMED: return; } // no default case, so the compiler can warn about missing cases } +void CConnman::LogPQLocalFault() const +{ + const auto backends{mlkem::GetBackendNames()}; + LogWarning("v2 pq: local_fault arith=%s keccak=%s: hybrid handshakes failed with %u distinct endpoints and none succeeded " + "since startup, so this node's own ML-KEM code may be at fault.", + backends.arith, backends.keccak, PQ_LOCAL_FAULT_THRESHOLD); +} + PQTransportStats CConnman::GetPQTransportStats() const { AssertLockNotHeld(m_pq_mutex); LOCK(m_pq_mutex); - return { + PQTransportStats stats{ .instance_id = m_pq_instance_id, .since = m_pq_since, .inbound = LoadPQCounts(m_pq_counters[PQDirection(/*inbound=*/true)]), .outbound = LoadPQCounts(m_pq_counters[PQDirection(/*inbound=*/false)]), .inbound_failures = m_pq_failures[PQDirection(/*inbound=*/true)], .outbound_failures = m_pq_failures[PQDirection(/*inbound=*/false)], + .fallback_set = {}, + .failure_streaks = {}, }; + const NodeSeconds now{Now()}; + for (const auto& [endpoint, history] : m_pq_history) { + if (history.expires) { + // A window that expired ended the streak; nothing to report until the next failure. + if (now < *history.expires) { + stats.fallback_set.push_back({endpoint, history.cause, history.reason, history.streak, *history.entered, *history.expires}); + } + } else if (history.streak > 0) { + stats.failure_streaks.push_back({endpoint, history.cause, history.reason, history.streak, history.last_failure, PQFallbackWindow(history)}); + } + } + return stats; } void CConnman::DisconnectNodes() diff --git a/src/net.h b/src/net.h index 5b6c47da7c..28fe36ca8b 100644 --- a/src/net.h +++ b/src/net.h @@ -35,7 +35,9 @@ #include #include +#include #include +#include #include #include #include @@ -45,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -863,6 +866,58 @@ struct PQFailureRing { /** At most this many entries in each failure ring. */ inline constexpr size_t PQ_FAILURE_RING_SIZE{256}; +/** Consecutive counted failures (outbound malformed_record, first_packet_failed and + * closed_after_switch) that put an endpoint in the fallback set. */ +inline constexpr uint32_t PQ_FAILURE_THRESHOLD{3}; +/** An endpoint's fallback windows: its first entry, its second, then every later one. */ +inline constexpr std::array PQ_FALLBACK_WINDOWS{ + std::chrono::seconds{3600}, std::chrono::seconds{14400}, std::chrono::seconds{86400}}; +/** At most this many endpoints in the fallback, streak and escalation history. */ +inline constexpr size_t PQ_MAX_ENDPOINT_HISTORY{1000}; +/** Distinct endpoints with counted failures, and no success since startup, that suggest a local fault. */ +inline constexpr size_t PQ_LOCAL_FAULT_THRESHOLD{8}; + +/** An endpoint's counted failures and fallback windows. */ +struct PQEndpointHistory { + //! Consecutive counted failures since the last fallback window ended. + uint32_t streak{0}; + //! Fallback windows that ended; saturates at PQ_FALLBACK_WINDOWS.size(). + uint8_t completed_windows{0}; + //! The active fallback window, if any. + std::optional entered{}; + std::optional expires{}; + //! The latest counted failure. + NodeSeconds last_failure{}; + PQOutcome cause{PQOutcome::MALFORMED_RECORD}; + //! Assigned only from the fixed reason vocabulary. + std::string reason{}; + //! The oldest insertion is evicted first; unique, so equal times can't tie. + uint64_t insertion_order{0}; +}; + +/** An endpoint in the fallback set. */ +struct PQFallbackStats { + PQEndpointKey endpoint; + //! The failure that completed the streak, and its reason. + PQOutcome cause; + std::string reason; + uint32_t streak; + NodeSeconds entered; + NodeSeconds expires; +}; + +/** An endpoint with counted failures that is not in the fallback set. */ +struct PQStreakStats { + PQEndpointKey endpoint; + //! The latest counted failure, and its reason. + PQOutcome cause; + std::string reason; + uint32_t streak; + NodeSeconds last_failure; + //! The window the endpoint would enter at PQ_FAILURE_THRESHOLD. + std::chrono::seconds next_window; +}; + /** An owning copy of the hybrid transport statistics, without atomics. */ struct PQTransportStats { //! Random per process start; counters, rings and history count from since. @@ -873,6 +928,8 @@ struct PQTransportStats { PQFailureRing inbound_failures; //! Outbound connections, manual ones included. PQFailureRing outbound_failures; + std::vector fallback_set; + std::vector failure_streaks; }; struct CNodeOptions @@ -1380,7 +1437,7 @@ class CConnman bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active); - void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CountingSemaphoreGrant<>&& grant_outbound, const char* strDest, ConnectionType conn_type, bool use_v2transport, bool is_archive_connection = false) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); + void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CountingSemaphoreGrant<>&& grant_outbound, const char* strDest, ConnectionType conn_type, bool use_v2transport, bool is_archive_connection = false) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !m_pq_mutex); bool CheckIncomingNonce(uint64_t nonce); void ASMapHealthCheck(); @@ -1476,7 +1533,7 @@ class CConnman * - Max total outbound connection capacity filled * - Max connection capacity for type is filled */ - bool AddConnection(const std::string& address, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); + bool AddConnection(const std::string& address, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !m_pq_mutex); size_t GetNodeCount(ConnectionDirection) const; std::map getNetLocalAddresses() const; @@ -1525,9 +1582,13 @@ class CConnman uint64_t GetTotalBytesRecv() const; uint64_t GetTotalBytesSent() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); - /** An owning copy of the hybrid transport counters and failure rings. */ + /** An owning copy of the hybrid transport counters, failure rings and endpoint history. */ PQTransportStats GetPQTransportStats() const EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); + /** Whether new outbound connections to endpoint run plain v2, because repeated hybrid + * failures put it in the fallback set. Ends a window that expired. */ + bool IsPQFallback(const PQEndpointKey& endpoint, NodeSeconds now) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); + /** Get a unique deterministic randomizer. */ CSipHasher GetDeterministicRandomizer(uint64_t id) const; @@ -1560,10 +1621,10 @@ class CConnman bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions); bool InitBinds(const Options& options); - void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex); + void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex, !m_pq_mutex); void AddAddrFetch(const std::string& strDest) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex); - void ProcessAddrFetch() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_unused_i2p_sessions_mutex); - void ThreadOpenConnections(std::vector connect, std::span seed_nodes) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex); + void ProcessAddrFetch() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_unused_i2p_sessions_mutex, !m_pq_mutex); + void ThreadOpenConnections(std::vector connect, std::span seed_nodes) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex, !m_pq_mutex); void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); void ThreadI2PAcceptIncoming(); void AcceptConnection(const ListenSocket& hListenSocket); @@ -1629,7 +1690,7 @@ class CConnman bool AlreadyConnectedToAddress(const CAddress& addr); bool AttemptToEvictConnection(); - CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type, bool use_v2transport, bool is_archive_connection = false) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); + CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type, bool use_v2transport, bool is_archive_connection = false) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !m_pq_mutex); void AddWhitelistPermissionFlags(NetPermissionFlags& flags, std::optional addr, const std::vector& ranges) const; void DeleteNode(CNode* pnode); @@ -1894,7 +1955,7 @@ class CConnman std::list m_reconnections GUARDED_BY(m_reconnections_mutex); /** Attempt reconnections, if m_reconnections non-empty. */ - void PerformReconnections() EXCLUSIVE_LOCKS_REQUIRED(!m_reconnections_mutex, !m_unused_i2p_sessions_mutex); + void PerformReconnections() EXCLUSIVE_LOCKS_REQUIRED(!m_reconnections_mutex, !m_unused_i2p_sessions_mutex, !m_pq_mutex); /** * Cap on the size of `m_unused_i2p_sessions`, to ensure it does not @@ -1943,6 +2004,15 @@ class CConnman struct PQLogLine { PQOutcome outcome; std::string_view reason; + //! FALLBACK: the end of the window. + std::optional until{}; + }; + + /** What AccountPQ() leaves to log. */ + struct PQLogLines { + std::vector outcomes; + //! Whether the one-time local-fault warning is due. + bool local_fault{false}; }; /** @@ -1959,10 +2029,19 @@ class CConnman */ void ObservePQ(CNode& node, bool finalizing) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); - /** Count each event of snapshot not yet accounted for node, once, and add a ring entry for - * each non-success outcome. Returns the lines to log. */ - std::vector AccountPQ(CNode& node, const PQHandshake::Snapshot& snapshot, NodeCloseCause cause, - bool finalizing, NodeSeconds now) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); + /** Count each event of snapshot not yet accounted for node, once, add a ring entry for each + * non-success outcome, and update the endpoint history. Returns what to log. */ + PQLogLines AccountPQ(CNode& node, const PQHandshake::Snapshot& snapshot, NodeCloseCause cause, + bool finalizing, NodeSeconds now) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); + + /** + * Count an outbound connection's failure against its endpoint. The third consecutive one + * enters the fallback set, unless the endpoint completed a hybrid handshake since startup. + * A failure during an active window neither extends it nor enters again. Returns the end of + * a window it started, and sets local_fault when the local-fault warning is due. + */ + std::optional CountPQFailure(const CNode& node, PQOutcome outcome, std::string_view reason, NodeSeconds now, + bool& local_fault) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); /** Add a non-success outcome to the ring of its direction. */ void AddPQFailure(PQFailureEntry entry) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); @@ -1971,6 +2050,9 @@ class CConnman * the logger rate-limits per call site. */ void LogPQOutcome(const CNode& node, const PQLogLine& line) const; + /** Warn that this node's own ML-KEM code may be at fault. */ + void LogPQLocalFault() const; + /** The hybrid negotiation mode of new v2 connections. Off until the option sets it (part 6); * set before the connection threads start. */ PQMode m_pq_mode{PQMode::OFF}; @@ -1982,6 +2064,15 @@ class CConnman std::array m_pq_failures GUARDED_BY(m_pq_mutex); const uint256 m_pq_instance_id{GetRandHash()}; const NodeSeconds m_pq_since{Now()}; + //! Outbound endpoints with counted failures: streaks, fallback windows and escalation. + std::map m_pq_history GUARDED_BY(m_pq_mutex); + uint64_t m_pq_history_insertions GUARDED_BY(m_pq_mutex){0}; + //! Outbound endpoints that completed a hybrid handshake since startup. Never evicted; its + //! size is bounded by the distinct successful outbound endpoints. + std::set m_pq_successful_endpoints GUARDED_BY(m_pq_mutex); + //! Distinct endpoints with counted failures, up to PQ_LOCAL_FAULT_THRESHOLD. + std::set m_pq_failed_endpoints_for_warning GUARDED_BY(m_pq_mutex); + bool m_pq_local_fault_warned GUARDED_BY(m_pq_mutex){false}; const CChainParams& m_params; diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index 12a69367d9..0cc65f4339 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -181,6 +182,32 @@ std::string Backends() return strprintf(" arith=%s keccak=%s", backends.arith, backends.keccak); } +/** Counts the log lines that contain text while alive. */ +class LogLineCounter +{ + const std::string m_text; + std::list>::iterator m_callback; + +public: + std::atomic m_count{0}; + + explicit LogLineCounter(std::string text) : m_text{std::move(text)} + { + m_callback = LogInstance().PushBackCallback([this](const std::string& line) { + if (line.find(m_text) != std::string::npos) ++m_count; + }); + } + ~LogLineCounter() { LogInstance().DeleteCallback(m_callback); } +}; + +/** Our outbound node's peer: a responder. */ +V2Transport Responder(V2PQOptions pq = {.mode = PQMode::NEGOTIATE}) { return {/*nodeid=*/1000, /*initiating=*/false, pq}; } +/** Our inbound node's peer: an initiator. */ +V2Transport Initiator(V2PQOptions pq = {.mode = PQMode::NEGOTIATE}) { return {/*nodeid=*/1000, /*initiating=*/true, pq}; } + +/** 64 bytes an initiator could send as its key: any 64 bytes are a valid ElligatorSwift key. */ +std::vector InitiatorKey() { return FastRandomContext{}.randbytes(EllSwiftPubKey::size()); } + struct PQNetSetup : public RegTestingSetup { NoMessages m_events; ConnmanTestMsg m_connman{0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()}; @@ -188,6 +215,7 @@ struct PQNetSetup : public RegTestingSetup { PQNetSetup() { + static_assert(PQ_FAILURE_THRESHOLD == 3); CConnman::Options options; options.m_max_automatic_connections = DEFAULT_MAX_PEER_CONNECTIONS; m_connman.Init(options); @@ -303,15 +331,83 @@ struct PQNetSetup : public RegTestingSetup { void Disconnect() { m_connman.DisconnectNodesPublic(); } PQTransportStats Stats() const { return m_connman.GetPQTransportStats(); } -}; -/** Our outbound node's peer: a responder. */ -V2Transport Responder(V2PQOptions pq = {.mode = PQMode::NEGOTIATE}) { return {/*nodeid=*/1000, /*initiating=*/false, pq}; } -/** Our inbound node's peer: an initiator. */ -V2Transport Initiator(V2PQOptions pq = {.mode = PQMode::NEGOTIATE}) { return {/*nodeid=*/1000, /*initiating=*/true, pq}; } + /** How a test connection ends. */ + enum class End { MALFORMED, CONFIRMATION, PEER_EOF, PEER_RESET, TIMEOUT, LOCAL, SEND_ERROR, LEGACY, SUCCESS }; -/** 64 bytes an initiator could send as its key: any 64 bytes are a valid ElligatorSwift key. */ -std::vector InitiatorKey() { return FastRandomContext{}.randbytes(EllSwiftPubKey::size()); } + /** One outbound connection to endpoint that ends as end; the connection manager finalizes it. */ + void Connect(const PQEndpointKey& endpoint, End end) + { + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::NEGOTIATE, endpoint)}; + switch (end) { + case End::MALFORMED: { // an offer of the wrong length + Pass(link); + RawPeer peer{/*initiator=*/false}; + peer.Initialize(NodeKey(link)); + Receive(link, peer.Key()); + Receive(link, peer.TerminatorAndVersion(MakeRecord(PQ_MLKEM1024, 100))); + Pass(link); + break; + } + case End::CONFIRMATION: { + V2Transport peer{Responder({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true})}; + Exchange(link, peer); + break; + } + case End::LEGACY: + case End::SUCCESS: { + V2Transport peer{end == End::LEGACY ? Responder({}) : Responder()}; + Exchange(link, peer); + BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().confirmed == (end == End::SUCCESS)); + link.node->RequestDisconnect(); + break; + } + case End::PEER_EOF: + case End::PEER_RESET: + case End::TIMEOUT: + case End::LOCAL: + case End::SEND_ERROR: { // we switch, and the connection closes before the peer's confirmation + V2Transport peer{Responder()}; + Pass(link); + BOOST_REQUIRE(NodeToPeer(link, peer)); + PeerToNode(link, peer); + if (end == End::SEND_ERROR) link.sock->m_fail_send = true; + Pass(link); + BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().switched); + if (end == End::PEER_EOF) link.pipes->recv.Eof(); + if (end == End::PEER_RESET) link.sock->m_fail_recv = true; + if (end == End::TIMEOUT) SetMockTime(GetTime() + 61s); + if (end == End::LOCAL) link.node->RequestDisconnect(); + Pass(link); + break; + } + } + BOOST_REQUIRE(link.node->fDisconnect); + Disconnect(); + } + + std::optional Streak(const PQEndpointKey& endpoint) const + { + for (const PQStreakStats& streak : Stats().failure_streaks) { + if (streak.endpoint == endpoint) return streak; + } + return std::nullopt; + } + + std::optional Fallback(const PQEndpointKey& endpoint) const + { + for (const PQFallbackStats& fallback : Stats().fallback_set) { + if (fallback.endpoint == endpoint) return fallback; + } + return std::nullopt; + } +}; + +/** A distinct outbound endpoint per index. */ +PQEndpointKey TestEndpoint(uint8_t subnet, int index) +{ + return LookupNumeric(strprintf("10.%d.%d.%d", subnet, index / 250, 1 + index % 250), 8333); +} } // namespace @@ -633,8 +729,10 @@ BOOST_AUTO_TEST_CASE(pq_abandoned) BOOST_AUTO_TEST_CASE(pq_closed_after_switch) { // We switched, and the peer closed or stopped responding before its confirmation arrived. + // Each connection goes to its own endpoint, so none enters the fallback set. + uint16_t port{8000}; for (const std::string cause : {"eof", "reset", "timeout", "local", "send_error"}) { - Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY)}; + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::NEGOTIATE, LookupNumeric("1.2.3.4", ++port))}; V2Transport peer{Responder()}; Pass(link); // our key and garbage BOOST_REQUIRE(NodeToPeer(link, peer)); @@ -936,6 +1034,10 @@ BOOST_AUTO_TEST_CASE(pq_cipher_state_closes) BOOST_CHECK(entry.outcome == PQOutcome::INTERNAL_ERROR); BOOST_CHECK_EQUAL(entry.reason, "cipher_state"); } + // A local fault never counts toward the fallback. + BOOST_CHECK(!Streak(endpoint)); + BOOST_CHECK(!m_connman.IsPQFallback(endpoint, Now())); + BOOST_CHECK_EQUAL(stats.outbound.fallback, 0U); } } @@ -1013,4 +1115,295 @@ BOOST_AUTO_TEST_CASE(pq_finalization_wipes) } } +BOOST_AUTO_TEST_CASE(pq_fallback_threshold) +{ + using enum End; + struct Case { + End end; + std::string cause; + std::vector reasons; + }; + int index{0}; + for (const Case& test : {Case{MALFORMED, "malformed_record", {"ek_length"}}, Case{CONFIRMATION, "first_packet_failed", {"length", "tag"}}, + Case{PEER_EOF, "closed_after_switch", {"eof"}}, Case{PEER_RESET, "closed_after_switch", {"reset"}}, + Case{TIMEOUT, "closed_after_switch", {"timeout"}}}) { + BOOST_TEST_MESSAGE("counted failure: " + test.cause + " " + test.reasons.front()); + const PQEndpointKey endpoint{TestEndpoint(3, index++)}; + Connect(endpoint, test.end); + Connect(endpoint, test.end); + // Our own closes, send errors and legacy outcomes are neutral: they neither advance nor + // reset the streak. + Connect(endpoint, LOCAL); + Connect(endpoint, SEND_ERROR); + Connect(endpoint, LEGACY); + BOOST_CHECK(!m_connman.IsPQFallback(endpoint, Now())); + const auto streak{Streak(endpoint)}; + BOOST_REQUIRE(streak); + BOOST_CHECK_EQUAL(streak->streak, 2U); + BOOST_CHECK_EQUAL(PQOutcomeString(streak->cause), test.cause); + BOOST_CHECK(std::ranges::count(test.reasons, streak->reason) == 1); + BOOST_CHECK(streak->next_window == 3600s); + + // The third enters the fallback set once, with one warning. + const uint64_t fallbacks{Stats().outbound.fallback}; + const NodeSeconds failure_time{Now() + (test.end == TIMEOUT ? 61s : 0s)}; + { + const std::string until{FormatISO8601DateTime(TicksSinceEpoch(failure_time + 3600s))}; + const std::string text{strprintf("v2 pq: fallback cause=%s until=%s role=initiator conn_type=outbound-full-relay peer=%d%s", + test.cause, until, m_next_id, Backends())}; + DebugLogHelper expect{text, AtLevel({text}, LineLevel::WARNING)}; + Connect(endpoint, test.end); + } + BOOST_CHECK(m_connman.IsPQFallback(endpoint, Now())); + PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.outbound.fallback, fallbacks + 1); + const auto fallback{Fallback(endpoint)}; + BOOST_REQUIRE(fallback); + BOOST_CHECK_EQUAL(PQOutcomeString(fallback->cause), test.cause); + BOOST_CHECK_EQUAL(fallback->streak, 3U); + BOOST_CHECK(fallback->entered == failure_time); + BOOST_CHECK(fallback->expires == failure_time + 3600s); + BOOST_CHECK(!Streak(endpoint)); + const PQFailureEntry& entry{stats.outbound_failures.entries.back()}; + BOOST_CHECK(entry.outcome == PQOutcome::FALLBACK); + BOOST_CHECK_EQUAL(entry.reason, test.cause); + BOOST_CHECK(entry.endpoint == endpoint); + BOOST_CHECK(!entry.inbound); + + // A failure from a connection that was already running doesn't enter again. + Connect(endpoint, test.end); + stats = Stats(); + BOOST_CHECK_EQUAL(stats.outbound.fallback, fallbacks + 1); + BOOST_CHECK(Fallback(endpoint)->expires == failure_time + 3600s); + } + + // Inbound failures never count: inbound connections never fall back. + const CService inbound_addr{LookupNumeric("10.3.1.1", 50000)}; + for (int i{0}; i < 3; ++i) { + Link link{Add(ConnectionType::INBOUND, PQMode::NEGOTIATE, std::nullopt, inbound_addr)}; + RawPeer peer{/*initiator=*/true}; + Receive(link, peer.Key()); + Pass(link); + peer.Initialize(NodeKey(link)); + Receive(link, peer.TerminatorAndVersion(MakeRecord(PQ_MLKEM1024, 100))); + Pass(link); + BOOST_REQUIRE(link.node->fDisconnect); + Disconnect(); + } + BOOST_CHECK_EQUAL(Stats().inbound.malformed_record, 3U); + BOOST_CHECK(!Streak(inbound_addr)); + BOOST_CHECK(!m_connman.IsPQFallback(inbound_addr, Now())); +} + +BOOST_AUTO_TEST_CASE(pq_fallback_windows) +{ + const PQEndpointKey endpoint{LookupNumeric("5.6.7.8", 18555)}; + const auto create_sock_orig{CreateSock}; + CreateSock = [](int, int, int) -> std::unique_ptr { return std::make_unique(); }; + // A new outbound connection to the endpoint, as the connection logic opens one. + const auto new_status{[&] { + const std::unique_ptr node{m_connman.ConnectNodeOnly("5.6.7.8:18555", ConnectionType::OUTBOUND_FULL_RELAY, /*use_v2transport=*/true)}; + BOOST_REQUIRE(node); + return node->m_transport->GetInfo().transport_pq_status; + }}; + BOOST_CHECK(new_status() == PQStatus::PENDING); + // A session that is running when the endpoint enters the fallback set. + Link running{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::NEGOTIATE, endpoint)}; + + for (const std::chrono::seconds window : {3600s, 14400s, 86400s, 86400s}) { + Connect(endpoint, End::MALFORMED); + Connect(endpoint, End::MALFORMED); + BOOST_REQUIRE(Streak(endpoint)); + BOOST_CHECK(Streak(endpoint)->next_window == window); + const NodeSeconds start{Now()}; + const uint64_t fallbacks{Stats().outbound.fallback}; + Connect(endpoint, End::MALFORMED); + BOOST_CHECK_EQUAL(Stats().outbound.fallback, fallbacks + 1); + BOOST_REQUIRE(Fallback(endpoint)); + BOOST_CHECK(Fallback(endpoint)->expires == start + window); + BOOST_CHECK(new_status() == PQStatus::FALLBACK); + + // A failure during the window neither extends it nor enters again. + SetMockTime(TicksSinceEpoch(start + window - 1s)); + Connect(endpoint, End::MALFORMED); + BOOST_CHECK(m_connman.IsPQFallback(endpoint, Now())); + BOOST_CHECK(Fallback(endpoint)->expires == start + window); + BOOST_CHECK_EQUAL(Stats().outbound.fallback, fallbacks + 1); + + // At expiry the window ends, and the streak restarts at 0. + SetMockTime(TicksSinceEpoch(start + window)); + BOOST_CHECK(!Fallback(endpoint)); + BOOST_CHECK(!m_connman.IsPQFallback(endpoint, Now())); + BOOST_CHECK(!Streak(endpoint)); + BOOST_CHECK(new_status() == PQStatus::PENDING); + } + // Existing sessions never renegotiate. + BOOST_CHECK(running.node->m_transport->GetInfo().transport_pq_status == PQStatus::PENDING); + CreateSock = create_sock_orig; +} + +BOOST_AUTO_TEST_CASE(pq_success_protection) +{ + const PQEndpointKey endpoint{TestEndpoint(4, 0)}; + // A connection that will fail late: we switch now, and the peer's bad confirmation waits. + Link late{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::NEGOTIATE, endpoint)}; + V2Transport bad_peer{Responder({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true})}; + Pass(late); + BOOST_REQUIRE(NodeToPeer(late, bad_peer)); + PeerToNode(late, bad_peer); + Pass(late); + BOOST_REQUIRE(late.node->m_transport->GetPQSnapshot().switched); + BOOST_CHECK(!NodeToPeer(late, bad_peer)); // the peer switched to other keys, and fails ours + const std::vector late_confirmation{PeerBytes(bad_peer)}; + + // An earlier failure, then a success: the history goes, and the endpoint is protected. + Connect(endpoint, End::MALFORMED); + BOOST_CHECK(Streak(endpoint)); + Connect(endpoint, End::SUCCESS); + BOOST_CHECK(!Streak(endpoint)); + + // The late failure arrives after the success, and can't undo it. + Receive(late, late_confirmation); + Pass(late); + BOOST_REQUIRE(late.node->m_transport->GetPQSnapshot().failure != PQFailure::NONE); + Disconnect(); + BOOST_CHECK(!Streak(endpoint)); + + // More than PQ_MAX_ENDPOINT_HISTORY other endpoints churn the history. + for (int i{1}; i <= int{PQ_MAX_ENDPOINT_HISTORY} + 1; ++i) Connect(TestEndpoint(4, i), End::MALFORMED); + BOOST_CHECK_EQUAL(Stats().failure_streaks.size(), PQ_MAX_ENDPOINT_HISTORY); + + // The protected endpoint never falls back; its failures are still counted. + for (const End end : {End::MALFORMED, End::CONFIRMATION, End::PEER_EOF, End::MALFORMED}) Connect(endpoint, end); + BOOST_CHECK(!m_connman.IsPQFallback(endpoint, Now())); + BOOST_CHECK(!Streak(endpoint)); + BOOST_CHECK(!Fallback(endpoint)); + const PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.outbound.fallback, 0U); + BOOST_CHECK_EQUAL(stats.outbound.first_packet_failed, 2U); + BOOST_CHECK_EQUAL(stats.outbound.closed_after_switch, 1U); +} + +BOOST_AUTO_TEST_CASE(pq_history_eviction) +{ + // 1,001 endpoints fail once each, all at the same time, inserted in descending key order so + // that insertion order and map order disagree. + const int newest{int{PQ_MAX_ENDPOINT_HISTORY}}; + for (int i{newest}; i >= 0; --i) Connect(TestEndpoint(6, i), End::MALFORMED); + PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.failure_streaks.size(), PQ_MAX_ENDPOINT_HISTORY); + // The first insertion, which has the largest key, is the one evicted. + BOOST_CHECK(!Streak(TestEndpoint(6, newest))); + BOOST_CHECK(Streak(TestEndpoint(6, 0))); + BOOST_CHECK(Streak(TestEndpoint(6, 1))); + BOOST_CHECK(Streak(TestEndpoint(6, newest - 1))); + // The rings keep their own bound and counts. + BOOST_CHECK_EQUAL(stats.outbound.malformed_record, PQ_MAX_ENDPOINT_HISTORY + 1); + BOOST_CHECK_EQUAL(stats.outbound_failures.last_sequence, PQ_MAX_ENDPOINT_HISTORY + 1); + BOOST_CHECK_EQUAL(stats.outbound_failures.entries.size(), PQ_FAILURE_RING_SIZE); + BOOST_CHECK_EQUAL(stats.outbound_failures.dropped, PQ_MAX_ENDPOINT_HISTORY + 1 - PQ_FAILURE_RING_SIZE); + + // Eviction follows insertion, not the latest failure or the key: the oldest remaining + // insertion fails again, later than every other, and is still the next to go. + SetMockTime(GetTime() + 1s); + Connect(TestEndpoint(6, newest - 1), End::MALFORMED); + BOOST_CHECK_EQUAL(Streak(TestEndpoint(6, newest - 1))->streak, 2U); + Connect(TestEndpoint(6, newest + 1), End::MALFORMED); + BOOST_CHECK(!Streak(TestEndpoint(6, newest - 1))); + BOOST_CHECK(Streak(TestEndpoint(6, newest - 2))); + BOOST_CHECK(Streak(TestEndpoint(6, 0))); + BOOST_CHECK(Streak(TestEndpoint(6, newest + 1))); + BOOST_CHECK_EQUAL(Stats().failure_streaks.size(), PQ_MAX_ENDPOINT_HISTORY); +} + +namespace { +/** The warning names the backends that ran, and no remedy: no option forces portable code yet. */ +std::string LocalFaultLine() +{ + const auto backends{mlkem::GetBackendNames()}; + return strprintf("v2 pq: local_fault arith=%s keccak=%s: hybrid handshakes failed with 8 distinct endpoints and none succeeded " + "since startup, so this node's own ML-KEM code may be at fault.", + backends.arith, backends.keccak); +} +} // namespace + +BOOST_AUTO_TEST_CASE(pq_local_fault_warning) +{ + // Whatever backends this machine runs. + LogLineCounter warnings{"v2 pq: local_fault"}; + for (int i{0}; i < 7; ++i) Connect(TestEndpoint(7, i), End::MALFORMED); + BOOST_CHECK_EQUAL(warnings.m_count, 0); + { + const std::string text{LocalFaultLine()}; + DebugLogHelper expect{text, AtLevel({text}, LineLevel::WARNING)}; + Connect(TestEndpoint(7, 7), End::MALFORMED); + } + BOOST_CHECK_EQUAL(warnings.m_count, 1); + // It warns once, and behavior doesn't change. + for (int i{0}; i < 12; ++i) Connect(TestEndpoint(7, i), End::MALFORMED); + BOOST_CHECK_EQUAL(warnings.m_count, 1); + BOOST_CHECK(m_connman.IsPQFallback(TestEndpoint(7, 0), Now()) == false); + BOOST_CHECK_EQUAL(Streak(TestEndpoint(7, 0))->streak, 2U); +} + +BOOST_AUTO_TEST_CASE(pq_local_fault_warning_portable) +{ + mlkem::ForcePortableForTesting portable; + BOOST_CHECK_EQUAL(mlkem::GetBackendNames().arith, "portable"); + const std::string text{LocalFaultLine()}; + BOOST_CHECK(text.find("arith=portable keccak=portable:") != std::string::npos); + LogLineCounter warnings{"v2 pq: local_fault"}; + { + DebugLogHelper expect{text, AtLevel({text}, LineLevel::WARNING)}; + for (int i{0}; i < 8; ++i) Connect(TestEndpoint(7, i), End::MALFORMED); + } + BOOST_CHECK_EQUAL(warnings.m_count, 1); +} + +BOOST_AUTO_TEST_CASE(pq_local_fault_after_success) +{ + // One success since startup rules out a local fault. + LogLineCounter warnings{"v2 pq: local_fault"}; + Connect(TestEndpoint(8, 0), End::SUCCESS); + for (int i{1}; i <= 12; ++i) Connect(TestEndpoint(8, i), End::MALFORMED); + BOOST_CHECK_EQUAL(warnings.m_count, 0); +} + +BOOST_AUTO_TEST_CASE(pq_fallback_connection) +{ + // A fallback connection to a peer that sends a malformed offer: plain v2, with the offer + // neither parsed nor validated. + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::FALLBACK)}; + Pass(link); + RawPeer raw{/*initiator=*/false}; + raw.Initialize(NodeKey(link)); + Receive(link, raw.Key()); + Receive(link, raw.TerminatorAndVersion(MakeRecord(PQ_MLKEM1024, 100))); + Pass(link); + BOOST_CHECK(!link.node->fDisconnect); + Transport::Info info{link.node->m_transport->GetInfo()}; + BOOST_CHECK(info.transport_type == TransportProtocolType::V2); + BOOST_CHECK(info.transport_pq_status == PQStatus::FALLBACK); + BOOST_CHECK(!info.transport_pq); + + // Toward an upgraded responder, which offers: our version stays empty, and the peer sees a + // legacy initiator. + Link upgraded{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::FALLBACK)}; + V2Transport peer{Responder()}; + Exchange(upgraded, peer); + BOOST_CHECK(!upgraded.node->fDisconnect); + info = upgraded.node->m_transport->GetInfo(); + BOOST_CHECK(info.transport_type == TransportProtocolType::V2); + BOOST_CHECK(info.transport_pq_status == PQStatus::FALLBACK); + BOOST_CHECK(peer.GetInfo().transport_pq_status == PQStatus::LEGACY_PEER); + + // Nothing is counted for either. + link.node->RequestDisconnect(); + upgraded.node->RequestDisconnect(); + Disconnect(); + const PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.outbound.switched + stats.outbound.legacy_peer + stats.outbound.malformed_record, 0U); + BOOST_CHECK(stats.outbound_failures.entries.empty()); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/net.h b/src/test/util/net.h index a1ecd814c7..450098bca4 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -92,7 +92,7 @@ struct ConnmanTestMsg : public CConnman { bool AlreadyConnectedPublic(const CAddress& addr) { return AlreadyConnectedToAddress(addr); }; CNode* ConnectNodePublic(PeerManager& peerman, const char* pszDest, ConnectionType conn_type) - EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !m_pq_mutex); void SocketHandlerConnectedPublic(const std::vector& nodes, const Sock::EventsPerSock& events_per_sock) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_pq_mutex) @@ -114,7 +114,7 @@ struct ConnmanTestMsg : public CConnman { /** ConnectNode() without adding the node to the connection manager; the caller owns it. */ CNode* ConnectNodeOnly(const char* dest, ConnectionType conn_type, bool use_v2transport) - EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex) + EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !m_pq_mutex) { return ConnectNode(CAddress{}, dest, /*fCountFailure=*/false, conn_type, use_v2transport); } From 4b7b3e3a2b51c494185e2515bbe4f695e6073001 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Mon, 5 Oct 2026 16:16:05 -0400 Subject: [PATCH 04/12] net: shed inbound PQ offers under handshake floods Bound the extra CPU an inbound connection flood can cost. Each offer costs the responder one ML-KEM-1024 key generation; above a rate threshold, responders stop offering and cost exactly what plain v2 costs today. - V2PQOptions gains an offer gate (a function pointer and context, like PQRandomSource). A negotiating responder calls it at its key generation point, once the initiator's 64-byte key has arrived; when it declines, the responder wipes the retained secret and sends empty version contents, and its status is off. - CConnman's gate counts the attempted offers per second. More than threshold_per_s in the current second starts shedding; ten consecutive complete seconds with fewer than half the threshold's attempts stop it. The quiet seconds before a flood don't count toward its end, so shedding can't flap within a second. The seconds come from the steady clock (MockableSteadyClock), so a wall clock step can't merge seconds or stretch a period; only the reported start time is wall-clock time. The socket handler also checks the clock, so shedding stops on time when the flood stops entirely. - Each shed offer increments the inbound shed counter and adds no ring entry, so a flood can't wash out the evidence. Starting and stopping each log one info line: "v2 pq: load_shedding started threshold_per_s=" and "v2 pq: load_shedding stopped". - The threshold defaults to DEFAULT_PQ_SHED_THRESHOLD_PER_S (1,000) and CConnman::Options::pq_shed_threshold_per_s overrides it for unit tests and the flood lab; there is no new user-facing option. - GetPQTransportStats() gains load_shedding {active, threshold_per_s, since} for part 6. - Outbound connections are never affected. The gate's mutex is taken under the transport's locks, and nothing holding it calls into a transport or takes m_pq_mutex. net_pq_tests drives inbound connections through the accept path under mock time: offers up to the threshold, the first one above it shed (empty contents, so the initiator sees a legacy responder; status off; shed +1), outbound connections still hybrid, a second at half the threshold restarting the quiet run, offers resuming only after ten quiet seconds, and each transition logged once at info level. A wall clock step back by an hour under a trickle of offers sheds nothing, and a step forward doesn't end a flood early. The real socket handler thread, run as Start() runs it, ends a flood that stopped. With the switch off the gate is never asked. Work item F4 of #206 (#184 part 5). Co-Authored-By: Claude Opus 5.5 --- src/net.cpp | 87 +++++++++++++++- src/net.h | 80 ++++++++++++++- src/test/net_pq_tests.cpp | 211 +++++++++++++++++++++++++++++++++++++- src/test/util/net.h | 17 +++ 4 files changed, 385 insertions(+), 10 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 40f484f02e..f85dd85206 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1211,6 +1211,12 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept } else if (m_initiating) { // Hold the version packet until the responder's arrives: it answers their offer. SetSendState(SendState::AWAITING_VERSION); + } else if (const PQOfferGate& gate{m_pq_options.offer_gate}; gate.allow && !gate.allow(gate.context)) { + // Load shedding: no offer, which is exactly plain v2. + m_pq_shed = true; + ClearHybridSecrets(); + SetSendState(SendState::READY); + AppendVersionPacket(VERSION_CONTENTS); } else { // Offer an encapsulation key in the version packet, and send nothing else until the // initiator's version packet is processed. The key is generated only now that the @@ -1863,6 +1869,7 @@ PQStatus V2Transport::GetPQStatus() const noexcept case PQMode::FALLBACK: return PQStatus::FALLBACK; case PQMode::NEGOTIATE: break; } + if (m_pq_shed) return PQStatus::OFF; const PQHandshake::Snapshot snapshot{m_pq.GetSnapshot()}; if (snapshot.confirmed) return PQStatus::HYBRID; switch (snapshot.failure) { @@ -2170,7 +2177,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, .prefer_evict = discouraged, .recv_flood_size = nReceiveFloodSize, .use_v2transport = use_v2transport, - .pq = {.mode = m_pq_mode}, + .pq = {.mode = m_pq_mode, .offer_gate = {.allow = &CConnman::AllowPQOffer, .context = this}}, // The actual remote address and source port. .pq_endpoint = addr, }); @@ -2603,6 +2610,15 @@ void CConnman::LogPQLocalFault() const PQTransportStats CConnman::GetPQTransportStats() const { AssertLockNotHeld(m_pq_mutex); + AssertLockNotHeld(m_pq_shed_mutex); + // The two mutexes never nest. + PQLoadSheddingStats load_shedding; + { + LOCK(m_pq_shed_mutex); + load_shedding = {.active = m_pq_shedding_since.has_value(), + .threshold_per_s = m_pq_shed_threshold, + .since = m_pq_shedding_since.value_or(NodeSeconds{})}; + } LOCK(m_pq_mutex); PQTransportStats stats{ .instance_id = m_pq_instance_id, @@ -2613,6 +2629,7 @@ PQTransportStats CConnman::GetPQTransportStats() const .outbound_failures = m_pq_failures[PQDirection(/*inbound=*/false)], .fallback_set = {}, .failure_streaks = {}, + .load_shedding = load_shedding, }; const NodeSeconds now{Now()}; for (const auto& [endpoint, history] : m_pq_history) { @@ -2628,6 +2645,73 @@ PQTransportStats CConnman::GetPQTransportStats() const return stats; } +bool CConnman::AllowPQOffer(void* connman) noexcept +{ + return static_cast(connman)->AllowPQOffer(); +} + +bool CConnman::AllowPQOffer() noexcept +{ + AssertLockNotHeld(m_pq_shed_mutex); + const PQShedSeconds now{PQShedNow()}; + bool stopped; + std::optional started; + bool offer; + { + LOCK(m_pq_shed_mutex); + stopped = AdvancePQShedding(now); + ++m_pq_shed_attempts; + if (!m_pq_shedding_since && m_pq_shed_attempts > m_pq_shed_threshold) { + m_pq_shedding_since = Now(); + // The quiet seconds before the flood don't count toward its end. + m_pq_quiet_since.reset(); + started = m_pq_shed_threshold; + } + offer = !m_pq_shedding_since; + } + if (!offer) ++m_pq_counters[PQDirection(/*inbound=*/true)].shed; + if (stopped) LogPQShedding(std::nullopt); + if (started) LogPQShedding(started); + return offer; +} + +bool CConnman::AdvancePQShedding(PQShedSeconds now) +{ + AssertLockHeld(m_pq_shed_mutex); + if (now > m_pq_shed_second) { + // Close the current second, and the seconds without attempts since: a second is quiet + // with fewer than half the threshold's attempts. + if (2 * m_pq_shed_attempts >= m_pq_shed_threshold) { + m_pq_quiet_since.reset(); + if (now > m_pq_shed_second + 1s) m_pq_quiet_since = m_pq_shed_second + 1s; + } else if (!m_pq_quiet_since) { + m_pq_quiet_since = m_pq_shed_second; + } + m_pq_shed_second = now; + m_pq_shed_attempts = 0; + } + if (m_pq_shedding_since && m_pq_quiet_since && now - *m_pq_quiet_since >= PQ_SHED_QUIET_PERIOD) { + m_pq_shedding_since.reset(); + return true; + } + return false; +} + +void CConnman::UpdatePQShedding(PQShedSeconds now) +{ + AssertLockNotHeld(m_pq_shed_mutex); + if (WITH_LOCK(m_pq_shed_mutex, return AdvancePQShedding(now))) LogPQShedding(std::nullopt); +} + +void CConnman::LogPQShedding(std::optional started_threshold) +{ + if (started_threshold) { + LogInfo("v2 pq: load_shedding started threshold_per_s=%u", *started_threshold); + } else { + LogInfo("v2 pq: load_shedding stopped"); + } +} + void CConnman::DisconnectNodes() { AssertLockNotHeld(m_nodes_mutex); @@ -2966,6 +3050,7 @@ void CConnman::ThreadSocketHandler() { DisconnectNodes(); NotifyNumConnectionsChanged(); + if (m_pq_mode == PQMode::NEGOTIATE) UpdatePQShedding(PQShedNow()); SocketHandler(); } } diff --git a/src/net.h b/src/net.h index 28fe36ca8b..b6c722a0af 100644 --- a/src/net.h +++ b/src/net.h @@ -468,6 +468,14 @@ class V1Transport final : public Transport bool ShouldReconnectV1() const noexcept override { return false; } }; +/** Asked by a negotiating responder, at its key generation point, whether to offer: load + * shedding (#184 part 5). Each call counts one attempted offer. */ +struct PQOfferGate { + using Allow = bool (*)(void* context) noexcept; + Allow allow{nullptr}; + void* context{nullptr}; +}; + /** The hybrid post-quantum negotiation of one v2 connection (doc/design/pq-transport.md). */ struct V2PQOptions { PQMode mode{PQMode::OFF}; @@ -477,6 +485,8 @@ struct V2PQOptions { /** Test only: invert byte 0 of the local ML-KEM shared secret before deriving keys, so the * peer's key confirmation check fails. EK and CT are unchanged. */ bool corrupt_shared_secret{false}; + /** Responders only; without one, a negotiating responder always offers. */ + PQOfferGate offer_gate{}; }; class V2Transport final : public Transport @@ -653,6 +663,8 @@ class V2Transport final : public Transport RecvState m_recv_state GUARDED_BY(m_recv_mutex); /** The hybrid negotiation state: KEM secrets and progress. */ PQHandshake m_pq GUARDED_BY(m_recv_mutex); + /** Whether the offer gate declined to offer (load shedding): plain v2, status off. */ + bool m_pq_shed GUARDED_BY(m_recv_mutex){false}; /** Lock for sending-side fields. If both sending and receiving fields are accessed, * m_recv_mutex must be acquired before m_send_mutex. */ @@ -877,6 +889,20 @@ inline constexpr size_t PQ_MAX_ENDPOINT_HISTORY{1000}; /** Distinct endpoints with counted failures, and no success since startup, that suggest a local fault. */ inline constexpr size_t PQ_LOCAL_FAULT_THRESHOLD{8}; +/** Inbound offers attempted within one second above which responders stop offering. R3's flood + * measurement (#222) sets the final value; CConnman::Options can override it. */ +inline constexpr uint64_t DEFAULT_PQ_SHED_THRESHOLD_PER_S{1000}; +/** Consecutive seconds with fewer than half the threshold's attempts that end load shedding. */ +inline constexpr std::chrono::seconds PQ_SHED_QUIET_PERIOD{10}; + +/** Inbound offer load shedding. */ +struct PQLoadSheddingStats { + bool active; + uint64_t threshold_per_s; + //! When the current shedding period started; the epoch (0) when inactive. + NodeSeconds since; +}; + /** An endpoint's counted failures and fallback windows. */ struct PQEndpointHistory { //! Consecutive counted failures since the last fallback window ended. @@ -930,6 +956,7 @@ struct PQTransportStats { PQFailureRing outbound_failures; std::vector fallback_set; std::vector failure_streaks; + PQLoadSheddingStats load_shedding; }; struct CNodeOptions @@ -1382,9 +1409,11 @@ class CConnman bool m_i2p_accept_incoming; bool whitelist_forcerelay = DEFAULT_WHITELISTFORCERELAY; bool whitelist_relay = DEFAULT_WHITELISTRELAY; + //! Inbound offer load shedding threshold (unit tests and the flood lab). + uint64_t pq_shed_threshold_per_s{DEFAULT_PQ_SHED_THRESHOLD_PER_S}; }; - void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_total_bytes_sent_mutex) + void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_total_bytes_sent_mutex, !m_pq_shed_mutex) { AssertLockNotHeld(m_total_bytes_sent_mutex); @@ -1416,6 +1445,11 @@ class CConnman m_onion_binds = connOptions.onion_binds; whitelist_forcerelay = connOptions.whitelist_forcerelay; whitelist_relay = connOptions.whitelist_relay; + { + LOCK(m_pq_shed_mutex); + // At least 1: a threshold of 0 would shed every offer and never stop. + m_pq_shed_threshold = std::max(connOptions.pq_shed_threshold_per_s, 1); + } } CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, const NetGroupManager& netgroupman, @@ -1423,7 +1457,7 @@ class CConnman ~CConnman(); - bool Start(CScheduler& scheduler, const Options& options) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !m_added_nodes_mutex, !m_addr_fetches_mutex, !mutexMsgProc); + bool Start(CScheduler& scheduler, const Options& options) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !m_added_nodes_mutex, !m_addr_fetches_mutex, !mutexMsgProc, !m_pq_shed_mutex); void StopThreads(); void StopNodes(); @@ -1583,7 +1617,7 @@ class CConnman uint64_t GetTotalBytesSent() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); /** An owning copy of the hybrid transport counters, failure rings and endpoint history. */ - PQTransportStats GetPQTransportStats() const EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); + PQTransportStats GetPQTransportStats() const EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex, !m_pq_shed_mutex); /** Whether new outbound connections to endpoint run plain v2, because repeated hybrid * failures put it in the fallback set. Ends a window that expired. */ @@ -1674,7 +1708,7 @@ class CConnman */ void SocketHandlerListening(const Sock::EventsPerSock& events_per_sock); - void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex, !m_pq_mutex); + void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex, !m_pq_mutex, !m_pq_shed_mutex); void ThreadDNSAddressSeed() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex); uint64_t CalculateKeyedNetGroup(const CNetAddr& ad) const; @@ -2058,7 +2092,8 @@ class CConnman PQMode m_pq_mode{PQMode::OFF}; mutable Mutex m_pq_mutex; - //! Event counters, inbound [0] and outbound [1]. Written under m_pq_mutex. + //! Event counters, inbound [0] and outbound [1]. Written under m_pq_mutex, except shed, + //! which the offer gate counts under m_pq_shed_mutex. std::array m_pq_counters; //! Failure rings, inbound [0] and outbound [1]: inbound noise can't evict outbound evidence. std::array m_pq_failures GUARDED_BY(m_pq_mutex); @@ -2074,6 +2109,41 @@ class CConnman std::set m_pq_failed_endpoints_for_warning GUARDED_BY(m_pq_mutex); bool m_pq_local_fault_warned GUARDED_BY(m_pq_mutex){false}; + /** + * Inbound offer load shedding, with one-second counters. The seconds come from the steady + * clock, so a wall clock step can't merge or stretch them; only the reported start time is + * wall-clock time. Its own mutex is taken under the transport's locks (the offer gate runs + * inside V2Transport), so nothing holding it calls into a transport, and it never nests with + * m_pq_mutex. + */ + using PQShedSeconds = std::chrono::time_point; + static PQShedSeconds PQShedNow() noexcept { return std::chrono::time_point_cast(MockableSteadyClock::now()); } + mutable Mutex m_pq_shed_mutex; + uint64_t m_pq_shed_threshold GUARDED_BY(m_pq_shed_mutex){DEFAULT_PQ_SHED_THRESHOLD_PER_S}; + //! The current second, and the offers attempted in it. + PQShedSeconds m_pq_shed_second GUARDED_BY(m_pq_shed_mutex){}; + uint64_t m_pq_shed_attempts GUARDED_BY(m_pq_shed_mutex){0}; + //! The first of the run of complete seconds with fewer than half the threshold's attempts. + std::optional m_pq_quiet_since GUARDED_BY(m_pq_shed_mutex); + //! When the current shedding period started (wall clock, for the stats); unset when offering. + std::optional m_pq_shedding_since GUARDED_BY(m_pq_shed_mutex); + + /** The offer gate of inbound transports: count one attempted offer, and decline it while + * shedding, counting it as shed. */ + bool AllowPQOffer() noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_pq_shed_mutex); + //! The PQOfferGate function; the transport calls it under its own locks, never under m_pq_shed_mutex. + static bool AllowPQOffer(void* connman) noexcept NO_THREAD_SAFETY_ANALYSIS; + + /** Close the seconds before now, and stop shedding after PQ_SHED_QUIET_PERIOD quiet ones. + * Returns whether shedding stopped. */ + bool AdvancePQShedding(PQShedSeconds now) EXCLUSIVE_LOCKS_REQUIRED(m_pq_shed_mutex); + + /** Stop shedding when the flood is over, even if no new offer is attempted. */ + void UpdatePQShedding(PQShedSeconds now) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_shed_mutex); + + /** Log a load shedding transition: started (with the threshold) or stopped. */ + static void LogPQShedding(std::optional started_threshold); + const CChainParams& m_params; friend struct ConnmanTestMsg; diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index 0cc65f4339..84e7237622 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -216,19 +216,44 @@ struct PQNetSetup : public RegTestingSetup { PQNetSetup() { static_assert(PQ_FAILURE_THRESHOLD == 3); - CConnman::Options options; - options.m_max_automatic_connections = DEFAULT_MAX_PEER_CONNECTIONS; - m_connman.Init(options); - m_connman.SetMsgProc(&m_events); + InitConnman(); m_connman.SetPeerConnectTimeout(60s); m_connman.SetPQMode(PQMode::NEGOTIATE); SetMockTime(GetTime()); + MockableSteadyClock::SetMockTime(STEADY_START); } ~PQNetSetup() { m_connman.Stop(); SetMockTime(0s); + MockableSteadyClock::ClearMockTime(); + } + + //! Where the mocked steady clock, which drives load shedding, starts. + static constexpr std::chrono::seconds STEADY_START{3600}; + + /** Init() with room for inbound peers, v2 accepted, and the given load shedding threshold. */ + void InitConnman(uint64_t shed_threshold = DEFAULT_PQ_SHED_THRESHOLD_PER_S) + { + CConnman::Options options; + options.m_max_automatic_connections = DEFAULT_MAX_PEER_CONNECTIONS; + options.m_local_services = NODE_P2P_V2; + options.pq_shed_threshold_per_s = shed_threshold; + m_connman.Init(options); + m_connman.SetMsgProc(&m_events); + } + + /** An inbound connection through the accept path, with the transport options it gives. */ + Link Accept(const CService& addr = LookupNumeric("10.9.0.1", 50000)) + { + auto pipes{std::make_shared()}; + m_connman.CreateNodeFromAcceptedSocketPublic(std::make_unique(pipes, std::make_shared()), + LookupNumeric("127.0.0.1", 18444), addr); + CNode* node{m_connman.TestNodes().back()}; + auto sock{std::static_pointer_cast(WITH_LOCK(node->m_sock_mutex, return node->m_sock))}; + BOOST_REQUIRE(node->IsInboundConn()); + return {pipes, sock, node}; } /** A connection the connection manager holds, as ConnectNode() or the accept path would add it. */ @@ -1406,4 +1431,182 @@ BOOST_AUTO_TEST_CASE(pq_fallback_connection) BOOST_CHECK(stats.outbound_failures.entries.empty()); } +BOOST_AUTO_TEST_CASE(pq_load_shedding) +{ + BOOST_CHECK_EQUAL(Stats().load_shedding.threshold_per_s, DEFAULT_PQ_SHED_THRESHOLD_PER_S); + // A low threshold, as unit tests and the flood lab set it. + InitConnman(/*shed_threshold=*/10); + BOOST_CHECK_EQUAL(Stats().load_shedding.threshold_per_s, 10U); + LogLineCounter started{"v2 pq: load_shedding started"}; + LogLineCounter stopped{"v2 pq: load_shedding stopped"}; + + // One attempted offer: an inbound connection whose 64-byte key arrived. + const auto attempt{[&] { + Link link{Accept()}; + Receive(link, InitiatorKey()); + Pass(link); + return link.node->m_transport->GetPQSnapshot().offer == PQOfferState::SENT; + }}; + // The steady clock drives the seconds; the wall clock only dates the start. + const NodeSeconds start{Now()}; + const auto at{[&](std::chrono::seconds offset) { + SetMockTime(TicksSinceEpoch(start + offset)); + MockableSteadyClock::SetMockTime(STEADY_START + offset); + }}; + + // Up to the threshold in one second, responders offer. + for (int i{0}; i < 10; ++i) BOOST_CHECK(attempt()); + BOOST_CHECK(!Stats().load_shedding.active); + BOOST_CHECK_EQUAL(started.m_count, 0); + + // More than the threshold within the second starts shedding, with one line. + { + const std::string text{"v2 pq: load_shedding started threshold_per_s=10"}; + DebugLogHelper expect{text, AtLevel({text}, LineLevel::INFO)}; + BOOST_CHECK(!attempt()); + } + // A shed responder sends empty contents, which is exactly plain v2: the peer sees a legacy + // responder, and the connection's status is off. + Link shed{Accept()}; + V2Transport peer{Initiator()}; + Exchange(shed, peer); + BOOST_CHECK(!shed.node->fDisconnect); + BOOST_CHECK(shed.node->m_transport->GetInfo().transport_type == TransportProtocolType::V2); + BOOST_CHECK(shed.node->m_transport->GetInfo().transport_pq_status == PQStatus::OFF); + BOOST_CHECK(peer.GetInfo().transport_pq_status == PQStatus::LEGACY_PEER); + BOOST_CHECK(shed.node->m_transport->GetPQSnapshot().legacy == PQLegacyReason::NONE); + PQTransportStats stats{Stats()}; + BOOST_CHECK(stats.load_shedding.active); + BOOST_CHECK(stats.load_shedding.since == start); + BOOST_CHECK_EQUAL(stats.inbound.shed, 2U); + BOOST_CHECK_EQUAL(started.m_count, 1); + + // Outbound connections are never affected. + Link outbound{Add(ConnectionType::OUTBOUND_FULL_RELAY)}; + V2Transport responder{Responder()}; + Exchange(outbound, responder); + BOOST_CHECK(outbound.node->m_transport->GetInfo().transport_pq_status == PQStatus::HYBRID); + + // Nine quiet seconds (fewer than half the threshold), then a second at half: not quiet, so + // shedding goes on and the run starts over. + for (int second{1}; second <= 10; ++second) { + at(std::chrono::seconds{second}); + for (int i{0}; i < (second == 10 ? 5 : 4); ++i) BOOST_CHECK(!attempt()); + } + // Ten quiet seconds: offers resume in the eleventh, with one line. + for (int second{11}; second <= 20; ++second) { + at(std::chrono::seconds{second}); + for (int i{0}; i < 4; ++i) BOOST_CHECK(!attempt()); + } + BOOST_CHECK(Stats().load_shedding.active); + at(21s); + { + const std::string text{"v2 pq: load_shedding stopped"}; + DebugLogHelper expect{text, AtLevel({text}, LineLevel::INFO)}; + BOOST_CHECK(attempt()); + } + stats = Stats(); + BOOST_CHECK(!stats.load_shedding.active); + BOOST_CHECK(stats.load_shedding.since == NodeSeconds{}); + BOOST_CHECK_EQUAL(stats.inbound.shed, 2U + 9 * 4 + 5 + 10 * 4); + // Shed offers add no ring entries: a flood can't wash out the evidence. + BOOST_CHECK(stats.inbound_failures.entries.empty()); + + // A second flood, which then stops entirely: the socket handler ends shedding on time even + // though no offer is attempted. + for (int i{0}; i < 11; ++i) attempt(); + BOOST_CHECK(Stats().load_shedding.active); + BOOST_CHECK(Stats().load_shedding.since == start + 21s); + at(31s); + m_connman.UpdatePQSheddingPublic(); + BOOST_CHECK(Stats().load_shedding.active); + at(32s); + m_connman.UpdatePQSheddingPublic(); + m_connman.UpdatePQSheddingPublic(); + BOOST_CHECK(!Stats().load_shedding.active); + BOOST_CHECK_EQUAL(started.m_count, 2); + BOOST_CHECK_EQUAL(stopped.m_count, 2); +} + +BOOST_AUTO_TEST_CASE(pq_load_shedding_off) +{ + // With the switch off, inbound transports never ask the gate: nothing is counted or shed. + InitConnman(/*shed_threshold=*/1); + m_connman.SetPQMode(PQMode::OFF); + for (int i{0}; i < 5; ++i) { + Link link{Accept()}; + Receive(link, InitiatorKey()); + Pass(link); + BOOST_CHECK(link.node->m_transport->GetInfo().transport_pq_status == PQStatus::OFF); + } + BOOST_CHECK(!Stats().load_shedding.active); + BOOST_CHECK_EQUAL(Stats().inbound.shed, 0U); +} + +BOOST_AUTO_TEST_CASE(pq_load_shedding_wall_clock_step) +{ + // Offers trickle in, one per second, far below the threshold, while the wall clock steps back + // an hour (an NTP correction or a VM restore). The steady clock still tells the seconds apart, + // so nothing is shed. + InitConnman(/*shed_threshold=*/5); + const auto attempt{[&] { + Link link{Accept()}; + Receive(link, InitiatorKey()); + Pass(link); + const bool offered{link.node->m_transport->GetPQSnapshot().offer == PQOfferState::SENT}; + link.node->RequestDisconnect(); + Disconnect(); + return offered; + }}; + const NodeSeconds wall{Now()}; + BOOST_CHECK(attempt()); + for (int i{1}; i <= 120; ++i) { + SetMockTime(TicksSinceEpoch(wall - 3600s + std::chrono::seconds{i})); + MockableSteadyClock::SetMockTime(STEADY_START + std::chrono::seconds{i}); + m_connman.UpdatePQSheddingPublic(); + BOOST_CHECK(attempt()); + } + BOOST_CHECK(!Stats().load_shedding.active); + BOOST_CHECK_EQUAL(Stats().inbound.shed, 0U); + + // A flood within one steady second starts shedding, and a wall clock step forward doesn't + // end it early: only ten quiet steady seconds do. + MockableSteadyClock::SetMockTime(STEADY_START + 200s); + for (int i{0}; i < 6; ++i) attempt(); + BOOST_CHECK(Stats().load_shedding.active); + SetMockTime(TicksSinceEpoch(wall + 3600s)); + m_connman.UpdatePQSheddingPublic(); + BOOST_CHECK(!attempt()); + BOOST_CHECK(Stats().load_shedding.active); + MockableSteadyClock::SetMockTime(STEADY_START + 211s); + m_connman.UpdatePQSheddingPublic(); + BOOST_CHECK(!Stats().load_shedding.active); + BOOST_CHECK(attempt()); +} + +BOOST_AUTO_TEST_CASE(pq_load_shedding_socket_thread) +{ + // The socket handler thread ends shedding on time when the flood stops entirely, without a + // further offer: the real thread function runs, as Start() runs it. + InitConnman(/*shed_threshold=*/2); + LogLineCounter stopped{"v2 pq: load_shedding stopped"}; + for (int i{0}; i < 3; ++i) { + Link link{Accept()}; + Receive(link, InitiatorKey()); + Pass(link); + link.node->RequestDisconnect(); + } + Disconnect(); // so the thread has no sockets to serve + BOOST_REQUIRE(Stats().load_shedding.active); + MockableSteadyClock::SetMockTime(STEADY_START + PQ_SHED_QUIET_PERIOD + 1s); + m_connman.StartSocketHandlerThread(); + const auto deadline{SteadyClock::now() + 10s}; + while (Stats().load_shedding.active && SteadyClock::now() < deadline) { + std::this_thread::sleep_for(10ms); + } + m_connman.StopSocketHandlerThread(); + BOOST_CHECK(!Stats().load_shedding.active); + BOOST_CHECK_EQUAL(stopped.m_count, 1); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/net.h b/src/test/util/net.h index 450098bca4..baffe3fdc9 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -107,6 +108,22 @@ struct ConnmanTestMsg : public CConnman { void SetPQMode(PQMode mode) { m_pq_mode = mode; } + void UpdatePQSheddingPublic() EXCLUSIVE_LOCKS_REQUIRED(!m_pq_shed_mutex) { UpdatePQShedding(PQShedNow()); } + + /** Run the socket handler thread, as Start() does. */ + void StartSocketHandlerThread() + { + threadSocketHandler = std::thread(&util::TraceThread, "net", [this] { ThreadSocketHandler(); }); + } + + /** Stop the socket handler thread, and let the connection manager run again. */ + void StopSocketHandlerThread() + { + interruptNet(); + if (threadSocketHandler.joinable()) threadSocketHandler.join(); + interruptNet.reset(); + } + void ObservePQPublic(CNode& node, bool finalizing) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex) { ObservePQ(node, finalizing); From 50881c9b6c37bdb3a31c17fcf39c16e2dedebe75 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Tue, 6 Oct 2026 12:37:46 -0400 Subject: [PATCH 05/12] test: pin the shed wipe, inbound-success and outbound-abandoned guards Three guards had no test, and each survived a mutation in the deep review of PR #235: - pq_shed_wipes_secrets: a shed responder wipes the retained ECDH secret and transcript at once (ClearHybridSecrets() in the shed branch), not only at finalization, since it runs plain v2 for the whole connection. - pq_inbound_success_does_not_protect: only outbound successes enter the success set, which is never evicted. An inbound peer's key is its address and source port, so an inbound success must neither protect an endpoint nor silence the local-fault warning. - pq_outbound_never_abandoned: only responders offer, so an outbound connection closed before the peer's version, by either side, counts no abandoned and adds no outbound ring entry. Follow-up to work items F2-F4 of #206 (#184 part 5). Co-Authored-By: Claude Opus 5.5 --- src/test/net_pq_tests.cpp | 64 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index 84e7237622..4c5270f76b 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -751,6 +751,30 @@ BOOST_AUTO_TEST_CASE(pq_abandoned) BOOST_CHECK_EQUAL(Stats().inbound.legacy_peer, 1U); } +BOOST_AUTO_TEST_CASE(pq_outbound_never_abandoned) +{ + // Only responders offer, so an outbound connection that closes before the peer's version is + // never abandoned, whoever closes it: the outbound count stays 0 and its ring stays empty. + for (const bool peer_closes : {true, false}) { + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::NEGOTIATE, TestEndpoint(10, peer_closes ? 1 : 2))}; + Pass(link); + Receive(link, InitiatorKey()); // any 64 bytes are a valid key, here the responder's + Pass(link); + BOOST_REQUIRE(!link.node->m_transport->GetPQSnapshot().version_received); + if (peer_closes) { + link.pipes->recv.Eof(); + } else { + link.node->RequestDisconnect(); + } + Pass(link); + BOOST_REQUIRE(link.node->fDisconnect); + Disconnect(); + } + const PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.outbound.abandoned, 0U); + BOOST_CHECK(stats.outbound_failures.entries.empty()); +} + BOOST_AUTO_TEST_CASE(pq_closed_after_switch) { // We switched, and the peer closed or stopped responding before its confirmation arrived. @@ -1394,6 +1418,27 @@ BOOST_AUTO_TEST_CASE(pq_local_fault_after_success) BOOST_CHECK_EQUAL(warnings.m_count, 0); } +BOOST_AUTO_TEST_CASE(pq_inbound_success_does_not_protect) +{ + // Only outbound successes enter the success set, which is never evicted. An inbound peer's + // key is its address and source port, so letting inbound successes in would let a peer that + // reconnects grow the set without bound, and silence the local-fault warning. + const CService addr{LookupNumeric("10.7.0.1", 8333)}; + Link inbound{Add(ConnectionType::INBOUND, PQMode::NEGOTIATE, std::nullopt, addr)}; + V2Transport peer{Initiator()}; + Exchange(inbound, peer); + BOOST_REQUIRE(inbound.node->m_transport->GetPQSnapshot().confirmed); + inbound.node->RequestDisconnect(); + Disconnect(); + LogLineCounter warnings{"v2 pq: local_fault"}; + // An outbound failure to the same address and port still starts a streak... + Connect(addr, End::MALFORMED); + BOOST_CHECK(Streak(addr)); + // ...and with no outbound success, eight failing endpoints still warn. + for (int i{1}; i < 8; ++i) Connect(TestEndpoint(9, i), End::MALFORMED); + BOOST_CHECK_EQUAL(warnings.m_count, 1); +} + BOOST_AUTO_TEST_CASE(pq_fallback_connection) { // A fallback connection to a peer that sends a malformed offer: plain v2, with the offer @@ -1543,6 +1588,25 @@ BOOST_AUTO_TEST_CASE(pq_load_shedding_off) BOOST_CHECK_EQUAL(Stats().inbound.shed, 0U); } +BOOST_AUTO_TEST_CASE(pq_shed_wipes_secrets) +{ + // A shed responder runs plain v2 for the rest of the connection, so it keeps no hybrid + // secret: the retained ECDH secret and transcript go at once, not at finalization. + InitConnman(/*shed_threshold=*/1); + Link offered{Accept()}; + Receive(offered, InitiatorKey()); + Pass(offered); + BOOST_REQUIRE(offered.node->m_transport->GetPQSnapshot().offer == PQOfferState::SENT); + BOOST_CHECK(dynamic_cast(*offered.node->m_transport).HoldsHybridSecretsForTesting()); + Link shed{Accept(LookupNumeric("10.9.0.2", 50001))}; + Receive(shed, InitiatorKey()); + Pass(shed); + BOOST_REQUIRE(Stats().load_shedding.active); + BOOST_REQUIRE(shed.node->m_transport->GetPQSnapshot().offer == PQOfferState::NONE); + BOOST_REQUIRE(!shed.node->fDisconnect); + BOOST_CHECK(!dynamic_cast(*shed.node->m_transport).HoldsHybridSecretsForTesting()); +} + BOOST_AUTO_TEST_CASE(pq_load_shedding_wall_clock_step) { // Offers trickle in, one per second, far below the threshold, while the wall clock steps back From 6aade8fa2beccb84ccf955a213e168151e8aa343 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Tue, 6 Oct 2026 12:39:36 -0400 Subject: [PATCH 06/12] test: restore mocked globals on every path in net_pq_tests Three tests changed process-wide state and restored it only at their end: pq_endpoint_identity, pq_name_proxy_endpoint and pq_fallback_windows replaced CreateSock, pq_name_proxy_endpoint also cleared the SOCKS5 interrupt, and pq_log_lines set fLogIPs. A failed BOOST_REQUIRE in between throws past the restore. Every later suite in the same test_qbit process would then connect through a mock socket or log differently, and fail far from the cause. A GlobalOverride guard, a MockCreateSock() helper built on it, and a Socks5InterruptCleared guard now restore them on every path, as ProxySettingsRestorerForTesting already does for the proxy settings. Co-Authored-By: Claude Opus 5.5 --- src/test/net_pq_tests.cpp | 62 +++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index 4c5270f76b..1e2bbd37d7 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -200,6 +201,44 @@ class LogLineCounter ~LogLineCounter() { LogInstance().DeleteCallback(m_callback); } }; +/** Sets a global while alive, and puts the old value back on every path: a failed BOOST_REQUIRE + * throws past the end of the test, and later suites in the same process must not see it. */ +template +class GlobalOverride +{ + T& m_global; + T m_previous; + +public: + GlobalOverride(T& global, T value) : m_global{global}, m_previous{std::exchange(global, std::move(value))} {} + ~GlobalOverride() { m_global = std::move(m_previous); } + GlobalOverride(const GlobalOverride&) = delete; + GlobalOverride& operator=(const GlobalOverride&) = delete; +}; + +/** A CreateSock that hands out sockets with contents, or ZeroSocks without. */ +GlobalOverride MockCreateSock(std::string contents = {}) +{ + if (contents.empty()) return {CreateSock, [](int, int, int) -> std::unique_ptr { return std::make_unique(); }}; + return {CreateSock, [contents](int, int, int) -> std::unique_ptr { return std::make_unique(contents); }}; +} + +/** Clears the SOCKS5 interrupt while alive, and sets it again on every path if it was set. Every + * CConnman::Interrupt() sets it, such as an earlier test's at teardown. */ +class Socks5InterruptCleared +{ + const bool m_was_set{g_socks5_interrupt}; + +public: + Socks5InterruptCleared() { g_socks5_interrupt.reset(); } + ~Socks5InterruptCleared() + { + if (m_was_set) g_socks5_interrupt(); + } + Socks5InterruptCleared(const Socks5InterruptCleared&) = delete; + Socks5InterruptCleared& operator=(const Socks5InterruptCleared&) = delete; +}; + /** Our outbound node's peer: a responder. */ V2Transport Responder(V2PQOptions pq = {.mode = PQMode::NEGOTIATE}) { return {/*nodeid=*/1000, /*initiating=*/false, pq}; } /** Our inbound node's peer: an initiator. */ @@ -474,9 +513,8 @@ BOOST_AUTO_TEST_CASE(pq_endpoint_identity) BOOST_CHECK(named.node->m_pq_endpoint == PQEndpointKey{(PQNameEndpoint{"seed.example", 18444})}); // Outbound construction captures the resolved destination and port. - const auto create_sock_orig{CreateSock}; - CreateSock = [](int, int, int) -> std::unique_ptr { return std::make_unique(); }; { + const auto create_sock{MockCreateSock()}; const std::unique_ptr outbound{m_connman.ConnectNodeOnly("5.6.7.8:18555", ConnectionType::MANUAL, /*use_v2transport=*/true)}; BOOST_REQUIRE(outbound); BOOST_CHECK(outbound->m_pq_endpoint == PQEndpointKey{LookupNumeric("5.6.7.8", 18555)}); @@ -484,7 +522,6 @@ BOOST_AUTO_TEST_CASE(pq_endpoint_identity) BOOST_REQUIRE(ipv6); BOOST_CHECK(ipv6->m_pq_endpoint == PQEndpointKey{LookupNumeric("2a01:4f8::2", 18555)}); } - CreateSock = create_sock_orig; // Inbound uses the actual remote address and source port. m_connman.CreateNodeFromAcceptedSocketPublic(std::make_unique(), LookupNumeric("127.0.0.1", 18444), LookupNumeric("9.8.7.6", 51234)); @@ -522,17 +559,12 @@ BOOST_AUTO_TEST_CASE(pq_name_proxy_endpoint) // the proxy for it, and the endpoint key is the normalized SOCKS destination and the // effective port, never the proxy's own address. const CService proxy{LookupNumeric("127.0.0.9", 9050)}; - const auto create_sock_orig{CreateSock}; - // Every CConnman's Interrupt(), such as an earlier test's at teardown, interrupts SOCKS5. - const bool socks5_interrupted{g_socks5_interrupt}; - g_socks5_interrupt.reset(); { + const Socks5InterruptCleared socks5_interrupt; ProxySettingsRestorerForTesting restore_proxies; BOOST_REQUIRE(SetNameProxy(Proxy{proxy})); // The proxy accepts without authentication, then reports the connection made. - CreateSock = [](int, int, int) -> std::unique_ptr { - return std::make_unique(std::string{"\x05\x00" "\x05\x00\x00\x01" "\x00\x00\x00\x00" "\x00\x00", 12}); - }; + const auto create_sock{MockCreateSock(std::string{"\x05\x00" "\x05\x00\x00\x01" "\x00\x00\x00\x00" "\x00\x00", 12})}; for (const auto& [dest, key] : {std::pair{"Seed.Example.COM.:18555", PQNameEndpoint{"seed.example.com", 18555}}, std::pair{"seed.example.com", PQNameEndpoint{"seed.example.com", Params().GetDefaultPort()}}, std::pair{"other.example:18555", PQNameEndpoint{"other.example", 18555}}}) { @@ -541,9 +573,7 @@ BOOST_AUTO_TEST_CASE(pq_name_proxy_endpoint) BOOST_CHECK(node->m_pq_endpoint == PQEndpointKey{key}); BOOST_CHECK(node->m_pq_endpoint != PQEndpointKey{proxy}); } - CreateSock = create_sock_orig; } - if (socks5_interrupted) g_socks5_interrupt(); // No later test sees the name proxy. BOOST_CHECK(!HaveNameProxy()); } @@ -1001,12 +1031,10 @@ BOOST_AUTO_TEST_CASE(pq_log_lines) { Link link{Add(ConnectionType::MANUAL)}; V2Transport peer{Responder()}; - const bool log_ips{fLogIPs}; - fLogIPs = true; + const GlobalOverride log_ips{fLogIPs, true}; const auto expect{ExpectLine(strprintf("v2 pq: switched role=initiator conn_type=manual peer=%d peeraddr=1.2.3.4:8333", link.node->GetId()), LineLevel::NET_DEBUG)}; Exchange(link, peer); - fLogIPs = log_ips; } Disconnect(); @@ -1247,8 +1275,7 @@ BOOST_AUTO_TEST_CASE(pq_fallback_threshold) BOOST_AUTO_TEST_CASE(pq_fallback_windows) { const PQEndpointKey endpoint{LookupNumeric("5.6.7.8", 18555)}; - const auto create_sock_orig{CreateSock}; - CreateSock = [](int, int, int) -> std::unique_ptr { return std::make_unique(); }; + const auto create_sock{MockCreateSock()}; // A new outbound connection to the endpoint, as the connection logic opens one. const auto new_status{[&] { const std::unique_ptr node{m_connman.ConnectNodeOnly("5.6.7.8:18555", ConnectionType::OUTBOUND_FULL_RELAY, /*use_v2transport=*/true)}; @@ -1288,7 +1315,6 @@ BOOST_AUTO_TEST_CASE(pq_fallback_windows) } // Existing sessions never renegotiate. BOOST_CHECK(running.node->m_transport->GetInfo().transport_pq_status == PQStatus::PENDING); - CreateSock = create_sock_orig; } BOOST_AUTO_TEST_CASE(pq_success_protection) From b066f9d8b6212ae38444fdd742140d15bc3cb16e Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Tue, 6 Oct 2026 12:39:56 -0400 Subject: [PATCH 07/12] doc: name load shedding in the off status of pq-transport.md A responder that sheds its offer under an inbound flood runs plain v2 and reports off (#206 F4), but the status list in doc/design/pq-transport.md named only the switch and local faults. Co-Authored-By: Claude Opus 5.5 --- doc/design/pq-transport.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/design/pq-transport.md b/doc/design/pq-transport.md index c78d76ae60..6953aad18b 100644 --- a/doc/design/pq-transport.md +++ b/doc/design/pq-transport.md @@ -309,8 +309,9 @@ qbit reports each v2 connection's negotiation status as one of these values: - `hybrid` once the peer's confirmation verifies; - `legacy_peer` when the peer's version packet carries no usable offer or accept, so the ECDH keys stay; -- `off` when the switch is off, or after a local key generation, key check or - encapsulation fault that left the connection on plain v2; +- `off` when the switch is off, when a responder sheds its offer under load, + or after a local key generation, key check or encapsulation fault that left + the connection on plain v2; - `fallback` for an outbound connection that runs plain v2 because of earlier failures; inbound connections never fall back. From 9eecea544662efccf645d25f627315cfb5dd2526 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Tue, 6 Oct 2026 13:04:34 -0400 Subject: [PATCH 08/12] test: check the shed wipe before the initiator's version arrives Part 4 now wipes the hybrid secrets whenever the peer's version is processed (#230, L5). pq_shed_wipes_secrets already checks right after the initiator's key, before any version, so a shed branch that doesn't wipe is still caught. With the merged part 4, a variant of the test that checks after the version lets that mutant survive. Make the precondition explicit, so the check can't drift past the version. Follow-up to work item F4 of #206 (#184 part 5). Co-Authored-By: Claude Opus 5.5 --- src/test/net_pq_tests.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index 1e2bbd37d7..ebd9e59018 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -1630,6 +1630,9 @@ BOOST_AUTO_TEST_CASE(pq_shed_wipes_secrets) BOOST_REQUIRE(Stats().load_shedding.active); BOOST_REQUIRE(shed.node->m_transport->GetPQSnapshot().offer == PQOfferState::NONE); BOOST_REQUIRE(!shed.node->fDisconnect); + // Check before the initiator's version arrives: processing it wipes whatever is still held, + // which would hide a shed branch that didn't wipe. + BOOST_REQUIRE(!shed.node->m_transport->GetPQSnapshot().version_received); BOOST_CHECK(!dynamic_cast(*shed.node->m_transport).HoldsHybridSecretsForTesting()); } From 9af8c209d43531e1a7e56dad8b9ef9b6c8739209 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Tue, 6 Oct 2026 13:06:01 -0400 Subject: [PATCH 09/12] test: pin the local-fault set's cap The distinct failing endpoints that the local-fault warning counts are never evicted, so the cap at PQ_LOCAL_FAULT_THRESHOLD is their only bound. No test saw it: the warning fires once either way, and an uncapped set survived mutation in the deep review of PR #235 (finding 5). ConnmanTestMsg::PQLocalFaultEndpoints() reads the set's size, test only. pq_local_fault_warning checks that the set holds exactly the threshold once the warning fires, and still does after 12 distinct endpoints have failed. Follow-up to work item F3 of #206 (#184 part 5). Co-Authored-By: Claude Opus 5.5 --- src/test/net_pq_tests.cpp | 4 ++++ src/test/util/net.h | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index ebd9e59018..73b344afb3 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -1414,11 +1414,15 @@ BOOST_AUTO_TEST_CASE(pq_local_fault_warning) Connect(TestEndpoint(7, 7), End::MALFORMED); } BOOST_CHECK_EQUAL(warnings.m_count, 1); + BOOST_CHECK_EQUAL(m_connman.PQLocalFaultEndpoints(), PQ_LOCAL_FAULT_THRESHOLD); // It warns once, and behavior doesn't change. for (int i{0}; i < 12; ++i) Connect(TestEndpoint(7, i), End::MALFORMED); BOOST_CHECK_EQUAL(warnings.m_count, 1); BOOST_CHECK(m_connman.IsPQFallback(TestEndpoint(7, 0), Now()) == false); BOOST_CHECK_EQUAL(Streak(TestEndpoint(7, 0))->streak, 2U); + // The set of failing endpoints stops growing at the threshold: it is never evicted, so the + // cap is its only bound. + BOOST_CHECK_EQUAL(m_connman.PQLocalFaultEndpoints(), PQ_LOCAL_FAULT_THRESHOLD); } BOOST_AUTO_TEST_CASE(pq_local_fault_warning_portable) diff --git a/src/test/util/net.h b/src/test/util/net.h index baffe3fdc9..7081638a9b 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -108,6 +108,12 @@ struct ConnmanTestMsg : public CConnman { void SetPQMode(PQMode mode) { m_pq_mode = mode; } + /** How many distinct failing endpoints the local-fault warning has counted. */ + size_t PQLocalFaultEndpoints() const EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex) + { + return WITH_LOCK(m_pq_mutex, return m_pq_failed_endpoints_for_warning.size()); + } + void UpdatePQSheddingPublic() EXCLUSIVE_LOCKS_REQUIRED(!m_pq_shed_mutex) { UpdatePQShedding(PQShedNow()); } /** Run the socket handler thread, as Start() does. */ From 4402db90bfe13c2e221202fcb26d5d6792c6d0e1 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Tue, 6 Oct 2026 13:53:09 -0400 Subject: [PATCH 10/12] test: build PQ test peers without a conditional of prvalues MSVC requires an accessible copy constructor for `cond ? a : b` with prvalue operands, and V2Transport has none, so test_bitcoin failed to build on the Windows CI job (C2280). Return the transport from a lambda instead. Co-Authored-By: Claude Opus 5.5 --- src/test/net_pq_tests.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index 73b344afb3..2d7ab12447 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -420,7 +420,9 @@ struct PQNetSetup : public RegTestingSetup { } case End::LEGACY: case End::SUCCESS: { - V2Transport peer{end == End::LEGACY ? Responder({}) : Responder()}; + // Not `cond ? a : b`: MSVC requires a copy constructor for a conditional of + // prvalues, and V2Transport has none. + V2Transport peer{[&] { if (end == End::LEGACY) return Responder({}); return Responder(); }()}; Exchange(link, peer); BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().confirmed == (end == End::SUCCESS)); link.node->RequestDisconnect(); @@ -687,8 +689,9 @@ BOOST_AUTO_TEST_CASE(pq_switched_then_failed) { for (const bool inbound : {false, true}) { Link link{Add(inbound ? ConnectionType::INBOUND : ConnectionType::OUTBOUND_FULL_RELAY)}; - V2Transport peer{inbound ? Initiator({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true}) : - Responder({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true})}; + // Not `cond ? a : b`: MSVC requires a copy constructor for a conditional of + // prvalues, and V2Transport has none. + V2Transport peer{[&] { if (inbound) return Initiator({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true}); return Responder({.mode = PQMode::NEGOTIATE, .corrupt_shared_secret = true}); }()}; Exchange(link, peer); const PQHandshake::Snapshot snapshot{link.node->m_transport->GetPQSnapshot()}; BOOST_CHECK(snapshot.switched); From 9d59cfb07b8f9e96a475ed4bc64467f1c4d669a2 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Tue, 6 Oct 2026 14:08:48 -0400 Subject: [PATCH 11/12] net: finalize PQ state of the connections deleted at shutdown Finalization ran only in DisconnectNodes(). StopNodes() swaps the nodes out of m_nodes and deletes them itself, so a node still held at shutdown was never marked finalized, and its pending decapsulation key and retained ECDH secret stayed until the transport's destructor (PR #235 review). StopNodes() now runs the same finalization, ObservePQ(finalizing), before deleting each node: it marks the node final and wipes the pending secrets. A shutdown close is our own decision, so like the network-off path it records nothing: no abandoned offer, ring entry or log line. ObservePQ() gains a record_outcomes flag for that; the nodes in the disconnected pool were already finalized by DisconnectNodes(). pq_shutdown_finalizes stops a connection manager that holds an inbound connection with its offer pending, an outbound one awaiting the version, and a hybrid one. At deletion no node is unfinalized or holds a hybrid secret, and no counter or ring changes. Follow-up to work item F2 of #206 (#184 part 5). Co-Authored-By: Claude Opus 5.5 --- src/net.cpp | 7 ++++-- src/net.h | 10 ++++---- src/test/net_pq_tests.cpp | 49 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index f85dd85206..64943a8a89 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2382,7 +2382,7 @@ PQCounts LoadPQCounts(const PQCounters& counters) noexcept } } // namespace -void CConnman::ObservePQ(CNode& node, bool finalizing) +void CConnman::ObservePQ(CNode& node, bool finalizing, bool record_outcomes) { AssertLockNotHeld(m_pq_mutex); // The transport takes and releases its own lock. @@ -2390,7 +2390,7 @@ void CConnman::ObservePQ(CNode& node, bool finalizing) // Nothing to account yet, as on every connection without the negotiation. if (!finalizing && !snapshot.switched && snapshot.legacy == PQLegacyReason::NONE && snapshot.failure == PQFailure::NONE) return; const NodeCloseCause cause{node.GetCloseCause()}; - const bool record{fNetworkActive}; + const bool record{record_outcomes && fNetworkActive}; PQLogLines lines; { LOCK(m_pq_mutex); @@ -4307,6 +4307,9 @@ void CConnman::StopNodes() for (CNode* pnode : nodes) { LogDebug(BCLog::NET, "Stopping node, %s", pnode->DisconnectMsg(fLogIPs)); pnode->CloseSocketDisconnect(); + // Wipe the hybrid negotiation's pending secrets before the node goes, as DisconnectNodes() + // does. A shutdown close is our own decision, so it records no outcome. + ObservePQ(*pnode, /*finalizing=*/true, /*record_outcomes=*/false); DeleteNode(pnode); } diff --git a/src/net.h b/src/net.h index b6c722a0af..4af94686e1 100644 --- a/src/net.h +++ b/src/net.h @@ -1460,8 +1460,8 @@ class CConnman bool Start(CScheduler& scheduler, const Options& options) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !m_added_nodes_mutex, !m_addr_fetches_mutex, !mutexMsgProc, !m_pq_shed_mutex); void StopThreads(); - void StopNodes(); - void Stop() + void StopNodes() EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); + void Stop() EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex) { StopThreads(); StopNodes(); @@ -2055,13 +2055,15 @@ class CConnman * hands the node to the disconnected pool: that derives closed_after_switch and abandoned * from the close cause, marks the node finalized so no later observation counts anything, * and wipes the transport's pending secrets. With the network off, every close is our own - * decision, so nothing is recorded, but finalization still wipes. + * decision, so nothing is recorded, but finalization still wipes. StopNodes() finalizes the + * connections it deletes at shutdown the same way, with record_outcomes false: shutdown + * closes are our own too. * * The transport is snapshotted under its own lock, which is released before m_pq_mutex is * taken; nothing calls into the transport, socket, peer manager or node enumeration while * holding m_pq_mutex. */ - void ObservePQ(CNode& node, bool finalizing) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); + void ObservePQ(CNode& node, bool finalizing, bool record_outcomes = true) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); /** Count each event of snapshot not yet accounted for node, once, add a ring entry for each * non-success outcome, and update the endpoint history. Returns what to log. */ diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index 2d7ab12447..116c40ebe6 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -49,8 +49,20 @@ namespace { class NoMessages final : public NetEventsInterface { public: + //! Nodes deleted, and of those, the ones deleted unfinalized or still holding hybrid secrets. + int m_deleted{0}; + int m_deleted_unfinalized{0}; + int m_deleted_with_secrets{0}; + void InitializeNode(const CNode&, ServiceFlags) override {} - void FinalizeNode(const CNode&) override {} + //! CConnman calls this right before it deletes the node. + void FinalizeNode(const CNode& node) override + { + ++m_deleted; + if (!node.m_pq_finalized) ++m_deleted_unfinalized; + const auto* transport{dynamic_cast(node.m_transport.get())}; + if (transport && transport->HoldsHybridSecretsForTesting()) ++m_deleted_with_secrets; + } bool HasAllDesirableServiceFlags(ServiceFlags) const override { return true; } bool HasUndesirableServiceFlags(ServiceFlags) const override { return false; } bool ProcessMessages(CNode*, std::atomic&) override EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) { return false; } @@ -1195,6 +1207,41 @@ BOOST_AUTO_TEST_CASE(pq_finalization_wipes) } } +BOOST_AUTO_TEST_CASE(pq_shutdown_finalizes) +{ + // Shutdown deletes the connections it still holds itself, without DisconnectNodes(). It + // finalizes them first: no node is deleted holding hybrid secrets, but a shutdown close is + // our own decision, so nothing is recorded, as with the network off. + Link inbound{Add(ConnectionType::INBOUND)}; // its offer is pending: decapsulation key and ECDH secret + Receive(inbound, InitiatorKey()); + Pass(inbound); + BOOST_REQUIRE(inbound.node->m_transport->GetPQSnapshot().offer == PQOfferState::SENT); + Link outbound{Add(ConnectionType::OUTBOUND_FULL_RELAY)}; // awaiting the version: the ECDH secret + Pass(outbound); + Receive(outbound, InitiatorKey()); + Pass(outbound); + Link hybrid{Add(ConnectionType::MANUAL)}; + V2Transport peer{Responder()}; + Exchange(hybrid, peer); + BOOST_REQUIRE(hybrid.node->m_transport->GetPQSnapshot().confirmed); + for (const Link& link : {inbound, outbound}) { + BOOST_REQUIRE(dynamic_cast(*link.node->m_transport).HoldsHybridSecretsForTesting()); + } + const PQTransportStats before{Stats()}; + const int deleted{m_events.m_deleted}; + m_connman.Stop(); + BOOST_CHECK_EQUAL(m_events.m_deleted - deleted, 3); + BOOST_CHECK_EQUAL(m_events.m_deleted_with_secrets, 0); + BOOST_CHECK_EQUAL(m_events.m_deleted_unfinalized, 0); + // No outcome: the pending offer isn't abandoned, and no ring entry is added. + const PQTransportStats after{Stats()}; + BOOST_CHECK_EQUAL(after.inbound.abandoned, before.inbound.abandoned); + BOOST_CHECK_EQUAL(after.inbound.switched, before.inbound.switched); + BOOST_CHECK_EQUAL(after.outbound.switched, before.outbound.switched); + BOOST_CHECK_EQUAL(after.inbound_failures.last_sequence, before.inbound_failures.last_sequence); + BOOST_CHECK_EQUAL(after.outbound_failures.last_sequence, before.outbound_failures.last_sequence); +} + BOOST_AUTO_TEST_CASE(pq_fallback_threshold) { using enum End; From 35524c44e5dbba465a93bc506c492411a673d599 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Tue, 6 Oct 2026 15:08:25 -0400 Subject: [PATCH 12/12] test: report would-block from DynSock where Windows reads it net_pq_tests failed on the Windows CI job: no handshake completed, offers were abandoned with reason reset instead of eof, and the rings stayed empty. The log shows the cause: "socket recv error, disconnecting peer=0: The operation completed successfully. (0)". DynSock::Pipe::GetBytes() signals an empty pipe like a non-blocking recv(2) with no data: it returns -1 and sets errno to EAGAIN. The socket handler reads the error with WSAGetLastError(). That is errno on POSIX, but on Windows it is Winsock's own last error, which the mock never set. So Windows saw error 0, treated every idle read as a permanent receive error, and closed the connection as PEER_RESET before the peer's next bytes arrived. Production is right: real sockets set the Winsock error, an orderly close still reads 0 (EOF), and a peer that closes with unread data resets the connection on Windows and Linux alike. GetBytes() now also calls WSASetLastError(WSAEWOULDBLOCK) on Windows. mock_socket_would_block pins it: an empty pipe reads -1 with the would-block error from WSAGetLastError(), EOF reads 0, and a socket handler pass over a connection with nothing to read leaves it open. Making the mock report error 0 on Linux reproduces the Windows failures exactly, and the new test catches it. Four checks dereferenced Streak() or Fallback() without requiring a value; on Windows one read an indeterminate streak. They now BOOST_REQUIRE it first. Co-Authored-By: Claude Opus 5.5 --- src/test/net_pq_tests.cpp | 30 ++++++++++++++++++++++++++++++ src/test/util/net.cpp | 7 ++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index 116c40ebe6..b28d0f2e3b 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -491,6 +491,32 @@ PQEndpointKey TestEndpoint(uint8_t subnet, int index) BOOST_FIXTURE_TEST_SUITE(net_pq_tests, PQNetSetup) +BOOST_AUTO_TEST_CASE(mock_socket_would_block) +{ + // These tests drive the socket handler through DynSock. An empty pipe must read like a + // non-blocking socket without data: -1, with the would-block error where the socket handler + // reads it, WSAGetLastError(). That is errno except on Windows, which keeps Winsock's error + // apart; with errno alone, Windows saw error 0 and closed every idle connection as a reset. + auto pipes{std::make_shared()}; + const DynSock sock{pipes, std::make_shared()}; + uint8_t buf[1]; + errno = 0; +#ifdef WIN32 + WSASetLastError(0); +#endif + BOOST_CHECK_EQUAL(sock.Recv(buf, sizeof(buf), 0), -1); + const int err{WSAGetLastError()}; + BOOST_CHECK(err == WSAEWOULDBLOCK || err == WSAEAGAIN); + pipes->recv.Eof(); + BOOST_CHECK_EQUAL(sock.Recv(buf, sizeof(buf), 0), 0); + + // So a socket handler pass over a connection with nothing to read keeps it open. + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY)}; + Pass(link); + BOOST_CHECK(!link.node->fDisconnect); + BOOST_CHECK(link.node->GetCloseCause() == NodeCloseCause::NONE); +} + BOOST_AUTO_TEST_CASE(pq_endpoint_identity) { // Name-proxy destinations: lowercase ASCII, one trailing root dot removed, the port kept. @@ -1301,6 +1327,7 @@ BOOST_AUTO_TEST_CASE(pq_fallback_threshold) Connect(endpoint, test.end); stats = Stats(); BOOST_CHECK_EQUAL(stats.outbound.fallback, fallbacks + 1); + BOOST_REQUIRE(Fallback(endpoint)); BOOST_CHECK(Fallback(endpoint)->expires == failure_time + 3600s); } @@ -1353,6 +1380,7 @@ BOOST_AUTO_TEST_CASE(pq_fallback_windows) SetMockTime(TicksSinceEpoch(start + window - 1s)); Connect(endpoint, End::MALFORMED); BOOST_CHECK(m_connman.IsPQFallback(endpoint, Now())); + BOOST_REQUIRE(Fallback(endpoint)); BOOST_CHECK(Fallback(endpoint)->expires == start + window); BOOST_CHECK_EQUAL(Stats().outbound.fallback, fallbacks + 1); @@ -1432,6 +1460,7 @@ BOOST_AUTO_TEST_CASE(pq_history_eviction) // insertion fails again, later than every other, and is still the next to go. SetMockTime(GetTime() + 1s); Connect(TestEndpoint(6, newest - 1), End::MALFORMED); + BOOST_REQUIRE(Streak(TestEndpoint(6, newest - 1))); BOOST_CHECK_EQUAL(Streak(TestEndpoint(6, newest - 1))->streak, 2U); Connect(TestEndpoint(6, newest + 1), End::MALFORMED); BOOST_CHECK(!Streak(TestEndpoint(6, newest - 1))); @@ -1469,6 +1498,7 @@ BOOST_AUTO_TEST_CASE(pq_local_fault_warning) for (int i{0}; i < 12; ++i) Connect(TestEndpoint(7, i), End::MALFORMED); BOOST_CHECK_EQUAL(warnings.m_count, 1); BOOST_CHECK(m_connman.IsPQFallback(TestEndpoint(7, 0), Now()) == false); + BOOST_REQUIRE(Streak(TestEndpoint(7, 0))); BOOST_CHECK_EQUAL(Streak(TestEndpoint(7, 0))->streak, 2U); // The set of failing endpoints stops growing at the threshold: it is never evicted, so the // cap is its only bound. diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index 73ae996b7a..589e0ac02b 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -261,7 +261,12 @@ ssize_t DynSock::Pipe::GetBytes(void* buf, size_t len, int flags) if (m_eof) { return 0; } - errno = EAGAIN; // Same as recv(2) on a non-blocking socket. + // Same as recv(2) on a non-blocking socket, reported where callers read it: + // WSAGetLastError() is errno except on Windows, where it is Winsock's own error. + errno = EAGAIN; +#ifdef WIN32 + WSASetLastError(WSAEWOULDBLOCK); +#endif return -1; }