banman: save the banlist in a JSON format on disk

Save the banlist in `banlist.json` instead of `banlist.dat`.

This makes it possible to store Tor v3 entries in the banlist on disk
(and any other addresses that cannot be serialized in addrv1 format).

Still read `banlist.dat` (if it exists) and merge its contents with
`banlist.json` (if it exists). This is because we need to read
`banlist.dat` at least once in order to convert it and it is easier to
read it every time (not just once).

Supersedes bitcoin#20904
Resolves bitcoin#19748

doc/files.md

@@ -55,7 +55,8 @@ Subdirectory       | File(s)               | Description
 `indexes/blockfilter/basic/`    | `fltrNNNNN.dat`<sup>[\[2\]](#note2)</sup> | Blockfilter index filters for the basic filtertype; *optional*, used if `-blockfilterindex=basic`
 `wallets/`         |                       | [Contains wallets](#multi-wallet-environment); can be specified by `-walletdir` option; if `wallets/` subdirectory does not exist, wallets reside in the [data directory](#data-directory-location)
 `./`               | `anchors.dat`         | Anchor IP address database, created on shutdown and deleted at startup. Anchors are last known outgoing block-relay-only peers that are tried to re-connect to on startup
-`./`               | `banlist.dat`         | Stores the IPs/subnets of banned nodes
+`./`               | `banlist.dat`         | Stores the addresses/subnets of banned nodes (deprecated). Bitcoin Core will not save the banlist to it. It would only read it on startup if `banlist.json` is not present.
+`./`               | `banlist.json`        | Stores the addresses/subnets of banned nodes.
 `./`               | `bitcoin.conf`        | User-defined [configuration settings](bitcoin-conf.md) for `bitcoind` or `bitcoin-qt`. File is not written to by the software and must be created manually. Path can be specified by `-conf` option
 `./`               | `bitcoind.pid`        | Stores the process ID (PID) of `bitcoind` or `bitcoin-qt` while running; created at start and deleted on shutdown; can be specified by `-pid` option
 `./`               | `debug.log`           | Contains debug information and general logging generated by `bitcoind` or `bitcoin-qt`; can be specified by `-debuglogfile` option

src/addrdb.cpp

@@ -11,13 +11,69 @@
 #include <cstdint>
 #include <hash.h>
 #include <logging/timer.h>
+#include <netbase.h>
 #include <random.h>
 #include <streams.h>
 #include <tinyformat.h>
+#include <univalue.h>
+#include <util/settings.h>
 #include <util/system.h>
 
+CBanEntry::CBanEntry(const UniValue& json)
+    : nVersion(json["version"].get_int()), nCreateTime(json["ban_created"].get_int64()),
+      nBanUntil(json["banned_until"].get_int64())
+{
+}
+
+UniValue CBanEntry::ToJson() const
+{
+    UniValue json(UniValue::VOBJ);
+    json.pushKV("version", nVersion);
+    json.pushKV("ban_created", nCreateTime);
+    json.pushKV("banned_until", nBanUntil);
+    return json;
+}
+
 namespace {
 
+/**
+ * Convert a `banmap_t` object to a JSON array.
+ * @param[in] bans Bans list to convert.
+ * @return a JSON array, similar to the one returned by the `listbanned` RPC. Suitable for
+ * passing to `BanMapFromJson()`.
+ */
+UniValue BanMapToJson(const banmap_t& bans)
+{
+    UniValue bans_json(UniValue::VARR);
+    for (const auto& [address, ban_entry] : bans) {
+        UniValue j = ban_entry.ToJson();
+        j.pushKV("address", address.ToString());
+        bans_json.push_back(j);
+    }
+    return bans_json;
+}
+
+/**
+ * Convert a JSON array to a `banmap_t` object.
+ * @param[in] bans_json JSON to convert, must be as returned by `BanMapToJson()`.
+ * @param[out] bans Bans list to create from the JSON.
+ * @return true if the conversion was successful.
+ * @throws std::runtime_error if the JSON does not have the expected fields.
+ */
+bool BanMapFromJson(const UniValue& bans_json, banmap_t& bans)
+{
+    for (const auto& ban_entry_json : bans_json.getValues()) {
+        CSubNet subnet;
+        const auto& subnet_str = ban_entry_json["address"].get_str();
+        if (!LookupSubNet(subnet_str, subnet)) {
+            return error("%s: Cannot parse banned address or subnet: %s", __func__, subnet_str);
+        }
+        bans.insert_or_assign(subnet, CBanEntry{ban_entry_json});
+    }
+
+    return true;
+}
+
 template <typename Stream, typename Data>
 bool SerializeDB(Stream& stream, const Data& data)
 {
@@ -119,18 +175,46 @@ bool DeserializeFileDB(const fs::path& path, Data& data)
 
 }
 
-CBanDB::CBanDB(fs::path ban_list_path) : m_ban_list_path(std::move(ban_list_path))
+CBanDB::CBanDB(fs::path ban_list_path)
+    : m_banlist_dat(ban_list_path.string() + ".dat"),
+      m_banlist_json(ban_list_path.string() + ".json")
 {
 }
 
 bool CBanDB::Write(const banmap_t& banSet)
 {
-    return SerializeFileDB("banlist", m_ban_list_path, banSet);
+    std::vector<std::string> errors;
+    if (util::WriteSettings(m_banlist_json, {{JSON_KEY, BanMapToJson(banSet)}}, errors)) {
+        return true;
+    }
+
+    for (const auto& err : errors) {
+        error("%s: %s", __func__, err);
+    }
+    return false;
 }
 
 bool CBanDB::Read(banmap_t& banSet)
 {
-    return DeserializeFileDB(m_ban_list_path, banSet);
+    if (!fs::exists(m_banlist_json)) {
+        return DeserializeFileDB(m_banlist_dat, banSet);
+    }
+
+    std::map<std::string, util::SettingsValue> settings;
+    std::vector<std::string> errors;
+
+    if (!util::ReadSettings(m_banlist_json, settings, errors)) {
+        for (const auto& err : errors) {
+            error("%s: %s", __func__, err);
+        }
+        return false;
+    }
+
+    try {
+        return BanMapFromJson(settings[JSON_KEY], banSet);
+    } catch (const std::runtime_error& e) {
+        return error("%s: Cannot parse %s: %s", __func__, m_banlist_json.string(), e.what());
+    }
 }
 
 CAddrDB::CAddrDB()

src/addrdb.h

@@ -9,6 +9,7 @@
 #include <fs.h>
 #include <net_types.h> // For banmap_t
 #include <serialize.h>
+#include <univalue.h>
 
 #include <string>
 #include <vector>
@@ -36,6 +37,13 @@ class CBanEntry
         nCreateTime = nCreateTimeIn;
     }
 
+    /**
+     * Create a ban entry from JSON.
+     * @param[in] json A JSON representation of a ban entry, as created by `ToJson()`.
+     * @throw std::runtime_error if the JSON does not have the expected fields.
+     */
+    explicit CBanEntry(const UniValue& json);
+
     SERIALIZE_METHODS(CBanEntry, obj)
     {
         uint8_t ban_reason = 2; //! For backward compatibility
@@ -48,6 +56,12 @@ class CBanEntry
         nCreateTime = 0;
         nBanUntil = 0;
     }
+
+    /**
+     * Generate a JSON representation of this ban entry.
+     * @return JSON suitable for passing to the `CBanEntry(const UniValue&)` constructor.
+     */
+    UniValue ToJson() const;
 };
 
 /** Access to the (IP) address database (peers.dat) */
@@ -62,11 +76,17 @@ class CAddrDB
     static bool Read(CAddrMan& addr, CDataStream& ssPeers);
 };
 
