merge bitcoin#22791: Fix asmap/addrman initialization order bug

This commit is contained in:
Kittywhiskers Van Gogh 2024-05-29 20:23:36 +00:00
parent 99b7812271
commit 9065eed969
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
13 changed files with 94 additions and 94 deletions

View File

@ -100,10 +100,11 @@ double CAddrInfo::GetChance(int64_t nNow) const
return fChance;
}
CAddrMan::CAddrMan(bool deterministic, int32_t consistency_check_ratio, bool _discriminatePorts)
CAddrMan::CAddrMan(std::vector<bool> asmap, bool deterministic, int32_t consistency_check_ratio, bool _discriminatePorts)
: insecure_rand{deterministic}
, nKey{deterministic ? uint256{1} : insecure_rand.rand256()}
, m_consistency_check_ratio{consistency_check_ratio}
, m_asmap{std::move(asmap)}
, discriminatePorts{_discriminatePorts}
{
for (auto& bucket : vvNew) {

View File

@ -149,22 +149,6 @@ static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2};
class CAddrMan
{
public:
// Compressed IP->ASN mapping, loaded from a file when a node starts.
// Should be always empty if no file was provided.
// This mapping is then used for bucketing nodes in Addrman.
//
// If asmap is provided, nodes will be bucketed by
// AS they belong to, in order to make impossible for a node
// to connect to several nodes hosted in a single AS.
// This is done in response to Erebus attack, but also to generally
// diversify the connections every node creates,
// especially useful when a large fraction of nodes
// operate under a couple of cloud providers.
//
// If a new asmap was provided, the existing records
// would be re-bucketed accordingly.
std::vector<bool> m_asmap;
// Read asmap from provided binary file
static std::vector<bool> DecodeAsmap(fs::path path);
@ -174,7 +158,7 @@ public:
template <typename Stream>
void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs);
explicit CAddrMan(bool deterministic, int32_t consistency_check_ratio, bool _discriminatePorts = false);
explicit CAddrMan(std::vector<bool> asmap, bool deterministic, int32_t consistency_check_ratio, bool _discriminatePorts = false);
~CAddrMan()
{
@ -296,6 +280,8 @@ public:
Check();
}
const std::vector<bool>& GetAsmap() const { return m_asmap; }
CAddrInfo GetAddressInfo(const CService& addr)
{
CAddrInfo addrRet;
@ -375,6 +361,22 @@ private:
/** Perform consistency checks every m_consistency_check_ratio operations (if non-zero). */
const int32_t m_consistency_check_ratio;
// Compressed IP->ASN mapping, loaded from a file when a node starts.
// Should be always empty if no file was provided.
// This mapping is then used for bucketing nodes in Addrman.
//
// If asmap is provided, nodes will be bucketed by
// AS they belong to, in order to make impossible for a node
// to connect to several nodes hosted in a single AS.
// This is done in response to Erebus attack, but also to generally
// diversify the connections every node creates,
// especially useful when a large fraction of nodes
// operate under a couple of cloud providers.
//
// If a new asmap was provided, the existing records
// would be re-bucketed accordingly.
const std::vector<bool> m_asmap;
// discriminate entries based on port. Should be false on mainnet/testnet and can be true on devnet/regtest
bool discriminatePorts GUARDED_BY(cs);

View File

@ -72,14 +72,14 @@ static void AddrManAdd(benchmark::Bench& bench)
CreateAddresses();
bench.run([&] {
CAddrMan addrman{/* deterministic */ false, /* consistency_check_ratio */ 0};
CAddrMan addrman{/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0};
AddAddressesToAddrMan(addrman);
});
}
static void AddrManSelect(benchmark::Bench& bench)
{
CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0);
CAddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
FillAddrMan(addrman);
@ -91,7 +91,7 @@ static void AddrManSelect(benchmark::Bench& bench)
static void AddrManGetAddr(benchmark::Bench& bench)
{
CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0);
CAddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
FillAddrMan(addrman);
@ -113,7 +113,7 @@ static void AddrManGood(benchmark::Bench& bench)
std::vector<std::unique_ptr<CAddrMan>> addrmans(addrman_count);
for (size_t i{0}; i < addrman_count; ++i) {
addrmans[i] = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ 0);
addrmans[i] = std::make_unique<CAddrMan>(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
FillAddrMan(*addrmans[i]);
}

View File

@ -1663,10 +1663,38 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
fDiscover = args.GetBoolArg("-discover", true);
const bool ignores_incoming_txs{args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY)};
assert(!node.addrman);
auto check_addrman = std::clamp<int32_t>(args.GetArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000);
node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ check_addrman);
{
// Initialize addrman
assert(!node.addrman);
// Read asmap file if configured
std::vector<bool> asmap;
if (args.IsArgSet("-asmap")) {
fs::path asmap_path = fs::path(args.GetArg("-asmap", ""));
if (asmap_path.empty()) {
asmap_path = DEFAULT_ASMAP_FILENAME;
}
if (!asmap_path.is_absolute()) {
asmap_path = GetDataDir() / asmap_path;
}
if (!fs::exists(asmap_path)) {
InitError(strprintf(_("Could not find asmap file %s"), asmap_path));
return false;
}
asmap = CAddrMan::DecodeAsmap(asmap_path);
if (asmap.size() == 0) {
InitError(strprintf(_("Could not parse asmap file %s"), asmap_path));
return false;
}
const uint256 asmap_version = SerializeHash(asmap);
LogPrintf("Using asmap version %s for IP bucketing\n", asmap_version.ToString());
} else {
LogPrintf("Using /16 prefix for IP bucketing\n");
}
auto check_addrman = std::clamp<int32_t>(args.GetArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000);
node.addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
// Load addresses from peers.dat
uiInterface.InitMessage(_("Loading P2P addresses…").translated);
int64_t nStart = GetTimeMillis();
@ -1675,11 +1703,12 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
LogPrintf("Loaded %i addresses from peers.dat %dms\n", node.addrman->size(), GetTimeMillis() - nStart);
} else {
// Addrman can be in an inconsistent state after failure, reset it
node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ check_addrman);
node.addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
LogPrintf("Recreating peers.dat\n");
adb.Write(*node.addrman);
}
}
assert(!node.banman);
node.banman = std::make_unique<BanMan>(GetDataDir() / "banlist", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME));
assert(!node.connman);
@ -1855,31 +1884,6 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
return InitError(ResolveErrMsg("externalip", strAddr));
}
// Read asmap file if configured
if (args.IsArgSet("-asmap")) {
fs::path asmap_path = fs::path(args.GetArg("-asmap", ""));
if (asmap_path.empty()) {
asmap_path = DEFAULT_ASMAP_FILENAME;
}
if (!asmap_path.is_absolute()) {
asmap_path = GetDataDir() / asmap_path;
}
if (!fs::exists(asmap_path)) {
InitError(strprintf(_("Could not find asmap file %s"), asmap_path));
return false;
}
std::vector<bool> asmap = CAddrMan::DecodeAsmap(asmap_path);
if (asmap.size() == 0) {
InitError(strprintf(_("Could not parse asmap file %s"), asmap_path));
return false;
}
const uint256 asmap_version = SerializeHash(asmap);
node.connman->SetAsmap(std::move(asmap));
LogPrintf("Using asmap version %s for IP bucketing\n", asmap_version.ToString());
} else {
LogPrintf("Using /16 prefix for IP bucketing\n");
}
#if ENABLE_ZMQ
g_zmq_notification_interface = CZMQNotificationInterface::Create();

