foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#27374: p2p: skip netgroup diversity of new connections for tor/i2p/cjdns

Browse source 
b5585ba5f9 p2p: skip netgroup diversity of new connections for tor/i2p/cjdns networks (stratospher)

Pull request description:

  Follow up for #27264.

  In order to make sure that our persistent outbound slots belong to different netgroups, distinct net groups of our peers are added to `setConnected`. We’d only open a persistent outbound connection to peers which have a different netgroup compared to those netgroups present in `setConnected`.

  Current `GetGroup()` logic assumes route-based diversification behaviour for tor/i2p/cjdns addresses (addresses are public key based and not route-based). Distinct netgroups possible (according to the current `GetGroup()` logic) for:
  1. tor => 030f, 031f, .. 03ff (16 possibilities)
  2. i2p => 040f, 041f, .. 04ff (16 possibilities)
  3. cjdns => 05fc0f, 05fc1f, ... 05fcff (16 possibilities)

  `setConnected` is used in `ThreadOpenConnections()` before making [outbound](84f4ac39fd/src/net.cpp (L1846)) and [anchor](84f4ac39fd/src/net.cpp (L1805)) connections to new peers so that they belong to distinct netgroups.

  **behaviour on master**

  - if we run a node only on tor/i2p/cjdns
  - we wouldn't be able to open more than 16 outbound connections(manual, block-relay-only anchor, outbound full relay, block-relay-only connections) because we run out of possible netgroups.
  - see https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1481322628
  - tested by changing `MAX_OUTBOUND_FULL_RELAY_CONNECTIONS` to 17 with `onlynet=onion` and observed how node wouldn't make more than 16 outbound connections.

  **behaviour on PR**

  - netgroup diversity checks are skipped for tor/i2p/cjdns addresses.
  - we don't insert tor/i2p/cjdns address in `setConnected` and `GetGroup` doesn't get called on tor/i2p/cjdns(see #27369)

ACKs for top commit:
  achow101:
    ACK b5585ba5f9
  mzumsande:
    ACK b5585ba5f9
  vasild:
    ACK b5585ba5f9

Tree-SHA512: c120b3f9ca7f0be3f29ea665cd2f7dfb40cd1d7ec7058984252fb6e0295e414f736c5b4fba03c31188188a5ae4f543fb2654f6ee9776bad745c7ca72d23d5b9b
This commit is contained in:
Andrew Chow 2023-04-13 18:14:12 -04:00
commit 2bfe43db16
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
1 changed files with 22 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
src/net.cpp
View file

@ -1703,10 +1703,11 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
//
CAddress addrConnect;
// Only connect out to one peer per network group (/16 for IPv4).
// Only connect out to one peer per ipv4/ipv6 network group (/16 for IPv4).
int nOutboundFullRelay = 0;
int nOutboundBlockRelay = 0;
std::set<std::vector<unsigned char> > setConnected;
int outbound_privacy_network_peers = 0;
std::set<std::vector<unsigned char>> setConnected; // netgroups of our ipv4/ipv6 outbound peers
{
LOCK(m_nodes_mutex);
@ -1714,7 +1715,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
if (pnode->IsFullOutboundConn()) nOutboundFullRelay++;
if (pnode->IsBlockOnlyConn()) nOutboundBlockRelay++;
// Make sure our persistent outbound slots belong to different netgroups.
// Make sure our persistent outbound slots to ipv4/ipv6 peers 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
@ -1728,7 +1729,19 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
case ConnectionType::MANUAL:
case ConnectionType::OUTBOUND_FULL_RELAY:
case ConnectionType::BLOCK_RELAY:
setConnected.insert(m_netgroupman.GetGroup(pnode->addr));
CAddress address{pnode->addr};
if (address.IsTor() || address.IsI2P() || address.IsCJDNS()) {
// Since our addrman-groups for these networks are
// random, without relation to the route we
// take to connect to these peers or to the
// difficulty in obtaining addresses with diverse
// groups, we don't worry about diversity with
// respect to our addrman groups when connecting to
// these networks.
++outbound_privacy_network_peers;
} else {
setConnected.insert(m_netgroupman.GetGroup(address));
}
} // no default case, so the compiler can warn about missing cases
}
}
@ -1886,8 +1899,11 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
}
LogPrint(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToStringAddrPort());
}
OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, conn_type);
// 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
// network connections(if any) belong in the same netgroup and size of setConnected would only be 1.
OpenNetworkConnection(addrConnect, (int)setConnected.size() + outbound_privacy_network_peers >= std::min(nMaxConnections - 1, 2), &grant, nullptr, conn_type);
}
}
}