Skip to content

Commit f619f8f

Browse files
kwvgkwvg
committed
merge bitcoin#20234: don't bind on 0.0.0.0 if binds are restricted to Tor
1 parent 1698336 commit f619f8f

File tree

5 files changed

+129
-29
lines changed

5 files changed

+129
-29
lines changed

‎src/init.cpp‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2501,25 +2501,34 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
25012501
return InitError(ResolveErrMsg("bind", bind_arg));
25022502
}
25032503

2504-
if (connOptions.onion_binds.empty()) {
2505-
connOptions.onion_binds.push_back(DefaultOnionServiceTarget());
2506-
}
2507-
2508-
if (args.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION)) {
2509-
const auto bind_addr = connOptions.onion_binds.front();
2510-
if (connOptions.onion_binds.size() > 1) {
2511-
InitWarning(strprintf(_("More than one onion bind address is provided. Using %s for the automatically created Tor onion service."), bind_addr.ToStringIPPort()));
2512-
}
2513-
StartTorControl(bind_addr);
2514-
}
2515-
25162504
for (const std::string& strBind : args.GetArgs("-whitebind")) {
25172505
NetWhitebindPermissions whitebind;
25182506
bilingual_str error;
25192507
if (!NetWhitebindPermissions::TryParse(strBind, whitebind, error)) return InitError(error);
25202508
connOptions.vWhiteBinds.push_back(whitebind);
25212509
}
25222510

2511+
// If the user did not specify -bind= or -whitebind= then we bind
2512+
// on any address - 0.0.0.0 (IPv4) and :: (IPv6).
2513+
connOptions.bind_on_any = args.GetArgs("-bind").empty() && args.GetArgs("-whitebind").empty();
2514+
2515+
CService onion_service_target;
2516+
if (!connOptions.onion_binds.empty()) {
2517+
onion_service_target = connOptions.onion_binds.front();
2518+
} else {
2519+
onion_service_target = DefaultOnionServiceTarget();
2520+
connOptions.onion_binds.push_back(onion_service_target);
2521+
}
2522+
2523+
if (args.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION)) {
2524+
if (connOptions.onion_binds.size() > 1) {
2525+
InitWarning(strprintf(_("More than one onion bind address is provided. Using %s "
2526+
"for the automatically created Tor onion service."),
2527+
onion_service_target.ToStringIPPort()));
2528+
}
2529+
StartTorControl(onion_service_target);
2530+
}
2531+
25232532
for (const auto& net : args.GetArgs("-whitelist")) {
25242533
NetWhitelistPermissions subnet;
25252534
bilingual_str error;

‎src/net.cpp‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3322,30 +3322,25 @@ bool CConnman::Bind(const CService &addr, unsigned int flags, NetPermissionFlags
33223322
return true;
33233323
}
33243324

3325-
bool CConnman::InitBinds(
3326-
const std::vector<CService>& binds,
3327-
const std::vector<NetWhitebindPermissions>& whiteBinds,
3328-
const std::vector<CService>& onion_binds)
3325+
bool CConnman::InitBinds(const Options& options)
33293326
{
33303327
bool fBound = false;
3331-
for (const auto& addrBind : binds) {
3328+
for (const auto& addrBind : options.vBinds) {
33323329
fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR), NetPermissionFlags::None);
33333330
}
3334-
for (const auto& addrBind : whiteBinds) {
3331+
for (const auto& addrBind : options.vWhiteBinds) {
33353332
fBound |= Bind(addrBind.m_service, (BF_EXPLICIT | BF_REPORT_ERROR), addrBind.m_flags);
33363333
}
3337-
if (binds.empty() && whiteBinds.empty()) {
3334+
for (const auto& addr_bind : options.onion_binds) {
3335+
fBound |= Bind(addr_bind, BF_EXPLICIT | BF_DONT_ADVERTISE, NetPermissionFlags::None);
3336+
}
3337+
if (options.bind_on_any) {
33383338
struct in_addr inaddr_any;
33393339
inaddr_any.s_addr = htonl(INADDR_ANY);
33403340
struct in6_addr inaddr6_any = IN6ADDR_ANY_INIT;
33413341
fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE, NetPermissionFlags::None);
33423342
fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE, NetPermissionFlags::None);
33433343
}
3344-
3345-
for (const auto& addr_bind : onion_binds) {
3346-
fBound |= Bind(addr_bind, BF_EXPLICIT | BF_DONT_ADVERTISE, NetPermissionFlags::None);
3347-
}
3348-
33493344
return fBound;
33503345
}
33513346

@@ -3364,7 +3359,7 @@ bool CConnman::Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_met
33643359
}
33653360
}
33663361

