laantungir/amethyst
1
0
Fork 0
mirror of https://github.com/vitorpamplona/amethyst.git synced 2026-10-05 19:28:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: audit fixes for the key backup and NIP-49 changes

Browse Source 
Backup screen (Android):
- The key-access gate lost its queued action on Android 8-10. There the
  device-credential activity always runs, stops the screen, and ON_STOP
  cleared the action. It was also lost when a wrong fingerprint (a soft
  failure) preceded the lockout PIN fallback. Now only a new request or the
  keyguard result replaces it.
- The nsec clipboard auto-clear ran in a composition scope, so leaving the
  screen within 60 s cancelled it and left the nsec on the clipboard. It now
  runs in the AccountViewModel scope.
- The copied nsec is flagged IS_SENSITIVE, so Android 13+ hides it in the
  system copy overlay, which is outside FLAG_SECURE.
- Password fields are disabled while scrypt runs, so the result always matches
  what is shown. Encrypt uses the ByteArray overload (no hex copy of the key).

NIP-49 (quartz):
- decrypt() rejects ciphertexts that are not 48 bytes. A shorter one with a
  valid tag decoded as a zero-padded key (test reproduces it).
- An OutOfMemoryError from scrypt, e.g. an ncryptsec made elsewhere at
  LOG_N 20 (1 GiB), becomes an IllegalStateException, so login reports it
  instead of crashing.
- The decrypted plaintext buffer inside LibSodiumInstance is wiped after
  copying out.

