merge bitcoin#20196: fix GetListenPort() to derive the proper port

continuation of 24205d94fe from dash#5982

includes:
- 0cfc0cd32239d3c08d2121e028b297022450b320
- 7d64ea4a01920bb55bc6de0de6766712ec792a11
This commit is contained in:
Kittywhiskers Van Gogh 2024-06-12 16:22:21 +00:00
parent c443cf4825
commit a6aa3735be
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
8 changed files with 400 additions and 10 deletions

View File

@ -2477,8 +2477,6 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
LogPrintf("::ChainActive().Height() = %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), args.GetBoolArg("-natpmp", DEFAULT_NATPMP));
@ -2495,15 +2493,18 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
connOptions.nSendBufferMaxSize = 1000 * args.GetArg("-maxsendbuffer", DEFAULT_MAXSENDBUFFER);
connOptions.nReceiveFloodSize = 1000 * args.GetArg("-maxreceivebuffer", DEFAULT_MAXRECEIVEBUFFER);
connOptions.m_added_nodes = args.GetArgs("-addnode");
connOptions.nMaxOutboundLimit = 1024 * 1024 * args.GetArg("-maxuploadtarget", DEFAULT_MAX_UPLOAD_TARGET);
connOptions.m_peer_connect_timeout = peer_connect_timeout;
// Port to bind to if `-bind=addr` is provided without a `:port` suffix.
const uint16_t default_bind_port =
static_cast<uint16_t>(args.GetArg("-port", Params().GetDefaultPort()));
for (const std::string& bind_arg : args.GetArgs("-bind")) {
CService bind_addr;
const size_t index = bind_arg.rfind('=');
if (index == std::string::npos) {
if (Lookup(bind_arg, bind_addr, GetListenPort(), false)) {
if (Lookup(bind_arg, bind_addr, default_bind_port, /*fAllowLookup=*/false)) {
connOptions.vBinds.push_back(bind_addr);
continue;
}
@ -2548,6 +2549,12 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
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;

View File

@ -151,6 +151,31 @@ void CConnman::AddAddrFetch(const std::string& strDest)
uint16_t GetListenPort()
{
// If -bind= is provided with ":port" part, use that (first one if multiple are provided).
for (const std::string& bind_arg : gArgs.GetArgs("-bind")) {
CService bind_addr;
constexpr uint16_t dummy_port = 0;
if (Lookup(bind_arg, bind_addr, dummy_port, /*fAllowLookup=*/false)) {
if (bind_addr.GetPort() != dummy_port) {
return bind_addr.GetPort();
}
}
}
// Otherwise, if -whitebind= without NetPermissionFlags::NoBan is provided, use that
// (-whitebind= is required to have ":port").
for (const std::string& whitebind_arg : gArgs.GetArgs("-whitebind")) {
NetWhitebindPermissions whitebind;
bilingual_str error;
if (NetWhitebindPermissions::TryParse(whitebind_arg, whitebind, error)) {
if (!NetPermissions::HasFlag(whitebind.m_flags, NetPermissionFlags::NoBan)) {
return whitebind.m_service.GetPort();
}
}
}
// Otherwise, if -port= is provided, use that. Otherwise use the default port.
return static_cast<uint16_t>(gArgs.GetArg("-port", Params().GetDefaultPort()));
}
@ -246,7 +271,17 @@ std::optional<CAddress> GetLocalAddrForPeer(CNode *pnode)
if (IsPeerAddrLocalGood(pnode) && (!addrLocal.IsRoutable() ||
rng.randbits((GetnScore(addrLocal) > LOCAL_MANUAL) ? 3 : 1) == 0))
{
addrLocal.SetIP(pnode->GetAddrLocal());
if (pnode->IsInboundConn()) {
// For inbound connections, assume both the address and the port
// as seen from the peer.
addrLocal = CAddress{pnode->GetAddrLocal(), addrLocal.nServices};
} else {
// For outbound connections, assume just the address as seen from
// the peer and leave the port in `addrLocal` as returned by
// `GetLocalAddress()` above. The peer has no way to observe our
// listening port when we have initiated the connection.
addrLocal.SetIP(pnode->GetAddrLocal());
}
}
if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false))
{

View File

@ -215,7 +215,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

View File

@ -3444,6 +3444,10 @@ void PeerManagerImpl::ProcessMessage(
LogPrint(BCLog::NET, "ProcessMessages: advertising address %s\n", addr.ToString());
PushAddress(*peer, addr, insecure_rand);
} else if (IsPeerAddrLocalGood(&pfrom)) {
// Override just the address with whatever the peer sees us as.
// Leave the port in addr as it was returned by GetLocalAddress()
// above, as this is an outbound connection and the peer cannot
// observe our listening port.
addr.SetIP(addrMe);
LogPrint(BCLog::NET, "ProcessMessages: advertising address %s\n", addr.ToString());
PushAddress(*peer, addr, insecure_rand);

View File

@ -6,15 +6,21 @@
#include <chainparams.h>
#include <clientversion.h>
#include <compat.h>
#include <net.h>
#include <net_processing.h>
#include <netaddress.h>
#include <netbase.h>
#include <netmessagemaker.h>
#include <serialize.h>
#include <span.h>
#include <streams.h>
#include <test/util/validation.h>
#include <timedata.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <util/system.h>
#include <validation.h>
#include <version.h>
#include <boost/test/unit_test.hpp>
@ -28,7 +34,7 @@
using namespace std::literals;
BOOST_FIXTURE_TEST_SUITE(net_tests, BasicTestingSetup)
BOOST_FIXTURE_TEST_SUITE(net_tests, RegTestingSetup)
BOOST_AUTO_TEST_CASE(cnode_listen_port)
{
@ -608,15 +614,15 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
// set up local addresses; all that's necessary to reproduce the bug is
// that a normal IPv4 address is among the entries, but if this address is
// !IsRoutable the undefined behavior is easier to trigger deterministically
in_addr raw_addr;
raw_addr.s_addr = htonl(0x7f000001);
const CNetAddr mapLocalHost_entry = CNetAddr(raw_addr);
{
LOCK(g_maplocalhost_mutex);
in_addr ipv4AddrLocal;
ipv4AddrLocal.s_addr = 0x0100007f;
CNetAddr addr = CNetAddr(ipv4AddrLocal);
LocalServiceInfo lsi;
lsi.nScore = 23;
lsi.nPort = 42;
mapLocalHost[addr] = lsi;
mapLocalHost[mapLocalHost_entry] = lsi;
}
// create a peer with an IPv4 address
@ -647,8 +653,79 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
// suppress no-checks-run warning; if this test fails, it's by triggering a sanitizer
BOOST_CHECK(1);
// Cleanup, so that we don't confuse other tests.
{
LOCK(g_maplocalhost_mutex);
mapLocalHost.erase(mapLocalHost_entry);
}
}
BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port)
{
// Test that GetLocalAddrForPeer() properly selects the address to self-advertise:
//
// 1. GetLocalAddrForPeer() calls GetLocalAddress() which returns an address that is
// not routable.
// 2. GetLocalAddrForPeer() overrides the address with whatever the peer has told us
// he sees us as.
// 2.1. For inbound connections we must override both the address and the port.
// 2.2. For outbound connections we must override only the address.
// Pretend that we bound to this port.
const uint16_t bind_port = 20001;
m_node.args->ForceSetArg("-bind", strprintf("3.4.5.6:%u", bind_port));
// Our address:port as seen from the peer, completely different from the above.
in_addr peer_us_addr;
peer_us_addr.s_addr = htonl(0x02030405);
const CAddress peer_us{CService{peer_us_addr, 20002}, NODE_NETWORK};
// Create a peer with a routable IPv4 address (outbound).
in_addr peer_out_in_addr;
peer_out_in_addr.s_addr = htonl(0x01020304);
CNode peer_out{/*id=*/0,
/*nLocalServicesIn=*/NODE_NETWORK,
/*sock=*/nullptr,
/*addrIn=*/CAddress{CService{peer_out_in_addr, 8333}, NODE_NETWORK},
/*nKeyedNetGroupIn=*/0,
/*nLocalHostNonceIn=*/0,
/*addrBindIn=*/CAddress{},
/*addrNameIn=*/std::string{},
/*conn_type_in=*/ConnectionType::OUTBOUND_FULL_RELAY,
/*inbound_onion=*/false};
peer_out.fSuccessfullyConnected = true;
peer_out.SetAddrLocal(peer_us);
// Without the fix peer_us:8333 is chosen instead of the proper peer_us:bind_port.
auto chosen_local_addr = GetLocalAddrForPeer(&peer_out);
BOOST_REQUIRE(chosen_local_addr);
const CService expected{peer_us_addr, bind_port};
BOOST_CHECK(*chosen_local_addr == expected);
// Create a peer with a routable IPv4 address (inbound).
in_addr peer_in_in_addr;
peer_in_in_addr.s_addr = htonl(0x05060708);
CNode peer_in{/*id=*/0,
/*nLocalServicesIn=*/NODE_NETWORK,
/*sock=*/nullptr,
/*addrIn=*/CAddress{CService{peer_in_in_addr, 8333}, NODE_NETWORK},
/*nKeyedNetGroupIn=*/0,
/*nLocalHostNonceIn=*/0,
/*addrBindIn=*/CAddress{},
/*addrNameIn=*/std::string{},
/*conn_type_in=*/ConnectionType::INBOUND,
/*inbound_onion=*/false};
peer_in.fSuccessfullyConnected = true;
peer_in.SetAddrLocal(peer_us);
// Without the fix peer_us:8333 is chosen instead of the proper peer_us:peer_us.GetPort().
chosen_local_addr = GetLocalAddrForPeer(&peer_in);
BOOST_REQUIRE(chosen_local_addr);
BOOST_CHECK(*chosen_local_addr == peer_us);
m_node.args->ForceSetArg("-bind", "");
}
BOOST_AUTO_TEST_CASE(LimitedAndReachable_Network)
{
@ -734,4 +811,108 @@ BOOST_AUTO_TEST_CASE(LocalAddress_BasicLifecycle)
BOOST_CHECK(!IsLocal(addr));
}
BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
{
// Tests the following scenario:
// * -bind=3.4.5.6:20001 is specified
// * we make an outbound connection to a peer
// * the peer reports he sees us as 2.3.4.5:20002 in the version message
// (20002 is a random port assigned by our OS for the outgoing TCP connection,
// we cannot accept connections to it)
// * we should self-advertise to that peer as 2.3.4.5:20001
// Pretend that we bound to this port.
const uint16_t bind_port = 20001;
m_node.args->ForceSetArg("-bind", strprintf("3.4.5.6:%u", bind_port));
m_node.args->ForceSetArg("-capturemessages", "1");
// Our address:port as seen from the peer - 2.3.4.5:20002 (different from the above).
in_addr peer_us_addr;
peer_us_addr.s_addr = htonl(0x02030405);
const CService peer_us{peer_us_addr, 20002};
// Create a peer with a routable IPv4 address.
in_addr peer_in_addr;
peer_in_addr.s_addr = htonl(0x01020304);
CNode peer{/*id=*/0,
/*nLocalServicesIn=*/NODE_NETWORK,
/*sock=*/nullptr,
/*addrIn=*/CAddress{CService{peer_in_addr, 8333}, NODE_NETWORK},
/*nKeyedNetGroupIn=*/0,
/*nLocalHostNonceIn=*/0,
/*addrBindIn=*/CAddress{},
/*addrNameIn=*/std::string{},
/*conn_type_in=*/ConnectionType::OUTBOUND_FULL_RELAY,
/*inbound_onion=*/false};
const uint64_t services{NODE_NETWORK};
const int64_t time{0};
const CNetMsgMaker msg_maker{PROTOCOL_VERSION};
// Force CChainState::IsInitialBlockDownload() to return false.
// Otherwise PushAddress() isn't called by PeerManager::ProcessMessage().
TestChainState& chainstate =
*static_cast<TestChainState*>(&m_node.chainman->ActiveChainstate());
chainstate.JumpOutOfIbd();
m_node.peerman->InitializeNode(&peer);
std::atomic<bool> interrupt_dummy{false};
std::chrono::microseconds time_received_dummy{0};
const auto msg_version =
msg_maker.Make(NetMsgType::VERSION, PROTOCOL_VERSION, services, time, services, peer_us);
CDataStream msg_version_stream{msg_version.data, SER_NETWORK, PROTOCOL_VERSION};
m_node.peerman->ProcessMessage(
peer, NetMsgType::VERSION, msg_version_stream, time_received_dummy, interrupt_dummy);
const auto msg_verack = msg_maker.Make(NetMsgType::VERACK);
CDataStream msg_verack_stream{msg_verack.data, SER_NETWORK, PROTOCOL_VERSION};
// Will set peer.fSuccessfullyConnected to true (necessary in SendMessages()).
m_node.peerman->ProcessMessage(
peer, NetMsgType::VERACK, msg_verack_stream, time_received_dummy, interrupt_dummy);
// Ensure that peer_us_addr:bind_port is sent to the peer.
const CService expected{peer_us_addr, bind_port};
bool sent{false};
const auto CaptureMessageOrig = CaptureMessage;
CaptureMessage = [&sent, &expected](const CAddress& addr,
const std::string& msg_type,
Span<const unsigned char> data,
bool is_incoming) -> void {
if (!is_incoming && msg_type == "addr") {
CDataStream s(data, SER_NETWORK, PROTOCOL_VERSION);
std::vector<CAddress> addresses;
s >> addresses;
for (const auto& addr : addresses) {
if (addr == expected) {
sent = true;
return;
}
}
}
};
{
LOCK(peer.cs_sendProcessing);
m_node.peerman->SendMessages(&peer);
}
BOOST_CHECK(sent);
CaptureMessage = CaptureMessageOrig;
chainstate.ResetIbd();
m_node.args->ForceSetArg("-capturemessages", "0");
m_node.args->ForceSetArg("-bind", "");
// PeerManager::ProcessMessage() calls AddTimeData() which changes the internal state
// in timedata.cpp and later confuses the test "timedata_tests/addtimedata". Thus reset
// that state as it was before our test was run.
TestOnlyResetTimeData();
}
BOOST_AUTO_TEST_SUITE_END()

View File

@ -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()

View File

@ -0,0 +1,75 @@
#!/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 the proper port is used for -externalip=
"""
from test_framework.test_framework import BitcoinTestFramework, SkipTest
from test_framework.util import assert_equal, p2p_port
# We need to bind to a routable address for this test to exercise the relevant code.
# To set a routable address on the machine use:
# Linux:
# ifconfig lo:0 1.1.1.1/32 up # to set up
# ifconfig lo:0 down # to remove it, after the test
# FreeBSD:
# ifconfig lo0 1.1.1.1/32 alias # to set up
# ifconfig lo0 1.1.1.1 -alias # to remove it, after the test
ADDR = '1.1.1.1'
# array of tuples [arguments, expected port in localaddresses]
EXPECTED = [
[['-externalip=2.2.2.2', '-port=30001'], 30001],
[['-externalip=2.2.2.2', '-port=30002', f'-bind={ADDR}'], 30002],
[['-externalip=2.2.2.2', f'-bind={ADDR}'], 'default_p2p_port'],
[['-externalip=2.2.2.2', '-port=30003', f'-bind={ADDR}:30004'], 30004],
[['-externalip=2.2.2.2', f'-bind={ADDR}:30005'], 30005],
[['-externalip=2.2.2.2:30006', '-port=30007'], 30006],
[['-externalip=2.2.2.2:30008', '-port=30009', f'-bind={ADDR}'], 30008],
[['-externalip=2.2.2.2:30010', f'-bind={ADDR}'], 30010],
[['-externalip=2.2.2.2:30011', '-port=30012', f'-bind={ADDR}:30013'], 30011],
[['-externalip=2.2.2.2:30014', f'-bind={ADDR}:30015'], 30014],
[['-externalip=2.2.2.2', '-port=30016', f'-bind={ADDR}:30017',
f'-whitebind={ADDR}:30018'], 30017],
[['-externalip=2.2.2.2', '-port=30019',
f'-whitebind={ADDR}:30020'], 30020],
]
class BindPortExternalIPTest(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.num_nodes = len(EXPECTED)
self.extra_args = list(map(lambda e: e[0], EXPECTED))
def add_options(self, parser):
parser.add_argument(
"--ihave1111", action='store_true', dest="ihave1111",
help=f"Run the test, assuming {ADDR} is configured on the machine",
default=False)
def skip_test_if_missing_module(self):
if not self.options.ihave1111:
raise SkipTest(
f"To run this test make sure that {ADDR} (a routable address) is assigned "
"to one of the interfaces on this machine and rerun with --ihave1111")
def run_test(self):
self.log.info("Test the proper port is used for -externalip=")
for i in range(len(EXPECTED)):
expected_port = EXPECTED[i][1]
if expected_port == 'default_p2p_port':
expected_port = p2p_port(i)
found = False
for local in self.nodes[i].getnetworkinfo()['localaddresses']:
if local['address'] == '2.2.2.2':
assert_equal(local['port'], expected_port)
found = True
break
assert found
if __name__ == '__main__':
BindPortExternalIPTest().main()

View File

@ -277,6 +277,7 @@ BASE_SCRIPTS = [
'p2p_connect_to_devnet.py',
'feature_sporks.py',
'rpc_getblockstats.py',
'feature_bind_port_externalip.py',
'wallet_encryption.py --legacy-wallet',
'wallet_encryption.py --descriptors',
'wallet_upgradetohd.py --legacy-wallet',
@ -316,6 +317,7 @@ BASE_SCRIPTS = [
'feature_loadblock.py',
'p2p_dos_header_tree.py',
'p2p_add_connections.py',
'feature_bind_port_discover.py',
'p2p_blockfilters.py',
'p2p_message_capture.py',
'feature_addrman.py',