From 9b3836710b8160d212aacd56154938e5bb4b26b7 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 31 Aug 2021 13:22:36 +0100 Subject: [build] Add netgroup.cpp|h These aren't used yet. --- src/Makefile.am | 2 ++ src/netgroup.cpp | 5 +++++ src/netgroup.h | 13 +++++++++++++ 3 files changed, 20 insertions(+) create mode 100644 src/netgroup.cpp create mode 100644 src/netgroup.h diff --git a/src/Makefile.am b/src/Makefile.am index c089bed0c9..476ff0a6c5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -180,6 +180,7 @@ BITCOIN_CORE_H = \ net_types.h \ netaddress.h \ netbase.h \ + netgroup.h \ netmessagemaker.h \ node/blockstorage.h \ node/caches.h \ @@ -352,6 +353,7 @@ libbitcoin_node_a_SOURCES = \ init.cpp \ mapport.cpp \ net.cpp \ + netgroup.cpp \ net_processing.cpp \ node/blockstorage.cpp \ node/caches.cpp \ diff --git a/src/netgroup.cpp b/src/netgroup.cpp new file mode 100644 index 0000000000..17a48ad025 --- /dev/null +++ b/src/netgroup.cpp @@ -0,0 +1,5 @@ +// Copyright (c) 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. + +#include diff --git a/src/netgroup.h b/src/netgroup.h new file mode 100644 index 0000000000..1fd3470e0f --- /dev/null +++ b/src/netgroup.h @@ -0,0 +1,13 @@ +// Copyright (c) 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. + +#ifndef BITCOIN_NETGROUP_H +#define BITCOIN_NETGROUP_H + +/** + * Netgroup manager + */ +class NetGroupManager {}; + +#endif // BITCOIN_NETGROUP_H -- cgit v1.2.3 From 17c24d458042229e00dd4e0b75a32e593be29564 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 31 Aug 2021 13:32:40 +0100 Subject: [init] Add netgroupman to node.context This is constructed before addrman and connman, and destructed afterwards. netgroupman does not currently do anything, but will have functionality added in future commits. --- src/init.cpp | 10 ++++++++-- src/node/context.cpp | 1 + src/node/context.h | 2 ++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index fdab296593..fa11fcde6b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -240,6 +241,7 @@ void Shutdown(NodeContext& node) node.connman.reset(); node.banman.reset(); node.addrman.reset(); + node.netgroupman.reset(); if (node.mempool && node.mempool->IsLoaded() && node.args->GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { DumpMempool(*node.mempool); @@ -1229,8 +1231,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const bool ignores_incoming_txs{args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY)}; { - // Initialize addrman - assert(!node.addrman); // Read asmap file if configured std::vector asmap; @@ -1254,6 +1254,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) LogPrintf("Using /16 prefix for IP bucketing\n"); } + // Initialize netgroup manager + assert(!node.netgroupman); + node.netgroupman = std::make_unique(); + + // Initialize addrman + assert(!node.addrman); uiInterface.InitMessage(_("Loading P2P addresses…").translated); if (const auto error{LoadAddrman(asmap, args, node.addrman)}) { return InitError(*error); diff --git a/src/node/context.cpp b/src/node/context.cpp index 893c32f1bc..0b31c10f44 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/src/node/context.h b/src/node/context.h index 644c997531..91ba456219 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -18,6 +18,7 @@ class CConnman; class CScheduler; class CTxMemPool; class ChainstateManager; +class NetGroupManager; class PeerManager; namespace interfaces { class Chain; @@ -43,6 +44,7 @@ struct NodeContext { std::unique_ptr addrman; std::unique_ptr connman; std::unique_ptr mempool; + std::unique_ptr netgroupman; std::unique_ptr fee_estimator; std::unique_ptr peerman; std::unique_ptr chainman; -- cgit v1.2.3 From 19431560e3e1124979c60f39eca9429c4a0df29f Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 31 Aug 2021 18:40:18 +0100 Subject: [net] Move asmap into NetGroupManager --- src/addrdb.cpp | 9 ++++--- src/addrdb.h | 3 ++- src/addrman.cpp | 54 +++++++++++++++---------------------- src/addrman.h | 5 ++-- src/addrman_impl.h | 21 +++------------ src/bench/addrman.cpp | 11 ++++---- src/init.cpp | 8 +++--- src/net.cpp | 18 ++++++++----- src/net.h | 6 ++++- src/netgroup.h | 34 ++++++++++++++++++++++- src/test/addrman_tests.cpp | 55 +++++++++++++++++++------------------- src/test/denialofservice_tests.cpp | 10 +++---- src/test/fuzz/addrman.cpp | 33 ++++++++++++----------- src/test/fuzz/connman.cpp | 13 ++++----- src/test/fuzz/deserialize.cpp | 4 ++- src/test/util/setup_common.cpp | 6 +++-- 16 files changed, 158 insertions(+), 132 deletions(-) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 1fa2644647..4845234e18 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -182,10 +183,10 @@ void ReadFromStream(AddrMan& addr, CDataStream& ssPeers) DeserializeDB(ssPeers, addr, false); } -std::optional LoadAddrman(const std::vector& asmap, const ArgsManager& args, std::unique_ptr& addrman) +std::optional LoadAddrman(const NetGroupManager& netgroupman, const ArgsManager& args, std::unique_ptr& addrman) { auto check_addrman = std::clamp(args.GetIntArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000); - addrman = std::make_unique(asmap, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); + addrman = std::make_unique(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); int64_t nStart = GetTimeMillis(); const auto path_addr{args.GetDataDirNet() / "peers.dat"}; @@ -194,7 +195,7 @@ std::optional LoadAddrman(const std::vector& asmap, const A LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman->size(), GetTimeMillis() - nStart); } catch (const DbNotFoundError&) { // Addrman can be in an inconsistent state after failure, reset it - addrman = std::make_unique(asmap, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); + addrman = std::make_unique(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); LogPrintf("Creating peers.dat because the file was not found (%s)\n", fs::quoted(fs::PathToString(path_addr))); DumpPeerAddresses(args, *addrman); } catch (const InvalidAddrManVersionError&) { @@ -203,7 +204,7 @@ std::optional LoadAddrman(const std::vector& asmap, const A return strprintf(_("Failed to rename invalid peers.dat file. Please move or delete it and try again.")); } // Addrman can be in an inconsistent state after failure, reset it - addrman = std::make_unique(asmap, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); + addrman = std::make_unique(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); LogPrintf("Creating new peers.dat because the file version was not compatible (%s). Original backed up to peers.dat.bak\n", fs::quoted(fs::PathToString(path_addr))); DumpPeerAddresses(args, *addrman); } catch (const std::exception& e) { diff --git a/src/addrdb.h b/src/addrdb.h index 4bdafb64e4..627ef3ac3c 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -17,6 +17,7 @@ class ArgsManager; class AddrMan; class CAddress; class CDataStream; +class NetGroupManager; struct bilingual_str; bool DumpPeerAddresses(const ArgsManager& args, const AddrMan& addr); @@ -48,7 +49,7 @@ public: }; /** Returns an error string on failure */ -std::optional LoadAddrman(const std::vector& asmap, const ArgsManager& args, std::unique_ptr& addrman); +std::optional LoadAddrman(const NetGroupManager& netgroupman, const ArgsManager& args, std::unique_ptr& addrman); /** * Dump the anchor IP address database (anchors.dat) diff --git a/src/addrman.cpp b/src/addrman.cpp index 2a08d99eef..1f74faa682 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -99,11 +99,11 @@ double AddrInfo::GetChance(int64_t nNow) const return fChance; } -AddrManImpl::AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio) +AddrManImpl::AddrManImpl(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio) : insecure_rand{deterministic} , nKey{deterministic ? uint256{1} : insecure_rand.rand256()} , m_consistency_check_ratio{consistency_check_ratio} - , m_asmap{std::move(asmap)} + , m_netgroupman{netgroupman} { for (auto& bucket : vvNew) { for (auto& entry : bucket) { @@ -219,8 +219,8 @@ void AddrManImpl::Serialize(Stream& s_) const // Store asmap checksum after bucket entries so that it // can be ignored by older clients for backward compatibility. uint256 asmap_checksum; - if (m_asmap.size() != 0) { - asmap_checksum = SerializeHash(m_asmap); + if (m_netgroupman.GetAsmap().size() != 0) { + asmap_checksum = SerializeHash(m_netgroupman.GetAsmap()); } s << asmap_checksum; } @@ -298,7 +298,7 @@ void AddrManImpl::Unserialize(Stream& s_) for (int n = 0; n < nTried; n++) { AddrInfo info; s >> info; - int nKBucket = info.GetTriedBucket(nKey, m_asmap); + int nKBucket = info.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); if (info.IsValid() && vvTried[nKBucket][nKBucketPos] == -1) { @@ -336,8 +336,8 @@ void AddrManImpl::Unserialize(Stream& s_) // to restore the entries to the buckets/positions they were in before // serialization. uint256 supplied_asmap_checksum; - if (m_asmap.size() != 0) { - supplied_asmap_checksum = SerializeHash(m_asmap); + if (m_netgroupman.GetAsmap().size() != 0) { + supplied_asmap_checksum = SerializeHash(m_netgroupman.GetAsmap()); } uint256 serialized_asmap_checksum; if (format >= Format::V2_ASMAP) { @@ -371,7 +371,7 @@ void AddrManImpl::Unserialize(Stream& s_) } else { // In case the new table data cannot be used (bucket count wrong or new asmap), // try to give them a reference based on their primary source address. - bucket = info.GetNewBucket(nKey, m_asmap); + bucket = info.GetNewBucket(nKey, m_netgroupman.GetAsmap()); bucket_position = info.GetBucketPosition(nKey, true, bucket); if (vvNew[bucket][bucket_position] == -1) { vvNew[bucket][bucket_position] = entry_index; @@ -495,7 +495,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) AssertLockHeld(cs); // remove the entry from all new buckets - const int start_bucket{info.GetNewBucket(nKey, m_asmap)}; + const int start_bucket{info.GetNewBucket(nKey, m_netgroupman.GetAsmap())}; for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; ++n) { const int bucket{(start_bucket + n) % ADDRMAN_NEW_BUCKET_COUNT}; const int pos{info.GetBucketPosition(nKey, true, bucket)}; @@ -510,7 +510,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) assert(info.nRefCount == 0); // which tried bucket to move the entry to - int nKBucket = info.GetTriedBucket(nKey, m_asmap); + int nKBucket = info.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); // first make space to add it (the existing tried entry there is moved to new, deleting whatever is there). @@ -526,7 +526,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) nTried--; // find which new bucket it belongs to - int nUBucket = infoOld.GetNewBucket(nKey, m_asmap); + int nUBucket = infoOld.GetNewBucket(nKey, m_netgroupman.GetAsmap()); int nUBucketPos = infoOld.GetBucketPosition(nKey, true, nUBucket); ClearNew(nUBucket, nUBucketPos); assert(vvNew[nUBucket][nUBucketPos] == -1); @@ -594,7 +594,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_ nNew++; } - int nUBucket = pinfo->GetNewBucket(nKey, source, m_asmap); + int nUBucket = pinfo->GetNewBucket(nKey, source, m_netgroupman.GetAsmap()); int nUBucketPos = pinfo->GetBucketPosition(nKey, true, nUBucket); bool fInsert = vvNew[nUBucket][nUBucketPos] == -1; if (vvNew[nUBucket][nUBucketPos] != nId) { @@ -610,7 +610,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_ pinfo->nRefCount++; vvNew[nUBucket][nUBucketPos] = nId; LogPrint(BCLog::ADDRMAN, "Added %s mapped to AS%i to new[%i][%i]\n", - addr.ToString(), addr.GetMappedAS(m_asmap), nUBucket, nUBucketPos); + addr.ToString(), addr.GetMappedAS(m_netgroupman.GetAsmap()), nUBucket, nUBucketPos); } else { if (pinfo->nRefCount == 0) { Delete(nId); @@ -650,7 +650,7 @@ bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT // which tried bucket to move the entry to - int tried_bucket = info.GetTriedBucket(nKey, m_asmap); + int tried_bucket = info.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket); // Will moving this address into tried evict another entry? @@ -669,7 +669,7 @@ bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT // move nId to the tried tables MakeTried(info, nId); LogPrint(BCLog::ADDRMAN, "Moved %s mapped to AS%i to tried[%i][%i]\n", - addr.ToString(), addr.GetMappedAS(m_asmap), tried_bucket, tried_bucket_pos); + addr.ToString(), addr.GetMappedAS(m_netgroupman.GetAsmap()), tried_bucket, tried_bucket_pos); return true; } } @@ -863,7 +863,7 @@ void AddrManImpl::ResolveCollisions_() AddrInfo& info_new = mapInfo[id_new]; // Which tried bucket to move the entry to. - int tried_bucket = info_new.GetTriedBucket(nKey, m_asmap); + int tried_bucket = info_new.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); int tried_bucket_pos = info_new.GetBucketPosition(nKey, false, tried_bucket); if (!info_new.IsValid()) { // id_new may no longer map to a valid address erase_collision = true; @@ -929,7 +929,7 @@ std::pair AddrManImpl::SelectTriedCollision_() const AddrInfo& newInfo = mapInfo[id_new]; // which tried bucket to move the entry to - int tried_bucket = newInfo.GetTriedBucket(nKey, m_asmap); + int tried_bucket = newInfo.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); int tried_bucket_pos = newInfo.GetBucketPosition(nKey, false, tried_bucket); const AddrInfo& info_old = mapInfo[vvTried[tried_bucket][tried_bucket_pos]]; @@ -945,13 +945,13 @@ std::optional AddrManImpl::FindAddressEntry_(const CAddress& ad if (!addr_info) return std::nullopt; if(addr_info->fInTried) { - int bucket{addr_info->GetTriedBucket(nKey, m_asmap)}; + int bucket{addr_info->GetTriedBucket(nKey, m_netgroupman.GetAsmap())}; return AddressPosition(/*tried_in=*/true, /*multiplicity_in=*/1, /*bucket_in=*/bucket, /*position_in=*/addr_info->GetBucketPosition(nKey, false, bucket)); } else { - int bucket{addr_info->GetNewBucket(nKey, m_asmap)}; + int bucket{addr_info->GetNewBucket(nKey, m_netgroupman.GetAsmap())}; return AddressPosition(/*tried_in=*/false, /*multiplicity_in=*/addr_info->nRefCount, /*bucket_in=*/bucket, @@ -1026,7 +1026,7 @@ int AddrManImpl::CheckAddrman() const if (!setTried.count(vvTried[n][i])) return -11; const auto it{mapInfo.find(vvTried[n][i])}; - if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_asmap) != n) { + if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_netgroupman.GetAsmap()) != n) { return -17; } if (it->second.GetBucketPosition(nKey, false, n) != i) { @@ -1154,13 +1154,8 @@ std::optional AddrManImpl::FindAddressEntry(const CAddress& add return entry; } -const std::vector& AddrManImpl::GetAsmap() const -{ - return m_asmap; -} - -AddrMan::AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio) - : m_impl(std::make_unique(std::move(asmap), deterministic, consistency_check_ratio)) {} +AddrMan::AddrMan(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio) + : m_impl(std::make_unique(netgroupman, deterministic, consistency_check_ratio)) {} AddrMan::~AddrMan() = default; @@ -1235,11 +1230,6 @@ void AddrMan::SetServices(const CService& addr, ServiceFlags nServices) m_impl->SetServices(addr, nServices); } -const std::vector& AddrMan::GetAsmap() const -{ - return m_impl->GetAsmap(); -} - std::optional AddrMan::FindAddressEntry(const CAddress& addr) { return m_impl->FindAddressEntry(addr); diff --git a/src/addrman.h b/src/addrman.h index 472282833b..a0063e8a9c 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -7,6 +7,7 @@ #define BITCOIN_ADDRMAN_H #include +#include #include #include #include @@ -88,7 +89,7 @@ protected: const std::unique_ptr m_impl; public: - explicit AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); + explicit AddrMan(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio); ~AddrMan(); @@ -172,8 +173,6 @@ public: //! Update an entry's service bits. void SetServices(const CService& addr, ServiceFlags nServices); - const std::vector& GetAsmap() const; - /** Test-only function * Find the address record in AddrMan and return information about its * position. diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 5e76f72342..3db4be6c8d 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -100,7 +100,7 @@ public: class AddrManImpl { public: - AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio); + AddrManImpl(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio); ~AddrManImpl(); @@ -140,8 +140,6 @@ public: std::optional FindAddressEntry(const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(!cs); - const std::vector& GetAsmap() const; - friend class AddrManDeterministic; private: @@ -212,21 +210,8 @@ 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 m_asmap; + /** Reference to the netgroup manager. netgroupman must be constructed before addrman and destructed after. */ + const NetGroupManager& m_netgroupman; //! Find an entry. AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index 34bc4380dd..76300f4db8 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -16,7 +17,7 @@ static constexpr size_t NUM_SOURCES = 64; static constexpr size_t NUM_ADDRESSES_PER_SOURCE = 256; -static const std::vector EMPTY_ASMAP; +static NetGroupManager EMPTY_NETGROUPMAN{std::vector()}; static constexpr uint32_t ADDRMAN_CONSISTENCY_CHECK_RATIO{0}; static std::vector g_sources; @@ -77,14 +78,14 @@ static void AddrManAdd(benchmark::Bench& bench) CreateAddresses(); bench.run([&] { - AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; + AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; AddAddressesToAddrMan(addrman); }); } static void AddrManSelect(benchmark::Bench& bench) { - AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; + AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; FillAddrMan(addrman); @@ -96,7 +97,7 @@ static void AddrManSelect(benchmark::Bench& bench) static void AddrManGetAddr(benchmark::Bench& bench) { - AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; + AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; FillAddrMan(addrman); @@ -125,7 +126,7 @@ static void AddrManAddThenGood(benchmark::Bench& bench) // // This has some overhead (exactly the result of AddrManAdd benchmark), but that overhead is constant so improvements in // AddrMan::Good() will still be noticeable. - AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; + AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; AddAddressesToAddrMan(addrman); markSomeAsGood(addrman); diff --git a/src/init.cpp b/src/init.cpp index fa11fcde6b..8afc08a21f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1256,12 +1256,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Initialize netgroup manager assert(!node.netgroupman); - node.netgroupman = std::make_unique(); + node.netgroupman = std::make_unique(std::move(asmap)); // Initialize addrman assert(!node.addrman); uiInterface.InitMessage(_("Loading P2P addresses…").translated); - if (const auto error{LoadAddrman(asmap, args, node.addrman)}) { + if (const auto error{LoadAddrman(*node.netgroupman, args, node.addrman)}) { return InitError(*error); } } @@ -1269,7 +1269,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.banman); node.banman = std::make_unique(gArgs.GetDataDirNet() / "banlist", &uiInterface, args.GetIntArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); assert(!node.connman); - node.connman = std::make_unique(GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max()), *node.addrman, args.GetBoolArg("-networkactive", true)); + node.connman = std::make_unique(GetRand(std::numeric_limits::max()), + GetRand(std::numeric_limits::max()), + *node.addrman, *node.netgroupman, args.GetBoolArg("-networkactive", true)); assert(!node.fee_estimator); // Don't initialize fee estimation with old data if we don't relay transactions, diff --git a/src/net.cpp b/src/net.cpp index 602d56ab98..e92694ad42 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1997,7 +1997,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) case ConnectionType::BLOCK_RELAY: case ConnectionType::ADDR_FETCH: case ConnectionType::FEELER: - setConnected.insert(pnode->addr.GetGroup(addrman.GetAsmap())); + setConnected.insert(pnode->addr.GetGroup(m_netgroupman.GetAsmap())); } // no default case, so the compiler can warn about missing cases } } @@ -2071,7 +2071,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) m_anchors.pop_back(); if (!addr.IsValid() || IsLocal(addr) || !IsReachable(addr) || !HasAllDesirableServiceFlags(addr.nServices) || - setConnected.count(addr.GetGroup(addrman.GetAsmap()))) continue; + setConnected.count(addr.GetGroup(m_netgroupman.GetAsmap()))) continue; addrConnect = addr; LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToString()); break; @@ -2112,7 +2112,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) } // Require outbound connections, other than feelers, to be to distinct network groups - if (!fFeeler && setConnected.count(addr.GetGroup(addrman.GetAsmap()))) { + if (!fFeeler && setConnected.count(addr.GetGroup(m_netgroupman.GetAsmap()))) { break; } @@ -2499,8 +2499,12 @@ void CConnman::SetNetworkActive(bool active) } } -CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, AddrMan& addrman_in, bool network_active) - : addrman(addrman_in), nSeed0(nSeed0In), nSeed1(nSeed1In) +CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, AddrMan& addrman_in, + const NetGroupManager& netgroupman, bool network_active) + : addrman(addrman_in) + , m_netgroupman{netgroupman} + , nSeed0(nSeed0In) + , nSeed1(nSeed1In) { SetTryNewOutboundPeer(false); @@ -2859,7 +2863,7 @@ void CConnman::GetNodeStats(std::vector& vstats) const for (CNode* pnode : m_nodes) { vstats.emplace_back(); pnode->CopyStats(vstats.back()); - vstats.back().m_mapped_as = pnode->addr.GetMappedAS(addrman.GetAsmap()); + vstats.back().m_mapped_as = pnode->addr.GetMappedAS(m_netgroupman.GetAsmap()); } } @@ -3094,7 +3098,7 @@ CSipHasher CConnman::GetDeterministicRandomizer(uint64_t id) const uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& ad) const { - std::vector vchNetGroup(ad.GetGroup(addrman.GetAsmap())); + std::vector vchNetGroup(ad.GetGroup(m_netgroupman.GetAsmap())); return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup.data(), vchNetGroup.size()).Finalize(); } diff --git a/src/net.h b/src/net.h index 1bc011c748..b253a90442 100644 --- a/src/net.h +++ b/src/net.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -787,7 +788,9 @@ public: m_onion_binds = connOptions.onion_binds; } - CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, bool network_active = true); + CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, const NetGroupManager& netgroupman, + bool network_active = true); + ~CConnman(); bool Start(CScheduler& scheduler, const Options& options); @@ -1085,6 +1088,7 @@ private: std::atomic fNetworkActive{true}; bool fAddressesInitialized{false}; AddrMan& addrman; + const NetGroupManager& m_netgroupman; std::deque m_addr_fetches GUARDED_BY(m_addr_fetches_mutex); Mutex m_addr_fetches_mutex; std::vector m_added_nodes GUARDED_BY(m_added_nodes_mutex); diff --git a/src/netgroup.h b/src/netgroup.h index 1fd3470e0f..fcf46bb2bb 100644 --- a/src/netgroup.h +++ b/src/netgroup.h @@ -5,9 +5,41 @@ #ifndef BITCOIN_NETGROUP_H #define BITCOIN_NETGROUP_H +#include + /** * Netgroup manager */ -class NetGroupManager {}; +class NetGroupManager { +public: + explicit NetGroupManager(std::vector asmap) + : m_asmap{std::move(asmap)} + {} + + /* Get a reference to (const) asmap. May be held as long as NetGroupManager + * exists, since the data is const. */ + const std::vector& GetAsmap() const { return m_asmap; } + +private: + /** Compressed IP->ASN mapping, loaded from a file when a node starts. + * + * This mapping is then used for bucketing nodes in Addrman and for + * ensuring we connect to a diverse set of peers in Connman. The map is + * empty if no file was provided. + * + * 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 is provided, the existing addrman records are + * re-bucketed. + * + * This is initialized in the constructor, const, and therefore is + * thread-safe. */ + const std::vector m_asmap; +}; #endif // BITCOIN_NETGROUP_H diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index efc30b6822..2a30d15e49 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -23,7 +23,7 @@ using namespace std::literals; using node::NodeContext; -static const std::vector EMPTY_ASMAP; +static NetGroupManager EMPTY_NETGROUPMAN{std::vector()}; static const bool DETERMINISTIC{true}; static int32_t GetCheckRatio(const NodeContext& node_ctx) @@ -62,7 +62,7 @@ BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(addrman_simple) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); @@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple) BOOST_CHECK(addrman->size() >= 1); // Test: reset addrman and test AddrMan::Add multiple addresses works as expected - addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); std::vector vAddr; vAddr.push_back(CAddress(ResolveService("250.1.1.3", 8333), NODE_NONE)); vAddr.push_back(CAddress(ResolveService("250.1.1.4", 8333), NODE_NONE)); @@ -106,7 +106,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple) BOOST_AUTO_TEST_CASE(addrman_ports) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); @@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(addrman_ports) BOOST_AUTO_TEST_CASE(addrman_select) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); @@ -194,7 +194,7 @@ BOOST_AUTO_TEST_CASE(addrman_select) BOOST_AUTO_TEST_CASE(addrman_new_collisions) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); @@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions) BOOST_AUTO_TEST_CASE(addrman_new_multiplicity) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CAddress addr{CAddress(ResolveService("253.3.3.3", 8333), NODE_NONE)}; int64_t start_time{GetAdjustedTime()}; addr.nTime = start_time; @@ -255,7 +255,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_multiplicity) BOOST_AUTO_TEST_CASE(addrman_tried_collisions) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); @@ -286,7 +286,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions) BOOST_AUTO_TEST_CASE(addrman_getaddr) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); // Test: Sanity check, GetAddr should never return anything if addrman // is empty. @@ -606,11 +606,12 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) BOOST_AUTO_TEST_CASE(addrman_serialization) { std::vector asmap1 = FromBytes(asmap_raw, sizeof(asmap_raw) * 8); + NetGroupManager netgroupman{asmap1}; const auto ratio = GetCheckRatio(m_node); - auto addrman_asmap1 = std::make_unique(asmap1, DETERMINISTIC, ratio); - auto addrman_asmap1_dup = std::make_unique(asmap1, DETERMINISTIC, ratio); - auto addrman_noasmap = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, ratio); + auto addrman_asmap1 = std::make_unique(netgroupman, DETERMINISTIC, ratio); + auto addrman_asmap1_dup = std::make_unique(netgroupman, DETERMINISTIC, ratio); + auto addrman_noasmap = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, ratio); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); @@ -639,8 +640,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) BOOST_CHECK(addr_pos1.position != addr_pos3.position); // deserializing non-asmaped peers.dat to asmaped addrman - addrman_asmap1 = std::make_unique(asmap1, DETERMINISTIC, ratio); - addrman_noasmap = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, ratio); + addrman_asmap1 = std::make_unique(netgroupman, DETERMINISTIC, ratio); + addrman_noasmap = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, ratio); addrman_noasmap->Add({addr}, default_source); stream << *addrman_noasmap; stream >> *addrman_asmap1; @@ -651,8 +652,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) BOOST_CHECK(addr_pos4 == addr_pos2); // used to map to different buckets, now maps to the same bucket. - addrman_asmap1 = std::make_unique(asmap1, DETERMINISTIC, ratio); - addrman_noasmap = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, ratio); + addrman_asmap1 = std::make_unique(netgroupman, DETERMINISTIC, ratio); + addrman_noasmap = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, ratio); CAddress addr1 = CAddress(ResolveService("250.1.1.1"), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.2.1.1"), NODE_NONE); addrman_noasmap->Add({addr, addr2}, default_source); @@ -671,7 +672,7 @@ BOOST_AUTO_TEST_CASE(remove_invalid) { // Confirm that invalid addresses are ignored in unserialization. - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); const CAddress new1{ResolveService("5.5.5.5"), NODE_NONE}; @@ -703,14 +704,14 @@ BOOST_AUTO_TEST_CASE(remove_invalid) BOOST_REQUIRE(pos + sizeof(tried2_raw_replacement) <= stream.size()); memcpy(stream.data() + pos, tried2_raw_replacement, sizeof(tried2_raw_replacement)); - addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); stream >> *addrman; BOOST_CHECK_EQUAL(addrman->size(), 2); } BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); BOOST_CHECK(addrman->size() == 0); @@ -743,7 +744,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) BOOST_AUTO_TEST_CASE(addrman_noevict) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); // Add 35 addresses. CNetAddr source = ResolveIP("252.2.2.2"); @@ -795,7 +796,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) BOOST_AUTO_TEST_CASE(addrman_evictionworks) { - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); BOOST_CHECK(addrman->size() == 0); @@ -865,7 +866,7 @@ static CDataStream AddrmanToStream(const AddrMan& addrman) BOOST_AUTO_TEST_CASE(load_addrman) { - AddrMan addrman{EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)}; + AddrMan addrman{EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)}; CService addr1, addr2, addr3; BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false)); @@ -884,7 +885,7 @@ BOOST_AUTO_TEST_CASE(load_addrman) // Test that the de-serialization does not throw an exception. CDataStream ssPeers1 = AddrmanToStream(addrman); bool exceptionThrown = false; - AddrMan addrman1{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)}; + AddrMan addrman1{EMPTY_NETGROUPMAN, !DETERMINISTIC, GetCheckRatio(m_node)}; BOOST_CHECK(addrman1.size() == 0); try { @@ -901,7 +902,7 @@ BOOST_AUTO_TEST_CASE(load_addrman) // Test that ReadFromStream creates an addrman with the correct number of addrs. CDataStream ssPeers2 = AddrmanToStream(addrman); - AddrMan addrman2{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)}; + AddrMan addrman2{EMPTY_NETGROUPMAN, !DETERMINISTIC, GetCheckRatio(m_node)}; BOOST_CHECK(addrman2.size() == 0); ReadFromStream(addrman2, ssPeers2); BOOST_CHECK(addrman2.size() == 3); @@ -939,7 +940,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) // Test that the de-serialization of corrupted peers.dat throws an exception. CDataStream ssPeers1 = MakeCorruptPeersDat(); bool exceptionThrown = false; - AddrMan addrman1{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)}; + AddrMan addrman1{EMPTY_NETGROUPMAN, !DETERMINISTIC, GetCheckRatio(m_node)}; BOOST_CHECK(addrman1.size() == 0); try { unsigned char pchMsgTmp[4]; @@ -955,7 +956,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) // Test that ReadFromStream fails if peers.dat is corrupt CDataStream ssPeers2 = MakeCorruptPeersDat(); - AddrMan addrman2{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)}; + AddrMan addrman2{EMPTY_NETGROUPMAN, !DETERMINISTIC, GetCheckRatio(m_node)}; BOOST_CHECK(addrman2.size() == 0); BOOST_CHECK_THROW(ReadFromStream(addrman2, ssPeers2), std::ios_base::failure); } @@ -963,7 +964,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) BOOST_AUTO_TEST_CASE(addrman_update_address) { // Tests updating nTime via Connected() and nServices via SetServices() - auto addrman = std::make_unique(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source{ResolveIP("252.2.2.2")}; CAddress addr{CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE)}; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index f03ff5ba3a..646ae28032 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -49,7 +49,7 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup) BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { const CChainParams& chainparams = Params(); - auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); // Disable inactivity checks for this test to avoid interference static_cast(connman.get())->SetPeerConnectTimeout(99999s); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, @@ -139,7 +139,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) { NodeId id{0}; const CChainParams& chainparams = Params(); - auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, false); @@ -217,7 +217,7 @@ BOOST_AUTO_TEST_CASE(block_relay_only_eviction) { NodeId id{0}; const CChainParams& chainparams = Params(); - auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, false); @@ -280,7 +280,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) { const CChainParams& chainparams = Params(); auto banman = std::make_unique(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, false); @@ -396,7 +396,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) { const CChainParams& chainparams = Params(); auto banman = std::make_unique(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, false); diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index ba917dec2a..af7a282781 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -37,11 +37,19 @@ void initialize_addrman() g_setup = testing_setup.get(); } +[[nodiscard]] inline NetGroupManager ConsumeNetGroupManager(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + std::vector asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); + if (!SanityCheckASMap(asmap, 128)) asmap.clear(); + return NetGroupManager(asmap); +} + FUZZ_TARGET_INIT(data_stream_addr_man, initialize_addrman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; CDataStream data_stream = ConsumeDataStream(fuzzed_data_provider); - AddrMan addr_man{/*asmap=*/std::vector(), /*deterministic=*/false, GetCheckRatio()}; + NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; + AddrMan addr_man(netgroupman, /*deterministic=*/false, GetCheckRatio()); try { ReadFromStream(addr_man, data_stream); } catch (const std::exception&) { @@ -124,8 +132,8 @@ void FillAddrman(AddrMan& addrman, FuzzedDataProvider& fuzzed_data_provider) class AddrManDeterministic : public AddrMan { public: - explicit AddrManDeterministic(std::vector asmap, FuzzedDataProvider& fuzzed_data_provider) - : AddrMan{std::move(asmap), /*deterministic=*/true, GetCheckRatio()} + explicit AddrManDeterministic(const NetGroupManager& netgroupman, FuzzedDataProvider& fuzzed_data_provider) + : AddrMan(netgroupman, /*deterministic=*/true, GetCheckRatio()) { WITH_LOCK(m_impl->cs, m_impl->insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); } @@ -223,19 +231,12 @@ public: } }; -[[nodiscard]] inline std::vector ConsumeAsmap(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - std::vector asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); - if (!SanityCheckASMap(asmap, 128)) asmap.clear(); - return asmap; -} - FUZZ_TARGET_INIT(addrman, initialize_addrman) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); SetMockTime(ConsumeTime(fuzzed_data_provider)); - std::vector asmap = ConsumeAsmap(fuzzed_data_provider); - auto addr_man_ptr = std::make_unique(asmap, fuzzed_data_provider); + NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; + auto addr_man_ptr = std::make_unique(netgroupman, fuzzed_data_provider); if (fuzzed_data_provider.ConsumeBool()) { const std::vector serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)}; CDataStream ds(serialized_data, SER_DISK, INIT_PROTO_VERSION); @@ -244,7 +245,7 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) try { ds >> *addr_man_ptr; } catch (const std::ios_base::failure&) { - addr_man_ptr = std::make_unique(asmap, fuzzed_data_provider); + addr_man_ptr = std::make_unique(netgroupman, fuzzed_data_provider); } } AddrManDeterministic& addr_man = *addr_man_ptr; @@ -313,9 +314,9 @@ FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); SetMockTime(ConsumeTime(fuzzed_data_provider)); - std::vector asmap = ConsumeAsmap(fuzzed_data_provider); - AddrManDeterministic addr_man1{asmap, fuzzed_data_provider}; - AddrManDeterministic addr_man2{asmap, fuzzed_data_provider}; + NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; + AddrManDeterministic addr_man1{netgroupman, fuzzed_data_provider}; + AddrManDeterministic addr_man2{netgroupman, fuzzed_data_provider}; CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index a14d28f4ef..4406779015 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -19,12 +19,12 @@ #include namespace { -const BasicTestingSetup* g_setup; +const TestingSetup* g_setup; } // namespace void initialize_connman() { - static const auto testing_setup = MakeNoLogFileContext<>(); + static const auto testing_setup = MakeNoLogFileContext(); g_setup = testing_setup.get(); } @@ -32,10 +32,11 @@ FUZZ_TARGET_INIT(connman, initialize_connman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; SetMockTime(ConsumeTime(fuzzed_data_provider)); - AddrMan addrman(/*asmap=*/std::vector(), - /*deterministic=*/false, - g_setup->m_node.args->GetIntArg("-checkaddrman", 0)); - CConnman connman{fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral(), addrman, fuzzed_data_provider.ConsumeBool()}; + CConnman connman{fuzzed_data_provider.ConsumeIntegral(), + fuzzed_data_provider.ConsumeIntegral(), + *g_setup->m_node.addrman, + *g_setup->m_node.netgroupman, + fuzzed_data_provider.ConsumeBool()}; CNetAddr random_netaddr; CNode random_node = ConsumeNode(fuzzed_data_provider); CSubNet random_subnet; diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index ed6f172a2a..0a7d0c55bd 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -200,7 +201,8 @@ FUZZ_TARGET_DESERIALIZE(blockmerkleroot, { BlockMerkleRoot(block, &mutated); }) FUZZ_TARGET_DESERIALIZE(addrman_deserialize, { - AddrMan am(/*asmap=*/std::vector(), + NetGroupManager netgroupman{std::vector()}; + AddrMan am(netgroupman, /*deterministic=*/false, g_setup->m_node.args->GetIntArg("-checkaddrman", 0)); DeserializeFromFuzzingInput(buffer, am); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 13f17ca277..1830ec05af 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -180,6 +180,7 @@ ChainTestingSetup::~ChainTestingSetup() m_node.connman.reset(); m_node.banman.reset(); m_node.addrman.reset(); + m_node.netgroupman.reset(); m_node.args = nullptr; WITH_LOCK(::cs_main, UnloadBlockIndex(m_node.mempool.get(), *m_node.chainman)); m_node.mempool.reset(); @@ -223,11 +224,12 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector(/*asmap=*/std::vector(), + m_node.netgroupman = std::make_unique(/*asmap=*/std::vector()); + m_node.addrman = std::make_unique(*m_node.netgroupman, /*deterministic=*/false, m_node.args->GetIntArg("-checkaddrman", 0)); m_node.banman = std::make_unique(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - m_node.connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); // Deterministic randomness for tests. + m_node.connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); // Deterministic randomness for tests. m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman, m_node.banman.get(), *m_node.chainman, *m_node.mempool, false); -- cgit v1.2.3 From 6b2268162e96bc4fe1a3ebad454996b1d3d4615c Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 1 Sep 2021 12:12:52 +0100 Subject: [netgroupman] Add GetMappedAS() and GetGroup() These currently call through to the CNetAddr methods. The logic will be moved in a future commit. --- src/addrman.cpp | 34 ++++++++++++------------ src/addrman_impl.h | 8 +++--- src/net.cpp | 12 ++++----- src/netgroup.cpp | 10 ++++++++ src/netgroup.h | 6 +++++ src/test/addrman_tests.cpp | 64 ++++++++++++++++++++++------------------------ src/test/fuzz/asmap.cpp | 4 ++- src/test/netbase_tests.cpp | 27 +++++++++---------- 8 files changed, 91 insertions(+), 74 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 1f74faa682..c407187ccc 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -43,17 +43,17 @@ static constexpr size_t ADDRMAN_SET_TRIED_COLLISION_SIZE{10}; /** The maximum time we'll spend trying to resolve a tried table collision, in seconds */ static constexpr int64_t ADDRMAN_TEST_WINDOW{40*60}; // 40 minutes -int AddrInfo::GetTriedBucket(const uint256& nKey, const std::vector& asmap) const +int AddrInfo::GetTriedBucket(const uint256& nKey, const NetGroupManager& netgroupman) const { uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetKey()).GetCheapHash(); - uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetCheapHash(); + uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << netgroupman.GetGroup(*this) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetCheapHash(); return hash2 % ADDRMAN_TRIED_BUCKET_COUNT; } -int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector& asmap) const +int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const NetGroupManager& netgroupman) const { - std::vector vchSourceGroupKey = src.GetGroup(asmap); - uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash(); + std::vector vchSourceGroupKey = netgroupman.GetGroup(src); + uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << netgroupman.GetGroup(*this) << vchSourceGroupKey).GetCheapHash(); uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetCheapHash(); return hash2 % ADDRMAN_NEW_BUCKET_COUNT; } @@ -298,7 +298,7 @@ void AddrManImpl::Unserialize(Stream& s_) for (int n = 0; n < nTried; n++) { AddrInfo info; s >> info; - int nKBucket = info.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); + int nKBucket = info.GetTriedBucket(nKey, m_netgroupman); int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); if (info.IsValid() && vvTried[nKBucket][nKBucketPos] == -1) { @@ -371,7 +371,7 @@ void AddrManImpl::Unserialize(Stream& s_) } else { // In case the new table data cannot be used (bucket count wrong or new asmap), // try to give them a reference based on their primary source address. - bucket = info.GetNewBucket(nKey, m_netgroupman.GetAsmap()); + bucket = info.GetNewBucket(nKey, m_netgroupman); bucket_position = info.GetBucketPosition(nKey, true, bucket); if (vvNew[bucket][bucket_position] == -1) { vvNew[bucket][bucket_position] = entry_index; @@ -495,7 +495,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) AssertLockHeld(cs); // remove the entry from all new buckets - const int start_bucket{info.GetNewBucket(nKey, m_netgroupman.GetAsmap())}; + const int start_bucket{info.GetNewBucket(nKey, m_netgroupman)}; for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; ++n) { const int bucket{(start_bucket + n) % ADDRMAN_NEW_BUCKET_COUNT}; const int pos{info.GetBucketPosition(nKey, true, bucket)}; @@ -510,7 +510,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) assert(info.nRefCount == 0); // which tried bucket to move the entry to - int nKBucket = info.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); + int nKBucket = info.GetTriedBucket(nKey, m_netgroupman); int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); // first make space to add it (the existing tried entry there is moved to new, deleting whatever is there). @@ -526,7 +526,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) nTried--; // find which new bucket it belongs to - int nUBucket = infoOld.GetNewBucket(nKey, m_netgroupman.GetAsmap()); + int nUBucket = infoOld.GetNewBucket(nKey, m_netgroupman); int nUBucketPos = infoOld.GetBucketPosition(nKey, true, nUBucket); ClearNew(nUBucket, nUBucketPos); assert(vvNew[nUBucket][nUBucketPos] == -1); @@ -594,7 +594,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_ nNew++; } - int nUBucket = pinfo->GetNewBucket(nKey, source, m_netgroupman.GetAsmap()); + int nUBucket = pinfo->GetNewBucket(nKey, source, m_netgroupman); int nUBucketPos = pinfo->GetBucketPosition(nKey, true, nUBucket); bool fInsert = vvNew[nUBucket][nUBucketPos] == -1; if (vvNew[nUBucket][nUBucketPos] != nId) { @@ -650,7 +650,7 @@ bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT // which tried bucket to move the entry to - int tried_bucket = info.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); + int tried_bucket = info.GetTriedBucket(nKey, m_netgroupman); int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket); // Will moving this address into tried evict another entry? @@ -863,7 +863,7 @@ void AddrManImpl::ResolveCollisions_() AddrInfo& info_new = mapInfo[id_new]; // Which tried bucket to move the entry to. - int tried_bucket = info_new.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); + int tried_bucket = info_new.GetTriedBucket(nKey, m_netgroupman); int tried_bucket_pos = info_new.GetBucketPosition(nKey, false, tried_bucket); if (!info_new.IsValid()) { // id_new may no longer map to a valid address erase_collision = true; @@ -929,7 +929,7 @@ std::pair AddrManImpl::SelectTriedCollision_() const AddrInfo& newInfo = mapInfo[id_new]; // which tried bucket to move the entry to - int tried_bucket = newInfo.GetTriedBucket(nKey, m_netgroupman.GetAsmap()); + int tried_bucket = newInfo.GetTriedBucket(nKey, m_netgroupman); int tried_bucket_pos = newInfo.GetBucketPosition(nKey, false, tried_bucket); const AddrInfo& info_old = mapInfo[vvTried[tried_bucket][tried_bucket_pos]]; @@ -945,13 +945,13 @@ std::optional AddrManImpl::FindAddressEntry_(const CAddress& ad if (!addr_info) return std::nullopt; if(addr_info->fInTried) { - int bucket{addr_info->GetTriedBucket(nKey, m_netgroupman.GetAsmap())}; + int bucket{addr_info->GetTriedBucket(nKey, m_netgroupman)}; return AddressPosition(/*tried_in=*/true, /*multiplicity_in=*/1, /*bucket_in=*/bucket, /*position_in=*/addr_info->GetBucketPosition(nKey, false, bucket)); } else { - int bucket{addr_info->GetNewBucket(nKey, m_netgroupman.GetAsmap())}; + int bucket{addr_info->GetNewBucket(nKey, m_netgroupman)}; return AddressPosition(/*tried_in=*/false, /*multiplicity_in=*/addr_info->nRefCount, /*bucket_in=*/bucket, @@ -1026,7 +1026,7 @@ int AddrManImpl::CheckAddrman() const if (!setTried.count(vvTried[n][i])) return -11; const auto it{mapInfo.find(vvTried[n][i])}; - if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_netgroupman.GetAsmap()) != n) { + if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_netgroupman) != n) { return -17; } if (it->second.GetBucketPosition(nKey, false, n) != i) { diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 3db4be6c8d..9d98cdde54 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -76,15 +76,15 @@ public: } //! Calculate in which "tried" bucket this entry belongs - int GetTriedBucket(const uint256 &nKey, const std::vector &asmap) const; + int GetTriedBucket(const uint256& nKey, const NetGroupManager& netgroupman) const; //! Calculate in which "new" bucket this entry belongs, given a certain source - int GetNewBucket(const uint256 &nKey, const CNetAddr& src, const std::vector &asmap) const; + int GetNewBucket(const uint256& nKey, const CNetAddr& src, const NetGroupManager& netgroupman) const; //! Calculate in which "new" bucket this entry belongs, using its default source - int GetNewBucket(const uint256 &nKey, const std::vector &asmap) const + int GetNewBucket(const uint256& nKey, const NetGroupManager& netgroupman) const { - return GetNewBucket(nKey, source, asmap); + return GetNewBucket(nKey, source, netgroupman); } //! Calculate in which position of a bucket to store this entry. diff --git a/src/net.cpp b/src/net.cpp index e92694ad42..f16f2ecc25 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1997,7 +1997,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) case ConnectionType::BLOCK_RELAY: case ConnectionType::ADDR_FETCH: case ConnectionType::FEELER: - setConnected.insert(pnode->addr.GetGroup(m_netgroupman.GetAsmap())); + setConnected.insert(m_netgroupman.GetGroup(pnode->addr)); } // no default case, so the compiler can warn about missing cases } } @@ -2071,7 +2071,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) m_anchors.pop_back(); if (!addr.IsValid() || IsLocal(addr) || !IsReachable(addr) || !HasAllDesirableServiceFlags(addr.nServices) || - setConnected.count(addr.GetGroup(m_netgroupman.GetAsmap()))) continue; + setConnected.count(m_netgroupman.GetGroup(addr))) continue; addrConnect = addr; LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToString()); break; @@ -2112,7 +2112,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) } // Require outbound connections, other than feelers, to be to distinct network groups - if (!fFeeler && setConnected.count(addr.GetGroup(m_netgroupman.GetAsmap()))) { + if (!fFeeler && setConnected.count(m_netgroupman.GetGroup(addr))) { break; } @@ -2863,7 +2863,7 @@ void CConnman::GetNodeStats(std::vector& vstats) const for (CNode* pnode : m_nodes) { vstats.emplace_back(); pnode->CopyStats(vstats.back()); - vstats.back().m_mapped_as = pnode->addr.GetMappedAS(m_netgroupman.GetAsmap()); + vstats.back().m_mapped_as = m_netgroupman.GetMappedAS(pnode->addr); } } @@ -3096,9 +3096,9 @@ CSipHasher CConnman::GetDeterministicRandomizer(uint64_t id) const return CSipHasher(nSeed0, nSeed1).Write(id); } -uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& ad) const +uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& address) const { - std::vector vchNetGroup(ad.GetGroup(m_netgroupman.GetAsmap())); + std::vector vchNetGroup(m_netgroupman.GetGroup(address)); return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup.data(), vchNetGroup.size()).Finalize(); } diff --git a/src/netgroup.cpp b/src/netgroup.cpp index 17a48ad025..54c65d356c 100644 --- a/src/netgroup.cpp +++ b/src/netgroup.cpp @@ -3,3 +3,13 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include + +std::vector NetGroupManager::GetGroup(const CNetAddr& address) const +{ + return address.GetGroup(m_asmap); +} + +uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const +{ + return address.GetMappedAS(m_asmap); +} diff --git a/src/netgroup.h b/src/netgroup.h index fcf46bb2bb..46afc6e3c5 100644 --- a/src/netgroup.h +++ b/src/netgroup.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_NETGROUP_H #define BITCOIN_NETGROUP_H +#include + #include /** @@ -20,6 +22,10 @@ public: * exists, since the data is const. */ const std::vector& GetAsmap() const { return m_asmap; } + std::vector GetGroup(const CNetAddr& address) const; + + uint32_t GetMappedAS(const CNetAddr& address) const; + private: /** Compressed IP->ASN mapping, loaded from a file when a node starts. * diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 2a30d15e49..12cf1176a6 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -357,27 +357,25 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); - std::vector asmap; // use /16 - - BOOST_CHECK_EQUAL(info1.GetTriedBucket(nKey1, asmap), 40); + BOOST_CHECK_EQUAL(info1.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN), 40); // Test: Make sure key actually randomizes bucket placement. A fail on // this test could be a security issue. - BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info1.GetTriedBucket(nKey2, asmap)); + BOOST_CHECK(info1.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN) != info1.GetTriedBucket(nKey2, EMPTY_NETGROUPMAN)); // Test: Two addresses with same IP but different ports can map to // different buckets because they have different keys. AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); - BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info2.GetTriedBucket(nKey1, asmap)); + BOOST_CHECK(info1.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN) != info2.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN)); std::set buckets; for (int i = 0; i < 255; i++) { AddrInfo infoi = AddrInfo( CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), ResolveIP("250.1.1." + ToString(i))); - int bucket = infoi.GetTriedBucket(nKey1, asmap); + int bucket = infoi.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the same /16 prefix should @@ -389,7 +387,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250." + ToString(j) + ".1.1"), NODE_NONE), ResolveIP("250." + ToString(j) + ".1.1")); - int bucket = infoj.GetTriedBucket(nKey1, asmap); + int bucket = infoj.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the different /16 prefix should map to more than @@ -409,27 +407,25 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); - std::vector asmap; // use /16 - // Test: Make sure the buckets are what we expect - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), 786); - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, source1, asmap), 786); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, EMPTY_NETGROUPMAN), 786); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, source1, EMPTY_NETGROUPMAN), 786); // Test: Make sure key actually randomizes bucket placement. A fail on // this test could be a security issue. - BOOST_CHECK(info1.GetNewBucket(nKey1, asmap) != info1.GetNewBucket(nKey2, asmap)); + BOOST_CHECK(info1.GetNewBucket(nKey1, EMPTY_NETGROUPMAN) != info1.GetNewBucket(nKey2, EMPTY_NETGROUPMAN)); // Test: Ports should not affect bucket placement in the addr AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), info2.GetNewBucket(nKey1, asmap)); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, EMPTY_NETGROUPMAN), info2.GetNewBucket(nKey1, EMPTY_NETGROUPMAN)); std::set buckets; for (int i = 0; i < 255; i++) { AddrInfo infoi = AddrInfo( CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), ResolveIP("250.1.1." + ToString(i))); - int bucket = infoi.GetNewBucket(nKey1, asmap); + int bucket = infoi.GetNewBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the same group (\16 prefix for IPv4) should @@ -442,7 +438,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) ResolveService( ToString(250 + (j / 255)) + "." + ToString(j % 256) + ".1.1"), NODE_NONE), ResolveIP("251.4.1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the same source groups should map to NO MORE @@ -454,7 +450,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), ResolveIP("250." + ToString(p) + ".1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the different source groups should map to MORE @@ -475,6 +471,9 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) // 101.8.0.0/16 AS8 BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) { + std::vector asmap = FromBytes(asmap_raw, sizeof(asmap_raw) * 8); + NetGroupManager ngm_asmap{asmap}; + CAddress addr1 = CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.1.1", 9999), NODE_NONE); @@ -486,27 +485,25 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); - std::vector asmap = FromBytes(asmap_raw, sizeof(asmap_raw) * 8); - - BOOST_CHECK_EQUAL(info1.GetTriedBucket(nKey1, asmap), 236); + BOOST_CHECK_EQUAL(info1.GetTriedBucket(nKey1, ngm_asmap), 236); // Test: Make sure key actually randomizes bucket placement. A fail on // this test could be a security issue. - BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info1.GetTriedBucket(nKey2, asmap)); + BOOST_CHECK(info1.GetTriedBucket(nKey1, ngm_asmap) != info1.GetTriedBucket(nKey2, ngm_asmap)); // Test: Two addresses with same IP but different ports can map to // different buckets because they have different keys. AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); - BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info2.GetTriedBucket(nKey1, asmap)); + BOOST_CHECK(info1.GetTriedBucket(nKey1, ngm_asmap) != info2.GetTriedBucket(nKey1, ngm_asmap)); std::set buckets; for (int j = 0; j < 255; j++) { AddrInfo infoj = AddrInfo( CAddress(ResolveService("101." + ToString(j) + ".1.1"), NODE_NONE), ResolveIP("101." + ToString(j) + ".1.1")); - int bucket = infoj.GetTriedBucket(nKey1, asmap); + int bucket = infoj.GetTriedBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the different /16 prefix MAY map to more than @@ -518,7 +515,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250." + ToString(j) + ".1.1"), NODE_NONE), ResolveIP("250." + ToString(j) + ".1.1")); - int bucket = infoj.GetTriedBucket(nKey1, asmap); + int bucket = infoj.GetTriedBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the different /16 prefix MAY NOT map to more than @@ -528,6 +525,9 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) { + std::vector asmap = FromBytes(asmap_raw, sizeof(asmap_raw) * 8); + NetGroupManager ngm_asmap{asmap}; + CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.2.1", 9999), NODE_NONE); @@ -538,27 +538,25 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); - std::vector asmap = FromBytes(asmap_raw, sizeof(asmap_raw) * 8); - // Test: Make sure the buckets are what we expect - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), 795); - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, source1, asmap), 795); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, ngm_asmap), 795); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, source1, ngm_asmap), 795); // Test: Make sure key actually randomizes bucket placement. A fail on // this test could be a security issue. - BOOST_CHECK(info1.GetNewBucket(nKey1, asmap) != info1.GetNewBucket(nKey2, asmap)); + BOOST_CHECK(info1.GetNewBucket(nKey1, ngm_asmap) != info1.GetNewBucket(nKey2, ngm_asmap)); // Test: Ports should not affect bucket placement in the addr AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), info2.GetNewBucket(nKey1, asmap)); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, ngm_asmap), info2.GetNewBucket(nKey1, ngm_asmap)); std::set buckets; for (int i = 0; i < 255; i++) { AddrInfo infoi = AddrInfo( CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), ResolveIP("250.1.1." + ToString(i))); - int bucket = infoi.GetNewBucket(nKey1, asmap); + int bucket = infoi.GetNewBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the same /16 prefix @@ -571,7 +569,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) ResolveService( ToString(250 + (j / 255)) + "." + ToString(j % 256) + ".1.1"), NODE_NONE), ResolveIP("251.4.1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the same source /16 prefix should not map to more @@ -583,7 +581,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), ResolveIP("101." + ToString(p) + ".1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the different source /16 prefixes usually map to MORE @@ -595,7 +593,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), ResolveIP("250." + ToString(p) + ".1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the different source /16 prefixes sometimes map to NO MORE diff --git a/src/test/fuzz/asmap.cpp b/src/test/fuzz/asmap.cpp index 95be963dc8..1720f8e0ab 100644 --- a/src/test/fuzz/asmap.cpp +++ b/src/test/fuzz/asmap.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include @@ -56,5 +57,6 @@ FUZZ_TARGET(asmap) memcpy(&ipv4, addr_data, addr_size); net_addr.SetIP(CNetAddr{ipv4}); } - (void)net_addr.GetMappedAS(asmap); + NetGroupManager netgroupman{asmap}; + (void)netgroupman.GetMappedAS(net_addr); } diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 8e6e911ec2..224dc88d0f 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -315,22 +316,22 @@ BOOST_AUTO_TEST_CASE(subnet_test) BOOST_AUTO_TEST_CASE(netbase_getgroup) { - std::vector asmap; // use /16 - BOOST_CHECK(ResolveIP("127.0.0.1").GetGroup(asmap) == std::vector({0})); // Local -> !Routable() - BOOST_CHECK(ResolveIP("257.0.0.1").GetGroup(asmap) == std::vector({0})); // !Valid -> !Routable() - BOOST_CHECK(ResolveIP("10.0.0.1").GetGroup(asmap) == std::vector({0})); // RFC1918 -> !Routable() - BOOST_CHECK(ResolveIP("169.254.1.1").GetGroup(asmap) == std::vector({0})); // RFC3927 -> !Routable() - BOOST_CHECK(ResolveIP("1.2.3.4").GetGroup(asmap) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // IPv4 - BOOST_CHECK(ResolveIP("::FFFF:0:102:304").GetGroup(asmap) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // RFC6145 - BOOST_CHECK(ResolveIP("64:FF9B::102:304").GetGroup(asmap) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // RFC6052 - BOOST_CHECK(ResolveIP("2002:102:304:9999:9999:9999:9999:9999").GetGroup(asmap) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // RFC3964 - BOOST_CHECK(ResolveIP("2001:0:9999:9999:9999:9999:FEFD:FCFB").GetGroup(asmap) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // RFC4380 - BOOST_CHECK(ResolveIP("2001:470:abcd:9999:9999:9999:9999:9999").GetGroup(asmap) == std::vector({(unsigned char)NET_IPV6, 32, 1, 4, 112, 175})); //he.net - BOOST_CHECK(ResolveIP("2001:2001:9999:9999:9999:9999:9999:9999").GetGroup(asmap) == std::vector({(unsigned char)NET_IPV6, 32, 1, 32, 1})); //IPv6 + NetGroupManager netgroupman{std::vector()}; // use /16 + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("127.0.0.1")) == std::vector({0})); // Local -> !Routable() + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("257.0.0.1")) == std::vector({0})); // !Valid -> !Routable() + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("10.0.0.1")) == std::vector({0})); // RFC1918 -> !Routable() + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("169.254.1.1")) == std::vector({0})); // RFC3927 -> !Routable() + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("1.2.3.4")) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // IPv4 + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("::FFFF:0:102:304")) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // RFC6145 + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("64:FF9B::102:304")) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // RFC6052 + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("2002:102:304:9999:9999:9999:9999:9999")) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // RFC3964 + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("2001:0:9999:9999:9999:9999:FEFD:FCFB")) == std::vector({(unsigned char)NET_IPV4, 1, 2})); // RFC4380 + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("2001:470:abcd:9999:9999:9999:9999:9999")) == std::vector({(unsigned char)NET_IPV6, 32, 1, 4, 112, 175})); //he.net + BOOST_CHECK(netgroupman.GetGroup(ResolveIP("2001:2001:9999:9999:9999:9999:9999:9999")) == std::vector({(unsigned char)NET_IPV6, 32, 1, 32, 1})); //IPv6 // baz.net sha256 hash: 12929400eb4607c4ac075f087167e75286b179c693eb059a01774b864e8fe505 std::vector internal_group = {NET_INTERNAL, 0x12, 0x92, 0x94, 0x00, 0xeb, 0x46, 0x07, 0xc4, 0xac, 0x07}; - BOOST_CHECK(CreateInternal("baz.net").GetGroup(asmap) == internal_group); + BOOST_CHECK(netgroupman.GetGroup(CreateInternal("baz.net")) == internal_group); } BOOST_AUTO_TEST_CASE(netbase_parsenetwork) -- cgit v1.2.3 From ddb4101e6377a998b7c598bf52217b47698ddec9 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 7 Sep 2021 14:03:54 +0100 Subject: [net] Only use public CNetAddr functions and data in GetMappedAS() and GetGroup() Also change parameter/variable names. This makes the next commit mostly move-only. --- src/netaddress.cpp | 54 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/src/netaddress.cpp b/src/netaddress.cpp index f7640329f8..eab7bfdb8e 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -722,34 +722,36 @@ Network CNetAddr::GetNetClass() const return m_net; } -uint32_t CNetAddr::GetMappedAS(const std::vector &asmap) const { - uint32_t net_class = GetNetClass(); - if (asmap.size() == 0 || (net_class != NET_IPV4 && net_class != NET_IPV6)) { +uint32_t CNetAddr::GetMappedAS(const std::vector &m_asmap) const { + const CNetAddr& address = *this; + uint32_t net_class = address.GetNetClass(); + if (m_asmap.size() == 0 || (net_class != NET_IPV4 && net_class != NET_IPV6)) { return 0; // Indicates not found, safe because AS0 is reserved per RFC7607. } std::vector ip_bits(128); - if (HasLinkedIPv4()) { + if (address.HasLinkedIPv4()) { // For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits) for (int8_t byte_i = 0; byte_i < 12; ++byte_i) { for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { ip_bits[byte_i * 8 + bit_i] = (IPV4_IN_IPV6_PREFIX[byte_i] >> (7 - bit_i)) & 1; } } - uint32_t ipv4 = GetLinkedIPv4(); + uint32_t ipv4 = address.GetLinkedIPv4(); for (int i = 0; i < 32; ++i) { ip_bits[96 + i] = (ipv4 >> (31 - i)) & 1; } } else { // Use all 128 bits of the IPv6 address otherwise - assert(IsIPv6()); + assert(address.IsIPv6()); + auto addr_bytes = address.GetAddrBytes(); for (int8_t byte_i = 0; byte_i < 16; ++byte_i) { - uint8_t cur_byte = m_addr[byte_i]; + uint8_t cur_byte = addr_bytes[byte_i]; for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { ip_bits[byte_i * 8 + bit_i] = (cur_byte >> (7 - bit_i)) & 1; } } } - uint32_t mapped_as = Interpret(asmap, ip_bits); + uint32_t mapped_as = Interpret(m_asmap, ip_bits); return mapped_as; } @@ -763,13 +765,13 @@ uint32_t CNetAddr::GetMappedAS(const std::vector &asmap) const { * @note No two connections will be attempted to addresses with the same network * group. */ -std::vector CNetAddr::GetGroup(const std::vector &asmap) const +std::vector CNetAddr::GetGroup(const std::vector &m_asmap) const { + const CNetAddr& address = *this; std::vector vchRet; - uint32_t net_class = GetNetClass(); // If non-empty asmap is supplied and the address is IPv4/IPv6, // return ASN to be used for bucketing. - uint32_t asn = GetMappedAS(asmap); + uint32_t asn = GetMappedAS(m_asmap); if (asn != 0) { // Either asmap was empty, or address has non-asmappable net class (e.g. TOR). vchRet.push_back(NET_IPV6); // IPv4 and IPv6 with same ASN should be in the same bucket for (int i = 0; i < 4; i++) { @@ -778,31 +780,34 @@ std::vector CNetAddr::GetGroup(const std::vector &asmap) co return vchRet; } - vchRet.push_back(net_class); + vchRet.push_back(address.GetNetClass()); + int nStartByte{0}; int nBits{0}; - if (IsLocal()) { + if (address.IsLocal()) { // all local addresses belong to the same group - } else if (IsInternal()) { - // all internal-usage addresses get their own group + } else if (address.IsInternal()) { + // All internal-usage addresses get their own group. + // Skip over the INTERNAL_IN_IPV6_PREFIX returned by CAddress::GetAddrBytes(). + nStartByte = INTERNAL_IN_IPV6_PREFIX.size(); nBits = ADDR_INTERNAL_SIZE * 8; - } else if (!IsRoutable()) { + } else if (!address.IsRoutable()) { // all other unroutable addresses belong to the same group - } else if (HasLinkedIPv4()) { + } else if (address.HasLinkedIPv4()) { // IPv4 addresses (and mapped IPv4 addresses) use /16 groups - uint32_t ipv4 = GetLinkedIPv4(); + uint32_t ipv4 = address.GetLinkedIPv4(); vchRet.push_back((ipv4 >> 24) & 0xFF); vchRet.push_back((ipv4 >> 16) & 0xFF); return vchRet; - } else if (IsTor() || IsI2P()) { + } else if (address.IsTor() || address.IsI2P()) { nBits = 4; - } else if (IsCJDNS()) { + } else if (address.IsCJDNS()) { // Treat in the same way as Tor and I2P because the address in all of // them is "random" bytes (derived from a public key). However in CJDNS // the first byte is a constant 0xfc, so the random bytes come after it. // Thus skip the constant 8 bits at the start. nBits = 12; - } else if (IsHeNet()) { + } else if (address.IsHeNet()) { // for he.net, use /36 groups nBits = 36; } else { @@ -811,13 +816,14 @@ std::vector CNetAddr::GetGroup(const std::vector &asmap) co } // Push our address onto vchRet. + auto addr_bytes = address.GetAddrBytes(); const size_t num_bytes = nBits / 8; - vchRet.insert(vchRet.end(), m_addr.begin(), m_addr.begin() + num_bytes); + vchRet.insert(vchRet.end(), addr_bytes.begin() + nStartByte, addr_bytes.begin() + nStartByte + num_bytes); nBits %= 8; // ...for the last byte, push nBits and for the rest of the byte push 1's if (nBits > 0) { - assert(num_bytes < m_addr.size()); - vchRet.push_back(m_addr[num_bytes] | ((1 << (8 - nBits)) - 1)); + assert(num_bytes < addr_bytes.size()); + vchRet.push_back(addr_bytes[num_bytes] | ((1 << (8 - nBits)) - 1)); } return vchRet; -- cgit v1.2.3 From 1b978a7e8c71dcc1501705022e66f6779c8c4528 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 1 Sep 2021 15:55:32 +0100 Subject: [netgroupman] Move GetMappedAS() and GetGroup() logic to NetGroupManager Reviewer hint: use: `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space` --- src/addrman.cpp | 4 +- src/netaddress.cpp | 108 ----------------------------------------------------- src/netaddress.h | 6 --- src/netgroup.cpp | 92 ++++++++++++++++++++++++++++++++++++++++++++- src/netgroup.h | 15 ++++++++ 5 files changed, 107 insertions(+), 118 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index c407187ccc..7abec560eb 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -610,7 +610,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_ pinfo->nRefCount++; vvNew[nUBucket][nUBucketPos] = nId; LogPrint(BCLog::ADDRMAN, "Added %s mapped to AS%i to new[%i][%i]\n", - addr.ToString(), addr.GetMappedAS(m_netgroupman.GetAsmap()), nUBucket, nUBucketPos); + addr.ToString(), m_netgroupman.GetMappedAS(addr), nUBucket, nUBucketPos); } else { if (pinfo->nRefCount == 0) { Delete(nId); @@ -669,7 +669,7 @@ bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT // move nId to the tried tables MakeTried(info, nId); LogPrint(BCLog::ADDRMAN, "Moved %s mapped to AS%i to tried[%i][%i]\n", - addr.ToString(), addr.GetMappedAS(m_netgroupman.GetAsmap()), tried_bucket, tried_bucket_pos); + addr.ToString(), m_netgroupman.GetMappedAS(addr), tried_bucket, tried_bucket_pos); return true; } } diff --git a/src/netaddress.cpp b/src/netaddress.cpp index eab7bfdb8e..bc1915aad9 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include @@ -722,113 +721,6 @@ Network CNetAddr::GetNetClass() const return m_net; } -uint32_t CNetAddr::GetMappedAS(const std::vector &m_asmap) const { - const CNetAddr& address = *this; - uint32_t net_class = address.GetNetClass(); - if (m_asmap.size() == 0 || (net_class != NET_IPV4 && net_class != NET_IPV6)) { - return 0; // Indicates not found, safe because AS0 is reserved per RFC7607. - } - std::vector ip_bits(128); - if (address.HasLinkedIPv4()) { - // For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits) - for (int8_t byte_i = 0; byte_i < 12; ++byte_i) { - for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { - ip_bits[byte_i * 8 + bit_i] = (IPV4_IN_IPV6_PREFIX[byte_i] >> (7 - bit_i)) & 1; - } - } - uint32_t ipv4 = address.GetLinkedIPv4(); - for (int i = 0; i < 32; ++i) { - ip_bits[96 + i] = (ipv4 >> (31 - i)) & 1; - } - } else { - // Use all 128 bits of the IPv6 address otherwise - assert(address.IsIPv6()); - auto addr_bytes = address.GetAddrBytes(); - for (int8_t byte_i = 0; byte_i < 16; ++byte_i) { - uint8_t cur_byte = addr_bytes[byte_i]; - for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { - ip_bits[byte_i * 8 + bit_i] = (cur_byte >> (7 - bit_i)) & 1; - } - } - } - uint32_t mapped_as = Interpret(m_asmap, ip_bits); - return mapped_as; -} - -/** - * Get the canonical identifier of our network group - * - * The groups are assigned in a way where it should be costly for an attacker to - * obtain addresses with many different group identifiers, even if it is cheap - * to obtain addresses with the same identifier. - * - * @note No two connections will be attempted to addresses with the same network - * group. - */ -std::vector CNetAddr::GetGroup(const std::vector &m_asmap) const -{ - const CNetAddr& address = *this; - std::vector vchRet; - // If non-empty asmap is supplied and the address is IPv4/IPv6, - // return ASN to be used for bucketing. - uint32_t asn = GetMappedAS(m_asmap); - if (asn != 0) { // Either asmap was empty, or address has non-asmappable net class (e.g. TOR). - vchRet.push_back(NET_IPV6); // IPv4 and IPv6 with same ASN should be in the same bucket - for (int i = 0; i < 4; i++) { - vchRet.push_back((asn >> (8 * i)) & 0xFF); - } - return vchRet; - } - - vchRet.push_back(address.GetNetClass()); - int nStartByte{0}; - int nBits{0}; - - if (address.IsLocal()) { - // all local addresses belong to the same group - } else if (address.IsInternal()) { - // All internal-usage addresses get their own group. - // Skip over the INTERNAL_IN_IPV6_PREFIX returned by CAddress::GetAddrBytes(). - nStartByte = INTERNAL_IN_IPV6_PREFIX.size(); - nBits = ADDR_INTERNAL_SIZE * 8; - } else if (!address.IsRoutable()) { - // all other unroutable addresses belong to the same group - } else if (address.HasLinkedIPv4()) { - // IPv4 addresses (and mapped IPv4 addresses) use /16 groups - uint32_t ipv4 = address.GetLinkedIPv4(); - vchRet.push_back((ipv4 >> 24) & 0xFF); - vchRet.push_back((ipv4 >> 16) & 0xFF); - return vchRet; - } else if (address.IsTor() || address.IsI2P()) { - nBits = 4; - } else if (address.IsCJDNS()) { - // Treat in the same way as Tor and I2P because the address in all of - // them is "random" bytes (derived from a public key). However in CJDNS - // the first byte is a constant 0xfc, so the random bytes come after it. - // Thus skip the constant 8 bits at the start. - nBits = 12; - } else if (address.IsHeNet()) { - // for he.net, use /36 groups - nBits = 36; - } else { - // for the rest of the IPv6 network, use /32 groups - nBits = 32; - } - - // Push our address onto vchRet. - auto addr_bytes = address.GetAddrBytes(); - const size_t num_bytes = nBits / 8; - vchRet.insert(vchRet.end(), addr_bytes.begin() + nStartByte, addr_bytes.begin() + nStartByte + num_bytes); - nBits %= 8; - // ...for the last byte, push nBits and for the rest of the byte push 1's - if (nBits > 0) { - assert(num_bytes < addr_bytes.size()); - vchRet.push_back(addr_bytes[num_bytes] | ((1 << (8 - nBits)) - 1)); - } - - return vchRet; -} - std::vector CNetAddr::GetAddrBytes() const { if (IsAddrV1Compatible()) { diff --git a/src/netaddress.h b/src/netaddress.h index 6d21dcd5cd..b9a8dc589a 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -202,12 +202,6 @@ public: //! Whether this address has a linked IPv4 address (see GetLinkedIPv4()). bool HasLinkedIPv4() const; - // The AS on the BGP path to the node we use to diversify - // peers in AddrMan bucketing based on the AS infrastructure. - // The ip->AS mapping depends on how asmap is constructed. - uint32_t GetMappedAS(const std::vector& asmap) const; - - std::vector GetGroup(const std::vector& asmap) const; std::vector GetAddrBytes() const; int GetReachabilityFrom(const CNetAddr* paddrPartner = nullptr) const; diff --git a/src/netgroup.cpp b/src/netgroup.cpp index 54c65d356c..a2f1f3460f 100644 --- a/src/netgroup.cpp +++ b/src/netgroup.cpp @@ -4,12 +4,100 @@ #include +#include + std::vector NetGroupManager::GetGroup(const CNetAddr& address) const { - return address.GetGroup(m_asmap); + std::vector vchRet; + // If non-empty asmap is supplied and the address is IPv4/IPv6, + // return ASN to be used for bucketing. + uint32_t asn = GetMappedAS(address); + if (asn != 0) { // Either asmap was empty, or address has non-asmappable net class (e.g. TOR). + vchRet.push_back(NET_IPV6); // IPv4 and IPv6 with same ASN should be in the same bucket + for (int i = 0; i < 4; i++) { + vchRet.push_back((asn >> (8 * i)) & 0xFF); + } + return vchRet; + } + + vchRet.push_back(address.GetNetClass()); + int nStartByte{0}; + int nBits{0}; + + if (address.IsLocal()) { + // all local addresses belong to the same group + } else if (address.IsInternal()) { + // All internal-usage addresses get their own group. + // Skip over the INTERNAL_IN_IPV6_PREFIX returned by CAddress::GetAddrBytes(). + nStartByte = INTERNAL_IN_IPV6_PREFIX.size(); + nBits = ADDR_INTERNAL_SIZE * 8; + } else if (!address.IsRoutable()) { + // all other unroutable addresses belong to the same group + } else if (address.HasLinkedIPv4()) { + // IPv4 addresses (and mapped IPv4 addresses) use /16 groups + uint32_t ipv4 = address.GetLinkedIPv4(); + vchRet.push_back((ipv4 >> 24) & 0xFF); + vchRet.push_back((ipv4 >> 16) & 0xFF); + return vchRet; + } else if (address.IsTor() || address.IsI2P()) { + nBits = 4; + } else if (address.IsCJDNS()) { + // Treat in the same way as Tor and I2P because the address in all of + // them is "random" bytes (derived from a public key). However in CJDNS + // the first byte is a constant 0xfc, so the random bytes come after it. + // Thus skip the constant 8 bits at the start. + nBits = 12; + } else if (address.IsHeNet()) { + // for he.net, use /36 groups + nBits = 36; + } else { + // for the rest of the IPv6 network, use /32 groups + nBits = 32; + } + + // Push our address onto vchRet. + auto addr_bytes = address.GetAddrBytes(); + const size_t num_bytes = nBits / 8; + vchRet.insert(vchRet.end(), addr_bytes.begin() + nStartByte, addr_bytes.begin() + nStartByte + num_bytes); + nBits %= 8; + // ...for the last byte, push nBits and for the rest of the byte push 1's + if (nBits > 0) { + assert(num_bytes < addr_bytes.size()); + vchRet.push_back(addr_bytes[num_bytes] | ((1 << (8 - nBits)) - 1)); + } + + return vchRet; } uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const { - return address.GetMappedAS(m_asmap); + uint32_t net_class = address.GetNetClass(); + if (m_asmap.size() == 0 || (net_class != NET_IPV4 && net_class != NET_IPV6)) { + return 0; // Indicates not found, safe because AS0 is reserved per RFC7607. + } + std::vector ip_bits(128); + if (address.HasLinkedIPv4()) { + // For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits) + for (int8_t byte_i = 0; byte_i < 12; ++byte_i) { + for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { + ip_bits[byte_i * 8 + bit_i] = (IPV4_IN_IPV6_PREFIX[byte_i] >> (7 - bit_i)) & 1; + } + } + uint32_t ipv4 = address.GetLinkedIPv4(); + for (int i = 0; i < 32; ++i) { + ip_bits[96 + i] = (ipv4 >> (31 - i)) & 1; + } + } else { + // Use all 128 bits of the IPv6 address otherwise + assert(address.IsIPv6()); + auto addr_bytes = address.GetAddrBytes(); + for (int8_t byte_i = 0; byte_i < 16; ++byte_i) { + uint8_t cur_byte = addr_bytes[byte_i]; + for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { + ip_bits[byte_i * 8 + bit_i] = (cur_byte >> (7 - bit_i)) & 1; + } + } + } + uint32_t mapped_as = Interpret(m_asmap, ip_bits); + return mapped_as; } diff --git a/src/netgroup.h b/src/netgroup.h index 46afc6e3c5..4e4edb8a88 100644 --- a/src/netgroup.h +++ b/src/netgroup.h @@ -22,8 +22,23 @@ public: * exists, since the data is const. */ const std::vector& GetAsmap() const { return m_asmap; } + /** + * Get the canonical identifier of the network group for address. + * + * The groups are assigned in a way where it should be costly for an attacker to + * obtain addresses with many different group identifiers, even if it is cheap + * to obtain addresses with the same identifier. + * + * @note No two connections will be attempted to addresses with the same network + * group. + */ std::vector GetGroup(const CNetAddr& address) const; + /** + * Get the autonomous system on the BGP path to address. + * + * The ip->AS mapping depends on how asmap is constructed. + */ uint32_t GetMappedAS(const CNetAddr& address) const; private: -- cgit v1.2.3 From 4709fc2019e27e74be02dc5fc123b9f6f46d7990 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 1 Sep 2021 16:34:23 +0100 Subject: [netgroupman] Move asmap checksum calculation to NetGroupManager --- src/addrman.cpp | 11 ++--------- src/netgroup.cpp | 8 ++++++++ src/netgroup.h | 4 ++++ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 7abec560eb..f74729d47b 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -218,11 +218,7 @@ void AddrManImpl::Serialize(Stream& s_) const } // Store asmap checksum after bucket entries so that it // can be ignored by older clients for backward compatibility. - uint256 asmap_checksum; - if (m_netgroupman.GetAsmap().size() != 0) { - asmap_checksum = SerializeHash(m_netgroupman.GetAsmap()); - } - s << asmap_checksum; + s << m_netgroupman.GetAsmapChecksum(); } template @@ -335,10 +331,7 @@ void AddrManImpl::Unserialize(Stream& s_) // If the bucket count and asmap checksum haven't changed, then attempt // to restore the entries to the buckets/positions they were in before // serialization. - uint256 supplied_asmap_checksum; - if (m_netgroupman.GetAsmap().size() != 0) { - supplied_asmap_checksum = SerializeHash(m_netgroupman.GetAsmap()); - } + uint256 supplied_asmap_checksum{m_netgroupman.GetAsmapChecksum()}; uint256 serialized_asmap_checksum; if (format >= Format::V2_ASMAP) { s >> serialized_asmap_checksum; diff --git a/src/netgroup.cpp b/src/netgroup.cpp index a2f1f3460f..5f42d6c719 100644 --- a/src/netgroup.cpp +++ b/src/netgroup.cpp @@ -4,8 +4,16 @@ #include +#include #include +uint256 NetGroupManager::GetAsmapChecksum() const +{ + if (!m_asmap.size()) return {}; + + return SerializeHash(m_asmap); +} + std::vector NetGroupManager::GetGroup(const CNetAddr& address) const { std::vector vchRet; diff --git a/src/netgroup.h b/src/netgroup.h index 4e4edb8a88..6f495b166b 100644 --- a/src/netgroup.h +++ b/src/netgroup.h @@ -6,6 +6,7 @@ #define BITCOIN_NETGROUP_H #include +#include #include @@ -22,6 +23,9 @@ public: * exists, since the data is const. */ const std::vector& GetAsmap() const { return m_asmap; } + /** Get a checksum identifying the asmap being used. */ + uint256 GetAsmapChecksum() const; + /** * Get the canonical identifier of the network group for address. * -- cgit v1.2.3 From 36f814c0e84d009c0e0aa26981a20ac4cf338a85 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 1 Sep 2021 16:35:45 +0100 Subject: [netgroupman] Remove NetGroupManager::GetAsmap() asmap no longer needs to be exposed anywhere outside NetGroupManager. --- src/netgroup.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/netgroup.h b/src/netgroup.h index 6f495b166b..2dd63ec66b 100644 --- a/src/netgroup.h +++ b/src/netgroup.h @@ -19,10 +19,6 @@ public: : m_asmap{std::move(asmap)} {} - /* Get a reference to (const) asmap. May be held as long as NetGroupManager - * exists, since the data is const. */ - const std::vector& GetAsmap() const { return m_asmap; } - /** Get a checksum identifying the asmap being used. */ uint256 GetAsmapChecksum() const; -- cgit v1.2.3