HomePhabricator

Make AddrMan support multiple ports per IP

Description

Make AddrMan support multiple ports per IP

Summary:

For a long part of Bitcoin's history, this codebase has aggressively avoided making automatic connections to anything but nodes running on port 8333. I'd like to propose changing that, and this is a first PR necessary for that.

The folklore justification (eventually actually added as a comment to the codebase in #20668) is that this is to prevent the Bitcoin P2P network from being leveraged to perform a DoS attack on other services, if their IP/port would get rumoured. It appears, at least the current network scale - and probably significantly larger - that the impact is very low at best (see calculations by vasild in #5150 (comment) e.g.). Another possible justification would be a risk that treating different IP:port combinations separately would help perform Eclipse attacks (by an attacker rumouring their own IP with many ports). This concern is (a) no different than what is possible with IPv6 (where large ranges of IP addresses are very cheaply available), and (b) already hopefully sufficiently addressed by addrman's design (which limits access through based selected based on network groups).

And this policy has downsides too; in particular, a fixed port is easy to detect, and a very obvious sign a Bitcoin node is running there.

One obstacle in moving away from a default port that is the fact that addrman is currently restricted to a single entry per IP address. If ports are no longer expected to be generally always the default one, we need to deal with the case where conflicting information is relayed. It turns out there is a very natural solution to this: treat (IP,port) combination exactly as we're treating IPs now; this automatically means that the same IP may appear with multiple ports, simply because those would be distinct entries. Given that indexing into addrman's bucket already uses the port number, the only change required is making all addrman lookup be (IP,port) (aka CService) based, rather than IP (aka CNetAddr) based.

This PR doesn't include any change to the actual outbound connection preference logic, as perhaps that's something that we want to phase in more gradually.

Backport of core#23306 and core#23354 .

Depends on D12340.

Test Plan:

ninja all check-all

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Differential Revision: https://reviews.bitcoinabc.org/D12342

Details

Provenance
Pieter Wuille <pieter@wuille.net>Authored on Oct 5 2021, 20:13
FabienCommitted on Oct 21 2022, 08:34
FabienPushed on Oct 21 2022, 08:34
Reviewer
Restricted Project
Differential Revision
D12342: Make AddrMan support multiple ports per IP
Parents
rABC55e57736b1e7: Make CAddrman::Select_ select buckets, not positions, first
Branches
Unknown
Tags
Unknown