From 7d64ea4a01920bb55bc6de0de6766712ec792a11 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Mon, 19 Oct 2020 15:32:54 +0200 Subject: [PATCH] net: only assume all local addresses if listening on any 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 https://github.com/bitcoin/bitcoin/issues/20184 (case 4.) --- src/init.cpp | 8 +- src/net.h | 8 ++ test/functional/feature_bind_port_discover.py | 78 +++++++++++++++++++ test/functional/test_runner.py | 1 + 4 files changed, 93 insertions(+), 2 deletions(-) create mode 100755 test/functional/feature_bind_port_discover.py diff --git a/src/init.cpp b/src/init.cpp index 05a8437043..a3d53c3fae 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1668,8 +1668,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)); @@ -1762,6 +1760,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; diff --git a/src/net.h b/src/net.h index ddc1d3dd7c..a38310938b 100644 --- a/src/net.h +++ b/src/net.h @@ -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 diff --git a/test/functional/feature_bind_port_discover.py b/test/functional/feature_bind_port_discover.py new file mode 100755 index 0000000000..6e07f2f16c --- /dev/null +++ b/test/functional/feature_bind_port_discover.py @@ -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() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 33afbfef6f..b0f24e3b97 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -292,6 +292,7 @@ BASE_SCRIPTS = [ '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',