diff options
author | MarcoFalke <falke.marco@gmail.com> | 2021-05-09 07:01:20 +0200 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2021-05-09 07:01:26 +0200 |
commit | 5925f1e652768a9502831b9ccf78d16cf3c37d29 (patch) | |
tree | 054b67cdf7a917fe6bd558879449eab1b9246950 /test | |
parent | 8d5a0583c497a67baa3db25fc4b359cf76685d83 (diff) | |
parent | 09205b33aa74e385caa2803aa6febc18ad1efa32 (diff) | |
download | bitcoin-5925f1e652768a9502831b9ccf78d16cf3c37d29.tar.xz |
Merge bitcoin/bitcoin#21872: net: Sanitize message type for logging
09205b33aa74e385caa2803aa6febc18ad1efa32 net: Clarify message header validation errors (W. J. van der Laan)
955eee76803c098978cf0bbc7f1f6d3c230544e2 net: Sanitize message type for logging (W. J. van der Laan)
Pull request description:
- Use `SanitizeString` when logging message errors to make sure that the message type is sanitized. I have checked all logging in `net.cpp`.
- For the `MESSAGESTART` error don't inspect and log header details at all: receiving invalid start bytes makes it likely that the packet isn't even formatted as valid P2P message. Logging the four unexpected start bytes (as hex) should be enough.
- Update `p2p_invalid_messages.py` test to check this.
- Improve error messages in a second commit.
Issue reported by gmaxwell.
ACKs for top commit:
MarcoFalke:
re-ACK 09205b33aa74e385caa2803aa6febc18ad1efa32 only change is log message fixup 🔂
practicalswift:
re-ACK 09205b33aa74e385caa2803aa6febc18ad1efa32
Tree-SHA512: 8fe5326af135cfcf39ea953d9074a8c966b9b85a810b06a2c45b8a745cf115de4f321e72fc769709d6bbecfc5953aab83176db6735b04c0bc6796f59272cadce
Diffstat (limited to 'test')
-rwxr-xr-x | test/functional/p2p_invalid_messages.py | 10 |
1 files changed, 5 insertions, 5 deletions
diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index c0b3c2cb12..788a81d4af 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -37,7 +37,7 @@ VALID_DATA_LIMIT = MAX_PROTOCOL_MESSAGE_LENGTH - 5 # Account for the 5-byte len class msg_unrecognized: """Nonsensical message. Modeled after similar types in test_framework.messages.""" - msgtype = b'badmsg' + msgtype = b'badmsg\x01' def __init__(self, *, str_data): self.str_data = str_data.encode() if not isinstance(str_data, bytes) else str_data @@ -104,7 +104,7 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_magic_bytes(self): self.log.info("Test message with invalid magic bytes disconnects peer") conn = self.nodes[0].add_p2p_connection(P2PDataStore()) - with self.nodes[0].assert_debug_log(['HEADER ERROR - MESSAGESTART (badmsg, 2 bytes), received ffffffff']): + with self.nodes[0].assert_debug_log(['Header error: Wrong MessageStart ffffffff received']): msg = conn.build_message(msg_unrecognized(str_data="d")) # modify magic bytes msg = b'\xff' * 4 + msg[4:] @@ -115,7 +115,7 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_checksum(self): self.log.info("Test message with invalid checksum logs an error") conn = self.nodes[0].add_p2p_connection(P2PDataStore()) - with self.nodes[0].assert_debug_log(['CHECKSUM ERROR (badmsg, 2 bytes), expected 78df0a04 was ffffffff']): + with self.nodes[0].assert_debug_log(['Header error: Wrong checksum (badmsg, 2 bytes), expected 78df0a04 was ffffffff']): msg = conn.build_message(msg_unrecognized(str_data="d")) # Checksum is after start bytes (4B), message type (12B), len (4B) cut_len = 4 + 12 + 4 @@ -130,7 +130,7 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_size(self): self.log.info("Test message with oversized payload disconnects peer") conn = self.nodes[0].add_p2p_connection(P2PDataStore()) - with self.nodes[0].assert_debug_log(['HEADER ERROR - SIZE (badmsg, 4000001 bytes)']): + with self.nodes[0].assert_debug_log(['Header error: Size too large (badmsg, 4000001 bytes)']): msg = msg_unrecognized(str_data="d" * (VALID_DATA_LIMIT + 1)) msg = conn.build_message(msg) conn.send_raw_message(msg) @@ -140,7 +140,7 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_msgtype(self): self.log.info("Test message with invalid message type logs an error") conn = self.nodes[0].add_p2p_connection(P2PDataStore()) - with self.nodes[0].assert_debug_log(['HEADER ERROR - COMMAND']): + with self.nodes[0].assert_debug_log(['Header error: Invalid message type']): msg = msg_unrecognized(str_data="d") msg = conn.build_message(msg) # Modify msgtype |