diff --git a/doc/README.md b/doc/README.md index 425b25991d..5080c78e33 100644 --- a/doc/README.md +++ b/doc/README.md @@ -76,6 +76,7 @@ The Dash Core repo's [root README](/README.md) contains relevant information on - [I2P Support](i2p.md) - [Init Scripts (systemd/upstart/openrc)](init.md) - [Managing Wallets](managing-wallets.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) diff --git a/doc/p2p-bad-ports.md b/doc/p2p-bad-ports.md new file mode 100644 index 0000000000..0dd7d36cf4 --- /dev/null +++ b/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) diff --git a/src/init.cpp b/src/init.cpp index 8967159b7b..0b3927e3a2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -582,6 +582,8 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-peertimeout=", strprintf("Specify a p2p connection timeout delay in seconds. After connecting to a peer, wait this amount of time before considering disconnection based on inactivity (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), 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=", strprintf("Listen for connections on . Nodes not using the default ports (default: %u, testnet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-proxy=", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_ELISION, 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); @@ -2418,6 +2420,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Port to bind to if `-bind=addr` is provided without a `:port` suffix. const uint16_t default_bind_port = static_cast(args.GetArg("-port", Params().GetDefaultPort())); + 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")) { std::optional bind_addr; @@ -2426,6 +2435,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) bind_addr = Lookup(bind_arg, default_bind_port, /*fAllowLookup=*/false); if (bind_addr.has_value()) { connOptions.vBinds.push_back(bind_addr.value()); + if (IsBadPort(bind_addr.GetPort())) { + InitWarning(BadPortWarning("-bind", bind_addr.GetPort())); + } continue; } } else { @@ -2453,6 +2465,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(); diff --git a/src/net.cpp b/src/net.cpp index 6d9d7fe1dd..2f3f1e9790 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3542,12 +3542,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe 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 ((!isMasternode || !Params().AllowMultiplePorts()) && addr.GetPort() != Params().GetDefaultPort(addr.GetNetwork()) && addr.GetPort() != GetListenPort() && nTries < 50) { + if ((!isMasternode || !Params().AllowMultiplePorts()) && addr.GetPort() != GetListenPort() && nTries < 50 && (addr.IsIPv4() || addr.IsIPv6()) && IsBadPort(addr.GetPort())) { continue; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f6085ffd7c..2070f82aff 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2406,8 +2406,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. diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 6466aec6fa..8f9afcdcd3 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -732,14 +732,6 @@ std::vector CNetAddr::GetAddrBytes() const return std::vector(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_TEREDO = NET_MAX; diff --git a/src/netaddress.h b/src/netaddress.h index 102de00225..dc5587bd88 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -203,7 +203,6 @@ public: enum Network GetNetwork() const; std::string ToStringAddr() const; - uint64_t GetHash() const; bool GetInAddr(struct in_addr* pipv4Addr) const; Network GetNetClass() const; @@ -564,6 +563,14 @@ public: class CServiceHash { public: + CServiceHash() + : m_salt_k0{GetRand(std::numeric_limits::max())}, + m_salt_k1{GetRand(std::numeric_limits::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); @@ -574,8 +581,8 @@ public: } private: - const uint64_t m_salt_k0 = GetRand(std::numeric_limits::max()); - const uint64_t m_salt_k1 = GetRand(std::numeric_limits::max()); + const uint64_t m_salt_k0; + const uint64_t m_salt_k1; }; #endif // BITCOIN_NETADDRESS_H diff --git a/src/netbase.cpp b/src/netbase.cpp index cf1ba19e65..3e3b8b91c1 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -720,3 +720,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; +} diff --git a/src/netbase.h b/src/netbase.h index 9478555d59..8dbedd70c5 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -248,4 +248,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 diff --git a/src/test/fuzz/netaddress.cpp b/src/test/fuzz/netaddress.cpp index 61279c32ec..1b5122b1c9 100644 --- a/src/test/fuzz/netaddress.cpp +++ b/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()); @@ -81,6 +80,8 @@ FUZZ_TARGET(netaddress) (void)service.GetKey(); (void)service.GetPort(); (void)service.ToStringAddrPort(); + (void)CServiceHash()(service); + (void)CServiceHash(0, 0)(service); const CNetAddr other_net_addr = ConsumeNetAddr(fuzzed_data_provider); (void)net_addr.GetReachabilityFrom(other_net_addr); diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index d9fe06d759..7bb89a9b8f 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -575,4 +575,24 @@ BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters) BOOST_CHECK(!LookupSubNet("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion\0example.com\0"s, ret)); } +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::max(); port > 0; --port) { + if (IsBadPort(port)) { + ++total_bad_ports; + } + } + BOOST_CHECK_EQUAL(total_bad_ports, 80); +} + BOOST_AUTO_TEST_SUITE_END()