3367-
if (fListen && !InitBinds(connOptions.vBinds, connOptions.vWhiteBinds, connOptions.onion_binds)) {
3362+
if (fListen && !InitBinds(connOptions)) {
33683363
if (clientInterface) {
33693364
clientInterface->ThreadSafeMessageBox(
33703365
_("Failed to listen on any port. Use -listen=0 if you want this."),

‎src/net.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,9 @@ friend class CNode;
851851
std::vector<NetWhitebindPermissions> vWhiteBinds;
852852
std::vector<CService> vBinds;
853853
std::vector<CService> onion_binds;
854+
/// True if the user did not specify -bind= or -whitebind= and thus
855+
/// we should bind on `0.0.0.0` (IPv4) and `::` (IPv6).
856+
bool bind_on_any;
854857
bool m_use_addrman_outgoing = true;
855858
std::vector<std::string> m_specified_outgoing;
856859
std::vector<std::string> m_added_nodes;
@@ -1223,10 +1226,7 @@ friend class CNode;
12231226

12241227
bool BindListenPort(const CService& bindAddr, bilingual_str& strError, NetPermissionFlags permissions);
12251228
bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions);
1226-
bool InitBinds(
1227-
const std::vector<CService>& binds,
1228-
const std::vector<NetWhitebindPermissions>& whiteBinds,
1229-
const std::vector<CService>& onion_binds);
1229+
bool InitBinds(const Options& options);
12301230

12311231
void ThreadOpenAddedConnections()
12321232
EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_unused_i2p_sessions_mutex, !mutexMsgProc);

‎test/functional/feature_bind_extra.py‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2014-2021 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""
6+
Test starting bitcoind with -bind and/or -bind=...=onion and confirm
7+
that bind happens on the expected ports.
8+
"""
9+
10+
import sys
11+
12+
from test_framework.netutil import (
13+
addr_to_hex,
14+
get_bind_addrs,
15+
)
16+
from test_framework.test_framework import (
17+
BitcoinTestFramework,
18+
SkipTest,
19+
)
20+
from test_framework.util import (
21+
PORT_MIN,
22+
PORT_RANGE,
23+
assert_equal,
24+
rpc_port,
25+
)
26+
27+
class BindExtraTest(BitcoinTestFramework):
28+
def set_test_params(self):
29+
self.setup_clean_chain = True
30+
# Avoid any -bind= on the command line. Force the framework to avoid
31+
# adding -bind=127.0.0.1.
32+
self.bind_to_localhost_only = False
33+
self.num_nodes = 2
34+
35+
def setup_network(self):
36+
# Override setup_network() because we want to put the result of
37+
# p2p_port() in self.extra_args[], before the nodes are started.
38+
# p2p_port() is not usable in set_test_params() because PortSeed.n is
39+
# not set at that time.
40+
41+
# Due to OS-specific network stats queries, we only run on Linux.
42+
self.log.info("Checking for Linux")
43+
if not sys.platform.startswith('linux'):
44+
raise SkipTest("This test can only be run on Linux.")
45+
46+
loopback_ipv4 = addr_to_hex("127.0.0.1")
47+
48+
# Start custom ports after p2p and rpc ports.
49+
port = PORT_MIN + 2 * PORT_RANGE
50+
51+
# Array of tuples [command line arguments, expected bind addresses].
52+
self.expected = []
53+
54+
# Node0, no normal -bind=... with -bind=...=onion, thus only the tor target.
55+
self.expected.append(
56+
[
57+
[f"-bind=127.0.0.1:{port}=onion"],
58+
[(loopback_ipv4, port)]
59+
],
60+
)
61+
port += 1
62+
63+
# Node1, both -bind=... and -bind=...=onion.
64+
self.expected.append(
65+
[
66+
[f"-bind=127.0.0.1:{port}", f"-bind=127.0.0.1:{port + 1}=onion"],
67+
[(loopback_ipv4, port), (loopback_ipv4, port + 1)]
68+
],
69+
)
70+
port += 2
71+
72+
self.extra_args = list(map(lambda e: e[0], self.expected))
73+
self.add_nodes(self.num_nodes, self.extra_args)
74+
# Don't start the nodes, as some of them would collide trying to bind on the same port.
75+
76+
def run_test(self):
77+
for i in range(len(self.expected)):
78+
self.log.info(f"Starting node {i} with {self.expected[i][0]}")
79+
self.start_node(i)
80+
pid = self.nodes[i].process.pid
81+
binds = set(get_bind_addrs(pid))
82+
# Remove IPv6 addresses because on some CI environments "::1" is not configured
83+
# on the system (so our test_ipv6_local() would return False), but it is
84+
# possible to bind on "::". This makes it unpredictable whether to expect
85+
# that bitcoind has bound on "::1" (for RPC) and "::" (for P2P).
86+
ipv6_addr_len_bytes = 32
87+
binds = set(filter(lambda e: len(e[0]) != ipv6_addr_len_bytes, binds))
88+
# Remove RPC ports. They are not relevant for this test.
89+
binds = set(filter(lambda e: e[1] != rpc_port(i), binds))
90+
assert_equal(binds, set(self.expected[i][1]))
91+
self.stop_node(i)
92+
self.log.info(f"Stopped node {i}")
93+
94+
if __name__ == '__main__':
95+
BindExtraTest().main()

‎test/functional/test_runner.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@
157157
'interface_zmq.py',
158158
'rpc_invalid_address_message.py',
159159
'interface_bitcoin_cli.py',
160+
'feature_bind_extra.py',
160161
'mempool_resurrect.py',
161162
'wallet_txn_doublespend.py --mineblock',
162163
'tool_wallet.py --legacy-wallet',

0 commit comments

Comments
 (0)