-/** Access to the banlist database (banlist.dat) */
+/** Access to the banlist databases (banlist.json and banlist.dat) */
 class CBanDB
 {
 private:
-    const fs::path m_ban_list_path;
+    /**
+     * JSON key under which the data is stored in the json database.
+     */
+    static constexpr const char* JSON_KEY = "banned_nets";
+
+    const fs::path m_banlist_dat;
+    const fs::path m_banlist_json;
 public:
     explicit CBanDB(fs::path ban_list_path);
     bool Write(const banmap_t& banSet);

src/banman.cpp

@@ -25,10 +25,10 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t
         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",
-            m_banned.size(), GetTimeMillis() - n_start);
+        LogPrint(BCLog::NET, "Loaded %d banned node addresses/subnets  %dms\n", m_banned.size(),
+                 GetTimeMillis() - n_start);
     } else {
-        LogPrintf("Invalid or missing banlist.dat; recreating\n");
+        LogPrintf("Invalid or missing banlist database; recreating\n");
         SetBannedSetDirty(true); // force write
         DumpBanlist();
     }
@@ -53,8 +53,8 @@ void BanMan::DumpBanlist()
         SetBannedSetDirty(false);
     }
 
-    LogPrint(BCLog::NET, "Flushed %d banned node ips/subnets to banlist.dat  %dms\n",
-        banmap.size(), GetTimeMillis() - n_start);
+    LogPrint(BCLog::NET, "Flushed %d banned node addresses/subnets to disk  %dms\n", banmap.size(),
+             GetTimeMillis() - n_start);
 }
 
 void BanMan::ClearBanned()
