Merge #14605: Return of the Banman

18185b5 scripted-diff: batch-recase BanMan variables (Carl Dong)
c2e04d3 banman: Add, use CBanEntry ctor that takes ban reason (Carl Dong)
1ffa4ce banman: reformulate nBanUtil calculation (Carl Dong)
daae598 banman: add thread annotations and mark members const where possible (Cory Fields)
84fc3fb scripted-diff: batch-rename BanMan members (Cory Fields)
af3503d net: move BanMan to its own files (Cory Fields)
d0469b2 banman: pass in default ban time as a parameter (Cory Fields)
2e56702 banman: pass the banfile path in (Cory Fields)
4c0d961 banman: create and split out banman (Cory Fields)
83c1ea2 net: split up addresses/ban dumps in preparation for moving them (Cory Fields)
136bd79 tests: remove member connman/peerLogic in TestingSetup (Cory Fields)
7cc2b9f net: Break disconnecting out of Ban() (Cory Fields)

Pull request description:

  **Old English à la Beowulf**
  ```
  Banman wæs bréme    --blaéd wíde sprang--
  Connmanes eafera    Coreum in.
  aéglaéca            léodum forstandan
  Swá bealdode        bearn Connmanes
  guma gúðum cúð      gódum daédum·
  dréah æfter dóme·   nealles druncne slóg
  ```

  **Modern English Translation**
  ```
  Banman was famed              --his renown spread wide--
  Conman's hier,                in Core-land.
  against the evil creature     defend the people
  Thus he was bold,             the son of Connman
  man famed in war,             for good deeds;
  he led his life for glory,    never, having drunk, slew
  ```

  --

  With @theuni's blessing, here is Banman, rebased. Original PR: bitcoin/bitcoin#11457

  --

  Followup PRs:
  1. Give `CNode` a `Disconnect` method ([source](bitcoin/bitcoin#14605 (comment)))
  2. Add a comment to `std::atomic_bool fDisconnect` in `net.h` that setting this to true will cause the node to be disconnected the next time `DisconnectNodes()` runs ([source](bitcoin/bitcoin#14605 (comment)))

Tree-SHA512: 9c207edbf577415c22c9811113e393322d936a843d4ff265186728152a67c057779ac4d4f27b895de9729f7a53e870f828b9ebc8bcdab757520c2aebe1e9be35

Author: laanwj

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 \

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()

src/addrdb.h

@@ -43,6 +43,11 @@ class CBanEntry
         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 <typename Stream, typename Operation>
@@ -92,9 +97,9 @@ class CAddrDB
 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);
 };

src/banman.cpp

@@ -0,0 +1,197 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2017 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <banman.h>
+
+#include <netaddress.h>
+#include <ui_interface.h>
+#include <util/system.h>
+#include <util/time.h>
+
+
+BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time)
+    : m_client_interface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time)
+{
+    if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist..."));
+
+    int64_t n_start = GetTimeMillis();
+    m_is_dirty = false;
+    banmap_t banmap;
+    if (m_ban_db.Read(banmap)) {
+        SetBanned(banmap);        // thread save setter
+        SetBannedSetDirty(false); // no need to write down, just read data
+        SweepBanned();            // sweep out unused entries
+
+        LogPrint(BCLog::NET, "Loaded %d banned node ips/subnets from banlist.dat  %dms\n",
+            banmap.size(), GetTimeMillis() - n_start);
+    } else {
+        LogPrintf("Invalid or missing banlist.dat; recreating\n");
+        SetBannedSetDirty(true); // force write
+        DumpBanlist();
+    }
+}
+
+BanMan::~BanMan()
+{
+    DumpBanlist();
+}
+
+void BanMan::DumpBanlist()
+{
+    SweepBanned(); // clean unused entries (if bantime has expired)
+
+    if (!BannedSetIsDirty()) return;
+
+    int64_t n_start = GetTimeMillis();
+
+    banmap_t banmap;
+    GetBanned(banmap);
+    if (m_ban_db.Write(banmap)) {
+        SetBannedSetDirty(false);
+    }
+
+    LogPrint(BCLog::NET, "Flushed %d banned node ips/subnets to banlist.dat  %dms\n",
+        banmap.size(), GetTimeMillis() - n_start);
+}
+
+void BanMan::ClearBanned()
+{
+    {
+        LOCK(m_cs_banned);
+        m_banned.clear();
+        m_is_dirty = true;
+    }
+    DumpBanlist(); //store banlist to disk
+    if (m_client_interface) m_client_interface->BannedListChanged();
+}
+
+bool BanMan::IsBanned(CNetAddr net_addr)
+{
+    LOCK(m_cs_banned);
+    for (const auto& it : m_banned) {
+        CSubNet sub_net = it.first;
+        CBanEntry ban_entry = it.second;
+
+        if (sub_net.Match(net_addr) && GetTime() < ban_entry.nBanUntil) {
+            return true;
+        }
+    }
+    return false;
+}
+
+bool BanMan::IsBanned(CSubNet sub_net)
+{
+    LOCK(m_cs_banned);
+    banmap_t::iterator i = m_banned.find(sub_net);
+    if (i != m_banned.end()) {
+        CBanEntry ban_entry = (*i).second;
+        if (GetTime() < ban_entry.nBanUntil) {
+            return true;
+        }
+    }
+    return false;
+}
+
+void BanMan::Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
+{
+    CSubNet sub_net(net_addr);
+    Ban(sub_net, ban_reason, ban_time_offset, since_unix_epoch);
+}
+
+void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
+{
+    CBanEntry ban_entry(GetTime(), ban_reason);
+
+    int64_t normalized_ban_time_offset = ban_time_offset;
+    bool normalized_since_unix_epoch = since_unix_epoch;
+    if (ban_time_offset <= 0) {
+        normalized_ban_time_offset = m_default_ban_time;
+        normalized_since_unix_epoch = false;
+    }
+    ban_entry.nBanUntil = (normalized_since_unix_epoch ? 0 : GetTime()) + normalized_ban_time_offset;
+
+    {
+        LOCK(m_cs_banned);
+        if (m_banned[sub_net].nBanUntil < ban_entry.nBanUntil) {
+            m_banned[sub_net] = ban_entry;
+            m_is_dirty = true;
+        } else
+            return;
+    }
+    if (m_client_interface) m_client_interface->BannedListChanged();
+
+    //store banlist to disk immediately if user requested ban
+    if (ban_reason == BanReasonManuallyAdded) DumpBanlist();
+}
+
+bool BanMan::Unban(const CNetAddr& net_addr)
+{
+    CSubNet sub_net(net_addr);
+    return Unban(sub_net);
+}
+
+bool BanMan::Unban(const CSubNet& sub_net)
+{
+    {
+        LOCK(m_cs_banned);
+        if (m_banned.erase(sub_net) == 0) return false;
+        m_is_dirty = true;
+    }
+    if (m_client_interface) m_client_interface->BannedListChanged();
+    DumpBanlist(); //store banlist to disk immediately
+    return true;
+}
+
+void BanMan::GetBanned(banmap_t& banmap)
+{
+    LOCK(m_cs_banned);
+    // Sweep the banlist so expired bans are not returned
+    SweepBanned();
+    banmap = m_banned; //create a thread safe copy
+}
+
+void BanMan::SetBanned(const banmap_t& banmap)
+{
+    LOCK(m_cs_banned);
+    m_banned = banmap;
+    m_is_dirty = true;
+}
+
+void BanMan::SweepBanned()
+{
+    int64_t now = GetTime();
+    bool notify_ui = false;
+    {
+        LOCK(m_cs_banned);
+        banmap_t::iterator it = m_banned.begin();
+        while (it != m_banned.end()) {
+            CSubNet sub_net = (*it).first;
+            CBanEntry ban_entry = (*it).second;
+            if (now > ban_entry.nBanUntil) {
+                m_banned.erase(it++);
+                m_is_dirty = true;
+                notify_ui = true;
+                LogPrint(BCLog::NET, "%s: Removed banned node ip/subnet from banlist.dat: %s\n", __func__, sub_net.ToString());
+            } else
+                ++it;
+        }
+    }
+    // update UI
+    if (notify_ui && m_client_interface) {
+        m_client_interface->BannedListChanged();
+    }
+}
+
+bool BanMan::BannedSetIsDirty()
+{
+    LOCK(m_cs_banned);
+    return m_is_dirty;
+}
+
+void BanMan::SetBannedSetDirty(bool dirty)
+{
+    LOCK(m_cs_banned); //reuse m_banned lock for the m_is_dirty flag
+    m_is_dirty = dirty;
+}

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 <cstdint>
+#include <memory>
+
+#include <addrdb.h>
+#include <fs.h>
+#include <sync.h>
+
+// NOTE: When adjusting this, update rpcnet:setban's help ("24h")
+static constexpr unsigned int DEFAULT_MISBEHAVING_BANTIME = 60 * 60 * 24; // Default 24-hour ban
+
+class CClientUIInterface;
+class CNetAddr;
+class CSubNet;
+
+// Denial-of-service detection/prevention
+// The idea is to detect peers that are behaving
+// badly and disconnect/ban them, but do it in a
+// one-coding-mistake-won't-shatter-the-entire-network
+// way.
+// IMPORTANT:  There should be nothing I can give a
+// node that it will forward on that will make that
+// node's peers drop it. If there is, an attacker
+// can isolate a node and/or try to split the network.
+// Dropping a node for sending stuff that is invalid
+// now but might be valid in a later version is also
+// dangerous, because it can cause a network split
+// between nodes running old code and nodes running
+// new code.
+
+class BanMan
+{
+public:
+    ~BanMan();
+    BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time);
+    void Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
+    void Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
+    void ClearBanned();
+    bool IsBanned(CNetAddr net_addr);
+    bool IsBanned(CSubNet sub_net);
+    bool Unban(const CNetAddr& net_addr);
+    bool Unban(const CSubNet& sub_net);
+    void GetBanned(banmap_t& banmap);
+    void DumpBanlist();
+
+private:
+    void SetBanned(const banmap_t& banmap);
+    bool BannedSetIsDirty();
+    //!set the "dirty" flag for the banlist
+    void SetBannedSetDirty(bool dirty = true);
+    //!clean unused entries (if bantime has expired)
+    void SweepBanned();
+
+    CCriticalSection m_cs_banned;
+    banmap_t m_banned GUARDED_BY(m_cs_banned);
+    bool m_is_dirty GUARDED_BY(m_cs_banned);
+    CClientUIInterface* m_client_interface = nullptr;
+    CBanDB m_ban_db;
+    const int64_t m_default_ban_time;
+};
+
+extern std::unique_ptr<BanMan> g_banman;
+#endif