From 8806c7baeb4a8fc97c80d80d018af4be74a9e8d3 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sat, 6 Jun 2020 13:28:47 -0400 Subject: [PATCH 01/14] merge bitcoin#19192: Extract net permissions doc --- src/init.cpp | 7 ++----- src/net_permissions.cpp | 8 ++++++++ src/net_permissions.h | 7 ++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index b923cde9e91a..27be2d13edec 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -58,6 +58,7 @@ #include #include #include +#include #include #include #include @@ -607,11 +608,7 @@ void SetupServerArgs(NodeContext& node) hidden_args.emplace_back("-natpmp"); #endif // USE_NATPMP argsman.AddArg("-whitebind=<[permissions@]addr>", "Bind to given address and whitelist peers connecting to it. " - "Use [host]:port notation for IPv6. Allowed permissions are bloomfilter (allow requesting BIP37 filtered blocks and transactions), " - "noban (do not ban for misbehavior), " - "forcerelay (relay transactions that are already in the mempool; implies relay), " - "relay (relay even in -blocksonly mode), " - "and mempool (allow requesting BIP35 mempool contents). " + "Use [host]:port notation for IPv6. Allowed permissions: " + Join(NET_PERMISSIONS_DOC, ", ") + ". " "Specify multiple permissions separated by commas (default: noban,mempool,relay). Can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-whitelist=<[permissions@]IP address or network>", "Whitelist peers connecting from the given IP address (e.g. 1.2.3.4) or " diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index 2ae4a44c85f6..5ae32b299946 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -8,6 +8,14 @@ #include #include +const std::vector NET_PERMISSIONS_DOC{ + "bloomfilter (allow requesting BIP37 filtered blocks and transactions)", + "noban (do not ban for misbehavior)", + "forcerelay (relay transactions that are already in the mempool; implies relay)", + "relay (relay even in -blocksonly mode)", + "mempool (allow requesting BIP35 mempool contents)", +}; + namespace { // Parse the following format: "perm1,perm2@xxxxxx" diff --git a/src/net_permissions.h b/src/net_permissions.h index 4066c7313388..71549ed4d399 100644 --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -2,15 +2,18 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include + #include #include -#include #ifndef BITCOIN_NET_PERMISSIONS_H #define BITCOIN_NET_PERMISSIONS_H struct bilingual_str; +extern const std::vector NET_PERMISSIONS_DOC; + enum NetPermissionFlags { PF_NONE = 0, @@ -30,6 +33,7 @@ enum NetPermissionFlags PF_ISIMPLICIT = (1U << 31), PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL, }; + class NetPermissions { public: @@ -48,6 +52,7 @@ class NetPermissions flags = static_cast(flags & ~f); } }; + class NetWhitebindPermissions : public NetPermissions { public: From 91d800dce3d568e68caeb21314e6b40c4ed20dde Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Thu, 21 May 2020 10:41:19 -0400 Subject: [PATCH 02/14] merge bitcoin#18968: noban precludes maxuploadtarget disconnects --- doc/reduce-traffic.md | 2 +- src/init.cpp | 2 +- src/net.cpp | 2 +- src/net_processing.cpp | 8 +++++--- test/functional/feature_maxuploadtarget.py | 7 +++---- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/doc/reduce-traffic.md b/doc/reduce-traffic.md index 1dd733b659a5..48fcb3131c85 100644 --- a/doc/reduce-traffic.md +++ b/doc/reduce-traffic.md @@ -23,7 +23,7 @@ longer serving historic blocks (blocks older than one week). Keep in mind that new nodes require other nodes that are willing to serve historic blocks. -Whitelisted peers will never be disconnected, although their traffic counts for +Peers with the `noban` permission will never be disconnected, although their traffic counts for calculating the target. ## 2. Disable "listening" (`-listen=0`) diff --git a/src/init.cpp b/src/init.cpp index 27be2d13edec..c51722eb5513 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -578,7 +578,7 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-maxreceivebuffer=", strprintf("Maximum per-connection receive buffer, *1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxsendbuffer=", strprintf("Maximum per-connection send buffer, *1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-maxuploadtarget=", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h), 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-maxuploadtarget=", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'noban' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onion=", "Use separate SOCKS5 proxy to reach peers via Tor hidden services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onlynet=", "Make outgoing connections only through network (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); diff --git a/src/net.cpp b/src/net.cpp index ee3163628b5c..c3209fa5c197 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3686,7 +3686,7 @@ void CConnman::RecordBytesSent(uint64_t bytes) nMaxOutboundTotalBytesSentInCycle = 0; } - // TODO, exclude whitebind peers + // TODO, exclude peers with noban permission nMaxOutboundTotalBytesSentInCycle += bytes; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0b1d7c5f1374..2eca64762dd4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1892,9 +1892,11 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c } const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); // disconnect node in case we have reached the outbound limit for serving historical blocks - // never disconnect whitelisted nodes - if (send && connman.OutboundTargetReached(true) && ( ((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom.HasPermission(PF_NOBAN)) - { + if (send && + connman.OutboundTargetReached(true) && + (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && + !pfrom.HasPermission(PF_NOBAN) // never disconnect nodes with the noban permission + ) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); //disconnect node diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py index 6641f2ced615..cafcbf35b139 100755 --- a/test/functional/feature_maxuploadtarget.py +++ b/test/functional/feature_maxuploadtarget.py @@ -144,8 +144,7 @@ def run_test(self): self.nodes[0].disconnect_p2ps() self.log.info("Restarting node 0 with noban permission and 1MB maxuploadtarget") - self.stop_node(0) - self.start_node(0, ["-whitelist=noban@127.0.0.1", "-maxuploadtarget=1", "-blockmaxsize=999000", "-maxtipage="+str(2*60*60*24*7), "-mocktime="+str(current_mocktime)]) + self.restart_node(0, ["-whitelist=noban@127.0.0.1", "-maxuploadtarget=1", "-blockmaxsize=999000", "-maxtipage="+str(2*60*60*24*7), "-mocktime="+str(current_mocktime)]) # Reconnect to self.nodes[0] self.nodes[0].add_p2p_connection(TestP2PConn()) @@ -158,9 +157,9 @@ def run_test(self): getdata_request.inv = [CInv(MSG_BLOCK, big_old_block)] self.nodes[0].p2p.send_and_ping(getdata_request) - assert_equal(len(self.nodes[0].getpeerinfo()), 1) #node is still connected because of the whitelist + assert_equal(len(self.nodes[0].getpeerinfo()), 1) #node is still connected because of the noban permission - self.log.info("Peer still connected after trying to download old block (whitelisted)") + self.log.info("Peer still connected after trying to download old block (noban permission)") if __name__ == '__main__': MaxUploadTest().main() From a13b72397b2a375aecb45642fd5694a8d34158a6 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sat, 6 Jun 2020 17:07:25 +0200 Subject: [PATCH 03/14] merge bitcoin#19191: Extract download permission from noban --- doc/reduce-traffic.md | 2 +- doc/release-notes-19191.md | 7 +++++++ src/init.cpp | 10 +++++----- src/net.cpp | 2 +- src/net_permissions.cpp | 5 ++++- src/net_permissions.h | 9 +++++---- src/net_processing.cpp | 4 ++-- src/test/netbase_tests.cpp | 3 ++- test/functional/feature_maxuploadtarget.py | 11 +++++++---- test/functional/p2p_blocksonly.py | 4 ++-- test/functional/p2p_permissions.py | 17 +++++++++-------- 11 files changed, 45 insertions(+), 29 deletions(-) create mode 100644 doc/release-notes-19191.md diff --git a/doc/reduce-traffic.md b/doc/reduce-traffic.md index 48fcb3131c85..5f135b1566f8 100644 --- a/doc/reduce-traffic.md +++ b/doc/reduce-traffic.md @@ -23,7 +23,7 @@ longer serving historic blocks (blocks older than one week). Keep in mind that new nodes require other nodes that are willing to serve historic blocks. -Peers with the `noban` permission will never be disconnected, although their traffic counts for +Peers with the `download` permission will never be disconnected, although their traffic counts for calculating the target. ## 2. Disable "listening" (`-listen=0`) diff --git a/doc/release-notes-19191.md b/doc/release-notes-19191.md new file mode 100644 index 000000000000..8df29f993146 --- /dev/null +++ b/doc/release-notes-19191.md @@ -0,0 +1,7 @@ +Updated settings +---------------- + +- A `download` permission has been extracted from the `noban` permission. For + compatibility, `noban` implies the `download` permission, but this may change + in future releases. Refer to the help of the affected settings `-whitebind` + and `-whitelist` for more details. (#19191) diff --git a/src/init.cpp b/src/init.cpp index c51722eb5513..a577223611c1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -578,7 +578,7 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-maxreceivebuffer=", strprintf("Maximum per-connection receive buffer, *1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxsendbuffer=", strprintf("Maximum per-connection send buffer, *1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-maxuploadtarget=", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'noban' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-maxuploadtarget=", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'download' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onion=", "Use separate SOCKS5 proxy to reach peers via Tor hidden services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onlynet=", "Make outgoing connections only through network (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -607,12 +607,12 @@ void SetupServerArgs(NodeContext& node) #else hidden_args.emplace_back("-natpmp"); #endif // USE_NATPMP - argsman.AddArg("-whitebind=<[permissions@]addr>", "Bind to given address and whitelist peers connecting to it. " + argsman.AddArg("-whitebind=<[permissions@]addr>", "Bind to the given address and add permission flags to the peers connecting to it. " "Use [host]:port notation for IPv6. Allowed permissions: " + Join(NET_PERMISSIONS_DOC, ", ") + ". " - "Specify multiple permissions separated by commas (default: noban,mempool,relay). Can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + "Specify multiple permissions separated by commas (default: download,noban,mempool,relay). Can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-whitelist=<[permissions@]IP address or network>", "Whitelist peers connecting from the given IP address (e.g. 1.2.3.4) or " - "CIDR notated network(e.g. 1.2.3.0/24). Uses same permissions as " + argsman.AddArg("-whitelist=<[permissions@]IP address or network>", "Add permission flags to the peers connecting from the given IP address (e.g. 1.2.3.4) or " + "CIDR-notated network (e.g. 1.2.3.0/24). Uses the same permissions as " "-whitebind. Can be specified multiple times." , ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); g_wallet_init_interface.AddWalletOptions(argsman); diff --git a/src/net.cpp b/src/net.cpp index c3209fa5c197..a5b28c6d722c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3686,7 +3686,7 @@ void CConnman::RecordBytesSent(uint64_t bytes) nMaxOutboundTotalBytesSentInCycle = 0; } - // TODO, exclude peers with noban permission + // TODO, exclude peers with download permission nMaxOutboundTotalBytesSentInCycle += bytes; } diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index 5ae32b299946..0973d664253e 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -10,10 +10,11 @@ const std::vector NET_PERMISSIONS_DOC{ "bloomfilter (allow requesting BIP37 filtered blocks and transactions)", - "noban (do not ban for misbehavior)", + "noban (do not ban for misbehavior; implies download)", "forcerelay (relay transactions that are already in the mempool; implies relay)", "relay (relay even in -blocksonly mode)", "mempool (allow requesting BIP35 mempool contents)", + "download (allow getheaders during IBD, no disconnect after maxuploadtarget limit)", }; namespace { @@ -46,6 +47,7 @@ bool TryParsePermissionFlags(const std::string& str, NetPermissionFlags& output, else if (permission == "noban") NetPermissions::AddFlag(flags, PF_NOBAN); else if (permission == "forcerelay") NetPermissions::AddFlag(flags, PF_FORCERELAY); else if (permission == "mempool") NetPermissions::AddFlag(flags, PF_MEMPOOL); + else if (permission == "download") NetPermissions::AddFlag(flags, PF_DOWNLOAD); else if (permission == "all") NetPermissions::AddFlag(flags, PF_ALL); else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY); else if (permission.length() == 0); // Allow empty entries @@ -72,6 +74,7 @@ std::vector NetPermissions::ToStrings(NetPermissionFlags flags) if (NetPermissions::HasFlag(flags, PF_FORCERELAY)) strings.push_back("forcerelay"); if (NetPermissions::HasFlag(flags, PF_RELAY)) strings.push_back("relay"); if (NetPermissions::HasFlag(flags, PF_MEMPOOL)) strings.push_back("mempool"); + if (NetPermissions::HasFlag(flags, PF_DOWNLOAD)) strings.push_back("download"); return strings; } diff --git a/src/net_permissions.h b/src/net_permissions.h index 71549ed4d399..78efc97dbe37 100644 --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -14,8 +14,7 @@ struct bilingual_str; extern const std::vector NET_PERMISSIONS_DOC; -enum NetPermissionFlags -{ +enum NetPermissionFlags { PF_NONE = 0, // Can query bloomfilter even if -peerbloomfilters is false PF_BLOOMFILTER = (1U << 1), @@ -24,14 +23,16 @@ enum NetPermissionFlags // Always relay transactions from this peer, even if already in mempool // Keep parameter interaction: forcerelay implies relay PF_FORCERELAY = (1U << 2) | PF_RELAY, + // Allow getheaders during IBD and block-download after maxuploadtarget limit + PF_DOWNLOAD = (1U << 6), // Can't be banned/disconnected/discouraged for misbehavior - PF_NOBAN = (1U << 4), + PF_NOBAN = (1U << 4) | PF_DOWNLOAD, // Can query the mempool PF_MEMPOOL = (1U << 5), // True if the user did not specifically set fine grained permissions PF_ISIMPLICIT = (1U << 31), - PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL, + PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL | PF_DOWNLOAD, }; class NetPermissions diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2eca64762dd4..a47a45a75458 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1895,7 +1895,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c if (send && connman.OutboundTargetReached(true) && (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && - !pfrom.HasPermission(PF_NOBAN) // never disconnect nodes with the noban permission + !pfrom.HasPermission(PF_DOWNLOAD) // nodes with the download permission may exceed target ) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); @@ -3407,7 +3407,7 @@ void PeerManagerImpl::ProcessMessage( } LOCK(cs_main); - if (::ChainstateActive().IsInitialBlockDownload() && !pfrom.HasPermission(PF_NOBAN)) { + if (::ChainstateActive().IsInitialBlockDownload() && !pfrom.HasPermission(PF_DOWNLOAD)) { LogPrint(BCLog::NET, "Ignoring %s from peer=%d because node is in initial block download\n", msg_type, pfrom.GetId()); return; } diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index d41b89ad3dd3..43f1ff27a989 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -521,12 +521,13 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay,mempool@1.2.3.4/32", whitelistPermissions, error)); const auto strings = NetPermissions::ToStrings(PF_ALL); - BOOST_CHECK_EQUAL(strings.size(), 5); + BOOST_CHECK_EQUAL(strings.size(), 6); BOOST_CHECK(std::find(strings.begin(), strings.end(), "bloomfilter") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "forcerelay") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "relay") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "noban") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "mempool") != strings.end()); + BOOST_CHECK(std::find(strings.begin(), strings.end(), "download") != strings.end()); } BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters) diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py index cafcbf35b139..1f57f41afb1f 100755 --- a/test/functional/feature_maxuploadtarget.py +++ b/test/functional/feature_maxuploadtarget.py @@ -143,8 +143,8 @@ def run_test(self): self.nodes[0].disconnect_p2ps() - self.log.info("Restarting node 0 with noban permission and 1MB maxuploadtarget") - self.restart_node(0, ["-whitelist=noban@127.0.0.1", "-maxuploadtarget=1", "-blockmaxsize=999000", "-maxtipage="+str(2*60*60*24*7), "-mocktime="+str(current_mocktime)]) + self.log.info("Restarting node 0 with download permission and 1MB maxuploadtarget") + self.restart_node(0, ["-whitelist=download@127.0.0.1", "-maxuploadtarget=1", "-blockmaxsize=999000", "-maxtipage="+str(2*60*60*24*7), "-mocktime="+str(current_mocktime)]) # Reconnect to self.nodes[0] self.nodes[0].add_p2p_connection(TestP2PConn()) @@ -157,9 +157,12 @@ def run_test(self): getdata_request.inv = [CInv(MSG_BLOCK, big_old_block)] self.nodes[0].p2p.send_and_ping(getdata_request) - assert_equal(len(self.nodes[0].getpeerinfo()), 1) #node is still connected because of the noban permission - self.log.info("Peer still connected after trying to download old block (noban permission)") + self.log.info("Peer still connected after trying to download old block (download permission)") + peer_info = self.nodes[0].getpeerinfo() + assert_equal(len(peer_info), 1) # node is still connected + assert_equal(peer_info[0]['permissions'], ['download']) + if __name__ == '__main__': MaxUploadTest().main() diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py index b28ffadcb678..f0e716e84f35 100755 --- a/test/functional/p2p_blocksonly.py +++ b/test/functional/p2p_blocksonly.py @@ -66,10 +66,10 @@ def run_test(self): second_peer = self.nodes[0].add_p2p_connection(P2PInterface()) peer_1_info = self.nodes[0].getpeerinfo()[0] assert_equal(peer_1_info['whitelisted'], True) - assert_equal(peer_1_info['permissions'], ['noban', 'forcerelay', 'relay', 'mempool']) + assert_equal(peer_1_info['permissions'], ['noban', 'forcerelay', 'relay', 'mempool', 'download']) peer_2_info = self.nodes[0].getpeerinfo()[1] assert_equal(peer_2_info['whitelisted'], True) - assert_equal(peer_2_info['permissions'], ['noban', 'forcerelay', 'relay', 'mempool']) + assert_equal(peer_2_info['permissions'], ['noban', 'forcerelay', 'relay', 'mempool', 'download']) assert_equal(self.nodes[0].testmempoolaccept([sigtx])[0]['allowed'], True) txid = self.nodes[0].testmempoolaccept([sigtx])[0]['txid'] diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index a724201f298b..c5ad7111e592 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -37,7 +37,8 @@ def run_test(self): self.checkpermission( # default permissions (no specific permissions) ["-whitelist=127.0.0.1"], - ["relay", "noban", "mempool"], + # Make sure the default values in the command line documentation match the ones here + ["relay", "noban", "mempool", "download"], True) self.checkpermission( @@ -49,7 +50,7 @@ def run_test(self): self.checkpermission( # relay permission removed (no specific permissions) ["-whitelist=127.0.0.1", "-whitelistrelay=0"], - ["noban", "mempool"], + ["noban", "mempool", "download"], True) self.checkpermission( @@ -57,7 +58,7 @@ def run_test(self): # Legacy parameter interaction which set whitelistrelay to true # if whitelistforcerelay is true ["-whitelist=127.0.0.1", "-whitelistforcerelay"], - ["forcerelay", "relay", "noban", "mempool"], + ["forcerelay", "relay", "noban", "mempool", "download"], True) # Let's make sure permissions are merged correctly @@ -68,32 +69,32 @@ def run_test(self): self.checkpermission( ["-whitelist=noban@127.0.0.1"], # Check parameter interaction forcerelay should activate relay - ["noban", "bloomfilter", "forcerelay", "relay"], + ["noban", "bloomfilter", "forcerelay", "relay", "download"], False) self.replaceinconfig(1, "whitebind=bloomfilter,forcerelay@" + ip_port, "bind=127.0.0.1") self.checkpermission( # legacy whitelistrelay should be ignored ["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"], - ["noban", "mempool"], + ["noban", "mempool", "download"], False) self.checkpermission( # legacy whitelistforcerelay should be ignored ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"], - ["noban", "mempool"], + ["noban", "mempool", "download"], False) self.checkpermission( # missing mempool permission to be considered legacy whitelisted ["-whitelist=noban@127.0.0.1"], - ["noban"], + ["noban", "download"], False) self.checkpermission( # all permission added ["-whitelist=all@127.0.0.1"], - ["forcerelay", "noban", "mempool", "bloomfilter", "relay"], + ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download"], False) self.stop_node(1) From 1244d595226005cf560ec59533558da3ecc340bf Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Fri, 2 Jun 2023 14:32:59 +0530 Subject: [PATCH 04/14] merge bitcoin#18991: Cache responses to GETADDR to prevent topology leaks --- src/addrman.h | 2 +- src/net.cpp | 19 ++++- src/net.h | 34 ++++++++ src/net_permissions.cpp | 3 + src/net_permissions.h | 4 +- src/net_processing.cpp | 17 ++-- src/test/fuzz/net_permissions.cpp | 1 + src/test/netbase_tests.cpp | 3 +- test/functional/p2p_getaddr_caching.py | 108 +++++++++++++++++++++++++ test/functional/p2p_permissions.py | 2 +- test/functional/test_runner.py | 1 + 11 files changed, 182 insertions(+), 12 deletions(-) create mode 100755 test/functional/p2p_getaddr_caching.py diff --git a/src/addrman.h b/src/addrman.h index 53cb22c19468..2139186beaaf 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -158,7 +158,7 @@ class CAddrInfo : public CAddress #define ADDRMAN_GETADDR_MAX_PCT 23 //! the maximum number of nodes to return in a getaddr call -#define ADDRMAN_GETADDR_MAX 2500 +#define ADDRMAN_GETADDR_MAX 1000 //! Convenience #define ADDRMAN_TRIED_BUCKET_COUNT (1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2) diff --git a/src/net.cpp b/src/net.cpp index a5b28c6d722c..6e6364501208 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3352,7 +3352,24 @@ CConnman::~CConnman() std::vector CConnman::GetAddresses() { - return addrman.GetAddr(); + std::vector addresses = addrman.GetAddr(); + if (m_banman) { + addresses.erase(std::remove_if(addresses.begin(), addresses.end(), + [this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}), + addresses.end()); + } + return addresses; +} + +std::vector CConnman::GetAddresses(Network requestor_network) +{ + const auto current_time = GetTime(); + if (m_addr_response_caches.find(requestor_network) == m_addr_response_caches.end() || + m_addr_response_caches[requestor_network].m_update_addr_response < current_time) { + m_addr_response_caches[requestor_network].m_addrs_response_cache = GetAddresses(); + m_addr_response_caches[requestor_network].m_update_addr_response = current_time + std::chrono::hours(21) + GetRandMillis(std::chrono::hours(6)); + } + return m_addr_response_caches[requestor_network].m_addrs_response_cache; } bool CConnman::AddNode(const std::string& strNode) diff --git a/src/net.h b/src/net.h index 525b489ac29a..cff17a38e09b 100644 --- a/src/net.h +++ b/src/net.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -67,6 +68,9 @@ static const int FEELER_INTERVAL = 120; static const unsigned int MAX_INV_SZ = 50000; /** The maximum number of new addresses to accumulate before announcing. */ static const unsigned int MAX_ADDR_TO_SEND = 1000; +// TODO: remove ADDRMAN_GETADDR_MAX and let the caller specify this limit with MAX_ADDR_TO_SEND. +static_assert(MAX_ADDR_TO_SEND == ADDRMAN_GETADDR_MAX, + "Max allowed ADDR message size should be equal to the max number of records returned from AddrMan."); /** Maximum length of incoming protocol messages (no message over 3 MiB is currently acceptable). */ static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 3 * 1024 * 1024; /** Maximum length of the user agent string in `version` message */ @@ -397,6 +401,13 @@ friend class CNode; // Addrman functions std::vector GetAddresses(); + /** + * Cache is used to minimize topology leaks, so it should + * be used for all non-trusted calls, for example, p2p. + * A non-malicious call (from RPC or a peer with addr permission) should + * call the function without a parameter to avoid using the cache. + */ + std::vector GetAddresses(Network requestor_network); // This allows temporarily exceeding m_max_outbound_full_relay, with the goal of finding // a peer that is better than all our current peers. @@ -592,6 +603,29 @@ friend class CNode; std::atomic nLastNodeId{0}; unsigned int nPrevNodeCount{0}; + /** + * Cache responses to addr requests to minimize privacy leak. + * Attack example: scraping addrs in real-time may allow an attacker + * to infer new connections of the victim by detecting new records + * with fresh timestamps (per self-announcement). + */ + struct CachedAddrResponse { + std::vector m_addrs_response_cache; + std::chrono::microseconds m_update_addr_response{0}; + }; + + /** + * Addr responses stored in different caches + * per network prevent cross-network node identification. + * If a node for example is multi-homed under Tor and IPv6, + * a single cache (or no cache at all) would let an attacker + * to easily detect that it is the same node by comparing responses. + * The used memory equals to 1000 CAddress records (or around 32 bytes) per + * distinct Network (up to 5) we have/had an inbound peer from, + * resulting in at most ~160 KB. + */ + std::map m_addr_response_caches; + /** * Services this instance offers. * diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index 0973d664253e..f2e519eb266e 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -15,6 +15,7 @@ const std::vector NET_PERMISSIONS_DOC{ "relay (relay even in -blocksonly mode)", "mempool (allow requesting BIP35 mempool contents)", "download (allow getheaders during IBD, no disconnect after maxuploadtarget limit)", + "addr (responses to GETADDR avoid hitting the cache and contain random records with the most up-to-date info)" }; namespace { @@ -50,6 +51,7 @@ bool TryParsePermissionFlags(const std::string& str, NetPermissionFlags& output, else if (permission == "download") NetPermissions::AddFlag(flags, PF_DOWNLOAD); else if (permission == "all") NetPermissions::AddFlag(flags, PF_ALL); else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY); + else if (permission == "addr") NetPermissions::AddFlag(flags, PF_ADDR); else if (permission.length() == 0); // Allow empty entries else { error = strprintf(_("Invalid P2P permission: '%s'"), permission); @@ -75,6 +77,7 @@ std::vector NetPermissions::ToStrings(NetPermissionFlags flags) if (NetPermissions::HasFlag(flags, PF_RELAY)) strings.push_back("relay"); if (NetPermissions::HasFlag(flags, PF_MEMPOOL)) strings.push_back("mempool"); if (NetPermissions::HasFlag(flags, PF_DOWNLOAD)) strings.push_back("download"); + if (NetPermissions::HasFlag(flags, PF_ADDR)) strings.push_back("addr"); return strings; } diff --git a/src/net_permissions.h b/src/net_permissions.h index 78efc97dbe37..cb5074490675 100644 --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -29,10 +29,12 @@ enum NetPermissionFlags { PF_NOBAN = (1U << 4) | PF_DOWNLOAD, // Can query the mempool PF_MEMPOOL = (1U << 5), + // Can request addrs without hitting a privacy-preserving cache + PF_ADDR = (1U << 7), // True if the user did not specifically set fine grained permissions PF_ISIMPLICIT = (1U << 31), - PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL | PF_DOWNLOAD, + PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL | PF_DOWNLOAD | PF_ADDR, }; class NetPermissions diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a47a45a75458..294d2eb30808 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3076,7 +3076,7 @@ void PeerManagerImpl::ProcessMessage( if (!pfrom.IsAddrRelayPeer()) { return; } - if (vAddr.size() > 1000) + if (vAddr.size() > MAX_ADDR_TO_SEND) { Misbehaving(pfrom.GetId(), 20, strprintf("%s message size = %u", msg_type, vAddr.size())); return; @@ -4026,12 +4026,15 @@ void PeerManagerImpl::ProcessMessage( pfrom.fSentAddr = true; pfrom.vAddrToSend.clear(); - std::vector vAddr = m_connman.GetAddresses(); + std::vector vAddr; + if (pfrom.HasPermission(PF_ADDR)) { + vAddr = m_connman.GetAddresses(); + } else { + vAddr = m_connman.GetAddresses(pfrom.addr.GetNetwork()); + } FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) { - if (!m_banman->IsDiscouraged(addr) && !m_banman->IsBanned(addr)) { - pfrom.PushAddress(addr, insecure_rand); - } + pfrom.PushAddress(addr, insecure_rand); } return; } @@ -4674,8 +4677,8 @@ bool PeerManagerImpl::SendMessages(CNode* pto) { pto->m_addr_known->insert(addr.GetKey()); vAddr.push_back(addr); - // receiver rejects addr messages larger than 1000 - if (vAddr.size() >= 1000) + // receiver rejects addr messages larger than MAX_ADDR_TO_SEND + if (vAddr.size() >= MAX_ADDR_TO_SEND) { m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr)); vAddr.clear(); diff --git a/src/test/fuzz/net_permissions.cpp b/src/test/fuzz/net_permissions.cpp index 9410c2f55dec..3620e16d3062 100644 --- a/src/test/fuzz/net_permissions.cpp +++ b/src/test/fuzz/net_permissions.cpp @@ -24,6 +24,7 @@ FUZZ_TARGET(net_permissions) NetPermissionFlags::PF_FORCERELAY, NetPermissionFlags::PF_NOBAN, NetPermissionFlags::PF_MEMPOOL, + NetPermissionFlags::PF_ADDR, NetPermissionFlags::PF_ISIMPLICIT, NetPermissionFlags::PF_ALL, }) : diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 43f1ff27a989..3150d772d54e 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -521,13 +521,14 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay,mempool@1.2.3.4/32", whitelistPermissions, error)); const auto strings = NetPermissions::ToStrings(PF_ALL); - BOOST_CHECK_EQUAL(strings.size(), 6); + BOOST_CHECK_EQUAL(strings.size(), 7); BOOST_CHECK(std::find(strings.begin(), strings.end(), "bloomfilter") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "forcerelay") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "relay") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "noban") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "mempool") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "download") != strings.end()); + BOOST_CHECK(std::find(strings.begin(), strings.end(), "addr") != strings.end()); } BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters) diff --git a/test/functional/p2p_getaddr_caching.py b/test/functional/p2p_getaddr_caching.py new file mode 100755 index 000000000000..9197845d822c --- /dev/null +++ b/test/functional/p2p_getaddr_caching.py @@ -0,0 +1,108 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test addr response caching""" + +import time +from test_framework.messages import ( + CAddress, + NODE_NETWORK, + msg_addr, + msg_getaddr, +) +from test_framework.mininode import ( + P2PInterface, + mininode_lock +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, +) + +MAX_ADDR_TO_SEND = 1000 + +def gen_addrs(n): + addrs = [] + for i in range(n): + addr = CAddress() + addr.time = int(time.time()) + addr.nServices = NODE_NETWORK + # Use first octets to occupy different AddrMan buckets + first_octet = i >> 8 + second_octet = i % 256 + addr.ip = "{}.{}.1.1".format(first_octet, second_octet) + addr.port = 9999 + addrs.append(addr) + return addrs + +class AddrReceiver(P2PInterface): + + def __init__(self): + super().__init__() + self.received_addrs = None + + def get_received_addrs(self): + with mininode_lock: + return self.received_addrs + + def on_addr(self, message): + self.received_addrs = [] + for addr in message.addrs: + self.received_addrs.append(addr.ip) + + def addr_received(self): + return self.received_addrs is not None + + +class AddrTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 1 + + def run_test(self): + self.log.info('Create connection that sends and requests addr messages') + addr_source = self.nodes[0].add_p2p_connection(P2PInterface()) + + msg_send_addrs = msg_addr() + self.log.info('Fill peer AddrMan with a lot of records') + # Since these addrs are sent from the same source, not all of them will be stored, + # because we allocate a limited number of AddrMan buckets per addr source. + total_addrs = 10000 + addrs = gen_addrs(total_addrs) + for i in range(int(total_addrs/MAX_ADDR_TO_SEND)): + msg_send_addrs.addrs = addrs[i * MAX_ADDR_TO_SEND:(i + 1) * MAX_ADDR_TO_SEND] + addr_source.send_and_ping(msg_send_addrs) + + responses = [] + self.log.info('Send many addr requests within short time to receive same response') + N = 5 + cur_mock_time = int(time.time()) + for i in range(N): + addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) + addr_receiver.send_and_ping(msg_getaddr()) + # Trigger response + cur_mock_time += 5 * 60 + self.nodes[0].setmocktime(cur_mock_time) + addr_receiver.wait_until(addr_receiver.addr_received) + responses.append(addr_receiver.get_received_addrs()) + for response in responses[1:]: + assert_equal(response, responses[0]) + assert(len(response) < MAX_ADDR_TO_SEND) + + cur_mock_time += 3 * 24 * 60 * 60 + self.nodes[0].setmocktime(cur_mock_time) + + self.log.info('After time passed, see a new response to addr request') + last_addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) + last_addr_receiver.send_and_ping(msg_getaddr()) + # Trigger response + cur_mock_time += 5 * 60 + self.nodes[0].setmocktime(cur_mock_time) + last_addr_receiver.wait_until(last_addr_receiver.addr_received) + # new response is different + assert(set(responses[0]) != set(last_addr_receiver.get_received_addrs())) + + +if __name__ == '__main__': + AddrTest().main() diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index c5ad7111e592..b8abd465d377 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -94,7 +94,7 @@ def run_test(self): self.checkpermission( # all permission added ["-whitelist=all@127.0.0.1"], - ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download"], + ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download", "addr"], False) self.stop_node(1) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 506a473b3d7c..9ed602f6bf45 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -173,6 +173,7 @@ 'rpc_blockchain.py', 'rpc_deprecated.py', 'wallet_disable.py', + 'p2p_getaddr_caching.py', 'p2p_getdata.py', 'rpc_net.py', 'wallet_keypool.py', From 56cd0dd80a1511b6776496f6f4cb07c845a2f7ed Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Thu, 1 Jun 2023 22:29:59 +0530 Subject: [PATCH 05/14] merge bitcoin#19658: Allow RPC to fetch all addrman records and add records to addrman --- src/addrman.cpp | 12 +++++--- src/addrman.h | 12 ++------ src/bench/addrman.cpp | 2 +- src/net.cpp | 8 +++--- src/net.h | 11 +++---- src/net_processing.cpp | 6 ++-- src/rpc/client.cpp | 1 + src/rpc/net.cpp | 59 ++++++++++++++++++++++++++++++++++---- src/test/addrman_tests.cpp | 12 ++++---- src/test/fuzz/addrman.cpp | 2 +- src/test/fuzz/connman.cpp | 4 +-- test/functional/rpc_net.py | 41 +++++++++++++------------- 12 files changed, 109 insertions(+), 61 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 1df0847dbb8f..c7fe861678ee 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -500,11 +500,15 @@ int CAddrMan::Check_() } #endif -void CAddrMan::GetAddr_(std::vector& vAddr) +void CAddrMan::GetAddr_(std::vector& vAddr, size_t max_addresses, size_t max_pct) { - unsigned int nNodes = ADDRMAN_GETADDR_MAX_PCT * vRandom.size() / 100; - if (nNodes > ADDRMAN_GETADDR_MAX) - nNodes = ADDRMAN_GETADDR_MAX; + size_t nNodes = vRandom.size(); + if (max_pct != 0) { + nNodes = max_pct * nNodes / 100; + } + if (max_addresses != 0) { + nNodes = std::min(nNodes, max_addresses); + } // gather a list of random nodes, skipping those of low quality for (unsigned int n = 0; n < vRandom.size(); n++) { diff --git a/src/addrman.h b/src/addrman.h index 2139186beaaf..21e767ebba7b 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -154,12 +154,6 @@ class CAddrInfo : public CAddress //! how recent a successful connection should be before we allow an address to be evicted from tried #define ADDRMAN_REPLACEMENT_HOURS 4 -//! the maximum percentage of nodes to return in a getaddr call -#define ADDRMAN_GETADDR_MAX_PCT 23 - -//! the maximum number of nodes to return in a getaddr call -#define ADDRMAN_GETADDR_MAX 1000 - //! Convenience #define ADDRMAN_TRIED_BUCKET_COUNT (1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2) #define ADDRMAN_NEW_BUCKET_COUNT (1 << ADDRMAN_NEW_BUCKET_COUNT_LOG2) @@ -266,7 +260,7 @@ class CAddrMan #endif //! Select several addresses at once. - void GetAddr_(std::vector &vAddr) EXCLUSIVE_LOCKS_REQUIRED(cs); + void GetAddr_(std::vector &vAddr, size_t max_addresses, size_t max_pct) EXCLUSIVE_LOCKS_REQUIRED(cs); /** We have successfully connected to this peer. Calling this function * updates the CAddress's nTime, which is used in our IsTerrible() @@ -708,13 +702,13 @@ class CAddrMan } //! Return a bunch of addresses, selected at random. - std::vector GetAddr() + std::vector GetAddr(size_t max_addresses, size_t max_pct) { Check(); std::vector vAddr; { LOCK(cs); - GetAddr_(vAddr); + GetAddr_(vAddr, max_addresses, max_pct); } Check(); return vAddr; diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index 26d934076829..ebdad5a4b8ff 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -98,7 +98,7 @@ static void AddrManGetAddr(benchmark::Bench& bench) FillAddrMan(addrman); bench.run([&] { - const auto& addresses = addrman.GetAddr(); + const auto& addresses = addrman.GetAddr(2500, 23); assert(addresses.size() > 0); }); } diff --git a/src/net.cpp b/src/net.cpp index 6e6364501208..6eddfebaaaf9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3350,9 +3350,9 @@ CConnman::~CConnman() Stop(); } -std::vector CConnman::GetAddresses() +std::vector CConnman::GetAddresses(size_t max_addresses, size_t max_pct) { - std::vector addresses = addrman.GetAddr(); + std::vector addresses = addrman.GetAddr(max_addresses, max_pct); if (m_banman) { addresses.erase(std::remove_if(addresses.begin(), addresses.end(), [this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}), @@ -3361,12 +3361,12 @@ std::vector CConnman::GetAddresses() return addresses; } -std::vector CConnman::GetAddresses(Network requestor_network) +std::vector CConnman::GetAddresses(Network requestor_network, size_t max_addresses, size_t max_pct) { const auto current_time = GetTime(); if (m_addr_response_caches.find(requestor_network) == m_addr_response_caches.end() || m_addr_response_caches[requestor_network].m_update_addr_response < current_time) { - m_addr_response_caches[requestor_network].m_addrs_response_cache = GetAddresses(); + m_addr_response_caches[requestor_network].m_addrs_response_cache = GetAddresses(max_addresses, max_pct); m_addr_response_caches[requestor_network].m_update_addr_response = current_time + std::chrono::hours(21) + GetRandMillis(std::chrono::hours(6)); } return m_addr_response_caches[requestor_network].m_addrs_response_cache; diff --git a/src/net.h b/src/net.h index cff17a38e09b..1d90023d272d 100644 --- a/src/net.h +++ b/src/net.h @@ -66,11 +66,8 @@ static const int WARNING_INTERVAL = 10 * 60; static const int FEELER_INTERVAL = 120; /** The maximum number of entries in an 'inv' protocol message */ static const unsigned int MAX_INV_SZ = 50000; -/** The maximum number of new addresses to accumulate before announcing. */ -static const unsigned int MAX_ADDR_TO_SEND = 1000; -// TODO: remove ADDRMAN_GETADDR_MAX and let the caller specify this limit with MAX_ADDR_TO_SEND. -static_assert(MAX_ADDR_TO_SEND == ADDRMAN_GETADDR_MAX, - "Max allowed ADDR message size should be equal to the max number of records returned from AddrMan."); +/** The maximum number of addresses from our addrman to return in response to a getaddr message. */ +static constexpr size_t MAX_ADDR_TO_SEND = 1000; /** Maximum length of incoming protocol messages (no message over 3 MiB is currently acceptable). */ static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 3 * 1024 * 1024; /** Maximum length of the user agent string in `version` message */ @@ -400,14 +397,14 @@ friend class CNode; void RelayInvFiltered(CInv &inv, const uint256 &relatedTxHash, const int minProtoVersion = MIN_PEER_PROTO_VERSION); // Addrman functions - std::vector GetAddresses(); + std::vector GetAddresses(size_t max_addresses, size_t max_pct); /** * Cache is used to minimize topology leaks, so it should * be used for all non-trusted calls, for example, p2p. * A non-malicious call (from RPC or a peer with addr permission) should * call the function without a parameter to avoid using the cache. */ - std::vector GetAddresses(Network requestor_network); + std::vector GetAddresses(Network requestor_network, size_t max_addresses, size_t max_pct); // This allows temporarily exceeding m_max_outbound_full_relay, with the goal of finding // a peer that is better than all our current peers. diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 294d2eb30808..3a008c05e722 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -152,6 +152,8 @@ static constexpr unsigned int INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK = 4 * 7 * IN static constexpr uint32_t MAX_GETCFILTERS_SIZE = 1000; /** Maximum number of cf hashes that may be requested with one getcfheaders. See BIP 157. */ static constexpr uint32_t MAX_GETCFHEADERS_SIZE = 2000; +/** the maximum percentage of addresses from our addrman to return in response to a getaddr message. */ +static constexpr size_t MAX_PCT_ADDR_TO_SEND = 23; struct COrphanTx { // When modifying, adapt the copy of this definition in tests/DoS_tests. @@ -4028,9 +4030,9 @@ void PeerManagerImpl::ProcessMessage( pfrom.vAddrToSend.clear(); std::vector vAddr; if (pfrom.HasPermission(PF_ADDR)) { - vAddr = m_connman.GetAddresses(); + vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } else { - vAddr = m_connman.GetAddresses(pfrom.addr.GetNetwork()); + vAddr = m_connman.GetAddresses(pfrom.addr.GetNetwork(), MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) { diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 4d1eb11979b5..4bcb47bec9bb 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -206,6 +206,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "unloadwallet", 1, "load_on_startup"}, { "upgradetohd", 3, "rescan"}, { "getnodeaddresses", 0, "count"}, + { "addpeeraddress", 1, "port"}, { "stop", 0, "wait" }, }; // clang-format on diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index c762ece0fbc5..51cc57931a97 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -788,7 +788,7 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request) RPCHelpMan{"getnodeaddresses", "\nReturn known addresses which can potentially be used to find new nodes in the network\n", { - {"count", RPCArg::Type::NUM, /* default */ "1", "How many addresses to return. Limited to the smaller of " + ToString(ADDRMAN_GETADDR_MAX) + " or " + ToString(ADDRMAN_GETADDR_MAX_PCT) + "% of all known addresses."}, + {"count", RPCArg::Type::NUM, /* default */ "1", "The maximum number of addresses to return. Specify 0 to return all known addresses."}, }, RPCResult{ RPCResult::Type::ARR, "", "", @@ -815,18 +815,16 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request) int count = 1; if (!request.params[0].isNull()) { count = request.params[0].get_int(); - if (count <= 0) { + if (count < 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Address count out of range"); } } // returns a shuffled list of CAddress - std::vector vAddr = node.connman->GetAddresses(); + std::vector vAddr = node.connman->GetAddresses(count, /* max_pct */ 0); UniValue ret(UniValue::VARR); - int address_return_count = std::min(count, vAddr.size()); - for (int i = 0; i < address_return_count; ++i) { + for (const CAddress& addr : vAddr) { UniValue obj(UniValue::VOBJ); - const CAddress& addr = vAddr[i]; obj.pushKV("time", (int)addr.nTime); obj.pushKV("services", (uint64_t)addr.nServices); obj.pushKV("address", addr.ToStringIP()); @@ -836,6 +834,54 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request) return ret; } +static UniValue addpeeraddress(const JSONRPCRequest& request) +{ + RPCHelpMan{"addpeeraddress", + "\nAdd the address of a potential peer to the address manager. This RPC is for testing only.\n", + { + {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address of the peer"}, + {"port", RPCArg::Type::NUM, RPCArg::Optional::NO, "The port of the peer"}, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::BOOL, "success", "whether the peer address was successfully added to the address manager"}, + }, + }, + RPCExamples{ + HelpExampleCli("addpeeraddress", "\"1.2.3.4\" 9999") + + HelpExampleRpc("addpeeraddress", "\"1.2.3.4\", 9999") + }, + }.Check(request); + + NodeContext& node = EnsureNodeContext(request.context); + if (!node.addrman) { + throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Address manager functionality missing or disabled"); + } + + UniValue obj(UniValue::VOBJ); + + std::string addr_string = request.params[0].get_str(); + uint16_t port = request.params[1].get_int(); + + CNetAddr net_addr; + if (!LookupHost(addr_string, net_addr, false)) { + obj.pushKV("success", false); + return obj; + } + CAddress address = CAddress({net_addr, port}, ServiceFlags(NODE_NETWORK)); + address.nTime = GetAdjustedTime(); + // The source address is set equal to the address. This is equivalent to the peer + // announcing itself. + if (!node.addrman->Add(address, address)) { + obj.pushKV("success", false); + return obj; + } + + obj.pushKV("success", true); + return obj; +} + // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -854,6 +900,7 @@ static const CRPCCommand commands[] = { "network", "cleardiscouraged", &cleardiscouraged, {} }, { "network", "setnetworkactive", &setnetworkactive, {"state"} }, { "network", "getnodeaddresses", &getnodeaddresses, {"count"} }, + { "hidden", "addpeeraddress", &addpeeraddress, {"address", "port"} }, }; // clang-format on diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index dd31ab1dddfe..0fb97ccd1a08 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -393,7 +393,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) // Test: Sanity check, GetAddr should never return anything if addrman // is empty. BOOST_CHECK_EQUAL(addrman.size(), 0U); - std::vector vAddr1 = addrman.GetAddr(); + std::vector vAddr1 = addrman.GetAddr(/* max_addresses */ 0, /* max_pct */0); BOOST_CHECK_EQUAL(vAddr1.size(), 0U); CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE); @@ -416,13 +416,15 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) BOOST_CHECK(addrman.Add(addr4, source2)); BOOST_CHECK(addrman.Add(addr5, source1)); - // GetAddr returns 23% of addresses, 23% of 5 is 1 rounded down. - BOOST_CHECK_EQUAL(addrman.GetAddr().size(), 1U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U); + // Net processing asks for 23% of addresses. 23% of 5 is 1 rounded down. + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U); // Test: Ensure GetAddr works with new and tried addresses. addrman.Good(CAddress(addr1, NODE_NONE)); addrman.Good(CAddress(addr2, NODE_NONE)); - BOOST_CHECK_EQUAL(addrman.GetAddr().size(), 1U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U); // Test: Ensure GetAddr still returns 23% when addrman has many addrs. for (unsigned int i = 1; i < (8 * 256); i++) { @@ -437,7 +439,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) if (i % 8 == 0) addrman.Good(addr); } - std::vector vAddr = addrman.GetAddr(); + std::vector vAddr = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23); size_t percent23 = (addrman.size() * 23) / 100; BOOST_CHECK_EQUAL(vAddr.size(), percent23); diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 14bc170d554e..ba0444a6b561 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -61,7 +61,7 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) (void)addr_man.Select(fuzzed_data_provider.ConsumeBool()); }, [&] { - (void)addr_man.GetAddr(); + (void)addr_man.GetAddr(fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral()); }, [&] { const std::optional opt_address = ConsumeDeserializable(fuzzed_data_provider); diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 9c21e7ce7d8a..3763f7b5a6f6 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -69,10 +69,10 @@ FUZZ_TARGET_INIT(connman, initialize_connman) (void)connman.ForNode(fuzzed_data_provider.ConsumeIntegral(), [&](auto) { return fuzzed_data_provider.ConsumeBool(); }); }, [&] { - (void)connman.GetAddresses(); + (void)connman.GetAddresses(fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral()); }, [&] { - (void)connman.GetAddresses(); + (void)connman.GetAddresses(fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}), fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral()); }, [&] { (void)connman.GetDeterministicRandomizer(fuzzed_data_provider.ConsumeIntegral()); diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 12752e52a08e..3c3198521817 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -19,8 +19,6 @@ from test_framework.mininode import P2PInterface import test_framework.messages from test_framework.messages import ( - CAddress, - msg_addr, NODE_NETWORK, ) @@ -168,31 +166,34 @@ def test_service_flags(self): def _test_getnodeaddresses(self): self.nodes[0].add_p2p_connection(P2PInterface()) - # send some addresses to the node via the p2p message addr - msg = msg_addr() + # Add some addresses to the Address Manager over RPC. Due to the way + # bucket and bucket position are calculated, some of these addresses + # will collide. imported_addrs = [] - for i in range(256): - a = "123.123.123.{}".format(i) + for i in range(10000): + first_octet = i >> 8 + second_octet = i % 256 + a = "{}.{}.1.1".format(first_octet, second_octet) imported_addrs.append(a) - addr = CAddress() - addr.time = 100000000 - addr.nServices = NODE_NETWORK - addr.ip = a - addr.port = 8333 - msg.addrs.append(addr) - self.nodes[0].p2p.send_and_ping(msg) - - # obtain addresses via rpc call and check they were ones sent in before - REQUEST_COUNT = 10 - node_addresses = self.nodes[0].getnodeaddresses(REQUEST_COUNT) - assert_equal(len(node_addresses), REQUEST_COUNT) + self.nodes[0].addpeeraddress(a, 8333) + + # Obtain addresses via rpc call and check they were ones sent in before. + # + # Maximum possible addresses in addrman is 10000, although actual + # number will usually be less due to bucket and bucket position + # collisions. + node_addresses = self.nodes[0].getnodeaddresses(0) + assert_greater_than(len(node_addresses), 5000) + assert_greater_than(10000, len(node_addresses)) for a in node_addresses: - # see penalty calculations for ADDRs with nTime <= 100000000 in net_processing.cpp - assert_equal(a["time"], self.mocktime - 5 * 24 * 60 * 60 - 2 * 60 * 60) + assert_equal(a["time"], self.mocktime) assert_equal(a["services"], NODE_NETWORK) assert a["address"] in imported_addrs assert_equal(a["port"], 8333) + node_addresses = self.nodes[0].getnodeaddresses(1) + assert_equal(len(node_addresses), 1) + assert_raises_rpc_error(-8, "Address count out of range", self.nodes[0].getnodeaddresses, -1) # addrman's size cannot be known reliably after insertion, as hash collisions may occur From 01130228f06440cf1b6d1532e414713e9511eec4 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sat, 3 Jun 2023 11:58:46 +0530 Subject: [PATCH 06/14] merge bitcoin#19697: Improvements on ADDR caching --- src/net.cpp | 45 +++++++++++++++++++---- src/net.h | 16 +++++---- src/net_processing.cpp | 2 +- src/test/fuzz/connman.cpp | 2 +- test/functional/p2p_getaddr_caching.py | 50 +++++++++----------------- 5 files changed, 67 insertions(+), 48 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 6eddfebaaaf9..178deb57127e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -125,6 +125,7 @@ constexpr const CConnman::CAllNodes CConnman::AllNodes; static const uint64_t RANDOMIZER_ID_NETGROUP = 0x6c0edd8036ef4036ULL; // SHA256("netgroup")[0:8] static const uint64_t RANDOMIZER_ID_LOCALHOSTNONCE = 0xd93e69e2bbfa5735ULL; // SHA256("localhostnonce")[0:8] +static const uint64_t RANDOMIZER_ID_ADDRCACHE = 0x1cf2e4ddd306dda9ULL; // SHA256("addrcache")[0:8] // // Global state variables // @@ -3361,15 +3362,47 @@ std::vector CConnman::GetAddresses(size_t max_addresses, size_t max_pc return addresses; } -std::vector CConnman::GetAddresses(Network requestor_network, size_t max_addresses, size_t max_pct) +std::vector CConnman::GetAddresses(CNode& requestor, size_t max_addresses, size_t max_pct) { + SOCKET socket; + WITH_LOCK(requestor.cs_hSocket, socket = requestor.hSocket); + auto local_socket_bytes = GetBindAddress(socket).GetAddrBytes(); + uint64_t cache_id = GetDeterministicRandomizer(RANDOMIZER_ID_ADDRCACHE) + .Write(requestor.addr.GetNetwork()) + .Write(local_socket_bytes.data(), local_socket_bytes.size()) + .Finalize(); const auto current_time = GetTime(); - if (m_addr_response_caches.find(requestor_network) == m_addr_response_caches.end() || - m_addr_response_caches[requestor_network].m_update_addr_response < current_time) { - m_addr_response_caches[requestor_network].m_addrs_response_cache = GetAddresses(max_addresses, max_pct); - m_addr_response_caches[requestor_network].m_update_addr_response = current_time + std::chrono::hours(21) + GetRandMillis(std::chrono::hours(6)); + auto r = m_addr_response_caches.emplace(cache_id, CachedAddrResponse{}); + CachedAddrResponse& cache_entry = r.first->second; + if (cache_entry.m_cache_entry_expiration < current_time) { // If emplace() added new one it has expiration 0. + cache_entry.m_addrs_response_cache = GetAddresses(max_addresses, max_pct); + // Choosing a proper cache lifetime is a trade-off between the privacy leak minimization + // and the usefulness of ADDR responses to honest users. + // + // Longer cache lifetime makes it more difficult for an attacker to scrape + // enough AddrMan data to maliciously infer something useful. + // By the time an attacker scraped enough AddrMan records, most of + // the records should be old enough to not leak topology info by + // e.g. analyzing real-time changes in timestamps. + // + // It takes only several hundred requests to scrape everything from an AddrMan containing 100,000 nodes, + // so ~24 hours of cache lifetime indeed makes the data less inferable by the time + // most of it could be scraped (considering that timestamps are updated via + // ADDR self-announcements and when nodes communicate). + // We also should be robust to those attacks which may not require scraping *full* victim's AddrMan + // (because even several timestamps of the same handful of nodes may leak privacy). + // + // On the other hand, longer cache lifetime makes ADDR responses + // outdated and less useful for an honest requestor, e.g. if most nodes + // in the ADDR response are no longer active. + // + // However, the churn in the network is known to be rather low. Since we consider + // nodes to be "terrible" (see IsTerrible()) if the timestamps are older than 30 days, + // max. 24 hours of "penalty" due to cache shouldn't make any meaningful difference + // in terms of the freshness of the response. + cache_entry.m_cache_entry_expiration = current_time + std::chrono::hours(21) + GetRandMillis(std::chrono::hours(6)); } - return m_addr_response_caches[requestor_network].m_addrs_response_cache; + return cache_entry.m_addrs_response_cache; } bool CConnman::AddNode(const std::string& strNode) diff --git a/src/net.h b/src/net.h index 1d90023d272d..ccd75e52c006 100644 --- a/src/net.h +++ b/src/net.h @@ -404,7 +404,7 @@ friend class CNode; * A non-malicious call (from RPC or a peer with addr permission) should * call the function without a parameter to avoid using the cache. */ - std::vector GetAddresses(Network requestor_network, size_t max_addresses, size_t max_pct); + std::vector GetAddresses(CNode& requestor, size_t max_addresses, size_t max_pct); // This allows temporarily exceeding m_max_outbound_full_relay, with the goal of finding // a peer that is better than all our current peers. @@ -608,20 +608,24 @@ friend class CNode; */ struct CachedAddrResponse { std::vector m_addrs_response_cache; - std::chrono::microseconds m_update_addr_response{0}; + std::chrono::microseconds m_cache_entry_expiration{0}; }; /** * Addr responses stored in different caches - * per network prevent cross-network node identification. + * per (network, local socket) prevent cross-network node identification. * If a node for example is multi-homed under Tor and IPv6, * a single cache (or no cache at all) would let an attacker * to easily detect that it is the same node by comparing responses. - * The used memory equals to 1000 CAddress records (or around 32 bytes) per + * Indexing by local socket prevents leakage when a node has multiple + * listening addresses on the same network. + * + * The used memory equals to 1000 CAddress records (or around 40 bytes) per * distinct Network (up to 5) we have/had an inbound peer from, - * resulting in at most ~160 KB. + * resulting in at most ~196 KB. Every separate local socket may + * add up to ~196 KB extra. */ - std::map m_addr_response_caches; + std::map m_addr_response_caches; /** * Services this instance offers. diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3a008c05e722..9bc123ab5119 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4032,7 +4032,7 @@ void PeerManagerImpl::ProcessMessage( if (pfrom.HasPermission(PF_ADDR)) { vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } else { - vAddr = m_connman.GetAddresses(pfrom.addr.GetNetwork(), MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); + vAddr = m_connman.GetAddresses(pfrom, MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) { diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 3763f7b5a6f6..4bc20e943c17 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -72,7 +72,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman) (void)connman.GetAddresses(fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral()); }, [&] { - (void)connman.GetAddresses(fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}), fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral()); + (void)connman.GetAddresses(random_node, fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral()); }, [&] { (void)connman.GetDeterministicRandomizer(fuzzed_data_provider.ConsumeIntegral()); diff --git a/test/functional/p2p_getaddr_caching.py b/test/functional/p2p_getaddr_caching.py index 9197845d822c..45de948d618c 100755 --- a/test/functional/p2p_getaddr_caching.py +++ b/test/functional/p2p_getaddr_caching.py @@ -5,12 +5,8 @@ """Test addr response caching""" import time -from test_framework.messages import ( - CAddress, - NODE_NETWORK, - msg_addr, - msg_getaddr, -) + +from test_framework.messages import msg_getaddr from test_framework.mininode import ( P2PInterface, mininode_lock @@ -20,21 +16,9 @@ assert_equal, ) +# As defined in net_processing. MAX_ADDR_TO_SEND = 1000 - -def gen_addrs(n): - addrs = [] - for i in range(n): - addr = CAddress() - addr.time = int(time.time()) - addr.nServices = NODE_NETWORK - # Use first octets to occupy different AddrMan buckets - first_octet = i >> 8 - second_octet = i % 256 - addr.ip = "{}.{}.1.1".format(first_octet, second_octet) - addr.port = 9999 - addrs.append(addr) - return addrs +MAX_PCT_ADDR_TO_SEND = 23 class AddrReceiver(P2PInterface): @@ -61,18 +45,16 @@ def set_test_params(self): self.num_nodes = 1 def run_test(self): - self.log.info('Create connection that sends and requests addr messages') - addr_source = self.nodes[0].add_p2p_connection(P2PInterface()) - - msg_send_addrs = msg_addr() self.log.info('Fill peer AddrMan with a lot of records') - # Since these addrs are sent from the same source, not all of them will be stored, - # because we allocate a limited number of AddrMan buckets per addr source. - total_addrs = 10000 - addrs = gen_addrs(total_addrs) - for i in range(int(total_addrs/MAX_ADDR_TO_SEND)): - msg_send_addrs.addrs = addrs[i * MAX_ADDR_TO_SEND:(i + 1) * MAX_ADDR_TO_SEND] - addr_source.send_and_ping(msg_send_addrs) + for i in range(10000): + first_octet = i >> 8 + second_octet = i % 256 + a = "{}.{}.1.1".format(first_octet, second_octet) + self.nodes[0].addpeeraddress(a, 9999) + + # Need to make sure we hit MAX_ADDR_TO_SEND records in the addr response later because + # only a fraction of all known addresses can be cached and returned. + assert(len(self.nodes[0].getnodeaddresses(0)) > int(MAX_ADDR_TO_SEND / (MAX_PCT_ADDR_TO_SEND / 100))) responses = [] self.log.info('Send many addr requests within short time to receive same response') @@ -84,11 +66,11 @@ def run_test(self): # Trigger response cur_mock_time += 5 * 60 self.nodes[0].setmocktime(cur_mock_time) - addr_receiver.wait_until(addr_receiver.addr_received) + addr_receiver.wait_until(addr_receiver.addr_received, timeout=120) responses.append(addr_receiver.get_received_addrs()) for response in responses[1:]: assert_equal(response, responses[0]) - assert(len(response) < MAX_ADDR_TO_SEND) + assert(len(response) == MAX_ADDR_TO_SEND) cur_mock_time += 3 * 24 * 60 * 60 self.nodes[0].setmocktime(cur_mock_time) @@ -99,7 +81,7 @@ def run_test(self): # Trigger response cur_mock_time += 5 * 60 self.nodes[0].setmocktime(cur_mock_time) - last_addr_receiver.wait_until(last_addr_receiver.addr_received) + last_addr_receiver.wait_until(last_addr_receiver.addr_received, timeout=120) # new response is different assert(set(responses[0]) != set(last_addr_receiver.get_received_addrs())) From d0d598dbb010300c5023978b8778ac3dea97dac8 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 2 Jun 2023 16:53:25 +0300 Subject: [PATCH 07/14] fix: resolve persistent p2p_getaddr_caching.py failures --- test/functional/p2p_getaddr_caching.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/functional/p2p_getaddr_caching.py b/test/functional/p2p_getaddr_caching.py index 45de948d618c..cad2e7550660 100755 --- a/test/functional/p2p_getaddr_caching.py +++ b/test/functional/p2p_getaddr_caching.py @@ -4,8 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test addr response caching""" -import time - from test_framework.messages import msg_getaddr from test_framework.mininode import ( P2PInterface, @@ -59,14 +57,14 @@ def run_test(self): responses = [] self.log.info('Send many addr requests within short time to receive same response') N = 5 - cur_mock_time = int(time.time()) + cur_mock_time = self.mocktime for i in range(N): addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) addr_receiver.send_and_ping(msg_getaddr()) # Trigger response cur_mock_time += 5 * 60 self.nodes[0].setmocktime(cur_mock_time) - addr_receiver.wait_until(addr_receiver.addr_received, timeout=120) + addr_receiver.wait_until(addr_receiver.addr_received) responses.append(addr_receiver.get_received_addrs()) for response in responses[1:]: assert_equal(response, responses[0]) @@ -81,7 +79,7 @@ def run_test(self): # Trigger response cur_mock_time += 5 * 60 self.nodes[0].setmocktime(cur_mock_time) - last_addr_receiver.wait_until(last_addr_receiver.addr_received, timeout=120) + last_addr_receiver.wait_until(last_addr_receiver.addr_received) # new response is different assert(set(responses[0]) != set(last_addr_receiver.get_received_addrs())) From 81c686e7db6906802697d9832267e29208bbb067 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 23 Sep 2020 08:40:38 +0200 Subject: [PATCH 08/14] merge bitcoin#20000: fix creation of "std::string"s with \0s --- src/test/base32_tests.cpp | 19 +++++++++++-------- src/test/base58_tests.cpp | 24 +++++++++++++----------- src/test/base64_tests.cpp | 19 +++++++++++-------- src/test/net_tests.cpp | 6 ++++-- src/test/netbase_tests.cpp | 26 ++++++++++++++------------ src/test/util_tests.cpp | 7 ++++--- 6 files changed, 57 insertions(+), 44 deletions(-) diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp index e4ee9e3ddab9..510f39b13aaf 100644 --- a/src/test/base32_tests.cpp +++ b/src/test/base32_tests.cpp @@ -6,6 +6,9 @@ #include #include +#include + +using namespace std::literals; BOOST_FIXTURE_TEST_SUITE(base32_tests, BasicTestingSetup) @@ -26,14 +29,14 @@ BOOST_AUTO_TEST_CASE(base32_testvectors) // Decoding strings with embedded NUL characters should fail bool failure; - (void)DecodeBase32(std::string("invalid", 7), &failure); - BOOST_CHECK_EQUAL(failure, true); - (void)DecodeBase32(std::string("AWSX3VPP", 8), &failure); - BOOST_CHECK_EQUAL(failure, false); - (void)DecodeBase32(std::string("AWSX3VPP\0invalid", 16), &failure); - BOOST_CHECK_EQUAL(failure, true); - (void)DecodeBase32(std::string("AWSX3VPPinvalid", 15), &failure); - BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase32("invalid\0"s, &failure); // correct size, invalid due to \0 + BOOST_CHECK(failure); + (void)DecodeBase32("AWSX3VPP"s, &failure); // valid + BOOST_CHECK(!failure); + (void)DecodeBase32("AWSX3VPP\0invalid"s, &failure); // correct size, invalid due to \0 + BOOST_CHECK(failure); + (void)DecodeBase32("AWSX3VPPinvalid"s, &failure); // invalid size + BOOST_CHECK(failure); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 07bdc4333c56..0b8a6baebe75 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -12,7 +12,9 @@ #include #include +#include +using namespace std::literals; extern UniValue read_json(const std::string& jsondata); @@ -58,14 +60,14 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58) BOOST_CHECK_MESSAGE(result.size() == expected.size() && std::equal(result.begin(), result.end(), expected.begin()), strTest); } - BOOST_CHECK(!DecodeBase58("invalid", result, 100)); - BOOST_CHECK(!DecodeBase58(std::string("invalid"), result, 100)); - BOOST_CHECK(!DecodeBase58(std::string("\0invalid", 8), result, 100)); + BOOST_CHECK(!DecodeBase58("invalid"s, result, 100)); + BOOST_CHECK(!DecodeBase58("invalid\0"s, result, 100)); + BOOST_CHECK(!DecodeBase58("\0invalid"s, result, 100)); - BOOST_CHECK(DecodeBase58(std::string("good", 4), result, 100)); - BOOST_CHECK(!DecodeBase58(std::string("bad0IOl", 7), result, 100)); - BOOST_CHECK(!DecodeBase58(std::string("goodbad0IOl", 11), result, 100)); - BOOST_CHECK(!DecodeBase58(std::string("good\0bad0IOl", 12), result, 100)); + BOOST_CHECK(DecodeBase58("good"s, result, 100)); + BOOST_CHECK(!DecodeBase58("bad0IOl"s, result, 100)); + BOOST_CHECK(!DecodeBase58("goodbad0IOl"s, result, 100)); + BOOST_CHECK(!DecodeBase58("good\0bad0IOl"s, result, 100)); // check that DecodeBase58 skips whitespace, but still fails with unexpected non-whitespace at the end. BOOST_CHECK(!DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t a", result, 3)); @@ -73,10 +75,10 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58) std::vector expected = ParseHex("971a55"); BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); - BOOST_CHECK(DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oh", 21), result, 100)); - BOOST_CHECK(!DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oi", 21), result, 100)); - BOOST_CHECK(!DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oh0IOl", 25), result, 100)); - BOOST_CHECK(!DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oh\00IOl", 26), result, 100)); + BOOST_CHECK(DecodeBase58Check("3vQB7B6MrGQZaxCuFg4oh"s, result, 100)); + BOOST_CHECK(!DecodeBase58Check("3vQB7B6MrGQZaxCuFg4oi"s, result, 100)); + BOOST_CHECK(!DecodeBase58Check("3vQB7B6MrGQZaxCuFg4oh0IOl"s, result, 100)); + BOOST_CHECK(!DecodeBase58Check("3vQB7B6MrGQZaxCuFg4oh\0" "0IOl"s, result, 100)); } BOOST_AUTO_TEST_CASE(base58_random_encode_decode) diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp index 7c3f2c6b115c..bb38d2c04520 100644 --- a/src/test/base64_tests.cpp +++ b/src/test/base64_tests.cpp @@ -6,6 +6,9 @@ #include #include +#include + +using namespace std::literals; BOOST_FIXTURE_TEST_SUITE(base64_tests, BasicTestingSetup) @@ -23,14 +26,14 @@ BOOST_AUTO_TEST_CASE(base64_testvectors) // Decoding strings with embedded NUL characters should fail bool failure; - (void)DecodeBase64(std::string("invalid", 7), &failure); - BOOST_CHECK_EQUAL(failure, true); - (void)DecodeBase64(std::string("nQB/pZw=", 8), &failure); - BOOST_CHECK_EQUAL(failure, false); - (void)DecodeBase64(std::string("nQB/pZw=\0invalid", 16), &failure); - BOOST_CHECK_EQUAL(failure, true); - (void)DecodeBase64(std::string("nQB/pZw=invalid", 15), &failure); - BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase64("invalid\0"s, &failure); + BOOST_CHECK(failure); + (void)DecodeBase64("nQB/pZw="s, &failure); + BOOST_CHECK(!failure); + (void)DecodeBase64("nQB/pZw=\0invalid"s, &failure); + BOOST_CHECK(failure); + (void)DecodeBase64("nQB/pZw=invalid\0"s, &failure); + BOOST_CHECK(failure); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 3579ce796dd8..7ccb4088ab53 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -26,6 +26,8 @@ #include +using namespace std::literals; + class CAddrManSerializationMock : public CAddrMan { public: @@ -107,8 +109,8 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false)); BOOST_CHECK(Lookup("250.7.2.2", addr2, 9999, false)); BOOST_CHECK(Lookup("250.7.3.3", addr3, 9999, false)); - BOOST_CHECK(Lookup(std::string("250.7.3.3", 9), addr3, 9999, false)); - BOOST_CHECK(!Lookup(std::string("250.7.3.3\0example.com", 21), addr3, 9999, false)); + BOOST_CHECK(Lookup("250.7.3.3"s, addr3, 9999, false)); + BOOST_CHECK(!Lookup("250.7.3.3\0example.com"s, addr3, 9999, false)); // Add three addresses to new table. CService source; diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 3150d772d54e..fdab9886180b 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -18,6 +18,8 @@ #include +using namespace std::literals; + BOOST_FIXTURE_TEST_SUITE(netbase_tests, BasicTestingSetup) static CNetAddr ResolveIP(const std::string& ip) @@ -534,20 +536,20 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters) { CNetAddr addr; - BOOST_CHECK(LookupHost(std::string("127.0.0.1", 9), addr, false)); - BOOST_CHECK(!LookupHost(std::string("127.0.0.1\0", 10), addr, false)); - BOOST_CHECK(!LookupHost(std::string("127.0.0.1\0example.com", 21), addr, false)); - BOOST_CHECK(!LookupHost(std::string("127.0.0.1\0example.com\0", 22), addr, false)); + BOOST_CHECK(LookupHost("127.0.0.1"s, addr, false)); + BOOST_CHECK(!LookupHost("127.0.0.1\0"s, addr, false)); + BOOST_CHECK(!LookupHost("127.0.0.1\0example.com"s, addr, false)); + BOOST_CHECK(!LookupHost("127.0.0.1\0example.com\0"s, addr, false)); CSubNet ret; - BOOST_CHECK(LookupSubNet(std::string("1.2.3.0/24", 10), ret)); - BOOST_CHECK(!LookupSubNet(std::string("1.2.3.0/24\0", 11), ret)); - BOOST_CHECK(!LookupSubNet(std::string("1.2.3.0/24\0example.com", 22), ret)); - BOOST_CHECK(!LookupSubNet(std::string("1.2.3.0/24\0example.com\0", 23), ret)); + BOOST_CHECK(LookupSubNet("1.2.3.0/24"s, ret)); + BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0"s, ret)); + BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com"s, ret)); + BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com\0"s, ret)); // We only do subnetting for IPv4 and IPv6 - BOOST_CHECK(!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion", 22), ret)); - BOOST_CHECK(!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion\0", 23), ret)); - BOOST_CHECK(!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion\0example.com", 34), ret)); - BOOST_CHECK(!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion\0example.com\0", 35), ret)); + BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion"s, ret)); + BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion\0"s, ret)); + BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion\0example.com"s, ret)); + BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion\0example.com\0"s, ret)); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 3cbc730ecd14..24c2db2e71b2 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -36,7 +37,7 @@ #include -using namespace std::string_literals; +using namespace std::literals; static const std::string STRING_WITH_EMBEDDED_NULL_CHAR{"1"s "\0" "1"s}; @@ -1256,9 +1257,9 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney) BOOST_CHECK(!ParseMoney("-1")); // Parsing strings with embedded NUL characters should fail - BOOST_CHECK(!ParseMoney(std::string("\0-1", 3))); + BOOST_CHECK(!ParseMoney("\0-1"s)); BOOST_CHECK(!ParseMoney(STRING_WITH_EMBEDDED_NULL_CHAR)); - BOOST_CHECK(!ParseMoney(std::string("1\0", 2))); + BOOST_CHECK(!ParseMoney("1\0"s)); } BOOST_AUTO_TEST_CASE(util_IsHex) From f003f00e9abb42c88706758f45bad62650a9b344 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sat, 1 Aug 2020 15:10:26 +0200 Subject: [PATCH 09/14] merge bitcoin#19638: Replace hidden service with onion service --- CONTRIBUTING.md | 2 +- doc/JSON-RPC-interface.md | 2 +- doc/files.md | 2 +- doc/tor.md | 20 ++++++++++---------- src/init.cpp | 4 ++-- src/qt/forms/optionsdialog.ui | 4 ++-- src/torcontrol.cpp | 4 ++-- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 29d3f169b229..3a1f9123cdd0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,7 +89,7 @@ include: Examples: feat(consensus): add new opcode for BIP-XXXX OP_CHECKAWESOMESIG - feat(net): automatically create hidden service, listen on Tor + feat(net): automatically create onion service, listen on Tor feat(qt): add feed bump button fix(log): fix typo in log message feat(rpc)!: modify gettransaction parameter type diff --git a/doc/JSON-RPC-interface.md b/doc/JSON-RPC-interface.md index f220d78f02b5..e384fa041335 100644 --- a/doc/JSON-RPC-interface.md +++ b/doc/JSON-RPC-interface.md @@ -60,7 +60,7 @@ RPC interface will be abused. are sent as clear text that can be read by anyone on your network path. Additionally, the RPC interface has not been hardened to withstand arbitrary Internet traffic, so changing the above settings - to expose it to the Internet (even using something like a Tor hidden + to expose it to the Internet (even using something like a Tor onion service) could expose you to unconsidered vulnerabilities. See `dashd -help` for more information about these settings and other settings described in this document. diff --git a/doc/files.md b/doc/files.md index 1e7629cc3b54..c2697269fff1 100644 --- a/doc/files.md +++ b/doc/files.md @@ -65,7 +65,7 @@ Subdirectory | File(s) | Description `./` | `fee_estimates.dat` | Stores statistics used to estimate minimum transaction fees and priorities required for confirmation `./` | `guisettings.ini.bak` | Backup of former [GUI settings](#gui-settings) after `-resetguisettings` option is used `./` | `mempool.dat` | Dump of the mempool's transactions -`./` | `onion_v3_private_key` | Cached Tor hidden service private key for `-listenonion` option +`./` | `onion_v3_private_key` | Cached Tor onion service private key for `-listenonion` option `./` | `peers.dat` | Peer IP address database (custom format) `./` | `settings.json` | Read-write settings set through GUI or RPC interfaces, augmenting manual settings from [dash.conf](dash-conf.md). File is created automatically if read-write settings storage is not disabled with `-nosettings` option. Path can be specified with `-settings` option `./` | `.cookie` | Session RPC authentication cookie; if used, created at start and deleted on shutdown; can be specified by `-rpccookiefile` option diff --git a/doc/tor.md b/doc/tor.md index 40245a516d31..c531687d31ff 100644 --- a/doc/tor.md +++ b/doc/tor.md @@ -1,6 +1,6 @@ # TOR SUPPORT IN DASH CORE -It is possible to run Dash Core as a Tor hidden service, and connect to such services. +It is possible to run Dash Core as a Tor onion service, and connect to such services. The following directions assume you have a Tor proxy running on port 9050. Many distributions default to having a SOCKS proxy listening on port 9050, but others @@ -17,12 +17,12 @@ outgoing connections, but more is possible. -proxy=ip:port Set the proxy server. If SOCKS5 is selected (default), this proxy server will be used to try to reach .onion addresses as well. - -onion=ip:port Set the proxy server to use for Tor hidden services. You do not + -onion=ip:port Set the proxy server to use for Tor onion services. You do not need to set this if it's the same as -proxy. You can use -noonion - to explicitly disable access to hidden services. + to explicitly disable access to onion services. -listen When using -proxy, listening is disabled by default. If you want - to run a hidden service (see next section), you'll need to enable + to run an onion service (see next section), you'll need to enable it explicitly. -connect=X When behind a Tor proxy, you can specify .onion addresses instead @@ -120,11 +120,11 @@ Note: All these nodes are hosted by masternodehosting.com ## 4. Automatically listen on Tor Starting with Tor version 0.2.7.1 it is possible, through Tor's control socket -API, to create and destroy 'ephemeral' hidden services programmatically. +API, to create and destroy 'ephemeral' onion services programmatically. Dash Core has been updated to make use of this. This means that if Tor is running (and proper authentication has been configured), -Dash Core automatically creates a hidden service to listen on. This will positively +Dash Core automatically creates a onion service to listen on. This will positively affect the number of available .onion nodes. This new feature is enabled by default if Dash Core is listening (`-listen`), and @@ -136,7 +136,7 @@ Connecting to Tor's control socket API requires one of two authentication method configured. It also requires the control socket to be enabled, e.g. put `ControlPort 9051` in `torrc` config file. For cookie authentication the user running dashd must have read access to the `CookieAuthFile` specified in Tor configuration. In some cases this is -preconfigured and the creation of a hidden service is automatic. If permission problems +preconfigured and the creation of an onion service is automatic. If permission problems are seen with `-debug=tor` they can be resolved by adding both the user running Tor and the user running dashd to the same group and setting permissions appropriately. On Debian-based systems the user running dashd can be added to the debian-tor group, @@ -153,8 +153,8 @@ in the tor configuration file. The hashed password can be obtained with the comm ## 5. Privacy recommendations -- Do not add anything but Dash Core ports to the hidden service created in section 2. - If you run a web service too, create a new hidden service for that. - Otherwise it is trivial to link them, which may reduce privacy. Hidden +- Do not add anything but Dash Core ports to the onion service created in section 2. + If you run a web service too, create a new onion service for that. + Otherwise it is trivial to link them, which may reduce privacy. Onion services created automatically (as in section 3) always have only one port open. diff --git a/src/init.cpp b/src/init.cpp index a577223611c1..435efe5beecb 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -573,13 +573,13 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-externalip=", "Specify your own public address", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-listen", "Accept connections from outside (default: 1 if no -proxy or -connect)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-listenonion", strprintf("Automatically create Tor hidden service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-listenonion", strprintf("Automatically create Tor onion service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxconnections=", strprintf("Maintain at most connections to peers (temporary service connections excluded) (default: %u)", DEFAULT_MAX_PEER_CONNECTIONS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxreceivebuffer=", strprintf("Maximum per-connection receive buffer, *1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxsendbuffer=", strprintf("Maximum per-connection send buffer, *1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxuploadtarget=", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'download' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-onion=", "Use separate SOCKS5 proxy to reach peers via Tor hidden services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-onion=", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onlynet=", "Make outgoing connections only through network (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index c717070c57cf..4e2b204ef285 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -703,10 +703,10 @@ - Connect to the Dash network through a separate SOCKS5 proxy for Tor hidden services. + Connect to the Dash network through a separate SOCKS5 proxy for Tor onion services. - Use separate SOCKS&5 proxy to reach peers via Tor hidden services: + Use separate SOCKS&5 proxy to reach peers via Tor onion services: diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 85d7fd574c9e..0b7a8afc83ac 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -413,7 +413,7 @@ static bool WriteBinaryFile(const fs::path &filename, const std::string &data) /****** Bitcoin specific TorController implementation ********/ /** Controller that connects to Tor control socket, authenticate, then create - * and maintain an ephemeral hidden service. + * and maintain an ephemeral onion service. */ class TorController { @@ -545,7 +545,7 @@ void TorController::auth_cb(TorControlConnection& _conn, const TorControlReply& if (private_key.empty()) { // No private key, generate one private_key = "NEW:ED25519-V3"; // Explicitly request key type - see issue #9214 } - // Request hidden service, redirect port. + // Request onion service, redirect port. // Note that the 'virtual' port is always the default port to avoid decloaking nodes using other ports. _conn.Command(strprintf("ADD_ONION %s Port=%i,%s", private_key, Params().GetDefaultPort(), m_target.ToStringIPPort()), std::bind(&TorController::add_onion_cb, this, std::placeholders::_1, std::placeholders::_2)); From 052b2d0dc123418a5f402eed5d7a98831ea88063 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 23 Dec 2020 21:13:44 +0100 Subject: [PATCH 10/14] merge bitcoin#20757: tor.md and -onlynet help updates --- doc/tor.md | 153 ++++++++++++++++++++++++++++++++++++--------------- src/init.cpp | 2 +- 2 files changed, 110 insertions(+), 45 deletions(-) diff --git a/doc/tor.md b/doc/tor.md index c531687d31ff..7b4b367ab615 100644 --- a/doc/tor.md +++ b/doc/tor.md @@ -16,14 +16,19 @@ outgoing connections, but more is possible. -proxy=ip:port Set the proxy server. If SOCKS5 is selected (default), this proxy server will be used to try to reach .onion addresses as well. + You need to use -noonion or -onion=0 to explicitly disable + outbound access to onion services. -onion=ip:port Set the proxy server to use for Tor onion services. You do not - need to set this if it's the same as -proxy. You can use -noonion + need to set this if it's the same as -proxy. You can use -onion=0 to explicitly disable access to onion services. + Note: Only the -proxy option sets the proxy for DNS requests; + with -onion they will not route over Tor, so use -proxy if you + have privacy concerns. -listen When using -proxy, listening is disabled by default. If you want - to run an onion service (see next section), you'll need to enable - it explicitly. + to manually configure an onion service (see section 3), you'll + need to enable it explicitly. -connect=X When behind a Tor proxy, you can specify .onion addresses instead -addnode=X of IP addresses or hostnames in these parameters. It requires @@ -33,7 +38,11 @@ outgoing connections, but more is possible. -onlynet=onion Make outgoing connections only to .onion addresses. Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple network types, e.g. - ipv4, ipv6, or onion. + ipv4, ipv6 or onion. If you use this option with values other + than onion you *cannot* disable onion connections; outgoing onion + connections will be enabled when you use -proxy or -onion. Use + -noonion or -onion=0 if you want to be sure there are no outbound + onion connections over the default proxy or your defined -proxy. An example how to start the client if the Tor proxy is running on local host on port 9050 and only allows .onion nodes to connect: @@ -44,8 +53,99 @@ In a typical situation, this suffices to run behind a Tor proxy: ./dashd -proxy=127.0.0.1:9050 +## 2. Automatically create a Dash Core onion service -## 2. Run a Dash Core hidden server +Dash Core makes use of Tor's control socket API to create and destroy +ephemeral onion services programmatically. This means that if Tor is running and +proper authentication has been configured, Dash Core automatically creates an +onion service to listen on. The goal is to increase the number of available +onion nodes. + +This feature is enabled by default if Dash Core is listening (`-listen`) and +it requires a Tor connection to work. It can be explicitly disabled with +`-listenonion=0`. If it is not disabled, it can be configured using the +`-torcontrol` and `-torpassword` settings. + +To see verbose Tor information in the dashd debug log, pass `-debug=tor`. + +### Control Port + +You may need to set up the Tor Control Port. On Linux distributions there may be +some or all of the following settings in `/etc/tor/torrc`, generally commented +out by default (if not, add them): + +``` +ControlPort 9051 +CookieAuthentication 1 +CookieAuthFileGroupReadable 1 +``` + +Add or uncomment those, save, and restart Tor (usually `systemctl restart tor` +or `sudo systemctl restart tor` on most systemd-based systems, including recent +Debian and Ubuntu, or just restart the computer). + +On some systems (such as Arch Linux), you may also need to add the following +line: + +``` +DataDirectoryGroupReadable 1 +``` + +### Authentication + +Connecting to Tor's control socket API requires one of two authentication +methods to be configured: cookie authentication or dashd's `-torpassword` +configuration option. + +#### Cookie authentication + +For cookie authentication, the user running dashd must have read access to +the `CookieAuthFile` specified in the Tor configuration. In some cases this is +preconfigured and the creation of an onion service is automatic. Don't forget to +use the `-debug=tor` dashd configuration option to enable Tor debug logging. + +If a permissions problem is seen in the debug log, e.g. `tor: Authentication +cookie /run/tor/control.authcookie could not be opened (check permissions)`, it +can be resolved by adding both the user running Tor and the user running +dashd to the same Tor group and setting permissions appropriately. + +On Debian-derived systems, the Tor group will likely be `debian-tor` and one way +to verify could be to list the groups and grep for a "tor" group name: + +``` +getent group | cut -d: -f1 | grep -i tor +``` + +You can also check the group of the cookie file. On most Linux systems, the Tor +auth cookie will usually be `/run/tor/control.authcookie`: + +``` +stat -c '%G' /run/tor/control.authcookie +``` + +Once you have determined the `${TORGROUP}` and selected the `${USER}` that will +run dashd, run this as root: + +``` +usermod -a -G ${TORGROUP} ${USER} +``` + +Then restart the computer (or log out) and log in as the `${USER}` that will run +dashd. + +#### `torpassword` authentication + +For the `-torpassword=password` option, the password is the clear text form that +was used when generating the hashed password for the `HashedControlPassword` +option in the Tor configuration file. + +The hashed password can be obtained with the command `tor --hash-password +password` (refer to the [Tor Dev +Manual](https://2019.www.torproject.org/docs/tor-manual.html.en) for more +details). + + +## 3. Manually create a Dash Core onion service If you configure your Tor system accordingly, it is possible to make your node also reachable from the Tor network. Add these lines to your /etc/tor/torrc (or equivalent @@ -101,7 +201,7 @@ for normal IPv4/IPv6 communication, use: ./dashd -onion=127.0.0.1:9050 -externalip=ssapp53tmftyjmjb.onion -discover -## 3. List of known Dash Core Tor relays +## 3.1. List of known Dash Core Tor relays Note: All these nodes are hosted by masternodehosting.com @@ -116,45 +216,10 @@ Note: All these nodes are hosted by masternodehosting.com * ys5upbdeotplam3y.onion * fijy6aikzxfea54i.onion +## 4. Privacy recommendations -## 4. Automatically listen on Tor - -Starting with Tor version 0.2.7.1 it is possible, through Tor's control socket -API, to create and destroy 'ephemeral' onion services programmatically. -Dash Core has been updated to make use of this. - -This means that if Tor is running (and proper authentication has been configured), -Dash Core automatically creates a onion service to listen on. This will positively -affect the number of available .onion nodes. - -This new feature is enabled by default if Dash Core is listening (`-listen`), and -requires a Tor connection to work. It can be explicitly disabled with `-listenonion=0` -and, if not disabled, configured using the `-torcontrol` and `-torpassword` settings. -To show verbose debugging information, pass `-debug=tor`. - -Connecting to Tor's control socket API requires one of two authentication methods to be -configured. It also requires the control socket to be enabled, e.g. put `ControlPort 9051` -in `torrc` config file. For cookie authentication the user running dashd must have read -access to the `CookieAuthFile` specified in Tor configuration. In some cases this is -preconfigured and the creation of an onion service is automatic. If permission problems -are seen with `-debug=tor` they can be resolved by adding both the user running Tor and -the user running dashd to the same group and setting permissions appropriately. On -Debian-based systems the user running dashd can be added to the debian-tor group, -which has the appropriate permissions. Before starting dashd you will need to re-login -to allow debian-tor group to be applied. Otherwise you will see the following notice: "tor: -Authentication cookie /run/tor/control.authcookie could not be opened (check permissions)" -on debug.log. - -An alternative authentication method is the use -of the `-torpassword=password` option. The `password` is the clear text form that -was used when generating the hashed password for the `HashedControlPassword` option -in the tor configuration file. The hashed password can be obtained with the command -`tor --hash-password password` (read the tor manual for more details). - -## 5. Privacy recommendations - -- Do not add anything but Dash Core ports to the onion service created in section 2. +- Do not add anything but Dash Core ports to the onion service created in section 3. If you run a web service too, create a new onion service for that. Otherwise it is trivial to link them, which may reduce privacy. Onion - services created automatically (as in section 3) always have only one port + services created automatically (as in section 2) always have only one port open. diff --git a/src/init.cpp b/src/init.cpp index 435efe5beecb..7b1175c338c7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -580,7 +580,7 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxuploadtarget=", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'download' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onion=", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-onlynet=", "Make outgoing connections only through network (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-onlynet=", "Make outgoing connections only through network (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks. Warning: if it is used with ipv4 or ipv6 but not onion and the -onion or -proxy option is set, then outbound onion connections will still be made; use -noonion or -onion=0 to disable outbound onion connections in this case.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peertimeout=", strprintf("Specify p2p connection timeout in seconds. This option determines the amount of time a peer may be inactive before the connection to it is dropped. (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); From 3bfcf7a16b5b310d336ece46cfa6db2ba9b32671 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Tue, 19 Jan 2021 15:35:56 +0100 Subject: [PATCH 11/14] merge bitcoin#20965: return NET_UNROUTABLE as not_publicly_routable, automate helps --- src/init.cpp | 2 +- src/netaddress.h | 2 +- src/netbase.cpp | 16 +++++++++++++++- src/netbase.h | 2 ++ src/rpc/net.cpp | 4 ++-- test/functional/feature_proxy.py | 4 ++-- test/functional/rpc_net.py | 6 ++++++ 7 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 7b1175c338c7..f70fd941c9f6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -580,7 +580,7 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxuploadtarget=", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'download' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onion=", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-onlynet=", "Make outgoing connections only through network (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks. Warning: if it is used with ipv4 or ipv6 but not onion and the -onion or -proxy option is set, then outbound onion connections will still be made; use -noonion or -onion=0 to disable outbound onion connections in this case.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-onlynet=", "Make outgoing connections only through network (" + Join(GetNetworkNames(), ", ") + "). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks. Warning: if it is used with ipv4 or ipv6 but not onion and the -onion or -proxy option is set, then outbound onion connections will still be made; use -noonion or -onion=0 to disable outbound onion connections in this case.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peertimeout=", strprintf("Specify p2p connection timeout in seconds. This option determines the amount of time a peer may be inactive before the connection to it is dropped. (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); diff --git a/src/netaddress.h b/src/netaddress.h index 708a91c07481..604a1533917c 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -37,7 +37,7 @@ static constexpr int ADDRV2_FORMAT = 0x20000000; * @note An address may belong to more than one network, for example `10.0.0.1` * belongs to both `NET_UNROUTABLE` and `NET_IPV4`. * Keep these sequential starting from 0 and `NET_MAX` as the last entry. - * We have loops like `for (int i = 0; i < NET_MAX; i++)` that expect to iterate + * We have loops like `for (int i = 0; i < NET_MAX; ++i)` that expect to iterate * over all enum values and also `GetExtNetwork()` "extends" this enum by * introducing standalone constants starting from `NET_MAX`. */ diff --git a/src/netbase.cpp b/src/netbase.cpp index 60bbe0533fd3..18763fade503 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -102,7 +102,7 @@ enum Network ParseNetwork(const std::string& net_in) { std::string GetNetworkName(enum Network net) { switch (net) { - case NET_UNROUTABLE: return "unroutable"; + case NET_UNROUTABLE: return "not_publicly_routable"; case NET_IPV4: return "ipv4"; case NET_IPV6: return "ipv6"; case NET_ONION: return "onion"; @@ -115,6 +115,20 @@ std::string GetNetworkName(enum Network net) assert(false); } +std::vector GetNetworkNames(bool append_unroutable) +{ + std::vector names; + for (int n = 0; n < NET_MAX; ++n) { + const enum Network network{static_cast(n)}; + if (network == NET_UNROUTABLE || network == NET_I2P || network == NET_CJDNS || network == NET_INTERNAL) continue; + names.emplace_back(GetNetworkName(network)); + } + if (append_unroutable) { + names.emplace_back(GetNetworkName(NET_UNROUTABLE)); + } + return names; +} + static bool LookupIntern(const std::string& name, std::vector& vIP, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function) { vIP.clear(); diff --git a/src/netbase.h b/src/netbase.h index fe54a935ccd3..90f7f604e9c2 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -55,6 +55,8 @@ std::vector WrappedGetAddrInfo(const std::string& name, bool allow_loo enum Network ParseNetwork(const std::string& net); std::string GetNetworkName(enum Network net); +/** Return a vector of publicly routable Network names; optionally append NET_UNROUTABLE. */ +std::vector GetNetworkNames(bool append_unroutable = false); bool SetProxy(enum Network net, const proxyType &addrProxy); bool GetProxy(enum Network net, proxyType &proxyInfoOut); bool IsProxy(const CNetAddr &addr); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 51cc57931a97..9986ff9b1a73 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -90,7 +90,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"}, {RPCResult::Type::STR, "addrbind", "(ip:port) Bind address of the connection to the peer"}, {RPCResult::Type::STR, "addrlocal", "(ip:port) Local address as reported by the peer"}, - {RPCResult::Type::STR, "network", "Network (ipv4, ipv6, or onion) the peer connected through"}, + {RPCResult::Type::STR, "network", "Network (" + Join(GetNetworkNames(/* append_unroutable */ true), ", ") + ")"}, {RPCResult::Type::STR, "mapped_as", "The AS in the BGP route to the peer used for diversifying peer selection"}, {RPCResult::Type::STR_HEX, "services", "The services offered"}, {RPCResult::Type::STR_HEX, "verified_proregtx_hash", true /*optional*/, "Only present when the peer is a masternode and successfully " @@ -506,7 +506,7 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request) { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "name", "network (ipv4, ipv6 or onion)"}, + {RPCResult::Type::STR, "name", "network (" + Join(GetNetworkNames(), ", ") + ")"}, {RPCResult::Type::BOOL, "limited", "is the network limited using -onlynet?"}, {RPCResult::Type::BOOL, "reachable", "is the network reachable?"}, {RPCResult::Type::STR, "proxy", "(\"host:port\") the proxy that is used for this network, or empty if none"}, diff --git a/test/functional/feature_proxy.py b/test/functional/feature_proxy.py index 208057137af3..230808630c62 100755 --- a/test/functional/feature_proxy.py +++ b/test/functional/feature_proxy.py @@ -44,8 +44,8 @@ RANGE_BEGIN = PORT_MIN + 2 * PORT_RANGE # Start after p2p and rpc ports -# Networks returned by RPC getpeerinfo, defined in src/netbase.cpp::GetNetworkName() -NET_UNROUTABLE = "unroutable" +# Networks returned by RPC getpeerinfo. +NET_UNROUTABLE = "not_publicly_routable" NET_IPV4 = "ipv4" NET_IPV6 = "ipv6" NET_ONION = "onion" diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 3c3198521817..920431277065 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -118,6 +118,9 @@ def _test_getnetworkinfo(self): for info in network_info: assert_net_servicesnames(int(info["localservices"], 0x10), info["localservicesnames"]) + # Check dynamically generated networks list in getnetworkinfo help output. + assert "(ipv4, ipv6, onion)" in self.nodes[0].help("getnetworkinfo") + self.log.info('Test extended connections info') self.connect_nodes(1, 2) self.nodes[1].ping() @@ -158,6 +161,9 @@ def _test_getpeerinfo(self): for info in peer_info: assert_net_servicesnames(int(info[0]["services"], 0x10), info[0]["servicesnames"]) + # Check dynamically generated networks list in getpeerinfo help output. + assert "(ipv4, ipv6, onion, not_publicly_routable)" in self.nodes[0].help("getpeerinfo") + def test_service_flags(self): self.nodes[0].add_p2p_connection(P2PInterface(), services=(1 << 4) | (1 << 63)) assert_equal(['UNKNOWN[2^4]', 'UNKNOWN[2^63]'], self.nodes[0].getpeerinfo()[-1]['servicesnames']) From 1ef6322fc2663d224585e525a301880405c5b374 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Mon, 2 Nov 2020 11:36:29 +0100 Subject: [PATCH 12/14] merge bitcoin#20284: ensure old versions don't parse peers.dat --- src/addrman.h | 78 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 25 deletions(-) diff --git a/src/addrman.h b/src/addrman.h index 21e767ebba7b..9566cfbf5bb1 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -7,6 +7,7 @@ #define BITCOIN_ADDRMAN_H #include +#include #include #include #include @@ -177,6 +178,28 @@ class CAddrMan mutable RecursiveMutex cs; private: + //! Serialization versions. + enum Format : uint8_t { + V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88 + V1_DETERMINISTIC = 1, //!< for pre-asmap files + V2_ASMAP = 2, //!< for files including asmap version + V3_BIP155 = 3, //!< same as V2_ASMAP plus addresses are in BIP155 format + }; + + //! The maximum format this software knows it can unserialize. Also, we always serialize + //! in this format. + //! The format (first byte in the serialized stream) can be higher than this and + //! still this software may be able to unserialize the file - if the second byte + //! (see `lowest_compatible` in `Unserialize()`) is less or equal to this. + static constexpr Format FILE_FORMAT = Format::V3_BIP155; + + //! The initial value of a field that is incremented every time an incompatible format + //! change is made (such that old software versions would not be able to parse and + //! understand the new file format). This is 32 because we overtook the "key size" + //! field which was 32 historically. + //! @note Don't increment this. Increment `lowest_compatible` in `Serialize()` instead. + static constexpr uint8_t INCOMPATIBILITY_BASE = 32; + //! last used nId int nIdCount GUARDED_BY(cs); @@ -282,14 +305,6 @@ class CAddrMan CAddrInfo GetAddressInfo_(const CService& addr) EXCLUSIVE_LOCKS_REQUIRED(cs); public: - //! Serialization versions. - enum class Format : uint8_t { - V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88 - V1_DETERMINISTIC = 1, //!< for pre-asmap files - V2_ASMAP = 2, //!< for files including asmap version - V3_BIP155 = 3, //!< same as V2_ASMAP plus addresses are in BIP155 format - }; - // 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. @@ -312,8 +327,18 @@ class CAddrMan /** * Serialized format. - * * version byte (@see `Format`) - * * 0x20 + nKey (serialized as if it were a vector, for backward compatibility) + * * format version byte (@see `Format`) + * * lowest compatible format version byte. This is used to help old software decide + * whether to parse the file. For example: + * * Bitcoin Core version N knows how to parse up to format=3. If a new format=4 is + * introduced in version N+1 that is compatible with format=3 and it is known that + * version N will be able to parse it, then version N+1 will write + * (format=4, lowest_compatible=3) in the first two bytes of the file, and so + * version N will still try to parse it. + * * Bitcoin Core version N+2 introduces a new incompatible format=5. It will write + * (format=5, lowest_compatible=5) and so any versions that do not know how to parse + * format=5 will not try to read the file. + * * nKey * * nNew * * nTried * * number of "new" buckets XOR 2**30 @@ -342,12 +367,17 @@ class CAddrMan { LOCK(cs); - // Always serialize in the latest version (currently Format::V3_BIP155). + // Always serialize in the latest version (FILE_FORMAT). OverrideStream s(&s_, s_.GetType(), s_.GetVersion() | ADDRV2_FORMAT); - s << static_cast(Format::V3_BIP155); - s << ((unsigned char)32); + s << static_cast(FILE_FORMAT); + + // Increment `lowest_compatible` iff a newly introduced format is incompatible with + // the previous one. + static constexpr uint8_t lowest_compatible = Format::V3_BIP155; + s << static_cast(INCOMPATIBILITY_BASE + lowest_compatible); + s << nKey; s << nNew; s << nTried; @@ -407,15 +437,6 @@ class CAddrMan Format format; s_ >> Using>(format); - static constexpr Format maximum_supported_format = Format::V3_BIP155; - if (format > maximum_supported_format) { - throw std::ios_base::failure(strprintf( - "Unsupported format of addrman database: %u. Maximum supported is %u. " - "Continuing operation without using the saved list of peers.", - static_cast(format), - static_cast(maximum_supported_format))); - } - int stream_version = s_.GetVersion(); if (format >= Format::V3_BIP155) { // Add ADDRV2_FORMAT to the version so that the CNetAddr and CAddress @@ -425,9 +446,16 @@ class CAddrMan OverrideStream s(&s_, s_.GetType(), stream_version); - unsigned char nKeySize; - s >> nKeySize; - if (nKeySize != 32) throw std::ios_base::failure("Incorrect keysize in addrman deserialization"); + uint8_t compat; + s >> compat; + const uint8_t lowest_compatible = compat - INCOMPATIBILITY_BASE; + if (lowest_compatible > FILE_FORMAT) { + throw std::ios_base::failure(strprintf( + "Unsupported format of addrman database: %u. It is compatible with formats >=%u, " + "but the maximum supported by this version of %s is %u.", + format, lowest_compatible, PACKAGE_NAME, static_cast(FILE_FORMAT))); + } + s >> nKey; s >> nNew; s >> nTried; From 31acbf1e2d8cecb625eb44d7fe690c9b1f8a277d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 31 May 2023 11:21:49 +0530 Subject: [PATCH 13/14] merge bitcoin#22025: Group and re-order CAddrMan members by access type --- src/addrman.h | 263 +++++++++++++++++++++++++------------------------- 1 file changed, 130 insertions(+), 133 deletions(-) diff --git a/src/addrman.h b/src/addrman.h index 9566cfbf5bb1..d3020d896805 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -171,139 +171,6 @@ static const int64_t ADDRMAN_TEST_WINDOW = 40*60; // 40 minutes */ class CAddrMan { -protected: - friend class CAddrManTest; - - //! critical section to protect the inner data structures - mutable RecursiveMutex cs; - -private: - //! Serialization versions. - enum Format : uint8_t { - V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88 - V1_DETERMINISTIC = 1, //!< for pre-asmap files - V2_ASMAP = 2, //!< for files including asmap version - V3_BIP155 = 3, //!< same as V2_ASMAP plus addresses are in BIP155 format - }; - - //! The maximum format this software knows it can unserialize. Also, we always serialize - //! in this format. - //! The format (first byte in the serialized stream) can be higher than this and - //! still this software may be able to unserialize the file - if the second byte - //! (see `lowest_compatible` in `Unserialize()`) is less or equal to this. - static constexpr Format FILE_FORMAT = Format::V3_BIP155; - - //! The initial value of a field that is incremented every time an incompatible format - //! change is made (such that old software versions would not be able to parse and - //! understand the new file format). This is 32 because we overtook the "key size" - //! field which was 32 historically. - //! @note Don't increment this. Increment `lowest_compatible` in `Serialize()` instead. - static constexpr uint8_t INCOMPATIBILITY_BASE = 32; - - //! last used nId - int nIdCount GUARDED_BY(cs); - - //! table with information about all nIds - std::map mapInfo GUARDED_BY(cs); - - //! find an nId based on its network address - std::map mapAddr GUARDED_BY(cs); - - //! randomly-ordered vector of all nIds - std::vector vRandom GUARDED_BY(cs); - - // number of "tried" entries - int nTried GUARDED_BY(cs); - - //! list of "tried" buckets - int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); - - //! number of (unique) "new" entries - int nNew GUARDED_BY(cs); - - //! list of "new" buckets - int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); - - //! last time Good was called (memory only) - int64_t nLastGood GUARDED_BY(cs); - - // discriminate entries based on port. Should be false on mainnet/testnet and can be true on devnet/regtest - bool discriminatePorts GUARDED_BY(cs); - - //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions. - std::set m_tried_collisions; - -protected: - //! secret key to randomize bucket select with - uint256 nKey; - - //! Source of random numbers for randomization in inner loops - FastRandomContext insecure_rand; - - //! Find an entry. - CAddrInfo* Find(const CService& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! find an entry, creating it if necessary. - //! nTime and nServices of the found node are updated, if necessary. - CAddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Swap two elements in vRandom. - void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Move an entry from the "new" table(s) to the "tried" table - void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Delete an entry. It must not be in tried, and have refcount 0. - void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Clear a position in a "new" table. This is the only place where entries are actually deleted. - void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Mark an entry "good", possibly moving it from "new" to "tried". - void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Add an entry to the "new" table. - bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Mark an entry as attempted to connect. - void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. - CAddrInfo Select_(bool newOnly) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. - void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Return a random to-be-evicted tried table address. - CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); - -#ifdef DEBUG_ADDRMAN - //! Perform consistency check. Returns an error code or zero. - int Check_() EXCLUSIVE_LOCKS_REQUIRED(cs); -#endif - - //! Select several addresses at once. - void GetAddr_(std::vector &vAddr, size_t max_addresses, size_t max_pct) EXCLUSIVE_LOCKS_REQUIRED(cs); - - /** We have successfully connected to this peer. Calling this function - * updates the CAddress's nTime, which is used in our IsTerrible() - * decisions and gossiped to peers. Callers should be careful that updating - * this information doesn't leak topology information to network spies. - * - * net_processing calls this function when it *disconnects* from a peer to - * not leak information about currently connected peers. - * - * @param[in] addr The address of the peer we were connected to - * @param[in] nTime The time that we were last connected to this peer - */ - void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Update an entry's service bits. - void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Get address info for address - CAddrInfo GetAddressInfo_(const CService& addr) EXCLUSIVE_LOCKS_REQUIRED(cs); - public: // Compressed IP->ASN mapping, loaded from a file when a node starts. // Should be always empty if no file was provided. @@ -770,7 +637,137 @@ class CAddrMan } return addrRet; } +protected: + //! secret key to randomize bucket select with + uint256 nKey; + + //! Source of random numbers for randomization in inner loops + FastRandomContext insecure_rand; + +private: + //! critical section to protect the inner data structures + mutable RecursiveMutex cs; + + //! Serialization versions. + enum Format : uint8_t { + V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88 + V1_DETERMINISTIC = 1, //!< for pre-asmap files + V2_ASMAP = 2, //!< for files including asmap version + V3_BIP155 = 3, //!< same as V2_ASMAP plus addresses are in BIP155 format + }; + + //! The maximum format this software knows it can unserialize. Also, we always serialize + //! in this format. + //! The format (first byte in the serialized stream) can be higher than this and + //! still this software may be able to unserialize the file - if the second byte + //! (see `lowest_compatible` in `Unserialize()`) is less or equal to this. + static constexpr Format FILE_FORMAT = Format::V3_BIP155; + + //! The initial value of a field that is incremented every time an incompatible format + //! change is made (such that old software versions would not be able to parse and + //! understand the new file format). This is 32 because we overtook the "key size" + //! field which was 32 historically. + //! @note Don't increment this. Increment `lowest_compatible` in `Serialize()` instead. + static constexpr uint8_t INCOMPATIBILITY_BASE = 32; + + //! last used nId + int nIdCount GUARDED_BY(cs); + + //! table with information about all nIds + std::map mapInfo GUARDED_BY(cs); + + //! find an nId based on its network address + std::map mapAddr GUARDED_BY(cs); + + //! randomly-ordered vector of all nIds + std::vector vRandom GUARDED_BY(cs); + + // number of "tried" entries + int nTried GUARDED_BY(cs); + + //! list of "tried" buckets + int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); + + //! number of (unique) "new" entries + int nNew GUARDED_BY(cs); + + //! list of "new" buckets + int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); + + //! last time Good was called (memory only) + int64_t nLastGood GUARDED_BY(cs); + + // discriminate entries based on port. Should be false on mainnet/testnet and can be true on devnet/regtest + bool discriminatePorts GUARDED_BY(cs); + + //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions. + std::set m_tried_collisions; + //! Find an entry. + CAddrInfo* Find(const CService& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! find an entry, creating it if necessary. + //! nTime and nServices of the found node are updated, if necessary. + CAddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Swap two elements in vRandom. + void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Move an entry from the "new" table(s) to the "tried" table + void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Delete an entry. It must not be in tried, and have refcount 0. + void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Clear a position in a "new" table. This is the only place where entries are actually deleted. + void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Mark an entry "good", possibly moving it from "new" to "tried". + void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Add an entry to the "new" table. + bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Mark an entry as attempted to connect. + void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. + CAddrInfo Select_(bool newOnly) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. + void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Return a random to-be-evicted tried table address. + CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); + +#ifdef DEBUG_ADDRMAN + //! Perform consistency check. Returns an error code or zero. + int Check_() EXCLUSIVE_LOCKS_REQUIRED(cs); +#endif + + //! Select several addresses at once. + void GetAddr_(std::vector &vAddr, size_t max_addresses, size_t max_pct) EXCLUSIVE_LOCKS_REQUIRED(cs); + + /** We have successfully connected to this peer. Calling this function + * updates the CAddress's nTime, which is used in our IsTerrible() + * decisions and gossiped to peers. Callers should be careful that updating + * this information doesn't leak topology information to network spies. + * + * net_processing calls this function when it *disconnects* from a peer to + * not leak information about currently connected peers. + * + * @param[in] addr The address of the peer we were connected to + * @param[in] nTime The time that we were last connected to this peer + */ + void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Update an entry's service bits. + void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Get address info for address + CAddrInfo GetAddressInfo_(const CService& addr) EXCLUSIVE_LOCKS_REQUIRED(cs); + + friend class CAddrManTest; }; #endif // BITCOIN_ADDRMAN_H From 99b336debafd3dbd5bd03f352c875d02a47ddd67 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Fri, 4 Jun 2021 16:09:25 +0200 Subject: [PATCH 14/14] merge bitcoin#22179: Torv2 removal followups --- src/addrman.cpp | 32 +++++++++++++++++++++++++++++ src/addrman.h | 5 +++++ src/netaddress.cpp | 2 +- src/test/addrman_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++ src/test/fuzz/netaddress.cpp | 2 +- 5 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index c7fe861678ee..4153cc1f4d48 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -73,6 +73,38 @@ double CAddrInfo::GetChance(int64_t nNow) const return fChance; } +void CAddrMan::RemoveInvalid() +{ + for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; ++bucket) { + for (size_t i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) { + const auto id = vvNew[bucket][i]; + if (id != -1 && !mapInfo[id].IsValid()) { + ClearNew(bucket, i); + } + } + } + + for (size_t bucket = 0; bucket < ADDRMAN_TRIED_BUCKET_COUNT; ++bucket) { + for (size_t i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) { + const auto id = vvTried[bucket][i]; + if (id == -1) { + continue; + } + const auto& addr_info = mapInfo[id]; + if (addr_info.IsValid()) { + continue; + } + vvTried[bucket][i] = -1; + --nTried; + SwapRandom(addr_info.nRandomPos, vRandom.size() - 1); + vRandom.pop_back(); + mapAddr.erase(addr_info); + mapInfo.erase(id); + m_tried_collisions.erase(id); + } + } +} + CAddrInfo* CAddrMan::Find(const CService& addr, int* pnId) { CService addr2 = addr; diff --git a/src/addrman.h b/src/addrman.h index d3020d896805..a0530d81fc47 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -454,6 +454,8 @@ class CAddrMan LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions\n", nLostUnk, nLost); } + RemoveInvalid(); + Check(); } @@ -767,6 +769,9 @@ class CAddrMan //! Get address info for address CAddrInfo GetAddressInfo_(const CService& addr) EXCLUSIVE_LOCKS_REQUIRED(cs); + //! Remove invalid addresses. + void RemoveInvalid() EXCLUSIVE_LOCKS_REQUIRED(cs); + friend class CAddrManTest; }; diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 78e7fa871bd3..989b6dcd0f38 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -475,7 +475,7 @@ bool CNetAddr::IsRoutable() const return false; if (!fAllowPrivateNet && IsRFC1918()) return false; - return !(IsRFC2544() || IsRFC3927() || IsRFC4862() || IsRFC6598() || IsRFC5737() || (IsRFC4193() && !IsTor()) || IsRFC4843() || IsRFC7343() || IsLocal() || IsInternal()); + return !(IsRFC2544() || IsRFC3927() || IsRFC4862() || IsRFC6598() || IsRFC5737() || IsRFC4193() || IsRFC4843() || IsRFC7343() || IsLocal() || IsInternal()); } /** diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 0fb97ccd1a08..f84d7d6f910a 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -783,6 +783,46 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) BOOST_CHECK(bucketAndEntry_asmap1_deser_addr1.second != bucketAndEntry_asmap1_deser_addr2.second); } +BOOST_AUTO_TEST_CASE(remove_invalid) +{ + // Confirm that invalid addresses are ignored in unserialization. + + CAddrManTest addrman; + CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); + + const CAddress new1{ResolveService("5.5.5.5"), NODE_NONE}; + const CAddress new2{ResolveService("6.6.6.6"), NODE_NONE}; + const CAddress tried1{ResolveService("7.7.7.7"), NODE_NONE}; + const CAddress tried2{ResolveService("8.8.8.8"), NODE_NONE}; + + addrman.Add({new1, tried1, new2, tried2}, CNetAddr{}); + addrman.Good(tried1); + addrman.Good(tried2); + BOOST_REQUIRE_EQUAL(addrman.size(), 4); + + stream << addrman; + + const std::string str{stream.str()}; + size_t pos; + + const char new2_raw[]{6, 6, 6, 6}; + const uint8_t new2_raw_replacement[]{0, 0, 0, 0}; // 0.0.0.0 is !IsValid() + pos = str.find(new2_raw, 0, sizeof(new2_raw)); + BOOST_REQUIRE(pos != std::string::npos); + BOOST_REQUIRE(pos + sizeof(new2_raw_replacement) <= stream.size()); + memcpy(stream.data() + pos, new2_raw_replacement, sizeof(new2_raw_replacement)); + + const char tried2_raw[]{8, 8, 8, 8}; + const uint8_t tried2_raw_replacement[]{255, 255, 255, 255}; // 255.255.255.255 is !IsValid() + pos = str.find(tried2_raw, 0, sizeof(tried2_raw)); + BOOST_REQUIRE(pos != std::string::npos); + BOOST_REQUIRE(pos + sizeof(tried2_raw_replacement) <= stream.size()); + memcpy(stream.data() + pos, tried2_raw_replacement, sizeof(tried2_raw_replacement)); + + addrman.Clear(); + stream >> addrman; + BOOST_CHECK_EQUAL(addrman.size(), 2); +} BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) { diff --git a/src/test/fuzz/netaddress.cpp b/src/test/fuzz/netaddress.cpp index 6e9bb47ff6d4..521b9d283e67 100644 --- a/src/test/fuzz/netaddress.cpp +++ b/src/test/fuzz/netaddress.cpp @@ -55,7 +55,7 @@ FUZZ_TARGET(netaddress) (void)net_addr.IsRFC3927(); (void)net_addr.IsRFC3964(); if (net_addr.IsRFC4193()) { - assert(net_addr.GetNetwork() == Network::NET_ONION || net_addr.GetNetwork() == Network::NET_INTERNAL || net_addr.GetNetwork() == Network::NET_UNROUTABLE); + assert(net_addr.GetNetwork() == Network::NET_INTERNAL || net_addr.GetNetwork() == Network::NET_UNROUTABLE); } (void)net_addr.IsRFC4380(); (void)net_addr.IsRFC4843();