aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/net_processing.cpp160
-rw-r--r--src/net_processing.h2
-rw-r--r--src/rpc/net.cpp2
-rw-r--r--src/test/denialofservice_tests.cpp20
4 files changed, 104 insertions, 80 deletions
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 60bdfbe9f5..adad428413 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -278,12 +278,6 @@ struct CNodeState {
const CService address;
//! Whether we have a fully established connection.
bool fCurrentlyConnected;
- //! Accumulated misbehaviour score for this peer.
- int nMisbehavior;
- //! Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission).
- 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.
const CBlockIndex *pindexBestKnownBlock;
//! The hash of the last unknown block this peer has announced.
@@ -432,13 +426,10 @@ struct CNodeState {
//! Whether this peer relays txs via wtxid
bool m_wtxid_relay{false};
- CNodeState(CAddress addrIn, std::string addrNameIn, bool is_inbound, bool is_manual) :
- address(addrIn), name(std::move(addrNameIn)), m_is_inbound(is_inbound),
- m_is_manual_connection (is_manual)
+ CNodeState(CAddress addrIn, bool is_inbound, bool is_manual)
+ : address(addrIn), m_is_inbound(is_inbound), m_is_manual_connection(is_manual)
{
fCurrentlyConnected = false;
- nMisbehavior = 0;
- m_should_discourage = false;
pindexBestKnownBlock = nullptr;
hashLastUnknownBlock.SetNull();
pindexLastCommonBlock = nullptr;
@@ -476,6 +467,50 @@ static CNodeState *State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
return &it->second;
}
+/**
+ * Data structure for an individual peer. This struct is not protected by
+ * cs_main since it does not contain validation-critical data.
+ *
+ * Memory is owned by shared pointers and this object is destructed when
+ * the refcount drops to zero.
+ *
+ * TODO: move most members from CNodeState to this structure.
+ * TODO: move remaining application-layer data members from CNode to this structure.
+ */
+struct Peer {
+ /** Same id as the CNode object for this peer */
+ const NodeId m_id{0};
+
+ /** Protects misbehavior data members */
+ Mutex m_misbehavior_mutex;
+ /** Accumulated misbehavior score for this peer */
+ int m_misbehavior_score GUARDED_BY(m_misbehavior_mutex){0};
+ /** Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). */
+ bool m_should_discourage GUARDED_BY(m_misbehavior_mutex){false};
+
+ Peer(NodeId id) : m_id(id) {}
+};
+
+using PeerRef = std::shared_ptr<Peer>;
+
+/**
+ * Map of all Peer objects, keyed by peer id. This map is protected
+ * by the global g_peer_mutex. Once a shared pointer reference is
+ * taken, the lock may be released. Individual fields are protected by
+ * their own locks.
+ */
+Mutex g_peer_mutex;
+static std::map<NodeId, PeerRef> g_peer_map GUARDED_BY(g_peer_mutex);
+
+/** Get a shared pointer to the Peer object.
+ * May return nullptr if the Peer object can't be found. */
+static PeerRef GetPeerRef(NodeId id)
+{
+ LOCK(g_peer_mutex);
+ auto it = g_peer_map.find(id);
+ return it != g_peer_map.end() ? it->second : nullptr;
+}
+
static void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
nPreferredDownload -= state->fPreferredDownload;
@@ -841,7 +876,12 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) {
NodeId nodeid = pnode->GetId();
{
LOCK(cs_main);
- mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName), pnode->IsInboundConn(), pnode->IsManualConn()));
+ mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, pnode->IsInboundConn(), pnode->IsManualConn()));
+ }
+ {
+ PeerRef peer = std::make_shared<Peer>(nodeid);
+ LOCK(g_peer_mutex);
+ g_peer_map.emplace_hint(g_peer_map.end(), nodeid, std::move(peer));
}
if(!pnode->IsInboundConn())
PushNodeVersion(*pnode, m_connman, GetTime());
@@ -870,13 +910,21 @@ void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const
void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) {
fUpdateConnectionTime = false;
LOCK(cs_main);
+ int misbehavior{0};
+ {
+ PeerRef peer = GetPeerRef(nodeid);
+ assert(peer != nullptr);
+ misbehavior = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score);
+ LOCK(g_peer_mutex);
+ g_peer_map.erase(nodeid);
+ }
CNodeState *state = State(nodeid);
assert(state != nullptr);
if (state->fSyncStarted)
nSyncStarted--;
- if (state->nMisbehavior == 0 && state->fCurrentlyConnected) {
+ if (misbehavior == 0 && state->fCurrentlyConnected) {
fUpdateConnectionTime = true;
}
@@ -906,17 +954,23 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim
}
bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats) {
- LOCK(cs_main);
- CNodeState *state = State(nodeid);
- if (state == nullptr)
- return false;
- stats.nMisbehavior = state->nMisbehavior;
- stats.nSyncHeight = state->pindexBestKnownBlock ? state->pindexBestKnownBlock->nHeight : -1;
- stats.nCommonHeight = state->pindexLastCommonBlock ? state->pindexLastCommonBlock->nHeight : -1;
- for (const QueuedBlock& queue : state->vBlocksInFlight) {
- if (queue.pindex)
- stats.vHeightInFlight.push_back(queue.pindex->nHeight);
+ {
+ LOCK(cs_main);
+ CNodeState* state = State(nodeid);
+ if (state == nullptr)
+ return false;
+ stats.nSyncHeight = state->pindexBestKnownBlock ? state->pindexBestKnownBlock->nHeight : -1;
+ stats.nCommonHeight = state->pindexLastCommonBlock ? state->pindexLastCommonBlock->nHeight : -1;
+ for (const QueuedBlock& queue : state->vBlocksInFlight) {
+ if (queue.pindex)
+ stats.vHeightInFlight.push_back(queue.pindex->nHeight);
+ }
}
+
+ PeerRef peer = GetPeerRef(nodeid);
+ if (peer == nullptr) return false;
+ stats.m_misbehavior_score = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score);
+
return true;
}
@@ -1060,21 +1114,21 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
* Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node
* to be discouraged, meaning the peer might be disconnected and added to the discouragement filter.
*/
-void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
+void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message)
{
assert(howmuch > 0);
- CNodeState* const state = State(pnode);
- if (state == nullptr) return;
+ PeerRef peer = GetPeerRef(pnode);
+ if (peer == nullptr) return;
- state->nMisbehavior += howmuch;
+ LOCK(peer->m_misbehavior_mutex);
+ peer->m_misbehavior_score += howmuch;
const std::string message_prefixed = message.empty() ? "" : (": " + message);
- if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD)
- {
- LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed);
- state->m_should_discourage = true;
+ if (peer->m_misbehavior_score >= DISCOURAGEMENT_THRESHOLD && peer->m_misbehavior_score - howmuch < DISCOURAGEMENT_THRESHOLD) {
+ LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, peer->m_misbehavior_score - howmuch, peer->m_misbehavior_score, message_prefixed);
+ peer->m_should_discourage = true;
} else {
- LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed);
+ LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, peer->m_misbehavior_score - howmuch, peer->m_misbehavior_score, message_prefixed);
}
}
@@ -1096,7 +1150,6 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s
case BlockValidationResult::BLOCK_CONSENSUS:
case BlockValidationResult::BLOCK_MUTATED:
if (!via_compact_block) {
- LOCK(cs_main);
Misbehaving(nodeid, 100, message);
return true;
}
@@ -1120,18 +1173,12 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s
case BlockValidationResult::BLOCK_INVALID_HEADER:
case BlockValidationResult::BLOCK_CHECKPOINT:
case BlockValidationResult::BLOCK_INVALID_PREV:
- {
- LOCK(cs_main);
- Misbehaving(nodeid, 100, message);
- }
+ Misbehaving(nodeid, 100, message);
return true;
// Conflicting (but not necessarily invalid) data or different policy:
case BlockValidationResult::BLOCK_MISSING_PREV:
- {
- // TODO: Handle this much more gracefully (10 DoS points is super arbitrary)
- LOCK(cs_main);
- Misbehaving(nodeid, 10, message);
- }
+ // TODO: Handle this much more gracefully (10 DoS points is super arbitrary)
+ Misbehaving(nodeid, 10, message);
return true;
case BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE:
case BlockValidationResult::BLOCK_TIME_FUTURE:
@@ -1155,11 +1202,8 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state,
break;
// The node is providing invalid data:
case TxValidationResult::TX_CONSENSUS:
- {
- LOCK(cs_main);
- Misbehaving(nodeid, 100, message);
- return true;
- }
+ Misbehaving(nodeid, 100, message);
+ return true;
// Conflicting (but not necessarily invalid) data or different policy:
case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE:
case TxValidationResult::TX_INPUTS_NOT_STANDARD:
@@ -1806,7 +1850,6 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac
BlockTransactions resp(req);
for (size_t i = 0; i < req.indexes.size(); i++) {
if (req.indexes[i] >= block.vtx.size()) {
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 100, "getblocktxn with out-of-bounds tx indices");
return;
}
@@ -2318,7 +2361,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
// Each connection can only send one version message
if (pfrom.nVersion != 0)
{
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1, "redundant version message");
return;
}
@@ -2478,7 +2520,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
if (pfrom.nVersion == 0) {
// Must have a version message before anything else
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1, "non-version message before version handshake");
return;
}
@@ -2545,7 +2586,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
if (!pfrom.fSuccessfullyConnected) {
// Must have a verack message before anything else
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1, "non-verack message before version handshake");
return;
}
@@ -2559,7 +2599,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
}
if (vAddr.size() > MAX_ADDR_TO_SEND)
{
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 20, strprintf("addr message size = %u", vAddr.size()));
return;
}
@@ -2638,7 +2677,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
vRecv >> vInv;
if (vInv.size() > MAX_INV_SZ)
{
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 20, strprintf("inv message size = %u", vInv.size()));
return;
}
@@ -2714,7 +2752,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
vRecv >> vInv;
if (vInv.size() > MAX_INV_SZ)
{
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 20, strprintf("getdata message size = %u", vInv.size()));
return;
}
@@ -3439,7 +3476,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
// Bypass the normal CBlock deserialization, as we don't want to risk deserializing 2000 full blocks.
unsigned int nCount = ReadCompactSize(vRecv);
if (nCount > MAX_HEADERS_RESULTS) {
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 20, strprintf("headers message size = %u", nCount));
return;
}
@@ -3641,7 +3677,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
if (!filter.IsWithinSizeConstraints())
{
// There is no excuse for sending a too-large filter
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 100, "too-large bloom filter");
}
else if (pfrom.m_tx_relay != nullptr)
@@ -3675,7 +3710,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
}
}
if (bad) {
- LOCK(cs_main);
Misbehaving(pfrom.GetId(), 100, "bad filteradd message");
}
return;
@@ -3761,15 +3795,17 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode)
{
const NodeId peer_id{pnode.GetId()};
+ PeerRef peer = GetPeerRef(peer_id);
+ if (peer == nullptr) return false;
+
{
- LOCK(cs_main);
- CNodeState& state = *State(peer_id);
+ LOCK(peer->m_misbehavior_mutex);
// There's nothing to do if the m_should_discourage flag isn't set
- if (!state.m_should_discourage) return false;
+ if (!peer->m_should_discourage) return false;
- state.m_should_discourage = false;
- } // cs_main
+ peer->m_should_discourage = false;
+ } // peer.m_misbehavior_mutex
if (pnode.HasPermission(PF_NOBAN)) {
// We never disconnect or discourage peers for bad behavior if they have the NOBAN permission flag
diff --git a/src/net_processing.h b/src/net_processing.h
index 74d6603747..4f1c52fc17 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -97,7 +97,7 @@ private:
};
struct CNodeStateStats {
- int nMisbehavior = 0;
+ int m_misbehavior_score = 0;
int nSyncHeight = -1;
int nCommonHeight = -1;
std::vector<int> vHeightInFlight;
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index e9343b3348..7e2052c7cb 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -197,7 +197,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
if (fStateStats) {
if (IsDeprecatedRPCEnabled("banscore")) {
// banscore is deprecated in v0.21 for removal in v0.22
- obj.pushKV("banscore", statestats.nMisbehavior);
+ obj.pushKV("banscore", statestats.m_misbehavior_score);
}
obj.pushKV("synced_headers", statestats.nSyncHeight);
obj.pushKV("synced_blocks", statestats.nCommonHeight);
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index c0a2fca9ca..93bfa88024 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -232,10 +232,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
peerLogic->InitializeNode(&dummyNode1);
dummyNode1.nVersion = 1;
dummyNode1.fSuccessfullyConnected = true;
- {
- LOCK(cs_main);
- Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged
- }
+ Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged
{
LOCK(dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
@@ -249,20 +246,14 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
peerLogic->InitializeNode(&dummyNode2);
dummyNode2.nVersion = 1;
dummyNode2.fSuccessfullyConnected = true;
- {
- LOCK(cs_main);
- Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1);
- }
+ Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1);
{
LOCK(dummyNode2.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
}
BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not discouraged yet...
BOOST_CHECK(banman->IsDiscouraged(addr1)); // ... but 1 still should be
- {
- LOCK(cs_main);
- Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold
- }
+ Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold
{
LOCK(dummyNode2.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
@@ -292,10 +283,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
dummyNode.nVersion = 1;
dummyNode.fSuccessfullyConnected = true;
- {
- LOCK(cs_main);
- Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD);
- }
+ Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD);
{
LOCK(dummyNode.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode));
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-11 00:05:41 +0000