From e73a407bb32ff875be8e46bb90c7c52b2293f2bb Mon Sep 17 00:00:00 2001 From: greenart7c3 Date: Mon, 10 Aug 2026 11:29:29 -0300 Subject: [PATCH] Fix GHSA-h9fv-9247-3582: NIP-46 freshness and replay protection --- .../19.json | 285 ++++++++++++++++++ .../nostrsigner/database/AppDatabase.kt | 14 +- .../nostrsigner/database/BunkerEventDao.kt | 21 ++ .../nostrsigner/database/BunkerEventEntity.kt | 26 ++ .../nostrsigner/service/ClearLogsWorker.kt | 6 + .../service/EventNotificationConsumer.kt | 26 ++ .../service/EventNotificationConsumerTest.kt | 129 ++++++++ 7 files changed, 506 insertions(+), 1 deletion(-) create mode 100644 app/schemas/com.greenart7c3.nostrsigner.database.AppDatabase/19.json create mode 100644 app/src/main/java/com/greenart7c3/nostrsigner/database/BunkerEventDao.kt create mode 100644 app/src/main/java/com/greenart7c3/nostrsigner/database/BunkerEventEntity.kt create mode 100644 app/src/test/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumerTest.kt diff --git a/app/schemas/com.greenart7c3.nostrsigner.database.AppDatabase/19.json b/app/schemas/com.greenart7c3.nostrsigner.database.AppDatabase/19.json new file mode 100644 index 00000000..10047853 --- /dev/null +++ b/app/schemas/com.greenart7c3.nostrsigner.database.AppDatabase/19.json @@ -0,0 +1,285 @@ +{ + "formatVersion": 1, + "database": { + "version": 19, + "identityHash": "a56de62f406b4670ae2cbaf4225fdc6e", + "entities": [ + { + "tableName": "application", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`key` TEXT NOT NULL, `name` TEXT NOT NULL, `relays` TEXT NOT NULL, `url` TEXT NOT NULL, `icon` TEXT NOT NULL, `description` TEXT NOT NULL, `pubKey` TEXT NOT NULL, `isConnected` INTEGER NOT NULL, `secret` TEXT NOT NULL, `useSecret` INTEGER NOT NULL, `signPolicy` INTEGER NOT NULL, `closeApplication` INTEGER NOT NULL, `deleteAfter` INTEGER NOT NULL, `lastUsed` INTEGER NOT NULL, `localKey` TEXT NOT NULL, PRIMARY KEY(`key`))", + "fields": [ + { + "fieldPath": "key", + "columnName": "key", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "relays", + "columnName": "relays", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "url", + "columnName": "url", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "icon", + "columnName": "icon", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "description", + "columnName": "description", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "pubKey", + "columnName": "pubKey", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "isConnected", + "columnName": "isConnected", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "secret", + "columnName": "secret", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "useSecret", + "columnName": "useSecret", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "signPolicy", + "columnName": "signPolicy", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "closeApplication", + "columnName": "closeApplication", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "deleteAfter", + "columnName": "deleteAfter", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "lastUsed", + "columnName": "lastUsed", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "localKey", + "columnName": "localKey", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "key" + ] + }, + "indices": [ + { + "name": "index_key", + "unique": true, + "columnNames": [ + "key" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_key` ON `${TABLE_NAME}` (`key`)" + }, + { + "name": "index_name", + "unique": false, + "columnNames": [ + "name" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_name` ON `${TABLE_NAME}` (`name`)" + } + ] + }, + { + "tableName": "applicationPermission", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER, `pkKey` TEXT NOT NULL, `type` TEXT NOT NULL, `kind` INTEGER, `acceptable` INTEGER NOT NULL, `rememberType` INTEGER NOT NULL, `acceptUntil` INTEGER NOT NULL, `rejectUntil` INTEGER NOT NULL, `relay` TEXT NOT NULL, PRIMARY KEY(`id`), FOREIGN KEY(`pkKey`) REFERENCES `application`(`key`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER" + }, + { + "fieldPath": "pkKey", + "columnName": "pkKey", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "type", + "columnName": "type", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "kind", + "columnName": "kind", + "affinity": "INTEGER" + }, + { + "fieldPath": "acceptable", + "columnName": "acceptable", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "rememberType", + "columnName": "rememberType", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "acceptUntil", + "columnName": "acceptUntil", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "rejectUntil", + "columnName": "rejectUntil", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "relay", + "columnName": "relay", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "permissions_by_pk_key", + "unique": false, + "columnNames": [ + "pkKey" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `permissions_by_pk_key` ON `${TABLE_NAME}` (`pkKey`)" + }, + { + "name": "permissions_unique", + "unique": true, + "columnNames": [ + "pkKey", + "type", + "kind", + "relay" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `permissions_unique` ON `${TABLE_NAME}` (`pkKey`, `type`, `kind`, `relay`)" + } + ], + "foreignKeys": [ + { + "table": "application", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "pkKey" + ], + "referencedColumns": [ + "key" + ] + } + ] + }, + { + "tableName": "bunker_event", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `eventId` TEXT NOT NULL, `time` INTEGER NOT NULL)", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "eventId", + "columnName": "eventId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "time", + "columnName": "time", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_bunker_event_id", + "unique": true, + "columnNames": [ + "eventId" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_bunker_event_id` ON `${TABLE_NAME}` (`eventId`)" + }, + { + "name": "index_bunker_event_time", + "unique": false, + "columnNames": [ + "time" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_bunker_event_time` ON `${TABLE_NAME}` (`time`)" + } + ] + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'a56de62f406b4670ae2cbaf4225fdc6e')" + ] + } +} \ No newline at end of file diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/database/AppDatabase.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/AppDatabase.kt index 66d839ba..1df7d8e3 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/AppDatabase.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/AppDatabase.kt @@ -165,17 +165,28 @@ val MIGRATION_17_18 = object : Migration(17, 18) { } } +val MIGRATION_18_19 = object : Migration(18, 19) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("CREATE TABLE IF NOT EXISTS `bunker_event` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `eventId` TEXT NOT NULL, `time` INTEGER NOT NULL)") + db.execSQL("CREATE UNIQUE INDEX IF NOT EXISTS `index_bunker_event_id` ON `bunker_event` (`eventId`)") + db.execSQL("CREATE INDEX IF NOT EXISTS `index_bunker_event_time` ON `bunker_event` (`time`)") + } +} + @Database( entities = [ ApplicationEntity::class, ApplicationPermissionsEntity::class, + BunkerEventEntity::class, ], - version = 18, + version = 19, ) @TypeConverters(Converters::class) abstract class AppDatabase : RoomDatabase() { abstract fun dao(): ApplicationDao + abstract fun bunkerEventDao(): BunkerEventDao + companion object { fun getDatabase( context: Context, @@ -209,6 +220,7 @@ abstract class AppDatabase : RoomDatabase() { .addMigrations(MIGRATION_15_16) .addMigrations(MIGRATION_16_17) .addMigrations(MIGRATION_17_18) + .addMigrations(MIGRATION_18_19) .build() instance diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/database/BunkerEventDao.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/BunkerEventDao.kt new file mode 100644 index 00000000..68741620 --- /dev/null +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/BunkerEventDao.kt @@ -0,0 +1,21 @@ +package com.greenart7c3.nostrsigner.database + +import androidx.room.Dao +import androidx.room.Insert +import androidx.room.OnConflictStrategy +import androidx.room.Query +import androidx.room.Transaction + +@Dao +interface BunkerEventDao { + @Insert(onConflict = OnConflictStrategy.IGNORE) + @Transaction + suspend fun insert(bunkerEvent: BunkerEventEntity): Long? + + @Query("SELECT EXISTS(SELECT 1 FROM bunker_event WHERE eventId = :eventId)") + suspend fun exists(eventId: String): Boolean + + @Query("DELETE FROM bunker_event WHERE time < :time") + @Transaction + suspend fun deleteOld(time: Long): Int +} diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/database/BunkerEventEntity.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/BunkerEventEntity.kt new file mode 100644 index 00000000..e6c22cd7 --- /dev/null +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/BunkerEventEntity.kt @@ -0,0 +1,26 @@ +package com.greenart7c3.nostrsigner.database + +import androidx.room.Entity +import androidx.room.Index +import androidx.room.PrimaryKey + +@Entity( + tableName = "bunker_event", + indices = [ + Index( + value = ["eventId"], + name = "index_bunker_event_id", + unique = true, + ), + Index( + value = ["time"], + name = "index_bunker_event_time", + ), + ], +) +data class BunkerEventEntity( + @PrimaryKey(autoGenerate = true) + val id: Int, + val eventId: String, + val time: Long, +) diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/ClearLogsWorker.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/ClearLogsWorker.kt index 95815cbd..0cea40b0 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/ClearLogsWorker.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/ClearLogsWorker.kt @@ -47,6 +47,12 @@ class ClearLogsWorker(appContext: Context, workerParams: WorkerParameters) : Cor AmberLog.d(Amber.TAG, "Trimmed $excessLogs excess log entries (cap: $MAX_LOG_ENTRIES)") } + val bunkerEventDao = Amber.instance.getDatabase(it.npub).bunkerEventDao() + val deletedBunkerEvents = bunkerEventDao.deleteOld(threeDaysAgo / 1000) + if (deletedBunkerEvents > 0) { + AmberLog.d(Amber.TAG, "Deleted $deletedBunkerEvents old bunker event entries") + } + val dao = Amber.instance.dao(it.npub) dao.updateExpiredPermissions(TimeUtils.now()) val deleted = dao.deleteOldApplications(now / 1000) diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt index 7f39cb77..5f8c803a 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt @@ -33,6 +33,7 @@ import com.greenart7c3.nostrsigner.LocalPreferences import com.greenart7c3.nostrsigner.R import com.greenart7c3.nostrsigner.SignerProviderQuery import com.greenart7c3.nostrsigner.database.ApplicationWithPermissions +import com.greenart7c3.nostrsigner.database.BunkerEventEntity import com.greenart7c3.nostrsigner.database.HistoryEntity import com.greenart7c3.nostrsigner.database.LogEntity import com.greenart7c3.nostrsigner.models.Account @@ -56,6 +57,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal import com.vitorpamplona.quartz.nip01Core.tags.people.taggedUsers import com.vitorpamplona.quartz.nip04Dm.crypto.EncryptedInfo +import com.vitorpamplona.quartz.nip40Expiration.expiration import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestConnect import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestNip04Decrypt @@ -130,6 +132,22 @@ class EventNotificationConsumer(private val applicationContext: Context) { return } + val now = TimeUtils.now() + if (event.createdAt < now - TimeUtils.FIVE_MINUTES) { + saveLog("Event ${event.id} is too old: ${now - event.createdAt}s ago", relay.url) + return + } + if (event.createdAt > now + TimeUtils.FIVE_MINUTES) { + saveLog("Event ${event.id} is in the future: ${event.createdAt - now}s ahead", relay.url) + return + } + + val expiration = event.expiration() + if (expiration != null && expiration < now) { + saveLog("Event ${event.id} has expired", relay.url) + return + } + NotificationUtils.getOrCreateBunkerChannel(applicationContext) NotificationUtils.getOrCreateErrorsChannel(applicationContext) @@ -167,6 +185,14 @@ class EventNotificationConsumer(private val applicationContext: Context) { saveLog("Tagged account ${taggedKey.toNPub()} not logged in", relay.url) return } + + val bunkerEventDao = Amber.instance.getDatabase(acc.npub).bunkerEventDao() + if (bunkerEventDao.exists(event.id)) { + saveLog("Event ${event.id} already processed (persistent cache)", relay.url) + return + } + bunkerEventDao.insert(BunkerEventEntity(0, event.id, event.createdAt)) + notify(event, acc, relay, connectionPrivKey) } diff --git a/app/src/test/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumerTest.kt b/app/src/test/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumerTest.kt new file mode 100644 index 00000000..6ce1923d --- /dev/null +++ b/app/src/test/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumerTest.kt @@ -0,0 +1,129 @@ +package com.greenart7c3.nostrsigner.service + +import com.greenart7c3.nostrsigner.Amber +import com.greenart7c3.nostrsigner.LocalPreferences +import com.greenart7c3.nostrsigner.database.AppDatabase +import com.greenart7c3.nostrsigner.database.ApplicationDao +import com.greenart7c3.nostrsigner.database.BunkerEventDao +import com.greenart7c3.nostrsigner.models.Account +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.crypto.verify +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import com.vitorpamplona.quartz.nip01Core.tags.people.taggedUsers +import com.vitorpamplona.quartz.nip40Expiration.ExpirationTag +import com.vitorpamplona.quartz.nip46RemoteSigner.NostrConnectEvent +import com.vitorpamplona.quartz.utils.TimeUtils +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.mockkStatic +import io.mockk.spyk +import io.mockk.unmockkObject +import io.mockk.unmockkStatic +import io.mockk.verify +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Before +import org.junit.Test + +class EventNotificationConsumerTest { + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + private lateinit var consumer: EventNotificationConsumer + private lateinit var bunkerEventDao: BunkerEventDao + private lateinit var amber: Amber + private lateinit var account: Account + private lateinit var dao: ApplicationDao + + @Before + fun setUp() { + mockkStatic("com.vitorpamplona.quartz.nip01Core.crypto.EventKt") + mockkStatic("com.vitorpamplona.quartz.nip01Core.tags.people.EventExtKt") + mockkStatic("com.vitorpamplona.quartz.nip40Expiration.EventExtKt") + + bunkerEventDao = mockk(relaxed = true) + dao = mockk(relaxed = true) + val database = mockk(relaxed = true) + every { database.bunkerEventDao() } returns bunkerEventDao + every { database.dao() } returns dao + + amber = mockk(relaxed = true) + every { amber.getDatabase(any()) } returns database + every { amber.dao(any()) } returns dao + installAmberInstance(amber) + + mockkObject(LocalPreferences) + account = newTestAccount(scope) + + consumer = spyk(EventNotificationConsumer(mockk(relaxed = true))) + every { consumer["notificationManager"]() } returns mockk(relaxed = true) + } + + @After + fun tearDown() { + unmockkObject(LocalPreferences) + unmockkStatic("com.vitorpamplona.quartz.nip01Core.crypto.EventKt") + unmockkStatic("com.vitorpamplona.quartz.nip01Core.tags.people.EventExtKt") + unmockkStatic("com.vitorpamplona.quartz.nip40Expiration.EventExtKt") + } + + @Test + fun `consume rejects old events`() = runBlocking { + val event = mockk() + every { event.verify() } returns true + every { event.kind } returns NostrConnectEvent.KIND + every { event.createdAt } returns (TimeUtils.now() - TimeUtils.FIVE_MINUTES - 1) + + consumer.consume(event, NormalizedRelayUrl("wss://relay.com")) + + verify(exactly = 0) { event.tags } + } + + @Test + fun `consume rejects future events`() = runBlocking { + val event = mockk() + every { event.verify() } returns true + every { event.kind } returns NostrConnectEvent.KIND + every { event.createdAt } returns (TimeUtils.now() + TimeUtils.FIVE_MINUTES + 1) + + consumer.consume(event, NormalizedRelayUrl("wss://relay.com")) + + verify(exactly = 0) { event.tags } + } + + @Test + fun `consume rejects expired events`() = runBlocking { + val event = mockk() + every { event.verify() } returns true + every { event.kind } returns NostrConnectEvent.KIND + every { event.createdAt } returns TimeUtils.now() + every { event.tags } returns arrayOf(arrayOf(ExpirationTag.TAG_NAME, (TimeUtils.now() - 10).toString())) + + consumer.consume(event, NormalizedRelayUrl("wss://relay.com")) + + // If it passed expiration, it would call taggedUsers() + verify(exactly = 0) { event.taggedUsers() } + } + + @Test + fun `consume rejects duplicate events persistently`() = runBlocking { + val event = mockk() + every { event.id } returns "event1" + every { event.verify() } returns true + every { event.kind } returns NostrConnectEvent.KIND + every { event.createdAt } returns TimeUtils.now() + every { event.tags } returns arrayOf(arrayOf("p", account.hexKey)) + + coEvery { LocalPreferences.loadFromEncryptedStorageSync(any(), any()) } returns account + coEvery { bunkerEventDao.exists("event1") } returns true + + consumer.consume(event, NormalizedRelayUrl("wss://relay.com")) + + coVerify(exactly = 1) { bunkerEventDao.exists("event1") } + coVerify(exactly = 0) { bunkerEventDao.insert(any()) } + } +}