View File

@ -664,14 +664,14 @@ Network CNode::ConnectedThroughNetwork() const
#undef X
#define X(name) stats.name = name
void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
void CNode::CopyStats(CNodeStats& stats, const std::vector<bool>& asmap)
{
stats.nodeid = this->GetId();
X(nServices);
X(addr);
X(addrBind);
stats.m_network = ConnectedThroughNetwork();
stats.m_mapped_as = addr.GetMappedAS(m_asmap);
stats.m_mapped_as = addr.GetMappedAS(asmap);
X(m_last_send);
X(m_last_recv);
X(nLastTXTime);
@ -2505,7 +2505,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
case ConnectionType::BLOCK_RELAY:
case ConnectionType::ADDR_FETCH:
case ConnectionType::FEELER:
setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap));
setConnected.insert(pnode->addr.GetGroup(addrman.GetAsmap()));
} // no default case, so the compiler can warn about missing cases
}
}
@ -2592,7 +2592,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
m_anchors.pop_back();
if (!addr.IsValid() || IsLocal(addr) || !IsReachable(addr) ||
!HasAllDesirableServiceFlags(addr.nServices) ||
setConnected.count(addr.GetGroup(addrman.m_asmap))) continue;
setConnected.count(addr.GetGroup(addrman.GetAsmap()))) continue;
addrConnect = addr;
LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToString());
break;
@ -2635,12 +2635,12 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
bool isMasternode = dmn != nullptr;
// Require outbound connections, other than feelers, to be to distinct network groups
if (!fFeeler && setConnected.count(addr.GetGroup(addrman.m_asmap))) {
if (!fFeeler && setConnected.count(addr.GetGroup(addrman.GetAsmap()))) {
break;
}
// if we selected an invalid address, restart
if (!addr.IsValid() || setConnected.count(addr.GetGroup(addrman.m_asmap)))
if (!addr.IsValid() || setConnected.count(addr.GetGroup(addrman.GetAsmap())))
break;
// don't try to connect to masternodes that we already have a connection to (most likely inbound)
@ -3870,7 +3870,7 @@ void CConnman::GetNodeStats(std::vector<CNodeStats>& vstats) const
continue;
}
vstats.emplace_back();
pnode->copyStats(vstats.back(), addrman.m_asmap);
pnode->CopyStats(vstats.back(), addrman.GetAsmap());
}
}
@ -4196,7 +4196,7 @@ CSipHasher CConnman::GetDeterministicRandomizer(uint64_t id) const
uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& ad) const
{
std::vector<unsigned char> vchNetGroup(ad.GetGroup(addrman.m_asmap));
std::vector<unsigned char> vchNetGroup(ad.GetGroup(addrman.GetAsmap()));
return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup.data(), vchNetGroup.size()).Finalize();
}

