From 5dc52b3b6f749c4b76c3bb985234a9f0668cb9ca Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 11 Jun 2023 12:26:18 -0700 Subject: [PATCH] merge bitcoin#27213: Diversify automatic outbound connections with respect to networks --- src/net.cpp | 52 +++++++++++++++++++++++++++++- src/net.h | 36 +++++++++++++++++++++ src/net_processing.cpp | 8 ++++- src/test/denialofservice_tests.cpp | 38 ++++++++++++++++++++-- src/test/util/net.h | 3 ++ 5 files changed, 133 insertions(+), 4 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 351c114d60..e4f0d26b27 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -107,6 +107,9 @@ static constexpr std::chrono::seconds MAX_UPLOAD_TIMEFRAME{60 * 60 * 24}; // A random time period (0 to 1 seconds) is added to feeler connections to prevent synchronization. static constexpr auto FEELER_SLEEP_WINDOW{1s}; +/** Frequency to attempt extra connections to reachable networks we're not connected to yet **/ +static constexpr auto EXTRA_NETWORK_PEER_INTERVAL{5min}; + /** Used to pass flags to the Bind() function */ enum BindFlags { BF_NONE = 0, @@ -2252,6 +2255,9 @@ void CConnman::DisconnectNodes() // close socket and cleanup pnode->CloseSocketDisconnect(this); + // update connection count by network + if (pnode->IsManualOrFullOutboundConn()) --m_network_conn_counts[pnode->addr.GetNetwork()]; + // hold in disconnected pool until all refs are released pnode->Release(); m_nodes_disconnected.push_back(pnode); @@ -3178,6 +3184,28 @@ std::unordered_set CConnman::GetReachableEmptyNetworks() const return networks; } +bool CConnman::MultipleManualOrFullOutboundConns(Network net) const +{ + AssertLockHeld(m_nodes_mutex); + return m_network_conn_counts[net] > 1; +} + +bool CConnman::MaybePickPreferredNetwork(std::optional& network) +{ + std::array nets{NET_IPV4, NET_IPV6, NET_ONION, NET_I2P, NET_CJDNS}; + Shuffle(nets.begin(), nets.end(), FastRandomContext()); + + LOCK(m_nodes_mutex); + for (const auto net : nets) { + if (IsReachable(net) && m_network_conn_counts[net] == 0 && addrman.Size(net) != 0) { + network = net; + return true; + } + } + + return false; +} + void CConnman::ThreadOpenConnections(const std::vector connect, CDeterministicMNManager& dmnman) { AssertLockNotHeld(m_unused_i2p_sessions_mutex); @@ -3213,6 +3241,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe // Minimum time before next feeler connection (in microseconds). auto next_feeler = GetExponentialRand(start, FEELER_INTERVAL); auto next_extra_block_relay = GetExponentialRand(start, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL); + auto next_extra_network_peer{GetExponentialRand(start, EXTRA_NETWORK_PEER_INTERVAL)}; const bool dnsseed = gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED); bool add_fixed_seeds = gArgs.GetBoolArg("-fixedseeds", DEFAULT_FIXEDSEEDS); const bool use_seednodes{gArgs.IsArgSet("-seednode")}; @@ -3341,6 +3370,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe auto now = GetTime(); bool anchor = false; bool fFeeler = false; + std::optional preferred_net; bool onion_only = false; // Determine what type of connection to open. Opening @@ -3391,6 +3421,17 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe next_feeler = GetExponentialRand(now, FEELER_INTERVAL); conn_type = ConnectionType::FEELER; fFeeler = true; + } else if (nOutboundFullRelay == m_max_outbound_full_relay && + m_max_outbound_full_relay == MAX_OUTBOUND_FULL_RELAY_CONNECTIONS && + now > next_extra_network_peer && + MaybePickPreferredNetwork(preferred_net)) { + // Full outbound connection management: Attempt to get at least one + // outbound peer from each reachable network by making extra connections + // and then protecting "only" peers from a network during outbound eviction. + // This is not attempted if the user changed -maxconnections to a value + // so low that less than MAX_OUTBOUND_FULL_RELAY_CONNECTIONS are made, + // to prevent interactions with otherwise protected outbound peers. + next_extra_network_peer = GetExponentialRand(now, EXTRA_NETWORK_PEER_INTERVAL); } else if (nOutboundOnionRelay < m_max_outbound_onion && IsReachable(Network::NET_ONION)) { onion_only = true; } else { @@ -3448,7 +3489,10 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe } } else { // Not a feeler - std::tie(addr, addr_last_try) = addrman.Select(); + // If preferred_net has a value set, pick an extra outbound + // peer from that network. The eviction logic in net_processing + // ensures that a peer from another network will be evicted. + std::tie(addr, addr_last_try) = addrman.Select(false, preferred_net); } auto dmn = mnList.GetMNByService(addr); @@ -3515,6 +3559,9 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe LogPrint(BCLog::NET, "Making feeler connection\n"); } } + + if (preferred_net != std::nullopt) LogPrint(BCLog::NET, "Making network specific connection to %s on %s.\n", addrConnect.ToStringAddrPort(), GetNetworkName(preferred_net.value())); + // Record addrman failure attempts when node has at least 2 persistent outbound connections to peers with // different netgroups in ipv4/ipv6 networks + all peers in Tor/I2P/CJDNS networks. // Don't record addrman failure attempts when node is offline. This can be identified since all local @@ -3877,6 +3924,9 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai { LOCK(m_nodes_mutex); m_nodes.push_back(pnode); + + // update connection count by network + if (pnode->IsManualOrFullOutboundConn()) ++m_network_conn_counts[pnode->addr.GetNetwork()]; } { if (m_edge_trig_events) { diff --git a/src/net.h b/src/net.h index 8a36625827..dfad5aa462 100644 --- a/src/net.h +++ b/src/net.h @@ -889,6 +889,22 @@ public: return m_conn_type == ConnectionType::MANUAL; } + bool IsManualOrFullOutboundConn() const + { + switch (m_conn_type) { + case ConnectionType::INBOUND: + case ConnectionType::FEELER: + case ConnectionType::BLOCK_RELAY: + case ConnectionType::ADDR_FETCH: + return false; + case ConnectionType::OUTBOUND_FULL_RELAY: + case ConnectionType::MANUAL: + return true; + } // no default case, so the compiler can warn about missing cases + + assert(false); + } + bool IsBlockOnlyConn() const { return m_conn_type == ConnectionType::BLOCK_RELAY; } @@ -1281,6 +1297,9 @@ public: EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc); bool CheckIncomingNonce(uint64_t nonce); + // alias for thread safety annotations only, not defined + RecursiveMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); + struct CFullyConnectedOnly { bool operator() (const CNode* pnode) const { return NodeFullyConnected(pnode); @@ -1534,6 +1553,8 @@ public: /** Return true if we should disconnect the peer for failing an inactivity check. */ bool ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const; + bool MultipleManualOrFullOutboundConns(Network net) const EXCLUSIVE_LOCKS_REQUIRED(m_nodes_mutex); + /** * RAII helper to atomically create a copy of `m_nodes` and add a reference * to each of the nodes. The nodes are released when this object is destroyed. @@ -1734,6 +1755,18 @@ private: */ std::vector GetCurrentBlockRelayOnlyConns() const; + /** + * Search for a "preferred" network, a reachable network to which we + * currently don't have any OUTBOUND_FULL_RELAY or MANUAL connections. + * There needs to be at least one address in AddrMan for a preferred + * network to be picked. + * + * @param[out] network Preferred network, if found. + * + * @return bool Whether a preferred network was found. + */ + bool MaybePickPreferredNetwork(std::optional& network); + // Whether the node should be passed out in ForEach* callbacks static bool NodeFullyConnected(const CNode* pnode); @@ -1776,6 +1809,9 @@ private: std::atomic nLastNodeId{0}; unsigned int nPrevNodeCount{0}; + // Stores number of full-tx connections (outbound and manual) per network + std::array m_network_conn_counts GUARDED_BY(m_nodes_mutex) = {}; + std::vector vPendingMasternodes; mutable RecursiveMutex cs_vPendingMasternodes; std::map, std::set> masternodeQuorumNodes GUARDED_BY(cs_vPendingMasternodes); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3c8cef6a28..547f516b78 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5311,13 +5311,16 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) // Pick the outbound-full-relay peer that least recently announced // us a new block, with ties broken by choosing the more recent // connection (higher node id) + // Protect peers from eviction if we don't have another connection + // to their network, counting both outbound-full-relay and manual peers. NodeId worst_peer = -1; int64_t oldest_block_announcement = std::numeric_limits::max(); // We want to prevent recently connected to Onion peers from being disconnected here, protect them as long as // there are more non_onion nodes than onion nodes so far size_t onion_count = 0; - m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + + m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_connman.GetNodesMutex()) { AssertLockHeld(::cs_main); if (pnode->addr.IsTor() && ++onion_count <= m_connman.GetMaxOutboundOnionNodeCount()) return; // Don't disconnect masternodes just because they were slow in block announcement @@ -5329,6 +5332,9 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) if (state == nullptr) return; // shouldn't be possible, but just in case // Don't evict our protected peers if (state->m_chain_sync.m_protect) return; + // If this is the only connection on a particular network that is + // OUTBOUND_FULL_RELAY or MANUAL, protect it. + if (!m_connman.MultipleManualOrFullOutboundConns(pnode->addr.GetNetwork())) return; if (state->m_last_block_announcement < oldest_block_announcement || (state->m_last_block_announcement == oldest_block_announcement && pnode->GetId() > worst_peer)) { worst_peer = pnode->GetId(); oldest_block_announcement = state->m_last_block_announcement; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 9cebbf5b8c..def090aa63 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -111,9 +111,19 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) peerman.FinalizeNode(dummyNode1); } -static void AddRandomOutboundPeer(NodeId& id, std::vector& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType) +static void AddRandomOutboundPeer(NodeId& id, std::vector& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType, bool onion_peer = false) { - CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); + CAddress addr; + + if (onion_peer) { + auto tor_addr{g_insecure_rand_ctx.randbytes(ADDR_TORV3_SIZE)}; + BOOST_REQUIRE(addr.SetSpecial(OnionToString(tor_addr))); + } + + while (!addr.IsRoutable()) { + addr = CAddress(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); + } + vNodes.emplace_back(new CNode{id++, /*sock=*/nullptr, addr, @@ -205,6 +215,30 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_CHECK(vNodes[max_outbound_full_relay-1]->fDisconnect == true); BOOST_CHECK(vNodes.back()->fDisconnect == false); + vNodes[max_outbound_full_relay - 1]->fDisconnect = false; + + // Add an onion peer, that will be protected because it is the only one for + // its network, so another peer gets disconnected instead. + SetMockTime(time_init); + AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY, /*onion_peer=*/true); + SetMockTime(time_later); + peerLogic->CheckForStaleTipAndEvictPeers(); + + for (int i = 0; i < max_outbound_full_relay - 2; ++i) { + BOOST_CHECK(vNodes[i]->fDisconnect == false); + } + BOOST_CHECK(vNodes[max_outbound_full_relay - 2]->fDisconnect == false); + BOOST_CHECK(vNodes[max_outbound_full_relay - 1]->fDisconnect == true); + BOOST_CHECK(vNodes[max_outbound_full_relay]->fDisconnect == false); + + // Add a second onion peer which won't be protected + SetMockTime(time_init); + AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY, /*onion_peer=*/true); + SetMockTime(time_later); + peerLogic->CheckForStaleTipAndEvictPeers(); + + BOOST_CHECK(vNodes.back()->fDisconnect == true); + for (const CNode *node : vNodes) { peerLogic->FinalizeNode(*node); } diff --git a/src/test/util/net.h b/src/test/util/net.h index 933309b698..ef2f3df2c1 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -29,7 +29,10 @@ struct ConnmanTestMsg : public CConnman { { LOCK(m_nodes_mutex); m_nodes.push_back(&node); + + if (node.IsManualOrFullOutboundConn()) ++m_network_conn_counts[node.addr.GetNetwork()]; } + void ClearTestNodes() { LOCK(m_nodes_mutex);