diff --git a/doc/files.md b/doc/files.md index c34476d0cc..58cd234390 100644 --- a/doc/files.md +++ b/doc/files.md @@ -56,7 +56,8 @@ Subdirectory | File(s) | Description `./` | `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 `evodb/` | |special txes and quorums database `llmq/` | |quorum signatures database -`./` | `banlist.dat` | Stores the IPs/subnets of banned nodes +`./` | `banlist.dat` | Stores the addresses/subnets of banned nodes (deprecated). `dashd` or `dash-qt` no longer save the banlist to this file, but read it on startup if `banlist.json` is not present. +`./` | `banlist.json` | Stores the addresses/subnets of banned nodes. `./` | `dash.conf` | User-defined [configuration settings](dash-conf.md) for `dashd` or `dash-qt`. File is not written to by the software and must be created manually. Path can be specified by `-conf` option `./` | `dashd.pid` | Stores the process ID (PID) of `dashd` or `dash-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 `dashd` or `dash-qt`; can be specified by `-debuglogfile` option @@ -111,7 +112,7 @@ Subdirectory | File | Description ## Legacy subdirectories and files -These subdirectories and files are no longer used by the Dash Core: +These subdirectories and files are no longer used by Dash Core: Path | Description | Repository notes ---------------|-------------|----------------- diff --git a/src/addrdb.cpp b/src/addrdb.cpp index a2155e662c..12e1ee4bac 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -10,15 +10,74 @@ #include #include #include +#include #include #include #include +#include +#include #include #include +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 { +static const char* BANMAN_JSON_ADDR_KEY = "address"; + +/** + * 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& it : bans) { + const auto& address = it.first; + const auto& ban_entry = it.second; + UniValue j = ban_entry.ToJson(); + j.pushKV(BANMAN_JSON_ADDR_KEY, 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. + * @throws std::runtime_error if the JSON does not have the expected fields or they contain + * unparsable values. + */ +void 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[BANMAN_JSON_ADDR_KEY].get_str(); + if (!LookupSubNet(subnet_str, subnet)) { + throw std::runtime_error( + strprintf("Cannot parse banned address or subnet: %s", subnet_str)); + } + bans.insert_or_assign(subnet, CBanEntry{ban_entry_json}); + } +} + template bool SerializeDB(Stream& stream, const Data& data) { @@ -120,18 +179,54 @@ bool DeserializeFileDB(const fs::path& path, Data& data, int version) } } // namespace -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, CLIENT_VERSION); + std::vector errors; + if (util::WriteSettings(m_banlist_json, {{JSON_KEY, BanMapToJson(banSet)}}, errors)) { + return true; + } + + for (const auto& err : errors) { + error("%s", err); + } + return false; } -bool CBanDB::Read(banmap_t& banSet) +bool CBanDB::Read(banmap_t& banSet, bool& dirty) { - return DeserializeFileDB(m_ban_list_path, banSet, CLIENT_VERSION); + // If the JSON banlist does not exist, then try to read the non-upgraded banlist.dat. + if (!fs::exists(m_banlist_json)) { + // If this succeeds then we need to flush to disk in order to create the JSON banlist. + dirty = true; + return DeserializeFileDB(m_banlist_dat, banSet, CLIENT_VERSION); + } + + dirty = false; + + std::map settings; + std::vector errors; + + if (!util::ReadSettings(m_banlist_json, settings, errors)) { + for (const auto& err : errors) { + LogPrintf("Cannot load banlist %s: %s\n", m_banlist_json.string(), err); + } + return false; + } + + try { + BanMapFromJson(settings[JSON_KEY], banSet); + } catch (const std::runtime_error& e) { + LogPrintf("Cannot parse banlist %s: %s\n", m_banlist_json.string(), e.what()); + return false; + } + + return true; } CAddrDB::CAddrDB() diff --git a/src/addrdb.h b/src/addrdb.h index 3e9605827d..88dec7db51 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -9,6 +9,7 @@ #include #include // For banmap_t #include +#include #include #include @@ -36,6 +37,13 @@ public: 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 @@ public: 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,15 +76,30 @@ public: 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); - bool Read(banmap_t& banSet); + + /** + * Read the banlist from disk. + * @param[out] banSet The loaded list. Set if `true` is returned, otherwise it is left + * in an undefined state. + * @param[out] dirty Indicates whether the loaded list needs flushing to disk. Set if + * `true` is returned, otherwise it is left in an undefined state. + * @return true on success + */ + bool Read(banmap_t& banSet, bool& dirty); }; /** diff --git a/src/banman.cpp b/src/banman.cpp index 807ad19faa..ba1db11577 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -18,20 +18,18 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist...").translated); 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 + if (m_ban_db.Read(m_banned, m_is_dirty)) { + 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("Recreating banlist.dat\n"); - SetBannedSetDirty(true); // force write - DumpBanlist(); + LogPrintf("Recreating the banlist database\n"); + m_banned = {}; + m_is_dirty = true; } + + DumpBanlist(); } BanMan::~BanMan() @@ -53,8 +51,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() @@ -177,13 +175,6 @@ void BanMan::GetBanned(banmap_t& banmap) 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(); @@ -198,7 +189,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, "Removed banned node address/subnet: %s\n", sub_net.ToString()); } else ++it; } diff --git a/src/banman.h b/src/banman.h index e11d4e0c82..06f18ea826 100644 --- a/src/banman.h +++ b/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 @@ -80,7 +81,6 @@ public: void DumpBanlist(); private: - void SetBanned(const banmap_t& banmap); bool BannedSetIsDirty(); //!set the "dirty" flag for the banlist void SetBannedSetDirty(bool dirty = true); diff --git a/src/init.cpp b/src/init.cpp index 9d206ab879..962681200e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1662,7 +1662,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc assert(!node.addrman); node.addrman = std::make_unique(); assert(!node.banman); - node.banman = std::make_unique(GetDataDir() / "banlist.dat", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); + node.banman = std::make_unique(GetDataDir() / "banlist", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); assert(!node.connman); node.connman = std::make_unique(GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max()), *node.addrman); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 0e3e619d2b..f737f5ecc4 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -226,7 +226,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_AUTO_TEST_CASE(DoS_banning) { const CChainParams& chainparams = Params(); - auto banman = std::make_unique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + auto banman = std::make_unique(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, *governance, m_node.cj_ctx, @@ -274,7 +274,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) BOOST_AUTO_TEST_CASE(DoS_banscore) { const CChainParams& chainparams = Params(); - auto banman = std::make_unique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + auto banman = std::make_unique(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, *governance, m_node.cj_ctx, @@ -320,7 +320,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) BOOST_AUTO_TEST_CASE(DoS_bantime) { const CChainParams& chainparams = Params(); - auto banman = std::make_unique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + auto banman = std::make_unique(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, *governance, m_node.cj_ctx, diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index bd2dce6a40..5fd53b2d58 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -8,8 +8,10 @@ #include #include #include +#include #include +#include #include #include #include @@ -37,8 +39,20 @@ FUZZ_TARGET_INIT(banman, initialize_banman) int limit_max_ops{300}; 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); + fs::path banlist_file = GetDataDir() / "fuzzed_banlist"; + + const bool start_with_corrupted_banlist{fuzzed_data_provider.ConsumeBool()}; + if (start_with_corrupted_banlist) { + const std::string sfx{fuzzed_data_provider.ConsumeBool() ? ".dat" : ".json"}; + assert(WriteBinaryFile(banlist_file.string() + sfx, + fuzzed_data_provider.ConsumeRandomLengthString())); + } else { + const bool force_read_and_write_to_err{fuzzed_data_provider.ConsumeBool()}; + if (force_read_and_write_to_err) { + banlist_file = fs::path{"path"} / "to" / "inaccessible" / "fuzzed_banlist"; + } + } + { BanMan ban_man{banlist_file, nullptr, ConsumeBanTimeOffset(fuzzed_data_provider)}; while (--limit_max_ops >= 0 && fuzzed_data_provider.ConsumeBool()) { @@ -79,5 +93,6 @@ FUZZ_TARGET_INIT(banman, initialize_banman) }); } } - fs::remove(banlist_file); + fs::remove(banlist_file.string() + ".dat"); + fs::remove(banlist_file.string() + ".json"); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index c393f1d8de..7a66cf10b1 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -261,7 +261,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + m_node.banman = std::make_unique(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman, m_node.banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, *governance, m_node.cj_ctx, m_node.llmq_ctx, false); diff --git a/test/functional/rpc_setban.py b/test/functional/rpc_setban.py index a185b353b3..16e4722379 100755 --- a/test/functional/rpc_setban.py +++ b/test/functional/rpc_setban.py @@ -22,7 +22,7 @@ class SetBanTests(BitcoinTestFramework): # Node 0 connects to Node 1, check that the noban permission is not granted self.connect_nodes(0, 1) peerinfo = self.nodes[1].getpeerinfo()[0] - assert not 'noban' in peerinfo['permissions'] + assert not "noban" in peerinfo["permissions"] # Node 0 get banned by Node 1 self.nodes[1].setban("127.0.0.1", "add") @@ -36,27 +36,40 @@ class SetBanTests(BitcoinTestFramework): self.restart_node(1, ['-whitelist=127.0.0.1']) self.connect_nodes(0, 1) peerinfo = self.nodes[1].getpeerinfo()[0] - assert 'noban' in peerinfo['permissions'] + assert "noban" in peerinfo["permissions"] # If we remove the ban, Node 0 should be able to reconnect even without noban permission self.nodes[1].setban("127.0.0.1", "remove") self.restart_node(1, []) self.connect_nodes(0, 1) peerinfo = self.nodes[1].getpeerinfo()[0] - assert not 'noban' in peerinfo['permissions'] + assert not "noban" in peerinfo["permissions"] self.log.info("Test that a non-IP address can be banned/unbanned") node = self.nodes[1] tor_addr = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion" ip_addr = "1.2.3.4" - assert(not self.is_banned(node, tor_addr)) - assert(not self.is_banned(node, ip_addr)) + assert not self.is_banned(node, tor_addr) + assert not self.is_banned(node, ip_addr) + node.setban(tor_addr, "add") - assert(self.is_banned(node, tor_addr)) - assert(not self.is_banned(node, ip_addr)) + assert self.is_banned(node, tor_addr) + assert not self.is_banned(node, ip_addr) + + self.log.info("Test the ban list is preserved through restart") + + self.restart_node(1) + assert self.is_banned(node, tor_addr) + assert not self.is_banned(node, ip_addr) + node.setban(tor_addr, "remove") - assert(not self.is_banned(self.nodes[1], tor_addr)) - assert(not self.is_banned(node, ip_addr)) + assert not self.is_banned(self.nodes[1], tor_addr) + assert not self.is_banned(node, ip_addr) + + self.restart_node(1) + assert not self.is_banned(node, tor_addr) + assert not self.is_banned(node, ip_addr) + if __name__ == '__main__': SetBanTests().main()