Skip to content

Commit 67881de

Browse files
author
MarcoFalke
committed
Merge #19272: net, test: invalid p2p messages and test framework improvements
56010f9 test: hoist p2p values to test framework constants (Jon Atack) 75447f0 test: improve msg sends and p2p disconnections in p2p_invalid_messages (Jon Atack) 5796019 test: refactor test_large_inv() into 3 tests with common method (Jon Atack) e2b21d8 test: add p2p_invalid_messages logging (Jon Atack) 9fa494d net: update misbehavior logging for oversized messages (Jon Atack) Pull request description: ...seen while reviewing #19264, #19252, #19304 and #19107: in `net_processing.cpp` - make the debug logging for oversized message size misbehavior the same for `addr`, `getdata`, `headers` and `inv` messages in `p2p_invalid_messages` - add missing logging - improve assertions/message sends, move cleanup disconnections outside the assertion scopes - split a slowish 3-part test into 3 order-independent tests - add a few p2p constants to the test framework ACKs for top commit: troygiorshev: reACK 56010f9 MarcoFalke: ACK 56010f9 🎛 Tree-SHA512: db67b70278f8d4c318907e105af54b54eb3afd15500f9aa0c98034f6fd4bd1cf9ad1663037bd9b237ff4890f3059b37291a6498d8d6ae2cc38efb9f045f73310
2 parents 532b134 + 56010f9 commit 67881de

File tree

5 files changed

+56
-37
lines changed

5 files changed

+56
-37
lines changed

