Skip to content

Commit

Permalink
net_processing: make all Misbehaving increments = 100
Browse files Browse the repository at this point in the history
This removes the need to actually track misbehavior score (see further commit), because any
Misbehaving node will immediately hit the discouragement threshold.
  • Loading branch information
sipa committed Mar 12, 2024
1 parent a1ac6b7 commit 3cbf1f3
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 23 deletions.
13 changes: 6 additions & 7 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1850,8 +1850,7 @@ bool PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati
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)
if (peer) Misbehaving(*peer, 10, message);
if (peer) Misbehaving(*peer, 100, message);
return true;
case BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE:
case BlockValidationResult::BLOCK_TIME_FUTURE:
Expand Down Expand Up @@ -2550,7 +2549,7 @@ bool PeerManagerImpl::CheckHeadersPoW(const std::vector<CBlockHeader>& headers,

// Are these headers connected to each other?
if (!CheckHeadersAreContinuous(headers)) {
Misbehaving(peer, 20, "non-continuous headers sequence");
Misbehaving(peer, 100, "non-continuous headers sequence");
return false;
}
return true;
Expand Down Expand Up @@ -3814,7 +3813,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,

if (vAddr.size() > MAX_ADDR_TO_SEND)
{
Misbehaving(*peer, 20, strprintf("%s message size = %u", msg_type, vAddr.size()));
Misbehaving(*peer, 100, strprintf("%s message size = %u", msg_type, vAddr.size()));
return;
}

Expand Down Expand Up @@ -3896,7 +3895,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
vRecv >> vInv;
if (vInv.size() > MAX_INV_SZ)
{
Misbehaving(*peer, 20, strprintf("inv message size = %u", vInv.size()));
Misbehaving(*peer, 100, strprintf("inv message size = %u", vInv.size()));
return;
}

Expand Down Expand Up @@ -3988,7 +3987,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
vRecv >> vInv;
if (vInv.size() > MAX_INV_SZ)
{
Misbehaving(*peer, 20, strprintf("getdata message size = %u", vInv.size()));
Misbehaving(*peer, 100, strprintf("getdata message size = %u", vInv.size()));
return;
}

Expand Down Expand Up @@ -4679,7 +4678,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
// 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) {
Misbehaving(*peer, 20, strprintf("headers message size = %u", nCount));
Misbehaving(*peer, 100, strprintf("headers message size = %u", nCount));
return;
}
headers.resize(nCount);
Expand Down
3 changes: 1 addition & 2 deletions src/test/denialofservice_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,15 +352,14 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
peerLogic->InitializeNode(*nodes[1], NODE_NETWORK);
nodes[1]->fSuccessfullyConnected = true;
connman->AddTestNode(*nodes[1]);
peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), DISCOURAGEMENT_THRESHOLD - 1);
BOOST_CHECK(peerLogic->SendMessages(nodes[1]));
// [0] is still discouraged/disconnected.
BOOST_CHECK(banman->IsDiscouraged(addr[0]));
BOOST_CHECK(nodes[0]->fDisconnect);
// [1] is not discouraged/disconnected yet.
BOOST_CHECK(!banman->IsDiscouraged(addr[1]));
BOOST_CHECK(!nodes[1]->fDisconnect);
peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), 1); // [1] reaches discouragement threshold
peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), DISCOURAGEMENT_THRESHOLD); // [1] reaches discouragement threshold
BOOST_CHECK(peerLogic->SendMessages(nodes[1]));
// Expect both [0] and [1] to be discouraged/disconnected now.
BOOST_CHECK(banman->IsDiscouraged(addr[0]));
Expand Down
3 changes: 2 additions & 1 deletion test/functional/p2p_addr_relay.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ def oversized_addr_test(self):

msg = self.setup_addr_msg(1010)
with self.nodes[0].assert_debug_log(['addr message size = 1010']):
addr_source.send_and_ping(msg)
addr_source.send_message(msg)
addr_source.wait_for_disconnect()

self.nodes[0].disconnect_p2ps()