View File

@ -679,7 +679,7 @@ public:
void CloseSocketDisconnect(CConnman* connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_hSocket);
void copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv);
void CopyStats(CNodeStats& stats, const std::vector<bool>& asmap) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv);
ServiceFlags GetLocalServices() const
{
@ -1186,8 +1186,6 @@ public:
*/
std::chrono::microseconds PoissonNextSendInbound(std::chrono::microseconds now, std::chrono::seconds average_interval);
void SetAsmap(std::vector<bool> asmap) { addrman.m_asmap = std::move(asmap); }
/** Return true if we should disconnect the peer for failing an inactivity check. */
bool ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const;

View File

@ -24,7 +24,7 @@ public:
virtual void Serialize(CDataStream& s) const = 0;
CAddrManSerializationMock()
: CAddrMan(/* deterministic */ true, /* consistency_check_ratio */ 100)
: CAddrMan(/* asmap */ std::vector<bool>(), /* deterministic */ true, /* consistency_check_ratio */ 100)
{}
};
@ -81,10 +81,9 @@ private:
public:
explicit CAddrManTest(bool makeDeterministic = true,
std::vector<bool> asmap = std::vector<bool>())
: CAddrMan(makeDeterministic, /* consistency_check_ratio */ 100)
: CAddrMan(asmap, makeDeterministic, /* consistency_check_ratio */ 100)
{
deterministic = makeDeterministic;
m_asmap = asmap;
}
CAddrInfo* Find(const CService& addr, int* pnId = nullptr)
@ -1024,7 +1023,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read)
// Test that the de-serialization does not throw an exception.
CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted);
bool exceptionThrown = false;
CAddrMan addrman1(/* deterministic */ false, /* consistency_check_ratio */ 100);
CAddrMan addrman1(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
BOOST_CHECK(addrman1.size() == 0);
try {
@ -1041,7 +1040,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read)
// Test that CAddrDB::Read creates an addrman with the correct number of addrs.
CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted);
CAddrMan addrman2(/* deterministic */ false, /* consistency_check_ratio */ 100);
CAddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
BOOST_CHECK(addrman2.size() == 0);
BOOST_CHECK(CAddrDB::Read(addrman2, ssPeers2));
BOOST_CHECK(addrman2.size() == 3);
@ -1055,7 +1054,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted)
// Test that the de-serialization of corrupted addrman throws an exception.
CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted);
bool exceptionThrown = false;
CAddrMan addrman1(/* deterministic */ false, /* consistency_check_ratio */ 100);
CAddrMan addrman1(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
BOOST_CHECK(addrman1.size() == 0);
try {
unsigned char pchMsgTmp[4];
@ -1071,7 +1070,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted)
// Test that CAddrDB::Read fails if peers.dat is corrupt
CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted);
CAddrMan addrman2(/* deterministic */ false, /* consistency_check_ratio */ 100);
CAddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
BOOST_CHECK(addrman2.size() == 0);
BOOST_CHECK(!CAddrDB::Read(addrman2, ssPeers2));
}

View File

