Merge bitcoin/bitcoin#23542: net: open p2p connections to nodes that listen on non-default ports

36ee76d1afbb278500fc8aa01606ec933b52c17d net: remove unused CNetAddr::GetHash() (Vasil Dimov)
d0abce9a50dd4f507e3a30348eabffb7552471d5 net: include the port when deciding a relay destination (Vasil Dimov)
2e38a0e6865187d1f0d0f016d3df7cce414a7c4f net: add CServiceHash constructor so the caller can provide the salts (Vasil Dimov)
97208634b96f2d9a55f2ead7b0ef407da729d7bd 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:
  https://github.com/bitcoin/bitcoin/pull/23306

ACKs for top commit:
  laanwj:
    Concept and light code review ACK 36ee76d1afbb278500fc8aa01606ec933b52c17d
  prayank23:
    ACK https://github.com/bitcoin/bitcoin/pull/23542/commits/36ee76d1afbb278500fc8aa01606ec933b52c17d
  stickies-v:
    tACK 36ee76d1a
  jonatack:
    ACK 36ee76d1afbb278500fc8aa01606ec933b52c17d
  glozow:
    utACK 36ee76d1afbb278500fc8aa01606ec933b52c17d

Tree-SHA512: 7f45ab7567c51c19fc50fabbaf84f0cc8883a8eef84272b76435c014c31d89144271d70dd387212cc1114213165d76b4d20a5ddb8dbc958fe7e74e6ddbd56d11

1 files changed, 22 insertions, 0 deletions

diff --git a/src/init.cpp b/src/init.cpp
index ce666534ae..9813a16563 100644
--- a/src/init.cpp
+++ b/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);
@@ -1687,12 +1689,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 {
@@ -1719,6 +1732,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();