diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e072d2a6..282b7910 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -37,12 +37,24 @@ jobs: - name: Linter (gradle) run: ./gradlew ktlintCheck + - name: Android lint (gradle) + run: ./gradlew lint --no-daemon + - name: Test (gradle) run: ./gradlew test --no-daemon + - name: Python security regressions + run: | + if [ -d tests ]; then + python3 -m unittest discover -s tests -p 'test_*.py' -v + fi + - name: Build APK (gradle) run: ./gradlew assembleDebug --no-daemon + - name: Build free release APK (gradle) + run: ./gradlew :app:assembleFreeRelease --no-daemon + - name: Upload APK uses: actions/upload-artifact@v7 with: diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/models/Account.kt b/app/src/main/java/com/greenart7c3/nostrsigner/models/Account.kt index 29dbb401..c61d59b0 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/models/Account.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/models/Account.kt @@ -1,5 +1,6 @@ package com.greenart7c3.nostrsigner.models +import android.app.KeyguardManager import android.widget.Toast import androidx.compose.runtime.Immutable import androidx.compose.runtime.Stable @@ -7,6 +8,7 @@ import androidx.compose.ui.platform.Clipboard import com.greenart7c3.nostrsigner.Amber import com.greenart7c3.nostrsigner.DataStoreAccess import com.greenart7c3.nostrsigner.R +import com.greenart7c3.nostrsigner.service.DeviceUnlockPolicy import com.greenart7c3.nostrsigner.service.nip44v3.Nip44v3 import com.greenart7c3.nostrsigner.ui.setSensitiveClip import com.vitorpamplona.quartz.nip01Core.core.Event @@ -42,6 +44,16 @@ class Account( // Watches name/picture to mark the account saveable; injectable so Compose // previews can build an Account without the Amber Application singleton. scope: CoroutineScope = Amber.instance.applicationIOScope, + private val keyAccessCheck: () -> Unit = { + // Until settings load, requireUnlockedDevice's default (false) is a guess, not a + // decision — the same pre-load window WebDavService fails closed on. + check(Amber.instance.isSettingsLoaded) { "Settings are not loaded yet" } + val required = Amber.instance.settings.requireUnlockedDevice + if (required) { + val keyguard = Amber.instance.getSystemService(KeyguardManager::class.java) + DeviceUnlockPolicy.requireAccess(required, keyguard?.isDeviceLocked ?: true) + } + }, ) { var signPolicy: Int = signPolicy set(value) { @@ -70,38 +82,77 @@ class Account( } } - suspend fun sign(eventTemplate: EventTemplate): T = signer.sign(eventTemplate) + internal fun requirePrivateKeyAccess() = keyAccessCheck() + + suspend fun sign(eventTemplate: EventTemplate): T { + requirePrivateKeyAccess() + return signer.sign(eventTemplate) + } fun signSync( createdAt: Long, kind: Int, tags: Array>, content: String, - ): T = signer.signerSync.sign(createdAt, kind, tags, content) + ): T { + requirePrivateKeyAccess() + return signer.signerSync.sign(createdAt, kind, tags, content) + } - fun nip49Encrypt(password: String): String = Nip49().encrypt(signer.keyPair.privKey!!.toHexKey(), password) + fun nip49Encrypt(password: String): String { + requirePrivateKeyAccess() + return Nip49().encrypt(signer.keyPair.privKey!!.toHexKey(), password) + } - suspend fun nip44Encrypt(plainText: String, toPublicKey: String): String = signer.nip44Encrypt(plainText, toPublicKey) + suspend fun nip44Encrypt(plainText: String, toPublicKey: String): String { + requirePrivateKeyAccess() + return signer.nip44Encrypt(plainText, toPublicKey) + } - suspend fun nip04Encrypt(plainText: String, toPublicKey: String): String = signer.nip04Encrypt(plainText, toPublicKey) + suspend fun nip04Encrypt(plainText: String, toPublicKey: String): String { + requirePrivateKeyAccess() + return signer.nip04Encrypt(plainText, toPublicKey) + } - suspend fun nip44Decrypt(cipherText: String, fromPublicKey: String): String = signer.nip44Decrypt(cipherText, fromPublicKey) + suspend fun nip44Decrypt(cipherText: String, fromPublicKey: String): String { + requirePrivateKeyAccess() + return signer.nip44Decrypt(cipherText, fromPublicKey) + } - suspend fun nip04Decrypt(cipherText: String, fromPublicKey: String): String = signer.nip04Decrypt(cipherText, fromPublicKey) + suspend fun nip04Decrypt(cipherText: String, fromPublicKey: String): String { + requirePrivateKeyAccess() + return signer.nip04Decrypt(cipherText, fromPublicKey) + } - fun nip44v3Encrypt(plainText: ByteArray, toPublicKey: String, kind: Int, scope: String): String = Nip44v3.encrypt(plainText, signer.keyPair.privKey!!, toPublicKey.hexToByteArray(), kind, scope) + fun nip44v3Encrypt(plainText: ByteArray, toPublicKey: String, kind: Int, scope: String): String { + requirePrivateKeyAccess() + return Nip44v3.encrypt(plainText, signer.keyPair.privKey!!, toPublicKey.hexToByteArray(), kind, scope) + } - fun nip44v3Decrypt(cipherText: String, fromPublicKey: String, kind: Int, scope: String): ByteArray = Nip44v3.decrypt(cipherText, signer.keyPair.privKey!!, fromPublicKey.hexToByteArray(), kind, scope) + fun nip44v3Decrypt(cipherText: String, fromPublicKey: String, kind: Int, scope: String): ByteArray { + requirePrivateKeyAccess() + return Nip44v3.decrypt(cipherText, signer.keyPair.privKey!!, fromPublicKey.hexToByteArray(), kind, scope) + } - suspend fun decrypt(encryptedContent: String, fromPublicKey: String): String = signer.decrypt(encryptedContent, fromPublicKey) + suspend fun decrypt(encryptedContent: String, fromPublicKey: String): String { + requirePrivateKeyAccess() + return signer.decrypt(encryptedContent, fromPublicKey) + } - suspend fun signPsbt(psbtHex: String): String = signer.signPsbt(psbtHex) + suspend fun signPsbt(psbtHex: String): String { + requirePrivateKeyAccess() + return signer.signPsbt(psbtHex) + } - suspend fun seedWords() = runCatching { DataStoreAccess.getEncryptedKey(Amber.instance, npub, DataStoreAccess.SEED_WORDS) }.getOrNull() ?: "" + suspend fun seedWords(): String { + requirePrivateKeyAccess() + return runCatching { DataStoreAccess.getEncryptedKey(Amber.instance, npub, DataStoreAccess.SEED_WORDS) }.getOrNull() ?: "" + } fun decryptZapEvent( data: String, ): String? { + requirePrivateKeyAccess() val event = Event.fromJson(data) as LnZapRequestEvent return try { PrivateZapRequestBuilder().decryptZapEvent( @@ -114,9 +165,15 @@ class Account( } } - suspend fun createMessageNIP17(template: EventTemplate): NIP17Factory.Result = NIP17Factory().createMessageNIP17(template, signer) + suspend fun createMessageNIP17(template: EventTemplate): NIP17Factory.Result { + requirePrivateKeyAccess() + return NIP17Factory().createMessageNIP17(template, signer) + } - suspend fun getNsec(): String = signer.keyPair.privKey!!.toNsec() + suspend fun getNsec(): String { + requirePrivateKeyAccess() + return signer.keyPair.privKey!!.toNsec() + } fun copyToClipboard(clipboardManager: Clipboard) { Amber.instance.applicationIOScope.launch { diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/ApplicationBackup.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/ApplicationBackup.kt index ca459bf8..8b62229e 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/ApplicationBackup.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/ApplicationBackup.kt @@ -115,6 +115,7 @@ internal fun backupKeyPair(identityPrivKey: ByteArray): KeyPair { } internal fun backupSigner(account: Account): NostrSignerInternal { + account.requirePrivateKeyAccess() val identityPrivKey = checkNotNull(account.signer.keyPair.privKey) { "ApplicationBackup: account has no private key" } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/DeviceUnlockPolicy.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/DeviceUnlockPolicy.kt new file mode 100644 index 00000000..b800383e --- /dev/null +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/DeviceUnlockPolicy.kt @@ -0,0 +1,9 @@ +package com.greenart7c3.nostrsigner.service + +object DeviceUnlockPolicy { + fun requireAccess(requireUnlockedDevice: Boolean, isDeviceLocked: Boolean) { + if (requireUnlockedDevice && isDeviceLocked) { + throw SecurityException("Private-key operations require an unlocked device") + } + } +} diff --git a/app/src/test/java/com/greenart7c3/nostrsigner/models/AccountCachedKeyLockTest.kt b/app/src/test/java/com/greenart7c3/nostrsigner/models/AccountCachedKeyLockTest.kt new file mode 100644 index 00000000..bdbcbbc3 --- /dev/null +++ b/app/src/test/java/com/greenart7c3/nostrsigner/models/AccountCachedKeyLockTest.kt @@ -0,0 +1,52 @@ +package com.greenart7c3.nostrsigner.models + +import com.greenart7c3.nostrsigner.service.DeviceUnlockPolicy +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import io.mockk.Called +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertThrows +import org.junit.Test + +class AccountCachedKeyLockTest { + @Test + fun `cached account cannot sign or decrypt after device lock`() { + val scope = CoroutineScope(SupervisorJob() + Dispatchers.Unconfined) + val signer = mockk() + var locked = false + val account = Account( + signer = signer, + hexKey = "owner", + npub = "npub-owner", + name = MutableStateFlow(""), + picture = MutableStateFlow(""), + signPolicy = 1, + didBackup = false, + scope = scope, + keyAccessCheck = { DeviceUnlockPolicy.requireAccess(true, locked) }, + ) + try { + account.requirePrivateKeyAccess() + locked = true + assertThrows(SecurityException::class.java) { + account.signSync(0, 1, emptyArray(), "synthetic-event") + } + assertThrows(SecurityException::class.java) { + runBlocking { account.nip44Decrypt("synthetic-ciphertext", "peer") } + } + assertThrows(SecurityException::class.java) { + runBlocking { account.seedWords() } + } + verify { signer wasNot Called } + } finally { + scope.cancel() + } + } +} diff --git a/app/src/test/java/com/greenart7c3/nostrsigner/service/ApplicationBackupCryptoTest.kt b/app/src/test/java/com/greenart7c3/nostrsigner/service/ApplicationBackupCryptoTest.kt index f2eb76e9..3c633300 100644 --- a/app/src/test/java/com/greenart7c3/nostrsigner/service/ApplicationBackupCryptoTest.kt +++ b/app/src/test/java/com/greenart7c3/nostrsigner/service/ApplicationBackupCryptoTest.kt @@ -45,6 +45,7 @@ class ApplicationBackupCryptoTest { signPolicy = 1, didBackup = true, scope = scope, + keyAccessCheck = {}, ) } diff --git a/app/src/test/java/com/greenart7c3/nostrsigner/service/DeviceUnlockPolicyTest.kt b/app/src/test/java/com/greenart7c3/nostrsigner/service/DeviceUnlockPolicyTest.kt new file mode 100644 index 00000000..addfd501 --- /dev/null +++ b/app/src/test/java/com/greenart7c3/nostrsigner/service/DeviceUnlockPolicyTest.kt @@ -0,0 +1,19 @@ +package com.greenart7c3.nostrsigner.service + +import org.junit.Assert.assertThrows +import org.junit.Test + +class DeviceUnlockPolicyTest { + @Test + fun `opt in blocks operations after the device locks`() { + DeviceUnlockPolicy.requireAccess(true, false) + assertThrows(SecurityException::class.java) { DeviceUnlockPolicy.requireAccess(true, true) } + DeviceUnlockPolicy.requireAccess(true, false) + } + + @Test + fun `default policy preserves authorized background operation`() { + DeviceUnlockPolicy.requireAccess(false, true) + DeviceUnlockPolicy.requireAccess(false, false) + } +} diff --git a/app/src/test/java/com/greenart7c3/nostrsigner/service/SubscriptionTestUtils.kt b/app/src/test/java/com/greenart7c3/nostrsigner/service/SubscriptionTestUtils.kt index 5148ed56..80cb252c 100644 --- a/app/src/test/java/com/greenart7c3/nostrsigner/service/SubscriptionTestUtils.kt +++ b/app/src/test/java/com/greenart7c3/nostrsigner/service/SubscriptionTestUtils.kt @@ -38,5 +38,6 @@ fun newTestAccount(scope: CoroutineScope): Account { signPolicy = 0, didBackup = false, scope = scope, + keyAccessCheck = {}, ) }