diff --git a/app/src/main/java/com/bitchat/android/mesh/SecurityManager.kt b/app/src/main/java/com/bitchat/android/mesh/SecurityManager.kt index 3f6846792..7efa7fb8d 100644 --- a/app/src/main/java/com/bitchat/android/mesh/SecurityManager.kt +++ b/app/src/main/java/com/bitchat/android/mesh/SecurityManager.kt @@ -3,6 +3,7 @@ package com.bitchat.android.mesh import android.util.Log import com.bitchat.android.crypto.EncryptionService import com.bitchat.android.protocol.BitchatPacket +import com.bitchat.android.sync.PacketIdUtil import com.bitchat.android.protocol.MessageType import com.bitchat.android.model.RoutedPacket import com.bitchat.android.noise.AuthenticatedNoiseSession @@ -241,18 +242,28 @@ class SecurityManager(private val encryptionService: EncryptionService, private /** * Generate message ID for duplicate detection */ + /** + * Identity used for replay and duplicate detection. + * + * This was a 32-bit `contentHashCode()` over at most the first 64 bytes of + * the payload. Two packets from the same peer in the same millisecond that + * agreed on that prefix collided, and a collision here is a *dropped + * message* — the second packet is discarded as a duplicate and there is no + * signal that it happened. + * + * `PacketIdUtil` is the identity the rest of the stack already uses for + * exactly this question (gossip sync membership, message IDs in + * `MessageHandler`), and iOS derives it the same way: the first 16 bytes of + * SHA-256 over type, senderID, timestamp and the **whole** payload. Using + * it here makes the security path agree with the sync path instead of + * carrying a weaker private notion of "same packet". + * + * Peer scoping is kept: `PacketIdUtil` covers the packet's own senderID, + * while this key is scoped by the peer the packet was received from, and + * those are not the same thing for a relayed packet. + */ private fun generateMessageID(packet: BitchatPacket, peerID: String): String { - return when (MessageType.fromValue(packet.type)) { - MessageType.FRAGMENT -> { - // For fragments, include the payload hash to distinguish different fragments - "${packet.timestamp}-$peerID-${packet.type}-${packet.payload.contentHashCode()}" - } - else -> { - // For other messages, use a truncated payload hash - val payloadHash = packet.payload.sliceArray(0 until minOf(64, packet.payload.size)).contentHashCode() - "${packet.timestamp}-$peerID-$payloadHash" - } - } + return "$peerID-${PacketIdUtil.computeIdHex(packet)}" } /** diff --git a/app/src/test/kotlin/com/bitchat/android/mesh/SecurityManagerTest.kt b/app/src/test/kotlin/com/bitchat/android/mesh/SecurityManagerTest.kt index 2a56e708b..e43e11b59 100644 --- a/app/src/test/kotlin/com/bitchat/android/mesh/SecurityManagerTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/mesh/SecurityManagerTest.kt @@ -598,4 +598,65 @@ class SecurityManagerTest { private fun String.hexToBytes(): ByteArray = chunked(2).map { it.toInt(16).toByte() }.toByteArray() + + // Duplicate-detection identity. + + /** + * Two distinct packets that agree on their first 64 bytes and share a + * timestamp. The old key hashed only that prefix, with a 32-bit + * `contentHashCode`, so these were "the same packet" and the second was + * silently dropped. + */ + private fun prefixSharingPair(): Pair { + val shared = ByteArray(64) { 0x7 } + val first = BitchatPacket( + version = 1u, + type = MessageType.NOISE_ENCRYPTED.value, + senderID = otherPeerID.hexToByteArrayForTest(), + recipientID = myPeerID.hexToByteArrayForTest(), + timestamp = 1_700_000_000_000uL, + payload = shared + byteArrayOf(0x01, 0x02, 0x03), + ttl = 7u + ) + val second = first.copy(payload = shared + byteArrayOf(0x0A, 0x0B, 0x0C)) + return first to second + } + + @Test + fun `packets differing only past the first 64 bytes are not treated as duplicates`() { + val (first, second) = prefixSharingPair() + + assertTrue(securityManager.validatePacket(first, otherPeerID)) + assertTrue( + "A distinct packet must not be dropped as a duplicate", + securityManager.validatePacket(second, otherPeerID) + ) + } + + @Test + fun `a genuine replay of the same packet is still rejected`() { + // The other half: strengthening the identity must not weaken replay + // protection, which is the reason this cache exists. + val (first, _) = prefixSharingPair() + + assertTrue(securityManager.validatePacket(first, otherPeerID)) + assertFalse( + "The identical packet must still be caught", + securityManager.validatePacket(first, otherPeerID) + ) + } + + @Test + fun `the same packet from two different peers is tracked separately`() { + // Peer scoping is deliberately kept: PacketIdUtil covers the packet's + // own senderID, which is not the peer it was received from once a + // packet has been relayed. + val (first, _) = prefixSharingPair() + + assertTrue(securityManager.validatePacket(first, otherPeerID)) + assertTrue(securityManager.validatePacket(first, unknownPeerID)) + } + + private fun String.hexToByteArrayForTest(): ByteArray = + chunked(2).map { it.toInt(16).toByte() }.toByteArray() }