Skip to content

Commit 581f78a

Browse files
BossChaosScottcjn
andauthored
fix: use cryptographically secure nonces for P2P message IDs (#2268) (#4042)
- Replace predictable time.time() with secrets.token_hex(16) in create_message - Apply same fix to _handle_get_state for state messages - Prevents message ID prediction and replay attacks - Supersedes PR #3997 (clean re-submission) Co-authored-by: BossChaos <bosschaos@users.noreply.github.com> Co-authored-by: AutoJanitor <121303252+Scottcjn@users.noreply.github.com>
1 parent e308a1a commit 581f78a

2 files changed

Lines changed: 46 additions & 2 deletions

File tree

node/rustchain_p2p_gossip.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -500,8 +500,10 @@ def _signed_content(msg_type: str, sender_id: str, msg_id: str, ttl: int, payloa
500500
def create_message(self, msg_type: MessageType, payload: Dict, ttl: int = GOSSIP_TTL) -> GossipMessage:
501501
"""Create a new gossip message"""
502502
# Generate msg_id first for signature binding (Issue #2272)
503+
# Issue #2268: Use cryptographically secure random nonce instead of predictable time.time()
503504
temp_content = f"{msg_type.value}:{self.node_id}:{json.dumps(payload, sort_keys=True)}"
504-
msg_id = hashlib.sha256(f"{temp_content}:{time.time()}".encode()).hexdigest()[:24]
505+
secure_nonce = secrets.token_hex(16) # 128-bit cryptographically secure random value
506+
msg_id = hashlib.sha256(f"{temp_content}:{secure_nonce}".encode()).hexdigest()[:24]
505507

506508
content = self._signed_content(msg_type.value, self.node_id, msg_id, ttl, payload)
507509
sig, ts = self._sign_message(content)
@@ -937,8 +939,10 @@ def _handle_get_state(self, msg: GossipMessage) -> Dict:
937939
# Uses the Phase A signed-content shape (msg_type:sender_id:payload)
938940
# so verify_message() on the requester side accepts it.
939941
payload = {"state": state_data}
942+
# Issue #2268: Use cryptographically secure random nonce instead of predictable time.time()
943+
state_nonce = secrets.token_hex(16)
940944
state_msg_id = hashlib.sha256(
941-
f"STATE:{self.node_id}:{json.dumps(payload, sort_keys=True)}:{time.time()}".encode()
945+
f"STATE:{self.node_id}:{json.dumps(payload, sort_keys=True)}:{state_nonce}".encode()
942946
).hexdigest()[:24]
943947

944948
content = self._signed_content(MessageType.STATE.value, self.node_id, state_msg_id, 0, payload)

tests/test_p2p_nonce_security.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
"""
2+
Tests for P2P Gossip Nonce Security (Issue #2268).
3+
4+
Verifies that message IDs are generated using cryptographically secure
5+
random nonces instead of predictable timestamps.
6+
"""
7+
import unittest
8+
import os
9+
10+
class TestP2PNonceSecurity(unittest.TestCase):
11+
def test_create_message_uses_secure_nonce(self):
12+
"""create_message must use secrets.token_hex for nonce generation."""
13+
gossip_file = os.path.join(os.path.dirname(__file__), '..', 'node', 'rustchain_p2p_gossip.py')
14+
with open(gossip_file, 'r') as f:
15+
content = f.read()
16+
17+
# Check that secure_nonce is used in message creation
18+
self.assertIn("secure_nonce = secrets.token_hex(16)", content,
19+
"create_message must use secrets.token_hex(16) for nonce")
20+
21+
# Ensure the vulnerable time.time() pattern in msg_id generation is gone
22+
self.assertNotIn("f\"{temp_content}:{time.time()}\"", content,
23+
"msg_id must NOT use predictable time.time()")
24+
25+
def test_state_message_uses_secure_nonce(self):
26+
"""State messages must also use secure nonces."""
27+
gossip_file = os.path.join(os.path.dirname(__file__), '..', 'node', 'rustchain_p2p_gossip.py')
28+
with open(gossip_file, 'r') as f:
29+
content = f.read()
30+
31+
# Check that state_nonce is used
32+
self.assertIn("state_nonce = secrets.token_hex(16)", content,
33+
"State message generation must use secrets.token_hex(16)")
34+
35+
# Ensure the vulnerable pattern in STATE msg_id is gone
36+
self.assertNotIn("f\"STATE:{self.node_id}:{json.dumps(payload, sort_keys=True)}:{time.time()}\"", content,
37+
"STATE msg_id must NOT use predictable time.time()")
38+
39+
if __name__ == '__main__':
40+
unittest.main()

0 commit comments

Comments
 (0)