bitcoin/test/functional/p2p_mutated_blocks.py
Ava Chow a52837b9e9
Merge bitcoin/bitcoin#29575: net_processing: make any misbehavior trigger immediate discouragement
6eecba475e net_processing: make MaybePunishNodeFor{Block,Tx} return void (Pieter Wuille)
ae60d485da net_processing: remove Misbehavior score and increments (Pieter Wuille)
6457c31197 net_processing: make all Misbehaving increments = 100 (Pieter Wuille)
5120ab1478 net_processing: drop 8 headers threshold for incoming BIP130 (Pieter Wuille)
944c54290d net_processing: drop Misbehavior for unconnecting headers (Pieter Wuille)
9f66ac7cf1 net_processing: do not treat non-connecting headers as response (Pieter Wuille)

Pull request description:

  So far, discouragement of peers triggers when their misbehavior score exceeds 100 points. Most types of misbehavior increment the score by 100, triggering immediate discouragement, but some types do not. This PR makes all increments equal to either 100 (meaning any misbehavior will immediately cause disconnection and discouragement) or 0 (making the behavior effectively unconditionally allowed), and then removes the logic for score accumulation.

  This simplifies the code a bit, but also makes protocol expectations clearer: if a peer misbehaves, they get disconnected. There is no good reason why certain types of protocol violations should be permitted 4 times (howmuch=20) or 9 times (howmuch=10), while many others are never allowed. Furthermore, the distinction between these looks arbitrary.

  The specific types of misbehavior that are changed to 100 are:
  * Sending us a `block` which does not connect to our header tree (which necessarily must have been unsollicited). [used to be score 10]
  * Sending us a `headers` with a non-continuous headers sequence. [used to be score 20]
  * Sending us more than 1000 addresses in a single `addr` or `addrv2` message [used to be score 20]
  * Sending us more than 50000 invs in a single `inv` message [used to be score 20]
  * Sending us more than 2000 headers in a single `headers` message [used to be score 20]

  The specific types of misbehavior that are changed to 0 are:
  * Sending us 10 (*) separate BIP130 headers announcements that do not connect to our block tree [used to be score 20]
  * Sending us more than 8 headers in a single `headers` message (which thus does not get treated as a BIP130 announcement) that does not connect to our block tree. [used to be score 10]

  I believe that none of these behaviors are unavoidable, except for the one marked (*) which can in theory happen still due to interaction between BIP130 and variations in system clocks (the max 2 hour in the future rule). This one has been removed entirely. In order to remove the impact of the bug it was designed to deal with, without relying on misbehavior, a separate improvement is included that makes `getheaders`-tracking more accurate.

  In another unrelated improvement, this also gets rid of the 8 header limit heuristic to determine whether an incoming non-connecting `headers` is a potential BIP130 announcement, as this rule is no longer needed to prevent spurious Misbehavior. Instead, any non-connecting `headers` is now treated as a potential announcement.

ACKs for top commit:
  sr-gi:
    ACK [6eecba4](6eecba475e)
  achow101:
    ACK 6eecba475e
  mzumsande:
    Code Review ACK 6eecba475e
  glozow:
    light code review / concept ACK 6eecba475e

Tree-SHA512: e11e8a652c4ec048d8961086110a3594feefbb821e13f45c14ef81016377be0db44b5311751ef635d6e026def1960aff33f644e78ece11cfb54f2b7daa96f946
2024-06-20 13:28:38 -04:00

115 lines
4.9 KiB
Python
Executable file

#!/usr/bin/env python3
# Copyright (c) The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""
Test that an attacker can't degrade compact block relay by sending unsolicited
mutated blocks to clear in-flight blocktxn requests from other honest peers.
"""
from test_framework.p2p import P2PInterface
from test_framework.messages import (
BlockTransactions,
msg_cmpctblock,
msg_block,
msg_blocktxn,
HeaderAndShortIDs,
)
from test_framework.test_framework import BitcoinTestFramework
from test_framework.blocktools import (
COINBASE_MATURITY,
create_block,
add_witness_commitment,
NORMAL_GBT_REQUEST_PARAMS,
)
from test_framework.util import assert_equal
from test_framework.wallet import MiniWallet
import copy
class MutatedBlocksTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
self.num_nodes = 1
self.extra_args = [
[
"-testactivationheight=segwit@1", # causes unconnected headers/blocks to not have segwit considered deployed
],
]
def run_test(self):
self.wallet = MiniWallet(self.nodes[0])
self.generate(self.wallet, COINBASE_MATURITY)
honest_relayer = self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="outbound-full-relay")
attacker = self.nodes[0].add_p2p_connection(P2PInterface())
# Create new block with two transactions (coinbase + 1 self-transfer).
# The self-transfer transaction is needed to trigger a compact block
# `getblocktxn` roundtrip.
tx = self.wallet.create_self_transfer()["tx"]
block = create_block(tmpl=self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS), txlist=[tx])
add_witness_commitment(block)
block.solve()
# Create mutated version of the block by changing the transaction
# version on the self-transfer.
mutated_block = copy.deepcopy(block)
mutated_block.vtx[1].version = 4
# Announce the new block via a compact block through the honest relayer
cmpctblock = HeaderAndShortIDs()
cmpctblock.initialize_from_block(block, use_witness=True)
honest_relayer.send_message(msg_cmpctblock(cmpctblock.to_p2p()))
# Wait for a `getblocktxn` that attempts to fetch the self-transfer
def self_transfer_requested():
if not honest_relayer.last_message.get('getblocktxn'):
return False
get_block_txn = honest_relayer.last_message['getblocktxn']
return get_block_txn.block_txn_request.blockhash == block.sha256 and \
get_block_txn.block_txn_request.indexes == [1]
honest_relayer.wait_until(self_transfer_requested, timeout=5)
# Block at height 101 should be the only one in flight from peer 0
peer_info_prior_to_attack = self.nodes[0].getpeerinfo()
assert_equal(peer_info_prior_to_attack[0]['id'], 0)
assert_equal([101], peer_info_prior_to_attack[0]["inflight"])
# Attempt to clear the honest relayer's download request by sending the
# mutated block (as the attacker).
with self.nodes[0].assert_debug_log(expected_msgs=["Block mutated: bad-txnmrklroot, hashMerkleRoot mismatch"]):
attacker.send_message(msg_block(mutated_block))
# Attacker should get disconnected for sending a mutated block
attacker.wait_for_disconnect(timeout=5)
# Block at height 101 should *still* be the only block in-flight from
# peer 0
peer_info_after_attack = self.nodes[0].getpeerinfo()
assert_equal(peer_info_after_attack[0]['id'], 0)
assert_equal([101], peer_info_after_attack[0]["inflight"])
# The honest relayer should be able to complete relaying the block by
# sending the blocktxn that was requested.
block_txn = msg_blocktxn()
block_txn.block_transactions = BlockTransactions(blockhash=block.sha256, transactions=[tx])
honest_relayer.send_and_ping(block_txn)
assert_equal(self.nodes[0].getbestblockhash(), block.hash)
# Check that unexpected-witness mutation check doesn't trigger on a header that doesn't connect to anything
assert_equal(len(self.nodes[0].getpeerinfo()), 1)
attacker = self.nodes[0].add_p2p_connection(P2PInterface())
block_missing_prev = copy.deepcopy(block)
block_missing_prev.hashPrevBlock = 123
block_missing_prev.solve()
# 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)
if __name__ == '__main__':
MutatedBlocksTest().main()