Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,35 @@ sealed class DashSdkError(
cause,
)

/**
* A state transition could not be signed because the signer has no
* usable private key for the requested public key — the stored blob
* is missing (never derived, wiped, or written under a different
* Keystore alias/policy) rather than the operation itself failing.
*
* Signing failures originate as free text in
* `KeystoreSigner.completeSign` (the "[MESSAGE_MARKER] <pubkeyHex>"
* string), travel through Rust, and come back under the catch-all
* platform-wallet codes; [fromPlatformWalletNative] recognizes the
* marker on the Kotlin boundary and surfaces this typed error so
* hosts can route users to key repair (e.g.
* `PlatformWalletManager.repairIdentityKey`) instead of treating it
* as an opaque [Generic] failure. Not retryable as-is — the key must
* be (re-)derived first.
*/
class SigningKeyUnavailable(message: String, cause: Throwable? = null) :
PlatformWallet(message, cause) {
companion object {
/**
* Stable prefix of the `KeystoreSigner` "missing key"
* completion error. `KeystoreSigner` builds its message from
* this constant, so the emitter and the matcher cannot
* drift.
*/
const val MESSAGE_MARKER = "no private key stored for"
}
}

/**
* Any other `PlatformWalletFFIResultCode` without a dedicated type.
* Carries the platform-wallet [nativeCode] (already de-offset) and
Expand Down Expand Up @@ -186,12 +215,24 @@ sealed class DashSdkError(
}
}

/**
* `PlatformWalletFFIResultCode::NotFound` (98) — the code the FFI's
* blanket `Option → result` conversion emits for every "requested
* <thing> not found" miss (e.g. an identity id that is not managed
* by the wallet).
*/
const val PLATFORM_WALLET_NOT_FOUND_CODE = 98

