Skip to content

Commit

Permalink
net: open p2p connections to nodes that listen on non-default ports
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@vasild
vasild committed Jan 6, 2022
1 parent 95833c0 commit c66edde
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 7 deletions.
9 changes: 8 additions & 1 deletion src/init.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ void SetupServerArgs(ArgsManager& argsman)
argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
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);
argsman.AddArg("-port=<port>", "Listen for connections on <port>. Not relevant for I2P (see doc/i2p.md).", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
argsman.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-proxyrandomize", strprintf("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)", DEFAULT_PROXYRANDOMIZE), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-seednode=<ip>", "Connect to a node to retrieve peer addresses, and disconnect. This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
Expand Down Expand Up @@ -1685,6 +1685,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
if (index == std::string::npos) {
if (Lookup(bind_arg, bind_addr, GetListenPort(), false)) {
connOptions.vBinds.push_back(bind_addr);
if (IsBadPort(bind_addr.GetPort())) {
InitWarning(strprintf(
_("-bind request to listen on port %u. This port is considered \"bad\" and "
"thus it is unlikely that any Bitcoin Core peers connect to it. See "
"https://fetch.spec.whatwg.org/#port-blocking"),
bind_addr.GetPort()));
}
continue;
}
} else {
Expand Down
8 changes: 2 additions & 6 deletions src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2096,12 +2096,8 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
continue;
}

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

Expand Down
88 changes: 88 additions & 0 deletions src/netbase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,3 +749,91 @@ void InterruptSocks5(bool interrupt)
{
interruptSocks5Recv = interrupt;
}

bool IsBadPort(uint16_t port)
{
switch (port) {
case 1: // tcpmux
case 7: // echo
case 9: // discard
case 11: // systat
case 13: // daytime
case 15: // netstat
case 17: // qotd
case 19: // chargen
case 20: // ftp data
case 21: // ftp access
case 22: // ssh
case 23: // telnet
case 25: // smtp
case 37: // time
case 42: // name
case 43: // nicname
case 53: // domain
case 69: // tftp
case 77: // priv-rjs
case 79: // finger
case 87: // ttylink
case 95: // supdup
case 101: // hostname
case 102: // iso-tsap
case 103: // gppitnp
case 104: // acr-nema
case 109: // pop2
case 110: // pop3
case 111: // sunrpc
case 113: // auth
case 115: // sftp
case 117: // uucp-path
case 119: // nntp
case 123: // NTP
case 135: // loc-srv /epmap
case 137: // netbios
case 139: // netbios
case 143: // imap2
case 161: // snmp
case 179: // BGP
case 389: // ldap
case 427: // SLP (Also used by Apple Filing Protocol)
case 465: // smtp+ssl
case 512: // print / exec
case 513: // login
case 514: // shell
case 515: // printer
case 526: // tempo
case 530: // courier
case 531: // chat
case 532: // netnews
case 540: // uucp
case 548: // AFP (Apple Filing Protocol)
case 554: // rtsp
case 556: // remotefs
case 563: // nntp+ssl
case 587: // smtp (rfc6409)
case 601: // syslog-conn (rfc3195)
case 636: // ldap+ssl
case 989: // ftps-data
case 990: // ftps
case 993: // ldap+ssl
case 995: // pop3+ssl
case 1719: // h323gatestat
case 1720: // h323hostcall
case 1723: // pptp
case 2049: // nfs
case 3659: // apple-sasl / PasswordServer
case 4045: // lockd
case 5060: // sip
case 5061: // sips
case 6000: // X11
case 6566: // sane-port
case 6665: // Alternate IRC [Apple addition]
case 6666: // Alternate IRC [Apple addition]
case 6667: // Standard IRC [Apple addition]
case 6668: // Alternate IRC [Apple addition]
case 6669: // Alternate IRC [Apple addition]
case 6697: // IRC + TLS
case 10080: // Amanda
return true;
}
return false;
}
13 changes: 13 additions & 0 deletions src/netbase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,4 +247,17 @@ void InterruptSocks5(bool interrupt);
*/
bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* auth, const Sock& socket);

/**
* Determine if a port is "bad" from the perspective of attempting to connect
* to a node on that port.
* See
* https://fetch.spec.whatwg.org/#port-blocking
* https://chromium.googlesource.com/chromium/src.git/+/refs/heads/main/net/base/port_util.cc
* https://hg.mozilla.org/mozilla-central/file/tip/netwerk/base/nsIOService.cpp
* https://github.com/bitcoin/bitcoin/pull/23306#issuecomment-947516736
* @param[in] port Port to check.
* @return true if the port is bad
*/
bool IsBadPort(uint16_t port);

#endif // BITCOIN_NETBASE_H
20 changes: 20 additions & 0 deletions src/test/netbase_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -576,4 +576,24 @@ BOOST_AUTO_TEST_CASE(caddress_unserialize_v2)
BOOST_CHECK(fixture_addresses == addresses_unserialized);
}

BOOST_AUTO_TEST_CASE(isbadport)
{
BOOST_CHECK(IsBadPort(1));
BOOST_CHECK(IsBadPort(22));
BOOST_CHECK(IsBadPort(6000));

BOOST_CHECK(!IsBadPort(80));
BOOST_CHECK(!IsBadPort(443));
BOOST_CHECK(!IsBadPort(8333));

// Check all ports, there must be 80 bad ports in total.
size_t total_bad_ports{0};
for (uint16_t port = std::numeric_limits<uint16_t>::max(); port > 0; --port) {
if (IsBadPort(port)) {
++total_bad_ports;
}
}
BOOST_CHECK_EQUAL(total_bad_ports, 80);
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit c66edde

Please sign in to comment.