From 2b3e19e9e854722b7bb04d11a579dc9df9435bda Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Fri, 28 Jul 2023 14:47:27 -0400 Subject: [PATCH] net: only allow 8 simultaneous forced inbound connections Github-Pull: #27600 Rebased-From: 75868022a904c1f77871abf962bf9b88a9c5faf6 --- src/net.cpp | 14 ++++++++++++++ src/net.h | 5 +++++ test/functional/p2p_eviction.py | 27 +++++++++++++++++---------- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 8b0581d65b..030a2c4b19 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1730,6 +1730,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, const CAddress& addr) { int nInbound = 0; + int nForced{0}; AddWhitelistPermissionFlags(permission_flags, addr, vWhitelistedRangeIncoming); @@ -1737,6 +1738,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, LOCK(m_nodes_mutex); for (const CNode* pnode : m_nodes) { if (pnode->IsInboundConn()) nInbound++; + if (pnode->m_forced_inbound) nForced++; } } @@ -1774,8 +1776,15 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, return; } + bool forced{false}; if (nInbound >= m_max_inbound) { + // Protect from force evicting everyone + if (nForced >= MAX_FORCED_INBOUND_CONNECTIONS) { + LogPrint(BCLog::NET, "connection from %s dropped (too many forced inbound)\n", addr.ToStringAddrPort()); + return; + } + // If the inbound connection attempt is granted ForceInbound permission, try a little harder // to make room by evicting a peer we may not have otherwise evicted. if (!AttemptToEvictConnection(NetPermissions::HasFlag(permission_flags, NetPermissionFlags::ForceInbound))) { @@ -1783,6 +1792,9 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, LogPrint(BCLog::NET, "failed to find an eviction candidate - connection dropped (full)\n"); return; } + + // We kicked someone out + forced = true; } NodeId id = GetNewNodeId(); @@ -1805,6 +1817,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, CNodeOptions{ .permission_flags = permission_flags, .prefer_evict = discouraged, + .forced_inbound = forced, .recv_flood_size = nReceiveFloodSize, .use_v2transport = use_v2transport, }); @@ -3749,6 +3762,7 @@ CNode::CNode(NodeId idIn, m_dest(addrNameIn), m_inbound_onion{inbound_onion}, m_prefer_evict{node_opts.prefer_evict}, + m_forced_inbound{node_opts.forced_inbound}, nKeyedNetGroup{nKeyedNetGroupIn}, m_conn_type{conn_type_in}, id{idIn}, diff --git a/src/net.h b/src/net.h index 786d6b7dc7..493d9a3530 100644 --- a/src/net.h +++ b/src/net.h @@ -85,6 +85,8 @@ static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60; static const int NUM_FDS_MESSAGE_CAPTURE = 1; /** Interval for ASMap Health Check **/ static constexpr std::chrono::hours ASMAP_HEALTH_CHECK_INTERVAL{24}; +/** Maximum number of forced inbound connections **/ +static const int MAX_FORCED_INBOUND_CONNECTIONS{8}; static constexpr bool DEFAULT_FORCEDNSSEED{false}; static constexpr bool DEFAULT_DNSSEED{true}; @@ -661,6 +663,8 @@ struct CNodeOptions NetPermissionFlags permission_flags = NetPermissionFlags::None; std::unique_ptr i2p_sam_session = nullptr; bool prefer_evict = false; + // True if ForceInbound connection required evicting a peer + bool forced_inbound{false}; size_t recv_flood_size{DEFAULT_MAXRECEIVEBUFFER * 1000}; bool use_v2transport = false; }; @@ -718,6 +722,7 @@ public: */ std::string cleanSubVer GUARDED_BY(m_subver_mutex){}; const bool m_prefer_evict{false}; // This peer is preferred for eviction. + const bool m_forced_inbound{false}; // This peer forced an inbound connection bool HasPermission(NetPermissionFlags permission) const { return NetPermissions::HasFlag(m_permission_flags, permission); } diff --git a/test/functional/p2p_eviction.py b/test/functional/p2p_eviction.py index 05298c7674..e41fbec16e 100755 --- a/test/functional/p2p_eviction.py +++ b/test/functional/p2p_eviction.py @@ -123,10 +123,11 @@ class P2PEvict(BitcoinTestFramework): assert evicted_peers[0] not in protected_peers self.log.info("Test that whitebind inbounds get extra eviction power") - # Only allow 1 inbound connection, but set whitebind - self.restart_node(0, extra_args=['-maxconnections=12', '-whitebind=127.0.0.1:30201', '-whitebind=forceinbound@127.0.0.1:30202']) - self.log.debug("Connect 1 peer") - node.add_p2p_connection(P2PInterface()) + # Allow 10 inbound connections, set whitebind and forceinbound + self.restart_node(0, extra_args=['-maxconnections=21', '-whitebind=127.0.0.1:30201', '-whitebind=forceinbound@127.0.0.1:30202']) + self.log.debug("Fill connections with unprivileged peers") + for i in range(10): + node.add_p2p_connection(P2PInterface()) # Create a peer that expects to be rejected # FIXME: "multiprocess, i686, DEBUG" CI task has a reliable timeout issue for v2transport @@ -134,6 +135,8 @@ class P2PEvict(BitcoinTestFramework): def connection_lost(self, exc): return + allowed_peers = [] + self.log.debug("Generic inbound gets rejected when full") with node.assert_debug_log(["failed to find an eviction candidate - connection dropped (full)"]): node.add_p2p_connection(RejectedPeer(), wait_for_verack=False, supports_v2_p2p=False) @@ -143,20 +146,24 @@ class P2PEvict(BitcoinTestFramework): node.add_p2p_connection(RejectedPeer(), wait_for_verack=False, supports_v2_p2p=False, dstport=30201) self.log.debug("ForceInbound whitebind inbound gets connected, even when full") - allowed_peer = node.add_p2p_connection(P2PInterface(), dstport=30202) + allowed_peers.append(node.add_p2p_connection(P2PInterface(), dstport=30202)) - assert_equal(len(node.getpeerinfo()), 1) + assert_equal(len(node.getpeerinfo()), 10) self.log.debug("Generic inbound gets rejected when whitebind peer is filling inbound slot") with node.assert_debug_log(["failed to find an eviction candidate - connection dropped (full)"]): node.add_p2p_connection(RejectedPeer(), supports_v2_p2p=False, wait_for_verack=False) - self.log.debug("ForceInbound whitebind inbound gets rejected when another whitebind peer is filling inbound slot") - with node.assert_debug_log(["failed to find an eviction candidate - connection dropped (full)"]): + self.log.debug("Fill force_inbound slots") + for i in range(7): + allowed_peers.append(node.add_p2p_connection(P2PInterface(), dstport=30202)) + + self.log.debug("ForceInbound gets rejected after 8 evictions") + with node.assert_debug_log([f"dropped (too many forced inbound)"]): node.add_p2p_connection(RejectedPeer(), supports_v2_p2p=False, dstport=30202, wait_for_verack=False) - assert_equal(len(node.getpeerinfo()), 1) - assert allowed_peer.is_connected + assert_equal(len(node.getpeerinfo()), 10) + assert [peer.is_connected for peer in allowed_peers] if __name__ == '__main__': P2PEvict(__file__).main()