@@ -188,7 +188,7 @@ void BanMan::SweepBanned()
                 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());
+                LogPrint(BCLog::NET, "%s: Removed banned node address/subnet: %s\n", __func__, sub_net.ToString());
             } else
                 ++it;
         }

src/banman.h

@@ -17,7 +17,8 @@
 
 // NOTE: When adjusting this, update rpcnet:setban's help ("24h")
 static constexpr unsigned int DEFAULT_MISBEHAVING_BANTIME = 60 * 60 * 24; // Default 24-hour ban
-// How often to dump addresses to banlist.dat
+
+/// How often to dump banned addresses/subnets to disk.
 static constexpr std::chrono::minutes DUMP_BANS_INTERVAL{15};
 
 class CClientUIInterface;
@@ -30,7 +31,7 @@ class CSubNet;
 // If an address or subnet is banned, we never accept incoming connections from
 // it and never create outgoing connections to it. We won't gossip its address
 // to other peers in addr messages. Banned addresses and subnets are stored to
-// banlist.dat on shutdown and reloaded on startup. Banning can be used to
+// disk on shutdown and reloaded on startup. Banning can be used to
 // prevent connections with spy nodes or other griefers.
 //
 // 2. Discouragement. If a peer misbehaves enough (see Misbehaving() in

src/init.cpp

@@ -1391,7 +1391,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA
     const bool ignores_incoming_txs{args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY)};
 
     assert(!node.banman);
-    node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME));
+    node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME));
     assert(!node.connman);
     node.connman = MakeUnique<CConnman>(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max()), args.GetBoolArg("-networkactive", true));
 

src/test/denialofservice_tests.cpp

@@ -222,7 +222,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
 BOOST_AUTO_TEST_CASE(peer_discouragement)
 {
     const CChainParams& chainparams = Params();
-    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
+    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
     auto peerLogic = PeerManager::make(chainparams, *connman, banman.get(), *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, false);
@@ -269,7 +269,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
 BOOST_AUTO_TEST_CASE(DoS_bantime)
 {
     const CChainParams& chainparams = Params();
-    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
+    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
     auto peerLogic = PeerManager::make(chainparams, *connman, banman.get(), *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, false);

src/test/fuzz/banman.cpp

@@ -33,8 +33,9 @@ FUZZ_TARGET_INIT(banman, initialize_banman)
 {
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
     SetMockTime(ConsumeTime(fuzzed_data_provider));
-    const fs::path banlist_file = GetDataDir() / "fuzzed_banlist.dat";
-    fs::remove(banlist_file);
+    const fs::path banlist_file = GetDataDir() / "fuzzed_banlist";
+    fs::remove(banlist_file.string() + ".dat");
+    fs::remove(banlist_file.string() + ".json");
     {
         BanMan ban_man{banlist_file, nullptr, ConsumeBanTimeOffset(fuzzed_data_provider)};
         while (fuzzed_data_provider.ConsumeBool()) {
@@ -77,5 +78,6 @@ FUZZ_TARGET_INIT(banman, initialize_banman)
                 });
         }
     }
-    fs::remove(banlist_file);
+    fs::remove(banlist_file.string() + ".dat");
+    fs::remove(banlist_file.string() + ".json");
 }

src/test/util/setup_common.cpp

@@ -187,7 +187,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
         throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString()));
     }
 
-    m_node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
+    m_node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     m_node.connman = MakeUnique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests.
     m_node.peerman = PeerManager::make(chainparams, *m_node.connman, m_node.banman.get(),
                                        *m_node.scheduler, *m_node.chainman, *m_node.mempool,