From 134c11e9f1ee09963f847f909964e671c035dcd7 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Tue, 29 Jan 2019 13:40:45 -1000 Subject: [PATCH 1/9] Merge #14929: net: Allow connections from misbehavior banned peers 0297be61a Allow connections from misbehavior banned peers. (Gregory Maxwell) Pull request description: This allows incoming connections from peers which are only banned due to an automatic misbehavior ban if doing so won't fill inbound. These peers are preferred for eviction when inbound fills, but may still be kept if they fall into the protected classes. This eviction preference lasts the entire life of the connection even if the ban expires. If they misbehave again they'll still get disconnected. The main purpose of banning on misbehavior is to prevent our connections from being wasted on unhelpful peers such as ones running incompatible consensus rules. For inbound peers this can be better accomplished with eviction preferences. A secondary purpose was to reduce resource waste from repeated abuse but virtually any attacker can get a nearly unlimited supply of addresses, so disconnection is about the best we can do. This can reduce the potential from negative impact due to incorrect misbehaviour bans. Tree-SHA512: 03bc8ec8bae365cc437daf70000c8f2edc512e37db821bc4e0fafa6cf56cc185e9ab40453aa02445f48d6a2e3e7268767ca2017655aca5383108416f1e2cf20f --- src/banman.cpp | 29 ++++++++++++++++++++++++++--- src/banman.h | 1 + src/net.cpp | 19 +++++++++++++++++-- src/net.h | 1 + 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/banman.cpp b/src/banman.cpp index 9933c829c5..47d64a8f31 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -67,14 +67,36 @@ void BanMan::ClearBanned() if (m_client_interface) m_client_interface->BannedListChanged(); } -bool BanMan::IsBanned(CNetAddr net_addr) +int BanMan::IsBannedLevel(CNetAddr net_addr) { + // Returns the most severe level of banning that applies to this address. + // 0 - Not banned + // 1 - Automatic misbehavior ban + // 2 - Any other ban + int level = 0; + auto current_time = GetTime(); LOCK(m_cs_banned); for (const auto& it : m_banned) { CSubNet sub_net = it.first; CBanEntry ban_entry = it.second; - if (sub_net.Match(net_addr) && GetTime() < ban_entry.nBanUntil) { + if (current_time < ban_entry.nBanUntil && sub_net.Match(net_addr)) { + if (ban_entry.banReason != BanReasonNodeMisbehaving) return 2; + level = 1; + } + } + return level; +} + +bool BanMan::IsBanned(CNetAddr net_addr) +{ + auto current_time = GetTime(); + LOCK(m_cs_banned); + for (const auto& it : m_banned) { + CSubNet sub_net = it.first; + CBanEntry ban_entry = it.second; + + if (current_time < ban_entry.nBanUntil && sub_net.Match(net_addr)) { return true; } } @@ -83,11 +105,12 @@ bool BanMan::IsBanned(CNetAddr net_addr) bool BanMan::IsBanned(CSubNet sub_net) { + auto current_time = GetTime(); LOCK(m_cs_banned); banmap_t::iterator i = m_banned.find(sub_net); if (i != m_banned.end()) { CBanEntry ban_entry = (*i).second; - if (GetTime() < ban_entry.nBanUntil) { + if (current_time < ban_entry.nBanUntil) { return true; } } diff --git a/src/banman.h b/src/banman.h index 69f62be368..a1a00309dd 100644 --- a/src/banman.h +++ b/src/banman.h @@ -42,6 +42,7 @@ public: void Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false); void Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false); void ClearBanned(); + int IsBannedLevel(CNetAddr net_addr); bool IsBanned(CNetAddr net_addr); bool IsBanned(CSubNet sub_net); bool Unban(const CNetAddr& net_addr); diff --git a/src/net.cpp b/src/net.cpp index ad47c4eccc..c9bd07fa59 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -848,6 +848,7 @@ struct NodeEvictionCandidate bool fRelayTxes; bool fBloomFilter; uint64_t nKeyedNetGroup; + bool prefer_evict; }; static bool ReverseCompareNodeMinPingTime(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b) @@ -935,7 +936,8 @@ bool CConnman::AttemptToEvictConnection() NodeEvictionCandidate candidate = {node->GetId(), node->nTimeConnected, node->nMinPingUsecTime, node->nLastBlockTime, node->nLastTXTime, HasAllDesirableServiceFlags(node->nServices), - node->fRelayTxes, node->pfilter != nullptr, node->nKeyedNetGroup}; + node->fRelayTxes, node->pfilter != nullptr, node->nKeyedNetGroup, + node->m_prefer_evict}; vEvictionCandidates.push_back(candidate); } } @@ -960,6 +962,14 @@ bool CConnman::AttemptToEvictConnection() if (vEvictionCandidates.empty()) return false; + // If any remaining peers are preferred for eviction consider only them. + // This happens after the other preferences since if a peer is really the best by other criteria (esp relaying blocks) + // then we probably don't want to evict it no matter what. + if (std::any_of(vEvictionCandidates.begin(),vEvictionCandidates.end(),[](NodeEvictionCandidate const &n){return n.prefer_evict;})) { + vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.begin(),vEvictionCandidates.end(), + [](NodeEvictionCandidate const &n){return !n.prefer_evict;}),vEvictionCandidates.end()); + } + // Identify the network group with the most connections and youngest member. // (vEvictionCandidates is already sorted by reverse connect time) uint64_t naMostConnections; @@ -1054,7 +1064,11 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { // on all platforms. Set it again here just to be sure. SetSocketNoDelay(hSocket); - if (m_banman && m_banman->IsBanned(addr) && !whitelisted) + int bannedlevel = m_banman ? m_banman->IsBannedLevel(addr) : 0; + + // Don't accept connections from banned peers, but if our inbound slots aren't almost full, accept + // if the only banning reason was an automatic misbehavior ban. + if (!whitelisted && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0)) { LogPrint(BCLog::NET, "%s (banned)\n", strDropped); CloseSocket(hSocket); @@ -1091,6 +1105,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true); pnode->AddRef(); pnode->fWhitelisted = whitelisted; + pnode->m_prefer_evict = bannedlevel > 0; m_msgproc->InitializeNode(pnode); if (fLogIPs) { diff --git a/src/net.h b/src/net.h index 716c49d4f0..5551f08caa 100644 --- a/src/net.h +++ b/src/net.h @@ -855,6 +855,7 @@ public: */ std::string cleanSubVer GUARDED_BY(cs_SubVer){}; CCriticalSection cs_SubVer; // used for both cleanSubVer and strSubVer + bool m_prefer_evict{false}; // This peer is preferred for eviction. bool fWhitelisted; // This peer can bypass DoS banning. bool fFeeler; // If true this node is being used as a short lived feeler. bool fOneShot; From a7ad763bcbc0fa592134cebe5d6d6cd8c7f48f0e Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 8 Feb 2019 08:58:27 -0500 Subject: [PATCH 2/9] Merge #15201: net: Add missing locking annotation for vNodes. vNodes is guarded by cs_vNodes. eea02be70e Add locking annotation for vNodes. vNodes is guarded by cs_vNodes. (practicalswift) Pull request description: Add locking annotation for `vNodes`. `vNodes` is guarded by `cs_vNodes`. Tree-SHA512: b1e18be22ba5b9dd153536380321b09b30a75a20575f975af9af94164f51982b32267ba0994e77c801513b59da05d923a974a9d2dfebdac48024c4bda98b53af --- src/net.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/net.h b/src/net.h index 5551f08caa..a64479c79d 100644 --- a/src/net.h +++ b/src/net.h @@ -210,7 +210,18 @@ public: CConnman(uint64_t seed0, uint64_t seed1); ~CConnman(); bool Start(CScheduler& scheduler, const Options& options); - void Stop(); + + // TODO: Remove NO_THREAD_SAFETY_ANALYSIS. Lock cs_vNodes before reading the variable vNodes. + // + // When removing NO_THREAD_SAFETY_ANALYSIS be aware of the following lock order requirements: + // * CheckForStaleTipAndEvictPeers locks cs_main before indirectly calling GetExtraOutboundCount + // which locks cs_vNodes. + // * ProcessMessage locks cs_main and g_cs_orphans before indirectly calling ForEachNode which + // locks cs_vNodes. + // + // Thus the implicit locking order requirement is: (1) cs_main, (2) g_cs_orphans, (3) cs_vNodes. + void Stop() NO_THREAD_SAFETY_ANALYSIS; + void Interrupt(); bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; @@ -561,7 +572,7 @@ private: std::map, std::set> masternodeQuorumRelayMembers; // protected by cs_vPendingMasternodes std::set masternodePendingProbes; mutable CCriticalSection cs_vPendingMasternodes; - std::vector vNodes; + std::vector vNodes GUARDED_BY(cs_vNodes); std::list vNodesDisconnected; std::unordered_map mapSocketToNode; mutable CCriticalSection cs_vNodes; From 9fd70ab11bf5e7a176f8b155d74795b40ff2ec28 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 14 Feb 2019 16:07:12 -0500 Subject: [PATCH 3/9] Merge #14626: Select orphan transaction uniformly for eviction 7257353b93 Select orphan transaction uniformly for eviction (Pieter Wuille) Pull request description: The previous code was biased towards evicting transactions whose txid has a larger gap (lexicographically) with the previous txid in the orphan pool. Tree-SHA512: e35f700aea5ed79d1bc57f64bffcb623424b40156fd0a12f05f74f981a8aa4175d5c18d042989243f7559242bdf1d6d720bcf588d28f43d74a798a4843f09c70 Signed-off-by: pasta --- src/net_processing.cpp | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ea136c1fb5..f19b35a921 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -114,6 +114,7 @@ struct COrphanTx { CTransactionRef tx; NodeId fromPeer; int64_t nTimeExpire; + size_t list_pos; size_t nTxSize; }; CCriticalSection g_cs_orphans; @@ -213,6 +214,8 @@ namespace { }; std::map::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans); + std::vector::iterator> g_orphan_list GUARDED_BY(g_cs_orphans); //! For random eviction + static size_t vExtraTxnForCompactIt GUARDED_BY(g_cs_orphans) = 0; static std::vector> vExtraTxnForCompact GUARDED_BY(g_cs_orphans); } // namespace @@ -956,8 +959,9 @@ bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRE return false; } - auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, sz}); + auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, g_orphan_list.size(), sz}); assert(ret.second); + g_orphan_list.push_back(ret.first); for (const CTxIn& txin : tx->vin) { mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first); } @@ -987,6 +991,18 @@ int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) if (itPrev->second.empty()) mapOrphanTransactionsByPrev.erase(itPrev); } + + size_t old_pos = it->second.list_pos; + assert(g_orphan_list[old_pos] == it); + if (old_pos + 1 != g_orphan_list.size()) { + // Unless we're deleting the last entry in g_orphan_list, move the last + // entry to the position we're deleting. + auto it_last = g_orphan_list.back(); + g_orphan_list[old_pos] = it_last; + it_last->second.list_pos = old_pos; + } + g_orphan_list.pop_back(); + assert(nMapOrphanTransactionsSize >= it->second.nTxSize); nMapOrphanTransactionsSize -= it->second.nTxSize; mapOrphanTransactions.erase(it); @@ -1040,11 +1056,8 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphansSize) while (!mapOrphanTransactions.empty() && nMapOrphanTransactionsSize > nMaxOrphansSize) { // Evict a random orphan: - uint256 randomhash = GetRandHash(); - std::map::iterator it = mapOrphanTransactions.lower_bound(randomhash); - if (it == mapOrphanTransactions.end()) - it = mapOrphanTransactions.begin(); - EraseOrphanTx(it->first); + size_t randompos = rng.randrange(g_orphan_list.size()); + EraseOrphanTx(g_orphan_list[randompos]->first); ++nEvicted; } return nEvicted; From 6c75d2027729c60acd63058b118d916c4f5c247b Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 14 Aug 2019 16:35:54 +0200 Subject: [PATCH 4/9] Merge #16248: Make whitebind/whitelist permissions more flexible c5b404e8f1973afe071a07c63ba1038eefe13f0f Add functional tests for flexible whitebind/list (nicolas.dorier) d541fa391844f658bd7035659b5b16695733dd56 Replace the use of fWhitelisted by permission checks (nicolas.dorier) ecd5cf7ea4c3644a30092100ffc399e30e193275 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier) e5b26deaaa6842f7dd7c4537ede000f965ea0189 Make whitebind/whitelist permissions more flexible (nicolas.dorier) Pull request description: # Motivation In 0.19, bloom filter will be disabled by default. I tried to make [a PR](https://github.com/bitcoin/bitcoin/pull/16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`. Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum. It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes. When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](https://github.com/bitcoin/bitcoin/pull/16176#issuecomment-500762907) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute. Doing so will also make follow up idea very easy to implement in a backward compatible way. # Implementation details The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`. The following permissions exists: * ForceRelay * Relay * NoBan * BloomFilter * Mempool Example: * `-whitelist=bloomfilter@127.0.0.1/32`. * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`. If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible) When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist` and add to it the permissions granted from `whitebind`. To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node. `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`. # Follow up idea Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way: * Changing `connect` at rpc and config file level to understand the permissions flags. * Changing the permissions of a peer at RPC level. ACKs for top commit: laanwj: re-ACK c5b404e8f1973afe071a07c63ba1038eefe13f0f Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577 --- src/Makefile.am | 2 + src/init.cpp | 20 ++-- src/masternode/masternode-sync.cpp | 2 +- src/net.cpp | 64 +++++++----- src/net.h | 35 ++++--- src/net_permissions.cpp | 105 ++++++++++++++++++++ src/net_permissions.h | 62 ++++++++++++ src/net_processing.cpp | 38 ++++--- src/qt/rpcconsole.cpp | 2 +- src/rpc/net.cpp | 8 +- src/test/netbase_tests.cpp | 79 +++++++++++++++ test/functional/p2p_permissions.py | 97 ++++++++++++++++++ test/functional/test_framework/test_node.py | 1 + test/functional/test_runner.py | 1 + 14 files changed, 449 insertions(+), 67 deletions(-) create mode 100644 src/net_permissions.cpp create mode 100644 src/net_permissions.h create mode 100644 test/functional/p2p_permissions.py diff --git a/src/Makefile.am b/src/Makefile.am index 34bd6dca22..94e2dfd54b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -213,6 +213,7 @@ BITCOIN_CORE_H = \ messagesigner.h \ miner.h \ net.h \ + net_permissions.h \ net_processing.h \ netaddress.h \ netbase.h \ @@ -570,6 +571,7 @@ libdash_common_a_SOURCES = \ keystore.cpp \ netaddress.cpp \ netbase.cpp \ + net_permissions.cpp \ policy/feerate.cpp \ protocol.cpp \ saltedhasher.cpp \ diff --git a/src/init.cpp b/src/init.cpp index 7911f07221..cc9ec2c794 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -2526,21 +2527,16 @@ bool AppInitMain() connOptions.vBinds.push_back(addrBind); } for (const std::string& strBind : gArgs.GetArgs("-whitebind")) { - CService addrBind; - if (!Lookup(strBind.c_str(), addrBind, 0, false)) { - return InitError(ResolveErrMsg("whitebind", strBind)); - } - if (addrBind.GetPort() == 0) { - return InitError(strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind)); - } - connOptions.vWhiteBinds.push_back(addrBind); + NetWhitebindPermissions whitebind; + std::string error; + if (!NetWhitebindPermissions::TryParse(strBind, whitebind, error)) return InitError(error); + connOptions.vWhiteBinds.push_back(whitebind); } for (const auto& net : gArgs.GetArgs("-whitelist")) { - CSubNet subnet; - LookupSubNet(net.c_str(), subnet); - if (!subnet.IsValid()) - return InitError(strprintf(_("Invalid netmask specified in -whitelist: '%s'"), net)); + NetWhitelistPermissions subnet; + std::string error; + if (!NetWhitelistPermissions::TryParse(net, subnet, error)) return InitError(error); connOptions.vWhitelistedRange.push_back(subnet); } diff --git a/src/masternode/masternode-sync.cpp b/src/masternode/masternode-sync.cpp index bb962b6461..a6f2ac6b96 100644 --- a/src/masternode/masternode-sync.cpp +++ b/src/masternode/masternode-sync.cpp @@ -181,7 +181,7 @@ void CMasternodeSync::ProcessTick(CConnman& connman) // NORMAL NETWORK MODE - TESTNET/MAINNET { - if ((pnode->fWhitelisted || pnode->m_manual_connection) && !netfulfilledman.HasFulfilledRequest(pnode->addr, strAllow)) { + if ((pnode->HasPermission(PF_NOBAN) || pnode->m_manual_connection) && !netfulfilledman.HasFulfilledRequest(pnode->addr, strAllow)) { netfulfilledman.RemoveAllFulfilledRequests(pnode->addr); netfulfilledman.AddFulfilledRequest(pnode->addr, strAllow); LogPrintf("CMasternodeSync::ProcessTick -- skipping mnsync restrictions for peer=%d\n", pnode->GetId()); diff --git a/src/net.cpp b/src/net.cpp index c9bd07fa59..a0278f8b94 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -98,7 +99,6 @@ enum BindFlags { BF_NONE = 0, BF_EXPLICIT = (1U << 0), BF_REPORT_ERROR = (1U << 1), - BF_WHITELIST = (1U << 2), }; #ifndef USE_WAKEUP_PIPE @@ -539,12 +539,10 @@ void CNode::CloseSocketDisconnect(CConnman* connman) statsClient.inc("peers.disconnect", 1.0f); } -bool CConnman::IsWhitelistedRange(const CNetAddr &addr) { - for (const CSubNet& subnet : vWhitelistedRange) { - if (subnet.Match(addr)) - return true; +void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const { + for (const auto& subnet : vWhitelistedRange) { + if (subnet.m_subnet.Match(addr)) NetPermissions::AddFlag(flags, subnet.m_flags); } - return false; } std::string CNode::GetAddrName() const { @@ -614,7 +612,8 @@ void CNode::copyStats(CNodeStats &stats, const std::vector &m_asmap) X(mapRecvBytesPerMsgCmd); X(nRecvBytes); } - X(fWhitelisted); + X(m_legacyWhitelisted); + X(m_permissionFlags); // It is common for nodes with good ping times to suddenly become lagged, // due to a new block arriving or other large transfer. @@ -907,7 +906,7 @@ bool CConnman::AttemptToEvictConnection() LOCK(cs_vNodes); for (const CNode* node : vNodes) { - if (node->fWhitelisted) + if (node->HasPermission(PF_NOBAN)) continue; if (!node->fInbound) continue; @@ -1018,7 +1017,20 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { } } - bool whitelisted = hListenSocket.whitelisted || IsWhitelistedRange(addr); + NetPermissionFlags permissionFlags = NetPermissionFlags::PF_NONE; + hListenSocket.AddSocketPermissionFlags(permissionFlags); + AddWhitelistPermissionFlags(permissionFlags, addr); + const bool noban = NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN); + bool legacyWhitelisted = false; + if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_ISIMPLICIT)) { + NetPermissions::ClearFlag(permissionFlags, PF_ISIMPLICIT); + if (gArgs.GetBoolArg("-whitelistforcerelay", false)) NetPermissions::AddFlag(permissionFlags, PF_FORCERELAY); + if (gArgs.GetBoolArg("-whitelistrelay", false)) NetPermissions::AddFlag(permissionFlags, PF_RELAY); + NetPermissions::AddFlag(permissionFlags, PF_MEMPOOL); + NetPermissions::AddFlag(permissionFlags, PF_NOBAN); + legacyWhitelisted = true; + } + { LOCK(cs_vNodes); for (const CNode* pnode : vNodes) { @@ -1068,7 +1080,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { // Don't accept connections from banned peers, but if our inbound slots aren't almost full, accept // if the only banning reason was an automatic misbehavior ban. - if (!whitelisted && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0)) + if (!noban && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0)) { LogPrint(BCLog::NET, "%s (banned)\n", strDropped); CloseSocket(hSocket); @@ -1102,9 +1114,15 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); CAddress addr_bind = GetBindAddress(hSocket); - CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true); + ServiceFlags nodeServices = nLocalServices; + if (NetPermissions::HasFlag(permissionFlags, PF_BLOOMFILTER)) { + nodeServices = static_cast(nodeServices | NODE_BLOOM); + } + CNode* pnode = new CNode(id, nodeServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true); pnode->AddRef(); - pnode->fWhitelisted = whitelisted; + pnode->m_permissionFlags = permissionFlags; + // If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility) + pnode->m_legacyWhitelisted = legacyWhitelisted; pnode->m_prefer_evict = bannedlevel > 0; m_msgproc->InitializeNode(pnode); @@ -2702,7 +2720,7 @@ void CConnman::ThreadMessageHandler() -bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, bool fWhitelisted) +bool CConnman::BindListenPort(const CService& addrBind, std::string& strError, NetPermissionFlags permissions) { strError = ""; int nOne = 1; @@ -2790,9 +2808,9 @@ bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, b } #endif - vhListenSocket.push_back(ListenSocket(hListenSocket, fWhitelisted)); + vhListenSocket.push_back(ListenSocket(hListenSocket, permissions)); - if (addrBind.IsRoutable() && fDiscover && !fWhitelisted) + if (addrBind.IsRoutable() && fDiscover && (permissions & PF_NOBAN) == 0) AddLocal(addrBind, LOCAL_BIND); return true; @@ -2889,11 +2907,11 @@ NodeId CConnman::GetNewNodeId() } -bool CConnman::Bind(const CService &addr, unsigned int flags) { +bool CConnman::Bind(const CService &addr, unsigned int flags, NetPermissionFlags permissions) { if (!(flags & BF_EXPLICIT) && !IsReachable(addr)) return false; std::string strError; - if (!BindListenPort(addr, strError, (flags & BF_WHITELIST) != 0)) { + if (!BindListenPort(addr, strError, permissions)) { if ((flags & BF_REPORT_ERROR) && clientInterface) { clientInterface->ThreadSafeMessageBox(strError, "", CClientUIInterface::MSG_ERROR); } @@ -2902,20 +2920,21 @@ bool CConnman::Bind(const CService &addr, unsigned int flags) { return true; } -bool CConnman::InitBinds(const std::vector& binds, const std::vector& whiteBinds) { +bool CConnman::InitBinds(const std::vector& binds, const std::vector& whiteBinds) +{ bool fBound = false; for (const auto& addrBind : binds) { - fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR)); + fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR), NetPermissionFlags::PF_NONE); } for (const auto& addrBind : whiteBinds) { - fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR | BF_WHITELIST)); + fBound |= Bind(addrBind.m_service, (BF_EXPLICIT | BF_REPORT_ERROR), addrBind.m_flags); } if (binds.empty() && whiteBinds.empty()) { struct in_addr inaddr_any; inaddr_any.s_addr = INADDR_ANY; struct in6_addr inaddr6_any = IN6ADDR_ANY_INIT; - fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE); - fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE); + fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE, NetPermissionFlags::PF_NONE); + fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE, NetPermissionFlags::PF_NONE); } return fBound; } @@ -3717,7 +3736,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn nVersion = 0; nNumWarningsSkipped = 0; nLastWarningTime = 0; - fWhitelisted = false; fOneShot = false; m_manual_connection = false; fClient = false; // set by version message diff --git a/src/net.h b/src/net.h index a64479c79d..078b84ec0b 100644 --- a/src/net.h +++ b/src/net.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -171,8 +172,9 @@ public: uint64_t nMaxOutboundLimit = 0; int64_t m_peer_connect_timeout = DEFAULT_PEER_CONNECT_TIMEOUT; std::vector vSeedNodes; - std::vector vWhitelistedRange; - std::vector vBinds, vWhiteBinds; + std::vector vWhitelistedRange; + std::vector vWhiteBinds; + std::vector vBinds; bool m_use_addrman_outgoing = true; std::vector m_specified_outgoing; std::vector m_added_nodes; @@ -472,15 +474,17 @@ public: private: struct ListenSocket { + public: SOCKET socket; - bool whitelisted; - - ListenSocket(SOCKET socket_, bool whitelisted_) : socket(socket_), whitelisted(whitelisted_) {} + inline void AddSocketPermissionFlags(NetPermissionFlags& flags) const { NetPermissions::AddFlag(flags, m_permissions); } + ListenSocket(SOCKET socket_, NetPermissionFlags permissions_) : socket(socket_), m_permissions(permissions_) {} + private: + NetPermissionFlags m_permissions; }; - bool BindListenPort(const CService &bindAddr, std::string& strError, bool fWhitelisted = false); - bool Bind(const CService &addr, unsigned int flags); - bool InitBinds(const std::vector& binds, const std::vector& whiteBinds); + bool BindListenPort(const CService& bindAddr, std::string& strError, NetPermissionFlags permissions); + bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions); + bool InitBinds(const std::vector& binds, const std::vector& whiteBinds); void ThreadOpenAddedConnections(); void AddOneShot(const std::string& strDest); void ProcessOneShot(); @@ -517,7 +521,7 @@ private: bool AttemptToEvictConnection(); CNode* ConnectNode(CAddress addrConnect, const char *pszDest = nullptr, bool fCountFailure = false, bool manual_connection = false); - bool IsWhitelistedRange(const CNetAddr &addr); + void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const; void DeleteNode(CNode* pnode); @@ -554,7 +558,7 @@ private: // Whitelisted ranges. Any node connecting from these is automatically // whitelisted (as well as those connecting to whitelisted binds). - std::vector vWhitelistedRange; + std::vector vWhitelistedRange; unsigned int nSendBufferMaxSize; unsigned int nReceiveFloodSize; @@ -650,7 +654,6 @@ void StartMapPort(); void InterruptMapPort(); void StopMapPort(); unsigned short GetListenPort(); -bool BindListenPort(const CService &bindAddr, std::string& strError, bool fWhitelisted = false); struct CombinerAll { @@ -755,7 +758,8 @@ public: mapMsgCmdSize mapSendBytesPerMsgCmd; uint64_t nRecvBytes; mapMsgCmdSize mapRecvBytesPerMsgCmd; - bool fWhitelisted; + NetPermissionFlags m_permissionFlags; + bool m_legacyWhitelisted; double dPingTime; double dPingWait; double dMinPing; @@ -867,7 +871,11 @@ public: std::string cleanSubVer GUARDED_BY(cs_SubVer){}; CCriticalSection cs_SubVer; // used for both cleanSubVer and strSubVer bool m_prefer_evict{false}; // This peer is preferred for eviction. - bool fWhitelisted; // This peer can bypass DoS banning. + bool HasPermission(NetPermissionFlags permission) const { + return NetPermissions::HasFlag(m_permissionFlags, permission); + } + // This boolean is unusued in actual processing, only present for backward compatibility at RPC/QT level + bool m_legacyWhitelisted{false}; bool fFeeler; // If true this node is being used as a short lived feeler. bool fOneShot; bool m_manual_connection; @@ -993,6 +1001,7 @@ private: const ServiceFlags nLocalServices; const int nMyStartingHeight; int nSendVersion; + NetPermissionFlags m_permissionFlags{ PF_NONE }; std::list vRecvMsg; // Used only by SocketHandler thread mutable CCriticalSection cs_addrName; diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp new file mode 100644 index 0000000000..07b3643067 --- /dev/null +++ b/src/net_permissions.cpp @@ -0,0 +1,105 @@ +// Copyright (c) 2009-2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include + +// The parse the following format "perm1,perm2@xxxxxx" +bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, size_t& readen, std::string& error) +{ + NetPermissionFlags flags = PF_NONE; + const auto atSeparator = str.find('@'); + + // if '@' is not found (ie, "xxxxx"), the caller should apply implicit permissions + if (atSeparator == std::string::npos) { + NetPermissions::AddFlag(flags, PF_ISIMPLICIT); + readen = 0; + } + // else (ie, "perm1,perm2@xxxxx"), let's enumerate the permissions by splitting by ',' and calculate the flags + else { + readen = 0; + // permissions == perm1,perm2 + const auto permissions = str.substr(0, atSeparator); + while (readen < permissions.length()) { + const auto commaSeparator = permissions.find(',', readen); + const auto len = commaSeparator == std::string::npos ? permissions.length() - readen : commaSeparator - readen; + // permission == perm1 + const auto permission = permissions.substr(readen, len); + readen += len; // We read "perm1" + if (commaSeparator != std::string::npos) readen++; // We read "," + + if (permission == "bloomfilter" || permission == "bloom") NetPermissions::AddFlag(flags, PF_BLOOMFILTER); + 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 == "all") NetPermissions::AddFlag(flags, PF_ALL); + else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY); + else if (permission.length() == 0); // Allow empty entries + else { + error = strprintf(_("Invalid P2P permission: '%s'"), permission); + return false; + } + } + readen++; + } + + output = flags; + error = ""; + return true; +} + +std::vector NetPermissions::ToStrings(NetPermissionFlags flags) +{ + std::vector strings; + if (NetPermissions::HasFlag(flags, PF_BLOOMFILTER)) strings.push_back("bloomfilter"); + if (NetPermissions::HasFlag(flags, PF_NOBAN)) strings.push_back("noban"); + 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"); + return strings; +} + +bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error) +{ + NetPermissionFlags flags; + size_t offset; + if (!TryParsePermissionFlags(str, flags, offset, error)) return false; + + const std::string strBind = str.substr(offset); + CService addrBind; + if (!Lookup(strBind.c_str(), addrBind, 0, false)) { + error = strprintf(_("Cannot resolve -%s address: '%s'"), "whitebind", strBind); + return false; + } + if (addrBind.GetPort() == 0) { + error = strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind); + return false; + } + + output.m_flags = flags; + output.m_service = addrBind; + error = ""; + return true; +} + +bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error) +{ + NetPermissionFlags flags; + size_t offset; + if (!TryParsePermissionFlags(str, flags, offset, error)) return false; + + const std::string net = str.substr(offset); + CSubNet subnet; + LookupSubNet(net.c_str(), subnet); + if (!subnet.IsValid()) { + error = strprintf(_("Invalid netmask specified in -whitelist: '%s'"), net); + return false; + } + + output.m_flags = flags; + output.m_subnet = subnet; + error = ""; + return true; +} diff --git a/src/net_permissions.h b/src/net_permissions.h new file mode 100644 index 0000000000..b3987de65f --- /dev/null +++ b/src/net_permissions.h @@ -0,0 +1,62 @@ +// Copyright (c) 2009-2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include + +#ifndef BITCOIN_NET_PERMISSIONS_H +#define BITCOIN_NET_PERMISSIONS_H +enum NetPermissionFlags +{ + PF_NONE = 0, + // Can query bloomfilter even if -peerbloomfilters is false + PF_BLOOMFILTER = (1U << 1), + // Relay and accept transactions from this peer, even if -blocksonly is true + PF_RELAY = (1U << 3), + // Always relay transactions from this peer, even if already in mempool or rejected from policy + // Keep parameter interaction: forcerelay implies relay + PF_FORCERELAY = (1U << 2) | PF_RELAY, + // Can't be banned for misbehavior + PF_NOBAN = (1U << 4), + // 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, +}; +class NetPermissions +{ +public: + NetPermissionFlags m_flags; + static std::vector ToStrings(NetPermissionFlags flags); + static inline bool HasFlag(const NetPermissionFlags& flags, NetPermissionFlags f) + { + return (flags & f) == f; + } + static inline void AddFlag(NetPermissionFlags& flags, NetPermissionFlags f) + { + flags = static_cast(flags | f); + } + static inline void ClearFlag(NetPermissionFlags& flags, NetPermissionFlags f) + { + flags = static_cast(flags & ~f); + } +}; +class NetWhitebindPermissions : public NetPermissions +{ +public: + static bool TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error); + CService m_service; +}; + +class NetWhitelistPermissions : public NetPermissions +{ +public: + static bool TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error); + CSubNet m_subnet; +}; + +#endif // BITCOIN_NET_PERMISSIONS_H \ No newline at end of file diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f19b35a921..dbb09f4aa7 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -418,7 +418,7 @@ static void UpdatePreferredDownload(CNode* node, CNodeState* state) EXCLUSIVE_LO nPreferredDownload -= state->fPreferredDownload; // Whether this node should be marked as a preferred download node. - state->fPreferredDownload = (!node->fInbound || node->fWhitelisted) && !node->fOneShot && !node->fClient; + state->fPreferredDownload = (!node->fInbound || node->HasPermission(PF_NOBAN)) && !node->fOneShot && !node->fClient; nPreferredDownload += state->fPreferredDownload; } @@ -1494,7 +1494,7 @@ 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->fWhitelisted) + if (send && connman->OutboundTargetReached(true) && ( ((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->HasPermission(PF_NOBAN)) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom->GetId()); @@ -1503,7 +1503,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c send = false; } // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold - if (send && !pfrom->fWhitelisted && ( + if (send && !pfrom->HasPermission(PF_NOBAN) && ( (((pfrom->GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom->GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (chainActive.Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) )) { LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold from peer=%d\n", pfrom->GetId()); @@ -2548,7 +2548,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr bool fBlocksOnly = !fRelayTxes; // Allow whitelisted peers to send data other than blocks in blocks only mode if whitelistrelay is true - if (pfrom->fWhitelisted && gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) + if (pfrom->HasPermission(PF_RELAY)) fBlocksOnly = false; LOCK(cs_main); @@ -2771,7 +2771,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } LOCK(cs_main); - if (IsInitialBlockDownload() && !pfrom->fWhitelisted) { + if (IsInitialBlockDownload() && !pfrom->HasPermission(PF_NOBAN)) { LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because node is in initial block download\n", pfrom->GetId()); return true; } @@ -2829,7 +2829,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (strCommand == NetMsgType::TX || strCommand == NetMsgType::DSTX || strCommand == NetMsgType::LEGACYTXLOCKREQUEST) { // Stop processing the transaction early if // We are in blocks only mode and peer is either not whitelisted or whitelistrelay is off - if (!fRelayTxes && (!pfrom->fWhitelisted || !gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY))) + if (!fRelayTxes && !pfrom->HasPermission(PF_RELAY)) { LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom->GetId()); return true; @@ -2988,7 +2988,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } } - if (pfrom->fWhitelisted && gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) { + if (pfrom->HasPermission(PF_FORCERELAY)) { // Always relay transactions received from whitelisted peers, even // if they were already in the mempool or rejected from it due // to policy, allowing the node to function as a gateway for @@ -3421,17 +3421,23 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } if (strCommand == NetMsgType::MEMPOOL) { - if (!(pfrom->GetLocalServices() & NODE_BLOOM) && !pfrom->fWhitelisted) + if (!(pfrom->GetLocalServices() & NODE_BLOOM) && !pfrom->HasPermission(PF_MEMPOOL)) { - LogPrint(BCLog::NET, "mempool request with bloom filters disabled, disconnect peer=%d\n", pfrom->GetId()); - pfrom->fDisconnect = true; + if (!pfrom->HasPermission(PF_NOBAN)) + { + LogPrint(BCLog::NET, "mempool request with bloom filters disabled, disconnect peer=%d\n", pfrom->GetId()); + pfrom->fDisconnect = true; + } return true; } - if (connman->OutboundTargetReached(false) && !pfrom->fWhitelisted) + if (connman->OutboundTargetReached(false) && !pfrom->HasPermission(PF_MEMPOOL)) { - LogPrint(BCLog::NET, "mempool request with bandwidth limit reached, disconnect peer=%d\n", pfrom->GetId()); - pfrom->fDisconnect = true; + if (!pfrom->HasPermission(PF_NOBAN)) + { + LogPrint(BCLog::NET, "mempool request with bandwidth limit reached, disconnect peer=%d\n", pfrom->GetId()); + pfrom->fDisconnect = true; + } return true; } @@ -3672,7 +3678,7 @@ bool PeerLogicValidation::SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_ if (state.fShouldBan) { state.fShouldBan = false; - if (pnode->fWhitelisted) + if (pnode->HasPermission(PF_NOBAN)) LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode->GetLogString()); else if (pnode->m_manual_connection) LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode->GetLogString()); @@ -4268,7 +4274,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // Check whether periodic sends should happen // Note: If this node is running in a Masternode mode, it makes no sense to delay outgoing txes // because we never produce any txes ourselves i.e. no privacy is lost in this case. - bool fSendTrickle = pto->fWhitelisted || fMasternodeMode; + bool fSendTrickle = pto->HasPermission(PF_NOBAN) || fMasternodeMode; if (pto->nNextInvSend < current_time) { fSendTrickle = true; if (pto->fInbound) { @@ -4432,7 +4438,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // Note: If all our peers are inbound, then we won't // disconnect our sync peer for stalling; we have bigger // problems if we can't get any outbound peers. - if (!pto->fWhitelisted) { + if (!pto->HasPermission(PF_NOBAN)) { LogPrintf("Timeout downloading headers from peer=%d, disconnecting\n", pto->GetId()); pto->fDisconnect = true; return true; diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 88e75d2554..2cb44d2631 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1246,7 +1246,7 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats) ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer)); ui->peerDirection->setText(stats->nodeStats.fInbound ? tr("Inbound") : tr("Outbound")); ui->peerHeight->setText(QString("%1").arg(QString::number(stats->nodeStats.nStartingHeight))); - ui->peerWhitelisted->setText(stats->nodeStats.fWhitelisted ? tr("Yes") : tr("No")); + ui->peerWhitelisted->setText(stats->nodeStats.m_legacyWhitelisted ? tr("Yes") : tr("No")); auto dmn = clientModel->getMasternodeList().GetMNByService(stats->nodeStats.addr); if (dmn == nullptr) { ui->peerNodeType->setText(tr("Regular")); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index dc6255e215..f1aea21ba4 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -188,7 +189,12 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) } obj.pushKV("inflight", heights); } - obj.pushKV("whitelisted", stats.fWhitelisted); + obj.pushKV("whitelisted", stats.m_legacyWhitelisted); + UniValue permissions(UniValue::VARR); + for (const auto& permission : NetPermissions::ToStrings(stats.m_permissionFlags)) { + permissions.push_back(permission); + } + obj.pushKV("permissions", permissions); UniValue sendPerMsgCmd(UniValue::VOBJ); for (const mapMsgCmdSize::value_type &i : stats.mapSendBytesPerMsgCmd) { diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 92d5e2c76c..707a1948f4 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -4,6 +4,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -449,4 +450,82 @@ BOOST_AUTO_TEST_CASE(netbase_parsenetwork) BOOST_CHECK_EQUAL(ParseNetwork(""), NET_UNROUTABLE); } +BOOST_AUTO_TEST_CASE(netpermissions_test) +{ + std::string error; + NetWhitebindPermissions whitebindPermissions; + NetWhitelistPermissions whitelistPermissions; + + // Detect invalid white bind + BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error)); + BOOST_CHECK(error.find("Cannot resolve -whitebind address") != std::string::npos); + BOOST_CHECK(!NetWhitebindPermissions::TryParse("127.0.0.1", whitebindPermissions, error)); + BOOST_CHECK(error.find("Need to specify a port with -whitebind") != std::string::npos); + BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error)); + + // If no permission flags, assume backward compatibility + BOOST_CHECK(NetWhitebindPermissions::TryParse("1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK(error.empty()); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_ISIMPLICIT); + BOOST_CHECK(NetPermissions::HasFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT)); + NetPermissions::ClearFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT); + BOOST_CHECK(!NetPermissions::HasFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE); + NetPermissions::AddFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT); + BOOST_CHECK(NetPermissions::HasFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT)); + + // Can set one permission + BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER); + BOOST_CHECK(NetWhitebindPermissions::TryParse("@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE); + + // Happy path, can parse flags + BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,forcerelay@1.2.3.4:32", whitebindPermissions, error)); + // forcerelay should also activate the relay permission + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY); + BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,relay,noban@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_RELAY | PF_NOBAN); + BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,forcerelay,noban@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK(NetWhitebindPermissions::TryParse("all@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_ALL); + + // Allow dups + BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,relay,noban,noban@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_RELAY | PF_NOBAN); + + // Allow empty + BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,relay,,noban@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_RELAY | PF_NOBAN); + BOOST_CHECK(NetWhitebindPermissions::TryParse(",@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE); + BOOST_CHECK(NetWhitebindPermissions::TryParse(",,@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE); + + // Detect invalid flag + BOOST_CHECK(!NetWhitebindPermissions::TryParse("bloom,forcerelay,oopsie@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK(error.find("Invalid P2P permission") != std::string::npos); + + // Check whitelist error + BOOST_CHECK(!NetWhitelistPermissions::TryParse("bloom,forcerelay,noban@1.2.3.4:32", whitelistPermissions, error)); + BOOST_CHECK(error.find("Invalid netmask specified in -whitelist") != std::string::npos); + + // Happy path for whitelist parsing + BOOST_CHECK(NetWhitelistPermissions::TryParse("noban@1.2.3.4", whitelistPermissions, error)); + BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, PF_NOBAN); + BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay@1.2.3.4/32", whitelistPermissions, error)); + BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, PF_BLOOMFILTER | PF_FORCERELAY | PF_NOBAN | PF_RELAY); + BOOST_CHECK(error.empty()); + BOOST_CHECK_EQUAL(whitelistPermissions.m_subnet.ToString(), "1.2.3.4/32"); + 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(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_AUTO_TEST_SUITE_END() diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py new file mode 100644 index 0000000000..1013055420 --- /dev/null +++ b/test/functional/p2p_permissions.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +# Copyright (c) 2015-2018 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 p2p permission message. + +Test that permissions are correctly calculated and applied +""" + +from test_framework.test_node import ErrorMatch +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + connect_nodes, + p2p_port, +) + +class P2PPermissionsTests(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.setup_clean_chain = True + self.extra_args = [[],[]] + + def run_test(self): + self.checkpermission( + # relay permission added + ["-whitelist=127.0.0.1", "-whitelistrelay"], + ["relay", "noban", "mempool"], + True) + + self.checkpermission( + # forcerelay and relay permission added + # Legacy parameter interaction which set whitelistrelay to true + # if whitelistforcerelay is true + ["-whitelist=127.0.0.1", "-whitelistforcerelay"], + ["forcerelay", "relay", "noban", "mempool"], + True) + + # Let's make sure permissions are merged correctly + # For this, we need to use whitebind instead of bind + # by modifying the configuration file. + ip_port = "127.0.0.1:{}".format(p2p_port(1)) + self.replaceinconfig(1, "bind=127.0.0.1", "whitebind=bloomfilter,forcerelay@" + ip_port) + self.checkpermission( + ["-whitelist=noban@127.0.0.1" ], + # Check parameter interaction forcerelay should activate relay + ["noban", "bloomfilter", "forcerelay", "relay" ], + 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"], + False) + + self.checkpermission( + # legacy whitelistforcerelay should be ignored + ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"], + ["noban", "mempool"], + False) + + self.checkpermission( + # missing mempool permission to be considered legacy whitelisted + ["-whitelist=noban@127.0.0.1"], + ["noban"], + False) + + self.checkpermission( + # all permission added + ["-whitelist=all@127.0.0.1"], + ["forcerelay", "noban", "mempool", "bloomfilter", "relay"], + False) + + self.stop_node(1) + self.nodes[1].assert_start_raises_init_error(["-whitelist=oopsie@127.0.0.1"], "Invalid P2P permission", match=ErrorMatch.PARTIAL_REGEX) + self.nodes[1].assert_start_raises_init_error(["-whitelist=noban@127.0.0.1:230"], "Invalid netmask specified in", match=ErrorMatch.PARTIAL_REGEX) + self.nodes[1].assert_start_raises_init_error(["-whitebind=noban@127.0.0.1/10"], "Cannot resolve -whitebind address", match=ErrorMatch.PARTIAL_REGEX) + + def checkpermission(self, args, expectedPermissions, whitelisted): + self.restart_node(1, args) + connect_nodes(self.nodes[0], 1) + peerinfo = self.nodes[1].getpeerinfo()[0] + assert_equal(peerinfo['whitelisted'], whitelisted) + assert_equal(len(expectedPermissions), len(peerinfo['permissions'])) + for p in expectedPermissions: + if not p in peerinfo['permissions']: + raise AssertionError("Expected permissions %r is not granted." % p) + + def replaceinconfig(self, nodeid, old, new): + with open(self.nodes[nodeid].bitcoinconf, encoding="utf8") as f: + newText=f.read().replace(old, new) + with open(self.nodes[nodeid].bitcoinconf, 'w', encoding="utf8") as f: + f.write(newText) + +if __name__ == '__main__': + P2PPermissionsTests().main() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index eea551e3ea..19c1272384 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -64,6 +64,7 @@ class TestNode(): self.index = i self.datadir = datadir self.chain = chain + self.bitcoinconf = os.path.join(self.datadir, "dash.conf") self.stdout_dir = os.path.join(self.datadir, "stdout") self.stderr_dir = os.path.join(self.datadir, "stderr") self.rpchost = rpchost diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 1f42f7b0ce..77a557cc1c 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -194,6 +194,7 @@ BASE_SCRIPTS = [ 'feature_includeconf.py', 'feature_logging.py', 'p2p_node_network_limited.py', + 'p2p_permissions.py', 'feature_blocksdir.py', 'feature_config_args.py', 'rpc_help.py', From ce8b04fc385a93a65fc3110e872025617ee99eee Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 16 Aug 2019 10:17:20 -0400 Subject: [PATCH 5/9] Merge #16618: [Fix] Allow connection of a noban banned peer d117f4541d4717e83c9396273e92960723622030 Add test for setban (nicolas.dorier) dc7529abf0197dccb876dc4a93cbdd2ad9f03e5c [Fix] Allow connection of a noban banned peer (nicolas.dorier) Pull request description: Reported by @MarcoFalke on https://github.com/bitcoin/bitcoin/pull/16248#discussion_r314026195 The bug would mean that if the peer connecting to you is banned, but whitelisted without specific permissions, it would not be able to connect to the node. The solution is just to move the same line below. ACKs for top commit: Sjors: Agree inline is more clear. utACK d117f45 MarcoFalke: ACK d117f4541d4717e83c9396273e92960723622030 Tree-SHA512: 0fed39acb1e8db67bb0bf4c4de3ad034ae776f38d55bd661f1ae0e1a4c6becaf1824ab46ed8279f2f31df3f4b29ff56461d8b167d3e9cece62cfe58b5a912811 --- src/net.cpp | 3 +- test/functional/rpc_setban.py | 47 ++++++++++++++++++++++++ test/functional/test_runner.py | 1 + test/lint/lint-spelling.ignore-words.txt | 1 + 4 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 test/functional/rpc_setban.py diff --git a/src/net.cpp b/src/net.cpp index a0278f8b94..0c93ff5838 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1020,7 +1020,6 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { NetPermissionFlags permissionFlags = NetPermissionFlags::PF_NONE; hListenSocket.AddSocketPermissionFlags(permissionFlags); AddWhitelistPermissionFlags(permissionFlags, addr); - const bool noban = NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN); bool legacyWhitelisted = false; if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_ISIMPLICIT)) { NetPermissions::ClearFlag(permissionFlags, PF_ISIMPLICIT); @@ -1080,7 +1079,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { // Don't accept connections from banned peers, but if our inbound slots aren't almost full, accept // if the only banning reason was an automatic misbehavior ban. - if (!noban && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0)) + if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0)) { LogPrint(BCLog::NET, "%s (banned)\n", strDropped); CloseSocket(hSocket); diff --git a/test/functional/rpc_setban.py b/test/functional/rpc_setban.py new file mode 100644 index 0000000000..a1a8196557 --- /dev/null +++ b/test/functional/rpc_setban.py @@ -0,0 +1,47 @@ +#!/usr/bin/env python3 +# Copyright (c) 2015-2019 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 the setban rpc call.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + connect_nodes, + p2p_port +) + +class SetBanTests(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.setup_clean_chain = True + self.extra_args = [[],[]] + + def run_test(self): + # Node 0 connects to Node 1, check that the noban permission is not granted + connect_nodes(self.nodes[0], 1) + peerinfo = self.nodes[1].getpeerinfo()[0] + assert(not 'noban' in peerinfo['permissions']) + + # Node 0 get banned by Node 1 + self.nodes[1].setban("127.0.0.1", "add") + + # Node 0 should not be able to reconnect + with self.nodes[1].assert_debug_log(expected_msgs=['dropped (banned)\n']): + self.restart_node(1, []) + self.nodes[0].addnode("127.0.0.1:" + str(p2p_port(1)), "onetry") + + # However, node 0 should be able to reconnect if it has noban permission + self.restart_node(1, ['-whitelist=127.0.0.1']) + connect_nodes(self.nodes[0], 1) + peerinfo = self.nodes[1].getpeerinfo()[0] + assert('noban' in peerinfo['permissions']) + + # If we remove the ban, Node 0 should be able to reconnect even without noban permission + self.nodes[1].setban("127.0.0.1", "remove") + self.restart_node(1, []) + connect_nodes(self.nodes[0], 1) + peerinfo = self.nodes[1].getpeerinfo()[0] + assert(not 'noban' in peerinfo['permissions']) + +if __name__ == '__main__': + SetBanTests().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 77a557cc1c..32430972ca 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -137,6 +137,7 @@ BASE_SCRIPTS = [ 'wallet_keypool.py', 'wallet_keypool_hd.py', 'p2p_mempool.py', + 'rpc_setban.py', 'mining_prioritisetransaction.py', 'p2p_invalid_locator.py', 'p2p_invalid_block.py', diff --git a/test/lint/lint-spelling.ignore-words.txt b/test/lint/lint-spelling.ignore-words.txt index 0d58094197..363ca4d49d 100644 --- a/test/lint/lint-spelling.ignore-words.txt +++ b/test/lint/lint-spelling.ignore-words.txt @@ -6,3 +6,4 @@ objext unparseable unselect useable +setban From 92bfe50955d6f24dd3a88bc2a78a82deec294b98 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 17 Jul 2021 23:20:16 -0500 Subject: [PATCH 6/9] partial Merge #16152: (Don't actually) Disable bloom filtering by default. This does not disable bloom filters, but does implement a minor refactoring Signed-off-by: pasta --- src/net_processing.h | 1 + src/validation.h | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/net_processing.h b/src/net_processing.h index 7ded9c1a7d..e628572fce 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -20,6 +20,7 @@ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE = 10; // this all static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100; /** Default for BIP61 (sending reject messages) */ static constexpr bool DEFAULT_ENABLE_BIP61 = true; +static const bool DEFAULT_PEERBLOOMFILTERS = true; class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface { private: diff --git a/src/validation.h b/src/validation.h index d5fd585a22..ec606cd915 100644 --- a/src/validation.h +++ b/src/validation.h @@ -135,8 +135,6 @@ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8; /** Maximum number of unconnecting headers announcements before DoS score */ static const int MAX_UNCONNECTING_HEADERS = 10; -static const bool DEFAULT_PEERBLOOMFILTERS = true; - /** Default for -stopatheight */ static const int DEFAULT_STOPATHEIGHT = 0; From 909a13082f03ddff1247460392f29cf2c0dd05c0 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 19 Aug 2019 11:18:47 +0200 Subject: [PATCH 7/9] Merge #16631: net: The default whitelistrelay should be true 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb Reformat p2p_permissions.py (nicolas.dorier) ce7eac3cb0e7d301db75de24e9a7b0af93c61311 [Fix] The default whitelistrelay should be true (nicolas.dorier) Pull request description: I thought `whitelistrelay` default was `false` when it is `true`. The root of the issue come from the fact that all references to `DEFAULT_` are not in the scope of this file, so hard coding of default values are used everywhere in `net.cpp`. I think that in a separate PR we should fix that more fundamentally everywhere. ACKs for top commit: promag: ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb. Sjors: re-ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb Tree-SHA512: f4a75f986fa2adf1a5f1c91605e0d261f7ac5ac8535fb05437d83b8392dbcf5cc1a47d755adcf8ad8dc67a88de28060187200fd3ce06545261a5c7ec0fea831a --- src/net.cpp | 4 +- src/net.h | 5 +++ src/validation.h | 4 -- test/functional/p2p_permissions.py | 66 ++++++++++++++++-------------- test/functional/rpc_setban.py | 0 5 files changed, 43 insertions(+), 36 deletions(-) mode change 100644 => 100755 test/functional/p2p_permissions.py mode change 100644 => 100755 test/functional/rpc_setban.py diff --git a/src/net.cpp b/src/net.cpp index 0c93ff5838..1039bfeb73 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1023,8 +1023,8 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { bool legacyWhitelisted = false; if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_ISIMPLICIT)) { NetPermissions::ClearFlag(permissionFlags, PF_ISIMPLICIT); - if (gArgs.GetBoolArg("-whitelistforcerelay", false)) NetPermissions::AddFlag(permissionFlags, PF_FORCERELAY); - if (gArgs.GetBoolArg("-whitelistrelay", false)) NetPermissions::AddFlag(permissionFlags, PF_RELAY); + if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(permissionFlags, PF_FORCERELAY); + if (gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) NetPermissions::AddFlag(permissionFlags, PF_RELAY); NetPermissions::AddFlag(permissionFlags, PF_MEMPOOL); NetPermissions::AddFlag(permissionFlags, PF_NOBAN); legacyWhitelisted = true; diff --git a/src/net.h b/src/net.h index 078b84ec0b..00be618fa0 100644 --- a/src/net.h +++ b/src/net.h @@ -49,6 +49,11 @@ class CScheduler; class CNode; class BanMan; +/** Default for -whitelistrelay. */ +static const bool DEFAULT_WHITELISTRELAY = true; +/** Default for -whitelistforcerelay. */ +static const bool DEFAULT_WHITELISTFORCERELAY = true; + /** Time between pings automatically sent out for latency probing and keepalive (in seconds). */ static const int PING_INTERVAL = 2 * 60; /** Time after which to disconnect, after waiting for a ping response (or inactivity). */ diff --git a/src/validation.h b/src/validation.h index ec606cd915..7cf36c8993 100644 --- a/src/validation.h +++ b/src/validation.h @@ -48,10 +48,6 @@ struct ChainTxData; struct LockPoints; -/** Default for -whitelistrelay. */ -static const bool DEFAULT_WHITELISTRELAY = true; -/** Default for -whitelistforcerelay. */ -static const bool DEFAULT_WHITELISTFORCERELAY = true; /** Default for -minrelaytxfee, minimum relay fee for transactions */ static const unsigned int DEFAULT_MIN_RELAY_TX_FEE = 1000; //! -maxtxfee default diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py old mode 100644 new mode 100755 index 1013055420..40b28d7533 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -23,18 +23,24 @@ class P2PPermissionsTests(BitcoinTestFramework): def run_test(self): self.checkpermission( - # relay permission added - ["-whitelist=127.0.0.1", "-whitelistrelay"], - ["relay", "noban", "mempool"], - True) + # default permissions (no specific permissions) + ["-whitelist=127.0.0.1"], + ["relay", "noban", "mempool"], + True) self.checkpermission( - # forcerelay and relay permission added - # Legacy parameter interaction which set whitelistrelay to true - # if whitelistforcerelay is true - ["-whitelist=127.0.0.1", "-whitelistforcerelay"], - ["forcerelay", "relay", "noban", "mempool"], - True) + # relay permission removed (no specific permissions) + ["-whitelist=127.0.0.1", "-whitelistrelay=0"], + ["noban", "mempool"], + True) + + self.checkpermission( + # forcerelay and relay permission added + # Legacy parameter interaction which set whitelistrelay to true + # if whitelistforcerelay is true + ["-whitelist=127.0.0.1", "-whitelistforcerelay"], + ["forcerelay", "relay", "noban", "mempool"], + True) # Let's make sure permissions are merged correctly # For this, we need to use whitebind instead of bind @@ -42,35 +48,35 @@ class P2PPermissionsTests(BitcoinTestFramework): ip_port = "127.0.0.1:{}".format(p2p_port(1)) self.replaceinconfig(1, "bind=127.0.0.1", "whitebind=bloomfilter,forcerelay@" + ip_port) self.checkpermission( - ["-whitelist=noban@127.0.0.1" ], - # Check parameter interaction forcerelay should activate relay - ["noban", "bloomfilter", "forcerelay", "relay" ], - False) + ["-whitelist=noban@127.0.0.1" ], + # Check parameter interaction forcerelay should activate relay + ["noban", "bloomfilter", "forcerelay", "relay" ], + 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"], - False) + # legacy whitelistrelay should be ignored + ["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"], + ["noban", "mempool"], + False) self.checkpermission( - # legacy whitelistforcerelay should be ignored - ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"], - ["noban", "mempool"], - False) + # legacy whitelistforcerelay should be ignored + ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"], + ["noban", "mempool"], + False) self.checkpermission( - # missing mempool permission to be considered legacy whitelisted - ["-whitelist=noban@127.0.0.1"], - ["noban"], - False) + # missing mempool permission to be considered legacy whitelisted + ["-whitelist=noban@127.0.0.1"], + ["noban"], + False) self.checkpermission( - # all permission added - ["-whitelist=all@127.0.0.1"], - ["forcerelay", "noban", "mempool", "bloomfilter", "relay"], - False) + # all permission added + ["-whitelist=all@127.0.0.1"], + ["forcerelay", "noban", "mempool", "bloomfilter", "relay"], + False) self.stop_node(1) self.nodes[1].assert_start_raises_init_error(["-whitelist=oopsie@127.0.0.1"], "Invalid P2P permission", match=ErrorMatch.PARTIAL_REGEX) diff --git a/test/functional/rpc_setban.py b/test/functional/rpc_setban.py old mode 100644 new mode 100755 From 9009f57e272ebd418d0e60094802fafbaf80bfaf Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 13 Dec 2018 13:43:12 +0100 Subject: [PATCH 8/9] Merge #14624: Some simple improvements to the RNG code e414486d56b9f06af7aeb07ce13e3c3780c2b69b Do not permit copying FastRandomContexts (Pieter Wuille) 022cf47dd7ef8f46e32a184e84f94d1e9f3a495c Simplify testing RNG code (Pieter Wuille) fd3e7973ffaaa15ed32e5aeadcb02956849b8fc7 Make unit tests use the insecure_rand_ctx exclusively (Pieter Wuille) 8d98d426116f0178612f14d1874d331042c4c4b7 Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) 273d02580aa736b7ccea8fce51d90541665fdbd1 Use a FastRandomContext in LimitOrphanTxSize (Pieter Wuille) 3db746beb407f7cdd9cd6a605a195bef1254b4c0 Introduce a Shuffle for FastRandomContext and use it in wallet and coinselection (Pieter Wuille) 8098379be5465f598220e1d6174fc57c56f9da42 Use a local FastRandomContext in a few more places in net (Pieter Wuille) 9695f31d7544778853aa373f0aeed629fa68d85e Make addrman use its local RNG exclusively (Pieter Wuille) Pull request description: This improves a few minor issues with the RNG code: * Avoid calling `GetRand*()` functions (which currently invoke OpenSSL, later may switch to using our own RNG pool) inside loops in addrman, networking code, `KnapsackSolver`, and `LimitOrphanSize` * Fix a currently unreachable bug in `FastRandomContext::randbytes`. * Make a number of simplifications to the unit tests' randomness code (some tests unnecessarily used their own RNG or the OpenSSL one, instead of using the unit test specific `insecure_rand_ctx`). * As a precaution, make it illegal to copy a `FastRandomContext`. Tree-SHA512: 084c70b533ea68ca7adc0186c39f0b3e0a5c0ae43a12c37286e5d42086e056a8cd026dde61b12c0a296dc80f87fdc87fe303b9e8e6161b460ac2086cf7615f9d --- src/addrman.cpp | 26 ++++++++----------- src/addrman.h | 5 +--- src/net.cpp | 9 ++++--- src/net_processing.cpp | 1 + src/random.cpp | 15 +++++++++++ src/random.h | 8 ++++++ src/test/addrman_tests.cpp | 6 ----- src/test/cuckoocache_tests.cpp | 39 ++++++++--------------------- src/test/denialofservice_tests.cpp | 2 +- src/test/prevector_tests.cpp | 4 +-- src/test/random_tests.cpp | 18 +++++++++---- src/test/test_dash.cpp | 3 +-- src/test/test_dash.h | 10 ++------ src/test/validation_block_tests.cpp | 6 ++--- 14 files changed, 74 insertions(+), 78 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index e227049106..cfb36cdd7a 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -239,7 +239,7 @@ void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime return; // find a bucket it is in now - int nRnd = RandomInt(ADDRMAN_NEW_BUCKET_COUNT); + int nRnd = insecure_rand.randrange(ADDRMAN_NEW_BUCKET_COUNT); int nUBucket = -1; for (unsigned int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) { int nB = (n + nRnd) % ADDRMAN_NEW_BUCKET_COUNT; @@ -319,7 +319,7 @@ bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimeP int nFactor = 1; for (int n = 0; n < pinfo->nRefCount; n++) nFactor *= 2; - if (nFactor > 1 && (RandomInt(nFactor) != 0)) + if (nFactor > 1 && (insecure_rand.randrange(nFactor) != 0)) return false; } else { pinfo = Create(addr, source, &nId); @@ -384,12 +384,12 @@ CAddrInfo CAddrMan::Select_(bool newOnly) // Use a 50% chance for choosing between tried and new table entries. if (!newOnly && - (nTried > 0 && (nNew == 0 || RandomInt(2) == 0))) { + (nTried > 0 && (nNew == 0 || insecure_rand.randbool() == 0))) { // use a tried node double fChanceFactor = 1.0; while (1) { - int nKBucket = RandomInt(ADDRMAN_TRIED_BUCKET_COUNT); - int nKBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE); + int nKBucket = insecure_rand.randrange(ADDRMAN_TRIED_BUCKET_COUNT); + int nKBucketPos = insecure_rand.randrange(ADDRMAN_BUCKET_SIZE); while (vvTried[nKBucket][nKBucketPos] == -1) { nKBucket = (nKBucket + insecure_rand.randbits(ADDRMAN_TRIED_BUCKET_COUNT_LOG2)) % ADDRMAN_TRIED_BUCKET_COUNT; nKBucketPos = (nKBucketPos + insecure_rand.randbits(ADDRMAN_BUCKET_SIZE_LOG2)) % ADDRMAN_BUCKET_SIZE; @@ -397,7 +397,7 @@ CAddrInfo CAddrMan::Select_(bool newOnly) int nId = vvTried[nKBucket][nKBucketPos]; assert(mapInfo.count(nId) == 1); CAddrInfo& info = mapInfo[nId]; - if (RandomInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30)) + if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) return info; fChanceFactor *= 1.2; } @@ -405,8 +405,8 @@ CAddrInfo CAddrMan::Select_(bool newOnly) // use a new node double fChanceFactor = 1.0; while (1) { - int nUBucket = RandomInt(ADDRMAN_NEW_BUCKET_COUNT); - int nUBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE); + int nUBucket = insecure_rand.randrange(ADDRMAN_NEW_BUCKET_COUNT); + int nUBucketPos = insecure_rand.randrange(ADDRMAN_BUCKET_SIZE); while (vvNew[nUBucket][nUBucketPos] == -1) { nUBucket = (nUBucket + insecure_rand.randbits(ADDRMAN_NEW_BUCKET_COUNT_LOG2)) % ADDRMAN_NEW_BUCKET_COUNT; nUBucketPos = (nUBucketPos + insecure_rand.randbits(ADDRMAN_BUCKET_SIZE_LOG2)) % ADDRMAN_BUCKET_SIZE; @@ -414,7 +414,7 @@ CAddrInfo CAddrMan::Select_(bool newOnly) int nId = vvNew[nUBucket][nUBucketPos]; assert(mapInfo.count(nId) == 1); CAddrInfo& info = mapInfo[nId]; - if (RandomInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30)) + if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) return info; fChanceFactor *= 1.2; } @@ -510,7 +510,7 @@ void CAddrMan::GetAddr_(std::vector& vAddr) if (vAddr.size() >= nNodes) break; - int nRndPos = RandomInt(vRandom.size() - n) + n; + int nRndPos = insecure_rand.randrange(vRandom.size() - n) + n; SwapRandom(n, nRndPos); assert(mapInfo.count(vRandom[n]) == 1); @@ -575,10 +575,6 @@ CAddrInfo CAddrMan::GetAddressInfo_(const CService& addr) return *pinfo; } -int CAddrMan::RandomInt(int nMax){ - return GetRandInt(nMax); -} - void CAddrMan::ResolveCollisions_() { for (std::set::iterator it = m_tried_collisions.begin(); it != m_tried_collisions.end();) { @@ -645,7 +641,7 @@ CAddrInfo CAddrMan::SelectTriedCollision_() std::set::iterator it = m_tried_collisions.begin(); // Selects a random element from m_tried_collisions - std::advance(it, GetRandInt(m_tried_collisions.size())); + std::advance(it, insecure_rand.randrange(m_tried_collisions.size())); int id_new = *it; // If id_new not found in mapInfo remove it from m_tried_collisions diff --git a/src/addrman.h b/src/addrman.h index b15743d961..f084bfc72a 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -276,9 +276,6 @@ protected: //! Return a random to-be-evicted tried table address. CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Wraps GetRandInt to allow tests to override RandomInt and make it determinismistic. - virtual int RandomInt(int nMax); - #ifdef DEBUG_ADDRMAN //! Perform consistency check. Returns an error code or zero. int Check_() EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -575,7 +572,7 @@ public: { LOCK(cs); std::vector().swap(vRandom); - nKey = GetRandHash(); + nKey = insecure_rand.rand256(); for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) { for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) { vvNew[bucket][entry] = -1; diff --git a/src/net.cpp b/src/net.cpp index 1039bfeb73..ae0be162f3 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -176,11 +176,12 @@ static std::vector convertSeed6(const std::vector &vSeedsIn const int64_t nOneWeek = 7*24*60*60; std::vector vSeedsOut; vSeedsOut.reserve(vSeedsIn.size()); + FastRandomContext rng; for (const auto& seed_in : vSeedsIn) { struct in6_addr ip; memcpy(&ip, seed_in.addr, sizeof(ip)); CAddress addr(CService(ip, seed_in.port), GetDesirableServiceFlags(NODE_NONE)); - addr.nTime = GetTime() - GetRand(nOneWeek) - nOneWeek; + addr.nTime = GetTime() - rng.randrange(nOneWeek) - nOneWeek; vSeedsOut.push_back(addr); } return vSeedsOut; @@ -230,16 +231,16 @@ void AdvertiseLocal(CNode *pnode) // If discovery is enabled, sometimes give our peer the address it // tells us that it sees us as in case it has a better idea of our // address than we do. + FastRandomContext rng; if (IsPeerAddrLocalGood(pnode) && (!addrLocal.IsRoutable() || - GetRand((GetnScore(addrLocal) > LOCAL_MANUAL) ? 8:2) == 0)) + rng.randbits((GetnScore(addrLocal) > LOCAL_MANUAL) ? 3 : 1) == 0)) { addrLocal.SetIP(pnode->GetAddrLocal()); } if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false)) { LogPrint(BCLog::NET, "AdvertiseLocal: advertising address %s\n", addrLocal.ToString()); - FastRandomContext insecure_rand; - pnode->PushAddress(addrLocal, insecure_rand); + pnode->PushAddress(addrLocal, rng); } } } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index dbb09f4aa7..d8f61de9e9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1053,6 +1053,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphansSize) nNextSweep = nMinExpTime + ORPHAN_TX_EXPIRE_INTERVAL; if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx due to expiration\n", nErased); } + FastRandomContext rng; while (!mapOrphanTransactions.empty() && nMapOrphanTransactionsSize > nMaxOrphansSize) { // Evict a random orphan: diff --git a/src/random.cpp b/src/random.cpp index 1f2e650af3..b356c59d42 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -430,6 +430,7 @@ uint256 FastRandomContext::rand256() std::vector FastRandomContext::randbytes(size_t len) { + if (requires_seed) RandomSeed(); std::vector ret(len); if (len > 0) { rng.Keystream(&ret[0], len); @@ -495,6 +496,20 @@ FastRandomContext::FastRandomContext(bool fDeterministic) : requires_seed(!fDete rng.SetKey(seed.begin(), 32); } +FastRandomContext& FastRandomContext::operator=(FastRandomContext&& from) noexcept +{ + requires_seed = from.requires_seed; + rng = from.rng; + std::copy(std::begin(from.bytebuf), std::end(from.bytebuf), std::begin(bytebuf)); + bytebuf_size = from.bytebuf_size; + bitbuf = from.bitbuf; + bitbuf_size = from.bitbuf_size; + from.requires_seed = true; + from.bytebuf_size = 0; + from.bitbuf_size = 0; + return *this; +} + void RandomInit() { RDRandInit(); diff --git a/src/random.h b/src/random.h index c035a046c2..b3478f72cf 100644 --- a/src/random.h +++ b/src/random.h @@ -80,6 +80,14 @@ public: /** Initialize with explicit seed (only for testing) */ explicit FastRandomContext(const uint256& seed); + // Do not permit copying a FastRandomContext (move it, or create a new one to get reseeded). + FastRandomContext(const FastRandomContext&) = delete; + FastRandomContext(FastRandomContext&&) = delete; + FastRandomContext& operator=(const FastRandomContext&) = delete; + + /** Move a FastRandomContext. If the original one is used again, it will be reseeded. */ + FastRandomContext& operator=(FastRandomContext&& from) noexcept; + /** Generate a random 64-bit integer. */ uint64_t rand64() { diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index aed0ffe3f4..fb4a549f0b 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -39,12 +39,6 @@ public: insecure_rand = FastRandomContext(true); } - int RandomInt(int nMax) override - { - state = (CHashWriter(SER_GETHASH, 0) << state).GetHash().GetCheapHash(); - return (unsigned int)(state % nMax); - } - CAddrInfo* Find(const CService& addr, int* pnId = nullptr) { LOCK(cs); diff --git a/src/test/cuckoocache_tests.cpp b/src/test/cuckoocache_tests.cpp index 324a7b5b05..ffc3343b73 100644 --- a/src/test/cuckoocache_tests.cpp +++ b/src/test/cuckoocache_tests.cpp @@ -22,40 +22,23 @@ * using BOOST_CHECK_CLOSE to fail. * */ -FastRandomContext local_rand_ctx(true); - BOOST_AUTO_TEST_SUITE(cuckoocache_tests); - -/** insecure_GetRandHash fills in a uint256 from local_rand_ctx - */ -static void insecure_GetRandHash(uint256& t) -{ - uint32_t* ptr = (uint32_t*)t.begin(); - for (uint8_t j = 0; j < 8; ++j) - *(ptr++) = local_rand_ctx.rand32(); -} - - - /* Test that no values not inserted into the cache are read out of it. * * There are no repeats in the first 200000 insecure_GetRandHash calls */ BOOST_AUTO_TEST_CASE(test_cuckoocache_no_fakes) { - local_rand_ctx = FastRandomContext(true); + SeedInsecureRand(true); CuckooCache::cache cc{}; size_t megabytes = 4; cc.setup_bytes(megabytes << 20); - uint256 v; for (int x = 0; x < 100000; ++x) { - insecure_GetRandHash(v); - cc.insert(v); + cc.insert(InsecureRand256()); } for (int x = 0; x < 100000; ++x) { - insecure_GetRandHash(v); - BOOST_CHECK(!cc.contains(v, false)); + BOOST_CHECK(!cc.contains(InsecureRand256(), false)); } }; @@ -65,7 +48,7 @@ BOOST_AUTO_TEST_CASE(test_cuckoocache_no_fakes) template static double test_cache(size_t megabytes, double load) { - local_rand_ctx = FastRandomContext(true); + SeedInsecureRand(true); std::vector hashes; Cache set{}; size_t bytes = megabytes * (1 << 20); @@ -75,7 +58,7 @@ static double test_cache(size_t megabytes, double load) for (uint32_t i = 0; i < n_insert; ++i) { uint32_t* ptr = (uint32_t*)hashes[i].begin(); for (uint8_t j = 0; j < 8; ++j) - *(ptr++) = local_rand_ctx.rand32(); + *(ptr++) = InsecureRand32(); } /** We make a copy of the hashes because future optimizations of the * cuckoocache may overwrite the inserted element, so the test is @@ -136,7 +119,7 @@ template static void test_cache_erase(size_t megabytes) { double load = 1; - local_rand_ctx = FastRandomContext(true); + SeedInsecureRand(true); std::vector hashes; Cache set{}; size_t bytes = megabytes * (1 << 20); @@ -146,7 +129,7 @@ static void test_cache_erase(size_t megabytes) for (uint32_t i = 0; i < n_insert; ++i) { uint32_t* ptr = (uint32_t*)hashes[i].begin(); for (uint8_t j = 0; j < 8; ++j) - *(ptr++) = local_rand_ctx.rand32(); + *(ptr++) = InsecureRand32(); } /** We make a copy of the hashes because future optimizations of the * cuckoocache may overwrite the inserted element, so the test is @@ -199,7 +182,7 @@ template static void test_cache_erase_parallel(size_t megabytes) { double load = 1; - local_rand_ctx = FastRandomContext(true); + SeedInsecureRand(true); std::vector hashes; Cache set{}; size_t bytes = megabytes * (1 << 20); @@ -209,7 +192,7 @@ static void test_cache_erase_parallel(size_t megabytes) for (uint32_t i = 0; i < n_insert; ++i) { uint32_t* ptr = (uint32_t*)hashes[i].begin(); for (uint8_t j = 0; j < 8; ++j) - *(ptr++) = local_rand_ctx.rand32(); + *(ptr++) = InsecureRand32(); } /** We make a copy of the hashes because future optimizations of the * cuckoocache may overwrite the inserted element, so the test is @@ -301,7 +284,7 @@ static void test_cache_generations() // iterations with non-deterministic values, so it isn't "overfit" to the // specific entropy in FastRandomContext(true) and implementation of the // cache. - local_rand_ctx = FastRandomContext(true); + SeedInsecureRand(true); // block_activity models a chunk of network activity. n_insert elements are // added to the cache. The first and last n/4 are stored for removal later @@ -318,7 +301,7 @@ static void test_cache_generations() for (uint32_t i = 0; i < n_insert; ++i) { uint32_t* ptr = (uint32_t*)inserts[i].begin(); for (uint8_t j = 0; j < 8; ++j) - *(ptr++) = local_rand_ctx.rand32(); + *(ptr++) = InsecureRand32(); } for (uint32_t i = 0; i < n_insert / 4; ++i) reads.push_back(inserts[i]); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 158d1bff81..789e158a12 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -133,7 +133,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) static void AddRandomOutboundPeer(std::vector &vNodes, PeerLogicValidation &peerLogic, CConnmanTest* connman) { - CAddress addr(ip(GetRandInt(0xffffffff)), NODE_NONE); + CAddress addr(ip(insecure_rand_ctx.randbits(32)), NODE_NONE); vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", /*fInboundIn=*/ false)); CNode &node = *vNodes.back(); node.SetSendVersion(PROTOCOL_VERSION); diff --git a/src/test/prevector_tests.cpp b/src/test/prevector_tests.cpp index b16f90779d..a789c582e7 100644 --- a/src/test/prevector_tests.cpp +++ b/src/test/prevector_tests.cpp @@ -209,8 +209,8 @@ public: prevector_tester() { SeedInsecureRand(); - rand_seed = insecure_rand_seed; - rand_cache = insecure_rand_ctx; + rand_seed = InsecureRand256(); + rand_cache = FastRandomContext(rand_seed); } }; diff --git a/src/test/random_tests.cpp b/src/test/random_tests.cpp index 158ca5f627..a5d3ea7b30 100644 --- a/src/test/random_tests.cpp +++ b/src/test/random_tests.cpp @@ -48,11 +48,19 @@ BOOST_AUTO_TEST_CASE(fastrandom_tests) BOOST_CHECK(GetRand(std::numeric_limits::max()) != uint64_t{10393729187455219830U}); BOOST_CHECK(GetRandInt(std::numeric_limits::max()) != int{769702006}); } - FastRandomContext ctx3; - FastRandomContext ctx4; - BOOST_CHECK(ctx3.rand64() != ctx4.rand64()); // extremely unlikely to be equal - BOOST_CHECK(ctx3.rand256() != ctx4.rand256()); - BOOST_CHECK(ctx3.randbytes(7) != ctx4.randbytes(7)); + + { + FastRandomContext ctx3, ctx4; + BOOST_CHECK(ctx3.rand64() != ctx4.rand64()); // extremely unlikely to be equal + } + { + FastRandomContext ctx3, ctx4; + BOOST_CHECK(ctx3.rand256() != ctx4.rand256()); + } + { + FastRandomContext ctx3, ctx4; + BOOST_CHECK(ctx3.randbytes(7) != ctx4.randbytes(7)); + } } BOOST_AUTO_TEST_CASE(fastrandom_randbits) diff --git a/src/test/test_dash.cpp b/src/test/test_dash.cpp index 2ccecaaa93..4b8c568a7f 100644 --- a/src/test/test_dash.cpp +++ b/src/test/test_dash.cpp @@ -30,8 +30,7 @@ const std::function G_TRANSLATION_FUN = nullptr; -uint256 insecure_rand_seed = GetRandHash(); -FastRandomContext insecure_rand_ctx(insecure_rand_seed); +FastRandomContext insecure_rand_ctx; extern bool fPrintToConsole; extern void noui_connect(); diff --git a/src/test/test_dash.h b/src/test/test_dash.h index 5a6ac4ebe4..41f25abf6f 100644 --- a/src/test/test_dash.h +++ b/src/test/test_dash.h @@ -19,7 +19,6 @@ #include -extern uint256 insecure_rand_seed; extern FastRandomContext insecure_rand_ctx; /** @@ -27,14 +26,9 @@ extern FastRandomContext insecure_rand_ctx; */ extern bool g_mock_deterministic_tests; -static inline void SeedInsecureRand(bool fDeterministic = false) +static inline void SeedInsecureRand(bool deterministic = false) { - if (fDeterministic) { - insecure_rand_seed = uint256(); - } else { - insecure_rand_seed = GetRandHash(); - } - insecure_rand_ctx = FastRandomContext(insecure_rand_seed); + insecure_rand_ctx = FastRandomContext(deterministic); } static inline uint32_t InsecureRand32() { return insecure_rand_ctx.rand32(); } diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 9c1a604916..5e2d90ef2a 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -100,8 +100,8 @@ void BuildChain(const uint256& root, int height, const unsigned int invalid_rate { if (height <= 0 || blocks.size() >= max_size) return; - bool gen_invalid = GetRand(100) < invalid_rate; - bool gen_fork = GetRand(100) < branch_rate; + bool gen_invalid = InsecureRandRange(100) < invalid_rate; + bool gen_fork = InsecureRandRange(100) < branch_rate; const std::shared_ptr pblock = gen_invalid ? BadBlock(root) : GoodBlock(root); blocks.push_back(pblock); @@ -153,7 +153,7 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) threads.create_thread([&blocks]() { bool ignored; for (int i = 0; i < 1000; i++) { - auto block = blocks[GetRand(blocks.size() - 1)]; + auto block = blocks[InsecureRandRange(blocks.size() - 1)]; ProcessNewBlock(Params(), block, true, &ignored); } From 657fea79a3a8c1d42ae6e558ef57d91e3fe78e11 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 24 Jan 2019 15:19:47 +0100 Subject: [PATCH 9/9] Merge #15193: Default -whitelistforcerelay to off a36d97d866e8a11f205d07c624ace7c3d1a2ded8 Default -whitelistforcerelay to off (Suhas Daftuar) Pull request description: No one seems to use this "feature", and at any rate the behavior of relaying transactions when they violate local policy is error-prone, if we ever consider changing the ban behavior of our software from one version to the next. Defaulting this to off means that users who use -whitelist won't be unexpectedly surprised by this interaction. If anyone is still relying on this feature, it can still be explicitly turned on. Tree-SHA512: 52650ad464a728d1648f496751e3f713077ea3a1de7278ed03531b2e8723e63cf2f6f41b56c98c0f73ffa22c36e01d9170b409ab452c737aca35b7ecd7a6b448 # Conflicts: # doc/release-notes.md # src/validation.h # test/functional/p2p_segwit.py --- src/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net.h b/src/net.h index 00be618fa0..61052e728d 100644 --- a/src/net.h +++ b/src/net.h @@ -52,7 +52,7 @@ class BanMan; /** Default for -whitelistrelay. */ static const bool DEFAULT_WHITELISTRELAY = true; /** Default for -whitelistforcerelay. */ -static const bool DEFAULT_WHITELISTFORCERELAY = true; +static const bool DEFAULT_WHITELISTFORCERELAY = false; /** Time between pings automatically sent out for latency probing and keepalive (in seconds). */ static const int PING_INTERVAL = 2 * 60;