Skip to content

Commit

Permalink
net: don't extra bind for Tor if binds are restricted
Browse files Browse the repository at this point in the history
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
bitcoin#19991).

This allows disabling binding on the onion port.

Fixes bitcoin#22726
  • Loading branch information
vasild committed Aug 17, 2021
1 parent fdd80b0 commit 1597069
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
2 changes: 2 additions & 0 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1732,6 +1732,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);
Expand Down
32 changes: 25 additions & 7 deletions test/functional/feature_bind_extra.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,20 @@
PORT_MIN,
PORT_RANGE,
assert_equal,
p2p_port,
rpc_port,
)

# From chainparamsbase.cpp:CreateBaseChainParams().
REGTEST_TOR_TARGET_PORT = 18445

class BindExtraTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
# 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 = 4

def setup_network(self):
# Override setup_network() because we want to put the result of
Expand All @@ -43,6 +47,7 @@ def setup_network(self):
if not sys.platform.startswith('linux'):
raise SkipTest("This test can only be run on Linux.")

any_ipv4 = addr_to_hex('0.0.0.0')
loopback_ipv4 = addr_to_hex("127.0.0.1")

# Start custom ports after p2p and rpc ports.
Expand All @@ -69,14 +74,29 @@ def setup_network(self):
)
port += 2

# Node2, no -bind, expected to bind on any + tor target.
self.expected.append(
[
["-listen=1"],
[(any_ipv4, p2p_port(2)), (loopback_ipv4, REGTEST_TOR_TARGET_PORT)]
]
)

# Node3, no -bind=...=onion, thus no extra port for Tor target.
self.expected.append(
[
['-bind=127.0.0.1:{}'.format(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)
self.log.info(f"Checking listening ports of node {i} with {self.expected[i][0]}")
pid = self.nodes[i].process.pid
binds = set(get_bind_addrs(pid))
# Remove IPv6 addresses because on some CI environments "::1" is not configured
Expand All @@ -88,8 +108,6 @@ def run_test(self):
# 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}")

if __name__ == '__main__':
BindExtraTest().main()

0 comments on commit 1597069

Please sign in to comment.