Skip to content

Commit 1dc6ad6

Browse files
delta1delta1
committed
Merge ba11eb3 into merged_master (Bitcoin PR bitcoin/bitcoin#23542)
2 parents 7d91422 + ba11eb3 commit 1dc6ad6

11 files changed

Lines changed: 274 additions & 20 deletions

File tree

‎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
@@ -479,6 +479,8 @@ void SetupServerArgs(ArgsManager& argsman)
479479
argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
480480
argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
481481
argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
482+
// TODO: remove the sentence "Nodes not using ... incoming connections." once the changes from
483+
// https://github.com/bitcoin/bitcoin/pull/23542 have become widespread.
482484
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);
483485
argsman.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
484486
argsman.AddArg("-proxyrandomize", strprintf("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)", DEFAULT_PROXYRANDOMIZE), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
@@ -1827,12 +1829,23 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
18271829
connOptions.nMaxOutboundLimit = *opt_max_upload;
18281830
connOptions.m_peer_connect_timeout = peer_connect_timeout;
18291831

1832+
const auto BadPortWarning = [](const char* prefix, uint16_t port) {
1833+
return strprintf(_("%s request to listen on port %u. This port is considered \"bad\" and "
1834+
"thus it is unlikely that any Bitcoin Core peers connect to it. See "
1835+
"doc/p2p-bad-ports.md for details and a full list."),
1836+
prefix,
1837+
port);
1838+
};
1839+
18301840
for (const std::string& bind_arg : args.GetArgs("-bind")) {
18311841
CService bind_addr;
18321842
const size_t index = bind_arg.rfind('=');
18331843
if (index == std::string::npos) {
18341844
if (Lookup(bind_arg, bind_addr, GetListenPort(), false)) {
18351845
connOptions.vBinds.push_back(bind_addr);
1846+
if (IsBadPort(bind_addr.GetPort())) {
1847+
InitWarning(BadPortWarning("-bind", bind_addr.GetPort()));
1848+
}
18361849
continue;
18371850
}
18381851
} else {
@@ -1859,6 +1872,15 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
18591872
// on any address - 0.0.0.0 (IPv4) and :: (IPv6).
18601873
connOptions.bind_on_any = args.GetArgs("-bind").empty() && args.GetArgs("-whitebind").empty();
18611874

1875+
// Emit a warning if a bad port is given to -port= but only if -bind and -whitebind are not
1876+
// given, because if they are, then -port= is ignored.
1877+
if (connOptions.bind_on_any && args.IsArgSet("-port")) {
1878+
const uint16_t port_arg = args.GetIntArg("-port", 0);
1879+
if (IsBadPort(port_arg)) {
1880+
InitWarning(BadPortWarning("-port", port_arg));
1881+
}
1882+
}
1883+
18621884
CService onion_service_target;
18631885
if (!connOptions.onion_binds.empty()) {
18641886
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
@@ -1779,8 +1779,10 @@ void PeerManagerImpl::RelayAddress(NodeId originator,
17791779
// Relay to a limited number of other nodes
17801780
// Use deterministic randomness to send to the same nodes for 24 hours
17811781
// at a time so the m_addr_knowns of the chosen nodes prevent repeats
1782-
const uint64_t hashAddr{addr.GetHash()};
1783-
const CSipHasher hasher{m_connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr).Write((GetTime() + hashAddr) / (24 * 60 * 60))};
1782+
const uint64_t hash_addr{CServiceHash(0, 0)(addr)};
1783+
const CSipHasher hasher{m_connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY)
1784+
.Write(hash_addr)
1785+
.Write((GetTime() + hash_addr) / (24 * 60 * 60))};
17841786
FastRandomContext insecure_rand;
17851787

17861788
// 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)