From 98dfca70956b09be1f6cbddcf4289843e752477f Mon Sep 17 00:00:00 2001 From: greenart7c3 Date: Mon, 10 Aug 2026 14:55:51 -0300 Subject: [PATCH] Envelope-encrypt NIP-46 connection secrets at rest (GHSA-5fjp-ghh8-wch8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-account Room database (amber_db_) stored two NIP-46 secret values as cleartext TEXT columns: the bunker connection `secret` and the `localKey` — the latter being a full Nostr private key. Anyone with access to the app's internal storage (rooted device, privilege-escalating malware, or a future bug exporting the DB) could recover these values and bypass the Keystore protection the main account nsec enjoys (CWE-312). Fix: envelope-encrypt both columns with the existing Keystore-backed AES-256-GCM key (SecureCryptoHelper) before Room persistence, and decrypt on read, so every existing consumer continues to see the plaintext values it already expects. - SecureCryptoHelper: add non-suspend encryptBlocking/decryptBlocking so Migration.migrate() and getByKeySync() can call them without a runBlocking bridge; suspend variants now delegate to the blocking implementations. - ApplicationEntityCrypto.kt (new): encryptForStorage/decryptFromStorage mappers + DecryptingPagingSource. Sentinel rule: empty values stay "" at rest (matches the WebDAV password idiom in LocalPreferences.kt:661-681), preserving `WHERE localKey != ''` enumeration in NotificationSubscription and the `localPubKey` derivation on empty localKey. - ApplicationDao: split methods touching `secret`/`localKey` into Room- generated `*Raw` (encrypted columns) and default-method wrappers that apply the mappers. `getBySecret` rewritten to decrypt and filter in Kotlin (random GCM IV breaks `WHERE secret = :secret`). - CachingApplicationDao: add delegating `*Raw` overrides so the decorator still instantiates; cache logic unchanged. - AppDatabase: add MIGRATION_18_19 (in-place envelope-encrypt of existing plaintext rows via compiled statement + transaction; empty values stay empty). Bump @Database version to 19. - Backup/restore: no changes — ApplicationBackup.buildPayload reads via the wrapped DAO (plaintext) and the JSON is already NIP-44 encrypted by the account key; restore goes through the wrapped insert (auto-encrypts). - Tests: new androidTest ApplicationEntityCryptoTest covers round-trip, raw-column-ciphertext assertion, empty sentinel, localPubKey derivation, getAllWithLocalKey filter, getBySecret (hit/miss/empty), insertApplicationWithPermissions, getAll, and the MIGRATION_18_19 row re-encryption. Requires a device/emulator (AndroidKeyStore unavailable under JVM test). - New room-testing androidTestImplementation dependency. Verified: ktlintCheck, lint (no issues), testFreeDebugUnitTest (0 failures), compileFreeDebugAndroidTestKotlin, assembleFreeDebug, assembleOfflineDebug, and the offline merged manifest check (no INTERNET/ACCESS_NETWORK_STATE/ CHANGE_NETWORK_STATE permissions leaked). --- app/build.gradle.kts | 1 + .../19.json | 235 +++++++++++++ .../database/ApplicationEntityCryptoTest.kt | 320 ++++++++++++++++++ .../nostrsigner/SecureCryptoHelper.kt | 26 +- .../nostrsigner/database/AppDatabase.kt | 53 ++- .../nostrsigner/database/ApplicationDao.kt | 175 +++++++--- .../database/ApplicationEntityCrypto.kt | 92 +++++ .../database/CachingApplicationDao.kt | 37 ++ gradle/libs.versions.toml | 1 + 9 files changed, 884 insertions(+), 56 deletions(-) create mode 100644 app/schemas/com.greenart7c3.nostrsigner.database.AppDatabase/19.json create mode 100644 app/src/androidTest/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCryptoTest.kt create mode 100644 app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCrypto.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 8509af8a..a22e93ed 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -247,6 +247,7 @@ dependencies { androidTestImplementation(libs.ext.junit) androidTestImplementation(libs.espresso.core) androidTestImplementation(libs.ui.test.junit4) + androidTestImplementation(libs.room.testing) debugImplementation(libs.leakcanary) debugImplementation(libs.ui.tooling) debugImplementation(libs.ui.test.manifest) 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..d1317b15 --- /dev/null +++ b/app/schemas/com.greenart7c3.nostrsigner.database.AppDatabase/19.json @@ -0,0 +1,235 @@ +{ + "formatVersion": 1, + "database": { + "version": 19, + "identityHash": "4d6bba48f6829e611cf02157dcc20cf5", + "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" + ] + } + ] + } + ], + "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, '4d6bba48f6829e611cf02157dcc20cf5')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCryptoTest.kt b/app/src/androidTest/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCryptoTest.kt new file mode 100644 index 00000000..72b2ec12 --- /dev/null +++ b/app/src/androidTest/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCryptoTest.kt @@ -0,0 +1,320 @@ +package com.greenart7c3.nostrsigner.database + +import androidx.room.Room +import androidx.room.testing.MigrationTestHelper +import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.greenart7c3.nostrsigner.SecureCryptoHelper +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Instrumented coverage for the GHSA-5fjp-ghh8-wch8 fix — envelope encryption + * of [ApplicationEntity.secret] and [ApplicationEntity.localKey] at the DAO + * boundary, plus the [MIGRATION_18_19] row re-encryption. + * + * Lives under `androidTest/` because [SecureCryptoHelper] depends on the + * AndroidKeyStore provider, which is unavailable under JVM unit tests. + */ +@RunWith(AndroidJUnit4::class) +class ApplicationEntityCryptoTest { + private lateinit var db: AppDatabase + private lateinit var dao: ApplicationDao + private val pubKey = "ab".repeat(32) + private val sampleSecret = "11111111-2222-3333-4444-555555555555" + private val sampleLocalKey = "cd".repeat(32) + + @Before + fun setUp() { + val context = InstrumentationRegistry.getInstrumentation().targetContext + db = Room.inMemoryDatabaseBuilder(context, AppDatabase::class.java) + .allowMainThreadQueries() + .build() + dao = db.dao() + } + + @After + fun tearDown() { + db.close() + } + + private fun newEntity( + key: String = "app1", + secret: String = sampleSecret, + localKey: String = sampleLocalKey, + ) = ApplicationEntity( + key = key, + name = "Test Bunker", + relays = emptyList(), + url = "", + icon = "", + description = "", + pubKey = pubKey, + isConnected = false, + secret = secret, + useSecret = true, + signPolicy = 1, + closeApplication = true, + deleteAfter = 0L, + lastUsed = 0L, + localKey = localKey, + ) + + @Test + fun insertAndRead_roundTrips_plaintextSecretAndLocalKey() = runBlocking { + dao.insertApplication(newEntity()) + + val read = dao.getByKey("app1")?.application + assertNotNull(read) + assertEquals(sampleSecret, read?.secret) + assertEquals(sampleLocalKey, read?.localKey) + } + + @Test + fun insertPersistsCiphertext_rawColumnDiffersFromPlaintext() = runBlocking { + dao.insertApplication(newEntity()) + + val rawCursor = db.openHelper.readableDatabase.query( + "SELECT `secret`, `localKey` FROM application WHERE `key` = ?", + arrayOf("app1"), + ) + rawCursor.use { c -> + assertTrue("row must exist", c.moveToFirst()) + val rawSecret = c.getString(0) + val rawLocalKey = c.getString(1) + assertNotEquals("stored secret must not be plaintext", sampleSecret, rawSecret) + assertNotEquals("stored localKey must not be plaintext", sampleLocalKey, rawLocalKey) + // Sentinel format: Base64 NO_WRAP of IV(12) ‖ ciphertext+tag (>= 16 bytes) + assertTrue("raw secret looks like Base64", rawSecret.matches(Regex("^[A-Za-z0-9+/=]+$"))) + assertTrue("raw localKey looks like Base64", rawLocalKey.matches(Regex("^[A-Za-z0-9+/=]+$"))) + // Round-trips through SecureCryptoHelper to the original plaintext. + assertEquals(sampleSecret, SecureCryptoHelper.decryptBlocking(rawSecret)) + assertEquals(sampleLocalKey, SecureCryptoHelper.decryptBlocking(rawLocalKey)) + } + } + + @Test + fun emptySentinel_storedAsEmpty_decryptedAsEmpty() = runBlocking { + dao.insertApplication(newEntity(secret = "", localKey = "")) + + val read = dao.getByKey("app1")?.application + assertNotNull(read) + assertEquals("", read?.secret) + assertEquals("", read?.localKey) + + // Raw column should also be empty (not encrypted to a Base64 token). + val rawCursor = db.openHelper.readableDatabase.query( + "SELECT `secret`, `localKey` FROM application WHERE `key` = ?", + arrayOf("app1"), + ) + rawCursor.use { c -> + assertTrue(c.moveToFirst()) + assertEquals("", c.getString(0)) + assertEquals("", c.getString(1)) + } + } + + @Test + fun localPubKey_accessorReturnsDerivedPubkey() = runBlocking { + dao.insertApplication(newEntity()) + + val read = dao.getByKey("app1")?.application + assertNotNull(read) + assertEquals( + localPubKeyFromPrivKey(sampleLocalKey), + read?.localPubKey, + ) + } + + @Test + fun getAllWithLocalKey_onlyReturnsRowsWithNonEmptyLocalKey() = runBlocking { + dao.insertApplication(newEntity(key = "withKey", localKey = sampleLocalKey)) + dao.insertApplication(newEntity(key = "withoutKey", localKey = "")) + + val rows = dao.getAllWithLocalKey(pubKey) + assertEquals(1, rows.size) + assertEquals("withKey", rows.first().key) + assertEquals(sampleLocalKey, rows.first().localKey) + } + + @Test + fun getBySecret_findsRowByPlaintextSecret() = runBlocking { + dao.insertApplication(newEntity(key = "bunker1", secret = sampleSecret)) + + val found = dao.getBySecret(sampleSecret)?.application + assertNotNull(found) + assertEquals("bunker1", found?.key) + assertEquals(sampleSecret, found?.secret) + } + + @Test + fun getBySecret_returnsNullForUnknownSecret() = runBlocking { + dao.insertApplication(newEntity(secret = sampleSecret)) + + assertNull(dao.getBySecret("not-a-known-secret")) + } + + @Test + fun getBySecret_findsRowWithEmptySecretWhenQueriedWithEmpty() = runBlocking { + dao.insertApplication(newEntity(key = "empty", secret = "")) + + val found = dao.getBySecret("")?.application + assertNotNull(found) + assertEquals("empty", found?.key) + } + + @Test + fun insertApplicationWithPermissions_encryptsAndReReadsPlaintext() = runBlocking { + val entity = newEntity(key = "permApp") + val perms = mutableListOf( + ApplicationPermissionsEntity( + id = null, + pkKey = "permApp", + type = "SIGN_EVENT", + kind = 1, + acceptable = true, + rememberType = 1, + acceptUntil = 0L, + rejectUntil = 0L, + ), + ) + dao.insertApplicationWithPermissions( + ApplicationWithPermissions( + application = entity, + permissions = perms, + ), + ) + + val read = dao.getByKey("permApp") + assertNotNull(read) + assertEquals(sampleSecret, read?.application?.secret) + assertEquals(sampleLocalKey, read?.application?.localKey) + assertEquals(1, read?.permissions?.size) + } + + @Test + fun getAll_decryptsAllRows() = runBlocking { + dao.insertApplication(newEntity(key = "a", secret = "secret-a", localKey = "11".repeat(32))) + dao.insertApplication(newEntity(key = "b", secret = "secret-b", localKey = "22".repeat(32))) + + val rows = dao.getAll(pubKey) + assertEquals(2, rows.size) + val byKey = rows.associateBy { it.key } + assertEquals("secret-a", byKey["a"]?.secret) + assertEquals("secret-b", byKey["b"]?.secret) + assertEquals("11".repeat(32), byKey["a"]?.localKey) + assertEquals("22".repeat(32), byKey["b"]?.localKey) + } + + private val dbName = "migration_test.db" + + @get:Rule + val migrationHelper: MigrationTestHelper = MigrationTestHelper( + InstrumentationRegistry.getInstrumentation(), + AppDatabase::class.java, + listOf(), + FrameworkSQLiteOpenHelperFactory(), + ) + + /** + * Verifies MIGRATION_18_19 envelope-encrypts existing plaintext columns. + * + * Creates a v18 DB (schema loaded from app/schemas/18.json), inserts + * plaintext `secret`/`localKey` rows directly via SQL, runs the migration + * to v19, then opens the DB via [AppDatabase] and asserts the DAO returns + * the original plaintext values while the raw columns now hold ciphertext. + */ + @Test + fun migration_18_19_encryptsPlaintextSecrets() { + val context = InstrumentationRegistry.getInstrumentation().targetContext + context.deleteDatabase(dbName) + + val v18 = migrationHelper.createDatabase(dbName, 18) + // v18 schema has the `application` table with `secret` and `localKey` TEXT columns. + v18.execSQL( + "INSERT INTO application " + + "(`key`, name, relays, url, icon, description, pubKey, isConnected, " + + "secret, useSecret, signPolicy, closeApplication, deleteAfter, lastUsed, localKey) " + + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + arrayOf( + "migApp", + "Migration App", + "", + "", + "", + "", + pubKey, + 0, + sampleSecret, + 1, + 1, + 1, + 0L, + 0L, + sampleLocalKey, + ), + ) + v18.execSQL( + "INSERT INTO application " + + "(`key`, name, relays, url, icon, description, pubKey, isConnected, " + + "secret, useSecret, signPolicy, closeApplication, deleteAfter, lastUsed, localKey) " + + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + arrayOf( + "migAppEmpty", + "Empty App", + "", + "", + "", + "", + pubKey, + 0, + "", + 0, + 1, + 1, + 0L, + 0L, + "", + ), + ) + v18.close() + + val migrated = migrationHelper.runMigrationsAndValidate(dbName, 19, false, MIGRATION_18_19) + // After migration the raw columns must be ciphertext for non-empty values + // and "" for the empty-sentinel row. + val rawCursor = migrated.query( + "SELECT `secret`, `localKey` FROM application WHERE `key` = ?", + arrayOf("migApp"), + ) + rawCursor.use { c -> + assertTrue(c.moveToFirst()) + val rawSecret = c.getString(0) + val rawLocalKey = c.getString(1) + assertNotEquals(sampleSecret, rawSecret) + assertNotEquals(sampleLocalKey, rawLocalKey) + assertEquals(sampleSecret, SecureCryptoHelper.decryptBlocking(rawSecret)) + assertEquals(sampleLocalKey, SecureCryptoHelper.decryptBlocking(rawLocalKey)) + } + val emptyCursor = migrated.query( + "SELECT `secret`, `localKey` FROM application WHERE `key` = ?", + arrayOf("migAppEmpty"), + ) + emptyCursor.use { c -> + assertTrue(c.moveToFirst()) + assertEquals("", c.getString(0)) + assertEquals("", c.getString(1)) + } + migrated.close() + context.deleteDatabase(dbName) + } +} diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt b/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt index 9157f4db..8613fc24 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt @@ -24,11 +24,27 @@ object SecureCryptoHelper { private val mutex = Mutex() suspend fun encrypt(plainText: String): String = mutex.withLock { + encryptBlocking(plainText) + } + + suspend fun decrypt(encryptedText: String): String = mutex.withLock { + decryptBlocking(encryptedText) + } + + /** + * Non-suspending equivalent of [encrypt] for callers that cannot suspend + * (Room [Migration.migrate] callbacks, [getByKeySync] synchronous DAO + * reads). The AndroidKeyStore Cipher instance is thread-safe to obtain and + * use; the suspend variant's [mutex] only guards against concurrent + * in-flight cipher init within coroutines and is intentionally omitted + * here so blocking callers do not need a [kotlinx.coroutines.runBlocking] + * bridge. + */ + fun encryptBlocking(plainText: String): String { val key = getOrCreateSecretKey() val cipher = Cipher.getInstance(TRANSFORMATION) - cipher.init(Cipher.ENCRYPT_MODE, key) - val iv = cipher.iv // System-generated, allowed + val iv = cipher.iv val cipherText = cipher.doFinal(plainText.toByteArray(Charsets.UTF_8)) @@ -39,7 +55,11 @@ object SecureCryptoHelper { return Base64.encodeToString(combined.array(), Base64.NO_WRAP) } - suspend fun decrypt(encryptedText: String): String = mutex.withLock { + /** + * Non-suspending equivalent of [decrypt]. See [encryptBlocking] for the + * rationale. + */ + fun decryptBlocking(encryptedText: String): String { val key = getOrCreateSecretKey() val data = Base64.decode(encryptedText, Base64.NO_WRAP) val buffer = ByteBuffer.wrap(data) 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..518a72aa 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/AppDatabase.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/AppDatabase.kt @@ -9,6 +9,7 @@ import androidx.room.migration.Migration import androidx.sqlite.db.SupportSQLiteDatabase import com.greenart7c3.nostrsigner.Amber import com.greenart7c3.nostrsigner.AmberLog +import com.greenart7c3.nostrsigner.SecureCryptoHelper import java.util.concurrent.Executors val MIGRATION_1_2 = @@ -165,12 +166,61 @@ val MIGRATION_17_18 = object : Migration(17, 18) { } } +/** + * GHSA-5fjp-ghh8-wch8 (CWE-312): envelope-encrypt the two NIP-46 secret columns + * of the `application` table — `secret` (bunker shared secret) and `localKey` + * (a full Nostr private key used for the relay-side connection identity) — + * with the existing Keystore-backed AES-256-GCM key in [SecureCryptoHelper]. + * + * Pre-migration these columns are ciphertext-unaware plaintext TEXT, so the + * migration reads each row, encrypts the non-empty values in place, and writes + * them back. Empty values stay `""` (sentinel — see + * [ApplicationEntity.encryptForStorage]) to preserve `WHERE localKey != ''` + * enumeration in `NotificationSubscription.kt:90-96`. + * + * Idempotency: Room migrations run exactly once per version bump. The + * migration is not safe to re-run on already-encrypted data (it would + * double-encrypt), so it must only run between schema 18 and 19. + */ +val MIGRATION_18_19 = object : Migration(18, 19) { + override fun migrate(db: SupportSQLiteDatabase) { + db.beginTransaction() + try { + val cursor = db.query("SELECT `key`, `secret`, `localKey` FROM application") + val updateStmt = db.compileStatement( + "UPDATE application SET `secret` = ?, `localKey` = ? WHERE `key` = ?", + ) + cursor.use { c -> + while (c.moveToNext()) { + val key = c.getString(0) + val secret = c.getString(1) + val localKey = c.getString(2) + val newSecret = if (secret.isEmpty()) "" else SecureCryptoHelper.encryptBlocking(secret) + val newLocalKey = if (localKey.isEmpty()) "" else SecureCryptoHelper.encryptBlocking(localKey) + updateStmt.bindString(1, newSecret) + updateStmt.bindString(2, newLocalKey) + updateStmt.bindString(3, key) + updateStmt.executeUpdateDelete() + // Reuse the compiled statement for the next row. + updateStmt.clearBindings() + } + } + db.setTransactionSuccessful() + } catch (e: Exception) { + AmberLog.e(Amber.TAG, "MIGRATION_18_19: failed to envelope-encrypt application secrets", e) + throw e + } finally { + db.endTransaction() + } + } +} + @Database( entities = [ ApplicationEntity::class, ApplicationPermissionsEntity::class, ], - version = 18, + version = 19, ) @TypeConverters(Converters::class) abstract class AppDatabase : RoomDatabase() { @@ -209,6 +259,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/ApplicationDao.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt index 30d9c04b..deaeef99 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt @@ -7,21 +7,57 @@ import androidx.room.Insert import androidx.room.OnConflictStrategy import androidx.room.Query import androidx.room.Transaction +import com.greenart7c3.nostrsigner.SecureCryptoHelper +/** + * Room DAO for the per-account `application` table. + * + * **At-rest envelope encryption** (GHSA-5fjp-ghh8-wch8, CWE-312): the two + * NIP-46 secret columns [ApplicationEntity.secret] and + * [ApplicationEntity.localKey] are persisted as Keystore-encrypted + * (`SecureCryptoHelper.encryptBlocking`) Base64 ciphertext. The non-`Raw` + * methods below are interface default methods that transparently encrypt on + * write and decrypt on read via the [ApplicationEntity.encryptForStorage] / + * [ApplicationEntity.decryptFromStorage] mappers, so every consumer of + * `ApplicationEntity` / `ApplicationWithPermissions` continues to see the + * plaintext values it already expects. The only at-rest reader is Room itself. + * + * The `*Raw` methods are Room-generated `@Query` / `@Insert` implementations + * operating on the raw (encrypted) column values. Callers must use the public + * default-method wrappers; the `Raw` methods are the persistence boundary. + * + * Sentinel: an empty `secret` or `localKey` is stored as `""` (not encrypted), + * matching `LocalPreferences.kt:661-681`'s WebDAV password idiom and preserving + * `WHERE localKey != ''` enumeration semantics in + * `NotificationSubscription.kt:90-96`. + * + * `getBySecret` cannot use a SQL `WHERE secret = :secret` predicate because + * GCM uses a random IV — the same plaintext produces a distinct ciphertext + * each call. The lookup is carried out in Kotlin by decrypting all rows of the + * (small, per-account) `application` table and filtering by the plaintext + * secret. Caller call-sites: `BunkerSingleEventHomeScreen.kt:83`, + * `BunkerConnectRequestScreen.kt:125`. + */ @Dao interface ApplicationDao { @Query("SELECT signPolicy FROM application WHERE `key` = :key") fun getSignPolicy(key: String): Int? @Query("SELECT * FROM application where pubKey = :pubKey order by name") - suspend fun getAll(pubKey: String): List + suspend fun getAllRaw(pubKey: String): List + + suspend fun getAll(pubKey: String): List = getAllRaw(pubKey).map { it.decryptFromStorage() } @Query("SELECT * FROM application where isConnected = 0") @Transaction - suspend fun getAllNotConnected(): List + suspend fun getAllNotConnectedRaw(): List + + suspend fun getAllNotConnected(): List = getAllNotConnectedRaw().map { it.decryptFromStorage() } @Query("SELECT a.* FROM application a WHERE a.pubKey = :pubKey ORDER BY a.lastUsed DESC") - fun getAllPaging(pubKey: String): PagingSource + fun getAllPagingRaw(pubKey: String): PagingSource + + fun getAllPaging(pubKey: String): PagingSource = DecryptingPagingSource(getAllPagingRaw(pubKey)) @Query("SELECT DISTINCT relays FROM application") fun getAllRelayLists(): List @@ -31,22 +67,48 @@ interface ApplicationDao { @Query("SELECT * FROM application WHERE `key` = :key") @Transaction - suspend fun getByKey(key: String): ApplicationWithPermissions? + suspend fun getByKeyRaw(key: String): ApplicationWithPermissions? + + suspend fun getByKey(key: String): ApplicationWithPermissions? = getByKeyRaw(key)?.decryptFromStorage() @Query("SELECT * FROM application WHERE `key` = :key") @Transaction - fun getByKeySync(key: String): ApplicationWithPermissions? + fun getByKeySyncRaw(key: String): ApplicationWithPermissions? + + fun getByKeySync(key: String): ApplicationWithPermissions? = getByKeySyncRaw(key)?.decryptFromStorage() @Query("SELECT * FROM application WHERE `name` = :name LIMIT 1") @Transaction - suspend fun getByName(name: String): ApplicationWithPermissions? + suspend fun getByNameRaw(name: String): ApplicationWithPermissions? - @Query("SELECT * FROM application WHERE secret = :secret") - @Transaction - suspend fun getBySecret(secret: String): ApplicationWithPermissions? + suspend fun getByName(name: String): ApplicationWithPermissions? = getByNameRaw(name)?.decryptFromStorage() + + @Query("SELECT * FROM application") + suspend fun getAllApplicationsRaw(): List + + /** + * Lookup by NIP-46 bunker connection `secret`. Cannot use SQL + * `WHERE secret = :secret` after envelope encryption (random GCM IV); + * decrypt all rows of the per-account table and filter in Kotlin. + * Tables are bounded by NIP-46 connection count (typically < 100). + */ + suspend fun getBySecret(secret: String): ApplicationWithPermissions? { + val matchKey = getAllApplicationsRaw().firstOrNull { row -> + if (row.secret.isEmpty()) { + secret.isEmpty() + } else { + runCatching { SecureCryptoHelper.decryptBlocking(row.secret) }.getOrDefault(null) == secret + } + }?.key ?: return null + // Eager-load the related permissions via the wrapped getByKey so callers + // receive the same shape the original @Transaction/SELECT joined query did. + return getByKey(matchKey) + } @Query("SELECT * FROM application WHERE pubKey = :pubKey AND localKey != ''") - suspend fun getAllWithLocalKey(pubKey: String): List + suspend fun getAllWithLocalKeyRaw(pubKey: String): List + + suspend fun getAllWithLocalKey(pubKey: String): List = getAllWithLocalKeyRaw(pubKey).map { it.decryptFromStorage() } @Query("UPDATE applicationPermission set acceptUntil = 0, rejectUntil = 0, rememberType = 0 where (acceptUntil < :time OR rejectUntil < :time) AND rememberType <> 4") fun updateExpiredPermissions(time: Long) @@ -107,13 +169,25 @@ interface ApplicationDao { @Insert(onConflict = OnConflictStrategy.REPLACE) @Transaction - suspend fun insertApplication(event: ApplicationEntity): Long? + suspend fun insertApplicationRaw(event: ApplicationEntity): Long? + + suspend fun insertApplication(event: ApplicationEntity): Long? = insertApplicationRaw(event.encryptForStorage()) @Insert(onConflict = OnConflictStrategy.REPLACE) - fun insertAll(events: List): List? + fun insertAllRaw(events: List): List? + + fun insertAll(events: List): List? = insertAllRaw(events.map { it.encryptForStorage() }) + + @Insert(onConflict = OnConflictStrategy.IGNORE) + @Transaction + suspend fun insertPermissions2Raw(permissions: List): List? + + suspend fun insertPermissions2(permissions: List): List? = insertPermissions2Raw(permissions) @Insert(onConflict = OnConflictStrategy.REPLACE) @Transaction + suspend fun insertPermissionsRaw(permissions: List): List? + suspend fun insertPermissions(permissions: List): List? { permissions.forEach { if (it.kind != null) { @@ -132,12 +206,46 @@ interface ApplicationDao { deletePermissions(it.pkKey, it.type) } } - return insertPermissions2(permissions) + return insertPermissions2Raw(permissions) } - @Insert(onConflict = OnConflictStrategy.IGNORE) @Transaction - suspend fun insertPermissions2(permissions: List): List? + suspend fun insertApplicationWithPermissions(application: ApplicationWithPermissions) { + deletePermissions(application.application.key) + insertApplicationRaw(application.application.encryptForStorage())?.let { + application.permissions.forEach { + it.pkKey = application.application.key + } + + insertPermissions(application.permissions) + } + } + + @Delete + @Transaction + suspend fun deleteRaw(entity: ApplicationEntity) + + suspend fun delete(entity: ApplicationEntity) = deleteRaw(entity) + + @Query("DELETE FROM application WHERE `key` = :key") + @Transaction + suspend fun delete(key: String) + + @Query("UPDATE application SET lastUsed = :time where `key` = :key") + @Transaction + suspend fun updateLastUsed(key: String, time: Long) + + @Query("UPDATE application SET name = :name, icon = :icon WHERE `key` = :key") + @Transaction + suspend fun updateNameAndIcon(key: String, name: String, icon: String) + + @Delete + @Transaction + suspend fun deletePermission(permission: ApplicationPermissionsEntity) + + @Query("DELETE FROM application WHERE deleteAfter < :time AND deleteAfter > 0") + @Transaction + suspend fun deleteOldApplications(time: Long): Int @Query("DELETE FROM applicationPermission WHERE pkKey = :key") @Transaction @@ -174,41 +282,4 @@ interface ApplicationDao { type: String, kind: Int, ) - - @Insert(onConflict = OnConflictStrategy.IGNORE) - @Transaction - suspend fun insertApplicationWithPermissions(application: ApplicationWithPermissions) { - deletePermissions(application.application.key) - insertApplication(application.application)?.let { - application.permissions.forEach { - it.pkKey = application.application.key - } - - insertPermissions(application.permissions) - } - } - - @Delete - @Transaction - suspend fun delete(entity: ApplicationEntity) - - @Query("DELETE FROM application WHERE `key` = :key") - @Transaction - suspend fun delete(key: String) - - @Query("UPDATE application SET lastUsed = :time where `key` = :key") - @Transaction - suspend fun updateLastUsed(key: String, time: Long) - - @Query("UPDATE application SET name = :name, icon = :icon WHERE `key` = :key") - @Transaction - suspend fun updateNameAndIcon(key: String, name: String, icon: String) - - @Delete - @Transaction - suspend fun deletePermission(permission: ApplicationPermissionsEntity) - - @Query("DELETE FROM application WHERE deleteAfter < :time AND deleteAfter > 0") - @Transaction - suspend fun deleteOldApplications(time: Long): Int } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCrypto.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCrypto.kt new file mode 100644 index 00000000..98251a73 --- /dev/null +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCrypto.kt @@ -0,0 +1,92 @@ +package com.greenart7c3.nostrsigner.database + +import androidx.paging.PagingSource +import androidx.paging.PagingState +import com.greenart7c3.nostrsigner.Amber +import com.greenart7c3.nostrsigner.AmberLog +import com.greenart7c3.nostrsigner.SecureCryptoHelper + +/** + * Envelope-encrypts / envelope-decrypts the two NIP-46 secret columns of + * [ApplicationEntity] ([secret] and [localKey]) using the Keystore-backed + * AES-256-GCM key in [SecureCryptoHelper]. + * + * Mitigates GHSA-5fjp-ghh8-wch8 (CWE-312): both columns previously lived as + * cleartext Room TEXT on the per-account `amber_db_` database, so any + * reader of the app's internal storage (rooted device, privilege-escalating + * malware, or a future bug exporting the DB) recovered every bunker connection + * secret and every locally generated connection private key. + * + * Sentinel rule for empty values: `""` stays `""` at rest. Mirrors the WebDAV + * password idiom at `LocalPreferences.kt:661-681` and preserves: + * - `ApplicationDao` `WHERE localKey != ''` semantics + * (used by `NotificationSubscription.kt:90-96` to enumerate connections). + * - `ApplicationEntity.localPubKey` derivation + * (`if (localKey.isNotEmpty()) localPubKeyFromPrivKey(localKey) else ""`). + * - `ApplicationEntity.shouldShowRelays` (`secret.isNotEmpty()`). + * + * The mappers are applied at the DAO boundary so every consumer of + * `ApplicationEntity` / `ApplicationWithPermissions` continues to see the + * plaintext values it already expects. The only at-rest reader is Room itself. + * + * Failure mode: if [SecureCryptoHelper.decryptBlocking] throws (e.g. the + * AndroidKeyStore key has become permanently unusable after a device KeyMint + * upgrade), the row is returned with the raw ciphertext in place. The caller + * will then treat the secret as a non-matching string (lookup miss) and the + * corrupted localKey as a non-hex value, which surfaces as a connection + * failure rather than a crash — matching the `InvalidKeyException`/log-and-skip + * convention in `LocalPreferences.loadFromEncryptedStorage`. + */ +private fun String.encryptField(): String = if (this.isEmpty()) this else SecureCryptoHelper.encryptBlocking(this) + +private fun String.decryptField(): String { + if (this.isEmpty()) return this + return try { + SecureCryptoHelper.decryptBlocking(this) + } catch (e: Exception) { + // Leave the ciphertext in place rather than crashing the DAO read. + // See class kdoc for the recovery contract. + AmberLog.w(Amber.TAG, "ApplicationEntityCrypto: failed to decrypt field", e) + this + } +} + +/** Encrypts [secret] and [localKey] for Room persistence. Inverse of [decryptFromStorage]. */ +fun ApplicationEntity.encryptForStorage(): ApplicationEntity = copy( + secret = secret.encryptField(), + localKey = localKey.encryptField(), +) + +/** Decrypts [secret] and [localKey] after a Room read. Inverse of [encryptForStorage]. */ +fun ApplicationEntity.decryptFromStorage(): ApplicationEntity = copy( + secret = secret.decryptField(), + localKey = localKey.decryptField(), +) + +/** Decrypts the embedded [ApplicationWithPermissions.application] after a Room read. */ +fun ApplicationWithPermissions.decryptFromStorage(): ApplicationWithPermissions = copy(application = application.decryptFromStorage()) + +/** + * Wraps a raw (encrypted-column) [PagingSource] and decrypts every + * [ApplicationEntity] emitted by a successful [LoadResult.Page]. Error and + * Invalid results are passed through unchanged. Used by + * [ApplicationDao.getAllPaging] so the `ApplicationsScreen` Pager sees + * plaintext entities exactly like the non-paging DAO reads. + */ +internal class DecryptingPagingSource( + private val delegate: PagingSource, +) : PagingSource() { + override fun getRefreshKey(state: PagingState): Int? = delegate.getRefreshKey(state) + + override suspend fun load(params: PagingSource.LoadParams): PagingSource.LoadResult = when (val result = delegate.load(params)) { + is PagingSource.LoadResult.Page -> PagingSource.LoadResult.Page( + data = result.data.map { it.decryptFromStorage() }, + prevKey = result.prevKey, + nextKey = result.nextKey, + itemsBefore = result.itemsBefore, + itemsAfter = result.itemsAfter, + ) + is PagingSource.LoadResult.Error -> result + is PagingSource.LoadResult.Invalid -> result + } +} diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt index 34b3c790..f9e96b2d 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt @@ -230,6 +230,43 @@ class CachingApplicationDao( invalidateApp(key) } + // -------- *Raw delegations (envelope-encrypted persistence boundary) -------- + // + // ApplicationDao now splits methods that touch the [ApplicationEntity.secret] + // / [ApplicationEntity.localKey] columns into a Room-generated `*Raw` + // (encrypted column values) and a public default-method wrapper that + // envelope-encrypts/decrypts via [ApplicationEntity.encryptForStorage]. + // CachingApplicationDao is a decorator over a Room-generated ApplicationDao, + // so its `*Raw` overrides simply forward to [delegate] — they are never + // invoked from the cache layer (the public wrappers above are the entry + // points), but Kotlin requires every interface member to be implemented. + + override suspend fun getAllRaw(pubKey: String): List = delegate.getAllRaw(pubKey) + + override suspend fun getAllNotConnectedRaw(): List = delegate.getAllNotConnectedRaw() + + override fun getAllPagingRaw(pubKey: String): PagingSource = delegate.getAllPagingRaw(pubKey) + + override suspend fun getByKeyRaw(key: String): ApplicationWithPermissions? = delegate.getByKeyRaw(key) + + override fun getByKeySyncRaw(key: String): ApplicationWithPermissions? = delegate.getByKeySyncRaw(key) + + override suspend fun getByNameRaw(name: String): ApplicationWithPermissions? = delegate.getByNameRaw(name) + + override suspend fun getAllApplicationsRaw(): List = delegate.getAllApplicationsRaw() + + override suspend fun getAllWithLocalKeyRaw(pubKey: String): List = delegate.getAllWithLocalKeyRaw(pubKey) + + override suspend fun insertApplicationRaw(event: ApplicationEntity): Long? = delegate.insertApplicationRaw(event) + + override fun insertAllRaw(events: List): List? = delegate.insertAllRaw(events) + + override suspend fun insertPermissions2Raw(permissions: List): List? = delegate.insertPermissions2Raw(permissions) + + override suspend fun insertPermissionsRaw(permissions: List): List? = delegate.insertPermissionsRaw(permissions) + + override suspend fun deleteRaw(entity: ApplicationEntity) = delegate.deleteRaw(entity) + companion object { private const val MAX_ENTRIES = 512 } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7e523c81..b1384012 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -59,6 +59,7 @@ room-compiler = { module = "androidx.room:room-compiler", version.ref = "roomKtx room-ktx = { module = "androidx.room:room-ktx", version.ref = "roomKtx" } room-runtime = { module = "androidx.room:room-runtime", version.ref = "roomKtx" } room-paging = { module = "androidx.room:room-paging", version.ref = "roomKtx" } +room-testing = { module = "androidx.room:room-testing", version.ref = "roomKtx" } security-crypto = { module = "androidx.security:security-crypto", version.ref = "securityCryptoKtx" } security-crypto-ktx = { module = "androidx.security:security-crypto-ktx", version.ref = "securityCryptoKtx" } ui = { module = "androidx.compose.ui:ui", version.ref = "compose_ui" }