From 654d9bc27647fb3797001472e2464dededb45d3f Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Sun, 11 Jun 2023 12:26:18 -0700 Subject: [PATCH 1/4] p2p: Introduce data struct to track connection counts by network Connman uses this new map to keep a count of active OUTBOUND_FULL_RELAY and MANUAL connections. Unused until next commit. Co-authored-by: Martin Zumsande --- src/net.cpp | 6 ++++++ src/net.h | 19 +++++++++++++++++++ src/test/util/net.h | 3 +++ 3 files changed, 28 insertions(+) diff --git a/src/net.cpp b/src/net.cpp index 282c8fb7414..bfbca4baf8c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1135,6 +1135,9 @@ void CConnman::DisconnectNodes() // close socket and cleanup pnode->CloseSocketDisconnect(); + // 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); @@ -2035,6 +2038,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()]; } } diff --git a/src/net.h b/src/net.h index 83fe0427d43..edff55ce7a3 100644 --- a/src/net.h +++ b/src/net.h @@ -465,6 +465,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; } @@ -1048,6 +1064,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) = {}; + /** * Cache responses to addr requests to minimize privacy leak. * Attack example: scraping addrs in real-time may allow an attacker diff --git a/src/test/util/net.h b/src/test/util/net.h index e6506b0d08e..b2f6ebb1637 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); From 034f61f83b9348664d868933dbbfd8f9f8882168 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 7 Feb 2023 16:03:32 -0500 Subject: [PATCH 2/4] p2p: Protect extra full outbound peers by network If a peer is the only one of its network, protect it from eviction. This improves the diversity of outbound connections with respect to reachable networks. Co-authored-by: Amiti Uttarwar --- src/net.cpp | 6 ++++++ src/net.h | 5 +++++ src/net_processing.cpp | 7 ++++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index bfbca4baf8c..00ee873c697 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1607,6 +1607,12 @@ 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; +} + void CConnman::ThreadOpenConnections(const std::vector connect) { AssertLockNotHeld(m_unused_i2p_sessions_mutex); diff --git a/src/net.h b/src/net.h index edff55ce7a3..a4c810b0ae2 100644 --- a/src/net.h +++ b/src/net.h @@ -793,6 +793,9 @@ public: void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* strDest, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); bool CheckIncomingNonce(uint64_t nonce); + // alias for thread safety annotations only, not defined + RecursiveMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); + bool ForNode(NodeId id, std::function func); void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); @@ -909,6 +912,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); + private: struct ListenSocket { public: diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6597019797c..624d1b0ccbc 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5146,10 +5146,12 @@ 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(); - 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); // Only consider outbound-full-relay peers that are not already @@ -5159,6 +5161,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; From 65cff00ceea48ac8a887ffea79aedb4251aa097f Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 8 Feb 2023 17:42:12 -0500 Subject: [PATCH 3/4] test: Add test for outbound protection by network Co-authored-by: Amiti Uttarwar --- src/test/denialofservice_tests.cpp | 38 ++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 1a06f16155c..2873d35eb03 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -106,9 +106,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, @@ -197,6 +207,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); } From 1b52d16d07be3b5d968157913f04d9cd1e2d3678 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 14 Feb 2023 17:40:14 -0500 Subject: [PATCH 4/4] p2p: network-specific management of outbound connections Diversify outbound connections with respect to networks: Every ~5 minutes, try to add an extra connection to a reachable network which we currently don't have a connection to. This is done defensively - only try management with respect to networks after all existing outbound slots are filled. The resulting situation with an extra outbound peer will be handled by the extra outbound eviction logic, which protects peers from eviction if they are the only ones for their network. Co-authored-by: Amiti Uttarwar --- src/net.cpp | 40 +++++++++++++++++++++++++++++++++++++++- src/net.h | 12 ++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index 00ee873c697..228d081aa60 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -90,6 +90,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, @@ -1613,6 +1616,22 @@ bool CConnman::MultipleManualOrFullOutboundConns(Network net) const 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) { AssertLockNotHeld(m_unused_i2p_sessions_mutex); @@ -1644,6 +1663,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) // 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); @@ -1755,6 +1775,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) auto now = GetTime(); bool anchor = false; bool fFeeler = false; + std::optional preferred_net; // Determine what type of connection to open. Opening // BLOCK_RELAY connections to addresses from anchors.dat gets the highest @@ -1804,6 +1825,17 @@ void CConnman::ThreadOpenConnections(const std::vector connect) 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 { // skip to next iteration of while loop continue; @@ -1857,7 +1889,10 @@ void CConnman::ThreadOpenConnections(const std::vector connect) } } 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); } // Require outbound IPv4/IPv6 connections, other than feelers, to be to distinct network groups @@ -1904,6 +1939,9 @@ void CConnman::ThreadOpenConnections(const std::vector connect) } LogPrint(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToStringAddrPort()); } + + 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 diff --git a/src/net.h b/src/net.h index a4c810b0ae2..39829f7f5bc 100644 --- a/src/net.h +++ b/src/net.h @@ -1031,6 +1031,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);