net: open p2p connections to nodes that listen on non-default ports

doc/README.md

@@ -79,6 +79,7 @@ The Bitcoin repo's [root README](/README.md) contains relevant information on th
 - [Init Scripts (systemd/upstart/openrc)](init.md)
 - [Managing Wallets](managing-wallets.md)
 - [Multisig Tutorial](multisig-tutorial.md)
+- [P2P bad ports definition and list](p2p-bad-ports.md)
 - [PSBT support](psbt.md)
 - [Reduce Memory](reduce-memory.md)
 - [Reduce Traffic](reduce-traffic.md)

doc/p2p-bad-ports.md

@@ -0,0 +1,114 @@
+When Bitcoin Core automatically opens outgoing P2P connections it chooses
+a peer (address and port) from its list of potential peers. This list is
+populated with unchecked data, gossiped over the P2P network by other peers.
+
+A malicious actor may gossip an address:port where no Bitcoin node is listening,
+or one where a service is listening that is not related to the Bitcoin network.
+As a result, this service may occasionally get connection attempts from Bitcoin
+nodes.
+
+"Bad" ports are ones used by services which are usually not open to the public
+and usually require authentication. A connection attempt (by Bitcoin Core,
+trying to connect because it thinks there is a Bitcoin node on that
+address:port) to such service may be considered a malicious action by an
+ultra-paranoid administrator. An example for such a port is 22 (ssh). On the
+other hand, connection attempts to public services that usually do not require
+authentication are unlikely to be considered a malicious action,
+e.g. port 80 (http).
+
+Below is a list of "bad" ports which Bitcoin Core avoids when choosing a peer to
+connect to. If a node is listening on such a port, it will likely receive less
+incoming connections.
+
+    1:     tcpmux
+    7:     echo
+    9:     discard
+    11:    systat
+    13:    daytime
+    15:    netstat
+    17:    qotd
+    19:    chargen
+    20:    ftp data
+    21:    ftp access
+    22:    ssh
+    23:    telnet
+    25:    smtp
+    37:    time
+    42:    name
+    43:    nicname
+    53:    domain
+    69:    tftp
+    77:    priv-rjs
+    79:    finger
+    87:    ttylink
+    95:    supdup
+    101:   hostname
+    102:   iso-tsap
+    103:   gppitnp
+    104:   acr-nema
+    109:   pop2
+    110:   pop3
+    111:   sunrpc
+    113:   auth
+    115:   sftp
+    117:   uucp-path
+    119:   nntp
+    123:   NTP
+    135:   loc-srv /epmap
+    137:   netbios
+    139:   netbios
+    143:   imap2
+    161:   snmp
+    179:   BGP
+    389:   ldap
+    427:   SLP (Also used by Apple Filing Protocol)
+    465:   smtp+ssl
+    512:   print / exec
+    513:   login
+    514:   shell
+    515:   printer
+    526:   tempo
+    530:   courier
+    531:   chat
+    532:   netnews
+    540:   uucp
+    548:   AFP (Apple Filing Protocol)
+    554:   rtsp
+    556:   remotefs
+    563:   nntp+ssl
+    587:   smtp (rfc6409)
+    601:   syslog-conn (rfc3195)
+    636:   ldap+ssl
+    989:   ftps-data
+    990:   ftps
+    993:   ldap+ssl
+    995:   pop3+ssl
+    1719:  h323gatestat
+    1720:  h323hostcall
+    1723:  pptp
+    2049:  nfs
+    3659:  apple-sasl / PasswordServer
+    4045:  lockd
+    5060:  sip
+    5061:  sips
+    6000:  X11
+    6566:  sane-port
+    6665:  Alternate IRC
+    6666:  Alternate IRC
+    6667:  Standard IRC
+    6668:  Alternate IRC
+    6669:  Alternate IRC
+    6697:  IRC + TLS
+    10080: Amanda
+
+For further information see:
+
+[pull/23306](https://github.com/bitcoin/bitcoin/pull/23306#issuecomment-947516736)
+
+[pull/23542](https://github.com/bitcoin/bitcoin/pull/23542)
+
+[fetch.spec.whatwg.org](https://fetch.spec.whatwg.org/#port-blocking)
+
+[chromium.googlesource.com](https://chromium.googlesource.com/chromium/src.git/+/refs/heads/main/net/base/port_util.cc)
+
+[hg.mozilla.org](https://hg.mozilla.org/mozilla-central/file/tip/netwerk/base/nsIOService.cpp)

src/init.cpp

@@ -466,6 +466,8 @@ 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);
+    // TODO: remove the sentence "Nodes not using ... incoming connections." once the changes from
+    // https://github.com/bitcoin/bitcoin/pull/23542 have become widespread.
     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("-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);
@@ -1698,12 +1700,23 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     connOptions.nMaxOutboundLimit = *opt_max_upload;
     connOptions.m_peer_connect_timeout = peer_connect_timeout;
 
+    const auto BadPortWarning = [](const char* prefix, uint16_t port) {
+        return strprintf(_("%s 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 "
+                           "doc/p2p-bad-ports.md for details and a full list."),
+                         prefix,
+                         port);
+    };
+
     for (const std::string& bind_arg : args.GetArgs("-bind")) {
         CService bind_addr;
         const size_t index = bind_arg.rfind('=');
         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(BadPortWarning("-bind", bind_addr.GetPort()));
+                }
                 continue;
             }
         } else {
@@ -1730,6 +1743,15 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // on any address - 0.0.0.0 (IPv4) and :: (IPv6).
     connOptions.bind_on_any = args.GetArgs("-bind").empty() && args.GetArgs("-whitebind").empty();
 
+    // Emit a warning if a bad port is given to -port= but only if -bind and -whitebind are not
+    // given, because if they are, then -port= is ignored.
+    if (connOptions.bind_on_any && args.IsArgSet("-port")) {
+        const uint16_t port_arg = args.GetIntArg("-port", 0);
+        if (IsBadPort(port_arg)) {
+            InitWarning(BadPortWarning("-port", port_arg));
+        }
+    }
+
     CService onion_service_target;
     if (!connOptions.onion_binds.empty()) {
         onion_service_target = connOptions.onion_binds.front();

src/net.cpp

@@ -2120,12 +2120,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;
             }
 

src/net_processing.cpp

@@ -1775,8 +1775,10 @@ void PeerManagerImpl::RelayAddress(NodeId originator,
     // Relay to a limited number of other nodes
     // Use deterministic randomness to send to the same nodes for 24 hours
     // at a time so the m_addr_knowns of the chosen nodes prevent repeats
-    const uint64_t hashAddr{addr.GetHash()};
-    const CSipHasher hasher{m_connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr).Write((GetTime() + hashAddr) / (24 * 60 * 60))};
+    const uint64_t hash_addr{CServiceHash(0, 0)(addr)};
+    const CSipHasher hasher{m_connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY)
+                                .Write(hash_addr)
+                                .Write((GetTime() + hash_addr) / (24 * 60 * 60))};
     FastRandomContext insecure_rand;
 
     // Relay reachable addresses to 2 peers. Unreachable addresses are relayed randomly to 1 or 2 peers.

src/netaddress.cpp

@@ -833,14 +833,6 @@ std::vector<unsigned char> CNetAddr::GetAddrBytes() const
     return std::vector<unsigned char>(m_addr.begin(), m_addr.end());
 }
 
-uint64_t CNetAddr::GetHash() const
-{
-    uint256 hash = Hash(m_addr);
-    uint64_t nRet;
-    memcpy(&nRet, &hash, sizeof(nRet));
-    return nRet;
-}
-
 // private extensions to enum Network, only returned by GetExtNetwork,
 // and only used in GetReachabilityFrom
 static const int NET_UNKNOWN = NET_MAX + 0;

src/netaddress.h

@@ -194,7 +194,6 @@ class CNetAddr
     enum Network GetNetwork() const;
     std::string ToString() const;
     std::string ToStringIP() const;
-    uint64_t GetHash() const;
     bool GetInAddr(struct in_addr* pipv4Addr) const;
     Network GetNetClass() const;
 
@@ -562,6 +561,14 @@ class CService : public CNetAddr
 class CServiceHash
 {
 public:
+    CServiceHash()
+        : m_salt_k0{GetRand(std::numeric_limits<uint64_t>::max())},
+          m_salt_k1{GetRand(std::numeric_limits<uint64_t>::max())}
+    {
+    }
+
+    CServiceHash(uint64_t salt_k0, uint64_t salt_k1) : m_salt_k0{salt_k0}, m_salt_k1{salt_k1} {}
+
     size_t operator()(const CService& a) const noexcept
     {
         CSipHasher hasher(m_salt_k0, m_salt_k1);
@@ -572,8 +579,8 @@ class CServiceHash
     }
 
 private:
-    const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max());
-    const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max());
+    const uint64_t m_salt_k0;
+    const uint64_t m_salt_k1;
 };
 
 #endif // BITCOIN_NETADDRESS_H

