From 8fccbe69b93b9703156e4c8a2c282e62ca533f7e Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 22 Jul 2025 11:06:57 +0200 Subject: [PATCH] Fix session destruction race condition causing handshake delays Root cause: When receiving a handshake initiation (32 bytes) from a peer with whom we already had an established session, the code would destroy the existing session to "help" the other side. This created a cascade: - Peer A completes handshake with Peer B - Peer A sends message, realizes no session, initiates handshake - Peer B destroys its working session to "help" - Peer B now has no session, initiates handshake - Both peers keep destroying each other's sessions Fix: - Never destroy an established session when receiving new handshake attempts - Add early check in handshake initiation to skip if session exists - Clear handshake rate limit timers when session already established This eliminates the delays and repeated handshakes seen in the logs. --- bitchat/Noise/NoiseSession.swift | 15 ++++----------- bitchat/Services/BluetoothMeshService.swift | 8 ++++++++ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/bitchat/Noise/NoiseSession.swift b/bitchat/Noise/NoiseSession.swift index da873cb9..2f8b2eb7 100644 --- a/bitchat/Noise/NoiseSession.swift +++ b/bitchat/Noise/NoiseSession.swift @@ -319,18 +319,11 @@ class NoiseSessionManager { var existingSession: NoiseSession? = nil if let existing = sessions[peerID] { - // If we have an established session, we might need to help the other side complete theirs + // If we have an established session, reject new handshake attempts if existing.isEstablished() { - // If this is a handshake initiation (32 bytes), the other side doesn't have a session - // We should complete the handshake to help them establish their session - if message.count == 32 { - // Remove existing session and create new one - sessions.removeValue(forKey: peerID) - shouldCreateNew = true - } else { - // For other handshake messages, ignore if already established - throw NoiseSessionError.alreadyEstablished - } + // Don't destroy our working session just because the other side is confused + // They should detect the established session through successful message exchange + throw NoiseSessionError.alreadyEstablished } else { // If we're in the middle of a handshake and receive a new initiation, // reset and start fresh (the other side may have restarted) diff --git a/bitchat/Services/BluetoothMeshService.swift b/bitchat/Services/BluetoothMeshService.swift index 29a9f0a5..56b4d641 100644 --- a/bitchat/Services/BluetoothMeshService.swift +++ b/bitchat/Services/BluetoothMeshService.swift @@ -3368,6 +3368,14 @@ extension BluetoothMeshService: CBPeripheralManagerDelegate { SecurityLogger.log("Initiating Noise handshake with \(peerID)", category: SecurityLogger.noise, level: .info) + // Check if we already have an established session + if noiseService.hasEstablishedSession(with: peerID) { + SecurityLogger.log("Already have established session with \(peerID)", category: SecurityLogger.noise, level: .debug) + // Clear any lingering handshake attempt time + handshakeAttemptTimes.removeValue(forKey: peerID) + return + } + // Check if we've recently tried to handshake with this peer if let lastAttempt = handshakeAttemptTimes[peerID], Date().timeIntervalSince(lastAttempt) < handshakeTimeout {