Merge #14929: net: Allow connections from misbehavior banned peers

0297be61a Allow connections from misbehavior banned peers. (Gregory Maxwell)

Pull request description:

  This allows incoming connections from peers which are only banned
   due to an automatic misbehavior ban if doing so won't fill inbound.

  These peers are preferred for eviction when inbound fills, but may
   still be kept if they fall into the protected classes.  This
   eviction preference lasts the entire life of the connection even
   if the ban expires.

  If they misbehave again they'll still get disconnected.

  The main purpose of banning on misbehavior is to prevent our
   connections from being wasted on unhelpful peers such as ones
   running incompatible consensus rules.  For inbound peers this
   can be better accomplished with eviction preferences.

  A secondary purpose was to reduce resource waste from repeated
   abuse but virtually any attacker can get a nearly unlimited
   supply of addresses, so disconnection is about the best we can
   do.

  This can reduce the potential from negative impact due to incorrect misbehaviour bans.

Tree-SHA512: 03bc8ec8bae365cc437daf70000c8f2edc512e37db821bc4e0fafa6cf56cc185e9ab40453aa02445f48d6a2e3e7268767ca2017655aca5383108416f1e2cf20f

4 files changed, 44 insertions, 4 deletions

diff --git a/src/banman.cpp b/src/banman.cpp
index 9933c829c5..47d64a8f31 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -67,14 +67,36 @@ void BanMan::ClearBanned()
     if (m_client_interface) m_client_interface->BannedListChanged();
 }
 
+int BanMan::IsBannedLevel(CNetAddr net_addr)
+{
+    // Returns the most severe level of banning that applies to this address.
+    // 0 - Not banned
+    // 1 - Automatic misbehavior ban
+    // 2 - Any other ban
+    int level = 0;
+    auto current_time = GetTime();
+    LOCK(m_cs_banned);
+    for (const auto& it : m_banned) {
+        CSubNet sub_net = it.first;
+        CBanEntry ban_entry = it.second;
+
+        if (current_time < ban_entry.nBanUntil && sub_net.Match(net_addr)) {
+            if (ban_entry.banReason != BanReasonNodeMisbehaving) return 2;
+            level = 1;
+        }
+    }
+    return level;
+}
+
 bool BanMan::IsBanned(CNetAddr net_addr)
 {
+    auto current_time = GetTime();
     LOCK(m_cs_banned);
     for (const auto& it : m_banned) {
         CSubNet sub_net = it.first;
         CBanEntry ban_entry = it.second;
 
-        if (sub_net.Match(net_addr) && GetTime() < ban_entry.nBanUntil) {
+        if (current_time < ban_entry.nBanUntil && sub_net.Match(net_addr)) {
             return true;
         }
     }
@@ -83,11 +105,12 @@ bool BanMan::IsBanned(CNetAddr net_addr)
 
 bool BanMan::IsBanned(CSubNet sub_net)
 {
+    auto current_time = GetTime();
     LOCK(m_cs_banned);
     banmap_t::iterator i = m_banned.find(sub_net);
     if (i != m_banned.end()) {
         CBanEntry ban_entry = (*i).second;
-        if (GetTime() < ban_entry.nBanUntil) {
+        if (current_time < ban_entry.nBanUntil) {
             return true;
         }
     }
diff --git a/src/banman.h b/src/banman.h
index 69f62be368..a1a00309dd 100644
--- a/src/banman.h
+++ b/src/banman.h
@@ -42,6 +42,7 @@ public:
     void Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
     void Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
     void ClearBanned();
+    int IsBannedLevel(CNetAddr net_addr);
     bool IsBanned(CNetAddr net_addr);
     bool IsBanned(CSubNet sub_net);
     bool Unban(const CNetAddr& net_addr);
diff --git a/src/net.cpp b/src/net.cpp
index 0490ccd6db..be249b4466 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -764,6 +764,7 @@ struct NodeEvictionCandidate
     bool fBloomFilter;
     CAddress addr;
     uint64_t nKeyedNetGroup;
+    bool prefer_evict;
 };
 
 static bool ReverseCompareNodeMinPingTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
@@ -832,7 +833,8 @@ bool CConnman::AttemptToEvictConnection()
             NodeEvictionCandidate candidate = {node->GetId(), node->nTimeConnected, node->nMinPingUsecTime,
                                                node->nLastBlockTime, node->nLastTXTime,
                                                HasAllDesirableServiceFlags(node->nServices),
-                                               node->fRelayTxes, node->pfilter != nullptr, node->addr, node->nKeyedNetGroup};
+                                               node->fRelayTxes, node->pfilter != nullptr, node->addr, node->nKeyedNetGroup,
+                                               node->m_prefer_evict};
             vEvictionCandidates.push_back(candidate);
         }
     }
@@ -857,6 +859,14 @@ bool CConnman::AttemptToEvictConnection()
 
     if (vEvictionCandidates.empty()) return false;
 
+    // If any remaining peers are preferred for eviction consider only them.
+    // This happens after the other preferences since if a peer is really the best by other criteria (esp relaying blocks)
+    //  then we probably don't want to evict it no matter what.
+    if (std::any_of(vEvictionCandidates.begin(),vEvictionCandidates.end(),[](NodeEvictionCandidate const &n){return n.prefer_evict;})) {
+        vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.begin(),vEvictionCandidates.end(),
+                                  [](NodeEvictionCandidate const &n){return !n.prefer_evict;}),vEvictionCandidates.end());
+    }
+
     // Identify the network group with the most connections and youngest member.
     // (vEvictionCandidates is already sorted by reverse connect time)
     uint64_t naMostConnections;
@@ -937,7 +947,11 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     // on all platforms.  Set it again here just to be sure.
     SetSocketNoDelay(hSocket);
 
-    if (m_banman && m_banman->IsBanned(addr) && !whitelisted)
+    int bannedlevel = m_banman ? m_banman->IsBannedLevel(addr) : 0;
+
+    // Don't accept connections from banned peers, but if our inbound slots aren't almost full, accept
+    // if the only banning reason was an automatic misbehavior ban.
+    if (!whitelisted && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0))
     {
         LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString());
         CloseSocket(hSocket);
@@ -961,6 +975,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true);
     pnode->AddRef();
     pnode->fWhitelisted = whitelisted;
+    pnode->m_prefer_evict = bannedlevel > 0;
     m_msgproc->InitializeNode(pnode);
 
     LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString());
diff --git a/src/net.h b/src/net.h
index 4449a36ecd..3c1a107b43 100644
--- a/src/net.h
+++ b/src/net.h
@@ -651,6 +651,7 @@ public:
     // the network or wire types and the cleaned string used when displayed or logged.
     std::string strSubVer GUARDED_BY(cs_SubVer), cleanSubVer GUARDED_BY(cs_SubVer);
     CCriticalSection cs_SubVer; // used for both cleanSubVer and strSubVer
+    bool m_prefer_evict{false}; // This peer is preferred for eviction.
     bool fWhitelisted{false}; // This peer can bypass DoS banning.
     bool fFeeler{false}; // If true this node is being used as a short lived feeler.
     bool fOneShot{false};