src/netbase.cpp

@@ -749,3 +749,93 @@ void InterruptSocks5(bool interrupt)
 {
     interruptSocks5Recv = interrupt;
 }
+
+bool IsBadPort(uint16_t port)
+{
+    /* Don't forget to update doc/p2p-bad-ports.md if you change this list. */
+
+    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
+    case 6666:  // Alternate IRC
+    case 6667:  // Standard IRC
+    case 6668:  // Alternate IRC
+    case 6669:  // Alternate IRC
+    case 6697:  // IRC + TLS
+    case 10080: // Amanda
+        return true;
+    }
+    return false;
+}

src/netbase.h

@@ -247,4 +247,13 @@ 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 doc/p2p-bad-ports.md
+ * @param[in] port Port to check.
+ * @returns whether the port is bad
+ */
+bool IsBadPort(uint16_t port);
+
 #endif // BITCOIN_NETBASE_H

src/test/fuzz/netaddress.cpp

@@ -16,7 +16,6 @@ FUZZ_TARGET(netaddress)
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
     const CNetAddr net_addr = ConsumeNetAddr(fuzzed_data_provider);
-    (void)net_addr.GetHash();
     (void)net_addr.GetNetClass();
     if (net_addr.GetNetwork() == Network::NET_IPV4) {
         assert(net_addr.IsIPv4());
@@ -84,6 +83,8 @@ FUZZ_TARGET(netaddress)
     (void)service.ToString();
     (void)service.ToStringIPPort();
     (void)service.ToStringPort();
+    (void)CServiceHash()(service);
+    (void)CServiceHash(0, 0)(service);
 
     const CNetAddr other_net_addr = ConsumeNetAddr(fuzzed_data_provider);
     (void)net_addr.GetReachabilityFrom(&other_net_addr);

src/test/netbase_tests.cpp

@@ -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()