-
Notifications
You must be signed in to change notification settings - Fork 58
fix(kotlin-sdk): typed retryable device-locked keystore errors + createWallet fail-fast #4463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b1ab209
6e9a8c8
16c4afb
72d2800
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,38 @@ 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 — 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, | ||
| } | ||
|
|
||
| @Serializable | ||
| private data class MasternodesEnvelope( | ||
| val success: Boolean, | ||
|
|
@@ -197,23 +230,200 @@ 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 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, 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)" | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 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 = | ||
| 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], | ||
| * 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 enableFileLogging(level: LogLevel = LogLevel.DEBUG, sessionRoot: String): Boolean { | ||
| fun installFileLogging( | ||
| level: LogLevel = LogLevel.DEBUG, | ||
| sessionRoot: String, | ||
| ): FileLoggingInstall { | ||
| val root = File(sessionRoot) | ||
| val blocked = firstUnwritableLogDestination(root) | ||
| if (blocked != null) { | ||
| android.util.Log.w( | ||
| LOG_TAG, | ||
| "SDK file logging NOT installed: session-root log " + | ||
| "destination cannot be created/written: " + | ||
| blocked.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 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}: 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 | ||
|
Comment on lines
+330
to
+353
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: A writable root does not prove native failure means ALREADY_SET The native boolean remains ambiguous after probing only the session root. source: ['codex']
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 16c4afb, taking your second option — validating every destination the native installer requires — since the native boundary itself is out of scope for this Kotlin-side PR. The pre-native gate now probes the full fixed destination set |
||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 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]). | ||
| */ | ||
| 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 | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.