Enforce mandatory packet signature verification (#546)

* security: enforce mandatory packet signature verification

* tests: add comprehensive unit tests for packet signature verification

app/src/main/java/com/bitchat/android/crypto/EncryptionService.kt

@@ -19,7 +19,7 @@ import java.util.concurrent.ConcurrentHashMap
  * This is the main interface for all encryption/decryption operations in bitchat.
  * It now uses the Noise protocol for secure transport encryption with proper session management.
  */
-class EncryptionService(private val context: Context) {
+open class EncryptionService(private val context: Context) {
     
     companion object {
         private const val TAG = "EncryptionService"
@@ -27,14 +27,14 @@ class EncryptionService(private val context: Context) {
     }
     
     // Core Noise encryption service
-    private val noiseService: NoiseEncryptionService = NoiseEncryptionService(context)
+    private val noiseService: NoiseEncryptionService by lazy { NoiseEncryptionService(context) }
     
     // Session tracking for established connections
     private val establishedSessions = ConcurrentHashMap<String, String>() // peerID -> fingerprint
     
     // Ed25519 signing keys (separate from Noise static keys)
-    private val ed25519PrivateKey: Ed25519PrivateKeyParameters
-    private val ed25519PublicKey: Ed25519PublicKeyParameters
+    private lateinit var ed25519PrivateKey: Ed25519PrivateKeyParameters
+    private lateinit var ed25519PublicKey: Ed25519PublicKeyParameters
     
     // Callbacks for UI state updates
     var onSessionEstablished: ((String) -> Unit)? = null // peerID
@@ -42,6 +42,13 @@ class EncryptionService(private val context: Context) {
     var onHandshakeRequired: ((String) -> Unit)? = null // peerID
     
     init {
+        initialize()
+    }
+
+    /**
+     * Initialization logic moved to method to allow overriding in tests
+     */
+    protected open fun initialize() {
         // Initialize or load Ed25519 signing keys
         val keyPair = loadOrCreateEd25519KeyPair()
         ed25519PrivateKey = keyPair.private as Ed25519PrivateKeyParameters
@@ -356,7 +363,7 @@ class EncryptionService(private val context: Context) {
     /**
      * Verify Ed25519 signature against data using a public key
      */
-    fun verifyEd25519Signature(signature: ByteArray, data: ByteArray, publicKeyBytes: ByteArray): Boolean {
+    open fun verifyEd25519Signature(signature: ByteArray, data: ByteArray, publicKeyBytes: ByteArray): Boolean {
         return try {
             val publicKey = Ed25519PublicKeyParameters(publicKeyBytes, 0)
             val verifier = Ed25519Signer()

app/src/main/java/com/bitchat/android/mesh/SecurityManager.kt

@@ -69,8 +69,11 @@ class SecurityManager(private val encryptionService: EncryptionService, private
             // They are signed/idempotent and we need to ensure first-announce per-connection can bind.
         }
         
-        // NEW: Signature verification logging (not rejecting yet)
-        verifyPacketSignatureWithLogging(packet, peerID)
+        // Enforce mandatory signature verification
+        if (!verifyPacketSignature(packet, peerID)) {
+            Log.w(TAG, "Dropping packet from $peerID due to signature verification failure")
+            return false
+        }
         
         Log.d(TAG, "Packet validation passed for $peerID, messageID: $messageID")
         return true
@@ -223,34 +226,50 @@ class SecurityManager(private val encryptionService: EncryptionService, private
     }
     
     /**
-     * Verify packet signature using peer's signing public key and log the result
+     * Verify packet signature using peer's signing public key
+     * Returns true only if signature is present and valid
      */
-    private fun verifyPacketSignatureWithLogging(packet: BitchatPacket, peerID: String) {
+    private fun verifyPacketSignature(packet: BitchatPacket, peerID: String): Boolean {
         try {
-            // Check if packet has a signature
+            // 1. Mandatory Signature Check
             if (packet.signature == null) {
-                Log.d(TAG, "📝 Signature check for $peerID: NO_SIGNATURE (packet type ${packet.type})")
-                return
+                Log.w(TAG, "❌ Signature check for $peerID: NO_SIGNATURE (packet type ${packet.type})")
+                return false
             }
             
-            // Try to get peer's signing public key from peer info
-            val peerInfo = delegate?.getPeerInfo(peerID)
-            val signingPublicKey = peerInfo?.signingPublicKey
+            // 2. Get Signing Public Key
+            var signingPublicKey: ByteArray? = null
+            
+            if (MessageType.fromValue(packet.type) == MessageType.ANNOUNCE) {
+                // Special Case: ANNOUNCE packets carry their own signing key
+                try {
+                    val announcement = com.bitchat.android.model.IdentityAnnouncement.decode(packet.payload)
+                    signingPublicKey = announcement?.signingPublicKey
+                } catch (e: Exception) {
+                    Log.w(TAG, "Failed to decode announcement for key extraction: ${e.message}")
+                }
+            } else {
+                // Standard Case: Get key from known peer info
+                val peerInfo = delegate?.getPeerInfo(peerID)
+                signingPublicKey = peerInfo?.signingPublicKey
+            }
             
             if (signingPublicKey == null) {
-                Log.d(TAG, "📝 Signature check for $peerID: NO_SIGNING_KEY (packet type ${packet.type})")
-                return
+                // If we don't have a key (and it's not an announce), we can't verify.
+                // For security, we must reject packets from unknown peers unless it's an announce.
+                Log.w(TAG, "❌ Signature check for $peerID: NO_SIGNING_KEY_AVAILABLE (packet type ${packet.type})")
+                return false
             }
             
-            // Get the canonical packet data for signature verification (without signature)
+            // 3. Get Canonical Data
             val packetDataForSigning = packet.toBinaryDataForSigning()
             if (packetDataForSigning == null) {
-                Log.w(TAG, "📝 Signature check for $peerID: ENCODING_ERROR (packet type ${packet.type})")
-                return
+                Log.w(TAG, "❌ Signature check for $peerID: ENCODING_ERROR (packet type ${packet.type})")
+                return false
             }
             
-            // Verify the signature using the peer's signing public key
-            val signature = packet.signature!! // We already checked for null above
+            // 4. Verify Signature
+            val signature = packet.signature!!
             val isSignatureValid = encryptionService.verifyEd25519Signature(
                 signature,
                 packetDataForSigning,
@@ -258,13 +277,16 @@ class SecurityManager(private val encryptionService: EncryptionService, private
             )
             
             if (isSignatureValid) {
-                Log.d(TAG, "📝 Signature check for $peerID: ✅ VALID (packet type ${packet.type})")
+                // Log.v(TAG, "✅ Signature verified for $peerID (type ${packet.type})")
+                return true
             } else {
-                Log.w(TAG, "📝 Signature check for $peerID: ❌ INVALID (packet type ${packet.type})")
+                Log.w(TAG, "❌ Signature INVALID for $peerID (type ${packet.type})")
+                return false
             }
             
         } catch (e: Exception) {
-            Log.w(TAG, "📝 Signature check for $peerID: ERROR - ${e.message} (packet type ${packet.type})")
+            Log.e(TAG, "❌ Signature verification error for $peerID: ${e.message}")
+            return false
         }
     }
     

app/src/test/kotlin/com/bitchat/android/mesh/SecurityManagerTest.kt

@@ -0,0 +1,255 @@
+package com.bitchat.android.mesh
+
+import android.os.Build
+import com.bitchat.android.crypto.EncryptionService
+import com.bitchat.android.model.IdentityAnnouncement
+import com.bitchat.android.protocol.BitchatPacket
+import com.bitchat.android.protocol.MessageType
+import org.junit.After
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.kotlin.*
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import org.robolectric.annotation.Config
+
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [Build.VERSION_CODES.P], manifest = Config.NONE)
+class SecurityManagerTest {
+
+    private lateinit var securityManager: SecurityManager
+    private lateinit var fakeEncryptionService: FakeEncryptionService
+    private lateinit var mockDelegate: SecurityManagerDelegate
+    
+    private val myPeerID = "1111222233334444"
+    private val otherPeerID = "aaaabbbbccccdddd"
+    private val unknownPeerID = "9999888877776666"
+
+    private val dummyPayload = "Hello World".toByteArray()
+    private val validSignature = ByteArray(64) { 1 }
+    private val invalidSignature = ByteArray(64) { 0 }
+    
+    // Key pairs (using dummy bytes for mock verification)
+    private val otherSigningKey = ByteArray(32) { 0xA }
+    private val otherNoiseKey = ByteArray(32) { 0xB }
+
+    // Fake implementation to bypass initialization issues in tests
+    open class FakeEncryptionService : EncryptionService(RuntimeEnvironment.getApplication()) {
+        var shouldVerify: Boolean = true
+        var lastVerifySignature: ByteArray? = null
+        var lastVerifyKey: ByteArray? = null
+
+        override fun initialize() {
+            // Do nothing to avoid KeyStore access in tests
+        }
+
+        override fun verifyEd25519Signature(signature: ByteArray, data: ByteArray, publicKeyBytes: ByteArray): Boolean {
+            lastVerifySignature = signature
+            lastVerifyKey = publicKeyBytes
+            
+            // Simple logic: if configured to verify, check if signature matches validSignature
+            // We use the signature bytes passed in setup()
+            if (shouldVerify) {
+                 return signature.contentEquals(byteArrayOf(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1))
+            }
+            return false
+        }
+    }
+
+    @Before
+    fun setup() {
+        fakeEncryptionService = FakeEncryptionService()
+        mockDelegate = mock()
+        
+        securityManager = SecurityManager(fakeEncryptionService, myPeerID)
+        securityManager.delegate = mockDelegate
+    }
+
+    @After
+    fun tearDown() {
+        if (::securityManager.isInitialized) {
+            securityManager.shutdown()
+        }
+    }
+
+    @Test
+    fun `validatePacket - rejects packet with missing signature`() {
+        val packet = BitchatPacket(
+            type = MessageType.MESSAGE.value,
+            ttl = 10u,
+            senderID = otherPeerID,
+            payload = dummyPayload
+        )
+        packet.signature = null
+
+        val result = securityManager.validatePacket(packet, otherPeerID)
+        
+        assertFalse("Packet without signature should be rejected", result)
+    }
+
+    @Test
+    fun `validatePacket - rejects packet with invalid signature`() {
+        setupKnownPeer(otherPeerID, otherSigningKey)
+        
+        val packet = BitchatPacket(
+            type = MessageType.MESSAGE.value,
+            ttl = 10u,
+            senderID = otherPeerID,
+            payload = dummyPayload
+        )
+        packet.signature = invalidSignature
+
+        val result = securityManager.validatePacket(packet, otherPeerID)
+        
+        assertFalse("Packet with invalid signature should be rejected", result)
+    }
+
+    @Test
+    fun `validatePacket - rejects packet from unknown peer (no key)`() {
+        whenever(mockDelegate.getPeerInfo(unknownPeerID)).thenReturn(null)
+        
+        val packet = BitchatPacket(
+            type = MessageType.MESSAGE.value,
+            ttl = 10u,
+            senderID = unknownPeerID,
+            payload = dummyPayload
+        )
+        packet.signature = validSignature
+
+        val result = securityManager.validatePacket(packet, unknownPeerID)
+        
+        assertFalse("Packet from unknown peer should be rejected (cannot verify signature)", result)
+    }
+
+    @Test
+    fun `validatePacket - accepts packet with valid signature from known peer`() {
+        setupKnownPeer(otherPeerID, otherSigningKey)
+        
+        val packet = BitchatPacket(
+            type = MessageType.MESSAGE.value,
+            ttl = 10u,
+            senderID = otherPeerID,
+            payload = dummyPayload
+        )
+        packet.signature = validSignature
+
+        val result = securityManager.validatePacket(packet, otherPeerID)
+        
+        assertTrue("Valid signed packet from known peer should be accepted", result)
+    }
+
+    @Test
+    fun `validatePacket - accepts ANNOUNCE packet from unknown peer (extracts key)`() {
+        val announcement = IdentityAnnouncement(
+            nickname = "New User",
+            noisePublicKey = otherNoiseKey,
+            signingPublicKey = otherSigningKey
+        )
+        val payload = announcement.encode()!!
+        
+        val packet = BitchatPacket(
+            type = MessageType.ANNOUNCE.value,
+            ttl = 10u,
+            senderID = unknownPeerID,
+            payload = payload
+        )
+        packet.signature = validSignature
+
+        whenever(mockDelegate.getPeerInfo(unknownPeerID)).thenReturn(null)
+        
+        val result = securityManager.validatePacket(packet, unknownPeerID)
+        
+        assertTrue("ANNOUNCE from unknown peer should be accepted (key extracted from payload)", result)
+        // Verify we used the correct key
+        assertTrue("Should have used extracted key for verification", 
+            fakeEncryptionService.lastVerifyKey.contentEquals(otherSigningKey))
+    }
+
+    @Test
+    fun `validatePacket - rejects ANNOUNCE packet with invalid signature`() {
+        val announcement = IdentityAnnouncement(
+            nickname = "New User",
+            noisePublicKey = otherNoiseKey,
+            signingPublicKey = otherSigningKey
+        )
+        val payload = announcement.encode()!!
+        
+        val packet = BitchatPacket(
+            type = MessageType.ANNOUNCE.value,
+            ttl = 10u,
+            senderID = unknownPeerID,
+            payload = payload
+        )
+        packet.signature = invalidSignature
+
+        val result = securityManager.validatePacket(packet, unknownPeerID)
+        
+        assertFalse("ANNOUNCE with invalid signature should be rejected", result)
+    }
+    
+    @Test
+    fun `validatePacket - rejects ANNOUNCE packet with malformed payload`() {
+        val packet = BitchatPacket(
+            type = MessageType.ANNOUNCE.value,
+            ttl = 10u,
+            senderID = unknownPeerID,
+            payload = byteArrayOf(0x00, 0x01, 0x02)
+        )
+        packet.signature = validSignature
+
+        val result = securityManager.validatePacket(packet, unknownPeerID)
+        
+        assertFalse("ANNOUNCE with malformed payload should be rejected (cannot extract key)", result)
+    }
+
+    @Test
+    fun `validatePacket - ignores own packets`() {
+        val packet = BitchatPacket(
+            type = MessageType.MESSAGE.value,
+            ttl = 10u,
+            senderID = myPeerID,
+            payload = dummyPayload
+        )
+        packet.signature = null
+
+        val result = securityManager.validatePacket(packet, myPeerID)
+        
+        assertFalse("Own packets should return false (skipped)", result)
+    }
+    
+    @Test
+    fun `validatePacket - detects duplicates`() {
+        setupKnownPeer(otherPeerID, otherSigningKey)
+        
+        val packet = BitchatPacket(
+            type = MessageType.MESSAGE.value,
+            ttl = 10u,
+            senderID = otherPeerID,
+            payload = dummyPayload
+        )
+        packet.signature = validSignature
+
+        val result1 = securityManager.validatePacket(packet, otherPeerID)
+        assertTrue("First packet should be accepted", result1)
+
+        val result2 = securityManager.validatePacket(packet, otherPeerID)
+        assertFalse("Duplicate packet should be rejected", result2)
+    }
+
+    private fun setupKnownPeer(peerID: String, signingKey: ByteArray) {
+        val info = PeerInfo(
+            id = peerID,
+            nickname = "Test User",
+            isConnected = true,
+            isDirectConnection = true,
+            noisePublicKey = ByteArray(32),
+            signingPublicKey = signingKey,
+            isVerifiedNickname = false,
+            lastSeen = System.currentTimeMillis()
+        )
+        whenever(mockDelegate.getPeerInfo(peerID)).thenReturn(info)
+    }
+}