diff --git a/app/src/androidTest/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCryptoTest.kt b/app/src/androidTest/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCryptoTest.kt index 72b2ec12..d0246668 100644 --- a/app/src/androidTest/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCryptoTest.kt +++ b/app/src/androidTest/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCryptoTest.kt @@ -173,6 +173,51 @@ class ApplicationEntityCryptoTest { assertEquals("empty", found?.key) } + /** + * Key-rotation write path (SecureCryptoHelper.rotateKey step 4): the DAO + * must accept already-encrypted column values written under the target + * Keystore key so the wrapper reads decrypt them back to plaintext. + */ + @Test + fun updateEncryptedColumnsRaw_rewritesRowCiphertext_readBackThroughWrapper() = runBlocking { + dao.insertApplication(newEntity(key = "rot")) + + val newSecret = "99999999-8888-7777-6666-555555555555" + val newLocalKey = "ef".repeat(32) + dao.updateEncryptedColumnsRaw( + key = "rot", + secret = SecureCryptoHelper.encryptBlocking(newSecret), + localKey = SecureCryptoHelper.encryptBlocking(newLocalKey), + ) + + val read = dao.getByKey("rot")?.application + assertNotNull(read) + assertEquals(newSecret, read?.secret) + assertEquals(newLocalKey, read?.localKey) + // Raw columns were replaced, not duplicated/concatenated. + val rawCursor = db.openHelper.readableDatabase.query( + "SELECT `secret` FROM application WHERE `key` = ?", + arrayOf("rot"), + ) + rawCursor.use { c -> + assertTrue(c.moveToFirst()) + assertNotEquals(newSecret, c.getString(0)) + assertEquals(newSecret, SecureCryptoHelper.decryptBlocking(c.getString(0))) + } + } + + @Test + fun updateEncryptedColumnsRaw_emptySentinel_roundTripsEmpty() = runBlocking { + dao.insertApplication(newEntity(key = "rotEmpty")) + + dao.updateEncryptedColumnsRaw(key = "rotEmpty", secret = "", localKey = "") + + val read = dao.getByKey("rotEmpty")?.application + assertNotNull(read) + assertEquals("", read?.secret) + assertEquals("", read?.localKey) + } + @Test fun insertApplicationWithPermissions_encryptsAndReReadsPlaintext() = runBlocking { val entity = newEntity(key = "permApp") diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt b/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt index 771ab7d3..c9bde93c 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt @@ -18,6 +18,7 @@ import kotlinx.coroutines.sync.withLock object SecureCryptoHelper { private const val ANDROID_KEYSTORE = "AndroidKeyStore" private const val KEY_ALIAS = "AMBER_AES_KEY" + private const val TAG = "SecureCryptoHelper" private const val TRANSFORMATION = "AES/GCM/NoPadding" private const val IV_SIZE = 12 // 96 bits private const val TAG_SIZE = 128 // bits @@ -117,7 +118,7 @@ object SecureCryptoHelper { keyGenerator.init(paramsBuilder.build()) return keyGenerator.generateKey() } catch (e: Exception) { - AmberLog.w("SecureCryptoHelper", "StrongBox generation failed, falling back to TEE", e) + AmberLog.w(TAG, "StrongBox generation failed, falling back to TEE", e) paramsBuilder.setIsStrongBoxBacked(false) keyGenerator.init(paramsBuilder.build()) return keyGenerator.generateKey() @@ -130,7 +131,9 @@ object SecureCryptoHelper { /** * Rotates the [KEY_ALIAS] Keystore key, re-encrypting every stored secret - * (per-account DataStore keys, app DataStore PIN, and WebDAV password). + * (per-account DataStore keys, app DataStore PIN, WebDAV password, and the + * per-account Room `application` tables' envelope-encrypted `secret` / + * `localKey` columns — see `ApplicationEntityCrypto.kt`). * * The Keystore `setUnlockedDeviceRequired` flag is set at key-generation * time, so toggling the opt-in policy requires destroying and recreating @@ -140,8 +143,8 @@ object SecureCryptoHelper { * it runs after all decryptions succeed). * * Uses internal non-locking cipher methods ([encryptWithKey]/ - * [decryptWithKey]) and raw DataStore helpers to avoid mutex reentrancy - * (Kotlin's [Mutex] is not reentrant). + * [decryptWithKey]) and raw DataStore/DAO helpers to avoid mutex + * reentrancy (Kotlin's [Mutex] is not reentrant). */ suspend fun rotateKey(context: Context, requireUnlockedDevice: Boolean) = mutex.withLock { val keyStore = getKeyStore() @@ -198,6 +201,28 @@ object SecureCryptoHelper { null } + // 1b. Read the per-account Room `application` tables and decrypt the + // envelope-encrypted `secret`/`localKey` columns (GHSA-5fjp-ghh8-wch8) + // with the old key — still before any destructive step. Rows whose + // columns fail to decrypt keep their raw ciphertext on write-back + // (rotation must not destroy data it cannot recover). + val decryptedAppRows = mutableMapOf>() + for (acc in accounts) { + val rows = runCatching { Amber.instance.dao(acc.npub).getAllApplicationsRaw() } + .onFailure { AmberLog.w(TAG, "Key rotation: failed to read application rows", it) } + .getOrElse { emptyList() } + decryptedAppRows[acc.npub] = rows.mapNotNull { row -> + if (row.secret.isEmpty() && row.localKey.isEmpty()) return@mapNotNull null + RotatedAppRow( + key = row.key, + secretPlain = row.secret.decryptOrNullWith(oldKey), + localKeyPlain = row.localKey.decryptOrNullWith(oldKey), + secretRaw = row.secret, + localKeyRaw = row.localKey, + ) + } + } + // 2. Delete the old key and generate a new one with the new policy. keyStore.deleteEntry(KEY_ALIAS) val newKey = generateSecretKey(requireUnlockedDevice) @@ -218,8 +243,52 @@ object SecureCryptoHelper { LocalPreferences.saveWebDavPasswordEncrypted(context, encryptWithKey(newKey, it)) } - AmberLog.d("SecureCryptoHelper", "Key rotation complete (requireUnlockedDevice=$requireUnlockedDevice)") + // 4. Re-encrypt the per-account `application` rows with the new key + // and write them back through the raw (ciphertext-in, ciphertext-out) + // update. Columns whose old-key decryption failed are written back + // verbatim, matching the `decryptField` failure contract in + // ApplicationEntityCrypto.kt. + for ((npub, rows) in decryptedAppRows) { + val dao = runCatching { Amber.instance.dao(npub) }.getOrNull() ?: continue + for (row in rows) { + runCatching { + dao.updateEncryptedColumnsRaw( + key = row.key, + secret = row.secretPlain?.let { encryptWithKey(newKey, it) } ?: row.secretRaw, + localKey = row.localKeyPlain?.let { encryptWithKey(newKey, it) } ?: row.localKeyRaw, + ) + }.onFailure { + AmberLog.w(TAG, "Key rotation: failed to re-encrypt application row", it) + } + } + } + + AmberLog.d(TAG, "Key rotation complete (requireUnlockedDevice=$requireUnlockedDevice)") } + + private fun String.decryptOrNullWith(key: SecretKey): String? = if (isEmpty()) { + null + } else { + try { + decryptWithKey(key, this) + } catch (_: Exception) { + null + } + } + + /** + * One `application`-table row staged during key rotation: [secretPlain] / + * [localKeyPlain] hold the old-key decryption result (or `null` when the + * ciphertext was unreadable — those rows keep [secretRaw]/[localKeyRaw] + * verbatim on write-back so no data is lost). + */ + private data class RotatedAppRow( + val key: String, + val secretPlain: String?, + val localKeyPlain: String?, + val secretRaw: String, + val localKeyRaw: String, + ) } fun Context.hasStrongBox(): Boolean { 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 deaeef99..b3bb2eac 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt @@ -239,6 +239,18 @@ interface ApplicationDao { @Transaction suspend fun updateNameAndIcon(key: String, name: String, icon: String) + /** + * Rewrites the two envelope-encrypted columns for one row. Key-rotation + * write path ([SecureCryptoHelper.rotateKey]): the values must already be + * ciphertext encrypted with the TARGET Keystore key — or the unchanged raw + * value when the old ciphertext could not be decrypted, so rotation never + * destroys data it cannot recover. Not wrapped by a public default method + * because callers deal exclusively in ciphertext (empty-sentinel included). + */ + @Query("UPDATE application SET secret = :secret, localKey = :localKey WHERE `key` = :key") + @Transaction + suspend fun updateEncryptedColumnsRaw(key: String, secret: String, localKey: String) + @Delete @Transaction suspend fun deletePermission(permission: ApplicationPermissionsEntity) 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 f9e96b2d..f5eb8c0e 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt @@ -257,6 +257,10 @@ class CachingApplicationDao( override suspend fun getAllWithLocalKeyRaw(pubKey: String): List = delegate.getAllWithLocalKeyRaw(pubKey) + // Key-rotation write path: secret/localKey never feed a cached read, so a + // plain delegation with no invalidation is correct. + override suspend fun updateEncryptedColumnsRaw(key: String, secret: String, localKey: String) = delegate.updateEncryptedColumnsRaw(key, secret, localKey) + override suspend fun insertApplicationRaw(event: ApplicationEntity): Long? = delegate.insertApplicationRaw(event) override fun insertAllRaw(events: List): List? = delegate.insertAllRaw(events)