diff --git a/bitchat/Noise/NoiseSessionManager.swift b/bitchat/Noise/NoiseSessionManager.swift index 7c14a991..ca2d8217 100644 --- a/bitchat/Noise/NoiseSessionManager.swift +++ b/bitchat/Noise/NoiseSessionManager.swift @@ -11,6 +11,11 @@ import CryptoKit import Foundation import BitFoundation +struct NoiseHandshakeProcessingResult { + let response: Data? + let didEstablishAuthenticatedSession: Bool +} + final class NoiseSessionManager { private var sessions: [PeerID: NoiseSession] = [:] /// A responder rehandshake must not evict a working transport session @@ -112,6 +117,21 @@ final class NoiseSessionManager { } func handleIncomingHandshake(from peerID: PeerID, message: Data) throws -> Data? { + try handleIncomingHandshakeWithResult( + from: peerID, + message: message + ).response + } + + /// Processes one exact handshake candidate and reports whether that + /// candidate completed authenticated establishment. The peer's retained + /// session may already be established while a replacement is only on + /// message one, so callers must not infer candidate completion from the + /// peer-level session table. + func handleIncomingHandshakeWithResult( + from peerID: PeerID, + message: Data + ) throws -> NoiseHandshakeProcessingResult { // Process everything within the synchronized block to prevent race conditions return try managerQueue.sync(flags: .barrier) { let session: NoiseSession @@ -164,8 +184,11 @@ final class NoiseSessionManager { do { let response = try session.processHandshakeMessage(message) - // Check if session is established after processing - if session.isEstablished() { + // Check the exact session that processed this message. A + // preserved peer-level session can remain established while a + // replacement candidate is still unauthenticated. + let didEstablishAuthenticatedSession = session.isEstablished() + if didEstablishAuthenticatedSession { guard let remoteKey = session.getRemoteStaticPublicKey(), authenticatedRemoteKey(remoteKey, matches: peerID) else { throw NoiseSessionError.peerIdentityMismatch @@ -185,7 +208,11 @@ final class NoiseSessionManager { } } - return response + return NoiseHandshakeProcessingResult( + response: response, + didEstablishAuthenticatedSession: + didEstablishAuthenticatedSession + ) } catch { // A failed candidate is discarded without touching the // established session. Ordinary failed handshakes retain the diff --git a/bitchat/Services/BLE/BLENoisePacketHandler.swift b/bitchat/Services/BLE/BLENoisePacketHandler.swift index dcced8b2..674a7bd5 100644 --- a/bitchat/Services/BLE/BLENoisePacketHandler.swift +++ b/bitchat/Services/BLE/BLENoisePacketHandler.swift @@ -2,6 +2,11 @@ import BitFoundation import BitLogger import Foundation +struct BLENoiseHandshakeHandlingResult { + let processed: Bool + let didEstablishAuthenticatedSession: Bool +} + /// Narrow environment for `BLENoisePacketHandler`. /// /// All queue hops (collections barrier writes, main-actor UI notification) @@ -16,8 +21,11 @@ struct BLENoisePacketHandlerEnvironment { let messageTTL: UInt8 /// Current time source. let now: () -> Date - /// Processes an inbound handshake message, returning an optional response payload (crypto). - let processHandshakeMessage: (_ peerID: PeerID, _ message: Data) throws -> Data? + /// Processes an inbound handshake message, returning its optional response + /// and whether that exact candidate authenticated (crypto). + let processHandshakeMessage: + (_ peerID: PeerID, _ message: Data) throws + -> NoiseHandshakeProcessingResult /// Whether any Noise session (established or pending) exists for the peer (crypto). let hasNoiseSession: (PeerID) -> Bool /// Initiates a fresh Noise handshake with the peer (crypto + send). @@ -54,12 +62,23 @@ final class BLENoisePacketHandler { /// from a rejected candidate while an older session remains established. @discardableResult func handleHandshake(_ packet: BitchatPacket, from peerID: PeerID) -> Bool { + handleHandshakeWithResult(packet, from: peerID).processed + } + + func handleHandshakeWithResult( + _ packet: BitchatPacket, + from peerID: PeerID + ) -> BLENoiseHandshakeHandlingResult { let env = environment // Use NoiseEncryptionService for handshake processing if PeerID(hexData: packet.recipientID) == env.localPeerID() { // Handshake is for us do { - if let response = try env.processHandshakeMessage(peerID, packet.payload) { + let result = try env.processHandshakeMessage( + peerID, + packet.payload + ) + if let response = result.response { // Send response let responsePacket = BitchatPacket( type: MessageType.noiseHandshake.rawValue, @@ -76,7 +95,11 @@ final class BLENoisePacketHandler { // Session establishment will trigger onPeerAuthenticated callback // which will send any pending messages at the right time - return true + return BLENoiseHandshakeHandlingResult( + processed: true, + didEstablishAuthenticatedSession: + result.didEstablishAuthenticatedSession + ) } catch NoiseSessionError.peerIdentityMismatch { // The candidate was already discarded by the session manager. // Do not let a spoofed claimed ID trigger a fresh outbound @@ -85,17 +108,26 @@ final class BLENoisePacketHandler { "Rejected Noise handshake whose static key does not match \(peerID.id.prefix(8))…", category: .security ) - return false + return BLENoiseHandshakeHandlingResult( + processed: false, + didEstablishAuthenticatedSession: false + ) } catch { SecureLogger.error("Failed to process handshake: \(error)") // Try initiating a new handshake if !env.hasNoiseSession(peerID) { env.initiateHandshake(peerID) } - return false + return BLENoiseHandshakeHandlingResult( + processed: false, + didEstablishAuthenticatedSession: false + ) } } - return false + return BLENoiseHandshakeHandlingResult( + processed: false, + didEstablishAuthenticatedSession: false + ) } func handleEncrypted(_ packet: BitchatPacket, from peerID: PeerID) { diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index 6b0a4d75..af4bc601 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -5770,15 +5770,11 @@ extension BLEService { } private func handleNoiseHandshake(_ packet: BitchatPacket, from peerID: PeerID) { - let wasEstablished = noiseService.hasEstablishedSession(with: peerID) - let processed = noisePacketHandler.handleHandshake(packet, from: peerID) - let isEstablished = noiseService.hasEstablishedSession(with: peerID) - // XX message 1 is exactly the unauthenticated 32-byte ephemeral key. - // While replacing an existing session, do not authenticate its ingress - // link until a later message completes and validates the candidate. - let completedAuthenticatedHandshake = !wasEstablished - || packet.payload.count != NoiseSecurityConstants.xxInitialMessageSize - if processed, isEstablished, completedAuthenticatedHandshake { + let result = noisePacketHandler.handleHandshakeWithResult( + packet, + from: peerID + ) + if result.didEstablishAuthenticatedSession { markNoiseAuthenticatedIngressLink(for: packet, peerID: peerID) } } @@ -5801,7 +5797,16 @@ extension BLEService { messageTTL: messageTTL, now: { Date() }, processHandshakeMessage: { [weak self] peerID, message in - try self?.noiseService.processHandshakeMessage(from: peerID, message: message) + guard let self else { + return NoiseHandshakeProcessingResult( + response: nil, + didEstablishAuthenticatedSession: false + ) + } + return try self.noiseService.processHandshakeMessageWithResult( + from: peerID, + message: message + ) }, hasNoiseSession: { [weak self] peerID in self?.noiseService.hasSession(with: peerID) ?? false diff --git a/bitchat/Services/NoiseEncryptionService.swift b/bitchat/Services/NoiseEncryptionService.swift index 3e0aae6b..b5171b2a 100644 --- a/bitchat/Services/NoiseEncryptionService.swift +++ b/bitchat/Services/NoiseEncryptionService.swift @@ -664,6 +664,18 @@ final class NoiseEncryptionService { /// Process an incoming handshake message func processHandshakeMessage(from peerID: PeerID, message: Data) throws -> Data? { + try processHandshakeMessageWithResult( + from: peerID, + message: message + ).response + } + + /// Process an incoming handshake message and report whether the exact + /// session that consumed it completed authenticated establishment. + func processHandshakeMessageWithResult( + from peerID: PeerID, + message: Data + ) throws -> NoiseHandshakeProcessingResult { // Validate peer ID guard peerID.isValid else { @@ -685,11 +697,14 @@ final class NoiseEncryptionService { // For handshakes, we process the raw data directly without NoiseMessage wrapper // The Noise protocol handles its own message format - let responsePayload = try sessionManager.handleIncomingHandshake(from: peerID, message: message) + let result = try sessionManager.handleIncomingHandshakeWithResult( + from: peerID, + message: message + ) // Return raw response without wrapper - return responsePayload + return result } /// Check if we have an established session with a peer diff --git a/bitchatTests/BLEServiceCoreTests.swift b/bitchatTests/BLEServiceCoreTests.swift index d84d216f..3835df80 100644 --- a/bitchatTests/BLEServiceCoreTests.swift +++ b/bitchatTests/BLEServiceCoreTests.swift @@ -529,6 +529,94 @@ struct BLEServiceCoreTests { #expect(outbound.count(ofType: .courierEnvelope) == 0) } + @Test + func replacementXXMessageOneWithPayloadCannotAuthenticateIngressLink() async throws { + let ble = makeService() + let victim = NoiseEncryptionService(keychain: MockKeychain()) + let victimPeerID = PeerID(publicKey: victim.getStaticPublicKeyData()) + + // Preserve a working victim session while an unauthenticated + // replacement candidate arrives on a newly bound physical link. + let message1 = try ble._test_noiseInitiateHandshake(with: victimPeerID) + let message2 = try #require( + try victim.processHandshakeMessage(from: ble.myPeerID, message: message1) + ) + let message3 = try #require( + try ble._test_noiseProcessHandshakeMessage( + from: victimPeerID, + message: message2 + ) + ) + _ = try victim.processHandshakeMessage( + from: ble.myPeerID, + message: message3 + ) + #expect(ble.canDeliverSecurely(to: victimPeerID)) + + let centralUUID = "central-replacement-xx-message-one" + ble._test_bindCentral(centralUUID, to: victimPeerID) + #expect( + !ble._test_isNoiseAuthenticatedCentral( + centralUUID, + for: victimPeerID + ) + ) + + // XX message one may legally carry a payload, so its length is not a + // reliable signal that the replacement handshake completed. + let unauthenticatedInitiator = NoiseHandshakeState( + role: .initiator, + pattern: .XX, + keychain: MockKeychain() + ) + let replacementMessage1 = try unauthenticatedInitiator.writeMessage( + payload: Data([0xA5]) + ) + #expect( + replacementMessage1.count + > NoiseSecurityConstants.xxInitialMessageSize + ) + + let packet = BitchatPacket( + type: MessageType.noiseHandshake.rawValue, + senderID: Data(hexString: victimPeerID.id) ?? Data(), + recipientID: Data(hexString: ble.myPeerID.id), + timestamp: UInt64(Date().timeIntervalSince1970 * 1000), + payload: replacementMessage1, + signature: nil, + ttl: TransportConfig.messageTTLDefault + ) + #expect( + ble._test_recordIngressIfNew( + packet: packet, + linkID: centralUUID + ) + ) + + let outbound = OutboundPacketTap() + ble._test_onOutboundPacket = outbound.record + ble._test_handlePacket( + packet, + fromPeerID: victimPeerID, + preseedPeer: false + ) + + // Waiting for the responder's message two proves the candidate was + // processed before checking its exact authentication result. + let candidateProcessed = await TestHelpers.waitUntil( + { outbound.count(ofType: .noiseHandshake) == 1 }, + timeout: TestConstants.longTimeout + ) + #expect(candidateProcessed) + #expect( + !ble._test_isNoiseAuthenticatedCentral( + centralUUID, + for: victimPeerID + ) + ) + #expect(ble.canDeliverSecurely(to: victimPeerID)) + } + /// A legitimate rotation announce necessarily arrives on a link still /// bound to the OLD ID, so its registry upsert stores the new peer /// disconnected. The successful rebind must promote it: a healed diff --git a/bitchatTests/Services/BLENoisePacketHandlerTests.swift b/bitchatTests/Services/BLENoisePacketHandlerTests.swift index 42db3642..0365de5b 100644 --- a/bitchatTests/Services/BLENoisePacketHandlerTests.swift +++ b/bitchatTests/Services/BLENoisePacketHandlerTests.swift @@ -8,6 +8,7 @@ struct BLENoisePacketHandlerTests { private final class Recorder { var handshakeResult: Result = .success(nil) + var handshakeAuthenticated = false var hasSession = false var decryptResult: Result = .success(Data()) @@ -38,7 +39,11 @@ struct BLENoisePacketHandlerTests { now: { now }, processHandshakeMessage: { peerID, message in recorder.processedHandshakes.append((peerID, message)) - return try recorder.handshakeResult.get() + return NoiseHandshakeProcessingResult( + response: try recorder.handshakeResult.get(), + didEstablishAuthenticatedSession: + recorder.handshakeAuthenticated + ) }, hasNoiseSession: { peerID in recorder.hasSessionQueries.append(peerID) @@ -110,6 +115,24 @@ struct BLENoisePacketHandlerTests { #expect(recorder.initiatedHandshakes.isEmpty) } + @Test + func handshakeResultPreservesExactCandidateAuthentication() { + let recorder = Recorder() + recorder.handshakeAuthenticated = true + let handler = makeHandler(recorder: recorder) + let packet = makeHandshakePacket( + recipientID: Data(hexString: localPeerID.id) + ) + + let result = handler.handleHandshakeWithResult( + packet, + from: remotePeerID + ) + + #expect(result.processed) + #expect(result.didEstablishAuthenticatedSession) + } + @Test func handshakeForAnotherPeerIsIgnored() { let recorder = Recorder()