diff --git a/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt b/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt index 88d41466..a43194cb 100644 --- a/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt +++ b/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt @@ -3,8 +3,11 @@ package com.bitchat.android.nostr import android.app.Application import android.util.Log import com.bitchat.android.model.BitchatMessage +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import java.util.Date /** @@ -18,7 +21,8 @@ class GeohashMessageHandler( private val repo: GeohashRepository, private val scope: CoroutineScope, private val dataManager: com.bitchat.android.ui.DataManager, - private val addChannelMessage: (String, BitchatMessage) -> Unit + private val addChannelMessage: (String, BitchatMessage) -> Unit, + private val signatureVerificationDispatcher: CoroutineDispatcher = Dispatchers.Default ) { companion object { private const val TAG = "GeohashMessageHandler" } @@ -44,6 +48,13 @@ class GeohashMessageHandler( if (event.kind != NostrKind.EPHEMERAL_EVENT && event.kind != NostrKind.GEOHASH_PRESENCE) return@launch val tagGeo = event.tags.firstOrNull { it.size >= 2 && it[0] == "g" }?.getOrNull(1) if (tagGeo == null || !tagGeo.equals(subscribedGeohash, true)) return@launch + val hasValidSignature = withContext(signatureVerificationDispatcher) { + event.isValidSignature() + } + if (!hasValidSignature) { + Log.w(TAG, "Rejecting geohash event ${event.id.take(8)}... with invalid signature") + return@launch + } if (dedupe(event.id)) return@launch // PoW validation (if enabled) - apply to chat messages primarily diff --git a/app/src/main/java/com/bitchat/android/nostr/LocationNotesManager.kt b/app/src/main/java/com/bitchat/android/nostr/LocationNotesManager.kt index a6927d71..0edb1fca 100644 --- a/app/src/main/java/com/bitchat/android/nostr/LocationNotesManager.kt +++ b/app/src/main/java/com/bitchat/android/nostr/LocationNotesManager.kt @@ -416,6 +416,11 @@ class LocationNotesManager private constructor() { Log.v(TAG, "Ignoring non-text-note event: kind=${event.kind}") return } + + if (!event.isValidSignature()) { + Log.w(TAG, "Rejecting note ${event.id.take(8)}... with invalid signature") + return + } // Check for geohash tag val geohashTag = event.tags.firstOrNull { it.size >= 2 && it[0] == "g" } diff --git a/app/src/main/java/com/bitchat/android/nostr/NostrClient.kt b/app/src/main/java/com/bitchat/android/nostr/NostrClient.kt index c1488037..0257c756 100644 --- a/app/src/main/java/com/bitchat/android/nostr/NostrClient.kt +++ b/app/src/main/java/com/bitchat/android/nostr/NostrClient.kt @@ -282,6 +282,11 @@ class NostrClient private constructor(private val context: Context) { handler: (content: String, senderPubkey: String, nickname: String?, timestamp: Int) -> Unit ) { try { + if (!event.isValidSignature()) { + Log.w(TAG, "🚫 Rejecting geohash event ${event.id.take(8)}... with invalid signature") + return + } + // Check Proof of Work validation for incoming geohash events val powSettings = PoWPreferenceManager.getCurrentSettings() if (powSettings.enabled && powSettings.difficulty > 0) { diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt new file mode 100644 index 00000000..44044f2d --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt @@ -0,0 +1,136 @@ +package com.bitchat.android.nostr + +import android.app.Application +import androidx.test.core.app.ApplicationProvider +import com.bitchat.android.ui.ChatState +import com.bitchat.android.ui.DataManager +import com.bitchat.android.ui.MessageManager +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Regression coverage for GH-733: geohash events (kind 20000/20001) were accepted from the + * relay without Schnorr signature verification, allowing full pubkey impersonation. + */ +@RunWith(RobolectricTestRunner::class) +class GeohashMessageHandlerSignatureTest { + + private val application: Application = ApplicationProvider.getApplicationContext() + + @OptIn(ExperimentalCoroutinesApi::class) + private val testDispatcher = UnconfinedTestDispatcher() + private val testScope = TestScope(testDispatcher) + + private lateinit var chatState: ChatState + private lateinit var dataManager: DataManager + private lateinit var messageManager: MessageManager + private lateinit var repo: GeohashRepository + private lateinit var handler: GeohashMessageHandler + + private val geohash = "u4pruy" + + @Before + fun setup() { + chatState = ChatState(scope = testScope) + dataManager = DataManager(context = application) + messageManager = MessageManager(state = chatState) + repo = GeohashRepository(application, chatState, dataManager) + handler = GeohashMessageHandler( + application = application, + repo = repo, + scope = testScope, + dataManager = dataManager, + addChannelMessage = messageManager::addChannelMessage, + signatureVerificationDispatcher = testDispatcher + ) + } + + private fun buildSignedEvent(identity: NostrIdentity, content: String): NostrEvent { + val unsigned = NostrEvent( + pubkey = identity.publicKeyHex, + createdAt = (System.currentTimeMillis() / 1000).toInt(), + kind = NostrKind.EPHEMERAL_EVENT, + tags = listOf(listOf("g", geohash)), + content = content + ) + return unsigned.sign(identity.privateKeyHex) + } + + @Test + fun onEvent_acceptsGenuinelySignedEvent() { + val victim = NostrIdentity.generate() + val genuine = buildSignedEvent(victim, "hello from the real victim") + + handler.onEvent(genuine, geohash) + + val stored = chatState.getChannelMessagesValue()["geo:$geohash"] + assertEquals(1, stored?.size) + assertEquals("hello from the real victim", stored?.first()?.content) + } + + @Test + fun onEvent_rejectsEventWithForgedSignature() { + val victim = NostrIdentity.generate() + val attacker = NostrIdentity.generate() + + // Attacker claims the victim's pubkey but signs with their own key - a forged event. + val forged = buildSignedEvent(victim, "impersonated message").copy( + sig = NostrEvent( + pubkey = attacker.publicKeyHex, + createdAt = (System.currentTimeMillis() / 1000).toInt(), + kind = NostrKind.EPHEMERAL_EVENT, + tags = listOf(listOf("g", geohash)), + content = "impersonated message" + ).sign(attacker.privateKeyHex).sig + ) + + // Sanity check: this event is indeed invalid before we even touch the handler. + assertEquals(false, forged.isValidSignature()) + + handler.onEvent(forged, geohash) + + val stored = chatState.getChannelMessagesValue()["geo:$geohash"] + assertNull("Forged event must not be rendered as a legitimate message", stored) + } + + @Test + fun onEvent_stillAcceptsGenuineEventAfterForgedCopyWithSameIdWasRejected() { + val victim = NostrIdentity.generate() + val attacker = NostrIdentity.generate() + val genuine = buildSignedEvent(victim, "hello from the real victim") + + // A forged event carrying the SAME id as the genuine one (ids are a content hash, + // independent of the signature), but signed by an attacker - relays can deliver this + // before the genuine copy arrives from another relay. + val forgedWithSameId = genuine.copy( + sig = NostrEvent( + pubkey = attacker.publicKeyHex, + createdAt = genuine.createdAt, + kind = NostrKind.EPHEMERAL_EVENT, + tags = listOf(listOf("g", geohash)), + content = "hello from the real victim" + ).sign(attacker.privateKeyHex).sig + ) + assertEquals(genuine.id, forgedWithSameId.id) + assertEquals(false, forgedWithSameId.isValidSignature()) + + // Forged copy arrives first and must be rejected without poisoning the dedup cache. + handler.onEvent(forgedWithSameId, geohash) + assertNull(chatState.getChannelMessagesValue()["geo:$geohash"]) + + // The genuine copy (same id, valid signature) arrives afterwards from another relay - + // it must still be rendered, not silently dropped as a "duplicate". + handler.onEvent(genuine, geohash) + + val stored = chatState.getChannelMessagesValue()["geo:$geohash"] + assertEquals(1, stored?.size) + assertEquals("hello from the real victim", stored?.first()?.content) + } +} diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/NostrBackgroundEventProcessorTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/NostrBackgroundEventProcessorTest.kt index d7fffc2c..152d4ad0 100644 --- a/app/src/test/kotlin/com/bitchat/android/nostr/NostrBackgroundEventProcessorTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/nostr/NostrBackgroundEventProcessorTest.kt @@ -36,17 +36,21 @@ class NostrBackgroundEventProcessorTest { fun `cold start processes more events than the removed handoff queue capacity`() = runBlocking { val application = ApplicationProvider.getApplicationContext() val processor = NostrBackgroundEventProcessor(application, scope) + val identity = NostrIdentity.generate() + val expectedIds = mutableSetOf() repeat(300) { index -> + val event = NostrEvent( + pubkey = identity.publicKeyHex, + createdAt = index + 1, + kind = NostrKind.EPHEMERAL_EVENT, + tags = listOf(listOf("g", "u4pruy")), + content = "message-$index" + ).sign(identity.privateKeyHex) + expectedIds += event.id + processor.onGeohashMessage( - event = NostrEvent( - id = "cold-start-$index", - pubkey = index.toString(16).padStart(64, '0'), - createdAt = 1, - kind = NostrKind.EPHEMERAL_EVENT, - tags = listOf(listOf("g", "u4pruy")), - content = "message-$index" - ), + event = event, geohash = "u4pruy" ) } @@ -58,7 +62,7 @@ class NostrBackgroundEventProcessorTest { } assertEquals( - (0 until 300).map { "cold-start-$it" }.toSet(), + expectedIds, AppStateStore.channelMessages.value["geo:u4pruy"].orEmpty().map { it.id }.toSet() ) }