From b1ab2091da7ef883de45a8bf1654a2ee27267c39 Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Sun, 23 Aug 2026 15:54:15 -0400 Subject: [PATCH 1/4] fix(kotlin-sdk): typed retryable device-locked keystore errors + createWallet fail-fast Field failure (two QA devices, wallet creation, no engine): during createWallet -> WalletStorage.storeMnemonic -> KeystoreManager.encrypt, Android Keystore2 denied the lock-bound MASTER_ALIAS AES operation with UserNotAuthenticatedException because ITS device-locked tracking said "locked" -- sometimes falsely, with the device demonstrably unlocked. The key carries setUnlockedDeviceRequired(true) and NO setUserAuthenticationRequired, so that exception on this alias can only mean the unlocked-device gate. The denial surfaced as an opaque wallet- creation failure after the full native create + rollback dance. Three changes, no key-parameter or alias changes: 1. Typed retryable error: device-locked denials on the non-auth-gated master-alias encrypt/decrypt paths are classified (narrow, name/ message-based, JVM-testable -- the isNoSecureLockScreenKeyGenFailure discipline) and rethrown as KeystoreDeviceLockedException, carrying the alias, the operation, and KeyguardManager.isDeviceLocked / isKeyguardLocked sampled AT THROW TIME, so logs separate a genuine lock from the false-locked Keystore2 defect. Documented as retryable-after-unlock. The auth-gated identity-key aliases are deliberately untouched -- their UserNotAuthenticatedException means "auth window closed" and keeps the BiometricGate prompt-and-retry contract. 2. createWallet fail-fast pre-check: if KeyguardManager.isDeviceLocked is true at entry, throw the typed exception BEFORE the native create -- no native wallet, no Room rows, nothing to roll back. Lock state is threaded the way the file already threads isDeviceSecure: a probe captured against the application context in WalletStorage's companion, handed to KeystoreManager. 3. Bounded false-locked retry in storeMnemonic: a denial whose sampled state says the device is NOT actually locked retries up to 3 times over ~2s (250/750/1000ms) before propagating; a genuinely locked device fails fast with no retry. Unit tests (Robolectric/JVM, module conventions): classifier matrix, mapping + at-throw-time sampling (false-locked vs genuinely locked), pre-check fail-fast (including keyguard-showing-but-not-secured), retry succeeds on 2nd attempt / gives up after 3 retries / no retry when genuinely locked. Co-Authored-By: Claude Fable 5 --- .../security/KeystoreDeviceLockedException.kt | 83 ++++++++ .../dashsdk/security/KeystoreManager.kt | 129 ++++++++++++- .../dashsdk/security/WalletStorage.kt | 115 ++++++++++- .../dashsdk/wallet/PlatformWalletManager.kt | 16 ++ .../KeystoreDeviceLockedDenialTest.kt | 162 ++++++++++++++++ .../WalletStorageDeviceLockedRetryTest.kt | 181 ++++++++++++++++++ 6 files changed, 672 insertions(+), 14 deletions(-) create mode 100644 packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt create mode 100644 packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedDenialTest.kt create mode 100644 packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt 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 new file mode 100644 index 00000000000..d7df7d46939 --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedException.kt @@ -0,0 +1,83 @@ +package org.dashfoundation.dashsdk.security + +import java.security.GeneralSecurityException + +/** + * `KeyguardManager` lock state sampled at a single instant — carried by + * [KeystoreDeviceLockedException] so a log line can distinguish a + * genuinely-locked device from the false-locked Keystore2 defect (see the + * exception's KDoc). + * + * - [isDeviceLocked] — `KeyguardManager.isDeviceLocked`: the device is locked + * AND secured (a credential is needed to get in). This is the signal the + * Keystore's `setUnlockedDeviceRequired` gate is SUPPOSED to track. + * - [isKeyguardLocked] — `KeyguardManager.isKeyguardLocked`: the keyguard + * (lock screen) is showing, secured or not. Can be true while + * [isDeviceLocked] is false (e.g. a non-secure swipe screen). + */ +data class DeviceLockState( + val isDeviceLocked: Boolean, + val isKeyguardLocked: Boolean, +) + +/** + * 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. + * + * **RETRYABLE AFTER UNLOCK.** This is never a permanent failure of the key + * or the data: the exact same operation succeeds once the Keystore + * considers the device unlocked. Callers should retry rather than treat the + * secret as lost. + * + * [lockState] is `KeyguardManager`'s view sampled AT THROW TIME, so field + * logs can separate the two classes this exception covers: + * + * - **Genuinely locked** ([DeviceLockState.isDeviceLocked] true): the + * denial is correct. Retrying before the user unlocks cannot succeed — + * fail fast and retry after the next unlock. + * - **False-locked** ([DeviceLockState.isDeviceLocked] false): the defect + * 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. + * + * 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]). + */ +class KeystoreDeviceLockedException( + /** Keystore alias whose operation was denied (or would be, for the pre-check). */ + val alias: String, + /** The denied operation: `"encrypt"`, `"decrypt"`, or a pre-check name. */ + val operation: String, + /** `KeyguardManager` lock state sampled when this was thrown. */ + val lockState: DeviceLockState, + cause: Throwable? = null, +) : GeneralSecurityException( + "Keystore denied '$operation' on lock-bound alias '$alias' as device-locked; " + + "KeyguardManager at throw time: isDeviceLocked=${lockState.isDeviceLocked} " + + "isKeyguardLocked=${lockState.isKeyguardLocked}" + + (if (lockState.isDeviceLocked) { + " (genuinely locked — retry after unlock)" + } else { + " (FALSE-LOCKED: device reports unlocked but Keystore denied — " + + "Keystore2 lock-state misreporting; retryable immediately)" + }), + cause, +) { + /** + * Whether `KeyguardManager` agreed the device was locked when this was + * thrown. False identifies the false-locked defect class — the device + * was demonstrably unlocked, so a short bounded retry may succeed. + */ + val deviceReportsLocked: Boolean get() = lockState.isDeviceLocked +} 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 e2efbc9e005..0e6cf1642eb 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 @@ -97,6 +97,19 @@ open class KeystoreManager( * with the degradation surfaced via [effectiveKeySecurityPolicy]. */ private val requireAuthGated: Boolean = false, + /** + * Prompt-free sampler of `KeyguardManager`'s CURRENT lock state + * (`isDeviceLocked` / `isKeyguardLocked`). Supplied by [WalletStorage] + * (which holds a `Context`); defaults to "unlocked" for the + * no-`Context` / unit-test construction path. Sampled AT THROW TIME + * when a lock-bound Keystore operation is denied as device-locked, so + * [KeystoreDeviceLockedException] can distinguish a genuinely-locked + * device from the false-locked Keystore2 defect (device demonstrably + * unlocked, Keystore still denying — observed in the field on two QA + * devices during wallet creation). + */ + private val deviceLockStateProbe: () -> DeviceLockState = + { DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) }, ) { /** @@ -112,6 +125,16 @@ open class KeystoreManager( KeySecurityPolicy.DEVICE_BOUND -> KEYS_ALIAS_DEVICE_BOUND } + /** + * `KeyguardManager`'s CURRENT lock state via [deviceLockStateProbe] — + * one prompt-free sample, no Keystore access. Used at throw time by the + * device-locked denial mapping (so [KeystoreDeviceLockedException] + * records whether the OS agreed the device was locked) and by + * [WalletStorage.ensureDeviceUnlocked]'s createWallet pre-check. + * `open` purely as a unit-test seam, like the rest of this class. + */ + open fun sampleDeviceLockState(): DeviceLockState = deviceLockStateProbe() + /** * The [KeySecurityPolicy] identity keys are EFFECTIVELY protected with * right now — [KeySecurityPolicy.DEVICE_BOUND] while a requested @@ -240,9 +263,13 @@ open class KeystoreManager( require(alias != KEYS_ALIAS) { "the legacy identity-keys alias is read-only (migration fallback only)" } - val cipher = Cipher.getInstance(AES_TRANSFORMATION) - cipher.init(Cipher.ENCRYPT_MODE, secretKey(alias)) - return EncryptedBlob(iv = cipher.iv, ciphertext = cipher.doFinal(plaintext)) + return try { + val cipher = Cipher.getInstance(AES_TRANSFORMATION) + cipher.init(Cipher.ENCRYPT_MODE, secretKey(alias)) + EncryptedBlob(iv = cipher.iv, ciphertext = cipher.doFinal(plaintext)) + } catch (e: Exception) { + rethrowClassifyingDeviceLockedDenial(e, alias, operation = "encrypt") + } } /** @@ -318,13 +345,50 @@ open class KeystoreManager( "the legacy identity-keys alias is decrypted only via decryptLegacyKeysBlob / " + "decryptLegacyRsaKeysBlob" } - val cipher = Cipher.getInstance(AES_TRANSFORMATION) - cipher.init( - Cipher.DECRYPT_MODE, - secretKey(alias), - GCMParameterSpec(GCM_TAG_BITS, blob.iv), - ) - return cipher.doFinal(blob.ciphertext) + return try { + val cipher = Cipher.getInstance(AES_TRANSFORMATION) + cipher.init( + Cipher.DECRYPT_MODE, + secretKey(alias), + GCMParameterSpec(GCM_TAG_BITS, blob.iv), + ) + cipher.doFinal(blob.ciphertext) + } catch (e: Exception) { + rethrowClassifyingDeviceLockedDenial(e, alias, operation = "decrypt") + } + } + + /** + * Map a Keystore device-locked denial on a lock-bound NON-auth-gated + * alias (the [MASTER_ALIAS] AES path — `setUnlockedDeviceRequired(true)` + * on lock-screen devices, no `setUserAuthenticationRequired`) to the + * typed, retryable [KeystoreDeviceLockedException]; every other + * exception is rethrown unchanged. The `KeyguardManager` lock state is + * sampled HERE, at throw time, so the exception records whether the OS + * agreed the device was locked (genuinely locked) or not (the + * false-locked Keystore2 defect — device demonstrably unlocked, its + * lock-state tracking stuck; hit on two QA devices during wallet + * creation). + * + * MUST NOT be applied to the auth-gated identity-key aliases: their + * `UserNotAuthenticatedException` means "auth window closed" and drives + * the `BiometricGate` prompt-and-retry contract (see [decrypt]) — both + * RSA branches return before reaching this mapping. + */ + internal fun rethrowClassifyingDeviceLockedDenial( + e: Exception, + alias: String, + operation: String, + ): Nothing { + if (isDeviceLockedKeystoreDenial(e)) { + throw KeystoreDeviceLockedException( + alias = alias, + operation = operation, + lockState = sampleDeviceLockState(), + cause = e, + ) + } + throw e } /** @@ -931,6 +995,51 @@ open class KeystoreManager( return false } + /** + * Whether [t]'s cause chain is a Keystore "device is locked" denial + * of an operation on a lock-bound key (`setUnlockedDeviceRequired`). + * Two authoritative signals, mirroring the deliberately-narrow + * [isNoSecureLockScreenKeyGenFailure] style: + * + * - any `UserNotAuthenticatedException` in the chain. Correct ONLY + * for keys with no `setUserAuthenticationRequired` gate (the + * [MASTER_ALIAS] AES key): with no auth gate to be "not + * authenticated" against, Keystore throws it solely for the + * unlocked-device requirement. The call sites guarantee this — + * the classification is applied to the non-identity (AES) branch + * of [encrypt]/[decrypt] only, never to the auth-gated RSA + * aliases where the same exception means "auth window closed". + * - a `KeyStoreException` / `InvalidKeyException` in the chain + * whose message explicitly names the locked device ("device + * locked" / "device is locked" / "unlocked device") — the + * Keystore2 wording of the same denial on API levels that wrap + * it differently. + * + * Anything else (BadPadding, transient internal errors, …) does NOT + * classify and must surface unchanged. Pure and JVM-testable — + * matches by type name and message, no Android classes + * (the [isNoSecureLockScreenKeyGenFailure] discipline). + */ + internal fun isDeviceLockedKeystoreDenial(t: Throwable): Boolean { + var cur: Throwable? = t + while (cur != null) { + val name = cur::class.java.name + if (name.endsWith("UserNotAuthenticatedException")) return true + if ((name.endsWith("KeyStoreException") || name.endsWith("InvalidKeyException")) && + keyStoreMessageNamesLockedDevice(cur.message.orEmpty()) + ) { + return true + } + cur = cur.cause + } + return false + } + + private fun keyStoreMessageNamesLockedDevice(msg: String): Boolean = + msg.contains("device locked", ignoreCase = true) || + msg.contains("device is locked", ignoreCase = true) || + msg.contains("unlocked device", ignoreCase = true) + private fun keyStoreMessageNamesLockScreen(msg: String): Boolean = // Only the explicit lock-screen requirement is authoritative. A // bare "generate_key" is NOT matched (dashpay/platform#4060 blocker 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 40de259fdd1..0ee6b83391c 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 @@ -4,12 +4,14 @@ import android.app.KeyguardManager import android.content.Context import android.security.keystore.KeyPermanentlyInvalidatedException 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.edit import androidx.datastore.preferences.core.stringPreferencesKey import androidx.datastore.preferences.core.stringSetPreferencesKey import androidx.datastore.preferences.preferencesDataStore +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -50,7 +52,10 @@ private val Context.secretsStore: DataStore by preferencesDataStore class WalletStorage( context: Context, private val keystore: KeystoreManager = - KeystoreManager(deviceSecureProbe = deviceSecureProbe(context)), + KeystoreManager( + deviceSecureProbe = deviceSecureProbe(context), + deviceLockStateProbe = deviceLockStateProbe(context), + ), ) { /** * Construct with an explicit identity-key [keySecurityPolicy] — @@ -59,7 +64,14 @@ class WalletStorage( * [KeySecurityPolicy.AUTH_GATED] default. */ constructor(context: Context, keySecurityPolicy: KeySecurityPolicy) : - this(context, KeystoreManager(keySecurityPolicy, deviceSecureProbe(context))) + this( + context, + KeystoreManager( + keySecurityPolicy, + deviceSecureProbe(context), + deviceLockStateProbe = deviceLockStateProbe(context), + ), + ) private val store = context.secretsStore @@ -189,11 +201,73 @@ class WalletStorage( } } + // ── Device lock state ───────────────────────────────────────────── + + /** + * Fail fast with [KeystoreDeviceLockedException] if `KeyguardManager` + * reports the device LOCKED right now (`isDeviceLocked` — locked AND + * secured). A [MASTER_ALIAS][KeystoreManager.MASTER_ALIAS] Keystore + * operation on a lock-screen device would be denied anyway + * (`setUnlockedDeviceRequired`), so callers about to do irreversible + * orchestration around such an operation — `PlatformWalletManager. + * createWallet`, whose native create precedes [storeMnemonic] — call + * this FIRST and fail with nothing created and nothing to roll back. + * Prompt-free, no Keystore access; a no-op when the device is unlocked + * (including keyguard-showing-but-not-secured states). + */ + fun ensureDeviceUnlocked(operation: String) { + val state = keystore.sampleDeviceLockState() + if (state.isDeviceLocked) { + throw KeystoreDeviceLockedException( + alias = KeystoreManager.MASTER_ALIAS, + operation = operation, + lockState = state, + ) + } + } + // ── Mnemonics ───────────────────────────────────────────────────── + /** + * Encrypt and persist the mnemonic under the + * [MASTER_ALIAS][KeystoreManager.MASTER_ALIAS] AES key. + * + * 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. + */ suspend fun storeMnemonic(walletId: ByteArray, mnemonic: String) { - val blob = keystore.encrypt(mnemonic.encodeToByteArray()) - store.edit { it[mnemonicKey(walletId)] = encode(blob) } + val plaintext = mnemonic.encodeToByteArray() + var attempt = 0 + while (true) { + try { + val blob = keystore.encrypt(plaintext) + store.edit { it[mnemonicKey(walletId)] = encode(blob) } + return + } catch (e: KeystoreDeviceLockedException) { + if (e.deviceReportsLocked || attempt >= DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS.size) { + throw e + } + 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) + } + } } /** @@ -971,5 +1045,38 @@ class WalletStorage( ?.isDeviceSecure == true } } + + /** + * A prompt-free sampler of `KeyguardManager`'s CURRENT lock state + * (`isDeviceLocked` / `isKeyguardLocked`), captured against the + * application context like [deviceSecureProbe] so each call reads + * live state. Handed to [KeystoreManager] so a device-locked + * Keystore denial can record — at throw time — whether the OS + * agreed the device was locked, separating a genuine lock from the + * false-locked Keystore2 defect (see + * [KeystoreDeviceLockedException]). A missing KeyguardManager + * samples as unlocked. + */ + fun deviceLockStateProbe(context: Context): () -> DeviceLockState { + val appContext = context.applicationContext + return { + val keyguard = + appContext.getSystemService(Context.KEYGUARD_SERVICE) as? KeyguardManager + DeviceLockState( + isDeviceLocked = keyguard?.isDeviceLocked == true, + isKeyguardLocked = keyguard?.isKeyguardLocked == true, + ) + } + } + + /** + * Backoff schedule for [storeMnemonic]'s FALSE-LOCKED retry (the + * Keystore denied the master-alias encrypt as device-locked while + * `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 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 07d143c1c57..2c302235637 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 @@ -743,6 +743,13 @@ class PlatformWalletManager( * seen (`Some(h)` pins a specific height). Mirror of Swift * `PlatformWalletManager.createWallet(..., birthHeight:)` * (`birthHeight: showImportOption ? 0 : nil`). + * @throws org.dashfoundation.dashsdk.security.KeystoreDeviceLockedException + * (RETRYABLE after unlock) if `KeyguardManager` reports the device + * locked at entry — 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). */ suspend fun createWallet( mnemonic: String, @@ -750,6 +757,15 @@ class PlatformWalletManager( createDefaultAccounts: Boolean = true, birthHeight: UInt? = null, ): ManagedPlatformWallet = withContext(Dispatchers.IO) { + // Fail-fast pre-check BEFORE the native create: on a genuinely + // locked device (KeyguardManager.isDeviceLocked) the storeMnemonic + // step below is guaranteed to be denied by the lock-bound + // MASTER_ALIAS key (setUnlockedDeviceRequired), which would force + // the full rollback dance. Failing here instead means no native + // wallet was created, no Room rows were written, and there is + // nothing to roll back — the typed KeystoreDeviceLockedException + // tells the caller to retry after unlock. + walletStorage.ensureDeviceUnlocked(operation = "createWallet") // Caller-allocated out-buffers: the JNI side validates both BEFORE // the native create so no fallible allocation follows the // persistence commit (a post-commit publish failure would strand 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 new file mode 100644 index 00000000000..123b9bcf6c0 --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreDeviceLockedDenialTest.kt @@ -0,0 +1,162 @@ +package org.dashfoundation.dashsdk.security + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertSame +import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue +import org.junit.Test +import java.security.GeneralSecurityException +import java.security.InvalidKeyException +import javax.crypto.BadPaddingException + +/** + * Pins the typed device-locked Keystore denial mapping (the QA field + * failure: `UserNotAuthenticatedException` from the lock-bound, NON-auth- + * gated [KeystoreManager.MASTER_ALIAS] AES key during wallet creation — + * sometimes while the device was demonstrably unlocked, i.e. Keystore2 + * lock-state misreporting). The classifier and the mapping are pure / + * probe-injected, so — like [KeystoreKeyGenPolicyTest] — they run on the + * plain JVM with type-name stand-ins for the Android Keystore exceptions + * (which cannot be constructed here). + */ +class KeystoreDeviceLockedDenialTest { + + /** + * Stand-in for `android.security.keystore.UserNotAuthenticatedException`; + * the classifier matches by type-name suffix (the + * [KeystoreManager.isNoSecureLockScreenKeyGenFailure] discipline). + */ + private class SimulatedUserNotAuthenticatedException : + GeneralSecurityException("User not authenticated") + + /** Stand-in for `android.security.KeyStoreException`. */ + private class SimulatedKeyStoreException(message: String) : + GeneralSecurityException(message) + + // ── Classifier ─────────────────────────────────────────────────────── + + @Test + fun shouldClassifyUserNotAuthenticatedAsDeviceLockedDenial() { + // The field shape: the master-alias AES key carries NO + // setUserAuthenticationRequired gate, so "user not authenticated" + // from it can only be the setUnlockedDeviceRequired denial. + assertTrue( + KeystoreManager.isDeviceLockedKeystoreDenial( + SimulatedUserNotAuthenticatedException(), + ), + ) + } + + @Test + fun shouldClassifyWrappedUserNotAuthenticatedInCauseChain() { + // Some API levels wrap the denial (e.g. cipher.init throwing + // InvalidKeyException whose cause is the Keystore denial). + val wrapped = InvalidKeyException( + "Keystore operation failed", + SimulatedUserNotAuthenticatedException(), + ) + assertTrue(KeystoreManager.isDeviceLockedKeystoreDenial(wrapped)) + } + + @Test + fun shouldClassifyKeyStoreExceptionNamingTheLockedDevice() { + assertTrue( + KeystoreManager.isDeviceLockedKeystoreDenial( + SimulatedKeyStoreException("Keystore operation failed: device locked"), + ), + ) + assertTrue( + KeystoreManager.isDeviceLockedKeystoreDenial( + InvalidKeyException("unlocked device required"), + ), + ) + } + + @Test + fun shouldNotClassifyUnrelatedCryptoOrKeystoreFailures() { + // A GCM tag failure, a generic Keystore fault, a transient internal + // error: none of these are the device-locked gate and none may be + // mapped to the retryable exception. + assertFalse( + KeystoreManager.isDeviceLockedKeystoreDenial(BadPaddingException("mac check failed")), + ) + assertFalse( + KeystoreManager.isDeviceLockedKeystoreDenial( + SimulatedKeyStoreException("System error (internal Keystore code: 4)"), + ), + ) + assertFalse( + KeystoreManager.isDeviceLockedKeystoreDenial( + InvalidKeyException("no key material"), + ), + ) + } + + // ── Mapping + at-throw-time lock-state sampling ────────────────────── + + private fun managerSampling(state: DeviceLockState): KeystoreManager = + KeystoreManager(deviceLockStateProbe = { state }) + + @Test + fun shouldMapDenialToTypedExceptionSamplingFalseLockedState() { + // The defect class: Keystore denies as device-locked while + // KeyguardManager says the device is NOT locked. The sampled state + // must ride the exception so logs (and the storeMnemonic retry) can + // tell this apart from a genuine lock. + val manager = managerSampling( + DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false), + ) + val denial = SimulatedUserNotAuthenticatedException() + + val thrown = assertThrows(KeystoreDeviceLockedException::class.java) { + manager.rethrowClassifyingDeviceLockedDenial( + denial, + KeystoreManager.MASTER_ALIAS, + operation = "encrypt", + ) + } + assertEquals(KeystoreManager.MASTER_ALIAS, thrown.alias) + assertEquals("encrypt", thrown.operation) + assertFalse(thrown.deviceReportsLocked) + assertFalse(thrown.lockState.isDeviceLocked) + assertSame(denial, thrown.cause) + assertTrue(thrown.message.orEmpty().contains("FALSE-LOCKED")) + } + + @Test + fun shouldMapDenialToTypedExceptionSamplingGenuinelyLockedState() { + val manager = managerSampling( + DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true), + ) + + val thrown = assertThrows(KeystoreDeviceLockedException::class.java) { + manager.rethrowClassifyingDeviceLockedDenial( + SimulatedUserNotAuthenticatedException(), + KeystoreManager.MASTER_ALIAS, + operation = "decrypt", + ) + } + assertEquals("decrypt", thrown.operation) + assertTrue(thrown.deviceReportsLocked) + assertTrue(thrown.lockState.isKeyguardLocked) + assertTrue(thrown.message.orEmpty().contains("genuinely locked")) + } + + @Test + fun shouldRethrowNonDenialExceptionsUnchanged() { + val manager = managerSampling( + DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false), + ) + val unrelated = BadPaddingException("mac check failed") + + val thrown = assertThrows(BadPaddingException::class.java) { + manager.rethrowClassifyingDeviceLockedDenial( + unrelated, + KeystoreManager.MASTER_ALIAS, + operation = "decrypt", + ) + } + assertSame(unrelated, thrown) + } +} 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 new file mode 100644 index 00000000000..9f625a8a508 --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageDeviceLockedRetryTest.kt @@ -0,0 +1,181 @@ +package org.dashfoundation.dashsdk.security + +import androidx.test.core.app.ApplicationProvider +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +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 the device-locked hardening around wallet creation's mnemonic + * persistence (the QA field failure — Keystore denied the lock-bound + * [KeystoreManager.MASTER_ALIAS] encrypt as device-locked during + * `createWallet`, on devices that were demonstrably unlocked): + * + * 1. [WalletStorage.ensureDeviceUnlocked] — the `createWallet` fail-fast + * pre-check: throws the typed, retryable [KeystoreDeviceLockedException] + * on a genuinely locked device BEFORE any native wallet exists. + * 2. [WalletStorage.storeMnemonic]'s bounded FALSE-LOCKED retry: a denial + * 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. + * + * The real AndroidKeyStore crypto cannot run on the JVM (see + * [KeySecurityPolicyTest]), so a fake [KeystoreManager] scripts the + * master-alias encrypt outcomes through the class's `open` test seams, + * exactly as [WalletStorageUpgradeMatrixTest] does for the identity-key + * ladder. + */ +@RunWith(RobolectricTestRunner::class) +class WalletStorageDeviceLockedRetryTest { + + private val walletId = ByteArray(32) { (it + 1).toByte() } + private val mnemonic = "abandon abandon abandon abandon abandon abandon " + + "abandon abandon abandon abandon abandon about" + + private lateinit var fake: FalseLockedFakeKeystoreManager + private lateinit var storage: WalletStorage + + @Before + fun setUp() = runBlocking { + fake = FalseLockedFakeKeystoreManager() + storage = WalletStorage(ApplicationProvider.getApplicationContext(), fake) + // Isolate from any state a prior test left in the shared DataStore file. + storage.deleteAll() + } + + // ── createWallet fail-fast pre-check ───────────────────────────────── + + @Test + fun shouldFailFastWhenDeviceIsGenuinelyLocked() { + fake.lockState = DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true) + + val thrown = assertThrows(KeystoreDeviceLockedException::class.java) { + storage.ensureDeviceUnlocked(operation = "createWallet") + } + assertEquals("createWallet", thrown.operation) + assertEquals(KeystoreManager.MASTER_ALIAS, thrown.alias) + assertTrue(thrown.deviceReportsLocked) + // Prompt-free and Keystore-free: the pre-check must never touch the + // master key (nothing to retry, nothing to roll back). + assertEquals(0, fake.masterEncryptCalls) + } + + @Test + fun shouldPassPreCheckWhenDeviceIsUnlocked() { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + storage.ensureDeviceUnlocked(operation = "createWallet") // must not throw + } + + @Test + fun shouldPassPreCheckWhenKeyguardShowsButDeviceIsNotSecurelyLocked() { + // 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. + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = true) + storage.ensureDeviceUnlocked(operation = "createWallet") // must not throw + } + + // ── storeMnemonic bounded FALSE-LOCKED retry ───────────────────────── + + @Test + fun shouldRetryFalseLockedDenialAndSucceedOnSecondAttempt() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + fake.failMasterEncrypts = 1 // deny once, then heal — the observed field pattern + + storage.storeMnemonic(walletId, mnemonic) + + assertEquals(2, fake.masterEncryptCalls) + // 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 + + var thrown: KeystoreDeviceLockedException? = null + try { + storage.storeMnemonic(walletId, mnemonic) + } catch (e: KeystoreDeviceLockedException) { + thrown = e + } + + assertTrue("expected the typed denial to propagate", thrown != null) + assertFalse(thrown!!.deviceReportsLocked) + // Initial attempt + the full 3-retry schedule (250/750/1000ms), + // then give up. + assertEquals(4, fake.masterEncryptCalls) + assertEquals(null, storage.retrieveMnemonic(walletId)) + } + + @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) + fake.failMasterEncrypts = Int.MAX_VALUE + + var thrown: KeystoreDeviceLockedException? = null + try { + storage.storeMnemonic(walletId, mnemonic) + } catch (e: KeystoreDeviceLockedException) { + thrown = e + } + + assertTrue("expected the typed denial to propagate", thrown != null) + assertTrue(thrown!!.deviceReportsLocked) + assertEquals(1, fake.masterEncryptCalls) + } + + @Test + fun shouldStoreWithoutRetryMachineryWhenKeystoreIsHealthy() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + + storage.storeMnemonic(walletId, mnemonic) + + assertEquals(1, fake.masterEncryptCalls) + assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) + } +} + +/** + * Master-alias-focused fake: scripts [failMasterEncrypts] device-locked + * denials (each carrying [lockState] sampled "at throw time", as the real + * mapping does) before letting encrypts succeed with a trivially reversible + * blob. 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 + + 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++ + if (failMasterEncrypts > 0) { + failMasterEncrypts-- + throw KeystoreDeviceLockedException( + alias = alias, + operation = "encrypt", + lockState = sampleDeviceLockState(), + ) + } + return EncryptedBlob(iv = ByteArray(12) { 7 }, ciphertext = plaintext.copyOf()) + } + + override fun decrypt(blob: EncryptedBlob, alias: String): ByteArray { + check(alias == MASTER_ALIAS) { "test fake only models the master alias" } + return blob.ciphertext.copyOf() + } +} From 6e9a8c8dcc821a0a2c7e5c41ecb6d3b4f08e6448 Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Sun, 23 Aug 2026 15:56:46 -0400 Subject: [PATCH 2/4] fix(kotlin-sdk): make the file-logging install failure diagnosable Field log line: "SDK tracing file logging (INFO)... NOT installed (subscriber already set or dir unwritable)" -- the native installer returns one boolean that conflates its two failure modes, so field logs could not say WHICH condition failed. Kotlin-side (the tracing crate cannot attach layers to an already- installed global subscriber, so a native re-route is not available): - New Sdk.installFileLogging returns a FileLoggingInstall outcome (INSTALLED / ALREADY_SET / SESSION_ROOT_UNWRITABLE) and logs the distinguishing condition as a warning: the unwritable case names the exact path; the already-set case names the in-process claimant when the subscriber went through this API (console logging via enableLogging first is the common cause) and states the fix -- install file logging before console logging. - The writability check (a real create-and-delete probe, the same operation the native open_file performs) runs BEFORE any native call, so the unwritable verdict is cheap, exact, and JVM-testable. - enableFileLogging keeps its Boolean signature as a compat wrapper. Unit tests: unwritable session root reported pre-native (a file at the root path), writable-dir and nested-dir probe true, file-as-root probe false. Co-Authored-By: Claude Fable 5 --- .../kotlin/org/dashfoundation/dashsdk/Sdk.kt | 125 +++++++++++++++++- .../dashsdk/SdkFileLoggingInstallTest.kt | 55 ++++++++ 2 files changed, 176 insertions(+), 4 deletions(-) create mode 100644 packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/SdkFileLoggingInstallTest.kt diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/Sdk.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/Sdk.kt index 1674e38716d..bd7591db8bc 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/Sdk.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/Sdk.kt @@ -9,6 +9,7 @@ import org.dashfoundation.dashsdk.errors.mapNativeErrors import org.dashfoundation.dashsdk.ffi.NativeCleaner import org.dashfoundation.dashsdk.ffi.NativeLoader import org.dashfoundation.dashsdk.ffi.SdkNative +import java.io.File import java.net.HttpURLConnection import java.net.URI import java.util.concurrent.atomic.AtomicLong @@ -155,6 +156,35 @@ class Sdk private constructor( ERROR(0), WARN(1), INFO(2), DEBUG(3), TRACE(4) } + /** + * Outcome of [installFileLogging]. The native installer returns a + * single boolean whose `false` conflates its two failure modes — + * field logs showed exactly that: "NOT installed (subscriber already + * set or dir unwritable)" with no way to tell which. This type keeps + * them apart so the failure is diagnosable from a log line. + */ + enum class FileLoggingInstall { + /** This call installed the file-logging subscriber. */ + INSTALLED, + + /** + * A process-global tracing subscriber was already set (first init + * wins — e.g. console logging via [enableLogging] ran first, or + * another in-process library installed one). The `tracing` API has + * no way to attach layers to an already-installed subscriber, so + * file logging cannot be added after the fact; install file + * logging FIRST if both are wanted. + */ + ALREADY_SET, + + /** + * The session root could not be created or written — file logging + * was not attempted (the native installer would have failed on the + * same directory). The path is named in the logged warning. + */ + SESSION_ROOT_UNWRITABLE, + } + @Serializable private data class MasternodesEnvelope( val success: Boolean, @@ -197,23 +227,110 @@ class Sdk private constructor( private val json = Json { ignoreUnknownKeys = true } + private const val LOG_TAG = "DashSdk" + /** One-time native library load + `dash_sdk_init`. Idempotent. */ fun initialize() = NativeLoader.ensureLoaded() + /** + * Best-effort record of which call claimed the process-global + * tracing subscriber, for the [ALREADY_SET][FileLoggingInstall.ALREADY_SET] + * diagnostic. In-process bookkeeping only — a subscriber installed + * outside this companion (another library) is invisible here, so a + * `null` value means "not through this API", not "none". + */ + @Volatile + private var subscriberClaimedBy: String? = null + /** Enable console (logcat) logging for SDK operations. */ fun enableLogging(level: LogLevel = LogLevel.DEBUG) { initialize() SdkNative.enableLogging(level.value) + // First-wins bookkeeping: the global tracing subscriber can only + // be installed once per process, and console logging installs + // one — the most common reason a later enableFileLogging finds + // the slot taken. + if (subscriberClaimedBy == null) { + subscriberClaimedBy = "console logging (enableLogging)" + } } /** * Route the global tracing subscriber to per-bucket files under - * [sessionRoot]. Returns false if a subscriber was already - * installed or the path is unwritable. + * [sessionRoot]. Returns true only when THIS call installed it — + * boolean-compat wrapper around [installFileLogging], which callers + * should prefer: it reports WHICH condition failed (subscriber + * already set vs. session root unwritable, with the path) instead + * of an undiagnosable false. */ - fun enableFileLogging(level: LogLevel = LogLevel.DEBUG, sessionRoot: String): Boolean { + fun enableFileLogging(level: LogLevel = LogLevel.DEBUG, sessionRoot: String): Boolean = + installFileLogging(level, sessionRoot) == FileLoggingInstall.INSTALLED + + /** + * [enableFileLogging] with a diagnosable outcome. On failure the + * distinguishing condition is also logged as a warning (tag + * `DashSdk`), so field logs no longer show the ambiguous + * "NOT installed (subscriber already set or dir unwritable)": + * + * - [FileLoggingInstall.SESSION_ROOT_UNWRITABLE] — [sessionRoot] + * could not be created/written; checked BEFORE touching the + * native installer, and the logged warning names the exact path. + * - [FileLoggingInstall.ALREADY_SET] — the directory IS writable + * but a global tracing subscriber already exists (first init + * wins). The `tracing` API cannot re-route an installed + * subscriber, so the fix is ordering: install file logging + * before [enableLogging]. The warning names the in-process + * claimant when it went through this API. + */ + fun installFileLogging( + level: LogLevel = LogLevel.DEBUG, + sessionRoot: String, + ): FileLoggingInstall { + val root = File(sessionRoot) + if (!sessionRootWritable(root)) { + android.util.Log.w( + LOG_TAG, + "SDK file logging NOT installed: session root cannot be " + + "created/written: ${root.absolutePath}", + ) + return FileLoggingInstall.SESSION_ROOT_UNWRITABLE + } initialize() - return SdkNative.enableFileLogging(level.value, sessionRoot) + return if (SdkNative.enableFileLogging(level.value, sessionRoot)) { + subscriberClaimedBy = "file logging (enableFileLogging)" + FileLoggingInstall.INSTALLED + } else { + val claimant = subscriberClaimedBy + ?: "something outside this API (another in-process library, " + + "or a subscriber surviving from an earlier init)" + android.util.Log.w( + LOG_TAG, + "SDK file logging NOT installed at ${root.absolutePath}: the " + + "directory is writable, so a global tracing subscriber was " + + "already set — by $claimant. First init wins and the tracing " + + "API cannot re-route an installed subscriber; call " + + "enableFileLogging before enableLogging to get file logs.", + ) + FileLoggingInstall.ALREADY_SET + } + } + + /** + * Whether [root] exists (or can be created) as a directory we can + * write a file into — probed with a real create-and-delete, the + * same operation the native installer's `open_file` will perform. + * Factored out (and `internal`) so the pre-native gate is + * JVM-testable. + */ + internal fun sessionRootWritable(root: File): Boolean = try { + root.mkdirs() + val probe = File(root, ".dash_sdk_write_probe") + probe.delete() + val created = probe.createNewFile() + probe.delete() + created && root.isDirectory + } catch (_: Exception) { + false } /** diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/SdkFileLoggingInstallTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/SdkFileLoggingInstallTest.kt new file mode 100644 index 00000000000..1252a805b12 --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/SdkFileLoggingInstallTest.kt @@ -0,0 +1,55 @@ +package org.dashfoundation.dashsdk + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.io.File + +/** + * Pins the diagnosable file-logging install gate. The field log line + * "SDK tracing file logging (INFO)… NOT installed (subscriber already set + * or dir unwritable)" could not say WHICH condition failed; the gate now + * separates them, and the unwritable check runs BEFORE any native call — + * which is also what makes this JVM-testable (the .so cannot load here, so + * reaching the native installer would throw). + */ +@RunWith(RobolectricTestRunner::class) +class SdkFileLoggingInstallTest { + + @get:Rule + val tmp = TemporaryFolder() + + @Test + fun shouldReportUnwritableSessionRootWithoutTouchingTheNativeInstaller() { + // A regular FILE at the session-root path: mkdirs and the probe + // write must both fail. Returning (rather than throwing + // UnsatisfiedLinkError from the native loader) proves the gate ran + // pre-native. + val fileNotDir = tmp.newFile("not-a-directory") + + val outcome = Sdk.installFileLogging( + level = Sdk.LogLevel.INFO, + sessionRoot = fileNotDir.absolutePath, + ) + + assertEquals(Sdk.FileLoggingInstall.SESSION_ROOT_UNWRITABLE, outcome) + } + + @Test + fun shouldProbeWritableSessionRootTrue() { + // An existing writable dir, and a nested not-yet-created one (the + // installer is expected to create the session tree). + assertTrue(Sdk.sessionRootWritable(tmp.root)) + assertTrue(Sdk.sessionRootWritable(File(tmp.root, "nested/session"))) + } + + @Test + fun shouldProbeFileAsSessionRootFalse() { + assertFalse(Sdk.sessionRootWritable(tmp.newFile("plain-file"))) + } +} From 16c4afb9e6fc3a29764d081d2d167bb47a45ae2a Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Mon, 24 Aug 2026 07:44:27 -0400 Subject: [PATCH 3/4] fix(kotlin-sdk): address device-locked classification + file-logging probe review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four PR #4463 review findings: - The writability probe used the fixed name .dash_sdk_write_probe and deleted that path first, so a caller-owned entry of the same name in the caller-selected session root could be destroyed, and concurrent probes raced each other. The probe now creates a uniquely named temp file (File.createTempFile) and deletes only the file it created. - rethrowClassifyingDeviceLockedDenial wrapped the generic AES encrypt/decrypt branches for ARBITRARY aliases, but only MASTER_ALIAS contractually carries no setUserAuthenticationRequired gate — for a host-provisioned auth-gated AES alias, UserNotAuthenticatedException means "auth window closed" and must keep driving the caller's prompt-and-retry path. Classification is now gated on alias == MASTER_ALIAS in the mapper itself; every other alias rethrows unchanged (pinned by new tests for the auth-gated identity alias and a custom host alias). - A writable session root did not prove a native false meant ALREADY_SET: the native installer also opens fixed child destinations (dash_sdk/run.log, platform_wallet/run.log, ..., build_info.txt), so e.g. a regular file at $root/dash_sdk failed native create_dir_all and was misreported as ALREADY_SET. The pre-native gate now probes every fixed destination the installer touches (list kept in sync with rs-platform-wallet-ffi/src/logging.rs), names the exact blocked path, and the ALREADY_SET docs state honestly that it is diagnosed by elimination over a bare native boolean. - enableLogging recorded itself as the subscriber CLAIMANT even though its native side swallows a lost try_init() race and reports nothing. It is now recorded separately as an ATTEMPT, and the ALREADY_SET warning presents it as "most likely" instead of asserting it, while a successful installFileLogging (whose native call does report the outcome) remains the only verifiable claimant. Co-Authored-By: Claude Fable 5 --- .../kotlin/org/dashfoundation/dashsdk/Sdk.kt | 185 +++++++++++++----- .../dashsdk/security/KeystoreManager.kt | 27 ++- .../dashsdk/SdkFileLoggingInstallTest.kt | 65 ++++++ .../KeystoreDeviceLockedDenialTest.kt | 43 ++++ 4 files changed, 265 insertions(+), 55 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/Sdk.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/Sdk.kt index bd7591db8bc..0fe753eb68d 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/Sdk.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/Sdk.kt @@ -178,9 +178,12 @@ class Sdk private constructor( ALREADY_SET, /** - * The session root could not be created or written — file logging - * was not attempted (the native installer would have failed on the - * same directory). The path is named in the logged warning. + * The session root — or one of the fixed log destinations the + * native installer creates/opens under it (`dash_sdk/run.log`, + * `build_info.txt`, …) — could not be created or written; file + * logging was not attempted (the native installer would have + * failed on the same path). The blocked path is named in the + * logged warning. */ SESSION_ROOT_UNWRITABLE, } @@ -233,25 +236,42 @@ class Sdk private constructor( fun initialize() = NativeLoader.ensureLoaded() /** - * Best-effort record of which call claimed the process-global - * tracing subscriber, for the [ALREADY_SET][FileLoggingInstall.ALREADY_SET] - * diagnostic. In-process bookkeeping only — a subscriber installed - * outside this companion (another library) is invisible here, so a - * `null` value means "not through this API", not "none". + * Best-effort record of which call is KNOWN to have claimed the + * process-global tracing subscriber, for the + * [ALREADY_SET][FileLoggingInstall.ALREADY_SET] diagnostic. Only + * [installFileLogging] can assert this: its native installer + * reports whether THIS call installed the subscriber. In-process + * bookkeeping only — a subscriber installed outside this companion + * (another library) is invisible here, so a `null` value means + * "not through this API", not "none". */ @Volatile private var subscriberClaimedBy: String? = null + /** + * Best-effort record of the first [enableLogging] call, kept apart + * from [subscriberClaimedBy] because it is only an ATTEMPT: the + * native console installer returns nothing and deliberately + * swallows a lost `try_init()` race, so the call proves the console + * path ran — not that it won the subscriber slot. A library outside + * this API may already have held it. + */ + @Volatile + private var subscriberAttemptedBy: String? = null + /** Enable console (logcat) logging for SDK operations. */ fun enableLogging(level: LogLevel = LogLevel.DEBUG) { initialize() SdkNative.enableLogging(level.value) - // First-wins bookkeeping: the global tracing subscriber can only - // be installed once per process, and console logging installs - // one — the most common reason a later enableFileLogging finds - // the slot taken. - if (subscriberClaimedBy == null) { - subscriberClaimedBy = "console logging (enableLogging)" + // First-wins bookkeeping, recorded as an ATTEMPT only, never as + // the claimant: console logging installs a subscriber when the + // slot is free — the most common reason a later + // enableFileLogging finds it taken — but the native call cannot + // report whether it actually won, so asserting a claim here + // would name the wrong claimant whenever another library got in + // first. + if (subscriberAttemptedBy == null) { + subscriberAttemptedBy = "console logging (enableLogging)" } } @@ -272,26 +292,37 @@ class Sdk private constructor( * `DashSdk`), so field logs no longer show the ambiguous * "NOT installed (subscriber already set or dir unwritable)": * - * - [FileLoggingInstall.SESSION_ROOT_UNWRITABLE] — [sessionRoot] - * could not be created/written; checked BEFORE touching the - * native installer, and the logged warning names the exact path. - * - [FileLoggingInstall.ALREADY_SET] — the directory IS writable - * but a global tracing subscriber already exists (first init - * wins). The `tracing` API cannot re-route an installed - * subscriber, so the fix is ordering: install file logging - * before [enableLogging]. The warning names the in-process - * claimant when it went through this API. + * - [FileLoggingInstall.SESSION_ROOT_UNWRITABLE] — [sessionRoot], + * or one of the fixed log destinations the native installer + * creates/opens under it, could not be created/written; checked + * BEFORE touching the native installer, and the logged warning + * names the exact blocked path. + * - [FileLoggingInstall.ALREADY_SET] — every fixed log destination + * IS writable, yet the native installer reported failure. With + * its I/O legs ruled out up front, that is attributed to a global + * tracing subscriber already existing (first init wins). The + * `tracing` API cannot re-route an installed subscriber, so the + * fix is ordering: install file logging before [enableLogging]. + * The warning names the in-process claimant when the subscriber + * verifiably went through this API, or the first in-process + * ATTEMPT ([enableLogging]) as "most likely". Diagnosed by + * elimination, not reported by the native boundary (a bare + * boolean): an I/O failure racing in between the probe and the + * install — or a destination a future native version adds — + * would still be misattributed here. */ fun installFileLogging( level: LogLevel = LogLevel.DEBUG, sessionRoot: String, ): FileLoggingInstall { val root = File(sessionRoot) - if (!sessionRootWritable(root)) { + val blocked = firstUnwritableLogDestination(root) + if (blocked != null) { android.util.Log.w( LOG_TAG, - "SDK file logging NOT installed: session root cannot be " + - "created/written: ${root.absolutePath}", + "SDK file logging NOT installed: session-root log " + + "destination cannot be created/written: " + + blocked.absolutePath, ) return FileLoggingInstall.SESSION_ROOT_UNWRITABLE } @@ -300,35 +331,97 @@ class Sdk private constructor( subscriberClaimedBy = "file logging (enableFileLogging)" FileLoggingInstall.INSTALLED } else { - val claimant = subscriberClaimedBy - ?: "something outside this API (another in-process library, " + - "or a subscriber surviving from an earlier init)" + val blame = when { + subscriberClaimedBy != null -> "by $subscriberClaimedBy" + subscriberAttemptedBy != null -> + "most likely by $subscriberAttemptedBy — recorded as " + + "an attempt only (its native installer cannot " + + "report whether it won the slot), so an earlier " + + "claim from outside this API is also possible" + else -> "by something outside this API (another " + + "in-process library, or a subscriber surviving " + + "from an earlier init)" + } android.util.Log.w( LOG_TAG, - "SDK file logging NOT installed at ${root.absolutePath}: the " + - "directory is writable, so a global tracing subscriber was " + - "already set — by $claimant. First init wins and the tracing " + - "API cannot re-route an installed subscriber; call " + - "enableFileLogging before enableLogging to get file logs.", + "SDK file logging NOT installed at ${root.absolutePath}: every " + + "fixed log destination is writable, so a global tracing " + + "subscriber was already set — $blame. First init wins and " + + "the tracing API cannot re-route an installed subscriber; " + + "call enableFileLogging before enableLogging to get file logs.", ) FileLoggingInstall.ALREADY_SET } } /** - * Whether [root] exists (or can be created) as a directory we can - * write a file into — probed with a real create-and-delete, the - * same operation the native installer's `open_file` will perform. - * Factored out (and `internal`) so the pre-native gate is - * JVM-testable. + * The fixed destinations `platform_wallet_enable_file_logging` + * creates/opens under the session root — keep in sync with + * `packages/rs-platform-wallet-ffi/src/logging.rs`. Probed by + * [firstUnwritableLogDestination] BEFORE the native call so that a + * native `false` can be attributed to the subscriber slot rather + * than to I/O: probing only the root was not enough (e.g. a regular + * FILE at `$root/dash_sdk` fails the native `create_dir_all` while + * the root itself probes writable — previously misreported as + * [FileLoggingInstall.ALREADY_SET]). */ - internal fun sessionRootWritable(root: File): Boolean = try { - root.mkdirs() - val probe = File(root, ".dash_sdk_write_probe") - probe.delete() - val created = probe.createNewFile() - probe.delete() - created && root.isDirectory + private val NATIVE_LOG_DESTINATIONS = listOf( + "dash_sdk/run.log", + "dash_sdk/metrics.log", + "platform_wallet/run.log", + "platform_wallet/metrics.log", + "dash_spv/run.log", + "key_wallet/run.log", + "grpc/run.log", + "build_info.txt", + ) + + /** + * Whether [root] — and every fixed log destination under it — can + * be created/written. Boolean wrapper around + * [firstUnwritableLogDestination]; `internal` so the pre-native + * gate is JVM-testable. + */ + internal fun sessionRootWritable(root: File): Boolean = + firstUnwritableLogDestination(root) == null + + /** + * The first path the native installer needs that cannot be + * created/written, or `null` when the whole fixed destination set + * probes writable. Parent directories are pre-created exactly as + * the native `create_dir_all` would (behavior-identical, not an + * extra side effect). Writability is probed with a real + * create-and-delete of a UNIQUELY named temp file: a fixed probe + * name could destroy a caller-owned entry of the same name in the + * caller-selected session root, and concurrent probes would delete + * each other's file and misreport a writable directory — only the + * file this invocation created is ever deleted. + */ + internal fun firstUnwritableLogDestination(root: File): File? { + if (!directoryWritable(root)) return root + for (relative in NATIVE_LOG_DESTINATIONS) { + val destination = File(root, relative) + val parent = destination.parentFile ?: root + if (!directoryWritable(parent)) return parent + // The native open is create(true).append(true): an existing + // entry must be a writable regular file (a directory, or an + // unwritable file, at the path fails that open). + if (destination.exists() && !(destination.isFile && destination.canWrite())) { + return destination + } + } + return null + } + + private fun directoryWritable(dir: File): Boolean = try { + dir.mkdirs() + if (!dir.isDirectory) { + false + } else { + val probe = File.createTempFile(".dash_sdk_write_probe", null, dir) + probe.delete() + true + } } catch (_: Exception) { false } 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 0e6cf1642eb..97ecbb20493 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 @@ -370,17 +370,24 @@ open class KeystoreManager( * lock-state tracking stuck; hit on two QA devices during wallet * creation). * - * MUST NOT be applied to the auth-gated identity-key aliases: their - * `UserNotAuthenticatedException` means "auth window closed" and drives - * the `BiometricGate` prompt-and-retry contract (see [decrypt]) — both - * RSA branches return before reaching this mapping. + * ONLY [MASTER_ALIAS] classifies — its key's contract guarantees no + * `setUserAuthenticationRequired` gate, which is what makes a + * `UserNotAuthenticatedException` from it unambiguous. Every other + * alias rethrows unchanged, enforced HERE and not just at the call + * sites: the generic AES branches of [encrypt]/[decrypt] accept + * arbitrary aliases, and a host-provisioned auth-gated AES alias + * throws the same `UserNotAuthenticatedException` to mean "auth window + * closed" — classifying that as device-locked would strand the + * caller's prompt-and-retry handling (exactly the `BiometricGate` + * contract the auth-gated RSA aliases depend on; those return before + * reaching this mapping, see [decrypt]). */ internal fun rethrowClassifyingDeviceLockedDenial( e: Exception, alias: String, operation: String, ): Nothing { - if (isDeviceLockedKeystoreDenial(e)) { + if (alias == MASTER_ALIAS && isDeviceLockedKeystoreDenial(e)) { throw KeystoreDeviceLockedException( alias = alias, operation = operation, @@ -1005,10 +1012,12 @@ open class KeystoreManager( * for keys with no `setUserAuthenticationRequired` gate (the * [MASTER_ALIAS] AES key): with no auth gate to be "not * authenticated" against, Keystore throws it solely for the - * unlocked-device requirement. The call sites guarantee this — - * the classification is applied to the non-identity (AES) branch - * of [encrypt]/[decrypt] only, never to the auth-gated RSA - * aliases where the same exception means "auth window closed". + * unlocked-device requirement. + * [rethrowClassifyingDeviceLockedDenial] guarantees this — it + * classifies [MASTER_ALIAS] only, never an arbitrary caller + * alias (which may carry an auth gate) and never the auth-gated + * RSA aliases, where the same exception means "auth window + * closed". * - a `KeyStoreException` / `InvalidKeyException` in the chain * whose message explicitly names the locked device ("device * locked" / "device is locked" / "unlocked device") — the diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/SdkFileLoggingInstallTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/SdkFileLoggingInstallTest.kt index 1252a805b12..f03bd3868a2 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/SdkFileLoggingInstallTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/SdkFileLoggingInstallTest.kt @@ -52,4 +52,69 @@ class SdkFileLoggingInstallTest { fun shouldProbeFileAsSessionRootFalse() { assertFalse(Sdk.sessionRootWritable(tmp.newFile("plain-file"))) } + + @Test + fun shouldNotDeleteACallerOwnedEntryNamedLikeTheProbe() { + // The probe once used the fixed name `.dash_sdk_write_probe` and + // deleted that path first — destroying a caller-owned file of the + // same name in the caller-selected session root (PR review). The + // probe must be uniquely named and delete only what it created. + val callerOwned = File(tmp.root, ".dash_sdk_write_probe") + callerOwned.writeText("caller data") + + assertTrue(Sdk.sessionRootWritable(tmp.root)) + + assertTrue(callerOwned.exists()) + assertEquals("caller data", callerOwned.readText()) + } + + @Test + fun shouldLeaveNoProbeResidueBehind() { + assertTrue(Sdk.sessionRootWritable(tmp.root)) + + val leftovers = tmp.root.walkTopDown() + .filter { it.isFile && it.name.startsWith(".dash_sdk_write_probe") } + .toList() + assertEquals(emptyList(), leftovers) + } + + @Test + fun shouldReportABlockedFixedLogDestinationWithoutTouchingTheNativeInstaller() { + // The ALREADY_SET misattribution shape (PR review): the root itself + // probes writable, but the native create_dir_all("dash_sdk") would + // fail on this regular FILE — previously reported as ALREADY_SET + // with no subscriber in sight. Returning SESSION_ROOT_UNWRITABLE + // (rather than throwing UnsatisfiedLinkError from the native + // loader) also proves the check ran pre-native. + tmp.newFile("dash_sdk") + + val outcome = Sdk.installFileLogging( + level = Sdk.LogLevel.INFO, + sessionRoot = tmp.root.absolutePath, + ) + + assertEquals(Sdk.FileLoggingInstall.SESSION_ROOT_UNWRITABLE, outcome) + } + + @Test + fun shouldNameTheBlockedDestinationNotJustTheRoot() { + tmp.newFile("platform_wallet") + + assertEquals( + File(tmp.root, "platform_wallet"), + Sdk.firstUnwritableLogDestination(tmp.root), + ) + } + + @Test + fun shouldRejectADirectorySquattingOnADestinationFile() { + // create(true).append(true).open on a path that is a directory + // fails natively; the probe must catch it up front. + File(tmp.root, "dash_sdk/run.log").mkdirs() + + assertEquals( + File(tmp.root, "dash_sdk/run.log"), + Sdk.firstUnwritableLogDestination(tmp.root), + ) + } } 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 123b9bcf6c0..6c61f681e0f 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,49 @@ class KeystoreDeviceLockedDenialTest { assertTrue(thrown.message.orEmpty().contains("genuinely locked")) } + @Test + fun shouldRethrowAuthGatedAliasUserNotAuthenticatedUnclassified() { + // The auth-gated identity-keys alias' NORMAL pre-prompt contract: + // UserNotAuthenticatedException means "auth window closed" and must + // reach the BiometricGate prompt-and-retry untouched — even while + // the device IS locked. Classifying it as the retryable + // device-locked type would strand the biometric path. + val manager = managerSampling( + DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true), + ) + val authWindowClosed = SimulatedUserNotAuthenticatedException() + + val thrown = assertThrows(SimulatedUserNotAuthenticatedException::class.java) { + manager.rethrowClassifyingDeviceLockedDenial( + authWindowClosed, + KeystoreManager.KEYS_ALIAS_AUTH_GATED, + operation = "decrypt", + ) + } + assertSame(authWindowClosed, thrown) + } + + @Test + fun shouldRethrowCustomAliasDenialUnclassified() { + // encrypt/decrypt accept arbitrary AES aliases, and a + // host-provisioned alias may carry setUserAuthenticationRequired — + // its UserNotAuthenticatedException is ambiguous, so only + // MASTER_ALIAS (contractually never auth-gated) may classify. + val manager = managerSampling( + DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true), + ) + val denial = SimulatedUserNotAuthenticatedException() + + val thrown = assertThrows(SimulatedUserNotAuthenticatedException::class.java) { + manager.rethrowClassifyingDeviceLockedDenial( + denial, + "com.example.host.customAlias", + operation = "encrypt", + ) + } + assertSame(denial, thrown) + } + @Test fun shouldRethrowNonDenialExceptionsUnchanged() { val manager = managerSampling( From 72d280077122cdb2295f9ebac357309cee8ff34e Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:27:00 -0400 Subject: [PATCH 4/4] fix(kotlin-sdk): gate the device-locked fail-fast on the master key's actual lock binding + scrub mnemonic retry buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two PR #4463 review findings: - The createWallet fail-fast pre-check rejected wallet creation whenever KeyguardManager reported the device locked, but an existing MASTER_ALIAS key is not necessarily lock-bound: generateWithLockScreenDegradation deliberately creates it WITHOUT setUnlockedDeviceRequired when no secure lock screen exists, and existing keys are never regenerated — so a user who enrolls a PIN later keeps the unbound key, and master-alias crypto keeps succeeding on the locked device. The pre-check (renamed ensureDeviceUnlocked → ensureMasterKeyNotLockBlocked to match its real contract) now delegates the verdict to the Keystore itself when the sampled state says locked: ONE preflight master-alias encrypt of a non-secret probe byte (the same operation storeMnemonic performs; ciphertext discarded, nothing persisted, never prompts — the master alias carries no auth gate). A denial classified device-locked by the existing MASTER_ALIAS-gated mapper proves the key IS lock-bound and rethrows as the typed KeystoreDeviceLockedException re-labeled with the caller's operation; a successful probe proves the key is unbound and creation proceeds. Any other probe failure logs and proceeds — best-effort, the real operation renders the final verdict on the pre-existing rollback path. No key rotation, no new aliases, no parameter changes: on a fresh install the probe provisions the master key exactly as the first storeMnemonic would have. - storeMnemonic's false-locked retry loop retained a plaintext ByteArray of the complete mnemonic across the whole backoff schedule and never cleared it — success, the final denial, and coroutine cancellation during a backoff delay all left it to the garbage collector. The loop is now wrapped in try/finally with plaintext.fill(0), matching the class's handling of its other raw secret arrays; cancellation during delay runs the finally too (pinned by test). Tests: locked-device + unbound-key pass-through (the blocker's exact scenario), locked + lock-bound typed fail-fast now proven via the Keystore-probe verdict, unlocked states pinned probe-free, and scrub assertions on the success, exhausted-denial, and cancelled-mid-backoff exits. 337 module unit tests green. Co-Authored-By: Claude Fable 5 --- .../dashsdk/security/KeystoreManager.kt | 2 +- .../dashsdk/security/WalletStorage.kt | 115 ++++++++++----- .../dashsdk/wallet/PlatformWalletManager.kt | 31 ++-- .../WalletStorageDeviceLockedRetryTest.kt | 139 ++++++++++++++++-- 4 files changed, 228 insertions(+), 59 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 97ecbb20493..ef99c57f884 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 @@ -130,7 +130,7 @@ open class KeystoreManager( * one prompt-free sample, no Keystore access. Used at throw time by the * device-locked denial mapping (so [KeystoreDeviceLockedException] * records whether the OS agreed the device was locked) and by - * [WalletStorage.ensureDeviceUnlocked]'s createWallet pre-check. + * [WalletStorage.ensureMasterKeyNotLockBlocked]'s createWallet pre-check. * `open` purely as a unit-test seam, like the rest of this class. */ open fun sampleDeviceLockState(): DeviceLockState = deviceLockStateProbe() 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 0ee6b83391c..2a5f8b95235 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 @@ -204,24 +204,63 @@ class WalletStorage( // ── Device lock state ───────────────────────────────────────────── /** - * Fail fast with [KeystoreDeviceLockedException] if `KeyguardManager` - * reports the device LOCKED right now (`isDeviceLocked` — locked AND - * secured). A [MASTER_ALIAS][KeystoreManager.MASTER_ALIAS] Keystore - * operation on a lock-screen device would be denied anyway - * (`setUnlockedDeviceRequired`), so callers about to do irreversible - * orchestration around such an operation — `PlatformWalletManager. - * createWallet`, whose native create precedes [storeMnemonic] — call - * this FIRST and fail with nothing created and nothing to roll back. - * Prompt-free, no Keystore access; a no-op when the device is unlocked + * Fail fast with [KeystoreDeviceLockedException] if the CURRENT device + * lock state would deny [MASTER_ALIAS][KeystoreManager.MASTER_ALIAS] + * Keystore operations. Callers about to do irreversible orchestration + * around such an operation — `PlatformWalletManager.createWallet`, whose + * native create precedes [storeMnemonic] — call this FIRST and fail with + * nothing created and nothing to roll back. + * + * `KeyguardManager.isDeviceLocked` alone cannot decide this: an + * existing master key is not necessarily lock-bound. + * [KeystoreManager]'s `generateWithLockScreenDegradation` deliberately + * generates it WITHOUT `setUnlockedDeviceRequired` when no secure lock + * screen exists, and existing keys are never regenerated — so a user + * who enrolls a PIN later keeps the unbound key, and master-alias + * crypto keeps succeeding on the locked device. When the sampled state + * says locked, the verdict is therefore delegated to the Keystore + * itself: ONE preflight master-alias encrypt of a non-secret probe byte + * (the same operation [storeMnemonic] performs; the ciphertext is + * discarded, nothing is persisted). A denial classified as + * device-locked (see `KeystoreManager.rethrowClassifyingDeviceLockedDenial`) + * proves the key IS lock-bound and rethrows as the typed exception, + * re-labeled with the caller's [operation]; a successful probe proves + * the key is unbound and the caller proceeds. Any OTHER probe failure + * neither proves nor disproves the binding — this pre-check is + * best-effort, so it logs and proceeds, leaving the verdict to the real + * operation (which fails with the pre-existing rollback path). + * + * Never prompts (the master alias carries no + * `setUserAuthenticationRequired` gate) and never rotates, re-keys, or + * 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). */ - fun ensureDeviceUnlocked(operation: String) { + fun ensureMasterKeyNotLockBlocked(operation: String) { val state = keystore.sampleDeviceLockState() - if (state.isDeviceLocked) { + if (!state.isDeviceLocked) return + try { + keystore.encrypt(ByteArray(1)) + Log.i( + TAG, + "$operation: device is locked but the master key is not lock-bound " + + "(generated without setUnlockedDeviceRequired on a then-lockless " + + "device) — proceeding", + ) + } catch (e: KeystoreDeviceLockedException) { throw KeystoreDeviceLockedException( - alias = KeystoreManager.MASTER_ALIAS, + alias = e.alias, operation = operation, - lockState = state, + lockState = e.lockState, + cause = e, + ) + } catch (e: Exception) { + Log.w( + TAG, + "$operation: master-key lock-binding preflight failed unclassified — " + + "proceeding and leaving the verdict to the real operation", + e, ) } } @@ -244,29 +283,39 @@ class WalletStorage( * waiting 2s cannot unlock a phone; the caller retries after unlock. */ suspend fun storeMnemonic(walletId: ByteArray, mnemonic: String) { + // The plaintext copy lives across the whole backoff schedule, so scrub + // it on EVERY exit — success, the final denial propagating, and + // cancellation during a backoff delay — matching this class's + // handling of its other raw secret arrays. val plaintext = mnemonic.encodeToByteArray() - var attempt = 0 - while (true) { - try { - val blob = keystore.encrypt(plaintext) - store.edit { it[mnemonicKey(walletId)] = encode(blob) } - return - } catch (e: KeystoreDeviceLockedException) { - if (e.deviceReportsLocked || attempt >= DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS.size) { - throw e + try { + var attempt = 0 + while (true) { + try { + val blob = keystore.encrypt(plaintext) + store.edit { it[mnemonicKey(walletId)] = encode(blob) } + return + } catch (e: KeystoreDeviceLockedException) { + if (e.deviceReportsLocked || + attempt >= DEVICE_FALSE_LOCKED_RETRY_DELAYS_MS.size + ) { + throw e + } + 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) } - 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) } + } finally { + plaintext.fill(0) } } 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 2c302235637..e1360fcdf57 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 @@ -744,12 +744,16 @@ class PlatformWalletManager( * `PlatformWalletManager.createWallet(..., birthHeight:)` * (`birthHeight: showImportOption ? 0 : nil`). * @throws org.dashfoundation.dashsdk.security.KeystoreDeviceLockedException - * (RETRYABLE after unlock) if `KeyguardManager` reports the device - * locked at entry — thrown BEFORE the native create, so nothing was + * (RETRYABLE after unlock) if the device is locked at entry AND the + * master key is actually lock-bound — decided by the Keystore itself + * via [WalletStorage.ensureMasterKeyNotLockBlocked]'s preflight + * 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). + * the full rollback below first). A locked device whose master key is + * NOT lock-bound (generated before a PIN was enrolled) proceeds + * normally. */ suspend fun createWallet( mnemonic: String, @@ -757,15 +761,18 @@ class PlatformWalletManager( createDefaultAccounts: Boolean = true, birthHeight: UInt? = null, ): ManagedPlatformWallet = withContext(Dispatchers.IO) { - // Fail-fast pre-check BEFORE the native create: on a genuinely - // locked device (KeyguardManager.isDeviceLocked) the storeMnemonic - // step below is guaranteed to be denied by the lock-bound - // MASTER_ALIAS key (setUnlockedDeviceRequired), which would force - // the full rollback dance. Failing here instead means no native - // wallet was created, no Room rows were written, and there is - // nothing to roll back — the typed KeystoreDeviceLockedException - // tells the caller to retry after unlock. - walletStorage.ensureDeviceUnlocked(operation = "createWallet") + // Fail-fast pre-check BEFORE the native create: on a locked device + // whose MASTER_ALIAS key is lock-bound (setUnlockedDeviceRequired) + // the storeMnemonic step below is guaranteed to be denied, which + // would force the full rollback dance. The pre-check preflights one + // master-alias encrypt so the Keystore itself renders that verdict + // (a key generated before any PIN existed carries no lock binding + // and keeps working while locked — creation must proceed then). + // Failing here instead means no native wallet was created, no Room + // rows were written, and there is nothing to roll back — the typed + // KeystoreDeviceLockedException tells the caller to retry after + // unlock. + walletStorage.ensureMasterKeyNotLockBlocked(operation = "createWallet") // Caller-allocated out-buffers: the JNI side validates both BEFORE // the native create so no fallible allocation follows the // persistence commit (a post-commit publish failure would strand 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 9f625a8a508..8ebac6044d6 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 @@ -1,7 +1,11 @@ package org.dashfoundation.dashsdk.security import androidx.test.core.app.ApplicationProvider +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertThrows @@ -17,9 +21,13 @@ import org.robolectric.RobolectricTestRunner * [KeystoreManager.MASTER_ALIAS] encrypt as device-locked during * `createWallet`, on devices that were demonstrably unlocked): * - * 1. [WalletStorage.ensureDeviceUnlocked] — the `createWallet` fail-fast - * pre-check: throws the typed, retryable [KeystoreDeviceLockedException] - * on a genuinely locked device BEFORE any native wallet exists. + * 1. [WalletStorage.ensureMasterKeyNotLockBlocked] — the `createWallet` + * fail-fast pre-check: throws the typed, retryable + * [KeystoreDeviceLockedException] BEFORE any native wallet exists when + * the device is locked AND the master key is actually lock-bound — + * decided by a preflight master-alias probe encrypt, because a key + * generated on a then-lockless device carries no lock binding and must + * not block creation. * 2. [WalletStorage.storeMnemonic]'s bounded FALSE-LOCKED retry: a denial * whose sampled `KeyguardManager` state says the device is NOT locked * (the Keystore2 misreporting defect) is retried up to 3 times; a @@ -53,23 +61,29 @@ class WalletStorageDeviceLockedRetryTest { @Test fun shouldFailFastWhenDeviceIsGenuinelyLocked() { + // masterKeyLockBound defaults true: the key carries + // setUnlockedDeviceRequired, so the Keystore denies the probe. fake.lockState = DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true) val thrown = assertThrows(KeystoreDeviceLockedException::class.java) { - storage.ensureDeviceUnlocked(operation = "createWallet") + storage.ensureMasterKeyNotLockBlocked(operation = "createWallet") } assertEquals("createWallet", thrown.operation) assertEquals(KeystoreManager.MASTER_ALIAS, thrown.alias) assertTrue(thrown.deviceReportsLocked) - // Prompt-free and Keystore-free: the pre-check must never touch the - // master key (nothing to retry, nothing to roll back). - assertEquals(0, fake.masterEncryptCalls) + // The verdict came from the Keystore itself: exactly one preflight + // probe encrypt, whose classified denial rides along as the cause. + assertEquals(1, fake.masterEncryptCalls) + assertTrue(thrown.cause is KeystoreDeviceLockedException) } @Test fun shouldPassPreCheckWhenDeviceIsUnlocked() { fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) - storage.ensureDeviceUnlocked(operation = "createWallet") // must not throw + storage.ensureMasterKeyNotLockBlocked(operation = "createWallet") // must not throw + // Unlocked is decided from KeyguardManager alone — prompt-free AND + // Keystore-free (no probe). + assertEquals(0, fake.masterEncryptCalls) } @Test @@ -78,7 +92,27 @@ class WalletStorageDeviceLockedRetryTest { // screen): the Keystore unlocked-device gate keys off the SECURE // lock, so this state must not block wallet creation. fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = true) - storage.ensureDeviceUnlocked(operation = "createWallet") // must not throw + storage.ensureMasterKeyNotLockBlocked(operation = "createWallet") // must not throw + assertEquals(0, fake.masterEncryptCalls) + } + + @Test + fun shouldPassPreCheckWhenDeviceIsLockedButMasterKeyIsNotLockBound() { + // A master key generated while the device had NO secure lock screen + // carries no setUnlockedDeviceRequired + // ([KeystoreManager]'s generateWithLockScreenDegradation) and existing + // keys are never regenerated — so after the user later enrolls a PIN, + // master-alias crypto still succeeds on the locked device and wallet + // creation must proceed. KeyguardManager.isDeviceLocked alone cannot + // decide this; only the Keystore can. + fake.lockState = DeviceLockState(isDeviceLocked = true, isKeyguardLocked = true) + fake.masterKeyLockBound = false + + storage.ensureMasterKeyNotLockBlocked(operation = "createWallet") // must not throw + + // The verdict came from the Keystore itself: exactly one preflight + // probe encrypt (discarded, nothing persisted). + assertEquals(1, fake.masterEncryptCalls) } // ── storeMnemonic bounded FALSE-LOCKED retry ───────────────────────── @@ -143,14 +177,77 @@ class WalletStorageDeviceLockedRetryTest { assertEquals(1, fake.masterEncryptCalls) assertEquals(mnemonic, storage.retrieveMnemonic(walletId)) } + + // ── storeMnemonic plaintext-buffer scrubbing ───────────────────────── + + @Test + fun shouldScrubMnemonicBufferAfterSuccessfulStore() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + + storage.storeMnemonic(walletId, mnemonic) + + // The encrypt really saw the phrase... + assertArrayEquals(mnemonic.encodeToByteArray(), fake.lastMasterPlaintextAtCall) + // ...and the retained plaintext copy was zeroed before returning. + assertBufferScrubbed(fake.lastMasterPlaintextRef) + // Scrubbing the input buffer must not corrupt what was stored. + 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 + + var thrown = false + try { + storage.storeMnemonic(walletId, mnemonic) + } catch (e: KeystoreDeviceLockedException) { + thrown = true + } + + assertTrue("expected the typed denial to propagate", thrown) + assertBufferScrubbed(fake.lastMasterPlaintextRef) + } + + @Test + fun shouldScrubMnemonicBufferWhenCancelledDuringRetryBackoff() = runBlocking { + fake.lockState = DeviceLockState(isDeviceLocked = false, isKeyguardLocked = false) + fake.failMasterEncrypts = Int.MAX_VALUE // park storeMnemonic in its backoff delay + val firstAttempt = CompletableDeferred() + fake.onMasterEncrypt = { firstAttempt.complete(Unit) } + + val job = launch { storage.storeMnemonic(walletId, mnemonic) } + firstAttempt.await() + // The first denial has happened; storeMnemonic is in (or headed into) + // its backoff delay — the retry loop's only suspension point, where + // this cancellation lands. join returns only after the coroutine has + // fully completed, finally blocks included. + job.cancelAndJoin() + + assertBufferScrubbed(fake.lastMasterPlaintextRef) + } + + private fun assertBufferScrubbed(buffer: ByteArray?) { + assertTrue("expected the plaintext buffer to have been captured", buffer != null) + assertTrue( + "expected the retained mnemonic plaintext buffer to be zeroed", + buffer!!.all { it == 0.toByte() }, + ) + } } /** * Master-alias-focused fake: scripts [failMasterEncrypts] device-locked * denials (each carrying [lockState] sampled "at throw time", as the real * mapping does) before letting encrypts succeed with a trivially reversible - * blob. Identity-key aliases are out of scope here — see - * [WalletStorageUpgradeMatrixTest]'s fake for that ladder. + * blob. [masterKeyLockBound] models the key's effective policy: when true + * (the default — a key generated on a lock-screen device carries + * `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. */ private class FalseLockedFakeKeystoreManager : KeystoreManager() { @@ -158,13 +255,29 @@ private class FalseLockedFakeKeystoreManager : KeystoreManager() { var failMasterEncrypts = 0 var masterEncryptCalls = 0 + /** Whether the fake master key carries the unlocked-device requirement. */ + var masterKeyLockBound = true + + /** The exact buffer reference the last master encrypt received. */ + var lastMasterPlaintextRef: ByteArray? = null + + /** Snapshot of that buffer's content AT CALL TIME (pre-scrub evidence). */ + var lastMasterPlaintextAtCall: 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++ - if (failMasterEncrypts > 0) { - failMasterEncrypts-- + 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",