From 38ee2a7a940f4e6c4db6f7784081d8fae42b7f8c Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 21 Jan 2019 18:45:59 +0100 Subject: [PATCH] Merge #14605: Return of the Banman MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 18185b57c32d0a43afeca4c125b9352c692923e9 scripted-diff: batch-recase BanMan variables (Carl Dong) c2e04d37f3841d109c1fe60693f9622e2836cc29 banman: Add, use CBanEntry ctor that takes ban reason (Carl Dong) 1ffa4ce27d4ea6c1067d8984455df97994c7713e banman: reformulate nBanUtil calculation (Carl Dong) daae598feb034f2f56e0b00ecfb4854d693d3641 banman: add thread annotations and mark members const where possible (Cory Fields) 84fc3fbd0304a7d6e660bf783c84bed2dd415141 scripted-diff: batch-rename BanMan members (Cory Fields) af3503d903b1a608cd212e2d74b274103199078c net: move BanMan to its own files (Cory Fields) d0469b2e9386a7a4b268cb9725347e7517acace6 banman: pass in default ban time as a parameter (Cory Fields) 2e56702ecedd83c4b7cb8de9de5c437c8c08e645 banman: pass the banfile path in (Cory Fields) 4c0d961eb0d7825a1e6f8389d7f5545114ee18c6 banman: create and split out banman (Cory Fields) 83c1ea2e5e66b8a83072e3d5ad6a4ced406eb1ba net: split up addresses/ban dumps in preparation for moving them (Cory Fields) 136bd7926c72659dd277a7b795ea17f72e523338 tests: remove member connman/peerLogic in TestingSetup (Cory Fields) 7cc2b9f6786f9bc33853220551eed33ca6b7b7b2 net: Break disconnecting out of Ban() (Cory Fields) Pull request description: **Old English à la Beowulf** ``` Banman wæs bréme --blaéd wíde sprang-- Connmanes eafera Coreum in. aéglaéca léodum forstandan Swá bealdode bearn Connmanes guma gúðum cúð gódum daédum· dréah æfter dóme· nealles druncne slóg ``` **Modern English Translation** ``` Banman was famed --his renown spread wide-- Conman's hier, in Core-land. against the evil creature defend the people Thus he was bold, the son of Connman man famed in war, for good deeds; he led his life for glory, never, having drunk, slew ``` -- With @theuni's blessing, here is Banman, rebased. Original PR: https://github.com/bitcoin/bitcoin/pull/11457 -- Followup PRs: 1. Give `CNode` a `Disconnect` method ([source](https://github.com/bitcoin/bitcoin/pull/14605#discussion_r248065847)) 2. Add a comment to `std::atomic_bool fDisconnect` in `net.h` that setting this to true will cause the node to be disconnected the next time `DisconnectNodes()` runs ([source](https://github.com/bitcoin/bitcoin/pull/14605#discussion_r248384309)) Tree-SHA512: 9c207edbf577415c22c9811113e393322d936a843d4ff265186728152a67c057779ac4d4f27b895de9729f7a53e870f828b9ebc8bcdab757520c2aebe1e9be35 --- src/Makefile.am | 2 + src/addrdb.cpp | 7 +- src/addrdb.h | 9 +- src/banman.cpp | 197 +++++++++++++++++++++++++ src/banman.h | 69 +++++++++ src/init.cpp | 19 ++- src/interfaces/node.cpp | 20 ++- src/interfaces/node.h | 6 +- src/net.cpp | 228 ++++------------------------- src/net.h | 46 +----- src/net_processing.cpp | 27 ++-- src/net_processing.h | 4 +- src/qt/rpcconsole.cpp | 16 +- src/rpc/net.cpp | 38 +++-- src/test/denialofservice_tests.cpp | 70 ++++++--- src/test/test_dash.cpp | 28 +--- src/test/test_dash.h | 6 - src/test/test_dash_main.cpp | 2 + 18 files changed, 454 insertions(+), 340 deletions(-) create mode 100644 src/banman.cpp create mode 100644 src/banman.h diff --git a/src/Makefile.am b/src/Makefile.am index 144a9588fa..34bd6dca22 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -118,6 +118,7 @@ BITCOIN_CORE_H = \ spentindex.h \ addrman.h \ attributes.h \ + banman.h \ base58.h \ batchedlogger.h \ bech32.h \ @@ -311,6 +312,7 @@ libdash_server_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libdash_server_a_SOURCES = \ addrdb.cpp \ addrman.cpp \ + banman.cpp \ batchedlogger.cpp \ bloom.cpp \ blockencodings.cpp \ diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 3e7b6577fe..c790ac1067 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -117,19 +117,18 @@ bool DeserializeFileDB(const fs::path& path, Data& data) } -CBanDB::CBanDB() +CBanDB::CBanDB(fs::path ban_list_path) : m_ban_list_path(std::move(ban_list_path)) { - pathBanlist = GetDataDir() / "banlist.dat"; } bool CBanDB::Write(const banmap_t& banSet) { - return SerializeFileDB("banlist", pathBanlist, banSet); + return SerializeFileDB("banlist", m_ban_list_path, banSet); } bool CBanDB::Read(banmap_t& banSet) { - return DeserializeFileDB(pathBanlist, banSet); + return DeserializeFileDB(m_ban_list_path, banSet); } CAddrDB::CAddrDB() diff --git a/src/addrdb.h b/src/addrdb.h index b467645d09..73889d77bf 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -43,6 +43,11 @@ public: nCreateTime = nCreateTimeIn; } + explicit CBanEntry(int64_t n_create_time_in, BanReason ban_reason_in) : CBanEntry(n_create_time_in) + { + banReason = ban_reason_in; + } + SERIALIZE_METHODS(CBanEntry, obj) { READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, obj.banReason); } void SetNull() @@ -84,9 +89,9 @@ public: class CBanDB { private: - fs::path pathBanlist; + const fs::path m_ban_list_path; public: - CBanDB(); + explicit CBanDB(fs::path ban_list_path); bool Write(const banmap_t& banSet); bool Read(banmap_t& banSet); }; diff --git a/src/banman.cpp b/src/banman.cpp new file mode 100644 index 0000000000..9933c829c5 --- /dev/null +++ b/src/banman.cpp @@ -0,0 +1,197 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2017 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 +#include +#include + + +BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time) + : m_client_interface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time) +{ + if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist...")); + + int64_t n_start = GetTimeMillis(); + m_is_dirty = false; + banmap_t banmap; + if (m_ban_db.Read(banmap)) { + SetBanned(banmap); // thread save setter + SetBannedSetDirty(false); // no need to write down, just read data + SweepBanned(); // sweep out unused entries + + LogPrint(BCLog::NET, "Loaded %d banned node ips/subnets from banlist.dat %dms\n", + banmap.size(), GetTimeMillis() - n_start); + } else { + LogPrintf("Invalid or missing banlist.dat; recreating\n"); + SetBannedSetDirty(true); // force write + DumpBanlist(); + } +} + +BanMan::~BanMan() +{ + DumpBanlist(); +} + +void BanMan::DumpBanlist() +{ + SweepBanned(); // clean unused entries (if bantime has expired) + + if (!BannedSetIsDirty()) return; + + int64_t n_start = GetTimeMillis(); + + banmap_t banmap; + GetBanned(banmap); + if (m_ban_db.Write(banmap)) { + SetBannedSetDirty(false); + } + + LogPrint(BCLog::NET, "Flushed %d banned node ips/subnets to banlist.dat %dms\n", + banmap.size(), GetTimeMillis() - n_start); +} + +void BanMan::ClearBanned() +{ + { + LOCK(m_cs_banned); + m_banned.clear(); + m_is_dirty = true; + } + DumpBanlist(); //store banlist to disk + if (m_client_interface) m_client_interface->BannedListChanged(); +} + +bool BanMan::IsBanned(CNetAddr net_addr) +{ + 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) { + return true; + } + } + return false; +} + +bool BanMan::IsBanned(CSubNet sub_net) +{ + 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) { + return true; + } + } + return false; +} + +void BanMan::Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch) +{ + CSubNet sub_net(net_addr); + Ban(sub_net, ban_reason, ban_time_offset, since_unix_epoch); +} + +void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch) +{ + CBanEntry ban_entry(GetTime(), ban_reason); + + int64_t normalized_ban_time_offset = ban_time_offset; + bool normalized_since_unix_epoch = since_unix_epoch; + if (ban_time_offset <= 0) { + normalized_ban_time_offset = m_default_ban_time; + normalized_since_unix_epoch = false; + } + ban_entry.nBanUntil = (normalized_since_unix_epoch ? 0 : GetTime()) + normalized_ban_time_offset; + + { + LOCK(m_cs_banned); + if (m_banned[sub_net].nBanUntil < ban_entry.nBanUntil) { + m_banned[sub_net] = ban_entry; + m_is_dirty = true; + } else + return; + } + if (m_client_interface) m_client_interface->BannedListChanged(); + + //store banlist to disk immediately if user requested ban + if (ban_reason == BanReasonManuallyAdded) DumpBanlist(); +} + +bool BanMan::Unban(const CNetAddr& net_addr) +{ + CSubNet sub_net(net_addr); + return Unban(sub_net); +} + +bool BanMan::Unban(const CSubNet& sub_net) +{ + { + LOCK(m_cs_banned); + if (m_banned.erase(sub_net) == 0) return false; + m_is_dirty = true; + } + if (m_client_interface) m_client_interface->BannedListChanged(); + DumpBanlist(); //store banlist to disk immediately + return true; +} + +void BanMan::GetBanned(banmap_t& banmap) +{ + LOCK(m_cs_banned); + // Sweep the banlist so expired bans are not returned + SweepBanned(); + banmap = m_banned; //create a thread safe copy +} + +void BanMan::SetBanned(const banmap_t& banmap) +{ + LOCK(m_cs_banned); + m_banned = banmap; + m_is_dirty = true; +} + +void BanMan::SweepBanned() +{ + int64_t now = GetTime(); + bool notify_ui = false; + { + LOCK(m_cs_banned); + banmap_t::iterator it = m_banned.begin(); + while (it != m_banned.end()) { + CSubNet sub_net = (*it).first; + CBanEntry ban_entry = (*it).second; + if (now > ban_entry.nBanUntil) { + m_banned.erase(it++); + m_is_dirty = true; + notify_ui = true; + LogPrint(BCLog::NET, "%s: Removed banned node ip/subnet from banlist.dat: %s\n", __func__, sub_net.ToString()); + } else + ++it; + } + } + // update UI + if (notify_ui && m_client_interface) { + m_client_interface->BannedListChanged(); + } +} + +bool BanMan::BannedSetIsDirty() +{ + LOCK(m_cs_banned); + return m_is_dirty; +} + +void BanMan::SetBannedSetDirty(bool dirty) +{ + LOCK(m_cs_banned); //reuse m_banned lock for the m_is_dirty flag + m_is_dirty = dirty; +} diff --git a/src/banman.h b/src/banman.h new file mode 100644 index 0000000000..69f62be368 --- /dev/null +++ b/src/banman.h @@ -0,0 +1,69 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. +#ifndef BITCOIN_BANMAN_H +#define BITCOIN_BANMAN_H + +#include +#include + +#include +#include +#include + +// NOTE: When adjusting this, update rpcnet:setban's help ("24h") +static constexpr unsigned int DEFAULT_MISBEHAVING_BANTIME = 60 * 60 * 24; // Default 24-hour ban + +class CClientUIInterface; +class CNetAddr; +class CSubNet; + +// Denial-of-service detection/prevention +// The idea is to detect peers that are behaving +// badly and disconnect/ban them, but do it in a +// one-coding-mistake-won't-shatter-the-entire-network +// way. +// IMPORTANT: There should be nothing I can give a +// node that it will forward on that will make that +// node's peers drop it. If there is, an attacker +// can isolate a node and/or try to split the network. +// Dropping a node for sending stuff that is invalid +// now but might be valid in a later version is also +// dangerous, because it can cause a network split +// between nodes running old code and nodes running +// new code. + +class BanMan +{ +public: + ~BanMan(); + BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time); + 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(); + bool IsBanned(CNetAddr net_addr); + bool IsBanned(CSubNet sub_net); + bool Unban(const CNetAddr& net_addr); + bool Unban(const CSubNet& sub_net); + void GetBanned(banmap_t& banmap); + void DumpBanlist(); + +private: + void SetBanned(const banmap_t& banmap); + bool BannedSetIsDirty(); + //!set the "dirty" flag for the banlist + void SetBannedSetDirty(bool dirty = true); + //!clean unused entries (if bantime has expired) + void SweepBanned(); + + CCriticalSection m_cs_banned; + banmap_t m_banned GUARDED_BY(m_cs_banned); + bool m_is_dirty GUARDED_BY(m_cs_banned); + CClientUIInterface* m_client_interface = nullptr; + CBanDB m_ban_db; + const int64_t m_default_ban_time; +}; + +extern std::unique_ptr g_banman; +#endif diff --git a/src/init.cpp b/src/init.cpp index a97242f16a..aae65104e5 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -103,9 +104,12 @@ static const bool DEFAULT_PROXYRANDOMIZE = true; static const bool DEFAULT_REST_ENABLE = false; static const bool DEFAULT_STOPAFTERBLOCKIMPORT = false; +// Dump addresses to banlist.dat every 15 minutes (900s) +static constexpr int DUMP_BANS_INTERVAL = 60 * 15; std::unique_ptr g_connman; std::unique_ptr peerLogic; +std::unique_ptr g_banman; #if !(ENABLE_WALLET) class DummyWalletInit : public WalletInitInterface { @@ -298,6 +302,7 @@ void PrepareShutdown() // destruct and reset all to nullptr. peerLogic.reset(); g_connman.reset(); + g_banman.reset(); g_txindex.reset(); if (g_is_mempool_loaded && gArgs.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { @@ -1883,11 +1888,12 @@ bool AppInitMain() // is not yet setup and may end up being set up twice if we // need to reindex later. + assert(!g_banman); + g_banman = MakeUnique(GetDataDir() / "banlist.dat", &uiInterface, gArgs.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); assert(!g_connman); g_connman = std::unique_ptr(new CConnman(GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max()))); - CConnman& connman = *g_connman; - peerLogic.reset(new PeerLogicValidation(&connman, scheduler, gArgs.GetBoolArg("-enablebip61", DEFAULT_ENABLE_BIP61))); + peerLogic.reset(new PeerLogicValidation(g_connman.get(), g_banman.get(), scheduler, gArgs.GetBoolArg("-enablebip61", DEFAULT_ENABLE_BIP61))); RegisterValidationInterface(peerLogic.get()); // sanitize comments per BIP-0014, format user agent and check total size @@ -2015,7 +2021,7 @@ bool AppInitMain() } #endif - pdsNotificationInterface = new CDSNotificationInterface(connman); + pdsNotificationInterface = new CDSNotificationInterface(*g_connman); RegisterValidationInterface(pdsNotificationInterface); uint64_t nMaxOutboundLimit = 0; //unlimited unless -maxuploadtarget is set @@ -2503,6 +2509,7 @@ bool AppInitMain() connOptions.nMaxFeeler = 1; connOptions.nBestHeight = chain_active_height; connOptions.uiInterface = &uiInterface; + connOptions.m_banman = g_banman.get(); connOptions.m_msgproc = peerLogic.get(); connOptions.nSendBufferMaxSize = 1000*gArgs.GetArg("-maxsendbuffer", DEFAULT_MAXSENDBUFFER); connOptions.nReceiveFloodSize = 1000*gArgs.GetArg("-maxreceivebuffer", DEFAULT_MAXRECEIVEBUFFER); @@ -2568,7 +2575,7 @@ bool AppInitMain() return InitError(strprintf(_("Invalid -socketevents ('%s') specified. Only these modes are supported: %s"), strSocketEventsMode, GetSupportedSocketEventsStr())); } - if (!connman.Start(scheduler, connOptions)) { + if (!g_connman->Start(scheduler, connOptions)) { return false; } @@ -2579,5 +2586,9 @@ bool AppInitMain() g_wallet_init_interface.Start(scheduler); + scheduler.scheduleEvery([]{ + g_banman->DumpBanlist(); + }, DUMP_BANS_INTERVAL * 1000); + return true; } diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 3359299389..613c9a020c 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -235,28 +236,35 @@ class NodeImpl : public Node } bool getBanned(banmap_t& banmap) override { - if (g_connman) { - g_connman->GetBanned(banmap); + if (g_banman) { + g_banman->GetBanned(banmap); return true; } return false; } bool ban(const CNetAddr& net_addr, BanReason reason, int64_t ban_time_offset) override { - if (g_connman) { - g_connman->Ban(net_addr, reason, ban_time_offset); + if (g_banman) { + g_banman->Ban(net_addr, reason, ban_time_offset); return true; } return false; } bool unban(const CSubNet& ip) override { - if (g_connman) { - g_connman->Unban(ip); + if (g_banman) { + g_banman->Unban(ip); return true; } return false; } + bool disconnect(const CNetAddr& net_addr) override + { + if (g_connman) { + return g_connman->DisconnectNode(net_addr); + } + return false; + } bool disconnect(NodeId id) override { if (g_connman) { diff --git a/src/interfaces/node.h b/src/interfaces/node.h index f2a918be27..0896670e57 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -18,6 +18,7 @@ #include #include +class BanMan; class CCoinControl; class CDeterministicMNList; class CFeeRate; @@ -165,7 +166,10 @@ public: //! Unban node. virtual bool unban(const CSubNet& ip) = 0; - //! Disconnect node. + //! Disconnect node by address. + virtual bool disconnect(const CNetAddr& net_addr) = 0; + + //! Disconnect node by id. virtual bool disconnect(NodeId id) = 0; //! Get total bytes recv. diff --git a/src/net.cpp b/src/net.cpp index 2fe02a21b8..12b20b6731 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -62,8 +63,8 @@ static_assert(MINIUPNPC_API_VERSION >= 10, "miniUPnPc API version >= 10 assumed" #include -// Dump addresses to peers.dat and banlist.dat every 15 minutes (900s) -#define DUMP_ADDRESSES_INTERVAL 900 +// Dump addresses to peers.dat every 15 minutes (900s) +static constexpr int DUMP_PEERS_INTERVAL = 15 * 60; /** Number of DNS seeds to query when the number of connections is low. */ static constexpr int DNSSEEDS_TO_QUERY_AT_ONCE = 3; @@ -507,26 +508,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo return pnode; } -void CConnman::DumpBanlist() -{ - SweepBanned(); // clean unused entries (if bantime has expired) - - if (!BannedSetIsDirty()) - return; - - int64_t nStart = GetTimeMillis(); - - CBanDB bandb; - banmap_t banmap; - GetBanned(banmap); - if (bandb.Write(banmap)) { - SetBannedSetDirty(false); - } - - LogPrint(BCLog::NET, "Flushed %d banned node ips/subnets to banlist.dat %dms\n", - banmap.size(), GetTimeMillis() - nStart); -} - void CNode::CloseSocketDisconnect(CConnman* connman) { AssertLockHeld(connman->cs_vNodes); @@ -558,157 +539,6 @@ void CNode::CloseSocketDisconnect(CConnman* connman) statsClient.inc("peers.disconnect", 1.0f); } -void CConnman::ClearBanned() -{ - { - LOCK(cs_setBanned); - setBanned.clear(); - setBannedIsDirty = true; - } - DumpBanlist(); //store banlist to disk - if(clientInterface) - clientInterface->BannedListChanged(); -} - -bool CConnman::IsBanned(CNetAddr ip) -{ - LOCK(cs_setBanned); - for (const auto& it : setBanned) { - CSubNet subNet = it.first; - CBanEntry banEntry = it.second; - - if (subNet.Match(ip) && GetTime() < banEntry.nBanUntil) { - return true; - } - } - return false; -} - -bool CConnman::IsBanned(CSubNet subnet) -{ - LOCK(cs_setBanned); - banmap_t::iterator i = setBanned.find(subnet); - if (i != setBanned.end()) - { - CBanEntry banEntry = (*i).second; - if (GetTime() < banEntry.nBanUntil) { - return true; - } - } - return false; -} - -void CConnman::Ban(const CNetAddr& addr, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) { - CSubNet subNet(addr); - Ban(subNet, banReason, bantimeoffset, sinceUnixEpoch); -} - -void CConnman::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) { - CBanEntry banEntry(GetTime()); - banEntry.banReason = banReason; - if (bantimeoffset <= 0) - { - bantimeoffset = gArgs.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME); - sinceUnixEpoch = false; - } - banEntry.nBanUntil = (sinceUnixEpoch ? 0 : GetTime() )+bantimeoffset; - - { - LOCK(cs_setBanned); - if (setBanned[subNet].nBanUntil < banEntry.nBanUntil) { - setBanned[subNet] = banEntry; - setBannedIsDirty = true; - } - else - return; - } - if(clientInterface) - clientInterface->BannedListChanged(); - { - LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { - if (subNet.Match(static_cast(pnode->addr))) - pnode->fDisconnect = true; - } - } - if(banReason == BanReasonManuallyAdded) - DumpBanlist(); //store banlist to disk immediately if user requested ban -} - -bool CConnman::Unban(const CNetAddr &addr) { - CSubNet subNet(addr); - return Unban(subNet); -} - -bool CConnman::Unban(const CSubNet &subNet) { - { - LOCK(cs_setBanned); - if (!setBanned.erase(subNet)) - return false; - setBannedIsDirty = true; - } - if(clientInterface) - clientInterface->BannedListChanged(); - DumpBanlist(); //store banlist to disk immediately - return true; -} - -void CConnman::GetBanned(banmap_t &banMap) -{ - LOCK(cs_setBanned); - // Sweep the banlist so expired bans are not returned - SweepBanned(); - banMap = setBanned; //create a thread safe copy -} - -void CConnman::SetBanned(const banmap_t &banMap) -{ - LOCK(cs_setBanned); - setBanned = banMap; - setBannedIsDirty = true; -} - -void CConnman::SweepBanned() -{ - int64_t now = GetTime(); - bool notifyUI = false; - { - LOCK(cs_setBanned); - banmap_t::iterator it = setBanned.begin(); - while(it != setBanned.end()) - { - CSubNet subNet = (*it).first; - CBanEntry banEntry = (*it).second; - if(now > banEntry.nBanUntil) - { - setBanned.erase(it++); - setBannedIsDirty = true; - notifyUI = true; - LogPrint(BCLog::NET, "%s: Removed banned node ip/subnet from banlist.dat: %s\n", __func__, subNet.ToString()); - } - else - ++it; - } - } - // update UI - if(notifyUI && clientInterface) { - clientInterface->BannedListChanged(); - } -} - -bool CConnman::BannedSetIsDirty() -{ - LOCK(cs_setBanned); - return setBannedIsDirty; -} - -void CConnman::SetBannedSetDirty(bool dirty) -{ - LOCK(cs_setBanned); //reuse setBanned lock for the isDirty flag - setBannedIsDirty = dirty; -} - - bool CConnman::IsWhitelistedRange(const CNetAddr &addr) { for (const CSubNet& subnet : vWhitelistedRange) { if (subnet.Match(addr)) @@ -1224,7 +1054,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { // on all platforms. Set it again here just to be sure. SetSocketNoDelay(hSocket); - if (IsBanned(addr) && !whitelisted) + if (m_banman && m_banman->IsBanned(addr) && !whitelisted) { LogPrint(BCLog::NET, "%s (banned)\n", strDropped); CloseSocket(hSocket); @@ -2257,12 +2087,6 @@ void CConnman::DumpAddresses() addrman.size(), GetTimeMillis() - nStart); } -void CConnman::DumpData() -{ - DumpAddresses(); - DumpBanlist(); -} - void CConnman::ProcessOneShot() { std::string strDest; @@ -2747,7 +2571,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai } if (!pszDest) { // banned or exact match? - if (IsBanned(addrConnect) || FindNode(addrConnect.ToStringIPPort())) + if ((m_banman && m_banman->IsBanned(addrConnect)) || FindNode(addrConnect.ToStringIPPort())) return; // local and not a connection to itself? bool fAllowLocal = Params().AllowMultiplePorts() && addrConnect.GetPort() != GetListenPort(); @@ -3032,7 +2856,6 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In) : nSeed0(nSeed0In), nSeed1(nSeed1In) { fNetworkActive = true; - setBannedIsDirty = false; fAddressesInitialized = false; nLastNodeId = 0; nPrevNodeCount = 0; @@ -3145,24 +2968,6 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) DumpAddresses(); } } - if (clientInterface) - clientInterface->InitMessage(_("Loading banlist...")); - // Load addresses from banlist.dat - nStart = GetTimeMillis(); - CBanDB bandb; - banmap_t banmap; - if (bandb.Read(banmap)) { - SetBanned(banmap); // thread save setter - SetBannedSetDirty(false); // no need to write down, just read data - SweepBanned(); // sweep out unused entries - - LogPrint(BCLog::NET, "Loaded %d banned node ips/subnets from banlist.dat %dms\n", - banmap.size(), GetTimeMillis() - nStart); - } else { - LogPrintf("Invalid or missing banlist.dat; recreating\n"); - SetBannedSetDirty(true); // force write - DumpBanlist(); - } uiInterface.InitMessage(_("Starting network threads...")); @@ -3260,7 +3065,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) threadMessageHandler = std::thread(&TraceThread >, "msghand", std::function(std::bind(&CConnman::ThreadMessageHandler, this))); // Dump network addresses - scheduler.scheduleEvery(std::bind(&CConnman::DumpData, this), DUMP_ADDRESSES_INTERVAL * 1000); + scheduler.scheduleEvery(std::bind(&CConnman::DumpAddresses, this), DUMP_PEERS_INTERVAL * 1000); return true; } @@ -3329,7 +3134,7 @@ void CConnman::Stop() if (fAddressesInitialized) { - DumpData(); + DumpAddresses(); fAddressesInitialized = false; } @@ -3660,6 +3465,25 @@ bool CConnman::DisconnectNode(const std::string& strNode) } return false; } + +bool CConnman::DisconnectNode(const CSubNet& subnet) +{ + bool disconnected = false; + LOCK(cs_vNodes); + for (CNode* pnode : vNodes) { + if (subnet.Match(pnode->addr)) { + pnode->fDisconnect = true; + disconnected = true; + } + } + return disconnected; +} + +bool CConnman::DisconnectNode(const CNetAddr& addr) +{ + return DisconnectNode(CSubNet(addr)); +} + bool CConnman::DisconnectNode(NodeId id) { LOCK(cs_vNodes); diff --git a/src/net.h b/src/net.h index 984efc7032..716c49d4f0 100644 --- a/src/net.h +++ b/src/net.h @@ -46,6 +46,7 @@ class CScheduler; class CNode; +class BanMan; /** Time between pings automatically sent out for latency probing and keepalive (in seconds). */ static const int PING_INTERVAL = 2 * 60; @@ -96,9 +97,6 @@ static const bool DEFAULT_FORCEDNSSEED = false; static const size_t DEFAULT_MAXRECEIVEBUFFER = 5 * 1000; static const size_t DEFAULT_MAXSENDBUFFER = 1 * 1000; -// NOTE: When adjusting this, update rpcnet:setban's help ("24h") -static const unsigned int DEFAULT_MISBEHAVING_BANTIME = 60 * 60 * 24; // Default 24-hour ban - #if defined USE_KQUEUE #define DEFAULT_SOCKETEVENTS "kqueue" #elif defined USE_EPOLL @@ -135,6 +133,7 @@ struct CSerializedNetMsg std::string command; }; + class NetEventsInterface; class CConnman { @@ -165,6 +164,7 @@ public: int nBestHeight = 0; CClientUIInterface* uiInterface = nullptr; NetEventsInterface* m_msgproc = nullptr; + BanMan* m_banman = nullptr; unsigned int nSendBufferMaxSize = 0; unsigned int nReceiveFloodSize = 0; uint64_t nMaxOutboundTimeframe = 0; @@ -189,6 +189,7 @@ public: nMaxFeeler = connOptions.nMaxFeeler; nBestHeight = connOptions.nBestHeight; clientInterface = connOptions.uiInterface; + m_banman = connOptions.m_banman; m_msgproc = connOptions.m_msgproc; nSendBufferMaxSize = connOptions.nSendBufferMaxSize; nReceiveFloodSize = connOptions.nReceiveFloodSize; @@ -376,30 +377,6 @@ public: void AddNewAddresses(const std::vector& vAddr, const CAddress& addrFrom, int64_t nTimePenalty = 0); std::vector GetAddresses(); - // Denial-of-service detection/prevention - // The idea is to detect peers that are behaving - // badly and disconnect/ban them, but do it in a - // one-coding-mistake-won't-shatter-the-entire-network - // way. - // IMPORTANT: There should be nothing I can give a - // node that it will forward on that will make that - // node's peers drop it. If there is, an attacker - // can isolate a node and/or try to split the network. - // Dropping a node for sending stuff that is invalid - // now but might be valid in a later version is also - // dangerous, because it can cause a network split - // between nodes running old code and nodes running - // new code. - void Ban(const CNetAddr& netAddr, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); - void Ban(const CSubNet& subNet, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); - void ClearBanned(); // needed for unit testing - bool IsBanned(CNetAddr ip); - bool IsBanned(CSubNet subnet); - bool Unban(const CNetAddr &ip); - bool Unban(const CSubNet &ip); - void GetBanned(banmap_t &banmap); - void SetBanned(const banmap_t &banmap); - // This allows temporarily exceeding nMaxOutbound, with the goal of finding // a peer that is better than all our current peers. void SetTryNewOutboundPeer(bool flag); @@ -433,6 +410,8 @@ public: size_t GetMaxOutboundNodeCount(); void GetNodeStats(std::vector& vstats); bool DisconnectNode(const std::string& node); + bool DisconnectNode(const CSubNet& subnet); + bool DisconnectNode(const CNetAddr& addr); bool DisconnectNode(NodeId id); ServiceFlags GetLocalServices() const; @@ -535,15 +514,7 @@ private: size_t SocketSendData(CNode *pnode); size_t SocketRecvData(CNode* pnode); - //!check is the banlist has unwritten changes - bool BannedSetIsDirty(); - //!set the "dirty" flag for the banlist - void SetBannedSetDirty(bool dirty=true); - //!clean unused entries (if bantime has expired) - void SweepBanned(); void DumpAddresses(); - void DumpData(); - void DumpBanlist(); // Network stats void RecordBytesRecv(uint64_t bytes); @@ -579,9 +550,6 @@ private: std::vector vhListenSocket; std::atomic fNetworkActive; - banmap_t setBanned GUARDED_BY(cs_setBanned); - CCriticalSection cs_setBanned; - bool setBannedIsDirty GUARDED_BY(cs_setBanned); bool fAddressesInitialized; CAddrMan addrman; std::deque vOneShots GUARDED_BY(cs_vOneShots); @@ -613,6 +581,7 @@ private: std::atomic nBestHeight; CClientUIInterface* clientInterface; NetEventsInterface* m_msgproc; + BanMan* m_banman; /** SipHasher seeds for deterministic randomness */ const uint64_t nSeed0, nSeed1; @@ -664,6 +633,7 @@ private: friend struct CConnmanTest; }; extern std::unique_ptr g_connman; +extern std::unique_ptr g_banman; void Discover(); void StartMapPort(); void InterruptMapPort(); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f5010e39ca..b484fcc7c8 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -1111,9 +1112,8 @@ static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Para (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT); } -PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, CScheduler &scheduler, bool enable_bip61) - : connman(connmanIn), m_stale_tip_check_time(0), m_enable_bip61(enable_bip61) { - +PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CScheduler &scheduler, bool enable_bip61) + : connman(connmanIn), m_banman(banman), m_stale_tip_check_time(0), m_enable_bip61(enable_bip61) { // Initialize global variables that cannot be constructed at startup. recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); @@ -3645,7 +3645,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; } -static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool PeerLogicValidation::SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); CNodeState &state = *State(pnode->GetId()); @@ -3663,14 +3663,16 @@ static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman, bool en 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()); - else { + else if (pnode->addr.IsLocal()) { + // Disconnect but don't ban _this_ local node + LogPrintf("Warning: disconnecting but not banning local peer %s!\n", pnode->GetLogString()); pnode->fDisconnect = true; - if (pnode->addr.IsLocal()) - LogPrintf("Warning: not banning local peer %s!\n", pnode->GetLogString()); - else - { - connman->Ban(pnode->addr, BanReasonNodeMisbehaving); + } else { + // Disconnect and ban all nodes sharing the address + if (m_banman) { + m_banman->Ban(pnode->addr, BanReasonNodeMisbehaving); } + connman->DisconnectNode(pnode->addr); } return true; } @@ -3798,7 +3800,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter } LOCK(cs_main); - SendRejectsAndCheckIfBanned(pfrom, connman, m_enable_bip61); + SendRejectsAndCheckIfBanned(pfrom, m_enable_bip61); return fMoreWork; } @@ -3995,8 +3997,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) if (!lockMain) return true; - if (SendRejectsAndCheckIfBanned(pto, connman, m_enable_bip61)) - return true; + if (SendRejectsAndCheckIfBanned(pto, m_enable_bip61)) return true; CNodeState &state = *State(pto->GetId()); // Address refresh broadcast diff --git a/src/net_processing.h b/src/net_processing.h index b59349b32a..7ded9c1a7d 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -24,9 +24,11 @@ static constexpr bool DEFAULT_ENABLE_BIP61 = true; class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface { private: CConnman* const connman; + BanMan* const m_banman; + bool SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main); public: - explicit PeerLogicValidation(CConnman* connmanIn, CScheduler &scheduler, bool enable_bip61); + PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CScheduler &scheduler, bool enable_bip61); /** * Overridden from CValidationInterface. diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 631a0e7290..88e75d2554 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1376,16 +1376,16 @@ void RPCConsole::banSelectedNode(int bantime) // Get currently selected peer address NodeId id = nodes.at(i).data().toLongLong(); - // Get currently selected peer address - int detailNodeRow = clientModel->getPeerTableModel()->getRowByNodeId(id); - if(detailNodeRow < 0) - return; + // Get currently selected peer address + int detailNodeRow = clientModel->getPeerTableModel()->getRowByNodeId(id); + if (detailNodeRow < 0) return; - // Find possible nodes, ban it and clear the selected node - const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow); - if(stats) { + // Find possible nodes, ban it and clear the selected node + const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow); + if (stats) { m_node.ban(stats->nodeStats.addr, BanReasonManuallyAdded, bantime); - } + m_node.disconnect(stats->nodeStats.addr); + } } clearSelectedNode(); clientModel->getBanTableModel()->refresh(); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 8d8ff5d77d..c45106e98d 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -5,6 +5,7 @@ #include +#include #include #include #include @@ -550,8 +551,9 @@ static UniValue setban(const JSONRPCRequest& request) + HelpExampleCli("setban", "\"192.168.0.0/24\" \"add\"") + HelpExampleRpc("setban", "\"192.168.0.6\", \"add\", 86400") ); - if(!g_connman) - throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + if (!g_banman) { + throw JSONRPCError(RPC_DATABASE_ERROR, "Error: Ban database not loaded"); + } CSubNet subNet; CNetAddr netAddr; @@ -573,8 +575,9 @@ static UniValue setban(const JSONRPCRequest& request) if (strCommand == "add") { - if (isSubnet ? g_connman->IsBanned(subNet) : g_connman->IsBanned(netAddr)) + if (isSubnet ? g_banman->IsBanned(subNet) : g_banman->IsBanned(netAddr)) { throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: IP/Subnet already banned"); + } int64_t banTime = 0; //use standard bantime if not specified if (!request.params[2].isNull()) @@ -584,12 +587,23 @@ static UniValue setban(const JSONRPCRequest& request) if (request.params[3].isTrue()) absolute = true; - isSubnet ? g_connman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute) : g_connman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute); + if (isSubnet) { + g_banman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute); + if (g_connman) { + g_connman->DisconnectNode(subNet); + } + } else { + g_banman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute); + if (g_connman) { + g_connman->DisconnectNode(netAddr); + } + } } else if(strCommand == "remove") { - if (!( isSubnet ? g_connman->Unban(subNet) : g_connman->Unban(netAddr) )) + if (!( isSubnet ? g_banman->Unban(subNet) : g_banman->Unban(netAddr) )) { throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously banned."); + } } return NullUniValue; } @@ -605,11 +619,12 @@ static UniValue listbanned(const JSONRPCRequest& request) + HelpExampleRpc("listbanned", "") ); - if(!g_connman) - throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + if(!g_banman) { + throw JSONRPCError(RPC_DATABASE_ERROR, "Error: Ban database not loaded"); + } banmap_t banMap; - g_connman->GetBanned(banMap); + g_banman->GetBanned(banMap); UniValue bannedAddresses(UniValue::VARR); for (const auto& entry : banMap) @@ -637,10 +652,11 @@ static UniValue clearbanned(const JSONRPCRequest& request) + HelpExampleCli("clearbanned", "") + HelpExampleRpc("clearbanned", "") ); - if(!g_connman) - throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + if (!g_banman) { + throw JSONRPCError(RPC_DATABASE_ERROR, "Error: Ban database not loaded"); + } - g_connman->ClearBanned(); + g_banman->ClearBanned(); return NullUniValue; } diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 6f59c4ced2..158d1bff81 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -4,6 +4,7 @@ // Unit tests for denial-of-service detection/prevention code +#include #include #include #include @@ -20,6 +21,23 @@ #include +struct CConnmanTest : public CConnman { + using CConnman::CConnman; + void AddNode(CNode& node) + { + LOCK(cs_vNodes); + vNodes.push_back(&node); + } + void ClearNodes() + { + LOCK(cs_vNodes); + for (CNode* node : vNodes) { + delete node; + } + vNodes.clear(); + } +}; + // Tests these internal-to-net_processing.cpp methods: extern bool AddOrphanTx(const CTransactionRef& tx, NodeId peer); extern void EraseOrphansFor(NodeId peer); @@ -58,6 +76,8 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup) // work. BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { + auto connman = MakeUnique(0x1337, 0x1337); + auto peerLogic = MakeUnique(connman.get(), nullptr, scheduler, false); // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -111,7 +131,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); } -static void AddRandomOutboundPeer(std::vector &vNodes, PeerLogicValidation &peerLogic) +static void AddRandomOutboundPeer(std::vector &vNodes, PeerLogicValidation &peerLogic, CConnmanTest* connman) { CAddress addr(ip(GetRandInt(0xffffffff)), NODE_NONE); vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", /*fInboundIn=*/ false)); @@ -122,11 +142,14 @@ static void AddRandomOutboundPeer(std::vector &vNodes, PeerLogicValidat node.nVersion = 1; node.fSuccessfullyConnected = true; - CConnmanTest::AddNode(node); + connman->AddNode(node); } BOOST_AUTO_TEST_CASE(stale_tip_peer_management) { + auto connman = MakeUnique(0x1337, 0x1337); + auto peerLogic = MakeUnique(connman.get(), nullptr, scheduler, false); + const Consensus::Params& consensusParams = Params().GetConsensus(); constexpr int nMaxOutbound = 8; CConnman::Options options; @@ -139,7 +162,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) // Mock some outbound peers for (int i=0; iCheckForStaleTipAndEvictPeers(consensusParams); @@ -164,7 +187,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) // If we add one more peer, something should get marked for eviction // on the next check (since we're mocking the time to be in the future, the // required time connected check should be satisfied). - AddRandomOutboundPeer(vNodes, *peerLogic); + AddRandomOutboundPeer(vNodes, *peerLogic, connman.get()); peerLogic->CheckForStaleTipAndEvictPeers(consensusParams); for (int i=0; iFinalizeNode(node->GetId(), dummy); } - CConnmanTest::ClearNodes(); + connman->ClearNodes(); } BOOST_AUTO_TEST_CASE(DoS_banning) { + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + auto connman = MakeUnique(0x1337, 0x1337); + auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); - connman->ClearBanned(); + banman->ClearBanned(); CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", true); dummyNode1.SetSendVersion(PROTOCOL_VERSION); @@ -212,8 +238,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK2(cs_main, dummyNode1.cs_sendProcessing); peerLogic->SendMessages(&dummyNode1); } - BOOST_CHECK(connman->IsBanned(addr1)); - BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned + BOOST_CHECK(banman->IsBanned(addr1)); + BOOST_CHECK(!banman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned CAddress addr2(ip(0xa0b0c002), NODE_NONE); CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", true); @@ -229,8 +255,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK2(cs_main, dummyNode2.cs_sendProcessing); peerLogic->SendMessages(&dummyNode2); } - BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet... - BOOST_CHECK(connman->IsBanned(addr1)); // ... but 1 still should be + BOOST_CHECK(!banman->IsBanned(addr2)); // 2 not banned yet... + BOOST_CHECK(banman->IsBanned(addr1)); // ... but 1 still should be { LOCK(cs_main); Misbehaving(dummyNode2.GetId(), 50); @@ -239,7 +265,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK2(cs_main, dummyNode2.cs_sendProcessing); peerLogic->SendMessages(&dummyNode2); } - BOOST_CHECK(connman->IsBanned(addr2)); + BOOST_CHECK(banman->IsBanned(addr2)); bool dummy; peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); @@ -248,8 +274,11 @@ BOOST_AUTO_TEST_CASE(DoS_banning) BOOST_AUTO_TEST_CASE(DoS_banscore) { + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + auto connman = MakeUnique(0x1337, 0x1337); + auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); - connman->ClearBanned(); + banman->ClearBanned(); gArgs.ForceSetArg("-banscore", "111"); // because 11 is my favorite number CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, 1, CAddress(), "", true); @@ -265,7 +294,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); peerLogic->SendMessages(&dummyNode1); } - BOOST_CHECK(!connman->IsBanned(addr1)); + BOOST_CHECK(!banman->IsBanned(addr1)); { LOCK(cs_main); Misbehaving(dummyNode1.GetId(), 10); @@ -274,7 +303,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); peerLogic->SendMessages(&dummyNode1); } - BOOST_CHECK(!connman->IsBanned(addr1)); + BOOST_CHECK(!banman->IsBanned(addr1)); { LOCK(cs_main); Misbehaving(dummyNode1.GetId(), 1); @@ -283,7 +312,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); peerLogic->SendMessages(&dummyNode1); } - BOOST_CHECK(connman->IsBanned(addr1)); + BOOST_CHECK(banman->IsBanned(addr1)); gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); bool dummy; @@ -292,8 +321,11 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) BOOST_AUTO_TEST_CASE(DoS_bantime) { + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + auto connman = MakeUnique(0x1337, 0x1337); + auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); - connman->ClearBanned(); + banman->ClearBanned(); int64_t nStartTime = GetTime(); SetMockTime(nStartTime); // Overrides future calls to GetTime() @@ -312,13 +344,13 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) LOCK2(cs_main, dummyNode.cs_sendProcessing); peerLogic->SendMessages(&dummyNode); } - BOOST_CHECK(connman->IsBanned(addr)); + BOOST_CHECK(banman->IsBanned(addr)); SetMockTime(nStartTime+60*60); - BOOST_CHECK(connman->IsBanned(addr)); + BOOST_CHECK(banman->IsBanned(addr)); SetMockTime(nStartTime+60*60*24+1); - BOOST_CHECK(!connman->IsBanned(addr)); + BOOST_CHECK(!banman->IsBanned(addr)); bool dummy; peerLogic->FinalizeNode(dummyNode.GetId(), dummy); diff --git a/src/test/test_dash.cpp b/src/test/test_dash.cpp index 3fcf2e4e20..2ccecaaa93 100644 --- a/src/test/test_dash.cpp +++ b/src/test/test_dash.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -29,28 +30,6 @@ const std::function G_TRANSLATION_FUN = nullptr; -void CConnmanTest::AddNode(CNode& node) -{ - LOCK2(g_connman->cs_vNodes, node.cs_hSocket); - g_connman->vNodes.push_back(&node); - g_connman->mapSocketToNode.emplace(node.hSocket, &node); -} - -void CConnmanTest::ClearNodes() -{ - LOCK(g_connman->cs_vNodes); - for (CNode* node : g_connman->vNodes) { - delete node; - } - g_connman->vNodes.clear(); - g_connman->mapSocketToNode.clear(); - - g_connman->mapReceivableNodes.clear(); - g_connman->mapSendableNodes.clear(); - LOCK(g_connman->cs_mapNodesWithDataToSend); - g_connman->mapNodesWithDataToSend.clear(); -} - uint256 insecure_rand_seed = GetRandHash(); FastRandomContext insecure_rand_ctx(insecure_rand_seed); @@ -113,8 +92,8 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha threadGroup.create_thread(boost::bind(&CScheduler::serviceQueue, &scheduler)); GetMainSignals().RegisterBackgroundSignalScheduler(scheduler); mempool.setSanityCheck(1.0); + g_banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); g_connman = MakeUnique(0x1337, 0x1337); // Deterministic randomness for tests. - connman = g_connman.get(); pblocktree.reset(new CBlockTreeDB(1 << 20, true)); pcoinsdbview.reset(new CCoinsViewDB(1 << 23, true)); g_txindex = MakeUnique(1 << 20, true); @@ -134,7 +113,6 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha constexpr int script_check_threads = 2; StartScriptCheckWorkerThreads(script_check_threads); g_parallel_script_checks = true; - peerLogic.reset(new PeerLogicValidation(connman, scheduler, /*enable_bip61=*/true)); } TestingSetup::~TestingSetup() @@ -150,7 +128,7 @@ TestingSetup::~TestingSetup() GetMainSignals().FlushBackgroundCallbacks(); GetMainSignals().UnregisterBackgroundSignalScheduler(); g_connman.reset(); - peerLogic.reset(); + g_banman.reset(); UnloadBlockIndex(); pcoinsTip.reset(); llmq::DestroyLLMQSystem(); diff --git a/src/test/test_dash.h b/src/test/test_dash.h index 194774356e..5a6ac4ebe4 100644 --- a/src/test/test_dash.h +++ b/src/test/test_dash.h @@ -65,17 +65,11 @@ private: */ class CConnman; class CNode; -struct CConnmanTest { - static void AddNode(CNode& node); - static void ClearNodes(); -}; class PeerLogicValidation; struct TestingSetup: public BasicTestingSetup { boost::thread_group threadGroup; - CConnman* connman; CScheduler scheduler; - std::unique_ptr peerLogic; explicit TestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~TestingSetup(); diff --git a/src/test/test_dash_main.cpp b/src/test/test_dash_main.cpp index a4e6243896..3162371404 100644 --- a/src/test/test_dash_main.cpp +++ b/src/test/test_dash_main.cpp @@ -4,6 +4,7 @@ #define BOOST_TEST_MODULE Dash Test Suite +#include #include #include @@ -13,6 +14,7 @@ #include std::unique_ptr g_connman; +std::unique_ptr g_banman; [[noreturn]] void Shutdown(void* parg) {