Skip to content

Commit fad68af

Browse files
author
MarcoFalke
committed
p2p: Ignore non-version msgs before version msg
Sending a non-version message before the initial version message is peer misbehavior. Though, it seems arbitrary and confusing to disconnect only after exactly 100 non-version messages. So remove the Misbehaving and instead rely on the existing disconnect-due-to-handshake-timeout logic.
1 parent ea79265 commit fad68af

File tree

3 files changed

+6
-21
lines changed

3 files changed

+6
-21
lines changed

src/net_processing.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2477,7 +2477,7 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
24772477

24782478
if (pfrom.nVersion == 0) {
24792479
// Must have a version message before anything else
2480-
Misbehaving(pfrom.GetId(), 1, "non-version message before version handshake");
2480+
LogPrint(BCLog::NET, "non-version message before version handshake. Message \"%s\" from peer=%d\n", SanitizeString(msg_type), pfrom.GetId());
24812481
return;
24822482
}
24832483

test/functional/p2p_leak.py

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
assert_greater_than_or_equal,
2525
)
2626

27-
DISCOURAGEMENT_THRESHOLD = 100
28-
2927

3028
class LazyPeer(P2PInterface):
3129
def __init__(self):
@@ -93,27 +91,16 @@ def set_test_params(self):
9391
self.num_nodes = 1
9492

9593
def run_test(self):
96-
# Peer that never sends a version. We will send a bunch of messages
97-
# from this peer anyway and verify eventual disconnection.
98-
no_version_disconnect_peer = self.nodes[0].add_p2p_connection(
99-
LazyPeer(), send_version=False, wait_for_verack=False)
100-
10194
# Another peer that never sends a version, nor any other messages. It shouldn't receive anything from the node.
10295
no_version_idle_peer = self.nodes[0].add_p2p_connection(LazyPeer(), send_version=False, wait_for_verack=False)
10396

10497
# Peer that sends a version but not a verack.
10598
no_verack_idle_peer = self.nodes[0].add_p2p_connection(NoVerackIdlePeer(), wait_for_verack=False)
10699

107-
# Send enough ping messages (any non-version message will do) prior to sending
108-
# version to reach the peer discouragement threshold. This should get us disconnected.
109-
for _ in range(DISCOURAGEMENT_THRESHOLD):
110-
no_version_disconnect_peer.send_message(msg_ping())
111-
112100
# Wait until we got the verack in response to the version. Though, don't wait for the node to receive the
113101
# verack, since we never sent one
114102
no_verack_idle_peer.wait_for_verack()
115103

116-
no_version_disconnect_peer.wait_until(lambda: no_version_disconnect_peer.ever_connected, check_connected=False)
117104
no_version_idle_peer.wait_until(lambda: no_version_idle_peer.ever_connected)
118105
no_verack_idle_peer.wait_until(lambda: no_verack_idle_peer.version_received)
119106

@@ -123,13 +110,9 @@ def run_test(self):
123110
#Give the node enough time to possibly leak out a message
124111
time.sleep(5)
125112

126-
# Expect this peer to be disconnected for misbehavior
127-
assert not no_version_disconnect_peer.is_connected
128-
129113
self.nodes[0].disconnect_p2ps()
130114

131115
# Make sure no unexpected messages came in
132-
assert no_version_disconnect_peer.unexpected_msg == False
133116
assert no_version_idle_peer.unexpected_msg == False
134117
assert no_verack_idle_peer.unexpected_msg == False
135118

@@ -148,7 +131,7 @@ def run_test(self):
148131
p2p_old_peer = self.nodes[0].add_p2p_connection(P2PInterface(), send_version=False, wait_for_verack=False)
149132
old_version_msg = msg_version()
150133
old_version_msg.nVersion = 31799
151-
with self.nodes[0].assert_debug_log(['peer=4 using obsolete version 31799; disconnecting']):
134+
with self.nodes[0].assert_debug_log(['peer=3 using obsolete version 31799; disconnecting']):
152135
p2p_old_peer.send_message(old_version_msg)
153136
p2p_old_peer.wait_for_disconnect()
154137

test/functional/p2p_timeouts.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,10 @@ def run_test(self):
5757
assert no_version_node.is_connected
5858
assert no_send_node.is_connected
5959

60-
no_verack_node.send_message(msg_ping())
61-
no_version_node.send_message(msg_ping())
60+
with self.nodes[0].assert_debug_log(['Unsupported message "ping" prior to verack from peer=0']):
61+
no_verack_node.send_message(msg_ping())
62+
with self.nodes[0].assert_debug_log(['non-version message before version handshake. Message "ping" from peer=1']):
63+
no_version_node.send_message(msg_ping())
6264

6365
sleep(1)
6466

0 commit comments

Comments
 (0)