Expand Down
12 changes: 7 additions & 5 deletions test/functional/p2p_addrv2_relay.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,6 @@ def run_test(self):
addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
msg = msg_addrv2()

self.log.info('Send too-large addrv2 message')
msg.addrs = ADDRS * 101
with self.nodes[0].assert_debug_log(['addrv2 message size = 1010']):
addr_source.send_and_ping(msg)

self.log.info('Check that addrv2 message content is relayed and added to addrman')
addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver())
msg.addrs = ADDRS
Expand All @@ -99,6 +94,13 @@ def run_test(self):
assert addr_receiver.addrv2_received_and_checked
assert_equal(len(self.nodes[0].getnodeaddresses(count=0, network="i2p")), 0)

self.log.info('Send too-large addrv2 message')
msg.addrs = ADDRS * 101
with self.nodes[0].assert_debug_log(['addrv2 message size = 1010']):
addr_source.send_message(msg)
addr_source.wait_for_disconnect()



if __name__ == '__main__':
AddrTest().main()
7 changes: 5 additions & 2 deletions test/functional/p2p_invalid_messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,9 @@ def test_oversized_msg(self, msg, size):
msg_type = msg.msgtype.decode('ascii')
self.log.info("Test {} message of size {} is logged as misbehaving".format(msg_type, size))
with self.nodes[0].assert_debug_log(['Misbehaving', '{} message size = {}'.format(msg_type, size)]):
self.nodes[0].add_p2p_connection(P2PInterface()).send_and_ping(msg)
conn = self.nodes[0].add_p2p_connection(P2PInterface())
conn.send_message(msg)
conn.wait_for_disconnect()
self.nodes[0].disconnect_p2ps()

def test_oversized_inv_msg(self):
Expand Down Expand Up @@ -322,7 +324,8 @@ def test_noncontinuous_headers_msg(self):
# delete arbitrary block header somewhere in the middle to break link
del block_headers[random.randrange(1, len(block_headers)-1)]
with self.nodes[0].assert_debug_log(expected_msgs=MISBEHAVING_NONCONTINUOUS_HEADERS_MSGS):
peer.send_and_ping(msg_headers(block_headers))
peer.send_message(msg_headers(block_headers))
peer.wait_for_disconnect()
self.nodes[0].disconnect_p2ps()

def test_resource_exhaustion(self):
Expand Down
9 changes: 4 additions & 5 deletions test/functional/p2p_mutated_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,10 @@ def self_transfer_requested():
block_missing_prev.hashPrevBlock = 123
block_missing_prev.solve()

# Attacker gets a DoS score of 10, not immediately disconnected, so we do it 10 times to get to 100
for _ in range(10):
assert_equal(len(self.nodes[0].getpeerinfo()), 2)
with self.nodes[0].assert_debug_log(expected_msgs=["AcceptBlock FAILED (prev-blk-not-found)"]):
attacker.send_message(msg_block(block_missing_prev))
# Check that non-connecting block causes disconnect
assert_equal(len(self.nodes[0].getpeerinfo()), 2)
with self.nodes[0].assert_debug_log(expected_msgs=["AcceptBlock FAILED (prev-blk-not-found)"]):
attacker.send_message(msg_block(block_missing_prev))
attacker.wait_for_disconnect(timeout=5)


Expand Down
4 changes: 3 additions & 1 deletion test/functional/p2p_unrequested_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,11 @@ def run_test(self):
tip = next_block

# Now send the block at height 5 and check that it wasn't accepted (missing header)
test_node.send_and_ping(msg_block(all_blocks[1]))
test_node.send_message(msg_block(all_blocks[1]))
test_node.wait_for_disconnect()
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblock, all_blocks[1].hash)
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblockheader, all_blocks[1].hash)
test_node = self.nodes[0].add_p2p_connection(P2PInterface())

# The block at height 5 should be accepted if we provide the missing header, though
headers_message = msg_headers()
Expand Down

0 comments on commit 3cbf1f3

Please sign in to comment.