fix(net): keep a dedicated onion listener when -bind is given and correct the bind release note

doc/release-notes-7300.md

@@ -3,6 +3,14 @@ P2P and network changes
 
 * Startup now fails if any configured `-bind`, `-whitebind` or the implicit Tor
   onion-service bind (`127.0.0.1:9996` by default) cannot be set up. Dash Core
-  v23 and earlier started as long as at least one bind succeeded. Use
-  `-bind=<addr>:<port>=onion` to move the onion bind, or `-listenonion=0` to
-  drop it, if the default address is not available on your system.
+  v23 and earlier started as long as at least one bind succeeded.
+
+* Nodes configured with `-bind` but no specific `-bind=<addr:port>=onion` now
+  refuse to start when `-listenonion` is enabled. This includes nodes without
+  Tor configured, since `-listenonion` is enabled by default when listening.
+  Shared binds cannot distinguish Tor-forwarded connections from direct
+  connections, which can grant Tor peers unintended IP-based whitelist
+  permissions. Nodes without `-bind`, including those using only `-whitebind`,
+  continue to get the default onion target. Users should add a specific
+  `-bind=<addr:port>=onion` to accept incoming Tor connections, or set
+  `-listenonion=0` to disable automatic onion service creation.

doc/tor.md

@@ -83,6 +83,9 @@ it requires a Tor connection to work. It can be explicitly disabled with
 `-listenonion=0`. If it is not disabled, it can be configured using the
 `-torcontrol` and `-torpassword` settings.
 
+When enabled with an explicit `-bind`, the automatic service requires a non-wildcard onion bind such as `-bind=127.0.0.1:<port>=onion`.
+Without `-bind`, Dash Core adds the default onion bind.
+
 To see verbose Tor information in the dashd debug log, pass `-debug=tor`.
 
 ### Control Port

src/init.cpp

@@ -653,7 +653,8 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-addnode=<ip>", strprintf("Add a node to connect to and attempt to keep the connection open (see the addnode RPC help for more info). This option can be specified multiple times to add multiple nodes; connections are limited to %u at a time and are counted separately from the -maxconnections limit.", MAX_ADDNODE_CONNECTIONS), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
     argsman.AddArg("-allowprivatenet", strprintf("Allow RFC1918 addresses to be relayed and connected to (default: %u)", DEFAULT_ALLOWPRIVATENET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-bantime=<n>", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
-    argsman.AddArg("-bind=<addr>[:<port>][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet: 127.0.0.1:%u=onion, devnet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion)", defaultBaseParams->OnionServiceTargetPort(), testnetBaseParams->OnionServiceTargetPort(), devnetBaseParams->OnionServiceTargetPort(), regtestBaseParams->OnionServiceTargetPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
+    argsman.AddArg("-bind=<addr>[:<port>][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet: 127.0.0.1:%u=onion, devnet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion). "
+                                                      "The default onion bind is added only when no -bind is specified. When -listenonion is enabled and -bind is specified, a non-wildcard =onion bind is required, and wildcard =onion binds are rejected.", defaultBaseParams->OnionServiceTargetPort(), testnetBaseParams->OnionServiceTargetPort(), devnetBaseParams->OnionServiceTargetPort(), regtestBaseParams->OnionServiceTargetPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
     argsman.AddArg("-cjdnsreachable", "If set, then this host is configured for CJDNS (connecting to fc00::/8 addresses would lead us to the CJDNS network, see doc/cjdns.md) (default: 0)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-connect=<ip>", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
     argsman.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
@@ -2675,17 +2676,18 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         }
     }
 
-    CService onion_service_target;
-    if (!connOptions.onion_binds.empty()) {
-        onion_service_target = connOptions.onion_binds.front();
-    } else if (!connOptions.vBinds.empty()) {
-        onion_service_target = connOptions.vBinds.front();
-    } else {
-        onion_service_target = DefaultOnionServiceTarget();
-        connOptions.onion_binds.push_back(onion_service_target);
+    if (connOptions.onion_binds.empty() && connOptions.vBinds.empty()) {
+        connOptions.onion_binds.push_back(DefaultOnionServiceTarget());
     }
 
     if (args.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION)) {
+        if (connOptions.onion_binds.empty()) {
+            return InitError(_("The automatic Tor onion service requires a dedicated onion bind. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0."));
+        }
+        if (std::ranges::any_of(connOptions.onion_binds, [](auto& b) { return b.IsBindAny(); })) {
+            return InitError(_("The automatic Tor onion service cannot use a wildcard onion bind because the Tor daemon wouldn't be able to forward incoming connections to us. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0."));
+        }
+        const CService& onion_service_target{connOptions.onion_binds[0]};
         if (connOptions.onion_binds.size() > 1) {
             InitWarning(strprintf(_("More than one onion bind address is provided. Using %s "
                                     "for the automatically created Tor onion service."),

test/functional/feature_bind_extra.py

@@ -87,5 +87,22 @@ def run_test(self):
             binds = set(filter(lambda e: e[1] != rpc_port(i), binds))
             assert_equal(binds, set(expected_services))
 
+        self.log.info("Test -listenonion with a normal bind and no dedicated onion bind")
+        self.stop_node(2)
+        self.nodes[2].assert_start_raises_init_error(
+            self.extra_args[2] + ["-listenonion=1", "-torcontrol=127.0.0.1:1"],
+            "Error: The automatic Tor onion service requires a dedicated onion bind. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0.",
+        )
+
+        self.log.info("Test -bind with dedicated onion bind starts when -listenonion=1")
+        self.restart_node(1, extra_args=self.extra_args[1] + ["-listenonion=1", "-torcontrol=127.0.0.1:1"])
+
+        self.log.info("Test wildcard onion bind with -listenonion=1")
+        self.stop_node(0)
+        self.nodes[0].assert_start_raises_init_error(
+            [f"-bind=0.0.0.0:{p2p_port(0)}=onion", "-listenonion=1", "-torcontrol=127.0.0.1:1"],
+            "Error: The automatic Tor onion service cannot use a wildcard onion bind because the Tor daemon wouldn't be able to forward incoming connections to us. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0.",
+        )
+
 if __name__ == '__main__':
     BindExtraTest().main()

test/functional/feature_proxy.py

@@ -49,6 +49,7 @@
 from test_framework.util import (
     assert_equal,
     p2p_port,
+    tor_port,
 )
 from test_framework.netutil import test_ipv6_local, test_unix_socket
 
@@ -436,7 +437,7 @@ def networks_dict(d):
         self.nodes[1].assert_start_raises_init_error(expected_msg=msg)
 
         self.log.info("Test passing -onlynet=onion without -proxy or -onion but with -listenonion=1 is ok")
-        self.start_node(1, extra_args=["-onlynet=onion", "-listenonion=1"])
+        self.start_node(1, extra_args=["-onlynet=onion", "-listenonion=1", f"-bind=127.0.0.1:{tor_port(1)}=onion"])
         self.stop_node(1)
 
         self.log.info("Test passing unknown network to -onlynet raises expected init error")