Skip to content

Commit

Permalink
net: only assume all local addresses if listening on any
Browse files Browse the repository at this point in the history
If `-bind=` is provided then we would bind only to a particular address
and should not add all the other addresses of the machine to the list of
local addresses.

Fixes bitcoin/bitcoin#20184 (case 4.)
  • Loading branch information
vasild authored and janus committed Jul 22, 2022
1 parent 6f29ae6 commit d1bc862
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 2 deletions.
8 changes: 6 additions & 2 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1664,8 +1664,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
LogPrintf("nBestHeight = %d\n", chain_active_height);
if (node.peerman) node.peerman->SetBestHeight(chain_active_height);

Discover();

// Map ports with UPnP or NAT-PMP.
StartMapPort(args.GetBoolArg("-upnp", DEFAULT_UPNP), gArgs.GetBoolArg("-natpmp", DEFAULT_NATPMP));

Expand Down Expand Up @@ -1758,6 +1756,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
StartTorControl(onion_service_target);
}

if (connOptions.bind_on_any) {
// Only add all IP addresses of the machine if we would be listening on
// any address - 0.0.0.0 (IPv4) and :: (IPv6).
Discover();
}

for (const auto& net : args.GetArgs("-whitelist")) {
NetWhitelistPermissions subnet;
bilingual_str error;
Expand Down
8 changes: 8 additions & 0 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,15 @@ enum class ConnectionType {

/** Convert ConnectionType enum to a string value */
std::string ConnectionTypeAsString(ConnectionType conn_type);

/**
* Look up IP addresses from all interfaces on the machine and add them to the
* list of local addresses to self-advertise.
* The loopback interface is skipped and only the first address from each
* interface is used.
*/
void Discover();

uint16_t GetListenPort();

enum
Expand Down
78 changes: 78 additions & 0 deletions test/functional/feature_bind_port_discover.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
#!/usr/bin/env python3
# Copyright (c) 2020-2021 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""
Test that -discover does not add all interfaces' addresses if we listen on only some of them
"""

from test_framework.test_framework import BitcoinTestFramework, SkipTest
from test_framework.util import assert_equal

# We need to bind to a routable address for this test to exercise the relevant code
# and also must have another routable address on another interface which must not
# be named "lo" or "lo0".
# To set these routable addresses on the machine, use:
# Linux:
# ifconfig lo:0 1.1.1.1/32 up && ifconfig lo:1 2.2.2.2/32 up # to set up
# ifconfig lo:0 down && ifconfig lo:1 down # to remove it, after the test
# FreeBSD:
# ifconfig em0 1.1.1.1/32 alias && ifconfig wlan0 2.2.2.2/32 alias # to set up
# ifconfig em0 1.1.1.1 -alias && ifconfig wlan0 2.2.2.2 -alias # to remove it, after the test
ADDR1 = '1.1.1.1'
ADDR2 = '2.2.2.2'

BIND_PORT = 31001

class BindPortDiscoverTest(BitcoinTestFramework):
def set_test_params(self):
# Avoid any -bind= on the command line. Force the framework to avoid adding -bind=127.0.0.1.
self.setup_clean_chain = True
self.bind_to_localhost_only = False
self.extra_args = [
['-discover', f'-port={BIND_PORT}'], # bind on any
['-discover', f'-bind={ADDR1}:{BIND_PORT}'],
]
self.num_nodes = len(self.extra_args)

def add_options(self, parser):
parser.add_argument(
"--ihave1111and2222", action='store_true', dest="ihave1111and2222",
help=f"Run the test, assuming {ADDR1} and {ADDR2} are configured on the machine",
default=False)

def skip_test_if_missing_module(self):
if not self.options.ihave1111and2222:
raise SkipTest(
f"To run this test make sure that {ADDR1} and {ADDR2} (routable addresses) are "
"assigned to the interfaces on this machine and rerun with --ihave1111and2222")

def run_test(self):
self.log.info(
"Test that if -bind= is not passed then all addresses are "
"added to localaddresses")
found_addr1 = False
found_addr2 = False
for local in self.nodes[0].getnetworkinfo()['localaddresses']:
if local['address'] == ADDR1:
found_addr1 = True
assert_equal(local['port'], BIND_PORT)
if local['address'] == ADDR2:
found_addr2 = True
assert_equal(local['port'], BIND_PORT)
assert found_addr1
assert found_addr2

self.log.info(
"Test that if -bind= is passed then only that address is "
"added to localaddresses")
found_addr1 = False
for local in self.nodes[1].getnetworkinfo()['localaddresses']:
if local['address'] == ADDR1:
found_addr1 = True
assert_equal(local['port'], BIND_PORT)
assert local['address'] != ADDR2
assert found_addr1

if __name__ == '__main__':
BindPortDiscoverTest().main()
1 change: 1 addition & 0 deletions test/functional/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@
'feature_loadblock.py',
'p2p_dos_header_tree.py',
'p2p_add_connections.py',
'feature_bind_port_discover.py',
'p2p_unrequested_blocks.py',
'p2p_blockfilters.py',
'p2p_message_capture.py',
Expand Down

0 comments on commit d1bc862

Please sign in to comment.