@ -28,17 +28,11 @@ class CAddrManDeterministic : public CAddrMan
public:
FuzzedDataProvider& m_fuzzed_data_provider;
explicit CAddrManDeterministic(FuzzedDataProvider& fuzzed_data_provider)
: CAddrMan(/* deterministic */ true, /* consistency_check_ratio */ 0)
explicit CAddrManDeterministic(std::vector<bool> asmap, FuzzedDataProvider& fuzzed_data_provider)
: CAddrMan(std::move(asmap), /* deterministic */ true, /* consistency_check_ratio */ 0)
, m_fuzzed_data_provider(fuzzed_data_provider)
{
WITH_LOCK(cs, insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)});
if (fuzzed_data_provider.ConsumeBool()) {
m_asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider);
if (!SanityCheckASMap(m_asmap)) {
m_asmap.clear();
}
}
}
/**
@ -224,11 +218,19 @@ public:
}
};
[[nodiscard]] inline std::vector<bool> ConsumeAsmap(FuzzedDataProvider& fuzzed_data_provider) noexcept
{
std::vector<bool> asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider);
if (!SanityCheckASMap(asmap)) asmap.clear();
return asmap;
}
FUZZ_TARGET_INIT(addrman, initialize_addrman)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
SetMockTime(ConsumeTime(fuzzed_data_provider));
auto addr_man_ptr = std::make_unique<CAddrManDeterministic>(fuzzed_data_provider);
std::vector<bool> asmap = ConsumeAsmap(fuzzed_data_provider);
auto addr_man_ptr = std::make_unique<CAddrManDeterministic>(asmap, fuzzed_data_provider);
if (fuzzed_data_provider.ConsumeBool()) {
const std::vector<uint8_t> serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)};
CDataStream ds(serialized_data, SER_DISK, INIT_PROTO_VERSION);
@ -237,7 +239,7 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman)
try {
ds >> *addr_man_ptr;
} catch (const std::ios_base::failure&) {
addr_man_ptr = std::make_unique<CAddrManDeterministic>(fuzzed_data_provider);
addr_man_ptr = std::make_unique<CAddrManDeterministic>(asmap, fuzzed_data_provider);
}
}
CAddrManDeterministic& addr_man = *addr_man_ptr;
@ -309,9 +311,9 @@ FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman)
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
SetMockTime(ConsumeTime(fuzzed_data_provider));
CAddrManDeterministic addr_man1{fuzzed_data_provider};
CAddrManDeterministic addr_man2{fuzzed_data_provider};
addr_man2.m_asmap = addr_man1.m_asmap;
std::vector<bool> asmap = ConsumeAsmap(fuzzed_data_provider);
CAddrManDeterministic addr_man1{asmap, fuzzed_data_provider};
CAddrManDeterministic addr_man2{asmap, fuzzed_data_provider};
CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION);

View File

@ -25,7 +25,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
{
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
SetMockTime(ConsumeTime(fuzzed_data_provider));
CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0);
CAddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
CConnman connman{fuzzed_data_provider.ConsumeIntegral<uint64_t>(), fuzzed_data_provider.ConsumeIntegral<uint64_t>(), addrman};
CNetAddr random_netaddr;
CNode random_node = ConsumeNode(fuzzed_data_provider);
@ -106,12 +106,6 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
[&] {
connman.RemoveAddedNode(random_string);
},
[&] {
const std::vector<bool> asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider);
if (SanityCheckASMap(asmap)) {
connman.SetAsmap(asmap);
}
},
[&] {
connman.SetNetworkActive(fuzzed_data_provider.ConsumeBool(), /* mn_sync = */ nullptr);
},

View File

@ -21,6 +21,6 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_data_stream_addr_man)
{
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
CDataStream data_stream = ConsumeDataStream(fuzzed_data_provider);
CAddrMan addr_man(/* deterministic */ false, /* consistency_check_ratio */ 0);
CAddrMan addr_man(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
CAddrDB::Read(addr_man, data_stream);
}

View File

@ -189,7 +189,7 @@ FUZZ_TARGET_DESERIALIZE(blockmerkleroot, {
BlockMerkleRoot(block, &mutated);
})
FUZZ_TARGET_DESERIALIZE(addrman_deserialize, {
CAddrMan am(/* deterministic */ false, /* consistency_check_ratio */ 0);
CAddrMan am(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
DeserializeFromFuzzingInput(buffer, am);
})
FUZZ_TARGET_DESERIALIZE(blockheader_deserialize, {

View File

@ -36,7 +36,7 @@ FUZZ_TARGET_INIT(net, initialize_net)
CallOneOf(
fuzzed_data_provider,
[&] {
CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0);
CAddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
CConnman connman{fuzzed_data_provider.ConsumeIntegral<uint64_t>(), fuzzed_data_provider.ConsumeIntegral<uint64_t>(), addrman};
node.CloseSocketDisconnect(&connman);
},
@ -46,7 +46,7 @@ FUZZ_TARGET_INIT(net, initialize_net)
return;
}
CNodeStats stats;
node.copyStats(stats, asmap);
node.CopyStats(stats, asmap);
},
[&] {
const CNode* add_ref_node = node.AddRef();

View File

@ -180,7 +180,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
SetupNetworking();
InitSignatureCache();
InitScriptExecutionCache();
m_node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ 0);
m_node.addrman = std::make_unique<CAddrMan>(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
m_node.chain = interfaces::MakeChain(m_node);
// while g_wallet_init_interface is init here at very early stage
// we can't get rid of unique_ptr from wallet/contex.h