Skip to content

Commit ba11eb3

Browse files
laanwjlaanwj
committed
Merge bitcoin#23542: net: open p2p connections to nodes that listen on non-default ports
36ee76d net: remove unused CNetAddr::GetHash() (Vasil Dimov) d0abce9 net: include the port when deciding a relay destination (Vasil Dimov) 2e38a0e net: add CServiceHash constructor so the caller can provide the salts (Vasil Dimov) 9720863 net: open p2p connections to nodes that listen on non-default ports (Vasil Dimov) Pull request description: By default, for mainnet, the p2p listening port is 8333. Bitcoin Core has a strong preference for only connecting to nodes that listen on that port. Remove that preference because connections over clearnet that involve port 8333 make it easy to detect, analyze, block or divert Bitcoin p2p traffic before the connection is even established (at TCP SYN time). For further justification see the OP of: bitcoin#23306 ACKs for top commit: laanwj: Concept and light code review ACK 36ee76d prayank23: ACK bitcoin@36ee76d stickies-v: tACK 36ee76d jonatack: ACK 36ee76d glozow: utACK 36ee76d Tree-SHA512: 7f45ab7567c51c19fc50fabbaf84f0cc8883a8eef84272b76435c014c31d89144271d70dd387212cc1114213165d76b4d20a5ddb8dbc958fe7e74e6ddbd56d11
2 parents 848b116 + 36ee76d commit ba11eb3

File tree

11 files changed

+274
-20
lines changed

11 files changed

+274
-20
lines changed

‎doc/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ The Bitcoin repo's [root README](/README.md) contains relevant information on th
7979
- [Init Scripts (systemd/upstart/openrc)](init.md)
8080
- [Managing Wallets](managing-wallets.md)
8181
- [Multisig Tutorial](multisig-tutorial.md)
82+
- [P2P bad ports definition and list](p2p-bad-ports.md)
8283
- [PSBT support](psbt.md)
8384
- [Reduce Memory](reduce-memory.md)
8485
- [Reduce Traffic](reduce-traffic.md)

