net: don't extra bind for Tor if binds are restricted

If only `-bind=addr:port` is given (without `-bind=...=onion`) then we
would bind to `addr:port` _and_ to `127.0.0.1:8334` in addition which
may be unexpected, assuming the semantic of `-bind=addr:port` is
"bind _only_ to `addr:port`".

Change the above to not do the additional bind: if only
`-bind=addr:port` is given (without `-bind=...=onion`) then bind to
`addr:port` (only). If we are creating a Tor hidden service then use
`addr:port` as target (same behavior as before
https://github.com/bitcoin/bitcoin/pull/19991).

This allows disabling binding on the onion port.

Fixes https://github.com/bitcoin/bitcoin/issues/22726
This commit is contained in:
Vasil Dimov 2021-08-02 18:54:06 +02:00
parent 2f6dca4d1c
commit 9a7e5f4d68
No known key found for this signature in database
GPG Key ID: 54DF06F64B55CBBF
2 changed files with 16 additions and 9 deletions

View File

@ -1890,6 +1890,8 @@ 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);

View File

@ -27,7 +27,7 @@ class BindExtraTest(BitcoinTestFramework):
# Avoid any -bind= on the command line. Force the framework to avoid
# adding -bind=127.0.0.1.
self.bind_to_localhost_only = False
self.num_nodes = 2
self.num_nodes = 3
def skip_test_if_missing_module(self):
# Due to OS-specific network stats queries, we only run on Linux.
@ -60,14 +60,21 @@ class BindExtraTest(BitcoinTestFramework):
)
port += 2
# Node2, no -bind=...=onion, thus no extra port for Tor target.
self.expected.append(
[
[f"-bind=127.0.0.1:{port}"],
[(loopback_ipv4, port)]
],
)
port += 1
self.extra_args = list(map(lambda e: e[0], self.expected))
self.add_nodes(self.num_nodes, self.extra_args)
# Don't start the nodes, as some of them would collide trying to bind on the same port.
self.setup_nodes()
def run_test(self):
for i in range(len(self.expected)):
self.log.info(f"Starting node {i} with {self.expected[i][0]}")
self.start_node(i)
for i, (args, expected_services) in enumerate(self.expected):
self.log.info(f"Checking listening ports of node {i} with {args}")
pid = self.nodes[i].process.pid
binds = set(get_bind_addrs(pid))
# Remove IPv6 addresses because on some CI environments "::1" is not configured
@ -78,9 +85,7 @@ class BindExtraTest(BitcoinTestFramework):
binds = set(filter(lambda e: len(e[0]) != ipv6_addr_len_bytes, binds))
# Remove RPC ports. They are not relevant for this test.
binds = set(filter(lambda e: e[1] != rpc_port(i), binds))
assert_equal(binds, set(self.expected[i][1]))
self.stop_node(i)
self.log.info(f"Stopped node {i}")
assert_equal(binds, set(expected_services))
if __name__ == '__main__':
BindExtraTest().main()