aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--doc/release-notes-19219.md23
-rw-r--r--src/addrdb.h28
-rw-r--r--src/banman.cpp41
-rw-r--r--src/banman.h63
-rw-r--r--src/init.cpp4
-rw-r--r--src/interfaces/node.cpp4
-rw-r--r--src/interfaces/node.h2
-rw-r--r--src/net.cpp25
-rw-r--r--src/net_permissions.h2
-rw-r--r--src/net_processing.cpp54
-rw-r--r--src/net_processing.h2
-rw-r--r--src/netaddress.h1
-rw-r--r--src/qt/rpcconsole.cpp2
-rw-r--r--src/rpc/net.cpp10
-rw-r--r--src/test/denialofservice_tests.cpp24
-rw-r--r--src/test/fuzz/addrdb.cpp12
16 files changed, 154 insertions, 143 deletions
diff --git a/doc/release-notes-19219.md b/doc/release-notes-19219.md
new file mode 100644
index 0000000000..b5ee885ddc
--- /dev/null
+++ b/doc/release-notes-19219.md
@@ -0,0 +1,23 @@
+#### Changes regarding misbehaving peers
+
+Peers that misbehave (e.g. send us invalid blocks) are now referred to as
+discouraged nodes in log output, as they're not (and weren't) strictly banned:
+incoming connections are still allowed from them, but they're preferred for
+eviction.
+
+Furthermore, a few additional changes are introduced to how discouraged
+addresses are treated:
+
+- Discouraging an address does not time out automatically after 24 hours
+ (or the `-bantime` setting). Depending on traffic from other peers,
+ discouragement may time out at an indeterminate time.
+
+- Discouragement is not persisted over restarts.
+
+- There is no method to list discouraged addresses. They are not returned by
+ the `listbanned` RPC. That RPC also no longer reports the `ban_reason`
+ field, as `"manually added"` is the only remaining option.
+
+- Discouragement cannot be removed with the `setban remove` RPC command.
+ If you need to remove a discouragement, you can remove all discouragements by
+ stop-starting your node.
diff --git a/src/addrdb.h b/src/addrdb.h
index c6d4307d69..8410c3776c 100644
--- a/src/addrdb.h
+++ b/src/addrdb.h
@@ -17,13 +17,6 @@ class CSubNet;
class CAddrMan;
class CDataStream;
-typedef enum BanReason
-{
- BanReasonUnknown = 0,
- BanReasonNodeMisbehaving = 1,
- BanReasonManuallyAdded = 2
-} BanReason;
-
class CBanEntry
{
public:
@@ -31,7 +24,6 @@ public:
int nVersion;
int64_t nCreateTime;
int64_t nBanUntil;
- uint8_t banReason;
CBanEntry()
{
@@ -44,31 +36,17 @@ public:
nCreateTime = nCreateTimeIn;
}
- explicit CBanEntry(int64_t n_create_time_in, BanReason ban_reason_in) : CBanEntry(n_create_time_in)
+ SERIALIZE_METHODS(CBanEntry, obj)
{
- banReason = ban_reason_in;
+ uint8_t ban_reason = 2; //! For backward compatibility
+ READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, ban_reason);
}
- SERIALIZE_METHODS(CBanEntry, obj) { READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, obj.banReason); }
-
void SetNull()
{
nVersion = CBanEntry::CURRENT_VERSION;
nCreateTime = 0;
nBanUntil = 0;
- banReason = BanReasonUnknown;
- }
-
- std::string banReasonToString() const
- {
- switch (banReason) {
- case BanReasonNodeMisbehaving:
- return "node misbehaving";
- case BanReasonManuallyAdded:
- return "manually added";
- default:
- return "unknown";
- }
}
};
diff --git a/src/banman.cpp b/src/banman.cpp
index 422904bb33..2fabacda0e 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -68,28 +68,13 @@ void BanMan::ClearBanned()
if (m_client_interface) m_client_interface->BannedListChanged();
}
-int BanMan::IsBannedLevel(CNetAddr net_addr)
+bool BanMan::IsDiscouraged(const 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;
+ return m_discouraged.contains(net_addr.GetAddrBytes());
}
-bool BanMan::IsBanned(CNetAddr net_addr)
+bool BanMan::IsBanned(const CNetAddr& net_addr)
{
auto current_time = GetTime();
LOCK(m_cs_banned);
@@ -104,7 +89,7 @@ bool BanMan::IsBanned(CNetAddr net_addr)
return false;
}
-bool BanMan::IsBanned(CSubNet sub_net)
+bool BanMan::IsBanned(const CSubNet& sub_net)
{
auto current_time = GetTime();
LOCK(m_cs_banned);
@@ -118,15 +103,21 @@ bool BanMan::IsBanned(CSubNet sub_net)
return false;
}
-void BanMan::Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
+void BanMan::Ban(const CNetAddr& net_addr, 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);
+ Ban(sub_net, ban_time_offset, since_unix_epoch);
+}
+
+void BanMan::Discourage(const CNetAddr& net_addr)
+{
+ LOCK(m_cs_banned);
+ m_discouraged.insert(net_addr.GetAddrBytes());
}
-void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
+void BanMan::Ban(const CSubNet& sub_net, int64_t ban_time_offset, bool since_unix_epoch)
{
- CBanEntry ban_entry(GetTime(), ban_reason);
+ CBanEntry ban_entry(GetTime());
int64_t normalized_ban_time_offset = ban_time_offset;
bool normalized_since_unix_epoch = since_unix_epoch;
@@ -146,8 +137,8 @@ void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ba
}
if (m_client_interface) m_client_interface->BannedListChanged();
- //store banlist to disk immediately if user requested ban
- if (ban_reason == BanReasonManuallyAdded) DumpBanlist();
+ //store banlist to disk immediately
+ DumpBanlist();
}
bool BanMan::Unban(const CNetAddr& net_addr)
diff --git a/src/banman.h b/src/banman.h
index 6bea2e75e9..f6bfbd1e49 100644
--- a/src/banman.h
+++ b/src/banman.h
@@ -6,6 +6,7 @@
#define BITCOIN_BANMAN_H
#include <addrdb.h>
+#include <bloom.h>
#include <fs.h>
#include <net_types.h> // For banmap_t
#include <sync.h>
@@ -23,32 +24,55 @@ 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.
+// Banman manages two related but distinct concepts:
+//
+// 1. Banning. This is configured manually by the user, through the setban RPC.
+// 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
+// prevent connections with spy nodes or other griefers.
+//
+// 2. Discouragement. If a peer misbehaves enough (see Misbehaving() in
+// net_processing.cpp), we'll mark that address as discouraged. We still allow
+// incoming connections from them, but they're preferred for eviction when
+// we receive new incoming connections. We never make outgoing connections to
+// them, and do not gossip their address to other peers. This is implemented as
+// a bloom filter. We can (probabilistically) test for membership, but can't
+// list all discouraged addresses or unmark them as discouraged. Discouragement
+// can prevent our limited connection slots being used up by incompatible
+// or broken peers.
+//
+// Neither banning nor discouragement are protections against denial-of-service
+// attacks, since if an attacker has a way to waste our resources and we
+// disconnect from them and ban that address, it's trivial for them to
+// reconnect from another IP address.
+//
+// Attempting to automatically disconnect or ban any class of peer carries the
+// risk of splitting the network. For example, if we banned/disconnected for a
+// transaction that fails a policy check and a future version changes the
+// policy check so the transaction is accepted, then that transaction could
+// cause the network to split between old nodes and new nodes.
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 Ban(const CNetAddr& net_addr, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
+ void Ban(const CSubNet& sub_net, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
+ void Discourage(const CNetAddr& net_addr);
void ClearBanned();
- int IsBannedLevel(CNetAddr net_addr);
- bool IsBanned(CNetAddr net_addr);
- bool IsBanned(CSubNet sub_net);
+
+ //! Return whether net_addr is banned
+ bool IsBanned(const CNetAddr& net_addr);
+
+ //! Return whether sub_net is exactly banned
+ bool IsBanned(const CSubNet& sub_net);
+
+ //! Return whether net_addr is discouraged.
+ bool IsDiscouraged(const CNetAddr& net_addr);
+
bool Unban(const CNetAddr& net_addr);
bool Unban(const CSubNet& sub_net);
void GetBanned(banmap_t& banmap);
@@ -68,6 +92,7 @@ private:
CClientUIInterface* m_client_interface = nullptr;
CBanDB m_ban_db;
const int64_t m_default_ban_time;
+ CRollingBloomFilter m_discouraged GUARDED_BY(m_cs_banned) {50000, 0.000001};
};
#endif
diff --git a/src/init.cpp b/src/init.cpp
index 0427368e70..2e39bcd9a5 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -431,8 +431,8 @@ void SetupServerArgs(NodeContext& node)
gArgs.AddArg("-addnode=<ip>", "Add a node to connect to and attempt to keep the connection open (see the `addnode` RPC command help for more info). This option can be specified multiple times to add multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
gArgs.AddArg("-asmap=<file>", strprintf("Specify asn mapping used for bucketing of the peers (default: %s). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
- gArgs.AddArg("-banscore=<n>", strprintf("Threshold for disconnecting misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
- gArgs.AddArg("-bantime=<n>", strprintf("Number of seconds to keep misbehaving peers from reconnecting (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+ gArgs.AddArg("-banscore=<n>", strprintf("Threshold for disconnecting and discouraging misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+ gArgs.AddArg("-bantime=<n>", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
gArgs.AddArg("-bind=<addr>", "Bind to given address and always listen on it. Use [host]:port notation for IPv6", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
gArgs.AddArg("-connect=<ip>", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
gArgs.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp
index 5b9a15f268..834a16ecf5 100644
--- a/src/interfaces/node.cpp
+++ b/src/interfaces/node.cpp
@@ -146,10 +146,10 @@ public:
}
return false;
}
- bool ban(const CNetAddr& net_addr, BanReason reason, int64_t ban_time_offset) override
+ bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) override
{
if (m_context.banman) {
- m_context.banman->Ban(net_addr, reason, ban_time_offset);
+ m_context.banman->Ban(net_addr, ban_time_offset);
return true;
}
return false;
diff --git a/src/interfaces/node.h b/src/interfaces/node.h
index 877a40568f..b88b5bc14e 100644
--- a/src/interfaces/node.h
+++ b/src/interfaces/node.h
@@ -122,7 +122,7 @@ public:
virtual bool getBanned(banmap_t& banmap) = 0;
//! Ban node.
- virtual bool ban(const CNetAddr& net_addr, BanReason reason, int64_t ban_time_offset) = 0;
+ virtual bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) = 0;
//! Unban node.
virtual bool unban(const CSubNet& ip) = 0;
diff --git a/src/net.cpp b/src/net.cpp
index 281232d801..0233b0fc2f 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1010,17 +1010,24 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
// on all platforms. Set it again here just to be sure.
SetSocketNoDelay(hSocket);
- 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 (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0))
+ // Don't accept connections from banned peers.
+ bool banned = m_banman->IsBanned(addr);
+ if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && banned)
{
LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString());
CloseSocket(hSocket);
return;
}
+ // Only accept connections from discouraged peers if our inbound slots aren't (almost) full.
+ bool discouraged = m_banman->IsDiscouraged(addr);
+ if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && nInbound + 1 >= nMaxInbound && discouraged)
+ {
+ LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToString());
+ CloseSocket(hSocket);
+ return;
+ }
+
if (nInbound >= nMaxInbound)
{
if (!AttemptToEvictConnection()) {
@@ -1044,7 +1051,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
pnode->m_permissionFlags = permissionFlags;
// If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility)
pnode->m_legacyWhitelisted = legacyWhitelisted;
- pnode->m_prefer_evict = bannedlevel > 0;
+ pnode->m_prefer_evict = discouraged;
m_msgproc->InitializeNode(pnode);
LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString());
@@ -2045,10 +2052,10 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
return;
}
if (!pszDest) {
- if (IsLocal(addrConnect) ||
- FindNode(static_cast<CNetAddr>(addrConnect)) || (m_banman && m_banman->IsBanned(addrConnect)) ||
- FindNode(addrConnect.ToStringIPPort()))
+ bool banned_or_discouraged = m_banman && (m_banman->IsDiscouraged(addrConnect) || m_banman->IsBanned(addrConnect));
+ if (IsLocal(addrConnect) || FindNode(static_cast<CNetAddr>(addrConnect)) || banned_or_discouraged || FindNode(addrConnect.ToStringIPPort())) {
return;
+ }
} else if (FindNode(std::string(pszDest)))
return;
diff --git a/src/net_permissions.h b/src/net_permissions.h
index e004067e75..2cf85120fe 100644
--- a/src/net_permissions.h
+++ b/src/net_permissions.h
@@ -24,7 +24,7 @@ enum NetPermissionFlags
// Always relay transactions from this peer, even if already in mempool
// Keep parameter interaction: forcerelay implies relay
PF_FORCERELAY = (1U << 2) | PF_RELAY,
- // Can't be banned for misbehavior
+ // Can't be banned/disconnected/discouraged for misbehavior
PF_NOBAN = (1U << 4),
// Can query the mempool
PF_MEMPOOL = (1U << 5),
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index e3053869ae..d0c3c943fc 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -249,8 +249,8 @@ struct CNodeState {
bool fCurrentlyConnected;
//! Accumulated misbehaviour score for this peer.
int nMisbehavior;
- //! Whether this peer should be disconnected and banned (unless whitelisted).
- bool fShouldBan;
+ //! Whether this peer should be disconnected and marked as discouraged (unless whitelisted with noban).
+ bool m_should_discourage;
//! String name of this peer (debugging/logging purposes).
const std::string name;
//! The best known block we know this peer has announced.
@@ -401,7 +401,7 @@ struct CNodeState {
{
fCurrentlyConnected = false;
nMisbehavior = 0;
- fShouldBan = false;
+ m_should_discourage = false;
pindexBestKnownBlock = nullptr;
hashLastUnknownBlock.SetNull();
pindexLastCommonBlock = nullptr;
@@ -1016,7 +1016,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
}
/**
- * Mark a misbehaving peer to be banned depending upon the value of `-banscore`.
+ * Increment peer's misbehavior score. If the new value surpasses banscore (specified on startup or by default), mark node to be discouraged, meaning the peer might be disconnected & added to the discouragement filter.
*/
void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
@@ -1032,14 +1032,14 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV
std::string message_prefixed = message.empty() ? "" : (": " + message);
if (state->nMisbehavior >= banscore && state->nMisbehavior - howmuch < banscore)
{
- LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) BAN THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed);
- state->fShouldBan = true;
+ LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed);
+ state->m_should_discourage = true;
} else
LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d)%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed);
}
/**
- * Potentially ban a node based on the contents of a BlockValidationState object
+ * Potentially mark a node discouraged based on the contents of a BlockValidationState object
*
* @param[in] via_compact_block this bool is passed in because net_processing should
* punish peers differently depending on whether the data was provided in a compact
@@ -1069,7 +1069,7 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s
break;
}
- // Ban outbound (but not inbound) peers if on an invalid chain.
+ // Discourage outbound (but not inbound) peers if on an invalid chain.
// Exempt HB compact block peers and manual connections.
if (!via_compact_block && !node_state->m_is_inbound && !node_state->m_is_manual_connection) {
Misbehaving(nodeid, 100, message);
@@ -1104,7 +1104,7 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s
}
/**
- * Potentially ban a node based on the contents of a TxValidationState object
+ * Potentially disconnect and discourage a node based on the contents of a TxValidationState object
*
* @return Returns true if the peer was punished (probably disconnected)
*/
@@ -1336,7 +1336,7 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB
}
/**
- * Handle invalid block rejection and consequent peer banning, maintain which
+ * Handle invalid block rejection and consequent peer discouragement, maintain which
* peers announce compact blocks.
*/
void PeerLogicValidation::BlockChecked(const CBlock& block, const BlockValidationState& state) {
@@ -2473,7 +2473,8 @@ void ProcessMessage(
if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60)
addr.nTime = nNow - 5 * 24 * 60 * 60;
pfrom.AddAddressKnown(addr);
- if (banman->IsBanned(addr)) continue; // Do not process banned addresses beyond remembering we received them
+ if (banman->IsDiscouraged(addr)) continue; // Do not process banned/discouraged addresses beyond remembering we received them
+ if (banman->IsBanned(addr)) continue;
bool fReachable = IsReachable(addr);
if (addr.nTime > nSince && !pfrom.fGetAddr && vAddr.size() <= 10 && addr.IsRoutable())
{
@@ -3115,7 +3116,7 @@ void ProcessMessage(
// relayed before full validation (see BIP 152), so we don't want to disconnect
// the peer if the header turns out to be for an invalid block.
// Note that if a peer tries to build on an invalid chain, that
- // will be detected and the peer will be banned.
+ // will be detected and the peer will be disconnected/discouraged.
return ProcessHeadersMessage(pfrom, connman, chainman, mempool, {cmpctblock.header}, chainparams, /*via_compact_block=*/true);
}
@@ -3201,7 +3202,7 @@ void ProcessMessage(
// 3. the block is otherwise invalid (eg invalid coinbase,
// block is too big, too many legacy sigops, etc).
// So if CheckBlock failed, #3 is the only possibility.
- // Under BIP 152, we don't DoS-ban unless proof of work is
+ // Under BIP 152, we don't discourage the peer unless proof of work is
// invalid (we don't require all the stateless checks to have
// been run). This is handled below, so just treat this as
// though the block was successfully read, and rely on the
@@ -3326,7 +3327,7 @@ void ProcessMessage(
std::vector<CAddress> vAddr = connman->GetAddresses();
FastRandomContext insecure_rand;
for (const CAddress &addr : vAddr) {
- if (!banman->IsBanned(addr)) {
+ if (!banman->IsDiscouraged(addr) && !banman->IsBanned(addr)) {
pfrom.PushAddress(addr, insecure_rand);
}
}
@@ -3561,25 +3562,26 @@ void ProcessMessage(
return;
}
-bool PeerLogicValidation::CheckIfBanned(CNode& pnode)
+bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode)
{
AssertLockHeld(cs_main);
CNodeState &state = *State(pnode.GetId());
- if (state.fShouldBan) {
- state.fShouldBan = false;
- if (pnode.HasPermission(PF_NOBAN))
+ if (state.m_should_discourage) {
+ state.m_should_discourage = false;
+ if (pnode.HasPermission(PF_NOBAN)) {
LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode.addr.ToString());
- else if (pnode.m_manual_connection)
+ } else if (pnode.m_manual_connection) {
LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode.addr.ToString());
- 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());
+ } else if (pnode.addr.IsLocal()) {
+ // Disconnect but don't discourage this local node
+ LogPrintf("Warning: disconnecting but not discouraging local peer %s!\n", pnode.addr.ToString());
pnode.fDisconnect = true;
} else {
- // Disconnect and ban all nodes sharing the address
+ // Disconnect and discourage all nodes sharing the address
+ LogPrintf("Disconnecting and discouraging peer %s!\n", pnode.addr.ToString());
if (m_banman) {
- m_banman->Ban(pnode.addr, BanReasonNodeMisbehaving);
+ m_banman->Discourage(pnode.addr);
}
connman->DisconnectNode(pnode.addr);
}
@@ -3679,7 +3681,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
}
LOCK(cs_main);
- CheckIfBanned(*pfrom);
+ MaybeDiscourageAndDisconnect(*pfrom);
return fMoreWork;
}
@@ -3882,7 +3884,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
if (!lockMain)
return true;
- if (CheckIfBanned(*pto)) return true;
+ if (MaybeDiscourageAndDisconnect(*pto)) return true;
CNodeState &state = *State(pto->GetId());
diff --git a/src/net_processing.h b/src/net_processing.h
index 19beca0cc4..eadf29e59f 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -31,7 +31,7 @@ private:
ChainstateManager& m_chainman;
CTxMemPool& m_mempool;
- bool CheckIfBanned(CNode& pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+ bool MaybeDiscourageAndDisconnect(CNode& pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
public:
PeerLogicValidation(CConnman* connman, BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool);
diff --git a/src/netaddress.h b/src/netaddress.h
index e640c07d32..552abc4119 100644
--- a/src/netaddress.h
+++ b/src/netaddress.h
@@ -90,6 +90,7 @@ class CNetAddr
uint32_t GetMappedAS(const std::vector<bool> &asmap) const;
std::vector<unsigned char> GetGroup(const std::vector<bool> &asmap) const;
+ std::vector<unsigned char> GetAddrBytes() const { return {std::begin(ip), std::end(ip)}; }
int GetReachabilityFrom(const CNetAddr *paddrPartner = nullptr) const;
explicit CNetAddr(const struct in6_addr& pipv6Addr, const uint32_t scope = 0);
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index dafd517ca8..ab2c37e6f8 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -1218,7 +1218,7 @@ void RPCConsole::banSelectedNode(int bantime)
// 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.ban(stats->nodeStats.addr, bantime);
m_node.disconnectByAddress(stats->nodeStats.addr);
}
}
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index df1e0fe623..393442e946 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -614,12 +614,12 @@ static UniValue setban(const JSONRPCRequest& request)
absolute = true;
if (isSubnet) {
- node.banman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute);
+ node.banman->Ban(subNet, banTime, absolute);
if (node.connman) {
node.connman->DisconnectNode(subNet);
}
} else {
- node.banman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute);
+ node.banman->Ban(netAddr, banTime, absolute);
if (node.connman) {
node.connman->DisconnectNode(netAddr);
}
@@ -628,7 +628,7 @@ static UniValue setban(const JSONRPCRequest& request)
else if(strCommand == "remove")
{
if (!( isSubnet ? node.banman->Unban(subNet) : node.banman->Unban(netAddr) )) {
- throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously banned.");
+ throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously manually banned.");
}
}
return NullUniValue;
@@ -637,7 +637,7 @@ static UniValue setban(const JSONRPCRequest& request)
static UniValue listbanned(const JSONRPCRequest& request)
{
RPCHelpMan{"listbanned",
- "\nList all banned IPs/Subnets.\n",
+ "\nList all manually banned IPs/Subnets.\n",
{},
RPCResult{RPCResult::Type::ARR, "", "",
{
@@ -646,7 +646,6 @@ static UniValue listbanned(const JSONRPCRequest& request)
{RPCResult::Type::STR, "address", ""},
{RPCResult::Type::NUM_TIME, "banned_until", ""},
{RPCResult::Type::NUM_TIME, "ban_created", ""},
- {RPCResult::Type::STR, "ban_reason", ""},
}},
}},
RPCExamples{
@@ -671,7 +670,6 @@ static UniValue listbanned(const JSONRPCRequest& request)
rec.pushKV("address", entry.first.ToString());
rec.pushKV("banned_until", banEntry.nBanUntil);
rec.pushKV("ban_created", banEntry.nCreateTime);
- rec.pushKV("ban_reason", banEntry.banReasonToString());
bannedAddresses.push_back(rec);
}
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 348b170536..1fe01fae04 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -238,8 +238,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
}
- BOOST_CHECK(banman->IsBanned(addr1));
- BOOST_CHECK(!banman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
+ BOOST_CHECK(banman->IsDiscouraged(addr1));
+ BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
CAddress addr2(ip(0xa0b0c002), NODE_NONE);
CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", true);
@@ -255,8 +255,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
LOCK2(cs_main, dummyNode2.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
}
- BOOST_CHECK(!banman->IsBanned(addr2)); // 2 not banned yet...
- BOOST_CHECK(banman->IsBanned(addr1)); // ... but 1 still should be
+ BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not banned yet...
+ BOOST_CHECK(banman->IsDiscouraged(addr1)); // ... but 1 still should be
{
LOCK(cs_main);
Misbehaving(dummyNode2.GetId(), 50);
@@ -265,7 +265,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
LOCK2(cs_main, dummyNode2.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
}
- BOOST_CHECK(banman->IsBanned(addr2));
+ BOOST_CHECK(banman->IsDiscouraged(addr2));
bool dummy;
peerLogic->FinalizeNode(dummyNode1.GetId(), dummy);
@@ -294,7 +294,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
}
- BOOST_CHECK(!banman->IsBanned(addr1));
+ BOOST_CHECK(!banman->IsDiscouraged(addr1));
{
LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 10);
@@ -303,7 +303,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
}
- BOOST_CHECK(!banman->IsBanned(addr1));
+ BOOST_CHECK(!banman->IsDiscouraged(addr1));
{
LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 1);
@@ -312,7 +312,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
}
- BOOST_CHECK(banman->IsBanned(addr1));
+ BOOST_CHECK(banman->IsDiscouraged(addr1));
gArgs.ForceSetArg("-banscore", ToString(DEFAULT_BANSCORE_THRESHOLD));
bool dummy;
@@ -344,13 +344,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
LOCK2(cs_main, dummyNode.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode));
}
- BOOST_CHECK(banman->IsBanned(addr));
-
- SetMockTime(nStartTime+60*60);
- BOOST_CHECK(banman->IsBanned(addr));
-
- SetMockTime(nStartTime+60*60*24+1);
- BOOST_CHECK(!banman->IsBanned(addr));
+ BOOST_CHECK(banman->IsDiscouraged(addr));
bool dummy;
peerLogic->FinalizeNode(dummyNode.GetId(), dummy);
diff --git a/src/test/fuzz/addrdb.cpp b/src/test/fuzz/addrdb.cpp
index 524cea83fe..ad6461650f 100644
--- a/src/test/fuzz/addrdb.cpp
+++ b/src/test/fuzz/addrdb.cpp
@@ -18,18 +18,11 @@ void test_one_input(const std::vector<uint8_t>& buffer)
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
const CBanEntry ban_entry = [&] {
- switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 3)) {
+ switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 2)) {
case 0:
return CBanEntry{fuzzed_data_provider.ConsumeIntegral<int64_t>()};
break;
- case 1:
- return CBanEntry{fuzzed_data_provider.ConsumeIntegral<int64_t>(), fuzzed_data_provider.PickValueInArray<BanReason>({
- BanReason::BanReasonUnknown,
- BanReason::BanReasonNodeMisbehaving,
- BanReason::BanReasonManuallyAdded,
- })};
- break;
- case 2: {
+ case 1: {
const std::optional<CBanEntry> ban_entry = ConsumeDeserializable<CBanEntry>(fuzzed_data_provider);
if (ban_entry) {
return *ban_entry;
@@ -39,5 +32,4 @@ void test_one_input(const std::vector<uint8_t>& buffer)
}
return CBanEntry{};
}();
- assert(!ban_entry.banReasonToString().empty());
}
generated by cgit v1.2.3 (git 2.26.2) at 2024-07-08 09:05:13 +0000