‎doc/p2p-bad-ports.md‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
When Bitcoin Core automatically opens outgoing P2P connections it chooses
2+
a peer (address and port) from its list of potential peers. This list is
3+
populated with unchecked data, gossiped over the P2P network by other peers.
4+
5+
A malicious actor may gossip an address:port where no Bitcoin node is listening,
6+
or one where a service is listening that is not related to the Bitcoin network.
7+
As a result, this service may occasionally get connection attempts from Bitcoin
8+
nodes.
9+
10+
"Bad" ports are ones used by services which are usually not open to the public
11+
and usually require authentication. A connection attempt (by Bitcoin Core,
12+
trying to connect because it thinks there is a Bitcoin node on that
13+
address:port) to such service may be considered a malicious action by an
14+
ultra-paranoid administrator. An example for such a port is 22 (ssh). On the
15+
other hand, connection attempts to public services that usually do not require
16+
authentication are unlikely to be considered a malicious action,
17+
e.g. port 80 (http).
18+
19+
Below is a list of "bad" ports which Bitcoin Core avoids when choosing a peer to
20+
connect to. If a node is listening on such a port, it will likely receive less
21+
incoming connections.
22+
23+
1: tcpmux
24+
7: echo
25+
9: discard
26+
11: systat
27+
13: daytime
28+
15: netstat
29+
17: qotd
30+
19: chargen
31+
20: ftp data
32+
21: ftp access
33+
22: ssh
34+
23: telnet
35+
25: smtp
36+
37: time
37+
42: name
38+
43: nicname
39+
53: domain
40+
69: tftp
41+
77: priv-rjs
42+
79: finger
43+
87: ttylink
44+
95: supdup
45+
101: hostname
46+
102: iso-tsap
47+
103: gppitnp
48+
104: acr-nema
49+
109: pop2
50+
110: pop3
51+
111: sunrpc
52+
113: auth
53+
115: sftp
54+
117: uucp-path
55+
119: nntp
56+
123: NTP
57+
135: loc-srv /epmap
58+
137: netbios
59+
139: netbios
60+
143: imap2
61+
161: snmp
62+
179: BGP
63+
389: ldap
64+
427: SLP (Also used by Apple Filing Protocol)
65+
465: smtp+ssl
66+
512: print / exec
67+
513: login
68+
514: shell
69+
515: printer
70+
526: tempo
71+
530: courier
72+
531: chat
73+
532: netnews
74+
540: uucp
75+
548: AFP (Apple Filing Protocol)
76+
554: rtsp
77+
556: remotefs
78+
563: nntp+ssl
79+
587: smtp (rfc6409)
80+
601: syslog-conn (rfc3195)
81+
636: ldap+ssl
82+
989: ftps-data
83+
990: ftps
84+
993: ldap+ssl
85+
995: pop3+ssl
86+
1719: h323gatestat
87+
1720: h323hostcall
88+
1723: pptp
89+
2049: nfs
90+
3659: apple-sasl / PasswordServer
91+
4045: lockd
92+
5060: sip
93+
5061: sips
94+
6000: X11
95+
6566: sane-port
96+
6665: Alternate IRC
97+
6666: Alternate IRC
98+
6667: Standard IRC
99+
6668: Alternate IRC
100+
6669: Alternate IRC
101+
6697: IRC + TLS
102+
10080: Amanda
103+
104+
For further information see:
105+
106+
[pull/23306](https://github.com/bitcoin/bitcoin/pull/23306#issuecomment-947516736)
107+
108+
[pull/23542](https://github.com/bitcoin/bitcoin/pull/23542)
109+
110+
[fetch.spec.whatwg.org](https://fetch.spec.whatwg.org/#port-blocking)
111+
112+
[chromium.googlesource.com](https://chromium.googlesource.com/chromium/src.git/+/refs/heads/main/net/base/port_util.cc)
113+
114+
[hg.mozilla.org](https://hg.mozilla.org/mozilla-central/file/tip/netwerk/base/nsIOService.cpp)

‎src/init.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,8 @@ void SetupServerArgs(ArgsManager& argsman)
466466
argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
467467
argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
468468
argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
469+
// TODO: remove the sentence "Nodes not using ... incoming connections." once the changes from
470+
// https://github.com/bitcoin/bitcoin/pull/23542 have become widespread.
469471
argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port>. Nodes not using the default ports (default: %u, testnet: %u, signet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
470472
argsman.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
471473
argsman.AddArg("-proxyrandomize", strprintf("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)", DEFAULT_PROXYRANDOMIZE), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
@@ -1687,12 +1689,23 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
16871689
connOptions.nMaxOutboundLimit = *opt_max_upload;
16881690
connOptions.m_peer_connect_timeout = peer_connect_timeout;
16891691

1692+
const auto BadPortWarning = [](const char* prefix, uint16_t port) {
1693+
return strprintf(_("%s request to listen on port %u. This port is considered \"bad\" and "
1694+
"thus it is unlikely that any Bitcoin Core peers connect to it. See "
1695+
"doc/p2p-bad-ports.md for details and a full list."),
1696+
prefix,
1697+
port);
1698+
};
1699+
16901700
for (const std::string& bind_arg : args.GetArgs("-bind")) {
16911701
CService bind_addr;
16921702
const size_t index = bind_arg.rfind('=');
16931703
if (index == std::string::npos) {
16941704
if (Lookup(bind_arg, bind_addr, GetListenPort(), false)) {
16951705
connOptions.vBinds.push_back(bind_addr);
1706+
if (IsBadPort(bind_addr.GetPort())) {
1707+
InitWarning(BadPortWarning("-bind", bind_addr.GetPort()));
1708+
}
16961709
continue;
16971710
}
16981711
} else {
@@ -1719,6 +1732,15 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
17191732
// on any address - 0.0.0.0 (IPv4) and :: (IPv6).
17201733
connOptions.bind_on_any = args.GetArgs("-bind").empty() && args.GetArgs("-whitebind").empty();
17211734

1735+
// Emit a warning if a bad port is given to -port= but only if -bind and -whitebind are not
1736+
// given, because if they are, then -port= is ignored.
1737+
if (connOptions.bind_on_any && args.IsArgSet("-port")) {
1738+
const uint16_t port_arg = args.GetIntArg("-port", 0);
1739+
if (IsBadPort(port_arg)) {
1740+
InitWarning(BadPortWarning("-port", port_arg));
1741+
}
1742+
}
1743+
17221744
CService onion_service_target;
17231745
if (!connOptions.onion_binds.empty()) {
17241746
onion_service_target = connOptions.onion_binds.front();

‎src/net.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2120,12 +2120,8 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
21202120
continue;
21212121
}
21222122

2123-
// Do not allow non-default ports, unless after 50 invalid
2124-
// addresses selected already. This is to prevent malicious peers
2125-
// from advertising themselves as a service on another host and
2126-
// port, causing a DoS attack as nodes around the network attempt
2127-
// to connect to it fruitlessly.
2128-
if (addr.GetPort() != Params().GetDefaultPort(addr.GetNetwork()) && nTries < 50) {
2123+
// Do not connect to bad ports, unless 50 invalid addresses have been selected already.
2124+
if (nTries < 50 && (addr.IsIPv4() || addr.IsIPv6()) && IsBadPort(addr.GetPort())) {
21292125
continue;
21302126
}
21312127

‎src/net_processing.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,8 +1775,10 @@ void PeerManagerImpl::RelayAddress(NodeId originator,
17751775
// Relay to a limited number of other nodes
17761776
// Use deterministic randomness to send to the same nodes for 24 hours
17771777
// at a time so the m_addr_knowns of the chosen nodes prevent repeats
1778-
const uint64_t hashAddr{addr.GetHash()};
1779-
const CSipHasher hasher{m_connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr).Write((GetTime() + hashAddr) / (24 * 60 * 60))};
1778+
const uint64_t hash_addr{CServiceHash(0, 0)(addr)};
1779+
const CSipHasher hasher{m_connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY)
1780+
.Write(hash_addr)
1781+
.Write((GetTime() + hash_addr) / (24 * 60 * 60))};
17801782
FastRandomContext insecure_rand;
17811783

17821784
// Relay reachable addresses to 2 peers. Unreachable addresses are relayed randomly to 1 or 2 peers.

‎src/netaddress.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -833,14 +833,6 @@ std::vector<unsigned char> CNetAddr::GetAddrBytes() const
833833
return std::vector<unsigned char>(m_addr.begin(), m_addr.end());
834834
}
835835

836-
uint64_t CNetAddr::GetHash() const
837-
{
838-
uint256 hash = Hash(m_addr);
839-
uint64_t nRet;
840-
memcpy(&nRet, &hash, sizeof(nRet));
841-
return nRet;
842-
}
843-
844836
// private extensions to enum Network, only returned by GetExtNetwork,
845837
// and only used in GetReachabilityFrom
846838
static const int NET_UNKNOWN = NET_MAX + 0;

‎src/netaddress.h‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,6 @@ class CNetAddr
194194
enum Network GetNetwork() const;
195195
std::string ToString() const;
196196
std::string ToStringIP() const;
197-
uint64_t GetHash() const;
198197
bool GetInAddr(struct in_addr* pipv4Addr) const;
199198
Network GetNetClass() const;
200199

@@ -562,6 +561,14 @@ class CService : public CNetAddr
562561
class CServiceHash
563562
{
564563
public:
564+
CServiceHash()
565+
: m_salt_k0{GetRand(std::numeric_limits<uint64_t>::max())},
566+
m_salt_k1{GetRand(std::numeric_limits<uint64_t>::max())}
567+
{
568+
}
569+
570+
CServiceHash(uint64_t salt_k0, uint64_t salt_k1) : m_salt_k0{salt_k0}, m_salt_k1{salt_k1} {}
571+
565572
size_t operator()(const CService& a) const noexcept
566573
{
567574
CSipHasher hasher(m_salt_k0, m_salt_k1);
@@ -572,8 +579,8 @@ class CServiceHash
572579
}
573580

574581
private:
575-
const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max());
576-
const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max());
582+
const uint64_t m_salt_k0;
583+
const uint64_t m_salt_k1;
577584
};
578585

579586
#endif // BITCOIN_NETADDRESS_H

‎src/netbase.cpp‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,3 +749,93 @@ void InterruptSocks5(bool interrupt)
749749
{
750750
interruptSocks5Recv = interrupt;
751751
}
752+
753+
bool IsBadPort(uint16_t port)
754+
{
755+
/* Don't forget to update doc/p2p-bad-ports.md if you change this list. */
756+
757+
switch (port) {
758+
case 1: // tcpmux
759+
case 7: // echo
760+
case 9: // discard
761+
case 11: // systat
762+
case 13: // daytime
763+
case 15: // netstat
764+
case 17: // qotd
765+
case 19: // chargen
766+
case 20: // ftp data
767+
case 21: // ftp access
768+
case 22: // ssh
769+
case 23: // telnet
770+
case 25: // smtp
771+
case 37: // time
772+
case 42: // name
773+
case 43: // nicname
774+
case 53: // domain
775+
case 69: // tftp
776+
case 77: // priv-rjs
777+
case 79: // finger
778+
case 87: // ttylink
779+
case 95: // supdup
780+
case 101: // hostname
781+
case 102: // iso-tsap
782+
case 103: // gppitnp
783+
case 104: // acr-nema
784+
case 109: // pop2
785+
case 110: // pop3
786+
case 111: // sunrpc
787+
case 113: // auth
788+
case 115: // sftp
789+
case 117: // uucp-path
790+
case 119: // nntp
791+
case 123: // NTP
792+
case 135: // loc-srv /epmap
793+
case 137: // netbios
794+
case 139: // netbios
795+
case 143: // imap2
796+
case 161: // snmp
797+
case 179: // BGP
798+
case 389: // ldap
799+
case 427: // SLP (Also used by Apple Filing Protocol)
800+
case 465: // smtp+ssl
801+
case 512: // print / exec
802+
case 513: // login
803+
case 514: // shell
804+
case 515: // printer
805+
case 526: // tempo
806+
case 530: // courier
807+
case 531: // chat
808+
case 532: // netnews
809+
case 540: // uucp
810+
case 548: // AFP (Apple Filing Protocol)
811+
case 554: // rtsp
812+
case 556: // remotefs
813+
case 563: // nntp+ssl
814+
case 587: // smtp (rfc6409)
815+
case 601: // syslog-conn (rfc3195)
816+
case 636: // ldap+ssl
817+
case 989: // ftps-data
818+
case 990: // ftps
819+
case 993: // ldap+ssl
820+
case 995: // pop3+ssl
821+
case 1719: // h323gatestat
822+
case 1720: // h323hostcall
823+
case 1723: // pptp
824+
case 2049: // nfs
825+
case 3659: // apple-sasl / PasswordServer
826+
case 4045: // lockd
827+
case 5060: // sip
828+
case 5061: // sips
829+
case 6000: // X11
830+
case 6566: // sane-port
831+
case 6665: // Alternate IRC
832+
case 6666: // Alternate IRC
833+
case 6667: // Standard IRC
834+
case 6668: // Alternate IRC
835+
case 6669: // Alternate IRC
836+
case 6697: // IRC + TLS
837+
case 10080: // Amanda
838+
return true;
839+
}
840+
return false;
841+
}

‎src/netbase.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,4 +247,13 @@ void InterruptSocks5(bool interrupt);
247247
*/
248248
bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* auth, const Sock& socket);
249249

250+
/**
251+
* Determine if a port is "bad" from the perspective of attempting to connect
252+
* to a node on that port.
253+
* @see doc/p2p-bad-ports.md
254+
* @param[in] port Port to check.
255+
* @returns whether the port is bad
256+
*/
257+
bool IsBadPort(uint16_t port);
258+
250259
#endif // BITCOIN_NETBASE_H

‎src/test/fuzz/netaddress.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ FUZZ_TARGET(netaddress)
1616
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
1717

1818
const CNetAddr net_addr = ConsumeNetAddr(fuzzed_data_provider);
19-
(void)net_addr.GetHash();
2019
(void)net_addr.GetNetClass();
2120
if (net_addr.GetNetwork() == Network::NET_IPV4) {
2221
assert(net_addr.IsIPv4());
@@ -84,6 +83,8 @@ FUZZ_TARGET(netaddress)
8483
(void)service.ToString();
8584
(void)service.ToStringIPPort();
8685
(void)service.ToStringPort();
86+
(void)CServiceHash()(service);
87+
(void)CServiceHash(0, 0)(service);
8788

8889
const CNetAddr other_net_addr = ConsumeNetAddr(fuzzed_data_provider);
8990
(void)net_addr.GetReachabilityFrom(&other_net_addr);

0 commit comments

Comments
 (0)