mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-01-09 11:27:28 -03:00
Merge bitcoin/bitcoin#20196: net: fix GetListenPort() to derive the proper port
7d64ea4a01
net: only assume all local addresses if listening on any (Vasil Dimov)0cfc0cd322
net: fix GetListenPort() to derive the proper port (Vasil Dimov)f98cdcb357
net: pass Span by value to CaptureMessage() (Vasil Dimov)3cb9d9c861
net: make CaptureMessage() mockable (Vasil Dimov)43868ba416
timedata: rename variables to match the coding style (Vasil Dimov)60da1eaa11
timedata: make it possible to reset the state (Vasil Dimov) Pull request description: `GetListenPort()` uses a simple logic: "if `-port=P` is given, then we must be listening on `P`, otherwise we must be listening on `8333`". This is however not true if `-bind=` has been provided with `:port` part or if `-whitebind=` has been provided. Thus, extend `GetListenPort()` to return the port from `-bind=` or `-whitebind=`, if any. Also, 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 ACKs for top commit: sipa: utACK7d64ea4a01
. I didn't review the tests in detail. jonatack: re-ACK7d64ea4a01
per `git range-diff08bcfa27
35ec977 7d64ea4`, changes are rebase-only, light re-review, re-ran the new tests locally Tree-SHA512: 45135ab9c0ec3cc8c83e3b3e58a1c1f77eaeaba00618d54f1010db1d23d6db7d9c0dc7807e72ebc34e8b2d0e91f1e0d0e9239d13b90f1bdce8be84459e7837f0
This commit is contained in:
commit
30308cc380
12 changed files with 454 additions and 27 deletions
14
src/init.cpp
14
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));
|
||||
|
||||
|
@ -1689,6 +1687,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
|
|||
connOptions.nMaxOutboundLimit = *opt_max_upload;
|
||||
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.GetIntArg("-port", Params().GetDefaultPort()));
|
||||
|
||||
const auto BadPortWarning = [](const char* prefix, uint16_t port) {
|
||||
return strprintf(_("%s request to listen on port %u. This port is considered \"bad\" and "
|
||||
"thus it is unlikely that any Bitcoin Core peers connect to it. See "
|
||||
|
@ -1701,7 +1703,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
|
|||
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);
|
||||
if (IsBadPort(bind_addr.GetPort())) {
|
||||
InitWarning(BadPortWarning("-bind", bind_addr.GetPort()));
|
||||
|
@ -1758,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;
|
||||
|
|
48
src/net.cpp
48
src/net.cpp
|
@ -126,6 +126,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.GetIntArg("-port", Params().GetDefaultPort()));
|
||||
}
|
||||
|
||||
|
@ -221,7 +246,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))
|
||||
{
|
||||
|
@ -3085,7 +3120,10 @@ uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& ad) const
|
|||
return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup.data(), vchNetGroup.size()).Finalize();
|
||||
}
|
||||
|
||||
void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Span<const unsigned char>& data, bool is_incoming)
|
||||
void CaptureMessageToFile(const CAddress& addr,
|
||||
const std::string& msg_type,
|
||||
Span<const unsigned char> data,
|
||||
bool is_incoming)
|
||||
{
|
||||
// Note: This function captures the message at the time of processing,
|
||||
// not at socket receive/send time.
|
||||
|
@ -3112,3 +3150,9 @@ void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Spa
|
|||
ser_writedata32(f, size);
|
||||
f.write(AsBytes(data));
|
||||
}
|
||||
|
||||
std::function<void(const CAddress& addr,
|
||||
const std::string& msg_type,
|
||||
Span<const unsigned char> data,
|
||||
bool is_incoming)>
|
||||
CaptureMessage = CaptureMessageToFile;
|
||||
|
|
21
src/net.h
21
src/net.h
|
@ -31,6 +31,7 @@
|
|||
#include <condition_variable>
|
||||
#include <cstdint>
|
||||
#include <deque>
|
||||
#include <functional>
|
||||
#include <map>
|
||||
#include <memory>
|
||||
#include <optional>
|
||||
|
@ -182,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
|
||||
|
@ -1272,7 +1281,17 @@ private:
|
|||
};
|
||||
|
||||
/** Dump binary message to file, with timestamp */
|
||||
void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Span<const unsigned char>& data, bool is_incoming);
|
||||
void CaptureMessageToFile(const CAddress& addr,
|
||||
const std::string& msg_type,
|
||||
Span<const unsigned char> data,
|
||||
bool is_incoming);
|
||||
|
||||
/** Defaults to `CaptureMessageToFile()`, but can be overridden by unit tests. */
|
||||
extern std::function<void(const CAddress& addr,
|
||||
const std::string& msg_type,
|
||||
Span<const unsigned char> data,
|
||||
bool is_incoming)>
|
||||
CaptureMessage;
|
||||
|
||||
struct NodeEvictionCandidate
|
||||
{
|
||||
|
|
|
@ -2711,6 +2711,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
|
|||
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);
|
||||
|
|
|
@ -4,17 +4,23 @@
|
|||
|
||||
#include <chainparams.h>
|
||||
#include <clientversion.h>
|
||||
#include <compat.h>
|
||||
#include <cstdint>
|
||||
#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/setup_common.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>
|
||||
|
@ -27,7 +33,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)
|
||||
{
|
||||
|
@ -607,15 +613,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
|
||||
|
@ -646,8 +652,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)
|
||||
{
|
||||
|
@ -728,4 +805,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 | NODE_WITNESS};
|
||||
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()
|
||||
|
|
|
@ -96,9 +96,10 @@ BOOST_AUTO_TEST_CASE(addtimedata)
|
|||
// not to fix this because it prevents possible attacks. See the comment in AddTimeData() or issue #4521
|
||||
// for a more detailed explanation.
|
||||
MultiAddTimeData(2, 100); // filter median is 100 now, but nTimeOffset will not change
|
||||
// We want this test to end with nTimeOffset==0, otherwise subsequent tests of the suite will fail.
|
||||
BOOST_CHECK_EQUAL(GetTimeOffset(), 0);
|
||||
|
||||
// We want this test to end with nTimeOffset==0, otherwise subsequent tests of the suite will fail.
|
||||
TestOnlyResetTimeData();
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
||||
|
|
|
@ -39,29 +39,31 @@ int64_t GetAdjustedTime()
|
|||
|
||||
#define BITCOIN_TIMEDATA_MAX_SAMPLES 200
|
||||
|
||||
static std::set<CNetAddr> g_sources;
|
||||
static CMedianFilter<int64_t> g_time_offsets{BITCOIN_TIMEDATA_MAX_SAMPLES, 0};
|
||||
static bool g_warning_emitted;
|
||||
|
||||
void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
|
||||
{
|
||||
LOCK(g_timeoffset_mutex);
|
||||
// Ignore duplicates
|
||||
static std::set<CNetAddr> setKnown;
|
||||
if (setKnown.size() == BITCOIN_TIMEDATA_MAX_SAMPLES)
|
||||
if (g_sources.size() == BITCOIN_TIMEDATA_MAX_SAMPLES)
|
||||
return;
|
||||
if (!setKnown.insert(ip).second)
|
||||
if (!g_sources.insert(ip).second)
|
||||
return;
|
||||
|
||||
// Add data
|
||||
static CMedianFilter<int64_t> vTimeOffsets(BITCOIN_TIMEDATA_MAX_SAMPLES, 0);
|
||||
vTimeOffsets.input(nOffsetSample);
|
||||
LogPrint(BCLog::NET, "added time data, samples %d, offset %+d (%+d minutes)\n", vTimeOffsets.size(), nOffsetSample, nOffsetSample / 60);
|
||||
g_time_offsets.input(nOffsetSample);
|
||||
LogPrint(BCLog::NET, "added time data, samples %d, offset %+d (%+d minutes)\n", g_time_offsets.size(), nOffsetSample, nOffsetSample / 60);
|
||||
|
||||
// There is a known issue here (see issue #4521):
|
||||
//
|
||||
// - The structure vTimeOffsets contains up to 200 elements, after which
|
||||
// - The structure g_time_offsets contains up to 200 elements, after which
|
||||
// any new element added to it will not increase its size, replacing the
|
||||
// oldest element.
|
||||
//
|
||||
// - The condition to update nTimeOffset includes checking whether the
|
||||
// number of elements in vTimeOffsets is odd, which will never happen after
|
||||
// number of elements in g_time_offsets is odd, which will never happen after
|
||||
// there are 200 elements.
|
||||
//
|
||||
// But in this case the 'bug' is protective against some attacks, and may
|
||||
|
@ -71,9 +73,9 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
|
|||
// So we should hold off on fixing this and clean it up as part of
|
||||
// a timing cleanup that strengthens it in a number of other ways.
|
||||
//
|
||||
if (vTimeOffsets.size() >= 5 && vTimeOffsets.size() % 2 == 1) {
|
||||
int64_t nMedian = vTimeOffsets.median();
|
||||
std::vector<int64_t> vSorted = vTimeOffsets.sorted();
|
||||
if (g_time_offsets.size() >= 5 && g_time_offsets.size() % 2 == 1) {
|
||||
int64_t nMedian = g_time_offsets.median();
|
||||
std::vector<int64_t> vSorted = g_time_offsets.sorted();
|
||||
// Only let other nodes change our time by so much
|
||||
int64_t max_adjustment = std::max<int64_t>(0, gArgs.GetIntArg("-maxtimeadjustment", DEFAULT_MAX_TIME_ADJUSTMENT));
|
||||
if (nMedian >= -max_adjustment && nMedian <= max_adjustment) {
|
||||
|
@ -81,8 +83,7 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
|
|||
} else {
|
||||
nTimeOffset = 0;
|
||||
|
||||
static bool fDone;
|
||||
if (!fDone) {
|
||||
if (!g_warning_emitted) {
|
||||
// If nobody has a time different than ours but within 5 minutes of ours, give a warning
|
||||
bool fMatch = false;
|
||||
for (const int64_t nOffset : vSorted) {
|
||||
|
@ -90,7 +91,7 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
|
|||
}
|
||||
|
||||
if (!fMatch) {
|
||||
fDone = true;
|
||||
g_warning_emitted = true;
|
||||
bilingual_str strMessage = strprintf(_("Please check that your computer's date and time are correct! If your clock is wrong, %s will not work properly."), PACKAGE_NAME);
|
||||
SetMiscWarning(strMessage);
|
||||
uiInterface.ThreadSafeMessageBox(strMessage, "", CClientUIInterface::MSG_WARNING);
|
||||
|
@ -108,3 +109,12 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
void TestOnlyResetTimeData()
|
||||
{
|
||||
LOCK(g_timeoffset_mutex);
|
||||
nTimeOffset = 0;
|
||||
g_sources.clear();
|
||||
g_time_offsets = CMedianFilter<int64_t>{BITCOIN_TIMEDATA_MAX_SAMPLES, 0};
|
||||
g_warning_emitted = false;
|
||||
}
|
||||
|
|
|
@ -75,4 +75,9 @@ int64_t GetTimeOffset();
|
|||
int64_t GetAdjustedTime();
|
||||
void AddTimeData(const CNetAddr& ip, int64_t nTime);
|
||||
|
||||
/**
|
||||
* Reset the internal state of GetTimeOffset(), GetAdjustedTime() and AddTimeData().
|
||||
*/
|
||||
void TestOnlyResetTimeData();
|
||||
|
||||
#endif // BITCOIN_TIMEDATA_H
|
||||
|
|
78
test/functional/feature_bind_port_discover.py
Executable file
78
test/functional/feature_bind_port_discover.py
Executable 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()
|
75
test/functional/feature_bind_port_externalip.py
Executable file
75
test/functional/feature_bind_port_externalip.py
Executable 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()
|
|
@ -20,7 +20,7 @@ LENGTH_SIZE = 4
|
|||
MSGTYPE_SIZE = 12
|
||||
|
||||
def mini_parser(dat_file):
|
||||
"""Parse a data file created by CaptureMessage.
|
||||
"""Parse a data file created by CaptureMessageToFile.
|
||||
|
||||
From the data file we'll only check the structure.
|
||||
|
||||
|
|
|
@ -276,6 +276,7 @@ BASE_SCRIPTS = [
|
|||
'feature_minchainwork.py',
|
||||
'rpc_estimatefee.py',
|
||||
'rpc_getblockstats.py',
|
||||
'feature_bind_port_externalip.py',
|
||||
'wallet_create_tx.py --legacy-wallet',
|
||||
'wallet_send.py --legacy-wallet',
|
||||
'wallet_send.py --descriptors',
|
||||
|
@ -291,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',
|
||||
|
|
Loading…
Reference in a new issue