From 7cc2b9f6786f9bc33853220551eed33ca6b7b7b2 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 4 Oct 2017 18:25:34 -0400 Subject: [PATCH] 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 acba05fd5ef..09460ec43ec 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 7fa5958c51c..6aa8ce0797f 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 98bd518ecc4..6f0d76ccf5d 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 430e148afa2..c13af50ec44 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 0e222bdfa47..56cd52ed21a 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 2949e439159..2989e1e9e55 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 6fdf80dc5f9..59bc8e80918 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") {