From c574e2994f41188c70dfc9e19f448b89a312ed84 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Mon, 31 Aug 2026 15:59:34 -0700 Subject: [PATCH 1/8] fix(kotlin-sdk): degrade mnemonic storage off lock binding on false-locked devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some OEM builds (HONOR/MagicOS Android 16 in the field; same mechanism as Google Issue Tracker 506989112 on Fairphone) perform unlocks that never satisfy the Keystore's UNLOCKED_DEVICE_REQUIRED gate, so the lock-bound master-alias key stays denied for the whole unlock session while KeyguardManager reports the device unlocked. storeMnemonic's bounded false-locked retry (built for the transient Keystore2 blip) can never outwait that, so wallet creation was unfixably failing on those devices. Add a last rung to the ladder: when the retry schedule exhausts still false-locked, treat the device's UNLOCKED_DEVICE_REQUIRED implementation as defective and store under a new never-lock-bound alias (MASTER_ALIAS_UNBOUND — same hardware-backed non-auth AES-256-GCM, no setUnlockedDeviceRequired ever), recording the defect durably in the same atomic edit. From then on mnemonic writes go straight to the unbound alias, the createWallet preflight stops probing, reads route by the blob's recorded alias (mnemonicalias., the privkeyalias discipline), and pre-existing lock-bound blobs are re-wrapped best-effort on their first successful read. Nothing is ever deleted or re-keyed, genuinely-locked denials keep failing fast, the auth-gated identity aliases are untouched, and healthy devices never provision the new alias — this is the dashpay/platform#4060 no-lock-screen downgrade driven by operational evidence instead of a missing lock screen. Co-Authored-By: Claude Fable 5 --- .../security/KeystoreDeviceLockedException.kt | 11 +- .../dashsdk/security/KeystoreManager.kt | 40 +++ .../dashsdk/security/WalletStorage.kt | 250 +++++++++++++++-- .../dashsdk/wallet/PlatformWalletManager.kt | 12 +- .../WalletStorageDeviceLockedRetryTest.kt | 255 +++++++++++++++--- 5 files changed, 500 insertions(+), 68 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt index d7df7d46939..97c233a7b44 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt @@ -46,8 +46,15 @@ data class DeviceLockState( * observed in the field (two QA devices, wallet creation) — the device is * demonstrably unlocked but Keystore2's internal lock-state tracking * still says "locked". A short bounded retry is worthwhile (see - * [WalletStorage.storeMnemonic]); persistent recurrence points at the - * platform bug, not at this SDK or its keys. + * [WalletStorage.storeMnemonic]); recurrence past that schedule means + * the defect is PERSISTENT for the unlock session (an OEM unlock class + * that never satisfies `UNLOCKED_DEVICE_REQUIRED` — observed on + * HONOR/MagicOS Android 16; same mechanism as Google Issue Tracker + * 506989112), at which point [WalletStorage.storeMnemonic] degrades the + * write to the never-lock-bound + * [KeystoreManager.MASTER_ALIAS_UNBOUND] instead of throwing this — so + * escaping this exception false-locked now means even that degradation + * failed (see the suppressed exception). * * NOT used for the auth-gated identity-key aliases: their * `UserNotAuthenticatedException` means "auth window closed" and keeps its diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt index ef99c57f884..2bc27d41ad5 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt @@ -34,6 +34,15 @@ import javax.crypto.spec.PSource * - [MASTER_ALIAS] `org.dashfoundation.wallet.master` — mnemonics and * general wallet secrets, under a non-auth AES-256-GCM key (name parity * with the iOS keychain service `org.dashfoundation.wallet`). + * - [MASTER_ALIAS_UNBOUND] `org.dashfoundation.wallet.master.unbound` — + * the same non-auth AES-256-GCM parameters as [MASTER_ALIAS] but + * guaranteed to NEVER carry `setUnlockedDeviceRequired`. The degradation + * target [WalletStorage] moves mnemonic blobs to on devices whose + * Keystore denies lock-bound operations while `KeyguardManager` reports + * the device unlocked (the persistent false-locked defect — an OEM + * unlock that never satisfies `UNLOCKED_DEVICE_REQUIRED`; see + * [KeystoreDeviceLockedException]). Provisioned lazily on first use, + * only ever on a device that demonstrated the defect. * - [KEYS_ALIAS_AUTH_GATED] `org.dashfoundation.wallet.keys.authgated` — * identity private keys under the default [KeySecurityPolicy.AUTH_GATED], * wrapped by an RSA-2048 OAEP(SHA-256) keypair. The PUBLIC key encrypts and @@ -640,6 +649,19 @@ open class KeystoreManager( KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEYSTORE, ) + // MASTER_ALIAS_UNBOUND's whole contract is the ABSENCE of lock + // binding — it exists only as the false-locked degradation target — + // so it never enters the lock-screen ladder: the params are dropped + // unconditionally, not probed. StrongBox→TEE fallback still applies. + if (alias == MASTER_ALIAS_UNBOUND) { + return try { + generator.init(spec(strongBox = true, lockBound = false)) + generator.generateKey() + } catch (_: StrongBoxUnavailableException) { + generator.init(spec(strongBox = false, lockBound = false)) + generator.generateKey() + } + } return generateWithLockScreenDegradation(alias) { strongBox, lockBound -> generator.init(spec(strongBox, lockBound)) generator.generateKey() @@ -873,6 +895,24 @@ open class KeystoreManager( companion object { const val MASTER_ALIAS = "org.dashfoundation.wallet.master" + /** + * Never-lock-bound variant of [MASTER_ALIAS]: identical non-auth + * AES-256-GCM parameters, but `setUnlockedDeviceRequired` is never + * applied at generation regardless of the lock-screen probe (see + * [generateAesKey]). [WalletStorage] writes mnemonic blobs under + * this alias INSTEAD of [MASTER_ALIAS] once a device has + * demonstrated the persistent false-locked Keystore defect — the + * Keystore denying a lock-bound operation while `KeyguardManager` + * reports the device unlocked, past the bounded retry (an OEM + * unlock class that never satisfies `UNLOCKED_DEVICE_REQUIRED`; + * Google Issue Tracker 506989112). The same downgrade + * [generateWithLockScreenDegradation] already performs for lockless + * devices (dashpay/platform#4060), here triggered by operational + * evidence instead of a missing lock screen. Healthy devices never + * provision this alias. + */ + const val MASTER_ALIAS_UNBOUND = "org.dashfoundation.wallet.master.unbound" + /** * **Legacy** identity-keys alias. Across the SDK's history this single * alias has held, in turn, two now-superseded wrapping keys, so on an diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt index 2a5f8b95235..26f4495b862 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt @@ -7,6 +7,7 @@ import android.security.keystore.UserNotAuthenticatedException import android.util.Log import androidx.datastore.core.DataStore import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.booleanPreferencesKey import androidx.datastore.preferences.core.edit import androidx.datastore.preferences.core.stringPreferencesKey import androidx.datastore.preferences.core.stringSetPreferencesKey @@ -31,7 +32,11 @@ private val Context.secretsStore: DataStore by preferencesDataStore * Values are ciphertext under [KeystoreManager]'s non-exportable Keystore * keys, stored base64 in a dedicated Preferences DataStore. * Key layout mirrors the iOS account naming: - * - `mnemonic.` — wallet mnemonics (master alias, AES-GCM) + * - `mnemonic.` — wallet mnemonics (master alias, AES-GCM; + * the producing alias is recorded per blob in + * `mnemonicalias.` once the false-locked degradation has + * moved writes to [KeystoreManager.MASTER_ALIAS_UNBOUND] — see + * [storeMnemonic]) * - `privkey.` — identity private keys (the [keystore]'s * [KeystoreManager.keysAlias]: RSA public-key encrypt / private-key * decrypt that is auth-gated or not per the keystore's @@ -235,11 +240,25 @@ class WalletStorage( * re-parameterizes anything — on a fresh install the probe provisions * the master key exactly as the first [storeMnemonic] would have. A * no-op — no Keystore access at all — when the device is unlocked - * (including keyguard-showing-but-not-secured states). + * (including keyguard-showing-but-not-secured states), and likewise + * once the false-locked defect is on record for this device + * ([isMasterKeyLockBindingDefectObserved]): from then on + * [storeMnemonic] writes under the never-lock-bound + * [MASTER_ALIAS_UNBOUND][KeystoreManager.MASTER_ALIAS_UNBOUND], which + * no lock state can deny, so there is nothing to preflight. */ - fun ensureMasterKeyNotLockBlocked(operation: String) { + suspend fun ensureMasterKeyNotLockBlocked(operation: String) { val state = keystore.sampleDeviceLockState() if (!state.isDeviceLocked) return + if (isMasterKeyLockBindingDefectObserved()) { + Log.i( + TAG, + "$operation: device is locked but this device's false-locked defect is on " + + "record — mnemonic writes target the never-lock-bound master alias, " + + "which no lock state can deny; proceeding", + ) + return + } try { keystore.encrypt(ByteArray(1)) Log.i( @@ -269,18 +288,43 @@ class WalletStorage( /** * Encrypt and persist the mnemonic under the - * [MASTER_ALIAS][KeystoreManager.MASTER_ALIAS] AES key. + * [MASTER_ALIAS][KeystoreManager.MASTER_ALIAS] AES key — or under the + * never-lock-bound + * [MASTER_ALIAS_UNBOUND][KeystoreManager.MASTER_ALIAS_UNBOUND] once + * this device has demonstrated the PERSISTENT false-locked Keystore + * defect (see below); the producing alias is recorded per blob + * (`mnemonicalias.`, same atomic edit) so reads decrypt + * under whichever alias actually wrote it. * - * Retries the FALSE-LOCKED Keystore denial only: when the encrypt is - * denied as device-locked but the sampled `KeyguardManager` state says - * the device is NOT actually locked ([KeystoreDeviceLockedException] - * with `deviceReportsLocked == false` — the Keystore2 lock-state - * misreporting defect, hit on two QA devices during wallet creation), - * the store is retried up to 3 times over ~2s (the - * [DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS] backoff schedule) before the - * exception propagates. A GENUINELY locked - * device (`deviceReportsLocked == true`) fails fast with no retry — - * waiting 2s cannot unlock a phone; the caller retries after unlock. + * Device-locked denial handling, in escalation order: + * - A GENUINELY locked device ([KeystoreDeviceLockedException] with + * `deviceReportsLocked == true`) fails fast with no retry — waiting + * 2s cannot unlock a phone; the caller retries after unlock. + * - A FALSE-LOCKED denial (Keystore denies as device-locked while the + * sampled `KeyguardManager` state says unlocked) is retried up to 3 + * times over ~2s ([DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS]) — enough + * for the transient Keystore2 misreporting first seen on two QA + * devices. + * - When the schedule exhausts still false-locked, the defect is + * persistent, not transient — an OEM unlock class that never + * satisfies `UNLOCKED_DEVICE_REQUIRED`, so no in-session retry can + * ever succeed (observed on HONOR/MagicOS Android 16; same + * mechanism as Google Issue Tracker 506989112). The store then + * DEGRADES instead of failing: the blob is encrypted under + * [MASTER_ALIAS_UNBOUND][KeystoreManager.MASTER_ALIAS_UNBOUND] and + * the defect durably recorded ([MASTER_LOCK_DEFECT_KEY], in the + * same atomic edit), after which every mnemonic write on this + * device goes straight to the unbound alias and + * [ensureMasterKeyNotLockBlocked] stops preflighting. This is the + * dashpay/platform#4060 no-lock-screen downgrade — hardware-backed + * AES, no lock binding — triggered by operational evidence instead + * of a missing lock screen, and only ever on the defective device. + * Nothing is deleted or re-keyed: existing [MASTER_ALIAS] blobs + * stay decryptable under their recorded alias and are re-wrapped + * opportunistically on their next successful read (see + * [retrieveMnemonicUtf8]). If the unbound encrypt itself fails, the + * original typed denial propagates with the heal failure attached + * as suppressed, and nothing is recorded. */ suspend fun storeMnemonic(walletId: ByteArray, mnemonic: String) { // The plaintext copy lives across the whole backoff schedule, so scrub @@ -289,17 +333,25 @@ class WalletStorage( // handling of its other raw secret arrays. val plaintext = mnemonic.encodeToByteArray() try { + if (isMasterKeyLockBindingDefectObserved()) { + storeMnemonicUnbound(walletId, plaintext) + return + } var attempt = 0 while (true) { try { val blob = keystore.encrypt(plaintext) - store.edit { it[mnemonicKey(walletId)] = encode(blob) } + store.edit { + it[mnemonicKey(walletId)] = encode(blob) + // A MASTER_ALIAS blob is the untagged default. + it.remove(mnemonicAliasKey(walletId)) + } return } catch (e: KeystoreDeviceLockedException) { - if (e.deviceReportsLocked || - attempt >= DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS.size - ) { - throw e + if (e.deviceReportsLocked) throw e + if (attempt >= DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS.size) { + healFalseLockedMnemonicStore(walletId, plaintext, e) + return } val delayMs = DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS[attempt] attempt++ @@ -319,6 +371,77 @@ class WalletStorage( } } + /** + * Whether THIS device has demonstrated the persistent false-locked + * Keystore defect — a lock-bound master-alias operation denied as + * device-locked past [storeMnemonic]'s full bounded retry while + * `KeyguardManager` reported the device unlocked. Recorded durably by + * the heal path in the same atomic edit as the first unbound-alias + * blob; never cleared (the defect is a property of the device's OS + * build, not of any wallet — and a healed device staying healed costs + * nothing on a healthy one, which never sets it). Host-legible so apps + * can surface the degraded protection level in telemetry/support + * flows, the [KeystoreManager.effectiveKeySecurityPolicy] discipline. + */ + suspend fun isMasterKeyLockBindingDefectObserved(): Boolean = + store.data.first()[MASTER_LOCK_DEFECT_KEY] == true + + /** + * Write [plaintext]'s blob under the never-lock-bound + * [MASTER_ALIAS_UNBOUND][KeystoreManager.MASTER_ALIAS_UNBOUND], tagging + * the blob with its producing alias in the same atomic edit. The write + * path once the defect is on record. Does not scrub [plaintext] — the + * caller owns the buffer. + */ + private suspend fun storeMnemonicUnbound(walletId: ByteArray, plaintext: ByteArray) { + val blob = keystore.encrypt(plaintext, KeystoreManager.MASTER_ALIAS_UNBOUND) + store.edit { + it[mnemonicKey(walletId)] = encode(blob) + it[mnemonicAliasKey(walletId)] = KeystoreManager.MASTER_ALIAS_UNBOUND + } + } + + /** + * [storeMnemonic]'s last rung: the false-locked retry schedule + * exhausted, so the device's `UNLOCKED_DEVICE_REQUIRED` implementation + * is treated as defective — store under the never-lock-bound alias and + * record the defect ([MASTER_LOCK_DEFECT_KEY]) atomically with the + * blob, so a crash between them cannot record a defect with no healed + * blob or vice versa. A failure of the unbound encrypt itself rethrows + * the original typed [denial] (still the truthful signal — retryable + * after a credential unlock) with the heal failure suppressed, and + * records nothing. + */ + private suspend fun healFalseLockedMnemonicStore( + walletId: ByteArray, + plaintext: ByteArray, + denial: KeystoreDeviceLockedException, + ) { + Log.w( + TAG, + "storeMnemonic: Keystore still denied the lock-bound master-alias encrypt as " + + "device-locked after the full false-locked retry schedule, with " + + "KeyguardManager reporting UNLOCKED (${denial.lockState}) — treating this " + + "device's UNLOCKED_DEVICE_REQUIRED implementation as defective and " + + "degrading mnemonic storage to the never-lock-bound " + + "'${KeystoreManager.MASTER_ALIAS_UNBOUND}' (the dashpay/platform#4060 " + + "downgrade, driven by operational evidence; cf. Google Issue Tracker " + + "506989112)", + denial, + ) + val blob = try { + keystore.encrypt(plaintext, KeystoreManager.MASTER_ALIAS_UNBOUND) + } catch (healError: Exception) { + denial.addSuppressed(healError) + throw denial + } + store.edit { + it[mnemonicKey(walletId)] = encode(blob) + it[mnemonicAliasKey(walletId)] = KeystoreManager.MASTER_ALIAS_UNBOUND + it[MASTER_LOCK_DEFECT_KEY] = true + } + } + /** * Decrypt the mnemonic as a display `String`. For explicit * user-facing reveal flows ONLY (seed backup, biometric reveal) — @@ -326,8 +449,7 @@ class WalletStorage( * (the FFI resolver, signers) must use [retrieveMnemonicUtf8]. */ suspend fun retrieveMnemonic(walletId: ByteArray): String? { - val encoded = store.data.first()[mnemonicKey(walletId)] ?: return null - val plain = keystore.decrypt(decode(encoded)) + val plain = retrieveMnemonicUtf8(walletId) ?: return null val phrase = plain.decodeToString() plain.fill(0) return phrase @@ -340,10 +462,61 @@ class WalletStorage( * bytes are consumed — unlike a String, a ByteArray can actually be * scrubbed, so the plaintext exposure window is bounded by the call * instead of by the garbage collector. + * + * Decrypts under the blob's RECORDED alias — the lock-bound + * [MASTER_ALIAS][KeystoreManager.MASTER_ALIAS] default, or + * [MASTER_ALIAS_UNBOUND][KeystoreManager.MASTER_ALIAS_UNBOUND] for a + * blob written after this device's false-locked degradation (see + * [storeMnemonic]). On a device with the defect on record, a + * successful read of a still-lock-bound blob also re-wraps it under + * the unbound alias (best-effort, see [rewrapMnemonicUnbound]) so it + * stops being hostage to the defective `UNLOCKED_DEVICE_REQUIRED` + * gate. */ suspend fun retrieveMnemonicUtf8(walletId: ByteArray): ByteArray? { - val encoded = store.data.first()[mnemonicKey(walletId)] ?: return null - return keystore.decrypt(decode(encoded)) + val prefs = store.data.first() + val encoded = prefs[mnemonicKey(walletId)] ?: return null + val alias = prefs[mnemonicAliasKey(walletId)] ?: KeystoreManager.MASTER_ALIAS + val plain = keystore.decrypt(decode(encoded), alias) + if (alias == KeystoreManager.MASTER_ALIAS && prefs[MASTER_LOCK_DEFECT_KEY] == true) { + rewrapMnemonicUnbound(walletId, plain) + } + return plain + } + + /** + * Opportunistic re-wrap for wallets that predate the false-locked + * degradation on a defective device: their blobs still live under the + * lock-bound [MASTER_ALIAS][KeystoreManager.MASTER_ALIAS], which this + * device's Keystore denies for stretches of every unlock session, so + * the first read that DOES get through (e.g. after a credential + * unlock) moves the blob to the never-lock-bound alias — after which + * it is always readable. Best-effort by design: any failure leaves the + * original blob and its key fully intact (decryptable exactly as often + * as before) and the next successful read simply tries again. Never + * scrubs [plain] — the caller owns that buffer. + */ + private suspend fun rewrapMnemonicUnbound(walletId: ByteArray, plain: ByteArray) { + try { + storeMnemonicUnbound(walletId, plain) + Log.i( + TAG, + "re-wrapped a lock-bound master-alias mnemonic blob under the " + + "never-lock-bound '${KeystoreManager.MASTER_ALIAS_UNBOUND}' (this " + + "device's false-locked defect is on record) — future reads no longer " + + "depend on the defective UNLOCKED_DEVICE_REQUIRED gate", + ) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Log.w( + TAG, + "best-effort re-wrap under '${KeystoreManager.MASTER_ALIAS_UNBOUND}' " + + "failed; the blob remains under the lock-bound master alias and the " + + "next successful read will retry", + e, + ) + } } /** @@ -354,7 +527,10 @@ class WalletStorage( store.data.first().contains(mnemonicKey(walletId)) suspend fun deleteMnemonic(walletId: ByteArray) { - store.edit { it.remove(mnemonicKey(walletId)) } + store.edit { + it.remove(mnemonicKey(walletId)) + it.remove(mnemonicAliasKey(walletId)) + } } /** Wallet ids (hex) that have a stored mnemonic — drives orphan detection. */ @@ -1038,6 +1214,9 @@ class WalletStorage( private fun mnemonicKey(walletId: ByteArray) = stringPreferencesKey(MNEMONIC_PREFIX + walletId.toHex()) + private fun mnemonicAliasKey(walletId: ByteArray) = + stringPreferencesKey(MNEMONIC_ALIAS_PREFIX + walletId.toHex()) + private fun privateKeyKey(pubkeyHex: String) = stringPreferencesKey(PRIVKEY_PREFIX + pubkeyHex.lowercase()) @@ -1058,6 +1237,18 @@ class WalletStorage( private companion object { const val MNEMONIC_PREFIX = "mnemonic." + + /** + * Per-wallet record of the AES alias that produced the mnemonic + * blob (`mnemonicalias.`), written atomically with the + * blob. Routes reads to the exact producing alias after the + * false-locked degradation moves writes to + * [KeystoreManager.MASTER_ALIAS_UNBOUND]; a missing tag means the + * lock-bound [KeystoreManager.MASTER_ALIAS] (every blob written + * before the tag existed). The `privkeyalias.` discipline, applied + * to mnemonics. + */ + const val MNEMONIC_ALIAS_PREFIX = "mnemonicalias." const val PRIVKEY_PREFIX = "privkey." /** Per-alias [KeystoreManager.keysAliasFingerprint] snapshot, taken at write time. */ @@ -1126,6 +1317,17 @@ class WalletStorage( */ internal val DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS = longArrayOf(250, 750, 1000) + /** + * Durable device-scoped record that the false-locked Keystore + * defect was demonstrated here — a lock-bound master-alias denial + * that outlasted the full [DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS] + * schedule while `KeyguardManager` reported the device unlocked. + * Set by [storeMnemonic]'s heal path atomically with the first + * [KeystoreManager.MASTER_ALIAS_UNBOUND] blob; never cleared. Read + * via [isMasterKeyLockBindingDefectObserved]. + */ + internal val MASTER_LOCK_DEFECT_KEY = booleanPreferencesKey("masterkeylockdefect") + private const val TAG = "WalletStorage" } } diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt index aba0c4ceb2d..eb1101e7a66 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt @@ -767,10 +767,14 @@ class PlatformWalletManager( * encrypt, and thrown BEFORE the native create, so nothing was * created and nothing needs rolling back — or if the Keystore denies * the mnemonic store as device-locked after the false-locked bounded - * retry in [WalletStorage.storeMnemonic] is exhausted (that path runs - * the full rollback below first). A locked device whose master key is - * NOT lock-bound (generated before a PIN was enrolled) proceeds - * normally. + * retry in [WalletStorage.storeMnemonic] is exhausted AND its + * last-rung degradation (re-encrypting under the never-lock-bound + * master alias) also failed (that path runs the full rollback below + * first). A locked device whose master key is NOT lock-bound + * (generated before a PIN was enrolled) proceeds normally, as does a + * device whose false-locked Keystore defect is already on record + * (mnemonic writes target the never-lock-bound alias, which no lock + * state can deny). */ suspend fun createWallet( mnemonic: String, diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt index 8ebac6044d6..f9fb115aa07 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt @@ -32,6 +32,13 @@ import org.robolectric.RobolectricTestRunner * whose sampled `KeyguardManager` state says the device is NOT locked * (the Keystore2 misreporting defect) is retried up to 3 times; a * genuinely-locked denial fails fast with no retry. + * 3. The last-rung DEGRADATION when that schedule exhausts still + * false-locked (the persistent defect — an OEM unlock class that never + * satisfies `UNLOCKED_DEVICE_REQUIRED`): the store re-encrypts under + * the never-lock-bound [KeystoreManager.MASTER_ALIAS_UNBOUND], records + * the defect durably, and from then on writes go straight to the + * unbound alias, reads route by the blob's recorded alias, and + * still-lock-bound blobs are re-wrapped on their first successful read. * * The real AndroidKeyStore crypto cannot run on the JVM (see * [KeySecurityPolicyTest]), so a fake [KeystoreManager] scripts the @@ -43,6 +50,7 @@ import org.robolectric.RobolectricTestRunner class WalletStorageDeviceLockedRetryTest { private val walletId = ByteArray(32) { (it + 1).toByte() } + private val siblingWalletId = ByteArray(32) { (it + 101).toByte() } private val mnemonic = "abandon abandon abandon abandon abandon abandon " + "abandon abandon abandon abandon abandon about" @@ -53,7 +61,8 @@ class WalletStorageDeviceLockedRetryTest { fun setUp() = runBlocking { fake = FalseLockedFakeKeystoreManager() storage = WalletStorage(ApplicationProvider.getApplicationContext(), fake) - // Isolate from any state a prior test left in the shared DataStore file. + // Isolate from any state a prior test left in the shared DataStore + // file — including the durable false-locked defect record. storage.deleteAll() } @@ -66,7 +75,7 @@ class WalletStorageDeviceLockedRetryTest { fake.lockState = DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true) val thrown = assertThrows(KeystoreDeviceLockedException::class.java) { - storage.ensureMasterKeyNotLockBlocked(operation = "createWallet") + runBlocking { storage.ensureMasterKeyNotLockBlocked(operation = "createWallet") } } assertEquals("createWallet", thrown.operation) assertEquals(KeystoreManager.MASTER_ALIAS, thrown.alias) @@ -78,7 +87,7 @@ class WalletStorageDeviceLockedRetryTest { } @Test - fun shouldPassPreCheckWhenDeviceIsUnlocked() { + fun shouldPassPreCheckWhenDeviceIsUnlocked() = runBlocking { fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) storage.ensureMasterKeyNotLockBlocked(operation = "createWallet") // must not throw // Unlocked is decided from KeyguardManager alone — prompt-free AND @@ -87,7 +96,7 @@ class WalletStorageDeviceLockedRetryTest { } @Test - fun shouldPassPreCheckWhenKeyguardShowsButDeviceIsNotSecurelyLocked() { + fun shouldPassPreCheckWhenKeyguardShowsButDeviceIsNotSecurelyLocked() = runBlocking { // isKeyguardLocked without isDeviceLocked (e.g. a non-secure swipe // screen): the Keystore unlocked-device gate keys off the SECURE // lock, so this state must not block wallet creation. @@ -97,7 +106,7 @@ class WalletStorageDeviceLockedRetryTest { } @Test - fun shouldPassPreCheckWhenDeviceIsLockedButMasterKeyIsNotLockBound() { + fun shouldPassPreCheckWhenDeviceIsLockedButMasterKeyIsNotLockBound() = runBlocking { // A master key generated while the device had NO secure lock screen // carries no setUnlockedDeviceRequired // ([KeystoreManager]'s generateWithLockScreenDegradation) and existing @@ -115,6 +124,24 @@ class WalletStorageDeviceLockedRetryTest { assertEquals(1, fake.masterEncryptCalls) } + @Test + fun shouldPassPreCheckOnLockedDeviceOnceDefectIsOnRecord() = runBlocking { + // Demonstrate the persistent defect (unlocked, denials outlast the + // schedule) so the degradation records it... + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + fake.failMasterEncrypts = Int.MAX_VALUE + storage.storeMnemonic(walletId, mnemonic) + assertTrue(storage.isMasterKeyLockBindingDefectObserved()) + val masterEncryptsSoFar = fake.masterEncryptCalls + + // ...then a GENUINELY locked entry must proceed with no probe at + // all: writes target the never-lock-bound alias, which no lock + // state can deny — there is nothing to preflight. + fake.lockState = DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true) + storage.ensureMasterKeyNotLockBlocked(operation = "createWallet") // must not throw + assertEquals(masterEncryptsSoFar, fake.masterEncryptCalls) + } + // ── storeMnemonic bounded FALSE-LOCKED retry ───────────────────────── @Test @@ -125,14 +152,22 @@ class WalletStorageDeviceLockedRetryTest { storage.storeMnemonic(walletId, mnemonic) assertEquals(2, fake.masterEncryptCalls) + // A transient blip must NOT record the persistent defect or touch + // the unbound alias — the retry alone absorbed it. + assertEquals(0, fake.unboundEncryptCalls) + assertFalse(storage.isMasterKeyLockBindingDefectObserved()) // The store really landed: the mnemonic round-trips. assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) } @Test - fun shouldGiveUpAfterThreeFalseLockedRetries() = runBlocking { - fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) - fake.failMasterEncrypts = Int.MAX_VALUE // never heals + fun shouldNotRetryWhenDeviceIsGenuinelyLocked() = runBlocking { + // The denial is CORRECT here — a 2s in-process retry cannot unlock + // a phone, so the exception must propagate immediately, and the + // degradation must NOT fire (a locked phone denying a lock-bound + // key is the gate working, not the defect). + fake.lockState = DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true) + fake.failMasterEncrypts = Int.MAX_VALUE var thrown: KeystoreDeviceLockedException? = null try { @@ -142,19 +177,65 @@ class WalletStorageDeviceLockedRetryTest { } assertTrue("expected the typed denial to propagate", thrown != null) - assertFalse(thrown!!.deviceReportsLocked) + assertTrue(thrown!!.deviceReportsLocked) + assertEquals(1, fake.masterEncryptCalls) + assertEquals(0, fake.unboundEncryptCalls) + assertFalse(storage.isMasterKeyLockBindingDefectObserved()) + } + + @Test + fun shouldStoreWithoutRetryMachineryWhenKeystoreIsHealthy() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + + storage.storeMnemonic(walletId, mnemonic) + + assertEquals(1, fake.masterEncryptCalls) + assertEquals(0, fake.unboundEncryptCalls) + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + } + + // ── last-rung degradation: the PERSISTENT false-locked defect ──────── + + @Test + fun shouldDegradeToUnboundAliasWhenFalseLockedRetriesExhaust() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + fake.failMasterEncrypts = Int.MAX_VALUE // never heals — the persistent defect + + storage.storeMnemonic(walletId, mnemonic) + // Initial attempt + the full 3-retry schedule (250/750/1000ms), - // then give up. + // then ONE unbound-alias encrypt instead of giving up. assertEquals(4, fake.masterEncryptCalls) - assertEquals(null, storage.retrieveMnemonic(walletId)) + assertEquals(1, fake.unboundEncryptCalls) + assertTrue(storage.isMasterKeyLockBindingDefectObserved()) + // The store really landed, and the read routes to the recorded + // alias (the fake rejects a blob decrypted under the wrong one). + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + assertEquals(1, fake.unboundDecryptCalls) + assertEquals(0, fake.masterDecryptCalls) } @Test - fun shouldNotRetryWhenDeviceIsGenuinelyLocked() = runBlocking { - // The denial is CORRECT here — a 2s in-process retry cannot unlock - // a phone, so the exception must propagate immediately. - fake.lockState = DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true) + fun shouldWriteStraightToUnboundAliasOnceDefectIsOnRecord() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + fake.failMasterEncrypts = Int.MAX_VALUE + storage.storeMnemonic(walletId, mnemonic) // demonstrates + records the defect + val masterEncryptsSoFar = fake.masterEncryptCalls + + storage.storeMnemonic(siblingWalletId, mnemonic) + + // No lock-bound attempt, no retry dance — straight to the alias + // that works on this device. + assertEquals(masterEncryptsSoFar, fake.masterEncryptCalls) + assertEquals(2, fake.unboundEncryptCalls) + assertEquals(mnemonic, storage.retrieveMnemonic(siblingWalletId)) + } + + @Test + fun shouldPropagateOriginalDenialWhenDegradationEncryptAlsoFails() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) fake.failMasterEncrypts = Int.MAX_VALUE + fake.failUnboundEncrypts = Int.MAX_VALUE // even the last rung fails var thrown: KeystoreDeviceLockedException? = null try { @@ -164,18 +245,56 @@ class WalletStorageDeviceLockedRetryTest { } assertTrue("expected the typed denial to propagate", thrown != null) - assertTrue(thrown!!.deviceReportsLocked) - assertEquals(1, fake.masterEncryptCalls) + assertFalse(thrown!!.deviceReportsLocked) + assertEquals(4, fake.masterEncryptCalls) + assertEquals(1, fake.unboundEncryptCalls) + // The heal failure rides along for diagnosis... + assertTrue(thrown.suppressed.any { it is IllegalStateException }) + // ...and nothing was recorded or persisted: the failed heal must + // not brand the device defective with no healed blob to show. + assertFalse(storage.isMasterKeyLockBindingDefectObserved()) + assertEquals(null, storage.retrieveMnemonic(walletId)) } @Test - fun shouldStoreWithoutRetryMachineryWhenKeystoreIsHealthy() = runBlocking { + fun shouldRewrapLockBoundBlobOnFirstSuccessfulReadAfterDefectRecorded() = runBlocking { fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + // A pre-existing wallet stored healthily under the lock-bound alias... + storage.storeMnemonic(walletId, mnemonic) + // ...then a sibling wallet's store demonstrates the persistent defect. + fake.failMasterEncrypts = Int.MAX_VALUE + storage.storeMnemonic(siblingWalletId, mnemonic) + fake.failMasterEncrypts = 0 + + // The first successful read of the still-lock-bound blob re-wraps it + // under the unbound alias (sibling's heal + this re-wrap = 2). + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + assertEquals(1, fake.masterDecryptCalls) + assertEquals(2, fake.unboundEncryptCalls) + // Subsequent reads route to the unbound alias — the lock-bound key + // is no longer consulted. + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + assertEquals(1, fake.masterDecryptCalls) + assertTrue(fake.unboundDecryptCalls >= 1) + } + + @Test + fun shouldKeepLockBoundBlobReadableWhenRewrapFails() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) storage.storeMnemonic(walletId, mnemonic) + fake.failMasterEncrypts = Int.MAX_VALUE + storage.storeMnemonic(siblingWalletId, mnemonic) // records the defect + fake.failMasterEncrypts = 0 - assertEquals(1, fake.masterEncryptCalls) + // Re-wrap is best-effort: its failure must not fail the read or + // corrupt the blob, and the next successful read tries again. + fake.failUnboundEncrypts = 1 + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) // retried re-wrap landed + assertEquals(2, fake.masterDecryptCalls) assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + assertEquals(2, fake.masterDecryptCalls) // now routed to unbound } // ── storeMnemonic plaintext-buffer scrubbing ───────────────────────── @@ -194,10 +313,24 @@ class WalletStorageDeviceLockedRetryTest { assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) } + @Test + fun shouldScrubMnemonicBufferAfterDegradedStore() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + fake.failMasterEncrypts = Int.MAX_VALUE // the degradation path runs + + storage.storeMnemonic(walletId, mnemonic) + + assertBufferScrubbed(fake.lastMasterPlaintextRef) + // The unbound encrypt saw the same (single) buffer — scrubbed too. + assertBufferScrubbed(fake.lastUnboundPlaintextRef) + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + } + @Test fun shouldScrubMnemonicBufferWhenFinalDenialPropagates() = runBlocking { fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) - fake.failMasterEncrypts = Int.MAX_VALUE // never heals — the schedule exhausts + fake.failMasterEncrypts = Int.MAX_VALUE + fake.failUnboundEncrypts = Int.MAX_VALUE // degradation fails too — it propagates var thrown = false try { @@ -246,14 +379,25 @@ class WalletStorageDeviceLockedRetryTest { * `setUnlockedDeviceRequired`), any encrypt while [lockState] reports the * device locked is denied, exactly as the real Keystore gate behaves; when * false (a key generated on a then-lockless device, never regenerated), - * encrypts succeed regardless of lock state. Identity-key aliases are out of - * scope here — see [WalletStorageUpgradeMatrixTest]'s fake for that ladder. + * encrypts succeed regardless of lock state. + * + * [KeystoreManager.MASTER_ALIAS_UNBOUND] is modeled per ITS contract: never + * lock-bound, so never denied by any lock state; [failUnboundEncrypts] + * scripts unclassified failures for the degradation-also-fails paths. Each + * blob's iv marks the alias that produced it and [decrypt] rejects a + * mismatch, so the tests prove reads route to the recorded alias. Identity- + * key aliases are out of scope here — see [WalletStorageUpgradeMatrixTest]'s + * fake for that ladder. */ private class FalseLockedFakeKeystoreManager : KeystoreManager() { var lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) var failMasterEncrypts = 0 var masterEncryptCalls = 0 + var failUnboundEncrypts = 0 + var unboundEncryptCalls = 0 + var masterDecryptCalls = 0 + var unboundDecryptCalls = 0 /** Whether the fake master key carries the unlocked-device requirement. */ var masterKeyLockBound = true @@ -264,31 +408,66 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { /** Snapshot of that buffer's content AT CALL TIME (pre-scrub evidence). */ var lastMasterPlaintextAtCall: ByteArray? = null + /** The exact buffer reference the last unbound-alias encrypt received. */ + var lastUnboundPlaintextRef: ByteArray? = null + /** Invoked at each master encrypt attempt (test synchronization hook). */ var onMasterEncrypt: (() -> Unit)? = null override fun sampleDeviceLockState(): DeviceLockState = lockState - override fun encrypt(plaintext: ByteArray, alias: String): EncryptedBlob { - check(alias == MASTER_ALIAS) { "test fake only models the master alias" } - masterEncryptCalls++ - lastMasterPlaintextRef = plaintext - lastMasterPlaintextAtCall = plaintext.copyOf() - onMasterEncrypt?.invoke() - val scriptedDenial = failMasterEncrypts > 0 - if (scriptedDenial) failMasterEncrypts-- - if (scriptedDenial || (masterKeyLockBound && lockState.isDeviceLocked)) { - throw KeystoreDeviceLockedException( - alias = alias, - operation = "encrypt", - lockState = sampleDeviceLockState(), - ) + override fun encrypt(plaintext: ByteArray, alias: String): EncryptedBlob = when (alias) { + MASTER_ALIAS -> { + masterEncryptCalls++ + lastMasterPlaintextRef = plaintext + lastMasterPlaintextAtCall = plaintext.copyOf() + onMasterEncrypt?.invoke() + val scriptedDenial = failMasterEncrypts > 0 + if (scriptedDenial) failMasterEncrypts-- + if (scriptedDenial || (masterKeyLockBound && lockState.isDeviceLocked)) { + throw KeystoreDeviceLockedException( + alias = alias, + operation = "encrypt", + lockState = sampleDeviceLockState(), + ) + } + blob(MASTER_IV_MARKER, plaintext) + } + MASTER_ALIAS_UNBOUND -> { + unboundEncryptCalls++ + lastUnboundPlaintextRef = plaintext + val scriptedFailure = failUnboundEncrypts > 0 + if (scriptedFailure) failUnboundEncrypts-- + check(!scriptedFailure) { "scripted unbound-alias encrypt failure" } + blob(UNBOUND_IV_MARKER, plaintext) } - return EncryptedBlob(iv = ByteArray(12) { 7 }, ciphertext = plaintext.copyOf()) + else -> error("test fake only models the master aliases, got '$alias'") } override fun decrypt(blob: EncryptedBlob, alias: String): ByteArray { - check(alias == MASTER_ALIAS) { "test fake only models the master alias" } + val expectedMarker = when (alias) { + MASTER_ALIAS -> { + masterDecryptCalls++ + MASTER_IV_MARKER + } + MASTER_ALIAS_UNBOUND -> { + unboundDecryptCalls++ + UNBOUND_IV_MARKER + } + else -> error("test fake only models the master aliases, got '$alias'") + } + check(blob.iv.all { it == expectedMarker }) { + "blob was decrypted under the wrong alias: '$alias' cannot open a blob " + + "whose iv marker is ${blob.iv.firstOrNull()}" + } return blob.ciphertext.copyOf() } + + private fun blob(ivMarker: Byte, plaintext: ByteArray) = + EncryptedBlob(iv = ByteArray(12) { ivMarker }, ciphertext = plaintext.copyOf()) + + private companion object { + const val MASTER_IV_MARKER: Byte = 7 + const val UNBOUND_IV_MARKER: Byte = 9 + } } From 5fa1dcaebe8671eba43214dee857c6b66bc17e9b Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 10 Sep 2026 13:22:38 -0700 Subject: [PATCH 2/8] fix(kotlin-sdk): detect the false-locked defect on mnemonic reads too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The degradation ladder added in the previous commit can only ever be entered from storeMnemonic, so the defect is discoverable only while WRITING a mnemonic — in practice only at wallet creation. On a wallet that predates the degradation the blob stays under the lock-bound MASTER_ALIAS, no mnemonic is ever written again, and nothing sets MASTER_LOCK_DEFECT_KEY. Since retrieveMnemonicUtf8's opportunistic re-wrap is GATED on that record, the self-heal it exists to provide can never fire on exactly the devices that need it most: the ones already carrying a wallet when the defective OEM gate shows up. Give the read the same bounded ladder as the write. The retry/classify loop both paths now share moves into retryingFalseLockedDenial, so a genuinely-locked denial still fails fast, a transient Keystore2 blip is still retried, and only a denial that outlasts the whole schedule counts as the defect. A denied read cannot heal itself — a refused decrypt never obtained the plaintext to re-encrypt — so it records the device and lets the original typed denial propagate. That record is the missing link: the next read that gets through (the gate jams for stretches of a session, not forever) finally re-wraps the blob onto the never-lock-bound alias, and later writes skip the lock-bound alias outright. Recording is best-effort — a DataStore failure is attached as suppressed rather than replacing the truthful, retryable denial. Two doc corrections found while doing it. isMasterKeyLockBindingDefectObserved claimed the record is "never cleared", but deleteAll() wipes the whole store including it; carving it out is the wrong fix — the test suite depends on deleteAll restoring a clean slate, and re-deriving the record costs one ladder (~2s) on a wiped store's next write — so the claim is narrowed to the targeted mutators and deleteAll's contract is stated. DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS and MASTER_LOCK_DEFECT_KEY were declared `internal` inside a `private companion object`, where internal is inert; they are private and now say so. Four tests, all on the read side the previous commit left untested: fail-fast when genuinely locked, no branding when a retry succeeds, recording when the schedule exhausts, and the end-to-end field shape — a wallet whose defect only a read ever observes still ends up off the defective gate. 432 tests, 0 failures. Co-Authored-By: Claude Opus 5 --- .../dashsdk/security/WalletStorage.kt | 187 ++++++++++++++---- .../WalletStorageDeviceLockedRetryTest.kt | 102 ++++++++++ 2 files changed, 255 insertions(+), 34 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt index 26f4495b862..86762c13823 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt @@ -337,35 +337,21 @@ class WalletStorage( storeMnemonicUnbound(walletId, plaintext) return } - var attempt = 0 - while (true) { - try { + retryingFalseLockedDenial( + operation = "storeMnemonic", + denied = "encrypt", + attempt = { val blob = keystore.encrypt(plaintext) store.edit { it[mnemonicKey(walletId)] = encode(blob) // A MASTER_ALIAS blob is the untagged default. it.remove(mnemonicAliasKey(walletId)) } - return - } catch (e: KeystoreDeviceLockedException) { - if (e.deviceReportsLocked) throw e - if (attempt >= DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS.size) { - healFalseLockedMnemonicStore(walletId, plaintext, e) - return - } - val delayMs = DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS[attempt] - attempt++ - Log.w( - TAG, - "storeMnemonic: Keystore denied encrypt as device-locked but " + - "KeyguardManager reports UNLOCKED (${e.lockState}) — the " + - "false-locked Keystore2 defect; retry $attempt/" + - "${DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS.size} in ${delayMs}ms", - e, - ) - delay(delayMs) - } - } + }, + onExhausted = { denial -> + healFalseLockedMnemonicStore(walletId, plaintext, denial) + }, + ) } finally { plaintext.fill(0) } @@ -374,14 +360,26 @@ class WalletStorage( /** * Whether THIS device has demonstrated the persistent false-locked * Keystore defect — a lock-bound master-alias operation denied as - * device-locked past [storeMnemonic]'s full bounded retry while - * `KeyguardManager` reported the device unlocked. Recorded durably by - * the heal path in the same atomic edit as the first unbound-alias - * blob; never cleared (the defect is a property of the device's OS - * build, not of any wallet — and a healed device staying healed costs - * nothing on a healthy one, which never sets it). Host-legible so apps - * can surface the degraded protection level in telemetry/support - * flows, the [KeystoreManager.effectiveKeySecurityPolicy] discipline. + * device-locked past the full bounded retry + * ([retryingFalseLockedDenial]) while `KeyguardManager` reported the + * device unlocked. + * + * Recorded durably from EITHER side of the alias, since either can be + * the first to meet the defect: + * - a denied WRITE, atomically with the first unbound-alias blob + * ([healFalseLockedMnemonicStore]); + * - a denied READ, which cannot heal itself but must still register + * the device ([recordLockBindingDefectFromDeniedRead]) — the only + * route on a wallet whose blob predates the degradation. + * + * Never cleared by any targeted mutator — the defect is a property of + * the device's OS build, not of any wallet, and a healed device + * staying healed costs nothing on a healthy one, which never sets it. + * A full [deleteAll] IS a reset, though: it drops the record with + * everything else, and the next mnemonic write simply re-derives it + * through the ladder. Host-legible so apps can surface the degraded + * protection level in telemetry/support flows, the + * [KeystoreManager.effectiveKeySecurityPolicy] discipline. */ suspend fun isMasterKeyLockBindingDefectObserved(): Boolean = store.data.first()[MASTER_LOCK_DEFECT_KEY] == true @@ -442,6 +440,101 @@ class WalletStorage( } } + /** + * Run [attempt] under the bounded false-locked retry schedule shared by + * every lock-bound master-alias operation. + * + * The three outcomes, in the order they are decided: + * - **Success** (first try or any retry) — returned as-is. A retry that + * succeeds is evidence of the TRANSIENT Keystore2 blip, so nothing is + * recorded: the device is not defective, it merely blipped. + * - **Genuinely locked** ([KeystoreDeviceLockedException.deviceReportsLocked]) + * — rethrown immediately with no retry. Waiting cannot unlock a phone. + * - **False-locked past the whole schedule** — handed to [onExhausted], + * the caller's degradation rung. Keystore denied a lock-bound + * operation for ~2s while `KeyguardManager` insisted the device was + * unlocked, which is the persistent OEM defect rather than a blip. + * + * [operation] and [denied] only shape the retry log line (`"storeMnemonic"` + * / `"encrypt"`). + */ + private suspend fun retryingFalseLockedDenial( + operation: String, + denied: String, + attempt: suspend () -> T, + onExhausted: suspend (KeystoreDeviceLockedException) -> T, + ): T { + var retries = 0 + while (true) { + try { + return attempt() + } catch (e: KeystoreDeviceLockedException) { + if (e.deviceReportsLocked) throw e + if (retries >= DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS.size) return onExhausted(e) + val delayMs = DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS[retries] + retries++ + Log.w( + TAG, + "$operation: Keystore denied $denied as device-locked but " + + "KeyguardManager reports UNLOCKED (${e.lockState}) — the " + + "false-locked Keystore2 defect; retry $retries/" + + "${DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS.size} in ${delayMs}ms", + e, + ) + delay(delayMs) + } + } + } + + /** + * Record the false-locked defect observed on a READ, then let the + * original denial propagate. + * + * A denied read cannot heal itself the way [storeMnemonic] can: the heal + * is a re-encrypt under the never-lock-bound alias, and a read that was + * refused never obtained the plaintext to re-encrypt. What it CAN do is + * put the defect on record, which is load-bearing for two later paths + * that are otherwise unreachable on a wallet created before the defect + * appeared (the field case — the blob predates the degradation, so no + * mnemonic is ever written again and the write ladder never runs): + * + * - [retrieveMnemonicUtf8]'s opportunistic [rewrapMnemonicUnbound] is + * gated on the record, so the first read that DOES get through (the + * Keystore denies lock-bound operations only for stretches of a + * session) finally moves the blob off the defective gate. Without a + * read-side record that re-wrap can never fire. + * - [storeMnemonic] and [ensureMasterKeyNotLockBlocked] stop betting on + * the lock-bound alias for everything they do afterwards. + * + * Best-effort: a DataStore failure here must not replace the truthful, + * retryable denial the caller needs to see, so it is attached as + * suppressed and the denial still wins. + */ + private suspend fun recordLockBindingDefectFromDeniedRead( + denial: KeystoreDeviceLockedException, + ): Nothing { + Log.w( + TAG, + "retrieveMnemonicUtf8: Keystore still denied the lock-bound master-alias " + + "decrypt as device-locked after the full false-locked retry schedule, " + + "with KeyguardManager reporting UNLOCKED (${denial.lockState}) — recording " + + "this device's UNLOCKED_DEVICE_REQUIRED implementation as defective so the " + + "next read that gets through re-wraps the blob under the never-lock-bound " + + "'${KeystoreManager.MASTER_ALIAS_UNBOUND}' (cf. Google Issue Tracker " + + "506989112). This read still fails — a refused decrypt has no plaintext " + + "to re-encrypt.", + denial, + ) + try { + store.edit { it[MASTER_LOCK_DEFECT_KEY] = true } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + denial.addSuppressed(e) + } + throw denial + } + /** * Decrypt the mnemonic as a display `String`. For explicit * user-facing reveal flows ONLY (seed backup, biometric reveal) — @@ -472,12 +565,33 @@ class WalletStorage( * the unbound alias (best-effort, see [rewrapMnemonicUnbound]) so it * stops being hostage to the defective `UNLOCKED_DEVICE_REQUIRED` * gate. + * + * A lock-bound decrypt runs the SAME bounded false-locked ladder as + * [storeMnemonic] ([retryingFalseLockedDenial]): a genuinely-locked + * denial fails fast, a transient one is retried, and one that outlasts + * the schedule puts the defect on record before the denial propagates + * ([recordLockBindingDefectFromDeniedRead]). The read itself still + * fails — there is no plaintext to re-encrypt — but recording it is + * what ARMS the re-wrap above on a wallet whose blob predates the + * degradation, where no mnemonic is ever written again and the write + * ladder therefore never runs. */ suspend fun retrieveMnemonicUtf8(walletId: ByteArray): ByteArray? { val prefs = store.data.first() val encoded = prefs[mnemonicKey(walletId)] ?: return null val alias = prefs[mnemonicAliasKey(walletId)] ?: KeystoreManager.MASTER_ALIAS - val plain = keystore.decrypt(decode(encoded), alias) + val blob = decode(encoded) + val plain = retryingFalseLockedDenial( + operation = "retrieveMnemonicUtf8", + denied = "decrypt", + attempt = { keystore.decrypt(blob, alias) }, + onExhausted = { denial -> recordLockBindingDefectFromDeniedRead(denial) }, + ) + // Deliberately the PRE-decrypt snapshot: this is the hot resolver + // path (Rust calls it synchronously for every derivation), so it must + // not pay a second DataStore read. The only writer that could have + // set the flag during the decrypt is the exhausted-ladder recorder + // above, and that path throws instead of reaching here. if (alias == KeystoreManager.MASTER_ALIAS && prefs[MASTER_LOCK_DEFECT_KEY] == true) { rewrapMnemonicUnbound(walletId, plain) } @@ -1206,6 +1320,11 @@ class WalletStorage( suspend fun deleteAll() { // Clears privkey.* entries too — take the same exclusion as the // targeted mutators so it can't interleave with a compound sweep. + // [MASTER_LOCK_DEFECT_KEY] goes with it: the record is scoped to + // THIS store, and re-deriving it costs one false-locked retry + // schedule (~2s) on the next mnemonic write, which a wiped store + // always has ahead of it. Carving it out instead would make a full + // wipe unable to restore a clean slate — including between tests. privateKeyMutex.withLock { store.edit { it.clear() } } @@ -1315,7 +1434,7 @@ class WalletStorage( * `KeyguardManager` reported the device unlocked): 3 retries, * ~2s total. Genuinely-locked denials never retry. */ - internal val DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS = longArrayOf(250, 750, 1000) + private val DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS = longArrayOf(250, 750, 1000) /** * Durable device-scoped record that the false-locked Keystore @@ -1326,7 +1445,7 @@ class WalletStorage( * [KeystoreManager.MASTER_ALIAS_UNBOUND] blob; never cleared. Read * via [isMasterKeyLockBindingDefectObserved]. */ - internal val MASTER_LOCK_DEFECT_KEY = booleanPreferencesKey("masterkeylockdefect") + private val MASTER_LOCK_DEFECT_KEY = booleanPreferencesKey("masterkeylockdefect") private const val TAG = "WalletStorage" } diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt index f9fb115aa07..ce3bcec69a7 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt @@ -297,6 +297,91 @@ class WalletStorageDeviceLockedRetryTest { assertEquals(2, fake.masterDecryptCalls) // now routed to unbound } + // ── retrieveMnemonicUtf8's false-locked ladder ─────────────────────── + + @Test + fun shouldFailFastWhenMnemonicReadIsDeniedOnGenuinelyLockedDevice() { + runBlocking { storage.storeMnemonic(walletId, mnemonic) } + fake.masterDecryptCalls = 0 + // Genuinely locked: the denial is CORRECT. Retrying cannot unlock a + // phone, and the device is not defective — nothing may be recorded. + fake.lockState = DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true) + + val thrown = assertThrows(KeystoreDeviceLockedException::class.java) { + runBlocking { storage.retrieveMnemonicUtf8(walletId) } + } + assertTrue(thrown.deviceReportsLocked) + assertEquals("decrypt", thrown.operation) + assertEquals(1, fake.masterDecryptCalls) + assertFalse(runBlocking { storage.isMasterKeyLockBindingDefectObserved() }) + } + + @Test + fun shouldRetryFalseLockedMnemonicReadAndSucceedOnSecondAttempt() = runBlocking { + storage.storeMnemonic(walletId, mnemonic) + fake.masterDecryptCalls = 0 + // One denial while KeyguardManager reports UNLOCKED — the transient + // Keystore2 blip the schedule exists to outwait. + fake.failMasterDecrypts = 1 + + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + assertEquals(2, fake.masterDecryptCalls) + // A retry that SUCCEEDS is a blip, not the persistent defect: the + // device must not be branded, or every healthy phone that ever + // blipped would downgrade itself permanently. + assertFalse(storage.isMasterKeyLockBindingDefectObserved()) + } + + @Test + fun shouldRecordDefectWhenFalseLockedMnemonicReadRetriesExhaust() = runBlocking { + storage.storeMnemonic(walletId, mnemonic) + fake.masterDecryptCalls = 0 + fake.failMasterDecrypts = Int.MAX_VALUE + + // The read itself still fails — a refused decrypt never obtained the + // plaintext, so unlike storeMnemonic it cannot heal in place. + assertThrows(KeystoreDeviceLockedException::class.java) { + runBlocking { storage.retrieveMnemonicUtf8(walletId) } + } + // One initial attempt plus the full DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS + // schedule (3 delays). + assertEquals(4, fake.masterDecryptCalls) + // What it CAN do — and must — is put the device on record. + assertTrue(storage.isMasterKeyLockBindingDefectObserved()) + } + + @Test + fun shouldRewrapOffTheDefectiveGateAfterOnlyReadsEverObservedIt() = runBlocking { + // The field shape (MO-972's sibling): a wallet created before the + // degradation existed. Its blob sits under the lock-bound alias and + // no mnemonic is EVER written again, so the write ladder never runs + // and only a read can discover the defect. + storage.storeMnemonic(walletId, mnemonic) + fake.masterDecryptCalls = 0 + fake.unboundEncryptCalls = 0 + + // Session 1 — the gate is jammed. The read fails, but registers. + fake.failMasterDecrypts = Int.MAX_VALUE + assertThrows(KeystoreDeviceLockedException::class.java) { + runBlocking { storage.retrieveMnemonicUtf8(walletId) } + } + assertTrue(storage.isMasterKeyLockBindingDefectObserved()) + assertEquals(0, fake.unboundEncryptCalls) + + // Session 2 — the gate lets a read through (e.g. after a credential + // unlock). The record armed the re-wrap, which now fires. + fake.failMasterDecrypts = 0 + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + assertEquals(1, fake.unboundEncryptCalls) + + // The lock-bound key is never consulted again, so a future jam + // cannot strand this wallet. + val masterDecryptsBefore = fake.masterDecryptCalls + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + assertEquals(masterDecryptsBefore, fake.masterDecryptCalls) + assertTrue(fake.unboundDecryptCalls >= 1) + } + // ── storeMnemonic plaintext-buffer scrubbing ───────────────────────── @Test @@ -394,6 +479,14 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { var lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) var failMasterEncrypts = 0 var masterEncryptCalls = 0 + + /** + * Scripted device-locked denials for MASTER_ALIAS **decrypts** — the + * read-side mirror of [failMasterEncrypts]. `Int.MAX_VALUE` models the + * persistent defect (the gate stays jammed for the whole session); + * a small count models the transient Keystore2 blip. + */ + var failMasterDecrypts = 0 var failUnboundEncrypts = 0 var unboundEncryptCalls = 0 var masterDecryptCalls = 0 @@ -448,6 +541,15 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { val expectedMarker = when (alias) { MASTER_ALIAS -> { masterDecryptCalls++ + val scriptedDenial = failMasterDecrypts > 0 + if (scriptedDenial) failMasterDecrypts-- + if (scriptedDenial || (masterKeyLockBound && lockState.isDeviceLocked)) { + throw KeystoreDeviceLockedException( + alias = alias, + operation = "decrypt", + lockState = sampleDeviceLockState(), + ) + } MASTER_IV_MARKER } MASTER_ALIAS_UNBOUND -> { From bdd82a8e69430703d9ee294103f0b19d778c1065 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 10 Sep 2026 13:39:29 -0700 Subject: [PATCH 3/8] fix(kotlin-sdk): stop the defective lock gate from breaking identity signing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MO-972: DashPay username creation fails outright on a HONOR PTP-N49 (MagicOS, Android 16). Signing an identity state transition dies pre-broadcast with "Protocol error: Generic Error: User not authenticated", one second after a successful biometric, twice. The wallet reported it as "Keystore auth window expired". There is no auth window. The wallet has run KeySecurityPolicy.DEVICE_BOUND since dash-wallet 7e7d53485 precisely to be rid of the authentication gate, and that worked — KEYS_ALIAS_DEVICE_BOUND carries no setUserAuthenticationRequired. What it does still carry is setUnlockedDeviceRequired, applied by ensureKeysKeyPair to every alias on any device with a lock screen. Android reports a denial of THAT gate with the same UserNotAuthenticatedException it uses for a closed auth window, and this device's OEM Keystore denies it while KeyguardManager reports the device unlocked — the defect the previous commits already handle for the master alias (cf. Google Issue Tracker 506989112, confirmed by Google on Fairphone 5/6; AOSP ties UNLOCKED_DEVICE_REQUIRED availability to how the device was unlocked). The SDK could not tell the two apart because it never tried: KeystoreManager.decrypt returns early for identity aliases and never reaches rethrowClassifyingDeviceLockedDenial, whose allowlist was MASTER_ALIAS alone. So the bare exception arrived at KeystoreSigner, which read it as a closed auth window, looked for a BiometricGate to re-prompt with, found none wired, and completed the sign generically. Classify it where it is unambiguous. The allowlist becomes {MASTER_ALIAS, KEYS_ALIAS_DEVICE_BOUND} — both lock-bound and NOT auth-gated, so the exception can only mean the lock gate. KEYS_ALIAS_AUTH_GATED stays excluded, since it carries both gates and only the auth one is fixable by prompting; the *_UNBOUND aliases stay excluded because they carry neither and must not promise a retry no unlock can satisfy. Then give identity keys the master alias's degradation ladder, targeting a new never-lock-bound KEYS_ALIAS_DEVICE_BOUND_UNBOUND. A denied read retries, records the device, and propagates truthfully; once recorded, new identity-key writes skip the lock-bound alias, and the first read that gets through re-wraps the stranded blob through the existing conditional migration, which now resolves the EFFECTIVE write alias rather than blindly the policy alias. Dropping lock binding costs nothing DEVICE_BOUND ever promised — hardware-backed where available, non-exportable and never auth-gated all survive; only the incidental "unlocked right now" hardening goes, on a device where that gate is broken anyway. AUTH_GATED is deliberately NOT given an unbound variant: its authentication gate is the real control, and no field evidence puts a defective device on it. Classifying also fixes two silent mistakes that only appear now the typed exception exists. KeystoreDeviceLockedException is a GeneralSecurityException, so retrievePrivateKey's recovery ladder and tryFormerRsaRecovery would have swallowed a lock denial into "wrong key" and returned null — a spurious re-derive for an intact key — and probeOpensBlob would have reported that key strandable to the health sheet. Both now treat it as what it is: retryable, and recoverable. Nine tests. Two pin the classifier allowlist prompt-free; seven cover the storage ladder in a new WalletStorageIdentityKeyLockDefectTest — fail fast when genuinely locked, no branding when a retry succeeds, recording when the schedule exhausts, writes moving off the gate, the end-to-end field shape where only a read ever observes the defect, best-effort re-wrap failure, and the no-spurious-re-derive guarantee. 441 tests, 0 failures, debug and release. Device verification is still owed: emulators classify every denial as genuinely locked, so the defective-OEM branch is unreachable there and this needs the HONOR PTP-N49 with QA. Co-Authored-By: Claude Opus 5 --- .../dashsdk/security/KeystoreManager.kt | 83 ++++- .../dashsdk/security/KeystoreSigner.kt | 14 + .../dashsdk/security/WalletStorage.kt | 109 +++++- .../KeystoreDeviceLockedDenialTest.kt | 49 +++ .../WalletStorageIdentityKeyLockDefectTest.kt | 338 ++++++++++++++++++ 5 files changed, 579 insertions(+), 14 deletions(-) create mode 100644 packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt index 2bc27d41ad5..1cbeb113101 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt @@ -347,6 +347,18 @@ open class KeystoreManager( e.addSuppressed(deleteError) } throw e + } catch (e: Exception) { + // A lock-bound identity alias denies exactly like the master + // AES key does, and Android reports it with the SAME + // `UserNotAuthenticatedException` it uses for a closed auth + // window. Left unclassified this arrives at `KeystoreSigner` + // looking like an expired auth window on a key that has no + // window at all (MO-972: DEVICE_BOUND signing died with + // "User not authenticated" one second after a successful + // biometric). The mapping is alias-gated — see + // [UNAMBIGUOUS_LOCK_BOUND_ALIASES] — so the auth-gated alias + // still reaches the BiometricGate untouched. + rethrowClassifyingDeviceLockedDenial(e, alias, operation = "decrypt") } return cipher.doFinal(blob.ciphertext) } @@ -396,7 +408,7 @@ open class KeystoreManager( alias: String, operation: String, ): Nothing { - if (alias == MASTER_ALIAS && isDeviceLockedKeystoreDenial(e)) { + if (alias in UNAMBIGUOUS_LOCK_BOUND_ALIASES && isDeviceLockedKeystoreDenial(e)) { throw KeystoreDeviceLockedException( alias = alias, operation = operation, @@ -868,6 +880,19 @@ open class KeystoreManager( generator.initialize(spec(strongBox = false, lockBound = true)) generator.generateKeyPair() } + } else if (alias == KEYS_ALIAS_DEVICE_BOUND_UNBOUND) { + // This alias's whole contract is the ABSENCE of lock binding — it + // exists only as the false-locked degradation target — so it never + // enters the lock-screen ladder: the parameter is dropped + // unconditionally, not probed (the MASTER_ALIAS_UNBOUND rule). + // StrongBox→TEE fallback still applies. + try { + generator.initialize(spec(strongBox = true, lockBound = false)) + generator.generateKeyPair() + } catch (_: StrongBoxUnavailableException) { + generator.initialize(spec(strongBox = false, lockBound = false)) + generator.generateKeyPair() + } } else { // DEVICE_BOUND: no auth gate exists to lie about — dropping the // (inherently lock-dependent) setUnlockedDeviceRequired bit on a @@ -951,6 +976,35 @@ open class KeystoreManager( */ const val KEYS_ALIAS_DEVICE_BOUND = "org.dashfoundation.wallet.keys.devicebound" + /** + * Never-lock-bound variant of [KEYS_ALIAS_DEVICE_BOUND]: the same + * non-auth-gated RSA-2048 OAEP wrapping pair, but + * `setUnlockedDeviceRequired` is never applied at generation + * regardless of the lock-screen probe (see [ensureKeysKeyPair]). + * + * The identity-key counterpart of [MASTER_ALIAS_UNBOUND], and the + * degradation target [WalletStorage] moves identity-key blobs to on a + * device whose Keystore denies lock-bound operations while + * `KeyguardManager` reports it unlocked. Dropping the lock binding + * costs nothing this policy ever promised — + * [KeySecurityPolicy.DEVICE_BOUND] guarantees hardware-backed, + * non-exportable and NOT auth-gated, and both survive here; only the + * incidental "device must be unlocked right now" hardening is given + * up, on a device where that gate is broken anyway. + * + * There is deliberately NO auth-gated counterpart: dropping lock + * binding under [KeySecurityPolicy.AUTH_GATED] would leave that + * policy's real control (the authentication gate) as the only + * protection while making its failures harder to tell apart, and no + * field evidence puts a defective device on that alias. An auth-gated + * install on a defective device keeps failing honestly instead. + * + * Provisioned lazily on first use, only ever on a device that + * demonstrated the defect. + */ + const val KEYS_ALIAS_DEVICE_BOUND_UNBOUND = + "org.dashfoundation.wallet.keys.devicebound.unbound" + /** Auth window for the auth-gated identity-keys alias, in seconds. */ const val AUTH_VALIDITY_SECONDS = 30 @@ -961,7 +1015,32 @@ open class KeystoreManager( * never through the RSA encrypt/decrypt path. */ fun isIdentityKeysAlias(alias: String): Boolean = - alias == KEYS_ALIAS_AUTH_GATED || alias == KEYS_ALIAS_DEVICE_BOUND + alias == KEYS_ALIAS_AUTH_GATED || + alias == KEYS_ALIAS_DEVICE_BOUND || + alias == KEYS_ALIAS_DEVICE_BOUND_UNBOUND + + /** + * Aliases whose keys carry `setUnlockedDeviceRequired` but NO + * `setUserAuthenticationRequired` — the only ones where a Keystore + * `UserNotAuthenticatedException` is unambiguous. With no + * authentication gate to be "not authenticated" against, Keystore + * raises it solely for the unlocked-device requirement, so + * [rethrowClassifyingDeviceLockedDenial] can safely map it to the + * typed, retryable [KeystoreDeviceLockedException]. + * + * [KEYS_ALIAS_AUTH_GATED] is excluded and must stay excluded: it + * carries BOTH gates, so the same exception means either "the device + * is locked" or "the auth window closed", and only the latter is + * fixable by prompting. Classifying it would strand the + * `BiometricGate` prompt-and-retry contract. + * + * The `*_UNBOUND` aliases are excluded for the opposite reason — + * they carry neither gate, so a denial there is not a lock denial at + * all and must surface raw rather than as a "retry after unlock" + * that can never come good. + */ + private val UNAMBIGUOUS_LOCK_BOUND_ALIASES = + setOf(MASTER_ALIAS, KEYS_ALIAS_DEVICE_BOUND) /** * Whether the lock-screen-bound key-generation parameters diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.kt index 53cfebcde3b..5a2fbb82631 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.kt @@ -331,6 +331,20 @@ class KeystoreSigner( /** * Decrypt the key; on an expired auth window, run the biometric gate * once and retry — mirroring KeychainSigner's LAContext flow. + * + * A [KeystoreDeviceLockedException] deliberately does NOT come here. + * It is the typed "the Keystore refused a lock-bound key" signal, which + * `WalletStorage` now raises for the non-auth-gated identity alias too + * (MO-972). Android reports that denial with the very same + * `UserNotAuthenticatedException` as a closed auth window, and treating + * the two alike is what made the field failure unreadable: signing on a + * `DEVICE_BOUND` install — a policy with no auth window at all — was + * reported as "Keystore auth window expired" one second after a + * successful biometric. Prompting cannot help either: the gate tracks + * the device's lock state, not recency of authentication, so a prompt + * would burn a user interaction and fail identically. Letting the typed + * exception escape completes the sign with its own explicit message + * (which alias, and what `KeyguardManager` said at the time) instead. */ private suspend fun retrieveKeyWithAuth(storageKey: String): ByteArray? = try { diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt index 86762c13823..6e28f95baaa 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt @@ -511,18 +511,18 @@ class WalletStorage( * suppressed and the denial still wins. */ private suspend fun recordLockBindingDefectFromDeniedRead( + operation: String, denial: KeystoreDeviceLockedException, ): Nothing { Log.w( TAG, - "retrieveMnemonicUtf8: Keystore still denied the lock-bound master-alias " + - "decrypt as device-locked after the full false-locked retry schedule, " + - "with KeyguardManager reporting UNLOCKED (${denial.lockState}) — recording " + - "this device's UNLOCKED_DEVICE_REQUIRED implementation as defective so the " + - "next read that gets through re-wraps the blob under the never-lock-bound " + - "'${KeystoreManager.MASTER_ALIAS_UNBOUND}' (cf. Google Issue Tracker " + - "506989112). This read still fails — a refused decrypt has no plaintext " + - "to re-encrypt.", + "$operation: Keystore still denied the lock-bound '${denial.alias}' decrypt as " + + "device-locked after the full false-locked retry schedule, with " + + "KeyguardManager reporting UNLOCKED (${denial.lockState}) — recording this " + + "device's UNLOCKED_DEVICE_REQUIRED implementation as defective so the next " + + "read that gets through re-wraps the blob under the matching " + + "never-lock-bound alias (cf. Google Issue Tracker 506989112). This read " + + "still fails — a refused decrypt has no plaintext to re-encrypt.", denial, ) try { @@ -585,7 +585,9 @@ class WalletStorage( operation = "retrieveMnemonicUtf8", denied = "decrypt", attempt = { keystore.decrypt(blob, alias) }, - onExhausted = { denial -> recordLockBindingDefectFromDeniedRead(denial) }, + onExhausted = { denial -> + recordLockBindingDefectFromDeniedRead("retrieveMnemonicUtf8", denial) + }, ) // Deliberately the PRE-decrypt snapshot: this is the hot resolver // path (Rust calls it synchronously for every derivation), so it must @@ -854,7 +856,7 @@ class WalletStorage( privateKey: ByteArray, ownerWalletId: ByteArray?, ) { - val encrypted = keystore.encryptForIdentityKeys(privateKey) + val encrypted = encryptIdentityKeyOffDefectiveGate(privateKey) val blob = encrypted.blob val fingerprint = encrypted.keyFingerprint val alias = encrypted.alias @@ -869,6 +871,45 @@ class WalletStorage( } } + /** + * Encrypt identity-key material, routing AROUND the lock-bound alias on a + * device that has demonstrated the false-locked Keystore defect. + * + * Normally this is just [KeystoreManager.encryptForIdentityKeys] — the + * policy alias, chosen by [KeySecurityPolicy]. Once the defect is on + * record, a [KeySecurityPolicy.DEVICE_BOUND] write goes to + * [KeystoreManager.KEYS_ALIAS_DEVICE_BOUND_UNBOUND] instead, because the + * policy alias carries `setUnlockedDeviceRequired` and this device's + * Keystore denies that gate for stretches of every unlock session — + * which at signing time surfaces as MO-972 ("User not authenticated" on + * a key with no auth window). The unbound alias keeps everything + * DEVICE_BOUND actually promises: hardware-backed where the device + * provides it, non-exportable, and never auth-gated. + * + * [KeySecurityPolicy.AUTH_GATED] is deliberately NOT redirected — see + * [KeystoreManager.KEYS_ALIAS_DEVICE_BOUND_UNBOUND]. Its authentication + * gate is the real control, and there is no evidence of a defective + * device on that alias; it keeps failing honestly instead of quietly + * shedding a gate. + * + * The producing alias rides back on the blob and is persisted per entry + * (`privkeyalias.`), so reads route to whichever alias + * actually wrote each one and nothing already stored is invalidated. + */ + private suspend fun encryptIdentityKeyOffDefectiveGate( + privateKey: ByteArray, + ): KeystoreManager.KeysAliasEncryptedBlob = + if (keystore.keySecurityPolicy == KeySecurityPolicy.DEVICE_BOUND && + isMasterKeyLockBindingDefectObserved() + ) { + keystore.encryptForIdentityKeysAlias( + KeystoreManager.KEYS_ALIAS_DEVICE_BOUND_UNBOUND, + privateKey, + ) + } else { + keystore.encryptForIdentityKeys(privateKey) + } + /** * The RSA identity-keys alias recorded as having written [pubkeyHex]'s * blob (`privkeyalias.`), falling back to the current policy @@ -991,7 +1032,32 @@ class WalletStorage( // former key opens it — a re-derive signal) — never stale // plaintext, and never an uncaught crypto exception. return try { - keystore.decrypt(blob, alias = recordedAlias) + val plain = retryingFalseLockedDenial( + operation = "retrievePrivateKey", + denied = "decrypt", + attempt = { keystore.decrypt(blob, alias = recordedAlias) }, + onExhausted = { denial -> + recordLockBindingDefectFromDeniedRead("retrievePrivateKey", denial) + }, + ) + // A device that just proved its lock gate is defective must + // stop keeping THIS key behind it. Re-encrypting under the + // effective write alias (now the never-lock-bound one) is the + // same best-effort, conditional rewrite the legacy migration + // uses, so a failure simply retries on the next read. + if (recordedAlias == KeystoreManager.KEYS_ALIAS_DEVICE_BOUND && + isMasterKeyLockBindingDefectObserved() + ) { + migrateToPolicyAlias(pubkeyHex, plain, encoded) + } + plain + } catch (e: KeystoreDeviceLockedException) { + // Retryable lock denial, NOT a wrong-key signal. It is a + // GeneralSecurityException, so without this clause it would + // fall into the recovery ladder below and end as `null` — a + // spurious "re-derive this key" for a key that is perfectly + // intact and readable as soon as the gate lets go. + throw e } catch (e: UserNotAuthenticatedException) { throw e // closed auth window — prompt and retry, never recovery } catch (e: KeyPermanentlyInvalidatedException) { @@ -1047,6 +1113,10 @@ class WalletStorage( private fun tryFormerRsaRecovery(blob: KeystoreManager.EncryptedBlob): ByteArray? = try { keystore.decryptLegacyRsaKeysBlob(blob) + } catch (e: KeystoreDeviceLockedException) { + // "The device is locked", never "not this key" — absorbing it to + // null would report an intact blob unrecoverable. + throw e } catch (e: UserNotAuthenticatedException) { throw e } catch (e: KeyPermanentlyInvalidatedException) { @@ -1083,7 +1153,12 @@ class WalletStorage( sourceEncoded: String, ) { try { - val migrated = keystore.encryptForIdentityKeys(plain) + // The EFFECTIVE write alias, not blindly the policy alias: on a + // device with the false-locked defect on record that is the + // never-lock-bound alias, which is what makes this the re-wrap + // that gets a stranded key off the defective gate as well as the + // forward-migration for a recovered legacy blob. + val migrated = encryptIdentityKeyOffDefectiveGate(plain) store.edit { val key = privateKeyKey(pubkeyHex) if (it[key] == sourceEncoded) { @@ -1307,6 +1382,16 @@ class WalletStorage( } else { false } + } catch (e: KeystoreDeviceLockedException) { + // The device's lock gate is shut (genuinely, or the false-locked + // defect). The blob and its key are intact and open as soon as + // the gate lets go, so this is RECOVERABLE — unconditionally, + // unlike UNAE. The [unaeProvesRecoverable] caveat exists because + // a closed AUTH gate hides WHICH key was asked; a device-locked + // denial carries its alias and proves nothing about ownership + // either way, so reporting "strandable" here would offer a + // re-derive for a perfectly good key. + true } catch (e: UserNotAuthenticatedException) { unaeProvesRecoverable } catch (e: GeneralSecurityException) { diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedDenialTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedDenialTest.kt index 6c61f681e0f..ccfae398869 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedDenialTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedDenialTest.kt @@ -143,6 +143,55 @@ class KeystoreDeviceLockedDenialTest { assertTrue(thrown.message.orEmpty().contains("genuinely locked")) } + @Test + fun shouldMapDeviceBoundIdentityAliasDenialToTypedException() { + // MO-972. KEYS_ALIAS_DEVICE_BOUND carries setUnlockedDeviceRequired + // but NO auth gate, so — exactly like MASTER_ALIAS — a Keystore + // "user not authenticated" from it can only be the unlocked-device + // denial. Left unclassified it reached KeystoreSigner looking like an + // expired auth window on a policy that HAS no auth window, and the + // wallet reported "Keystore auth window expired" one second after a + // successful biometric. + val manager = managerSampling( + DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false), + ) + val denial = SimulatedUserNotAuthenticatedException() + + val thrown = assertThrows(KeystoreDeviceLockedException::class.java) { + manager.rethrowClassifyingDeviceLockedDenial( + denial, + KeystoreManager.KEYS_ALIAS_DEVICE_BOUND, + operation = "decrypt", + ) + } + assertEquals(KeystoreManager.KEYS_ALIAS_DEVICE_BOUND, thrown.alias) + assertEquals("decrypt", thrown.operation) + assertFalse(thrown.deviceReportsLocked) + assertSame(denial, thrown.cause) + } + + @Test + fun shouldRethrowUnboundAliasDenialsUnclassified() { + // The *_UNBOUND aliases carry NEITHER gate, so a denial there is not + // a lock denial at all. Classifying it would promise "retry after + // unlock" for a failure no unlock can fix, and would send the + // degradation ladders chasing a device defect that is not the one + // they heal. + val manager = managerSampling( + DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true), + ) + for (alias in listOf( + KeystoreManager.MASTER_ALIAS_UNBOUND, + KeystoreManager.KEYS_ALIAS_DEVICE_BOUND_UNBOUND, + )) { + val raw = SimulatedUserNotAuthenticatedException() + val thrown = assertThrows(SimulatedUserNotAuthenticatedException::class.java) { + manager.rethrowClassifyingDeviceLockedDenial(raw, alias, operation = "decrypt") + } + assertSame(raw, thrown) + } + } + @Test fun shouldRethrowAuthGatedAliasUserNotAuthenticatedUnclassified() { // The auth-gated identity-keys alias' NORMAL pre-prompt contract: diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt new file mode 100644 index 00000000000..720f8fd4e92 --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt @@ -0,0 +1,338 @@ +package org.dashfoundation.dashsdk.security + +import androidx.test.core.app.ApplicationProvider +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Pins MO-972: identity-key **signing** dying on a device whose Keystore + * denies lock-bound keys while `KeyguardManager` reports it unlocked. + * + * [KeySecurityPolicy.DEVICE_BOUND] exists precisely so a host with its own + * PIN never meets an authentication gate, and the production wallet adopted + * it for that reason. But [KeystoreManager.KEYS_ALIAS_DEVICE_BOUND] still + * carries `setUnlockedDeviceRequired`, and Android reports THAT denial with + * the very same `UserNotAuthenticatedException` as a closed auth window — so + * on the defective device the failure came back as "User not authenticated" + * from a policy with no auth window, one second after a successful biometric. + * + * The storage-side contract this fixes, in the order a device meets it: + * 1. a genuinely-locked denial still fails fast, unretried and unrecorded; + * 2. a denial that outlasts the bounded false-locked schedule records the + * device, and the read still fails truthfully; + * 3. once recorded, new identity-key writes go to the never-lock-bound + * [KeystoreManager.KEYS_ALIAS_DEVICE_BOUND_UNBOUND]; + * 4. the first read that DOES get through re-wraps the stranded blob onto + * that alias, after which the defective gate is out of the signing path; + * 5. throughout, a lock denial is never mistaken for a wrong key — the + * recovery ladder must not report an intact key as needing a re-derive. + * + * The classification itself (which aliases may map the ambiguous exception) + * is pinned separately and prompt-free in [KeystoreDeviceLockedDenialTest]; + * the fake here raises the typed exception directly, exactly as a classified + * [KeystoreManager.decrypt] would. + */ +@RunWith(RobolectricTestRunner::class) +class WalletStorageIdentityKeyLockDefectTest { + + private val pubkeyHex = "02" + "ab".repeat(32) + private val privateKey = ByteArray(32) { (it + 3).toByte() } + private val walletId = ByteArray(32) { (it + 11).toByte() } + private val mnemonic = "abandon abandon abandon abandon abandon abandon " + + "abandon abandon abandon abandon abandon about" + + private lateinit var fake: DeviceBoundLockDefectFakeKeystore + private lateinit var storage: WalletStorage + + @Before + fun setUp() = runBlocking { + fake = DeviceBoundLockDefectFakeKeystore() + storage = WalletStorage(ApplicationProvider.getApplicationContext(), fake) + // Shared DataStore file — clear prior state, the defect record included. + storage.deleteAll() + } + + /** Drive the master-alias write ladder until the defect is on record. */ + private suspend fun recordDefectViaMnemonicWrite() { + fake.failMasterEncrypts = Int.MAX_VALUE + storage.storeMnemonic(walletId, mnemonic) + fake.failMasterEncrypts = 0 + assertTrue(storage.isMasterKeyLockBindingDefectObserved()) + } + + @Test + fun shouldFailFastWhenIdentityKeyReadIsDeniedOnGenuinelyLockedDevice() { + runBlocking { storage.storePrivateKey(pubkeyHex, privateKey) } + fake.deviceBoundDecryptCalls = 0 + fake.lockState = DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true) + fake.failDeviceBoundDecrypts = Int.MAX_VALUE + + val thrown = assertThrows(KeystoreDeviceLockedException::class.java) { + runBlocking { storage.retrievePrivateKey(pubkeyHex) } + } + assertTrue(thrown.deviceReportsLocked) + assertEquals(KeystoreManager.KEYS_ALIAS_DEVICE_BOUND, thrown.alias) + // Genuinely locked is a CORRECT denial: no retry, and the device is + // not branded defective. + assertEquals(1, fake.deviceBoundDecryptCalls) + assertFalse(runBlocking { storage.isMasterKeyLockBindingDefectObserved() }) + } + + @Test + fun shouldNotTreatALockDenialAsAWrongKey() = runBlocking { + storage.storePrivateKey(pubkeyHex, privateKey) + fake.failDeviceBoundDecrypts = Int.MAX_VALUE + + // KeystoreDeviceLockedException IS a GeneralSecurityException, so + // without an explicit clause it would fall into the recovery ladder + // and surface as `null` — which the signer and key-health paths read + // as "this key is stranded, re-derive it". The key is intact; only + // the gate is shut. + assertThrows(KeystoreDeviceLockedException::class.java) { + runBlocking { storage.retrievePrivateKey(pubkeyHex) } + } + // Same reasoning for the health probe: an intact key behind a shut + // gate must not be offered for repair. + assertTrue(storage.probeIdentityKeyRecoverability(pubkeyHex)) + } + + @Test + fun shouldRetryFalseLockedIdentityReadAndSucceedWithoutBrandingTheDevice() = runBlocking { + storage.storePrivateKey(pubkeyHex, privateKey) + fake.deviceBoundDecryptCalls = 0 + fake.failDeviceBoundDecrypts = 1 // one transient blip + + assertArrayEquals(privateKey, storage.retrievePrivateKey(pubkeyHex)) + assertEquals(2, fake.deviceBoundDecryptCalls) + assertFalse(storage.isMasterKeyLockBindingDefectObserved()) + assertEquals(0, fake.unboundEncryptCalls) + } + + @Test + fun shouldRecordDefectWhenFalseLockedIdentityReadRetriesExhaust() = runBlocking { + storage.storePrivateKey(pubkeyHex, privateKey) + fake.deviceBoundDecryptCalls = 0 + fake.failDeviceBoundDecrypts = Int.MAX_VALUE + + assertThrows(KeystoreDeviceLockedException::class.java) { + runBlocking { storage.retrievePrivateKey(pubkeyHex) } + } + // One attempt plus the full DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS + // schedule (3 delays). + assertEquals(4, fake.deviceBoundDecryptCalls) + assertTrue(storage.isMasterKeyLockBindingDefectObserved()) + } + + @Test + fun shouldWriteNewIdentityKeysToTheUnboundAliasOnceDefectIsOnRecord() = runBlocking { + recordDefectViaMnemonicWrite() + fake.unboundEncryptCalls = 0 + fake.deviceBoundEncryptCalls = 0 + + storage.storePrivateKey(pubkeyHex, privateKey) + + // The lock-bound alias is not consulted at all, so this key can never + // be taken hostage by the defective gate. + assertEquals(1, fake.unboundEncryptCalls) + assertEquals(0, fake.deviceBoundEncryptCalls) + assertArrayEquals(privateKey, storage.retrievePrivateKey(pubkeyHex)) + assertEquals(0, fake.deviceBoundDecryptCalls) + } + + @Test + fun shouldRewrapStrandedIdentityKeyOffTheDefectiveGateOnFirstReadThatSucceeds() = runBlocking { + // The MO-972 field shape: the key was stored BEFORE the device's + // defect was known, so it sits under the lock-bound alias. + storage.storePrivateKey(pubkeyHex, privateKey) + assertEquals(1, fake.deviceBoundEncryptCalls) + fake.unboundEncryptCalls = 0 + + // Session 1 — the gate is jammed. Signing fails, but registers the + // device. Only a read can discover this: the key is already stored, + // so nothing writes an identity key again. + fake.failDeviceBoundDecrypts = Int.MAX_VALUE + assertThrows(KeystoreDeviceLockedException::class.java) { + runBlocking { storage.retrievePrivateKey(pubkeyHex) } + } + assertTrue(storage.isMasterKeyLockBindingDefectObserved()) + assertEquals(0, fake.unboundEncryptCalls) + + // Session 2 — the gate lets a read through. The record armed the + // re-wrap, which moves the blob to the never-lock-bound alias. + fake.failDeviceBoundDecrypts = 0 + assertArrayEquals(privateKey, storage.retrievePrivateKey(pubkeyHex)) + assertEquals(1, fake.unboundEncryptCalls) + + // From here signing never touches the defective gate again — even + // while it is jammed solid. + fake.deviceBoundDecryptCalls = 0 + fake.failDeviceBoundDecrypts = Int.MAX_VALUE + assertArrayEquals(privateKey, storage.retrievePrivateKey(pubkeyHex)) + assertEquals(0, fake.deviceBoundDecryptCalls) + assertTrue(fake.unboundDecryptCalls >= 1) + } + + @Test + fun shouldKeepStrandedKeyIntactWhenRewrapFails() = runBlocking { + storage.storePrivateKey(pubkeyHex, privateKey) + recordDefectViaMnemonicWrite() + fake.unboundEncryptCalls = 0 + + // Re-wrap is best-effort: its failure must neither fail the read nor + // corrupt the blob, and the next read tries again. + fake.failUnboundEncrypts = 1 + assertArrayEquals(privateKey, storage.retrievePrivateKey(pubkeyHex)) + assertNotNull(storage.retrievePrivateKey(pubkeyHex)) // retried re-wrap lands + + fake.deviceBoundDecryptCalls = 0 + assertArrayEquals(privateKey, storage.retrievePrivateKey(pubkeyHex)) + assertEquals(0, fake.deviceBoundDecryptCalls) // now on the unbound alias + } +} + +/** + * Fake [KeystoreManager] for the DEVICE_BOUND identity aliases plus the + * master alias (needed only to drive the write ladder that records the + * defect). + * + * [KeystoreManager.KEYS_ALIAS_DEVICE_BOUND] is modeled per its real + * contract — lock-bound, so deniable — and raises the TYPED + * [KeystoreDeviceLockedException] the production `decrypt` now produces for + * it. [KeystoreManager.KEYS_ALIAS_DEVICE_BOUND_UNBOUND] is modeled per ITS + * contract: never lock-bound, so never denied by any lock state. Each blob's + * leading byte marks the alias that produced it and [decrypt] rejects a + * mismatch, so the tests prove reads route to the recorded alias. + */ +private class DeviceBoundLockDefectFakeKeystore : + KeystoreManager(KeySecurityPolicy.DEVICE_BOUND) { + + var lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + + /** Scripted device-locked denials for the LOCK-BOUND identity alias. */ + var failDeviceBoundDecrypts = 0 + + /** Scripted device-locked denials for the master-alias encrypt. */ + var failMasterEncrypts = 0 + + /** Scripted unclassified failures of the unbound-alias encrypt. */ + var failUnboundEncrypts = 0 + + var deviceBoundDecryptCalls = 0 + var unboundDecryptCalls = 0 + var deviceBoundEncryptCalls = 0 + var unboundEncryptCalls = 0 + + override fun sampleDeviceLockState(): DeviceLockState = lockState + + override fun effectiveKeySecurityPolicy(): KeySecurityPolicy = keySecurityPolicy + + override fun hasIdentityKeysKey(alias: String): Boolean = isIdentityKeysAlias(alias) + + override fun keysAliasFingerprintOrNull(alias: String): String? = + if (isIdentityKeysAlias(alias)) fpOf(alias) else null + + override fun keysAliasFingerprint(alias: String): String = fpOf(alias) + + override fun hasLegacyKeysKey(): Boolean = false + + override fun hasLegacyRsaKeysKey(): Boolean = false + + override fun decryptLegacyKeysBlob(blob: EncryptedBlob): ByteArray? = null + + override fun decryptLegacyRsaKeysBlob(blob: EncryptedBlob): ByteArray? = null + + override fun opensUnderNonGatedDeviceBoundSibling(blob: EncryptedBlob): Boolean = false + + override fun encryptForIdentityKeys(plaintext: ByteArray): KeysAliasEncryptedBlob = + encryptForIdentityKeysAlias(KEYS_ALIAS_DEVICE_BOUND, plaintext) + + override fun encryptForIdentityKeysAlias( + alias: String, + plaintext: ByteArray, + ): KeysAliasEncryptedBlob { + when (alias) { + KEYS_ALIAS_DEVICE_BOUND -> deviceBoundEncryptCalls++ + KEYS_ALIAS_DEVICE_BOUND_UNBOUND -> { + unboundEncryptCalls++ + val scripted = failUnboundEncrypts > 0 + if (scripted) failUnboundEncrypts-- + check(!scripted) { "scripted unbound-alias encrypt failure" } + } + else -> error("fake models only the DEVICE_BOUND identity aliases, got '$alias'") + } + return KeysAliasEncryptedBlob(rsaBlob(alias, plaintext), fpOf(alias), alias) + } + + override fun encrypt(plaintext: ByteArray, alias: String): EncryptedBlob = when (alias) { + MASTER_ALIAS -> { + val scripted = failMasterEncrypts > 0 + if (scripted) failMasterEncrypts-- + if (scripted || lockState.isDeviceLocked) { + throw KeystoreDeviceLockedException( + alias = alias, + operation = "encrypt", + lockState = sampleDeviceLockState(), + ) + } + EncryptedBlob(iv = ByteArray(12) { 9 }, ciphertext = plaintext.copyOf()) + } + MASTER_ALIAS_UNBOUND -> + EncryptedBlob(iv = ByteArray(12) { 8 }, ciphertext = plaintext.copyOf()) + else -> error("fake models only the master aliases for AES, got '$alias'") + } + + override fun decrypt(blob: EncryptedBlob, alias: String): ByteArray { + when (alias) { + KEYS_ALIAS_DEVICE_BOUND -> { + deviceBoundDecryptCalls++ + val scripted = failDeviceBoundDecrypts > 0 + if (scripted) failDeviceBoundDecrypts-- + if (scripted || lockState.isDeviceLocked) { + // What the production decrypt now raises for this alias: + // it is lock-bound and NOT auth-gated, so Keystore's + // "user not authenticated" can only be the lock gate. + throw KeystoreDeviceLockedException( + alias = alias, + operation = "decrypt", + lockState = sampleDeviceLockState(), + ) + } + } + KEYS_ALIAS_DEVICE_BOUND_UNBOUND -> unboundDecryptCalls++ + MASTER_ALIAS, MASTER_ALIAS_UNBOUND -> return blob.ciphertext.copyOf() + else -> error("fake cannot decrypt under '$alias'") + } + check(blob.ciphertext[0] == aliasTag(alias)) { + "blob was decrypted under the wrong alias: '$alias' cannot open a blob " + + "produced by tag ${blob.ciphertext[0]}" + } + val len = blob.ciphertext[1].toInt() and 0xFF + return blob.ciphertext.copyOfRange(2, 2 + len) + } + + private fun fpOf(alias: String): String = "fake-fp-$alias" + + private fun aliasTag(alias: String): Byte = + if (alias == KEYS_ALIAS_DEVICE_BOUND) 1 else 2 + + private fun rsaBlob(alias: String, plain: ByteArray): EncryptedBlob { + val ct = ByteArray(RSA_BLOB_BYTES) + ct[0] = aliasTag(alias) + ct[1] = plain.size.toByte() + plain.copyInto(ct, 2) + return EncryptedBlob(iv = ByteArray(0), ciphertext = ct) + } + + private companion object { + const val RSA_BLOB_BYTES = 2048 / 8 + } +} From 4a3c4e3dce90df0ba55304b0a2b128e6985b3aeb Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 10 Sep 2026 13:47:14 -0700 Subject: [PATCH 4/8] docs(kotlin-sdk): widen the device-locked exception KDoc to the alias it now covers The class doc still named MASTER_ALIAS as the only thrower and excluded 'the auth-gated identity-key aliases' as a group. Both went stale in the previous commit: KEYS_ALIAS_DEVICE_BOUND now throws it too, and the exclusion is specifically KEYS_ALIAS_AUTH_GATED (both gates, ambiguous) plus the *_UNBOUND aliases (neither gate, so not a lock denial at all). Co-Authored-By: Claude Opus 5 --- .../security/KeystoreDeviceLockedException.kt | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt index 97c233a7b44..2aa0d6516a8 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt @@ -23,13 +23,14 @@ data class DeviceLockState( /** * The Android Keystore denied an operation on a lock-screen-bound key * because ITS device-locked tracking says the device is locked — thrown by - * [KeystoreManager.encrypt] / [KeystoreManager.decrypt] for the - * [KeystoreManager.MASTER_ALIAS] AES key (which carries - * `setUnlockedDeviceRequired(true)` on lock-screen devices and NO - * `setUserAuthenticationRequired` gate, so a Keystore "user not - * authenticated" denial there can only mean the device-locked gate), and by - * the `PlatformWalletManager.createWallet` pre-check before any native - * wallet exists. + * [KeystoreManager.encrypt] / [KeystoreManager.decrypt] for the aliases + * that carry `setUnlockedDeviceRequired(true)` on lock-screen devices and + * NO `setUserAuthenticationRequired` gate, so a Keystore "user not + * authenticated" denial there can only mean the device-locked gate + * ([KeystoreManager.MASTER_ALIAS], the AES key; and + * [KeystoreManager.KEYS_ALIAS_DEVICE_BOUND], the non-auth-gated identity + * keypair — MO-972), and by the `PlatformWalletManager.createWallet` + * pre-check before any native wallet exists. * * **RETRYABLE AFTER UNLOCK.** This is never a permanent failure of the key * or the data: the exact same operation succeeds once the Keystore @@ -56,10 +57,12 @@ data class DeviceLockState( * escaping this exception false-locked now means even that degradation * failed (see the suppressed exception). * - * NOT used for the auth-gated identity-key aliases: their - * `UserNotAuthenticatedException` means "auth window closed" and keeps its - * own prompt-and-retry contract via `BiometricGate` (see - * [KeystoreManager.decrypt]). + * NOT used for [KeystoreManager.KEYS_ALIAS_AUTH_GATED]: it carries BOTH + * gates, so the same `UserNotAuthenticatedException` may equally mean + * "auth window closed", and only that reading is fixable by prompting — + * it keeps its prompt-and-retry contract via `BiometricGate` (see + * [KeystoreManager.decrypt]). Nor for the `*_UNBOUND` aliases, which carry + * neither gate, so a denial there is not a lock denial at all. */ class KeystoreDeviceLockedException( /** Keystore alias whose operation was denied (or would be, for the pre-check). */ From c1ef36c9494958dde3f303c3e79460a116dec73d Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 10 Sep 2026 19:29:54 -0700 Subject: [PATCH 5/8] fix(kotlin-sdk)!: close the re-wrap write races and unpublish the preflight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from automated review of c574e2994f, all confirmed against the branch before fixing. BLOCKING — the opportunistic re-wrap could resurrect a deleted mnemonic. retrieveMnemonicUtf8 takes a DataStore snapshot, decrypts, and only then edits; it holds no mnemonic lock, so deleteMnemonic or a newer storeMnemonic can land in between. storeMnemonicUnbound wrote unconditionally, so the stale ciphertext went back in — restoring a seed the user had just destroyed, or clobbering a newer one. The read path now uses a compare-and-set edit that fires only while the entry still holds the exact encoded blob it read AND is still untagged (an untagged entry being the lock-bound MASTER_ALIAS default). Both races become a no-op, which is correct: the racing writer already wrote the state the user asked for. storeMnemonic's own writes stay unconditional — they ARE the user's intent. This is the migrateToPolicyAlias discipline applied to mnemonics. Cancellation during the re-wrap could strand decrypted seed bytes. The caller owns the plaintext buffer and scrubs it, but only ever receives it by return; rewrapMnemonicUnbound deliberately rethrows CancellationException, so a cancellation inside its suspending store.edit unwound past the return with nobody left to zero the buffer. The read now scrubs before propagating any throwable that prevents the return. Ordinary re-wrap failures never reach it — they stay best-effort inside the helper, exactly as before. ensureMasterKeyNotLockBlocked is now internal. The previous commit made it suspend (it consults the durable defect record, a suspending DataStore read) without acknowledging that WalletStorage is public and the JVM signature gains a Continuation — a source AND binary break. It is a createWallet preflight helper that was never meant to be API: the only production caller is PlatformWalletManager.createWallet, already suspend; the KotlinExampleApp and dash-wallet never call it. Narrowing it is itself breaking, hence the "!" — every other WalletStorage method the example app uses (retrieveMnemonic, storePrivateKey, hasPrivateKey, listEntryNames, listWalletIdsWithMnemonic) was already suspend and is untouched. Three regression tests, each verified to FAIL with its fix defeated and pass with it restored: delete-during-re-wrap, overwrite-during-re-wrap, and cancel-during-re-wrap. A test hook fires inside the unbound encrypt, which is precisely the window between the read's snapshot and the re-wrap's edit. 444 tests, 0 failures, debug and release. Co-Authored-By: Claude Opus 5 --- .../dashsdk/security/WalletStorage.kt | 59 ++++++++++++- .../WalletStorageDeviceLockedRetryTest.kt | 87 ++++++++++++++++++- 2 files changed, 141 insertions(+), 5 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt index 6e28f95baaa..3340e078e46 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt @@ -247,7 +247,7 @@ class WalletStorage( * [MASTER_ALIAS_UNBOUND][KeystoreManager.MASTER_ALIAS_UNBOUND], which * no lock state can deny, so there is nothing to preflight. */ - suspend fun ensureMasterKeyNotLockBlocked(operation: String) { + internal suspend fun ensureMasterKeyNotLockBlocked(operation: String) { val state = keystore.sampleDeviceLockState() if (!state.isDeviceLocked) return if (isMasterKeyLockBindingDefectObserved()) { @@ -595,7 +595,20 @@ class WalletStorage( // set the flag during the decrypt is the exhausted-ladder recorder // above, and that path throws instead of reaching here. if (alias == KeystoreManager.MASTER_ALIAS && prefs[MASTER_LOCK_DEFECT_KEY] == true) { - rewrapMnemonicUnbound(walletId, plain) + try { + rewrapMnemonicUnbound(walletId, plain, encoded) + } catch (t: Throwable) { + // The caller owns [plain] and scrubs it — but only ever + // receives it by RETURN. rewrapMnemonicUnbound deliberately + // rethrows CancellationException (never swallow structured + // concurrency), so a cancellation inside its suspending + // store.edit would unwind past the return and strand decrypted + // seed bytes on the heap with nobody left to zero them. Scrub + // here before propagating. Ordinary re-wrap failures never + // reach this — they stay best-effort inside the helper. + plain.fill(0) + throw t + } } return plain } @@ -612,9 +625,13 @@ class WalletStorage( * as before) and the next successful read simply tries again. Never * scrubs [plain] — the caller owns that buffer. */ - private suspend fun rewrapMnemonicUnbound(walletId: ByteArray, plain: ByteArray) { + private suspend fun rewrapMnemonicUnbound( + walletId: ByteArray, + plain: ByteArray, + sourceEncoded: String, + ) { try { - storeMnemonicUnbound(walletId, plain) + rewrapMnemonicUnboundIfUnchanged(walletId, plain, sourceEncoded) Log.i( TAG, "re-wrapped a lock-bound master-alias mnemonic blob under the " + @@ -635,6 +652,40 @@ class WalletStorage( } } + /** + * Compare-and-set half of [rewrapMnemonicUnbound]: replace the blob ONLY + * while the entry still holds exactly what the read observed. + * + * [retrieveMnemonicUtf8] runs without any mnemonic lock, so between its + * DataStore snapshot and this edit another coroutine can legitimately + * [deleteMnemonic] the wallet or [storeMnemonic] a new phrase over it. An + * unconditional write would then RESURRECT a just-deleted mnemonic, or + * clobber a newer one with the stale ciphertext this read happened to + * decrypt — the atomicity of the edit itself does not cover the + * read-to-write interval. Guarding on the exact encoded blob AND on the + * alias tag still being absent (an untagged entry is the lock-bound + * [KeystoreManager.MASTER_ALIAS] default — see [storeMnemonic]) makes the + * re-wrap a no-op in both races, which is the correct outcome: the winner + * already wrote the state the user asked for. + * + * The migrateToPolicyAlias discipline, applied to mnemonics. + */ + private suspend fun rewrapMnemonicUnboundIfUnchanged( + walletId: ByteArray, + plain: ByteArray, + sourceEncoded: String, + ) { + val blob = keystore.encrypt(plain, KeystoreManager.MASTER_ALIAS_UNBOUND) + store.edit { prefs -> + val stillOriginal = prefs[mnemonicKey(walletId)] == sourceEncoded && + prefs[mnemonicAliasKey(walletId)] == null + if (stillOriginal) { + prefs[mnemonicKey(walletId)] = encode(blob) + prefs[mnemonicAliasKey(walletId)] = KeystoreManager.MASTER_ALIAS_UNBOUND + } + } + } + /** * Whether a mnemonic is stored for [walletId]. Existence-only — never * decrypts, never materializes plaintext (Swift `hasMnemonic(for:)`). diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt index ce3bcec69a7..caee2a81543 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt @@ -2,6 +2,7 @@ package org.dashfoundation.dashsdk.security import androidx.test.core.app.ApplicationProvider import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking @@ -382,6 +383,77 @@ class WalletStorageDeviceLockedRetryTest { assertTrue(fake.unboundDecryptCalls >= 1) } + // ── re-wrap must never outrun a concurrent write ───────────────────── + + /** Put the defect on record without disturbing [walletId]'s own blob. */ + private suspend fun recordDefectViaSiblingWrite() { + fake.failMasterEncrypts = Int.MAX_VALUE + storage.storeMnemonic(siblingWalletId, mnemonic) + fake.failMasterEncrypts = 0 + assertTrue(storage.isMasterKeyLockBindingDefectObserved()) + } + + @Test + fun shouldNotResurrectAMnemonicDeletedDuringTheRewrap() = runBlocking { + storage.storeMnemonic(walletId, mnemonic) + recordDefectViaSiblingWrite() + + // retrieveMnemonicUtf8 holds a snapshot taken before the delete. + // Without a compare-and-set the re-wrap writes that stale ciphertext + // back and resurrects a seed the user just destroyed. + fake.onUnboundEncrypt = { + fake.onUnboundEncrypt = null + runBlocking { storage.deleteMnemonic(walletId) } + } + + storage.retrieveMnemonic(walletId) // the read itself still succeeds + + assertFalse("a deleted mnemonic must stay deleted", storage.hasMnemonic(walletId)) + } + + @Test + fun shouldNotClobberAMnemonicRewrittenDuringTheRewrap() = runBlocking { + val replacement = "zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo wrong" + storage.storeMnemonic(walletId, mnemonic) + recordDefectViaSiblingWrite() + + // Same window, but the racing writer stores a NEW phrase. The stale + // re-wrap must not overwrite it with the one this read decrypted. + fake.onUnboundEncrypt = { + fake.onUnboundEncrypt = null + runBlocking { storage.storeMnemonic(walletId, replacement) } + } + + storage.retrieveMnemonic(walletId) + + assertEquals( + "the newer mnemonic must survive the stale re-wrap", + replacement, + storage.retrieveMnemonic(walletId), + ) + } + + @Test + fun shouldScrubPlaintextWhenCancelledDuringTheRewrap() { + runBlocking { + storage.storeMnemonic(walletId, mnemonic) + recordDefectViaSiblingWrite() + } + fake.lastMasterDecryptRef = null + + // Cancellation inside the re-wrap unwinds PAST the return, so the + // caller never receives the buffer and can never scrub it. + fake.onUnboundEncrypt = { + fake.onUnboundEncrypt = null + throw CancellationException("cancelled mid-re-wrap") + } + + assertThrows(CancellationException::class.java) { + runBlocking { storage.retrieveMnemonicUtf8(walletId) } + } + assertBufferScrubbed(fake.lastMasterDecryptRef) + } + // ── storeMnemonic plaintext-buffer scrubbing ───────────────────────── @Test @@ -507,6 +579,16 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { /** Invoked at each master encrypt attempt (test synchronization hook). */ var onMasterEncrypt: (() -> Unit)? = null + /** + * Invoked at each UNBOUND-alias encrypt, i.e. inside the re-wrap and + * BEFORE its `store.edit` — the exact window in which a concurrent + * delete/overwrite must be able to win. + */ + var onUnboundEncrypt: (() -> Unit)? = null + + /** The buffer the last master decrypt handed back (scrub evidence). */ + var lastMasterDecryptRef: ByteArray? = null + override fun sampleDeviceLockState(): DeviceLockState = lockState override fun encrypt(plaintext: ByteArray, alias: String): EncryptedBlob = when (alias) { @@ -529,6 +611,7 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { MASTER_ALIAS_UNBOUND -> { unboundEncryptCalls++ lastUnboundPlaintextRef = plaintext + onUnboundEncrypt?.invoke() val scriptedFailure = failUnboundEncrypts > 0 if (scriptedFailure) failUnboundEncrypts-- check(!scriptedFailure) { "scripted unbound-alias encrypt failure" } @@ -562,7 +645,9 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { "blob was decrypted under the wrong alias: '$alias' cannot open a blob " + "whose iv marker is ${blob.iv.firstOrNull()}" } - return blob.ciphertext.copyOf() + return blob.ciphertext.copyOf().also { + if (alias == MASTER_ALIAS) lastMasterDecryptRef = it + } } private fun blob(ivMarker: Byte, plaintext: ByteArray) = From 728ae411bebd79481f9c350485f72f277358b139 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 10 Sep 2026 23:35:57 -0700 Subject: [PATCH 6/8] fix(kotlin-sdk): pin the defect record to the device, scrub identity keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more findings from automated review of c1ef36c949. The durable defect record was trusted on its own, and it is portable. MASTER_LOCK_DEFECT_KEY is an ordinary DataStore boolean, but it is what authorizes writing secrets without the unlocked-device gate. A host app permitting Android backup or device-to-device transfer could carry that preference to a different handset, where — Keystore keys being neither backed up nor restored — a healthy device would inherit the downgrade having never demonstrated the defect. Neither known consumer is exposed (dash-wallet sets allowBackup=false, fullBackupContent=false and excludes every domain from both cloud-backup and device-transfer; KotlinExampleApp sets allowBackup=false), but this SDK ships to Maven Central and a third-party host is not bound by either. The decision is now two-part and needs BOTH halves: the flag says the defect was seen, and KeystoreManager.hasUnboundMasterKey() says it was seen HERE. A Keystore key cannot travel, so requiring MASTER_ALIAS_UNBOUND to exist locally pins the downgrade to the device that earned it. The write-heal path already provisions that alias; the read-side recorder holds no plaintext to encrypt, so it now creates the key explicitly (the ensureMasterKeyNotLockBlocked probe-encrypt idiom) and records nothing if that fails. A flag arriving without its key is simply not believed — and deliberately not cleared, because writing from a read path is what the re-wrap races taught us to avoid, and an inert flag costs nothing. migrateToPolicyAlias rethrew CancellationException without scrubbing. Same defect class as the mnemonic re-wrap fixed in c1ef36c949, on the identity-key side — fixing only half the pattern was the oversight. Every caller hands the helper the plaintext it is about to return, so a cancellation unwinds past that return with nobody left to zero it. Scrubbing inside the helper covers all three callers at once: the legacy migration, the recovery ladder, and the defective-gate re-wrap. Two regression tests, each verified to FAIL with its fix defeated: shouldIgnoreADefectFlagWithoutItsDeviceLocalKeystoreWitness models the restore (flag kept, Keystore key gone) and asserts writes return to the lock-bound alias; shouldScrubIdentityKeyPlaintextWhenCancelledDuringMigration cancels inside the unbound identity encrypt and asserts the decrypted key is zeroed. One existing expectation updated — the denied read now performs one extra unbound encrypt to provision the witness, and still cannot re-wrap the blob because it never obtained the plaintext. 446 tests, 0 failures, debug and release. Co-Authored-By: Claude Opus 5 --- .../dashsdk/security/KeystoreManager.kt | 16 ++++++ .../dashsdk/security/WalletStorage.kt | 33 +++++++++++- .../WalletStorageDeviceLockedRetryTest.kt | 45 ++++++++++++++++- .../WalletStorageIdentityKeyLockDefectTest.kt | 50 ++++++++++++++++++- 4 files changed, 140 insertions(+), 4 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt index 1cbeb113101..f5dd8ac04e7 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt @@ -475,6 +475,22 @@ open class KeystoreManager( open fun hasLegacyKeysKey(): Boolean = (androidKeyStore().getKey(KEYS_ALIAS, null) as? SecretKey) != null + /** + * Whether the never-lock-bound [MASTER_ALIAS_UNBOUND] AES key exists in + * THIS device's Keystore. + * + * Device-local evidence that the false-locked defect was demonstrated + * HERE. Keystore keys are non-exportable and never restored by Android + * backup or device-to-device transfer, so unlike the DataStore flag that + * records the defect this cannot travel to another handset — which is + * exactly what [WalletStorage.isMasterKeyLockBindingDefectObserved] + * needs to avoid authorizing the lock-gate downgrade on a healthy device + * that merely inherited a restored preference. Presence check only: no + * crypto, no prompt, and it never generates the key. + */ + open fun hasUnboundMasterKey(): Boolean = + (androidKeyStore().getKey(MASTER_ALIAS_UNBOUND, null) as? SecretKey) != null + /** * Whether [KEYS_ALIAS] currently holds the **former RSA identity-keys * keypair** from the pre-alias-split scheme (dashpay/platform#4060) — the diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt index 3340e078e46..cda484ee34b 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt @@ -372,6 +372,19 @@ class WalletStorage( * the device ([recordLockBindingDefectFromDeniedRead]) — the only * route on a wallet whose blob predates the degradation. * + * Two-part evidence, and BOTH halves are required. The durable flag says + * the defect was seen; [KeystoreManager.hasUnboundMasterKey] says it was + * seen *on this device*. The flag is an ordinary DataStore boolean, so a + * host app that permits Android backup or device-to-device transfer could + * carry it to a different handset — where, trusted alone, it would + * authorize the lock-gate downgrade on a healthy phone that never + * demonstrated anything. Keystore keys are non-exportable and never + * restored, so requiring the unbound alias to exist locally pins the + * decision to the device that earned it. A flag arriving without its key + * is simply not believed; it is deliberately NOT cleared here, because + * this is a read and writing from a read path is what the re-wrap races + * taught us to avoid — an inert flag costs nothing. + * * Never cleared by any targeted mutator — the defect is a property of * the device's OS build, not of any wallet, and a healed device * staying healed costs nothing on a healthy one, which never sets it. @@ -382,7 +395,8 @@ class WalletStorage( * [KeystoreManager.effectiveKeySecurityPolicy] discipline. */ suspend fun isMasterKeyLockBindingDefectObserved(): Boolean = - store.data.first()[MASTER_LOCK_DEFECT_KEY] == true + store.data.first()[MASTER_LOCK_DEFECT_KEY] == true && + keystore.hasUnboundMasterKey() /** * Write [plaintext]'s blob under the never-lock-bound @@ -526,6 +540,14 @@ class WalletStorage( denial, ) try { + // Provision the device-local witness in the same breath as the + // flag. [isMasterKeyLockBindingDefectObserved] requires BOTH, so a + // flag without this Keystore key is inert — and unlike the + // write-heal path this one holds no plaintext to encrypt, so the + // key has to be created explicitly (the + // ensureMasterKeyNotLockBlocked probe-encrypt idiom). If it cannot + // be created, nothing is recorded and the next read retries. + keystore.encrypt(ByteArray(1), KeystoreManager.MASTER_ALIAS_UNBOUND) store.edit { it[MASTER_LOCK_DEFECT_KEY] = true } } catch (e: CancellationException) { throw e @@ -1227,6 +1249,15 @@ class WalletStorage( // coroutine was cancelled during the encrypt / store.edit suspend // points, rethrow so the cancellation propagates. Only genuine // rewrite failures below stay best-effort (retry on the next read). + // + // Scrub first. Every caller hands us the plaintext it is about to + // RETURN, and a cancellation here unwinds past that return, so the + // owner never gets the chance to zero it — stranding a decrypted + // identity key on the heap. The mnemonic re-wrap takes the same + // precaution; doing it inside this helper covers all three callers + // (legacy migration, recovery ladder, and the defective-gate + // re-wrap) at once. + plain.fill(0) throw cancellation } catch (_: Throwable) { // Best-effort: a rewrite failure must not lose the value the caller diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt index caee2a81543..94050db301a 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt @@ -367,11 +367,16 @@ class WalletStorageDeviceLockedRetryTest { runBlocking { storage.retrieveMnemonicUtf8(walletId) } } assertTrue(storage.isMasterKeyLockBindingDefectObserved()) - assertEquals(0, fake.unboundEncryptCalls) + // The denied read provisions the device-local witness (one unbound + // encrypt of a probe byte) but CANNOT re-wrap the blob — it never got + // the plaintext — so the entry is still under the lock-bound alias. + assertEquals(1, fake.unboundEncryptCalls) + assertEquals(0, fake.unboundDecryptCalls) // Session 2 — the gate lets a read through (e.g. after a credential // unlock). The record armed the re-wrap, which now fires. fake.failMasterDecrypts = 0 + fake.unboundEncryptCalls = 0 assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) assertEquals(1, fake.unboundEncryptCalls) @@ -383,6 +388,32 @@ class WalletStorageDeviceLockedRetryTest { assertTrue(fake.unboundDecryptCalls >= 1) } + @Test + fun shouldIgnoreADefectFlagWithoutItsDeviceLocalKeystoreWitness() = runBlocking { + // Earn the defect record on "this" device. + fake.failMasterEncrypts = Int.MAX_VALUE + storage.storeMnemonic(walletId, mnemonic) + fake.failMasterEncrypts = 0 + assertTrue(storage.isMasterKeyLockBindingDefectObserved()) + + // Now model a restore onto a DIFFERENT handset: Android can carry the + // DataStore preference, but never the Keystore key. A healthy phone + // must not inherit the lock-gate downgrade from a portable boolean. + fake.unboundKeyProvisioned = false + + assertFalse( + "a flag without its device-local witness must not be believed", + storage.isMasterKeyLockBindingDefectObserved(), + ) + + // ...and the write path must go back to the lock-bound alias. + fake.unboundEncryptCalls = 0 + fake.masterEncryptCalls = 0 + storage.storeMnemonic(siblingWalletId, mnemonic) + assertEquals(1, fake.masterEncryptCalls) + assertEquals(0, fake.unboundEncryptCalls) + } + // ── re-wrap must never outrun a concurrent write ───────────────────── /** Put the defect on record without disturbing [walletId]'s own blob. */ @@ -567,6 +598,15 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { /** Whether the fake master key carries the unlocked-device requirement. */ var masterKeyLockBound = true + /** + * Whether MASTER_ALIAS_UNBOUND exists in this fake's Keystore — the + * device-local witness. Set by any unbound-alias encrypt (which + * provisions the key for real), and clearable to model a DataStore + * restored onto a DIFFERENT device, where the preference survives but + * the Keystore key cannot. + */ + var unboundKeyProvisioned = false + /** The exact buffer reference the last master encrypt received. */ var lastMasterPlaintextRef: ByteArray? = null @@ -591,6 +631,8 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { override fun sampleDeviceLockState(): DeviceLockState = lockState + override fun hasUnboundMasterKey(): Boolean = unboundKeyProvisioned + override fun encrypt(plaintext: ByteArray, alias: String): EncryptedBlob = when (alias) { MASTER_ALIAS -> { masterEncryptCalls++ @@ -612,6 +654,7 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { unboundEncryptCalls++ lastUnboundPlaintextRef = plaintext onUnboundEncrypt?.invoke() + unboundKeyProvisioned = true val scriptedFailure = failUnboundEncrypts > 0 if (scriptedFailure) failUnboundEncrypts-- check(!scriptedFailure) { "scripted unbound-alias encrypt failure" } diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt index 720f8fd4e92..aab8175f970 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt @@ -181,6 +181,34 @@ class WalletStorageIdentityKeyLockDefectTest { assertTrue(fake.unboundDecryptCalls >= 1) } + @Test + fun shouldScrubIdentityKeyPlaintextWhenCancelledDuringMigration() { + runBlocking { + storage.storePrivateKey(pubkeyHex, privateKey) + recordDefectViaMnemonicWrite() + } + fake.lastIdentityDecryptRef = null + + // migrateToPolicyAlias rethrows CancellationException by design, so a + // cancellation inside it unwinds PAST retrievePrivateKey's return and + // the owner never gets the buffer to scrub. Covers the legacy + // migration and recovery ladder too — they share the helper. + fake.onUnboundIdentityEncrypt = { + fake.onUnboundIdentityEncrypt = null + throw kotlin.coroutines.cancellation.CancellationException("cancelled mid-migration") + } + + assertThrows(kotlin.coroutines.cancellation.CancellationException::class.java) { + runBlocking { storage.retrievePrivateKey(pubkeyHex) } + } + val buf = fake.lastIdentityDecryptRef + assertNotNull("expected the identity-key plaintext to have been captured", buf) + assertTrue( + "decrypted identity key must be zeroed when it cannot be returned", + buf!!.all { it == 0.toByte() }, + ) + } + @Test fun shouldKeepStrandedKeyIntactWhenRewrapFails() = runBlocking { storage.storePrivateKey(pubkeyHex, privateKey) @@ -233,6 +261,21 @@ private class DeviceBoundLockDefectFakeKeystore : override fun sampleDeviceLockState(): DeviceLockState = lockState + /** The device-local witness: provisioned by any unbound-alias encrypt. */ + var unboundMasterKeyProvisioned = false + + /** + * Invoked inside the UNBOUND identity-alias encrypt — i.e. inside + * `migrateToPolicyAlias`, after the plaintext is in hand and before the + * caller can return it. + */ + var onUnboundIdentityEncrypt: (() -> Unit)? = null + + /** The buffer the last identity decrypt handed back (scrub evidence). */ + var lastIdentityDecryptRef: ByteArray? = null + + override fun hasUnboundMasterKey(): Boolean = unboundMasterKeyProvisioned + override fun effectiveKeySecurityPolicy(): KeySecurityPolicy = keySecurityPolicy override fun hasIdentityKeysKey(alias: String): Boolean = isIdentityKeysAlias(alias) @@ -263,6 +306,7 @@ private class DeviceBoundLockDefectFakeKeystore : KEYS_ALIAS_DEVICE_BOUND -> deviceBoundEncryptCalls++ KEYS_ALIAS_DEVICE_BOUND_UNBOUND -> { unboundEncryptCalls++ + onUnboundIdentityEncrypt?.invoke() val scripted = failUnboundEncrypts > 0 if (scripted) failUnboundEncrypts-- check(!scripted) { "scripted unbound-alias encrypt failure" } @@ -285,8 +329,10 @@ private class DeviceBoundLockDefectFakeKeystore : } EncryptedBlob(iv = ByteArray(12) { 9 }, ciphertext = plaintext.copyOf()) } - MASTER_ALIAS_UNBOUND -> + MASTER_ALIAS_UNBOUND -> { + unboundMasterKeyProvisioned = true EncryptedBlob(iv = ByteArray(12) { 8 }, ciphertext = plaintext.copyOf()) + } else -> error("fake models only the master aliases for AES, got '$alias'") } @@ -316,7 +362,7 @@ private class DeviceBoundLockDefectFakeKeystore : "produced by tag ${blob.ciphertext[0]}" } val len = blob.ciphertext[1].toInt() and 0xFF - return blob.ciphertext.copyOfRange(2, 2 + len) + return blob.ciphertext.copyOfRange(2, 2 + len).also { lastIdentityDecryptRef = it } } private fun fpOf(alias: String): String = "fake-fp-$alias" From 0e7c28fe6ade8e3ccd61632b31b413d5a5c8a5cb Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Fri, 11 Sep 2026 19:50:30 -0700 Subject: [PATCH 7/8] fix(kotlin-sdk): close the last unwitnessed path and stop vouching for mismatched blobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings on 728ae411be, the first raised independently by both reviewers. The read-path re-wrap was the one place still trusting the portable flag alone. Every other consumer went through isMasterKeyLockBindingDefectObserved(), which now demands the device-local witness, but retrieveMnemonicUtf8 read MASTER_LOCK_DEFECT_KEY straight off its snapshot. That mattered more than a missed refactor: the re-wrap encrypts under MASTER_ALIAS_UNBOUND, which PROVISIONS that alias — so a restored flag reaching this line would mint the very evidence the gate checks it against, and the downgrade would authorize itself on a healthy device. The witness is now required before the re-wrap runs, so it can no longer bootstrap. It stays a non-suspending Keystore presence check, so the hot resolver path still pays no second DataStore read. probeOpensBlob returned true unconditionally for a device-locked denial. The previous commit justified that by arguing such a denial "proves nothing about ownership either way" — which is precisely the argument for honouring unaeProvesRecoverable, not for overriding it. Like UNAE, a device-locked denial is thrown at cipher.init BEFORE the ciphertext is examined, so it reports the gate, not the key. Where the caller has independent proof of ownership (stored fingerprint matches the recorded alias) the blob really is intact behind a shut gate; where it does not, returning true reported a blob belonging to a REPLACED key as healthy and suppressed the re-derive the key-health sheet exists to offer. It now obeys the same flag as every other pre-ciphertext throw. Two regression tests, each verified to FAIL with its fix defeated: shouldNotLetARestoredFlagMintItsOwnWitnessViaTheRewrap asserts the restored flag performs zero unbound encrypts, and shouldNotReportALockDeniedBlobRecoverableWhenTheFingerprintMismatches rotates the alias fingerprint under a lock-denied read. 448 tests, 0 failures, debug and release. Co-Authored-By: Claude Opus 5 --- .../dashsdk/security/WalletStorage.kt | 44 ++++++++++++------- .../WalletStorageDeviceLockedRetryTest.kt | 24 ++++++++++ .../WalletStorageIdentityKeyLockDefectTest.kt | 24 +++++++++- 3 files changed, 75 insertions(+), 17 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt index cda484ee34b..ac22f38d88d 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt @@ -611,12 +611,23 @@ class WalletStorage( recordLockBindingDefectFromDeniedRead("retrieveMnemonicUtf8", denial) }, ) - // Deliberately the PRE-decrypt snapshot: this is the hot resolver - // path (Rust calls it synchronously for every derivation), so it must - // not pay a second DataStore read. The only writer that could have - // set the flag during the decrypt is the exhausted-ladder recorder + // Deliberately the PRE-decrypt snapshot for the flag: this is the hot + // resolver path (Rust calls it synchronously for every derivation), so + // it must not pay a second DataStore read. The only writer that could + // have set the flag during the decrypt is the exhausted-ladder recorder // above, and that path throws instead of reaching here. - if (alias == KeystoreManager.MASTER_ALIAS && prefs[MASTER_LOCK_DEFECT_KEY] == true) { + // + // The witness is checked HERE rather than inherited from the flag: the + // re-wrap encrypts under MASTER_ALIAS_UNBOUND, which PROVISIONS that + // alias, so a restored flag reaching this line would mint its own + // evidence and the device-local gate would authorize itself. Requiring + // the witness first makes that impossible. hasUnboundMasterKey is a + // non-suspending Keystore presence check, so the hot path still pays + // no second DataStore read. + if (alias == KeystoreManager.MASTER_ALIAS && + prefs[MASTER_LOCK_DEFECT_KEY] == true && + keystore.hasUnboundMasterKey() + ) { try { rewrapMnemonicUnbound(walletId, plain, encoded) } catch (t: Throwable) { @@ -1464,17 +1475,18 @@ class WalletStorage( } else { false } - } catch (e: KeystoreDeviceLockedException) { - // The device's lock gate is shut (genuinely, or the false-locked - // defect). The blob and its key are intact and open as soon as - // the gate lets go, so this is RECOVERABLE — unconditionally, - // unlike UNAE. The [unaeProvesRecoverable] caveat exists because - // a closed AUTH gate hides WHICH key was asked; a device-locked - // denial carries its alias and proves nothing about ownership - // either way, so reporting "strandable" here would offer a - // re-derive for a perfectly good key. - true - } catch (e: UserNotAuthenticatedException) { + } catch (_: KeystoreDeviceLockedException) { + // Same epistemic status as UNAE, so it obeys the same flag. A + // device-locked denial is thrown at `cipher.init`, BEFORE the + // ciphertext is examined, so it says the gate is shut and nothing + // about whether this alias actually wrote the blob. When the + // caller has independent proof of ownership (the stored + // fingerprint matches the recorded alias) the key really is intact + // behind a shut gate — recoverable. When it does not, returning + // true would report a blob belonging to a REPLACED key as healthy + // and suppress the re-derive the key-health sheet must offer. + unaeProvesRecoverable + } catch (_: UserNotAuthenticatedException) { unaeProvesRecoverable } catch (e: GeneralSecurityException) { false diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt index 94050db301a..a3945574e03 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt @@ -388,6 +388,30 @@ class WalletStorageDeviceLockedRetryTest { assertTrue(fake.unboundDecryptCalls >= 1) } + @Test + fun shouldNotLetARestoredFlagMintItsOwnWitnessViaTheRewrap() = runBlocking { + storage.storeMnemonic(walletId, mnemonic) + fake.failMasterEncrypts = Int.MAX_VALUE + storage.storeMnemonic(siblingWalletId, mnemonic) // earns the record + fake.failMasterEncrypts = 0 + + // Model the restore: the preference survived, the Keystore key did not. + fake.unboundKeyProvisioned = false + fake.unboundEncryptCalls = 0 + + // The re-wrap encrypts under the unbound alias, which PROVISIONS it — + // so if the read path trusted the portable flag alone it would mint + // the very evidence the flag is supposed to be checked against. + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + + assertEquals( + "a restored flag must not be able to create its own witness", + 0, + fake.unboundEncryptCalls, + ) + assertFalse(storage.isMasterKeyLockBindingDefectObserved()) + } + @Test fun shouldIgnoreADefectFlagWithoutItsDeviceLocalKeystoreWitness() = runBlocking { // Earn the defect record on "this" device. diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt index aab8175f970..14775cf51a6 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageIdentityKeyLockDefectTest.kt @@ -105,6 +105,25 @@ class WalletStorageIdentityKeyLockDefectTest { assertTrue(storage.probeIdentityKeyRecoverability(pubkeyHex)) } + @Test + fun shouldNotReportALockDeniedBlobRecoverableWhenTheFingerprintMismatches() = runBlocking { + storage.storePrivateKey(pubkeyHex, privateKey) + + // The alias key was replaced since the blob was written, so the stored + // fingerprint no longer matches: this blob belongs to a key that is + // gone. A device-locked denial is thrown at cipher.init, before the + // ciphertext is ever examined, so it cannot vouch for ownership — + // reporting "recoverable" here would hide the re-derive the key-health + // sheet must offer. + fake.fingerprintSuffix = "-rotated" + fake.failDeviceBoundDecrypts = Int.MAX_VALUE + + assertFalse( + "a pre-ciphertext denial must not vouch for a mismatched blob", + storage.probeIdentityKeyRecoverability(pubkeyHex), + ) + } + @Test fun shouldRetryFalseLockedIdentityReadAndSucceedWithoutBrandingTheDevice() = runBlocking { storage.storePrivateKey(pubkeyHex, privateKey) @@ -365,7 +384,10 @@ private class DeviceBoundLockDefectFakeKeystore : return blob.ciphertext.copyOfRange(2, 2 + len).also { lastIdentityDecryptRef = it } } - private fun fpOf(alias: String): String = "fake-fp-$alias" + /** Appended to every alias fingerprint — flip it to model a rotated key. */ + var fingerprintSuffix: String = "" + + private fun fpOf(alias: String): String = "fake-fp-$alias$fingerprintSuffix" private fun aliasTag(alias: String): Byte = if (alias == KEYS_ALIAS_DEVICE_BOUND) 1 else 2 From 97c5b53a82350c4d202617967f6ebe645ae7fd8e Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Mon, 14 Sep 2026 00:21:22 -0700 Subject: [PATCH 8/8] fix(kotlin-sdk): give the legacy RSA health probe its own ownership evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit probeIdentityKeyRecoverability derived unaeProvesRecoverable for the policy alias but called the legacy rung with no argument at all, so it defaulted to true. The retained former keypair at KEYS_ALIAS is auth-gated, and a closed window throws UserNotAuthenticatedException at cipher.init — before the ciphertext is examined — so after that alias has been regenerated the probe reported a blob the current key can never open as healthy, suppressing the re-derive the key-health sheet exists to offer. It is the same defect the parameter was introduced to prevent, on the one rung that never received it. The legacy rung now compares the stored fingerprint against keysAliasFingerprintOrNull(KEYS_ALIAS). Pre-alias-split blobs carry the FORMER key's fingerprint (retrievePrivateKey rung 3), so that is exactly the ownership test; a blob predating fingerprint recording has no evidence and is treated as strandable, erring toward offering repair for a key that is deterministically re-derivable anyway. Scope note: this rung is pre-existing and outside the PR's diff — both reviewers flagged it while reviewing the surrounding changes. Their stated mechanism does not hold, though: they attributed it to KeystoreDeviceLockedException reaching this path, and it cannot. decryptLegacyRsaKeysBlob runs a raw cipher.init that never calls rethrowClassifyingDeviceLockedDenial, and KEYS_ALIAS is deliberately excluded from UNAMBIGUOUS_LOCK_BOUND_ALIASES because it carries both gates. The reachable route is UNAE, and the fix is the same either way. The fake now models a fingerprint for KEYS_ALIAS, which the real implementation returns from the keypair's certificate — without that the legacy rung had no observable ownership signal to test. authGatedFormerRsaKeyWithRotatedFingerprintIsNotRecoverable pins the rotated case and was verified to fail with the argument removed; the existing authGatedFormerRsaKeyReportsRecoverable still pins the matching case. 449 tests, 0 failures, debug and release. Co-Authored-By: Claude Opus 5 --- .../dashsdk/security/WalletStorage.kt | 19 +++++++++++- .../WalletStorageUpgradeMatrixTest.kt | 29 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt index ac22f38d88d..323a6268d98 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt @@ -1440,13 +1440,30 @@ class WalletStorage( val storedFingerprint = prefs[privateKeyFingerprintKey(pubkeyHex)] val unaeProvesRecoverable = storedFingerprint != null && storedFingerprint == keystore.keysAliasFingerprintOrNull(recordedAlias) + // The legacy rung needs its OWN ownership evidence, for the same reason + // the policy rung above does. The retained former keypair at + // [KeystoreManager.KEYS_ALIAS] is auth-gated, so a closed window throws + // `UserNotAuthenticatedException` at cipher.init — before the ciphertext + // is examined — and a bare default of `true` would vouch for a blob that + // key does not own. After the alias has been regenerated, that reported a + // stale blob as healthy and suppressed the re-derive the key-health sheet + // exists to offer. Pre-alias-split blobs carry the FORMER key's + // fingerprint (see [retrievePrivateKey]'s rung 3), so comparing against + // that alias's current certificate is exactly the ownership test; a blob + // predating fingerprint recording has no evidence and is treated as + // strandable, which errs toward offering a repair for a key that is + // deterministically re-derivable anyway. + val legacyRsaProvesRecoverable = storedFingerprint != null && + storedFingerprint == keystore.keysAliasFingerprintOrNull(KeystoreManager.KEYS_ALIAS) return ( keystore.hasIdentityKeysKey(recordedAlias) && probeOpensBlob(unaeProvesRecoverable) { keystore.decrypt(blob, recordedAlias) } ) || ( keystore.hasLegacyRsaKeysKey() && - probeOpensBlob { keystore.decryptLegacyRsaKeysBlob(blob) } + probeOpensBlob(legacyRsaProvesRecoverable) { + keystore.decryptLegacyRsaKeysBlob(blob) + } ) } diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt index 6201f107360..157e24e6d76 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt @@ -296,6 +296,26 @@ class WalletStorageUpgradeMatrixTest { * auth-gated KEYS_ALIAS RSA key that would open the blob after auth reports * recoverable when the window is closed (UserNotAuth), rather than stranded. */ + /** + * The counterpart of [authGatedFormerRsaKeyReportsRecoverable]: same closed + * window on the former RSA key, but the alias has since been REGENERATED so + * the stored fingerprint no longer matches. `UserNotAuthenticatedException` + * is thrown at cipher.init, before the ciphertext is examined, so it says + * nothing about ownership — and defaulting it to "recoverable" reported a + * blob the current key can never open as healthy, suppressing the + * key-health repair flow. + */ + @Test + fun authGatedFormerRsaKeyWithRotatedFingerprintIsNotRecoverable() = runBlocking { + fake.keysAliasKind = FakeKeystoreManager.KeysAliasKind.RSA + fake.scheme = FakeKeystoreManager.Scheme.FORMER_RSA + storage.storePrivateKey(pub, secret) + + fake.legacyRsaFingerprintSuffix = "-regenerated" + fake.throwAuthOnLegacyRsaDecrypt = true + assertFalse(storage.probeIdentityKeyRecoverability(pub)) + } + @Test fun authGatedFormerRsaKeyReportsRecoverable() = runBlocking { fake.keysAliasKind = FakeKeystoreManager.KeysAliasKind.RSA @@ -628,6 +648,9 @@ private class FakeKeystoreManager : * the alias stays present (and, being auth-gated, usually locked). */ var policyFingerprintSuffix: String = "" + + /** Set to model the former KEYS_ALIAS keypair having been regenerated. */ + var legacyRsaFingerprintSuffix: String = "" var throwAuthOnLegacyRsaDecrypt: Boolean = false var throwInvalidatedOnLegacyRsaDecrypt: Boolean = false var throwInvalidatedOnPolicyDecrypt: Boolean = false @@ -659,6 +682,12 @@ private class FakeKeystoreManager : POLICY_ALIAS -> if (policyKeyProvisioned) fpOf(POLICY_ALIAS) + policyFingerprintSuffix else null KEYS_ALIAS_DEVICE_BOUND -> if (deviceBoundKeyPresent) fpOf(KEYS_ALIAS_DEVICE_BOUND) else null + // The retained former RSA keypair has a certificate too, so the real + // implementation returns a fingerprint for it. Modelling that is what + // lets the legacy rung prove (or disprove) ownership. + KeystoreManager.KEYS_ALIAS -> + if (keysAliasKind == KeysAliasKind.RSA) FP_FORMER_RSA + legacyRsaFingerprintSuffix + else null else -> null }