src/net_processing.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2454,7 +2454,7 @@ void ProcessMessage(
24542454
if (vAddr.size() > 1000)
24552455
{
24562456
LOCK(cs_main);
2457-
Misbehaving(pfrom.GetId(), 20, strprintf("message addr size() = %u", vAddr.size()));
2457+
Misbehaving(pfrom.GetId(), 20, strprintf("addr message size = %u", vAddr.size()));
24582458
return;
24592459
}
24602460

@@ -2530,7 +2530,7 @@ void ProcessMessage(
25302530
if (vInv.size() > MAX_INV_SZ)
25312531
{
25322532
LOCK(cs_main);
2533-
Misbehaving(pfrom.GetId(), 20, strprintf("message inv size() = %u", vInv.size()));
2533+
Misbehaving(pfrom.GetId(), 20, strprintf("inv message size = %u", vInv.size()));
25342534
return;
25352535
}
25362536

@@ -2596,7 +2596,7 @@ void ProcessMessage(
25962596
if (vInv.size() > MAX_INV_SZ)
25972597
{
25982598
LOCK(cs_main);
2599-
Misbehaving(pfrom.GetId(), 20, strprintf("message getdata size() = %u", vInv.size()));
2599+
Misbehaving(pfrom.GetId(), 20, strprintf("getdata message size = %u", vInv.size()));
26002600
return;
26012601
}
26022602

test/functional/p2p_addr_relay.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ def run_test(self):
4949
addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
5050
msg = msg_addr()
5151

52-
self.log.info('Send too large addr message')
52+
self.log.info('Send too-large addr message')
5353
msg.addrs = ADDRS * 101
54-
with self.nodes[0].assert_debug_log(['message addr size() = 1010']):
54+
with self.nodes[0].assert_debug_log(['addr message size = 1010']):
5555
addr_source.send_and_ping(msg)
5656

5757
self.log.info('Check that addr message content is relayed and added to addrman')

test/functional/p2p_invalid_messages.py

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
from test_framework.messages import (
88
CBlockHeader,
99
CInv,
10+
MAX_HEADERS_RESULTS,
11+
MAX_INV_SIZE,
12+
MAX_PROTOCOL_MESSAGE_LENGTH,
1013
msg_getdata,
1114
msg_headers,
1215
msg_inv,
@@ -24,8 +27,7 @@
2427
wait_until,
2528
)
2629

27-
MSG_LIMIT = 4 * 1000 * 1000 # 4MB, per MAX_PROTOCOL_MESSAGE_LENGTH
28-
VALID_DATA_LIMIT = MSG_LIMIT - 5 # Account for the 5-byte length prefix
30+
VALID_DATA_LIMIT = MAX_PROTOCOL_MESSAGE_LENGTH - 5 # Account for the 5-byte length prefix
2931

3032
class msg_unrecognized:
3133
"""Nonsensical message. Modeled after similar types in test_framework.messages."""
@@ -53,11 +55,13 @@ def run_test(self):
5355
self.test_checksum()
5456
self.test_size()
5557
self.test_msgtype()
56-
self.test_large_inv()
58+
self.test_oversized_inv_msg()
59+
self.test_oversized_getdata_msg()
60+
self.test_oversized_headers_msg()
5761
self.test_resource_exhaustion()
5862

5963
def test_buffer(self):
60-
self.log.info("Test message with header split across two buffers, should be received")
64+
self.log.info("Test message with header split across two buffers is received")
6165
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
6266
# Create valid message
6367
msg = conn.build_message(msg_ping(nonce=12345))
@@ -76,83 +80,94 @@ def test_buffer(self):
7680
self.nodes[0].disconnect_p2ps()
7781

7882
def test_magic_bytes(self):
83+
self.log.info("Test message with invalid magic bytes disconnects peer")
7984
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
8085
with self.nodes[0].assert_debug_log(['PROCESSMESSAGE: INVALID MESSAGESTART badmsg']):
8186
msg = conn.build_message(msg_unrecognized(str_data="d"))
8287
# modify magic bytes
8388
msg = b'\xff' * 4 + msg[4:]
8489
conn.send_raw_message(msg)
8590
conn.wait_for_disconnect(timeout=1)
86-
self.nodes[0].disconnect_p2ps()
91+
self.nodes[0].disconnect_p2ps()
8792

8893
def test_checksum(self):
94+
self.log.info("Test message with invalid checksum logs an error")
8995
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
9096
with self.nodes[0].assert_debug_log(['CHECKSUM ERROR (badmsg, 2 bytes), expected 78df0a04 was ffffffff']):
9197
msg = conn.build_message(msg_unrecognized(str_data="d"))
9298
# Checksum is after start bytes (4B), message type (12B), len (4B)
9399
cut_len = 4 + 12 + 4
94100
# modify checksum
95101
msg = msg[:cut_len] + b'\xff' * 4 + msg[cut_len + 4:]
96-
self.nodes[0].p2p.send_raw_message(msg)
102+
conn.send_raw_message(msg)
97103
conn.sync_with_ping(timeout=1)
98-
self.nodes[0].disconnect_p2ps()
104+
self.nodes[0].disconnect_p2ps()
99105

100106
def test_size(self):
107+
self.log.info("Test message with oversized payload disconnects peer")
101108
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
102109
with self.nodes[0].assert_debug_log(['']):
103-
# Create a message with oversized payload
104110
msg = msg_unrecognized(str_data="d" * (VALID_DATA_LIMIT + 1))
105111
msg = conn.build_message(msg)
106-
self.nodes[0].p2p.send_raw_message(msg)
112+
conn.send_raw_message(msg)
107113
conn.wait_for_disconnect(timeout=1)
108-
self.nodes[0].disconnect_p2ps()
114+
self.nodes[0].disconnect_p2ps()
109115

110116
def test_msgtype(self):
117+
self.log.info("Test message with invalid message type logs an error")
111118
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
112119
with self.nodes[0].assert_debug_log(['PROCESSMESSAGE: ERRORS IN HEADER']):
113120
msg = msg_unrecognized(str_data="d")
114121
msg.msgtype = b'\xff' * 12
115122
msg = conn.build_message(msg)
116123
# Modify msgtype
117124
msg = msg[:7] + b'\x00' + msg[7 + 1:]
118-
self.nodes[0].p2p.send_raw_message(msg)
125+
conn.send_raw_message(msg)
119126
conn.sync_with_ping(timeout=1)
120-
self.nodes[0].disconnect_p2ps()
121-
122-
def test_large_inv(self):
123-
conn = self.nodes[0].add_p2p_connection(P2PInterface())
124-
with self.nodes[0].assert_debug_log(['Misbehaving', '(0 -> 20): message inv size() = 50001']):
125-
msg = msg_inv([CInv(MSG_TX, 1)] * 50001)
126-
conn.send_and_ping(msg)
127-
with self.nodes[0].assert_debug_log(['Misbehaving', '(20 -> 40): message getdata size() = 50001']):
128-
msg = msg_getdata([CInv(MSG_TX, 1)] * 50001)
129-
conn.send_and_ping(msg)
130-
with self.nodes[0].assert_debug_log(['Misbehaving', '(40 -> 60): headers message size = 2001']):
131-
msg = msg_headers([CBlockHeader()] * 2001)
132-
conn.send_and_ping(msg)
133127
self.nodes[0].disconnect_p2ps()
134128

129+
def test_oversized_msg(self, msg, size):
130+
msg_type = msg.msgtype.decode('ascii')
131+
self.log.info("Test {} message of size {} is logged as misbehaving".format(msg_type, size))
132+
with self.nodes[0].assert_debug_log(['Misbehaving', '{} message size = {}'.format(msg_type, size)]):
133+
self.nodes[0].add_p2p_connection(P2PInterface()).send_and_ping(msg)
134+
self.nodes[0].disconnect_p2ps()
135+
136+
def test_oversized_inv_msg(self):
137+
size = MAX_INV_SIZE + 1
138+
self.test_oversized_msg(msg_inv([CInv(MSG_TX, 1)] * size), size)
139+
140+
def test_oversized_getdata_msg(self):
141+
size = MAX_INV_SIZE + 1
142+
self.test_oversized_msg(msg_getdata([CInv(MSG_TX, 1)] * size), size)
143+
144+
def test_oversized_headers_msg(self):
145+
size = MAX_HEADERS_RESULTS + 1
146+
self.test_oversized_msg(msg_headers([CBlockHeader()] * size), size)
147+
135148
def test_resource_exhaustion(self):
149+
self.log.info("Test node stays up despite many large junk messages")
136150
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
137151
conn2 = self.nodes[0].add_p2p_connection(P2PDataStore())
138152
msg_at_size = msg_unrecognized(str_data="b" * VALID_DATA_LIMIT)
139-
assert len(msg_at_size.serialize()) == MSG_LIMIT
140-
141-
self.log.info("Sending a bunch of large, junk messages to test memory exhaustion. May take a bit...")
153+
assert len(msg_at_size.serialize()) == MAX_PROTOCOL_MESSAGE_LENGTH
142154

143-
# Run a bunch of times to test for memory exhaustion.
155+
self.log.info("(a) Send 80 messages, each of maximum valid data size (4MB)")
144156
for _ in range(80):
145157
conn.send_message(msg_at_size)
146158

147159
# Check that, even though the node is being hammered by nonsense from one
148160
# connection, it can still service other peers in a timely way.
161+
self.log.info("(b) Check node still services peers in a timely way")
149162
for _ in range(20):
150163
conn2.sync_with_ping(timeout=2)
151164

152-
# Peer 1, despite being served up a bunch of nonsense, should still be connected.
153-
self.log.info("Waiting for node to drop junk messages.")
165+
self.log.info("(c) Wait for node to drop junk messages, while remaining connected")
154166
conn.sync_with_ping(timeout=400)
167+
168+
# Despite being served up a bunch of nonsense, the peers should still be connected.
155169
assert conn.is_connected
170+
assert conn2.is_connected
156171
self.nodes[0].disconnect_p2ps()
157172

158173

test/functional/test_framework/messages.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@
4545

4646
BIP125_SEQUENCE_NUMBER = 0xfffffffd # Sequence number that is BIP 125 opt-in and BIP 68-opt-out
4747

48+
MAX_PROTOCOL_MESSAGE_LENGTH = 4000000 # Maximum length of incoming protocol messages
49+
MAX_HEADERS_RESULTS = 2000 # Number of headers sent in one getheaders result
50+
MAX_INV_SIZE = 50000 # Maximum number of entries in an 'inv' protocol message
51+
4852
NODE_NETWORK = (1 << 0)
4953
NODE_GETUTXO = (1 << 1)
5054
NODE_BLOOM = (1 << 2)

test/functional/test_framework/mininode.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
from test_framework.messages import (
2828
CBlockHeader,
29+
MAX_HEADERS_RESULTS,
2930
MIN_VERSION_SUPPORTED,
3031
msg_addr,
3132
msg_block,
@@ -553,7 +554,6 @@ def on_getheaders(self, message):
553554
return
554555

555556
headers_list = [self.block_store[self.last_block_hash]]
556-
maxheaders = 2000
557557
while headers_list[-1].sha256 not in locator.vHave:
558558
# Walk back through the block store, adding headers to headers_list
559559
# as we go.
@@ -569,7 +569,7 @@ def on_getheaders(self, message):
569569
break
570570

571571
# Truncate the list if there are too many headers
572-
headers_list = headers_list[:-maxheaders - 1:-1]
572+
headers_list = headers_list[:-MAX_HEADERS_RESULTS - 1:-1]
573573
response = msg_headers(headers_list)
574574

575575
if response is not None:

0 commit comments

Comments
 (0)