CLI: `amy login` and `amy key decrypt` accept hand-copied keys (UPPERCASE,
grouped), like the apps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012hhtLQhig6Wmt5U4C3owaP
This commit is contained in:
Claude
2026-09-26 22:29:47 +00:00
parent b3aebaa95b
commit 83ccbbd2a3
6 changed files with 79 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/keyBackup/AccountBackupScreen.kt
+31 -14
View File
@@ -24,6 +24,7 @@ import android.app.Activity
import android.content.ClipData
import android.content.Context
import android.content.ContextWrapper
import android.os.PersistableBundle
import android.view.WindowManager
import android.widget.Toast
import androidx.activity.compose.rememberLauncherForActivityResult
@@ -91,6 +92,7 @@ import androidx.compose.ui.window.DialogProperties
import androidx.fragment.app.FragmentActivity
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.compose.LifecycleEventEffect
import androidx.lifecycle.viewModelScope
import com.vitorpamplona.amethyst.commons.icons.symbols.Icon
import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbol
import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols
@@ -138,7 +140,6 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel
import com.vitorpamplona.amethyst.ui.screen.loggedIn.mockAccountViewModel
import com.vitorpamplona.amethyst.ui.screen.loggedIn.qrcode.BackButton
import com.vitorpamplona.amethyst.ui.screen.loggedIn.qrcode.QrCodeDrawer
import com.vitorpamplona.quartz.nip01Core.core.toHexKey
import com.vitorpamplona.quartz.nip19Bech32.toNsec
import com.vitorpamplona.quartz.nip49PrivKeyEnc.Nip49
import kotlinx.coroutines.CoroutineScope
@@ -217,7 +218,7 @@ private fun AccountBackupScreenContent(
} else {
val gate = rememberKeyAccessGate(accountViewModel)
SecretKeyCard(nsec, gate)
SecretKeyCard(nsec, gate, accountViewModel)
EncryptedKeyCard(accountViewModel, gate)
@@ -269,10 +270,10 @@ private fun BackupHeader() {
private fun SecretKeyCard(
nsec: String,
gate: KeyAccessGate,
accountViewModel: AccountViewModel,
) {
val context = LocalContext.current
val clipboard = LocalClipboard.current
val scope = rememberCoroutineScope()
var revealed by remember { mutableStateOf(false) }
var showQr by remember { mutableStateOf(false) }
@@ -337,7 +338,7 @@ private fun SecretKeyCard(
Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
FilledTonalButton(
modifier = Modifier.weight(1f),
onClick = { gate.withAccess { copyNSec(context, scope, nsec, clipboard) } },
onClick = { gate.withAccess { copyNSec(context, accountViewModel.viewModelScope, nsec, clipboard) } },
) {
ButtonContent(MaterialSymbols.ContentCopy, stringRes(Res.string.backup_keys_copy))
}
@@ -393,7 +394,8 @@ private fun EncryptedKeyCard(
var encrypted by remember { mutableStateOf<String?>(null) }
var showQr by remember { mutableStateOf(false) }
// Never leave the encrypted key or the typed password around while in the background.
// Drop the result while in the background. The typed password is kept so switching to a
// password manager to fetch it doesn't wipe the fields.
LifecycleEventEffect(Lifecycle.Event.ON_STOP) {
encrypted = null
showQr = false
@@ -414,7 +416,9 @@ private fun EncryptedKeyCard(
scope.launch {
val result =
withContext(Dispatchers.Default) {
runCatching { Nip49().encrypt(privKey.toHexKey(), currentPassword) }.getOrNull()
runCatching {
Nip49().encrypt(privKey, currentPassword, Nip49.DEFAULT_LOG_N, Nip49.EncryptedInfo.CLIENT_DOES_NOT_TRACK)
}.getOrNull()
}
working = false
if (result != null) {
@@ -471,6 +475,8 @@ private fun EncryptedKeyCard(
.semantics { contentType = ContentType.NewPassword },
value = password,
onValueChange = { password = it },
// What gets encrypted is the password at tap time: don't let it change underneath.
enabled = !working,
singleLine = true,
label = { Text(stringRes(Res.string.account_backup_encrypted_password)) },
supportingText = {
@@ -505,6 +511,7 @@ private fun EncryptedKeyCard(
.semantics { contentType = ContentType.NewPassword },
value = repeated,
onValueChange = { repeated = it },
enabled = !working,
singleLine = true,
label = { Text(stringRes(Res.string.account_backup_encrypted_repeat_password)) },
isError = mismatch,
@@ -667,7 +674,11 @@ private class KeyAccessGate {
var isUnlocked by mutableStateOf(false)
private set
/** The action waiting on the keyguard fallback activity to return. */
/**
* The action waiting on the keyguard fallback activity to return. Only a new request or
* that activity's result replaces it: the activity stops this screen (so ON_STOP must not
* clear it), and a wrong fingerprint before the lockout fallback is not a final failure.
*/
var pending: (() -> Unit)? = null
var prompt: ((onApproved: () -> Unit) -> Unit)? = null
@@ -685,7 +696,6 @@ private class KeyAccessGate {
fun lock() {
isUnlocked = false
pending = null
}
}
@@ -717,10 +727,7 @@ private fun rememberKeyAccessGate(accountViewModel: AccountViewModel): KeyAccess
gate.pending = null
onApproved()
},
onError = { title, message ->
gate.pending = null
accountViewModel.toastManager.toast(title, message)
},
onError = { title, message -> accountViewModel.toastManager.toast(title, message) },
)
}
}
@@ -745,8 +752,9 @@ private fun copyNSec(
nsec: String,
clipboardManager: Clipboard,
) {
// The auto-clear below must outlive this screen, so [scope] is not a composition scope.
scope.launch {
clipboardManager.setText(nsec)
clipboardManager.setClipEntry(ClipEntry(sensitiveClip(nsec)))
Toast
.makeText(
context,
@@ -756,7 +764,6 @@ private fun copyNSec(
// Best-effort auto-clear: after a delay, wipe the clipboard only if it
// still holds this exact nsec (don't clobber anything copied since).
// On Android 13+ the OS also shows its own sensitive-content UI.
delay(CLIPBOARD_CLEAR_DELAY_MS)
if (clipboardManager.getText() == nsec) {
clipboardManager.setClipEntry(ClipEntry(ClipData.newPlainText("", "")))
@@ -764,6 +771,16 @@ private fun copyNSec(
}
}
/**
* Marks the clip as sensitive so Android 13+ hides it in the copy confirmation overlay and
* clipboard previews. That overlay is a system window, outside this screen's FLAG_SECURE.
*/
private fun sensitiveClip(text: String): ClipData =
ClipData.newPlainText("", text).apply {
// ClipDescription.EXTRA_IS_SENSITIVE, spelled out: the constant is API 33, the key works earlier.
description.extras = PersistableBundle().apply { putBoolean("android.content.extra.IS_SENSITIVE", true) }
}
@Composable
private fun ShowKeyQRDialog(
qrCode: String,
cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/KeyCommands.kt
+2 -1
View File
@@ -25,6 +25,7 @@ import com.vitorpamplona.amethyst.cli.Identity
import com.vitorpamplona.amethyst.cli.Output
import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray
import com.vitorpamplona.quartz.nip01Core.core.toHexKey
import com.vitorpamplona.quartz.nip19Bech32.Bech32Transcription
import com.vitorpamplona.quartz.nip19Bech32.bech32.bechToBytes
import com.vitorpamplona.quartz.nip19Bech32.toNpub
import com.vitorpamplona.quartz.nip49PrivKeyEnc.Nip49
@@ -121,7 +122,7 @@ object KeyCommands {
private fun decrypt(rest: Array<String>): Int {
val args = Args(rest)
val ncryptsec = args.positional(0, "ncryptsec").trim()
val ncryptsec = Bech32Transcription.normalize(args.positional(0, "ncryptsec"))
if (!ncryptsec.startsWith("ncryptsec")) return Output.error("bad_args", "expected an ncryptsec1… string")
// Read both spellings eagerly so passing both doesn't trip rejectUnknown().
val pwAlias = args.flag("pw")
cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/LoginCommand.kt
+2 -1
View File
@@ -30,6 +30,7 @@ import com.vitorpamplona.quartz.nip05DnsIdentifiers.Nip05Client
import com.vitorpamplona.quartz.nip05DnsIdentifiers.OkHttpNip05Fetcher
import com.vitorpamplona.quartz.nip05DnsIdentifiers.resolveUserHexOrNull
import com.vitorpamplona.quartz.nip06KeyDerivation.Nip06
import com.vitorpamplona.quartz.nip19Bech32.Bech32Transcription
import com.vitorpamplona.quartz.nip19Bech32.toNpub
import com.vitorpamplona.quartz.nip46RemoteSigner.signer.NostrSignerRemote
import com.vitorpamplona.quartz.nip49PrivKeyEnc.Nip49
@@ -90,7 +91,7 @@ object LoginCommand {
args.rejectUnknown("password", "pw", "private")
val identity =
resolveIdentity(key, args)
resolveIdentity(Bech32Transcription.normalize(key), args)
?: return Output.error(
"bad_key",
"could not parse '$key' as any supported identifier",
quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip49PrivKeyEnc/Nip49.kt
+27 -3
View File
@@ -43,6 +43,9 @@ class Nip49 {
*/
const val MIN_PASSWORD_LENGTH = 12
/** scrypt cost used when creating: 2^16 rounds, 64 MiB. Safe on low-end phones. */
const val DEFAULT_LOG_N = 16
/**
* Length as scrypt sees it: code points of the NFKC-normalized password, so
* an emoji (a UTF-16 surrogate pair) counts once and compatibility forms
@@ -64,10 +67,13 @@ class Nip49 {
): String {
check(encryptedInfo != null) { "Couldn't decode key" }
check(encryptedInfo.version == EncryptedInfo.V) { "invalid version" }
// 32-byte key + 16-byte tag. A shorter payload can still carry a valid tag and
// would otherwise come back as a zero-padded "key".
check(encryptedInfo.encryptedKey.size == 48) { "invalid encrypted key length" }
val normalizedPassword = UnicodeNormalizer().normalizeNFKC(password).encodeToByteArray()
val n = 2.0.pow(encryptedInfo.logn.toDouble()).toInt()
val key = SCrypt.scrypt(normalizedPassword, encryptedInfo.salt, n, 8, 1, 32)
val key = deriveKey(normalizedPassword, encryptedInfo.salt, n, encryptedInfo.logn.toInt())
val m = ByteArray(32)
try {
@@ -97,7 +103,7 @@ class Nip49 {
fun encrypt(
secretKeyHex: String,
password: String,
logn: Int = 16,
logn: Int = DEFAULT_LOG_N,
ksb: Byte = EncryptedInfo.CLIENT_DOES_NOT_TRACK,
): String = encrypt(secretKeyHex.hexToByteArray(), password, logn, ksb)
@@ -113,7 +119,7 @@ class Nip49 {
val normalizedPassword = UnicodeNormalizer().normalizeNFKC(password).encodeToByteArray()
val n = 2.0.pow(logn.toDouble()).toInt()
val key = SCrypt.scrypt(normalizedPassword, salt, n, 8, 1, 32)
val key = deriveKey(normalizedPassword, salt, n, logn)
val ciphertext = ByteArray(48)
try {
@@ -149,6 +155,24 @@ class Nip49 {
).encodePayload()
}
/**
* scrypt allocates 128 * r * N bytes up front: 1 GiB at LOG_N 20, which NIP-49 allows and
* other clients may choose. Past the heap that is an OutOfMemoryError, which callers'
* `catch (e: Exception)` would miss, crashing instead of reporting the key as unusable.
*/
private fun deriveKey(
normalizedPassword: ByteArray,
salt: ByteArray,
n: Int,
logn: Int,
): ByteArray =
try {
SCrypt.scrypt(normalizedPassword, salt, n, 8, 1, 32)
} catch (e: Error) {
normalizedPassword.fill(0)
throw IllegalStateException("Not enough memory for this key's scrypt cost (LOG_N $logn)", e)
}
class EncryptedInfo(
val version: Byte,
val logn: Byte,
quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/LibSodiumInstance.kt
+2
View File
@@ -39,6 +39,8 @@ object LibSodiumInstance {
try {
val plaintext = XChaCha20Poly1305.decrypt(ciphertext, ad, nPub, k)
plaintext.copyInto(message)
// The caller owns the only copy it should have; don't leave another on the heap.
plaintext.fill(0)
true
} catch (_: Exception) {
false
quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip49PrivKeyEnc/Nip49SpecTest.kt
+15
View File
@@ -22,6 +22,7 @@ package com.vitorpamplona.quartz.nip49PrivKeyEnc
import com.vitorpamplona.quartz.nip19Bech32.Bech32Transcription
import com.vitorpamplona.quartz.nip19Bech32.bech32.bechToBytes
import com.vitorpamplona.quartz.nip44Encryption.crypto.XChaCha20Poly1305
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
@@ -117,4 +118,18 @@ class Nip49SpecTest {
// The spec's normalization vector: 4 code points typed, 3 after NFKC.
assertEquals(3, Nip49.passwordLength("\u212B\u2126\u1E9B\u0323"))
}
@Test
fun authenticatedButShortCiphertextIsRejected() {
// A tag-valid payload that encrypts only 16 bytes must not decode as a zero-padded key.
val password = "nostr"
val salt = ByteArray(16) { it.toByte() }
val nonce = ByteArray(24) { (it + 1).toByte() }
val ksb = Nip49.EncryptedInfo.CLIENT_DOES_NOT_TRACK
val key = SCrypt.scrypt(password.encodeToByteArray(), salt, 2, 8, 1, 32)
val shortCiphertext = XChaCha20Poly1305.encrypt(ByteArray(16) { 7 }, byteArrayOf(ksb), nonce, key)
val info = Nip49.EncryptedInfo(Nip49.EncryptedInfo.V, 1, salt, nonce, ksb, shortCiphertext)
assertFailsWith<IllegalStateException> { nip49.decrypt(info, password) }
}
}