phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-25 18:53:23 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Clean up separated ban/discourage interface

Browse source
This commit is contained in:
Pieter Wuille 2020-06-10 17:11:38 -07:00
parent b691f2df5f
commit 2ad58381ff
13 changed files with 77 additions and 113 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
src/addrdb.h
View file

@ -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";
}
}
};

46
src/banman.cpp
View file

@ -68,30 +68,16 @@ 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
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)) {
return 2;
}
}
return m_discouraged.contains(net_addr.GetAddrBytes()) ? 1 : 0;
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);
if (m_discouraged.contains(net_addr.GetAddrBytes())) return true;
for (const auto& it : m_banned) {
CSubNet sub_net = it.first;
CBanEntry ban_entry = it.second;
@ -103,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);
@ -117,21 +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)
{
if (ban_reason == BanReasonNodeMisbehaving) {
LOCK(m_cs_banned);
m_discouraged.insert(net_addr.GetAddrBytes());
return;
}
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::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
void BanMan::Discourage(const CNetAddr& net_addr)
{
assert(ban_reason == BanReasonManuallyAdded);
CBanEntry ban_entry(GetTime(), ban_reason);
LOCK(m_cs_banned);
m_discouraged.insert(net_addr.GetAddrBytes());
}
void BanMan::Ban(const CSubNet& sub_net, int64_t ban_time_offset, bool since_unix_epoch)
{
CBanEntry ban_entry(GetTime());
int64_t normalized_ban_time_offset = ban_time_offset;
bool normalized_since_unix_epoch = since_unix_epoch;
@ -151,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)

18
src/banman.h
View file

@ -59,12 +59,20 @@ 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);

4
src/interfaces/node.cpp
View file

@ -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;

2
src/interfaces/node.h
View file

@ -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;

16
src/net.cpp
View file

@ -1010,10 +1010,9 @@ 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.
if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && bannedlevel == 2)
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);
@ -1021,7 +1020,8 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
}
// Only accept connections from discouraged peers if our inbound slots aren't (almost) full.
if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && nInbound + 1 >= nMaxInbound && bannedlevel >= 1)
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);
@ -1051,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());
@ -2052,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;

2
src/net_permissions.h
View file

@ -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),

33
src/net_processing.cpp
View file

@ -252,8 +252,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.
@ -404,7 +404,7 @@ struct CNodeState {
{
fCurrentlyConnected = false;
nMisbehavior = 0;
fShouldBan = false;
m_should_discourage = false;
pindexBestKnownBlock = nullptr;
hashLastUnknownBlock.SetNull();
pindexLastCommonBlock = nullptr;
@ -1036,7 +1036,7 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV
if (state->nMisbehavior >= banscore && state->nMisbehavior - howmuch < banscore)
{
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->fShouldBan = true;
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);
}
@ -2476,7 +2476,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())
{
@ -3329,7 +3330,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);
}
}
@ -3564,26 +3565,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()) {
} 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);
}
@ -3683,7 +3684,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
}
LOCK(cs_main);
CheckIfBanned(*pfrom);
MaybeDiscourageAndDisconnect(*pfrom);
return fMoreWork;
}
@ -3886,7 +3887,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());

2
src/net_processing.h
View file

@ -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);

2
src/qt/rpcconsole.cpp
View file

@ -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);
}
}

7
src/rpc/net.cpp
View file

@ -601,8 +601,7 @@ static UniValue setban(const JSONRPCRequest& request)
if (strCommand == "add")
{
if ((isSubnet && node.banman->IsBanned(subNet)) ||
(!isSubnet && node.banman->IsBannedLevel(netAddr) == BanReasonManuallyAdded)) {
if (isSubnet ? node.banman->IsBanned(subNet) : node.banman->IsBanned(netAddr)) {
throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: IP/Subnet already banned");
}
@ -615,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);
}

18
src/test/denialofservice_tests.cpp
View file

@ -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,7 +344,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
LOCK2(cs_main, dummyNode.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode));
}
BOOST_CHECK(banman->IsBanned(addr));
BOOST_CHECK(banman->IsDiscouraged(addr));
bool dummy;
peerLogic->FinalizeNode(dummyNode.GetId(), dummy);

12
src/test/fuzz/addrdb.cpp
View file

@ -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());
}