/**
* Map a de-offset `PlatformWalletFFIResultCode` value into the
* [PlatformWallet] subtree — mirror of Swift's
* `PlatformWalletError(result:)` construction. Retry-semantics-bearing
* codes get dedicated types; the rest fall through to
* [PlatformWallet.Generic].
* [PlatformWallet.Generic] — except the `KeystoreSigner` "missing
* key" completion error, which travels as free text through Rust and
* is recognized by its [PlatformWallet.SigningKeyUnavailable]
* message marker here (only on the catch-all codes, so the dedicated
* retry-semantics types are never overridden).
*/
private fun fromPlatformWalletNative(
code: Int,
Expand All @@ -200,13 +241,26 @@ sealed class DashSdkError(
): DashSdkError = when (code) {
// PlatformWalletFFIResultCode variants (platform-wallet-ffi/src/error.rs)
1 -> PlatformWallet.InvalidHandle(message, cause) // ErrorInvalidHandle
6 -> PlatformWallet.WalletOperation(message, cause) // ErrorWalletOperation
6 -> // ErrorWalletOperation
if (isSigningKeyUnavailable(message)) {
PlatformWallet.SigningKeyUnavailable(message, cause)
} else {
PlatformWallet.WalletOperation(message, cause)
}
16 -> PlatformWallet.ShieldedBroadcastFailed(message, cause) // ErrorShieldedBroadcastFailed
18 -> PlatformWallet.ShieldedSpendUnconfirmed(message, cause) // ErrorShieldedSpendUnconfirmed
19 -> PlatformWallet.ShieldedNoRecordedAnchor(message, cause) // ErrorShieldedNoRecordedAnchor
20 -> PlatformWallet.TransactionBroadcastUnconfirmed(message, cause) // ErrorTransactionBroadcastUnconfirmed
else -> PlatformWallet.Generic(code, message, cause)
else ->
if (isSigningKeyUnavailable(message)) {
PlatformWallet.SigningKeyUnavailable(message, cause)
} else {
PlatformWallet.Generic(code, message, cause)
}
}

private fun isSigningKeyUnavailable(message: String): Boolean =
message.contains(PlatformWallet.SigningKeyUnavailable.MESSAGE_MARKER)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import android.util.Log
import androidx.room.withTransaction
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.asCoroutineDispatcher
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import org.dashfoundation.dashsdk.ffi.AccountSpecData
Expand Down Expand Up @@ -122,6 +125,44 @@ class PlatformWalletPersistenceHandler(
/** Open rounds keyed by walletId hex (a round is per-walletId). */
private val buffers = HashMap<String, ChangesetBuffer>()

/**
* An identity key whose private-half derivation/storage failed — the
* key was persisted **watch-only** and cannot sign until re-derived
* (e.g. via `PlatformWalletManager.repairIdentityKey`).
*/
data class PendingIdentityKey(
/** Hex of the wallet the key belongs to. */
val walletIdHex: String,
/** Base58 of the owning identity id. */
val identityIdBase58: String,
/** On-identity key id. */
val keyId: Int,
/** Lowercase hex of the compressed public key (the storage key). */
val publicKeyHex: String,
/** Derivation breadcrumb: identity index. */
val identityIndex: Int,
/** Derivation breadcrumb: key index. */
val keyIndex: Int,
/** Human-readable failure reason (exception message or contract miss). */
val reason: String,
/** Epoch millis of the (latest) failure. */
val failedAtMs: Long,
)

private val _pendingIdentityKeys =
MutableStateFlow<Map<String, PendingIdentityKey>>(emptyMap())

/**
* Queryable "keys pending" state: identity keys whose private half
* could not be derived/stored by [onPersistIdentityKeyUpsert] (keyed by
* public-key hex). Such keys are persisted watch-only — signing with
* them fails — so hosts should watch this flow and surface a repair
* path. An entry clears automatically when a later persist round (or an
* explicit re-derive that replays the upsert) stores the key.
*/
val pendingIdentityKeys: StateFlow<Map<String, PendingIdentityKey>> =
_pendingIdentityKeys.asStateFlow()

/**
* Stage a write. If a round is open for [walletId] the op is buffered
* for the round's single transaction; otherwise it runs immediately
Expand Down Expand Up @@ -783,17 +824,50 @@ class PlatformWalletPersistenceHandler(
val deriver = privateKeyDeriver
val derivedKeychainId: String? =
if (derivationIndicesIsSome && deriver != null && !readOnly) {
runCatching {
val keyOwnerWalletId = if (walletIdIsSome) keyWalletId else walletId
val outcome = runCatching {
deriver.deriveAndStore(
walletId = if (walletIdIsSome) keyWalletId else walletId,
walletId = keyOwnerWalletId,
publicKeyData = publicKeyData,
identityIndex = identityIndex,
keyIndex = keyIndex,
)
}.getOrElse { t ->
Log.w(TAG, "identity private-key derive/store failed; key stays watch-only", t)
null
}
val id = outcome.getOrNull()
if (id != null) {
// Stored — clear any earlier failure for this pubkey.
clearPendingIdentityKey(publicKeyData.toHex())
} else {
// NOT silent (dashpay/platform#4053): the key is being
// persisted watch-only, so every signature with it will
// fail until it is re-derived. Log loudly and record a
// queryable pending entry (see [pendingIdentityKeys]).
val reason = outcome.exceptionOrNull()?.let { t ->
t.message ?: t.javaClass.simpleName
} ?: "deriver returned no storage identifier"
Log.e(
TAG,
"identity private-key derive/store FAILED — key " +
"${publicKeyData.toHex()} (identity ${identityId.toBase58String()}, " +
"keyId $keyId, slot $identityIndex/$keyIndex) is persisted " +
"WATCH-ONLY and cannot sign until re-derived " +
"(see PlatformWalletPersistenceHandler.pendingIdentityKeys): $reason",
outcome.exceptionOrNull(),
)
recordPendingIdentityKey(
PendingIdentityKey(
walletIdHex = keyOwnerWalletId.toHex(),
identityIdBase58 = identityId.toBase58String(),
keyId = keyId,
publicKeyHex = publicKeyData.toHex(),
identityIndex = identityIndex,
keyIndex = keyIndex,
reason = reason,
failedAtMs = System.currentTimeMillis(),
),
)
}
id
} else {
null
}
Expand Down Expand Up @@ -2056,6 +2130,34 @@ class PlatformWalletPersistenceHandler(
}
}

// ── Pending identity-key bookkeeping (#4053) ──────────────────────

private fun recordPendingIdentityKey(entry: PendingIdentityKey) {
_pendingIdentityKeys.value = _pendingIdentityKeys.value + (entry.publicKeyHex to entry)
}

private fun clearPendingIdentityKey(publicKeyHex: String) {
if (publicKeyHex in _pendingIdentityKeys.value) {
_pendingIdentityKeys.value = _pendingIdentityKeys.value - publicKeyHex
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Concurrent pending-key updates can overwrite each other

recordPendingIdentityKey and clearPendingIdentityKey perform non-atomic read-modify-write operations on MutableStateFlow.value. The persistence callback runs synchronously on its Rust caller thread, while the newly added markIdentityKeyRepaired can be invoked from an arbitrary host thread through repairIdentityKey. If a repair clears key A while a persistence callback records key B, both can read the same map and one write can discard the other. Losing the record leaves a watch-only key without the queryable pending state this PR adds; losing the clear leaves a repaired key stale. Use MutableStateFlow.update or another atomic synchronization mechanism for both mutations.

source: ['sonnet5-security-auditor']

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in f8dd367Concurrent pending-key updates can overwrite each other no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

}

/**
* Drop [publicKeyHex] from [pendingIdentityKeys] after a successful
* out-of-band repair.
*
* [onPersistIdentityKeyUpsert] is the only *persist-callback* path that
* clears a pending entry, but [org.dashfoundation.dashsdk.wallet.PlatformWalletManager.repairIdentityKey]
* re-derives and stores the private key directly through the deriver,
* bypassing that callback — so it must call this on success or a repaired
* key would linger in [pendingIdentityKeys] until an unrelated re-persist
* happens to fire for the same key. Idempotent: clearing an absent key is a
* no-op.
*/
internal fun markIdentityKeyRepaired(publicKeyHex: String) {
clearPendingIdentityKey(publicKeyHex)
}

// ── Error / threading guards ──────────────────────────────────────

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package org.dashfoundation.dashsdk.security

/**
* Security policy for the Keystore alias that wraps **identity private
* keys** ([WalletStorage.storePrivateKey] / [WalletStorage.retrievePrivateKey]).
*
* The SDK's default alias gates every identity-key decrypt behind Android
* user authentication (biometric / device credential) with a short
* post-unlock validity window — the right model when the SDK owns the
* auth UX. Host apps that already gate wallet access behind their own
* auth model (e.g. an app-level PIN that decrypts the wallet) end up with
* a *second*, redundant auth prompt at signing time — and signing fails
* outside the ~30 s window entirely when no [BiometricGate] is wired.
* [DEVICE_BOUND] lets such hosts opt into a non-gated (but still
* hardware-backed, non-exportable) wrapping key instead.
*
* ## Semantics
*
* - [AUTH_GATED] — the default; behavior matches the historical one. New
* identity keys are wrapped under [KeystoreManager.KEYS_ALIAS_AUTH_GATED]
* (the legacy [KeystoreManager.KEYS_ALIAS] is kept read-only so pre-RSA
* blobs migrate rather than strand): encrypt (store) never prompts, decrypt
* (sign) requires user authentication within
* [KeystoreManager.AUTH_VALIDITY_SECONDS] of a biometric / device-credential
* auth, re-promptable through a [BiometricGate].
* - [DEVICE_BOUND] — identity keys are wrapped under the separate
* [KeystoreManager.KEYS_ALIAS_DEVICE_BOUND] alias: the same
* StrongBox-preferring, non-exportable RSA wrapping pair, but with **no**
* `setUserAuthenticationRequired` gate, so decrypts never throw
* `UserNotAuthenticatedException` and never need a [BiometricGate].
* Keys remain bound to this device's Keystore (`setUnlockedDeviceRequired`
* still applies) — the host app is responsible for gating *access* to
* signing flows (PIN, biometrics, session policy) itself.
*
* ## Choosing and switching
*
* The two policies use **distinct Keystore aliases**, and a blob written
* under one alias can only be decrypted by that alias's private key. Pick
* the policy once per install and construct [WalletStorage] /
* [KeystoreManager] with it consistently: switching an existing install to
* the other policy leaves previously stored identity keys undecryptable
* (they surface through the key-health / re-derive path, e.g.
* `PlatformWalletManager.repairIdentityKey`, which re-encrypts under the
* current policy's alias). Mnemonics ([KeystoreManager.MASTER_ALIAS]) are
* unaffected — this policy governs identity keys only.
*/
enum class KeySecurityPolicy {
/**
* Identity-key decrypts require Android user authentication within
* [KeystoreManager.AUTH_VALIDITY_SECONDS] (the historical default).
*/
AUTH_GATED,

/**
* Identity-key decrypts are hardware-backed but not auth-gated; the
* host app supplies its own authentication model.
*/
DEVICE_BOUND,
}
Loading
Loading