From 3faae99c3d8e512f9d3f6e7fb0785c60d4bed654 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Tue, 14 Sep 2021 14:54:40 +0300 Subject: [PATCH 1/2] p2p: Diversify connections only w.r.t *persistent* outbound peers ADDR_FETCH and FEELER are short-lived connections, and they should not affect our choice of peers. Also, improve comments. --- src/net.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index d1895b7172..e19decbda8 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1721,19 +1721,22 @@ void CConnman::ThreadOpenConnections(const std::vector connect) if (pnode->IsFullOutboundConn()) nOutboundFullRelay++; if (pnode->IsBlockOnlyConn()) nOutboundBlockRelay++; - // Netgroups for inbound and manual peers are not excluded because our goal here - // is to not use multiple of our limited outbound slots on a single netgroup - // but inbound and manual peers do not use our outbound slots. Inbound peers - // also have the added issue that they could be attacker controlled and used - // to prevent us from connecting to particular hosts if we used them here. + // Make sure our persistent outbound slots belong to different netgroups. switch (pnode->m_conn_type) { + // We currently don't take inbound connections into account. Since they are + // free to make, an attacker could make them to prevent us from connecting to + // certain peers. case ConnectionType::INBOUND: + // Manually selected connections should not affect how we select outbound + // peers from addrman. case ConnectionType::MANUAL: + // Short-lived outbound connections should not affect how we select outbound + // peers from addrman. + case ConnectionType::ADDR_FETCH: + case ConnectionType::FEELER: break; case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: - case ConnectionType::ADDR_FETCH: - case ConnectionType::FEELER: setConnected.insert(m_netgroupman.GetGroup(pnode->addr)); } // no default case, so the compiler can warn about missing cases } From 72e8ffd7f8dbf908e65da6d012ede914596737ab Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Tue, 14 Sep 2021 15:19:41 +0300 Subject: [PATCH 2/2] p2p: Account for MANUAL conns when diversifying persistent outbound conns Previously, we would make connections to peer from the netgroups to which our MANUAL outbound connections belong. However, they should be seen as regular connections from Addrman when it comes to netgroup diversity check, since the same rationale can be applied. Note, this has nothing to do with how we connect to MANUAL connections: we connect to them unconditionally. --- src/net.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index e19decbda8..9b803180f9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1727,14 +1727,12 @@ void CConnman::ThreadOpenConnections(const std::vector connect) // free to make, an attacker could make them to prevent us from connecting to // certain peers. case ConnectionType::INBOUND: - // Manually selected connections should not affect how we select outbound - // peers from addrman. - case ConnectionType::MANUAL: // Short-lived outbound connections should not affect how we select outbound // peers from addrman. case ConnectionType::ADDR_FETCH: case ConnectionType::FEELER: break; + case ConnectionType::MANUAL: case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: setConnected.insert(m_netgroupman.GetGroup(pnode->addr));