From 87ccfe3f6ca8804fa0fe892c8e57d1b84227ac97 Mon Sep 17 00:00:00 2001 From: Taksh Date: Sat, 15 Aug 2026 13:58:41 +0530 Subject: [PATCH 1/2] Use the shared packet identity for duplicate detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replay and duplicate detection keyed on 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 were the same packet as far as this cache was concerned, and a collision here is a dropped message: the second is discarded and nothing reports it. PacketIdUtil is the identity the rest of the stack already uses for this question — gossip sync membership, and the message IDs MessageHandler assigns — and iOS derives it identically: first 16 bytes of SHA-256 over type, senderID, timestamp and the whole payload. The security path now agrees with the sync path instead of carrying a weaker private notion of "same packet", and the FRAGMENT special case disappears because the full payload is covered either way. Peer scoping is deliberately kept. PacketIdUtil covers the packet's own senderID, which is not the peer it arrived from once relayed. --- .../bitchat/android/mesh/SecurityManager.kt | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) 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)}" } /** From 4ef1b9c76576a7ddb5d857de17fcc3632bc39e5c Mon Sep 17 00:00:00 2001 From: Taksh Date: Sat, 15 Aug 2026 13:58:41 +0530 Subject: [PATCH 2/2] Pin the collision, the replay, and the peer scoping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The collision case fails on main: two packets sharing a 64-byte prefix and a timestamp, where the second was silently dropped. The other two pass before and after on purpose. Replay of an identical packet must still be caught, and the same packet arriving from two different peers must still be tracked separately — strengthening the identity must not quietly weaken either. --- .../android/mesh/SecurityManagerTest.kt | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) 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() }