From 7cc2b9f6786f9bc33853220551eed33ca6b7b7b2 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 4 Oct 2017 18:25:34 -0400 Subject: [PATCH 01/12] net: Break disconnecting out of Ban() These are separate events which need to be carried out by separate subsystems. This also cleans up some whitespace and tabs in qt to avoid getting flagged by the linter. Current behavior is preserved. --- src/interfaces/node.cpp | 7 +++++++ src/interfaces/node.h | 5 ++++- src/net.cpp | 26 +++++++++++++++++++------- src/net.h | 2 ++ src/net_processing.cpp | 14 +++++++------- src/qt/rpcconsole.cpp | 16 ++++++++-------- src/rpc/net.cpp | 8 +++++++- 7 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index acba05fd5e..09460ec43e 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -144,6 +144,13 @@ public: } 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 7fa5958c51..6aa8ce0797 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -113,7 +113,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 98bd518ecc..6f0d76ccf5 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -554,13 +554,6 @@ void CConnman::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t ba } 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 } @@ -2643,6 +2636,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 430e148afa..c13af50ec4 100644 --- a/src/net.h +++ b/src/net.h @@ -282,6 +282,8 @@ public: size_t GetNodeCount(NumConnections num); 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; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0e222bdfa4..56cd52ed21 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2961,14 +2961,14 @@ static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman, bool en LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode->addr.ToString()); else if (pnode->m_manual_connection) LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode->addr.ToString()); - 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->addr.ToString()); pnode->fDisconnect = true; - if (pnode->addr.IsLocal()) - LogPrintf("Warning: not banning local peer %s!\n", pnode->addr.ToString()); - else - { - connman->Ban(pnode->addr, BanReasonNodeMisbehaving); - } + } else { + // Disconnect and ban all nodes sharing the address + connman->Ban(pnode->addr, BanReasonNodeMisbehaving); + connman->DisconnectNode(pnode->addr); } return true; } diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 2949e43915..2989e1e9e5 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1216,16 +1216,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 6fdf80dc5f..59bc8e8091 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -565,7 +565,13 @@ 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_connman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute); + g_connman->DisconnectNode(subNet); + } else { + g_connman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute); + g_connman->DisconnectNode(netAddr); + } } else if(strCommand == "remove") { From 136bd7926c72659dd277a7b795ea17f72e523338 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 5 Oct 2017 11:52:50 -0400 Subject: [PATCH 02/12] tests: remove member connman/peerLogic in TestingSetup --- src/test/denialofservice_tests.cpp | 38 ++++++++++++++++++++++++++---- src/test/test_bitcoin.cpp | 18 -------------- src/test/test_bitcoin.h | 6 ----- 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 8cf614bc8d..911ba5d9aa 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -20,6 +20,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); @@ -57,6 +74,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(), scheduler, false); // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -109,7 +128,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(g_insecure_rand_ctx.randbits(32)), NODE_NONE); vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", /*fInboundIn=*/ false)); @@ -120,11 +139,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(), scheduler, false); + const Consensus::Params& consensusParams = Params().GetConsensus(); constexpr int nMaxOutbound = 8; CConnman::Options options; @@ -137,7 +159,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) // Mock some outbound peers for (int i=0; iCheckForStaleTipAndEvictPeers(consensusParams); @@ -162,7 +184,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 connman = MakeUnique(0x1337, 0x1337); + auto peerLogic = MakeUnique(connman.get(), scheduler, false); connman->ClearBanned(); CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -246,6 +270,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning) BOOST_AUTO_TEST_CASE(DoS_banscore) { + auto connman = MakeUnique(0x1337, 0x1337); + auto peerLogic = MakeUnique(connman.get(), scheduler, false); connman->ClearBanned(); gArgs.ForceSetArg("-banscore", "111"); // because 11 is my favorite number @@ -290,6 +316,8 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) BOOST_AUTO_TEST_CASE(DoS_bantime) { + auto connman = MakeUnique(0x1337, 0x1337); + auto peerLogic = MakeUnique(connman.get(), scheduler, false); connman->ClearBanned(); int64_t nStartTime = GetTime(); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 858bb512fc..a988a58111 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -24,21 +24,6 @@ const std::function G_TRANSLATION_FUN = nullptr; FastRandomContext g_insecure_rand_ctx; -void CConnmanTest::AddNode(CNode& node) -{ - LOCK(g_connman->cs_vNodes); - g_connman->vNodes.push_back(&node); -} - -void CConnmanTest::ClearNodes() -{ - LOCK(g_connman->cs_vNodes); - for (const CNode* node : g_connman->vNodes) { - delete node; - } - g_connman->vNodes.clear(); -} - std::ostream& operator<<(std::ostream& os, const uint256& num) { os << num.ToString(); @@ -109,8 +94,6 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha for (int i=0; i < nScriptCheckThreads-1; i++) threadGroup.create_thread(&ThreadScriptCheck); g_connman = MakeUnique(0x1337, 0x1337); // Deterministic randomness for tests. - connman = g_connman.get(); - peerLogic.reset(new PeerLogicValidation(connman, scheduler, /*enable_bip61=*/true)); } TestingSetup::~TestingSetup() @@ -120,7 +103,6 @@ TestingSetup::~TestingSetup() GetMainSignals().FlushBackgroundCallbacks(); GetMainSignals().UnregisterBackgroundSignalScheduler(); g_connman.reset(); - peerLogic.reset(); UnloadBlockIndex(); pcoinsTip.reset(); pcoinsdbview.reset(); diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h index 31d90c0151..71520232ac 100644 --- a/src/test/test_bitcoin.h +++ b/src/test/test_bitcoin.h @@ -68,17 +68,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(); From 83c1ea2e5e66b8a83072e3d5ad6a4ced406eb1ba Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 5 Oct 2017 12:46:54 -0400 Subject: [PATCH 03/12] net: split up addresses/ban dumps in preparation for moving them --- src/net.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 6f0d76ccf5..993efc3db9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -41,8 +41,11 @@ #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; + +// Dump addresses to banlist.dat every 15 minutes (900s) +static constexpr int DUMP_BANS_INTERVAL = 60 * 15; // We add a random period time (0 to 1 seconds) to feeler connections to prevent synchronization. #define FEELER_SLEEP_WINDOW 1 @@ -1768,12 +1771,6 @@ void CConnman::DumpAddresses() addrman.size(), GetTimeMillis() - nStart); } -void CConnman::DumpData() -{ - DumpAddresses(); - DumpBanlist(); -} - void CConnman::ProcessOneShot() { std::string strDest; @@ -2450,7 +2447,8 @@ 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); + scheduler.scheduleEvery(std::bind(&CConnman::DumpBanlist, this), DUMP_BANS_INTERVAL * 1000); return true; } @@ -2509,7 +2507,8 @@ void CConnman::Stop() if (fAddressesInitialized) { - DumpData(); + DumpAddresses(); + DumpBanlist(); fAddressesInitialized = false; } From 4c0d961eb0d7825a1e6f8389d7f5545114ee18c6 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 5 Oct 2017 13:10:58 -0400 Subject: [PATCH 04/12] banman: create and split out banman Some say he has always been. --- src/init.cpp | 17 ++++-- src/interfaces/node.cpp | 12 ++--- src/net.cpp | 81 +++++++++++++++------------- src/net.h | 85 +++++++++++++++++------------- src/net_processing.cpp | 16 +++--- src/net_processing.h | 4 +- src/rpc/net.cpp | 37 ++++++++----- src/test/denialofservice_tests.cpp | 41 +++++++------- src/test/test_bitcoin.cpp | 3 ++ src/test/test_bitcoin_main.cpp | 1 + 10 files changed, 172 insertions(+), 125 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index e495a68d55..86b39c34ea 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -73,8 +73,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; #ifdef WIN32 // Win32 LevelDB doesn't use filedescriptors, and the ones used for @@ -199,6 +203,7 @@ void Shutdown(InitInterfaces& interfaces) // 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)) { @@ -1290,11 +1295,12 @@ bool AppInitMain(InitInterfaces& interfaces) // is not yet setup and may end up being set up twice if we // need to reindex later. + assert(!g_banman); + g_banman = MakeUnique(&uiInterface); 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 @@ -1704,6 +1710,7 @@ bool AppInitMain(InitInterfaces& interfaces) 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); @@ -1749,7 +1756,7 @@ bool AppInitMain(InitInterfaces& interfaces) connOptions.m_specified_outgoing = connect; } } - if (!connman.Start(scheduler, connOptions)) { + if (!g_connman->Start(scheduler, connOptions)) { return false; } @@ -1762,5 +1769,9 @@ bool AppInitMain(InitInterfaces& interfaces) client->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 09460ec43e..c535b29315 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -122,24 +122,24 @@ public: } 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; diff --git a/src/net.cpp b/src/net.cpp index 993efc3db9..5479130320 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -44,9 +44,6 @@ // Dump addresses to peers.dat every 15 minutes (900s) static constexpr int DUMP_PEERS_INTERVAL = 15 * 60; -// Dump addresses to banlist.dat every 15 minutes (900s) -static constexpr int DUMP_BANS_INTERVAL = 60 * 15; - // We add a random period time (0 to 1 seconds) to feeler connections to prevent synchronization. #define FEELER_SLEEP_WINDOW 1 @@ -460,7 +457,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo return pnode; } -void CConnman::DumpBanlist() +void BanMan::DumpBanlist() { SweepBanned(); // clean unused entries (if bantime has expired) @@ -491,7 +488,7 @@ void CNode::CloseSocketDisconnect() } } -void CConnman::ClearBanned() +void BanMan::ClearBanned() { { LOCK(cs_setBanned); @@ -503,7 +500,7 @@ void CConnman::ClearBanned() clientInterface->BannedListChanged(); } -bool CConnman::IsBanned(CNetAddr ip) +bool BanMan::IsBanned(CNetAddr ip) { LOCK(cs_setBanned); for (const auto& it : setBanned) { @@ -517,7 +514,7 @@ bool CConnman::IsBanned(CNetAddr ip) return false; } -bool CConnman::IsBanned(CSubNet subnet) +bool BanMan::IsBanned(CSubNet subnet) { LOCK(cs_setBanned); banmap_t::iterator i = setBanned.find(subnet); @@ -531,12 +528,12 @@ bool CConnman::IsBanned(CSubNet subnet) return false; } -void CConnman::Ban(const CNetAddr& addr, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) { +void BanMan::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) { +void BanMan::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) { CBanEntry banEntry(GetTime()); banEntry.banReason = banReason; if (bantimeoffset <= 0) @@ -561,12 +558,12 @@ void CConnman::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t ba DumpBanlist(); //store banlist to disk immediately if user requested ban } -bool CConnman::Unban(const CNetAddr &addr) { +bool BanMan::Unban(const CNetAddr &addr) { CSubNet subNet(addr); return Unban(subNet); } -bool CConnman::Unban(const CSubNet &subNet) { +bool BanMan::Unban(const CSubNet &subNet) { { LOCK(cs_setBanned); if (!setBanned.erase(subNet)) @@ -579,7 +576,7 @@ bool CConnman::Unban(const CSubNet &subNet) { return true; } -void CConnman::GetBanned(banmap_t &banMap) +void BanMan::GetBanned(banmap_t &banMap) { LOCK(cs_setBanned); // Sweep the banlist so expired bans are not returned @@ -587,14 +584,14 @@ void CConnman::GetBanned(banmap_t &banMap) banMap = setBanned; //create a thread safe copy } -void CConnman::SetBanned(const banmap_t &banMap) +void BanMan::SetBanned(const banmap_t &banMap) { LOCK(cs_setBanned); setBanned = banMap; setBannedIsDirty = true; } -void CConnman::SweepBanned() +void BanMan::SweepBanned() { int64_t now = GetTime(); bool notifyUI = false; @@ -622,13 +619,13 @@ void CConnman::SweepBanned() } } -bool CConnman::BannedSetIsDirty() +bool BanMan::BannedSetIsDirty() { LOCK(cs_setBanned); return setBannedIsDirty; } -void CConnman::SetBannedSetDirty(bool dirty) +void BanMan::SetBannedSetDirty(bool dirty) { LOCK(cs_setBanned); //reuse setBanned lock for the isDirty flag setBannedIsDirty = dirty; @@ -1103,7 +1100,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, "connection from %s dropped (banned)\n", addr.ToString()); CloseSocket(hSocket); @@ -2075,7 +2072,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai } if (!pszDest) { if (IsLocal(addrConnect) || - FindNode(static_cast(addrConnect)) || IsBanned(addrConnect) || + FindNode(static_cast(addrConnect)) || (m_banman && m_banman->IsBanned(addrConnect)) || FindNode(addrConnect.ToStringIPPort())) return; } else if (FindNode(std::string(pszDest))) @@ -2376,24 +2373,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...")); @@ -2448,11 +2427,38 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) // Dump network addresses scheduler.scheduleEvery(std::bind(&CConnman::DumpAddresses, this), DUMP_PEERS_INTERVAL * 1000); - scheduler.scheduleEvery(std::bind(&CConnman::DumpBanlist, this), DUMP_BANS_INTERVAL * 1000); return true; } +BanMan::BanMan(CClientUIInterface* client_interface) : clientInterface(client_interface) +{ + if (clientInterface) clientInterface->InitMessage(_("Loading banlist...")); + // Load addresses from banlist.dat + + int64_t nStart = GetTimeMillis(); + setBannedIsDirty = false; + 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(); + } +} + +BanMan::~BanMan() +{ + DumpBanlist(); +} + class CNetCleanup { public: @@ -2508,7 +2514,6 @@ void CConnman::Stop() if (fAddressesInitialized) { DumpAddresses(); - DumpBanlist(); fAddressesInitialized = false; } diff --git a/src/net.h b/src/net.h index c13af50ec4..020e600b2c 100644 --- a/src/net.h +++ b/src/net.h @@ -86,7 +86,7 @@ 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 +static constexpr unsigned int DEFAULT_MISBEHAVING_BANTIME = 60 * 60 * 24; // Default 24-hour ban typedef int64_t NodeId; @@ -114,6 +114,50 @@ struct CSerializedNetMsg std::string command; }; + +class BanMan +{ +public: + // 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. + ~BanMan(); + BanMan(CClientUIInterface* client_interface); + 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 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(); + + banmap_t setBanned; + CCriticalSection cs_setBanned; + bool setBannedIsDirty; + CClientUIInterface* clientInterface = nullptr; +}; + class NetEventsInterface; class CConnman { @@ -136,6 +180,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; @@ -158,6 +203,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; @@ -238,30 +284,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); @@ -370,15 +392,7 @@ private: NodeId GetNewNodeId(); size_t SocketSendData(CNode *pnode) const; - //!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); @@ -411,9 +425,6 @@ private: std::vector vhListenSocket; std::atomic fNetworkActive{true}; - banmap_t setBanned GUARDED_BY(cs_setBanned); - CCriticalSection cs_setBanned; - bool setBannedIsDirty GUARDED_BY(cs_setBanned){false}; bool fAddressesInitialized{false}; CAddrMan addrman; std::deque vOneShots GUARDED_BY(cs_vOneShots); @@ -439,6 +450,7 @@ private: std::atomic nBestHeight; CClientUIInterface* clientInterface; NetEventsInterface* m_msgproc; + BanMan* m_banman; /** SipHasher seeds for deterministic randomness */ const uint64_t nSeed0, nSeed1; @@ -468,6 +480,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 56cd52ed21..6dd6368f74 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -841,9 +841,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)); @@ -2943,7 +2942,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()); @@ -2967,7 +2966,9 @@ static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman, bool en pnode->fDisconnect = true; } else { // Disconnect and ban all nodes sharing the address - connman->Ban(pnode->addr, BanReasonNodeMisbehaving); + if (m_banman) { + m_banman->Ban(pnode->addr, BanReasonNodeMisbehaving); + } connman->DisconnectNode(pnode->addr); } return true; @@ -3092,7 +3093,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; } @@ -3293,8 +3294,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 0113e25f7e..39c22d7118 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -23,9 +23,11 @@ static constexpr bool DEFAULT_ENABLE_BIP61{false}; 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* connman, CScheduler &scheduler, bool enable_bip61); + PeerLogicValidation(CConnman* connman, BanMan* banman, CScheduler &scheduler, bool enable_bip61); /** * Overridden from CValidationInterface. diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 59bc8e8091..19561c4fc7 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -531,8 +531,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; @@ -554,8 +555,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()) @@ -566,17 +568,22 @@ static UniValue setban(const JSONRPCRequest& request) absolute = true; if (isSubnet) { - g_connman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute); - g_connman->DisconnectNode(subNet); + g_banman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute); + if (g_connman) { + g_connman->DisconnectNode(subNet); + } } else { - g_connman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute); - g_connman->DisconnectNode(netAddr); + 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; } @@ -593,11 +600,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) @@ -626,10 +634,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 911ba5d9aa..9776ea223d 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -75,7 +75,7 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup) BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { auto connman = MakeUnique(0x1337, 0x1337); - auto peerLogic = MakeUnique(connman.get(), scheduler, false); + auto peerLogic = MakeUnique(connman.get(), nullptr, scheduler, false); // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -145,7 +145,7 @@ static void AddRandomOutboundPeer(std::vector &vNodes, PeerLogicValidat BOOST_AUTO_TEST_CASE(stale_tip_peer_management) { auto connman = MakeUnique(0x1337, 0x1337); - auto peerLogic = MakeUnique(connman.get(), scheduler, false); + auto peerLogic = MakeUnique(connman.get(), nullptr, scheduler, false); const Consensus::Params& consensusParams = Params().GetConsensus(); constexpr int nMaxOutbound = 8; @@ -216,10 +216,11 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_AUTO_TEST_CASE(DoS_banning) { + auto banman = MakeUnique(nullptr); auto connman = MakeUnique(0x1337, 0x1337); - auto peerLogic = MakeUnique(connman.get(), scheduler, false); + 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); @@ -234,8 +235,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(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); @@ -251,8 +252,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK2(cs_main, dummyNode2.cs_sendProcessing); BOOST_CHECK(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); @@ -261,7 +262,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK2(cs_main, dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); } - BOOST_CHECK(connman->IsBanned(addr2)); + BOOST_CHECK(banman->IsBanned(addr2)); bool dummy; peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); @@ -270,10 +271,11 @@ BOOST_AUTO_TEST_CASE(DoS_banning) BOOST_AUTO_TEST_CASE(DoS_banscore) { + auto banman = MakeUnique(nullptr); auto connman = MakeUnique(0x1337, 0x1337); - auto peerLogic = MakeUnique(connman.get(), scheduler, false); + 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); @@ -289,7 +291,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(!connman->IsBanned(addr1)); + BOOST_CHECK(!banman->IsBanned(addr1)); { LOCK(cs_main); Misbehaving(dummyNode1.GetId(), 10); @@ -298,7 +300,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(!connman->IsBanned(addr1)); + BOOST_CHECK(!banman->IsBanned(addr1)); { LOCK(cs_main); Misbehaving(dummyNode1.GetId(), 1); @@ -307,7 +309,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(connman->IsBanned(addr1)); + BOOST_CHECK(banman->IsBanned(addr1)); gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); bool dummy; @@ -316,10 +318,11 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) BOOST_AUTO_TEST_CASE(DoS_bantime) { + auto banman = MakeUnique(nullptr); auto connman = MakeUnique(0x1337, 0x1337); - auto peerLogic = MakeUnique(connman.get(), scheduler, false); + auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); - connman->ClearBanned(); + banman->ClearBanned(); int64_t nStartTime = GetTime(); SetMockTime(nStartTime); // Overrides future calls to GetTime() @@ -338,13 +341,13 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) LOCK2(cs_main, dummyNode.cs_sendProcessing); BOOST_CHECK(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_bitcoin.cpp b/src/test/test_bitcoin.cpp index a988a58111..6d1f70f9a1 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -93,6 +93,8 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha nScriptCheckThreads = 3; for (int i=0; i < nScriptCheckThreads-1; i++) threadGroup.create_thread(&ThreadScriptCheck); + + g_banman = MakeUnique(nullptr); g_connman = MakeUnique(0x1337, 0x1337); // Deterministic randomness for tests. } @@ -103,6 +105,7 @@ TestingSetup::~TestingSetup() GetMainSignals().FlushBackgroundCallbacks(); GetMainSignals().UnregisterBackgroundSignalScheduler(); g_connman.reset(); + g_banman.reset(); UnloadBlockIndex(); pcoinsTip.reset(); pcoinsdbview.reset(); diff --git a/src/test/test_bitcoin_main.cpp b/src/test/test_bitcoin_main.cpp index 6c066d3fea..caa35c0efc 100644 --- a/src/test/test_bitcoin_main.cpp +++ b/src/test/test_bitcoin_main.cpp @@ -11,6 +11,7 @@ #include std::unique_ptr g_connman; +std::unique_ptr g_banman; [[noreturn]] void Shutdown(void* parg) { From 2e56702ecedd83c4b7cb8de9de5c437c8c08e645 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 5 Oct 2017 13:35:20 -0400 Subject: [PATCH 05/12] banman: pass the banfile path in There's no need to hard-code the path here. Passing it in means that there are no ordering concerns wrt establishing the datadir. --- src/addrdb.cpp | 7 +++---- src/addrdb.h | 4 ++-- src/init.cpp | 2 +- src/net.cpp | 9 +++------ src/net.h | 3 ++- src/test/denialofservice_tests.cpp | 6 +++--- src/test/test_bitcoin.cpp | 2 +- 7 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 1590bce074..c6083f5554 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -105,19 +105,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 90eca44bdb..88da31a6fc 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -92,9 +92,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/init.cpp b/src/init.cpp index 86b39c34ea..ded157d49e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1296,7 +1296,7 @@ bool AppInitMain(InitInterfaces& interfaces) // need to reindex later. assert(!g_banman); - g_banman = MakeUnique(&uiInterface); + g_banman = MakeUnique(GetDataDir() / "banlist.dat", &uiInterface); assert(!g_connman); g_connman = std::unique_ptr(new CConnman(GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max()))); diff --git a/src/net.cpp b/src/net.cpp index 5479130320..fd74c58292 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -466,10 +466,9 @@ void BanMan::DumpBanlist() int64_t nStart = GetTimeMillis(); - CBanDB bandb; banmap_t banmap; GetBanned(banmap); - if (bandb.Write(banmap)) { + if (m_ban_db.Write(banmap)) { SetBannedSetDirty(false); } @@ -2431,16 +2430,14 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) return true; } -BanMan::BanMan(CClientUIInterface* client_interface) : clientInterface(client_interface) +BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface) : clientInterface(client_interface), m_ban_db(std::move(ban_file)) { if (clientInterface) clientInterface->InitMessage(_("Loading banlist...")); - // Load addresses from banlist.dat int64_t nStart = GetTimeMillis(); setBannedIsDirty = false; - CBanDB bandb; banmap_t banmap; - if (bandb.Read(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 diff --git a/src/net.h b/src/net.h index 020e600b2c..5e6fc14b38 100644 --- a/src/net.h +++ b/src/net.h @@ -133,7 +133,7 @@ public: // between nodes running old code and nodes running // new code. ~BanMan(); - BanMan(CClientUIInterface* client_interface); + BanMan(fs::path ban_file, CClientUIInterface* client_interface); 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 @@ -156,6 +156,7 @@ private: CCriticalSection cs_setBanned; bool setBannedIsDirty; CClientUIInterface* clientInterface = nullptr; + CBanDB m_ban_db; }; class NetEventsInterface; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 9776ea223d..4a88613a89 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -216,7 +216,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_AUTO_TEST_CASE(DoS_banning) { - auto banman = MakeUnique(nullptr); + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); auto connman = MakeUnique(0x1337, 0x1337); auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); @@ -271,7 +271,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) BOOST_AUTO_TEST_CASE(DoS_banscore) { - auto banman = MakeUnique(nullptr); + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); auto connman = MakeUnique(0x1337, 0x1337); auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); @@ -318,7 +318,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) BOOST_AUTO_TEST_CASE(DoS_bantime) { - auto banman = MakeUnique(nullptr); + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); auto connman = MakeUnique(0x1337, 0x1337); auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 6d1f70f9a1..7bce74b46b 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -94,7 +94,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha for (int i=0; i < nScriptCheckThreads-1; i++) threadGroup.create_thread(&ThreadScriptCheck); - g_banman = MakeUnique(nullptr); + g_banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); g_connman = MakeUnique(0x1337, 0x1337); // Deterministic randomness for tests. } From d0469b2e9386a7a4b268cb9725347e7517acace6 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 5 Oct 2017 13:41:45 -0400 Subject: [PATCH 06/12] banman: pass in default ban time as a parameter Removes the dependency on arg parsing. --- src/init.cpp | 2 +- src/net.cpp | 4 ++-- src/net.h | 3 ++- src/test/denialofservice_tests.cpp | 6 +++--- src/test/test_bitcoin.cpp | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index ded157d49e..0f59dfbef5 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1296,7 +1296,7 @@ bool AppInitMain(InitInterfaces& interfaces) // need to reindex later. assert(!g_banman); - g_banman = MakeUnique(GetDataDir() / "banlist.dat", &uiInterface); + 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()))); diff --git a/src/net.cpp b/src/net.cpp index fd74c58292..f6491bc8db 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -537,7 +537,7 @@ void BanMan::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t bant banEntry.banReason = banReason; if (bantimeoffset <= 0) { - bantimeoffset = gArgs.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME); + bantimeoffset = m_default_ban_time; sinceUnixEpoch = false; } banEntry.nBanUntil = (sinceUnixEpoch ? 0 : GetTime() )+bantimeoffset; @@ -2430,7 +2430,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) return true; } -BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface) : clientInterface(client_interface), m_ban_db(std::move(ban_file)) +BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time) : clientInterface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time) { if (clientInterface) clientInterface->InitMessage(_("Loading banlist...")); diff --git a/src/net.h b/src/net.h index 5e6fc14b38..a6a536d68a 100644 --- a/src/net.h +++ b/src/net.h @@ -133,7 +133,7 @@ public: // between nodes running old code and nodes running // new code. ~BanMan(); - BanMan(fs::path ban_file, CClientUIInterface* client_interface); + BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time); 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 @@ -157,6 +157,7 @@ private: bool setBannedIsDirty; CClientUIInterface* clientInterface = nullptr; CBanDB m_ban_db; + int64_t m_default_ban_time; }; class NetEventsInterface; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 4a88613a89..29bb34bddc 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -216,7 +216,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_AUTO_TEST_CASE(DoS_banning) { - auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = MakeUnique(0x1337, 0x1337); auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); @@ -271,7 +271,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) BOOST_AUTO_TEST_CASE(DoS_banscore) { - auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = MakeUnique(0x1337, 0x1337); auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); @@ -318,7 +318,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) BOOST_AUTO_TEST_CASE(DoS_bantime) { - auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = MakeUnique(0x1337, 0x1337); auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 7bce74b46b..75bd8db67e 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -94,7 +94,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha for (int i=0; i < nScriptCheckThreads-1; i++) threadGroup.create_thread(&ThreadScriptCheck); - g_banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); + g_banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); g_connman = MakeUnique(0x1337, 0x1337); // Deterministic randomness for tests. } From af3503d903b1a608cd212e2d74b274103199078c Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 5 Oct 2017 16:40:43 -0400 Subject: [PATCH 07/12] net: move BanMan to its own files --- src/Makefile.am | 2 + src/banman.cpp | 195 +++++++++++++++++++++++++++++ src/banman.h | 69 ++++++++++ src/init.cpp | 1 + src/interfaces/node.cpp | 1 + src/interfaces/node.h | 1 + src/net.cpp | 190 +--------------------------- src/net.h | 49 +------- src/net_processing.cpp | 1 + src/rpc/net.cpp | 1 + src/test/denialofservice_tests.cpp | 1 + src/test/test_bitcoin.cpp | 1 + src/test/test_bitcoin_main.cpp | 1 + 13 files changed, 276 insertions(+), 237 deletions(-) create mode 100644 src/banman.cpp create mode 100644 src/banman.h diff --git a/src/Makefile.am b/src/Makefile.am index 09daaebd23..4b07f06c95 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -96,6 +96,7 @@ BITCOIN_CORE_H = \ addrdb.h \ addrman.h \ attributes.h \ + banman.h \ base58.h \ bech32.h \ bloom.h \ @@ -225,6 +226,7 @@ libbitcoin_server_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libbitcoin_server_a_SOURCES = \ addrdb.cpp \ addrman.cpp \ + banman.cpp \ bloom.cpp \ blockencodings.cpp \ blockfilter.cpp \ diff --git a/src/banman.cpp b/src/banman.cpp new file mode 100644 index 0000000000..59cfe26318 --- /dev/null +++ b/src/banman.cpp @@ -0,0 +1,195 @@ +// 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) + : clientInterface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time) +{ + if (clientInterface) clientInterface->InitMessage(_("Loading banlist...")); + + int64_t nStart = GetTimeMillis(); + setBannedIsDirty = 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() - nStart); + } 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 nStart = 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() - nStart); +} + +void BanMan::ClearBanned() +{ + { + LOCK(cs_setBanned); + setBanned.clear(); + setBannedIsDirty = true; + } + DumpBanlist(); //store banlist to disk + if (clientInterface) clientInterface->BannedListChanged(); +} + +bool BanMan::IsBanned(CNetAddr netAddr) +{ + LOCK(cs_setBanned); + for (const auto& it : setBanned) { + CSubNet subNet = it.first; + CBanEntry banEntry = it.second; + + if (subNet.Match(netAddr) && GetTime() < banEntry.nBanUntil) { + return true; + } + } + return false; +} + +bool BanMan::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 BanMan::Ban(const CNetAddr& netAddr, const BanReason& banReason, int64_t bantimeoffset, bool sinceUnixEpoch) +{ + CSubNet subNet(netAddr); + Ban(subNet, banReason, bantimeoffset, sinceUnixEpoch); +} + +void BanMan::Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bantimeoffset, bool sinceUnixEpoch) +{ + CBanEntry banEntry(GetTime()); + banEntry.banReason = banReason; + if (bantimeoffset <= 0) { + bantimeoffset = m_default_ban_time; + 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(); + + //store banlist to disk immediately if user requested ban + if (banReason == BanReasonManuallyAdded) DumpBanlist(); +} + +bool BanMan::Unban(const CNetAddr& netAddr) +{ + CSubNet subNet(netAddr); + return Unban(subNet); +} + +bool BanMan::Unban(const CSubNet& subNet) +{ + { + LOCK(cs_setBanned); + if (setBanned.erase(subNet) == 0) return false; + setBannedIsDirty = true; + } + if (clientInterface) clientInterface->BannedListChanged(); + DumpBanlist(); //store banlist to disk immediately + return true; +} + +void BanMan::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 BanMan::SetBanned(const banmap_t& banMap) +{ + LOCK(cs_setBanned); + setBanned = banMap; + setBannedIsDirty = true; +} + +void BanMan::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 BanMan::BannedSetIsDirty() +{ + LOCK(cs_setBanned); + return setBannedIsDirty; +} + +void BanMan::SetBannedSetDirty(bool dirty) +{ + LOCK(cs_setBanned); //reuse setBanned lock for the setBannedIsDirty flag + setBannedIsDirty = dirty; +} diff --git a/src/banman.h b/src/banman.h new file mode 100644 index 0000000000..898ae85197 --- /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& netAddr, const BanReason& banReason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); + void Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); + void ClearBanned(); // needed for unit testing + bool IsBanned(CNetAddr netAddr); + bool IsBanned(CSubNet subNet); + bool Unban(const CNetAddr& netAddr); + bool Unban(const CSubNet& subNet); + 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(); + + banmap_t setBanned; + CCriticalSection cs_setBanned; + bool setBannedIsDirty; + CClientUIInterface* clientInterface = nullptr; + CBanDB m_ban_db; + int64_t m_default_ban_time; +}; + +extern std::unique_ptr g_banman; +#endif diff --git a/src/init.cpp b/src/init.cpp index 0f59dfbef5..77d0505610 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index c535b29315..c574f960e6 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 6aa8ce0797..54c2d78338 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -18,6 +18,7 @@ #include #include +class BanMan; class CCoinControl; class CFeeRate; class CNodeStats; diff --git a/src/net.cpp b/src/net.cpp index f6491bc8db..0490ccd6db 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -9,6 +9,7 @@ #include +#include #include #include #include @@ -457,25 +458,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo return pnode; } -void BanMan::DumpBanlist() -{ - SweepBanned(); // clean unused entries (if bantime has expired) - - if (!BannedSetIsDirty()) - return; - - int64_t nStart = 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() - nStart); -} - void CNode::CloseSocketDisconnect() { fDisconnect = true; @@ -487,150 +469,6 @@ void CNode::CloseSocketDisconnect() } } -void BanMan::ClearBanned() -{ - { - LOCK(cs_setBanned); - setBanned.clear(); - setBannedIsDirty = true; - } - DumpBanlist(); //store banlist to disk - if(clientInterface) - clientInterface->BannedListChanged(); -} - -bool BanMan::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 BanMan::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 BanMan::Ban(const CNetAddr& addr, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) { - CSubNet subNet(addr); - Ban(subNet, banReason, bantimeoffset, sinceUnixEpoch); -} - -void BanMan::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) { - CBanEntry banEntry(GetTime()); - banEntry.banReason = banReason; - if (bantimeoffset <= 0) - { - bantimeoffset = m_default_ban_time; - 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(); - if(banReason == BanReasonManuallyAdded) - DumpBanlist(); //store banlist to disk immediately if user requested ban -} - -bool BanMan::Unban(const CNetAddr &addr) { - CSubNet subNet(addr); - return Unban(subNet); -} - -bool BanMan::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 BanMan::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 BanMan::SetBanned(const banmap_t &banMap) -{ - LOCK(cs_setBanned); - setBanned = banMap; - setBannedIsDirty = true; -} - -void BanMan::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 BanMan::BannedSetIsDirty() -{ - LOCK(cs_setBanned); - return setBannedIsDirty; -} - -void BanMan::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)) @@ -2430,32 +2268,6 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) return true; } -BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time) : clientInterface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time) -{ - if (clientInterface) clientInterface->InitMessage(_("Loading banlist...")); - - int64_t nStart = GetTimeMillis(); - setBannedIsDirty = 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() - nStart); - } else { - LogPrintf("Invalid or missing banlist.dat; recreating\n"); - SetBannedSetDirty(true); // force write - DumpBanlist(); - } -} - -BanMan::~BanMan() -{ - DumpBanlist(); -} - class CNetCleanup { public: diff --git a/src/net.h b/src/net.h index a6a536d68a..3606b4d7ba 100644 --- a/src/net.h +++ b/src/net.h @@ -37,6 +37,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; @@ -85,9 +86,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 constexpr unsigned int DEFAULT_MISBEHAVING_BANTIME = 60 * 60 * 24; // Default 24-hour ban - typedef int64_t NodeId; struct AddedNodeInfo @@ -115,51 +113,6 @@ struct CSerializedNetMsg }; -class BanMan -{ -public: - // 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. - ~BanMan(); - BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time); - 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 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(); - - banmap_t setBanned; - CCriticalSection cs_setBanned; - bool setBannedIsDirty; - CClientUIInterface* clientInterface = nullptr; - CBanDB m_ban_db; - int64_t m_default_ban_time; -}; - class NetEventsInterface; class CConnman { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6dd6368f74..62b7d4e966 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 19561c4fc7..7994d3b125 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 29bb34bddc..e5d62a3ab2 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 diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 75bd8db67e..ad7fa01710 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include diff --git a/src/test/test_bitcoin_main.cpp b/src/test/test_bitcoin_main.cpp index caa35c0efc..46b63b93b4 100644 --- a/src/test/test_bitcoin_main.cpp +++ b/src/test/test_bitcoin_main.cpp @@ -4,6 +4,7 @@ #define BOOST_TEST_MODULE Bitcoin Test Suite +#include #include #include From 84fc3fbd0304a7d6e660bf783c84bed2dd415141 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 5 Oct 2017 17:04:14 -0400 Subject: [PATCH 08/12] scripted-diff: batch-rename BanMan members -BEGIN VERIFY SCRIPT- sed -i "s/clientInterface/m_client_interface/g" src/banman.h src/banman.cpp sed -i "s/setBannedIsDirty/m_is_dirty/g" src/banman.h src/banman.cpp sed -i "s/cs_setBanned/m_cs_banned/g" src/banman.h src/banman.cpp sed -i "s/setBanned/m_banned/g" src/banman.h src/banman.cpp -END VERIFY SCRIPT- --- src/banman.cpp | 74 +++++++++++++++++++++++++------------------------- src/banman.h | 8 +++--- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/src/banman.cpp b/src/banman.cpp index 59cfe26318..dc16aab282 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -12,12 +12,12 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time) - : clientInterface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time) + : m_client_interface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time) { - if (clientInterface) clientInterface->InitMessage(_("Loading banlist...")); + if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist...")); int64_t nStart = GetTimeMillis(); - setBannedIsDirty = false; + m_is_dirty = false; banmap_t banmap; if (m_ban_db.Read(banmap)) { SetBanned(banmap); // thread save setter @@ -59,18 +59,18 @@ void BanMan::DumpBanlist() void BanMan::ClearBanned() { { - LOCK(cs_setBanned); - setBanned.clear(); - setBannedIsDirty = true; + LOCK(m_cs_banned); + m_banned.clear(); + m_is_dirty = true; } DumpBanlist(); //store banlist to disk - if (clientInterface) clientInterface->BannedListChanged(); + if (m_client_interface) m_client_interface->BannedListChanged(); } bool BanMan::IsBanned(CNetAddr netAddr) { - LOCK(cs_setBanned); - for (const auto& it : setBanned) { + LOCK(m_cs_banned); + for (const auto& it : m_banned) { CSubNet subNet = it.first; CBanEntry banEntry = it.second; @@ -83,9 +83,9 @@ bool BanMan::IsBanned(CNetAddr netAddr) bool BanMan::IsBanned(CSubNet subNet) { - LOCK(cs_setBanned); - banmap_t::iterator i = setBanned.find(subNet); - if (i != setBanned.end()) { + LOCK(m_cs_banned); + banmap_t::iterator i = m_banned.find(subNet); + if (i != m_banned.end()) { CBanEntry banEntry = (*i).second; if (GetTime() < banEntry.nBanUntil) { return true; @@ -111,14 +111,14 @@ void BanMan::Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bant banEntry.nBanUntil = (sinceUnixEpoch ? 0 : GetTime()) + bantimeoffset; { - LOCK(cs_setBanned); - if (setBanned[subNet].nBanUntil < banEntry.nBanUntil) { - setBanned[subNet] = banEntry; - setBannedIsDirty = true; + LOCK(m_cs_banned); + if (m_banned[subNet].nBanUntil < banEntry.nBanUntil) { + m_banned[subNet] = banEntry; + m_is_dirty = true; } else return; } - if (clientInterface) clientInterface->BannedListChanged(); + if (m_client_interface) m_client_interface->BannedListChanged(); //store banlist to disk immediately if user requested ban if (banReason == BanReasonManuallyAdded) DumpBanlist(); @@ -133,28 +133,28 @@ bool BanMan::Unban(const CNetAddr& netAddr) bool BanMan::Unban(const CSubNet& subNet) { { - LOCK(cs_setBanned); - if (setBanned.erase(subNet) == 0) return false; - setBannedIsDirty = true; + LOCK(m_cs_banned); + if (m_banned.erase(subNet) == 0) return false; + m_is_dirty = true; } - if (clientInterface) clientInterface->BannedListChanged(); + if (m_client_interface) m_client_interface->BannedListChanged(); DumpBanlist(); //store banlist to disk immediately return true; } void BanMan::GetBanned(banmap_t& banMap) { - LOCK(cs_setBanned); + LOCK(m_cs_banned); // Sweep the banlist so expired bans are not returned SweepBanned(); - banMap = setBanned; //create a thread safe copy + banMap = m_banned; //create a thread safe copy } void BanMan::SetBanned(const banmap_t& banMap) { - LOCK(cs_setBanned); - setBanned = banMap; - setBannedIsDirty = true; + LOCK(m_cs_banned); + m_banned = banMap; + m_is_dirty = true; } void BanMan::SweepBanned() @@ -162,14 +162,14 @@ void BanMan::SweepBanned() int64_t now = GetTime(); bool notifyUI = false; { - LOCK(cs_setBanned); - banmap_t::iterator it = setBanned.begin(); - while (it != setBanned.end()) { + LOCK(m_cs_banned); + banmap_t::iterator it = m_banned.begin(); + while (it != m_banned.end()) { CSubNet subNet = (*it).first; CBanEntry banEntry = (*it).second; if (now > banEntry.nBanUntil) { - setBanned.erase(it++); - setBannedIsDirty = true; + m_banned.erase(it++); + m_is_dirty = true; notifyUI = true; LogPrint(BCLog::NET, "%s: Removed banned node ip/subnet from banlist.dat: %s\n", __func__, subNet.ToString()); } else @@ -177,19 +177,19 @@ void BanMan::SweepBanned() } } // update UI - if (notifyUI && clientInterface) { - clientInterface->BannedListChanged(); + if (notifyUI && m_client_interface) { + m_client_interface->BannedListChanged(); } } bool BanMan::BannedSetIsDirty() { - LOCK(cs_setBanned); - return setBannedIsDirty; + LOCK(m_cs_banned); + return m_is_dirty; } void BanMan::SetBannedSetDirty(bool dirty) { - LOCK(cs_setBanned); //reuse setBanned lock for the setBannedIsDirty flag - setBannedIsDirty = 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 index 898ae85197..9339717158 100644 --- a/src/banman.h +++ b/src/banman.h @@ -57,10 +57,10 @@ private: //!clean unused entries (if bantime has expired) void SweepBanned(); - banmap_t setBanned; - CCriticalSection cs_setBanned; - bool setBannedIsDirty; - CClientUIInterface* clientInterface = nullptr; + banmap_t m_banned; + CCriticalSection m_cs_banned; + bool m_is_dirty; + CClientUIInterface* m_client_interface = nullptr; CBanDB m_ban_db; int64_t m_default_ban_time; }; From daae598feb034f2f56e0b00ecfb4854d693d3641 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Fri, 6 Oct 2017 16:56:56 -0400 Subject: [PATCH 09/12] banman: add thread annotations and mark members const where possible Also remove misleading comment. ClearBanned is used by rpc as well. --- src/banman.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/banman.h b/src/banman.h index 9339717158..61200dbe96 100644 --- a/src/banman.h +++ b/src/banman.h @@ -41,7 +41,7 @@ public: BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time); void Ban(const CNetAddr& netAddr, const BanReason& banReason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); void Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); - void ClearBanned(); // needed for unit testing + void ClearBanned(); bool IsBanned(CNetAddr netAddr); bool IsBanned(CSubNet subNet); bool Unban(const CNetAddr& netAddr); @@ -57,12 +57,12 @@ private: //!clean unused entries (if bantime has expired) void SweepBanned(); - banmap_t m_banned; CCriticalSection m_cs_banned; - bool m_is_dirty; + 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; - int64_t m_default_ban_time; + const int64_t m_default_ban_time; }; extern std::unique_ptr g_banman; From 1ffa4ce27d4ea6c1067d8984455df97994c7713e Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 30 Oct 2018 21:13:13 -0700 Subject: [PATCH 10/12] banman: reformulate nBanUtil calculation Avoid reassigning parameters. --- src/banman.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/banman.cpp b/src/banman.cpp index dc16aab282..19c8e37523 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -104,11 +104,14 @@ void BanMan::Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bant { CBanEntry banEntry(GetTime()); banEntry.banReason = banReason; + + int64_t normalized_bantimeoffset = bantimeoffset; + bool normalized_sinceUnixEpoch = sinceUnixEpoch; if (bantimeoffset <= 0) { - bantimeoffset = m_default_ban_time; - sinceUnixEpoch = false; + normalized_bantimeoffset = m_default_ban_time; + normalized_sinceUnixEpoch = false; } - banEntry.nBanUntil = (sinceUnixEpoch ? 0 : GetTime()) + bantimeoffset; + banEntry.nBanUntil = (normalized_sinceUnixEpoch ? 0 : GetTime()) + normalized_bantimeoffset; { LOCK(m_cs_banned); From c2e04d37f3841d109c1fe60693f9622e2836cc29 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Thu, 3 Jan 2019 21:26:10 +0800 Subject: [PATCH 11/12] banman: Add, use CBanEntry ctor that takes ban reason --- src/addrdb.h | 5 +++++ src/banman.cpp | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/addrdb.h b/src/addrdb.h index 88da31a6fc..290b63dd12 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; + } + ADD_SERIALIZE_METHODS; template diff --git a/src/banman.cpp b/src/banman.cpp index 19c8e37523..56cbe941c6 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -102,8 +102,7 @@ void BanMan::Ban(const CNetAddr& netAddr, const BanReason& banReason, int64_t ba void BanMan::Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bantimeoffset, bool sinceUnixEpoch) { - CBanEntry banEntry(GetTime()); - banEntry.banReason = banReason; + CBanEntry banEntry(GetTime(), banReason); int64_t normalized_bantimeoffset = bantimeoffset; bool normalized_sinceUnixEpoch = sinceUnixEpoch; From 18185b57c32d0a43afeca4c125b9352c692923e9 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 30 Oct 2018 21:33:27 -0700 Subject: [PATCH 12/12] scripted-diff: batch-recase BanMan variables -BEGIN VERIFY SCRIPT- sed -i "s/banMap/banmap/g" src/banman.h src/banman.cpp sed -i "s/netAddr/net_addr/g" src/banman.h src/banman.cpp sed -i "s/sinceUnixEpoch/since_unix_epoch/g" src/banman.h src/banman.cpp sed -i "s/bantimeoffset/ban_time_offset/g" src/banman.h src/banman.cpp sed -i "s/subNet/sub_net/g" src/banman.h src/banman.cpp sed -i "s/banReason/ban_reason/g" src/banman.h src/banman.cpp sed -i "s/notifyUI/notify_ui/g" src/banman.h src/banman.cpp sed -i "s/banEntry/ban_entry/g" src/banman.h src/banman.cpp sed -i "s/nStart/n_start/g" src/banman.h src/banman.cpp -END VERIFY SCRIPT- --- src/banman.cpp | 84 +++++++++++++++++++++++++------------------------- src/banman.h | 16 +++++----- 2 files changed, 50 insertions(+), 50 deletions(-) diff --git a/src/banman.cpp b/src/banman.cpp index 56cbe941c6..9933c829c5 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -16,7 +16,7 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t { if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist...")); - int64_t nStart = GetTimeMillis(); + int64_t n_start = GetTimeMillis(); m_is_dirty = false; banmap_t banmap; if (m_ban_db.Read(banmap)) { @@ -25,7 +25,7 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t SweepBanned(); // sweep out unused entries LogPrint(BCLog::NET, "Loaded %d banned node ips/subnets from banlist.dat %dms\n", - banmap.size(), GetTimeMillis() - nStart); + banmap.size(), GetTimeMillis() - n_start); } else { LogPrintf("Invalid or missing banlist.dat; recreating\n"); SetBannedSetDirty(true); // force write @@ -44,7 +44,7 @@ void BanMan::DumpBanlist() if (!BannedSetIsDirty()) return; - int64_t nStart = GetTimeMillis(); + int64_t n_start = GetTimeMillis(); banmap_t banmap; GetBanned(banmap); @@ -53,7 +53,7 @@ void BanMan::DumpBanlist() } LogPrint(BCLog::NET, "Flushed %d banned node ips/subnets to banlist.dat %dms\n", - banmap.size(), GetTimeMillis() - nStart); + banmap.size(), GetTimeMillis() - n_start); } void BanMan::ClearBanned() @@ -67,55 +67,55 @@ void BanMan::ClearBanned() if (m_client_interface) m_client_interface->BannedListChanged(); } -bool BanMan::IsBanned(CNetAddr netAddr) +bool BanMan::IsBanned(CNetAddr net_addr) { LOCK(m_cs_banned); for (const auto& it : m_banned) { - CSubNet subNet = it.first; - CBanEntry banEntry = it.second; + CSubNet sub_net = it.first; + CBanEntry ban_entry = it.second; - if (subNet.Match(netAddr) && GetTime() < banEntry.nBanUntil) { + if (sub_net.Match(net_addr) && GetTime() < ban_entry.nBanUntil) { return true; } } return false; } -bool BanMan::IsBanned(CSubNet subNet) +bool BanMan::IsBanned(CSubNet sub_net) { LOCK(m_cs_banned); - banmap_t::iterator i = m_banned.find(subNet); + banmap_t::iterator i = m_banned.find(sub_net); if (i != m_banned.end()) { - CBanEntry banEntry = (*i).second; - if (GetTime() < banEntry.nBanUntil) { + CBanEntry ban_entry = (*i).second; + if (GetTime() < ban_entry.nBanUntil) { return true; } } return false; } -void BanMan::Ban(const CNetAddr& netAddr, const BanReason& banReason, int64_t bantimeoffset, bool sinceUnixEpoch) +void BanMan::Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch) { - CSubNet subNet(netAddr); - Ban(subNet, banReason, bantimeoffset, sinceUnixEpoch); + CSubNet sub_net(net_addr); + Ban(sub_net, ban_reason, ban_time_offset, since_unix_epoch); } -void BanMan::Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bantimeoffset, bool sinceUnixEpoch) +void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch) { - CBanEntry banEntry(GetTime(), banReason); + CBanEntry ban_entry(GetTime(), ban_reason); - int64_t normalized_bantimeoffset = bantimeoffset; - bool normalized_sinceUnixEpoch = sinceUnixEpoch; - if (bantimeoffset <= 0) { - normalized_bantimeoffset = m_default_ban_time; - normalized_sinceUnixEpoch = false; + 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; } - banEntry.nBanUntil = (normalized_sinceUnixEpoch ? 0 : GetTime()) + normalized_bantimeoffset; + ban_entry.nBanUntil = (normalized_since_unix_epoch ? 0 : GetTime()) + normalized_ban_time_offset; { LOCK(m_cs_banned); - if (m_banned[subNet].nBanUntil < banEntry.nBanUntil) { - m_banned[subNet] = banEntry; + if (m_banned[sub_net].nBanUntil < ban_entry.nBanUntil) { + m_banned[sub_net] = ban_entry; m_is_dirty = true; } else return; @@ -123,20 +123,20 @@ void BanMan::Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bant if (m_client_interface) m_client_interface->BannedListChanged(); //store banlist to disk immediately if user requested ban - if (banReason == BanReasonManuallyAdded) DumpBanlist(); + if (ban_reason == BanReasonManuallyAdded) DumpBanlist(); } -bool BanMan::Unban(const CNetAddr& netAddr) +bool BanMan::Unban(const CNetAddr& net_addr) { - CSubNet subNet(netAddr); - return Unban(subNet); + CSubNet sub_net(net_addr); + return Unban(sub_net); } -bool BanMan::Unban(const CSubNet& subNet) +bool BanMan::Unban(const CSubNet& sub_net) { { LOCK(m_cs_banned); - if (m_banned.erase(subNet) == 0) return false; + if (m_banned.erase(sub_net) == 0) return false; m_is_dirty = true; } if (m_client_interface) m_client_interface->BannedListChanged(); @@ -144,42 +144,42 @@ bool BanMan::Unban(const CSubNet& subNet) return true; } -void BanMan::GetBanned(banmap_t& banMap) +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 + banmap = m_banned; //create a thread safe copy } -void BanMan::SetBanned(const banmap_t& banMap) +void BanMan::SetBanned(const banmap_t& banmap) { LOCK(m_cs_banned); - m_banned = banMap; + m_banned = banmap; m_is_dirty = true; } void BanMan::SweepBanned() { int64_t now = GetTime(); - bool notifyUI = false; + bool notify_ui = false; { LOCK(m_cs_banned); banmap_t::iterator it = m_banned.begin(); while (it != m_banned.end()) { - CSubNet subNet = (*it).first; - CBanEntry banEntry = (*it).second; - if (now > banEntry.nBanUntil) { + CSubNet sub_net = (*it).first; + CBanEntry ban_entry = (*it).second; + if (now > ban_entry.nBanUntil) { m_banned.erase(it++); m_is_dirty = true; - notifyUI = true; - LogPrint(BCLog::NET, "%s: Removed banned node ip/subnet from banlist.dat: %s\n", __func__, subNet.ToString()); + 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 (notifyUI && m_client_interface) { + if (notify_ui && m_client_interface) { m_client_interface->BannedListChanged(); } } diff --git a/src/banman.h b/src/banman.h index 61200dbe96..69f62be368 100644 --- a/src/banman.h +++ b/src/banman.h @@ -39,18 +39,18 @@ class BanMan public: ~BanMan(); BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time); - void Ban(const CNetAddr& netAddr, const BanReason& banReason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); - void Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); + 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 netAddr); - bool IsBanned(CSubNet subNet); - bool Unban(const CNetAddr& netAddr); - bool Unban(const CSubNet& subNet); - void GetBanned(banmap_t& banMap); + 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); + void SetBanned(const banmap_t& banmap); bool BannedSetIsDirty(); //!set the "dirty" flag for the banlist void SetBannedSetDirty(bool dirty = true);