From 8b0acb649d9389e51c371a02af1103c1f2df4eff Mon Sep 17 00:00:00 2001 From: Andrea-lyz Date: Sat, 16 May 2026 18:43:09 +0200 Subject: [PATCH 1/2] fix: persist symmetric keys + byte-identical metadata; stop wiping keys on keybox edits Five issues that together caused keystore-pinned apps to be silently logged out across reboots and config changes. All flow from the same root cause: GeneratedKeyPersistence loses information on save -> reload. 1. Symmetric keys (AES, HMAC, 3DES) were never persisted at all - GeneratedKeyPersistence.save only accepted KeyPair, ignoring SecretKey - AndroidX security MasterKey (AES-GCM-256) regenerated on every reboot, making EncryptedSharedPreferences undecryptable - Apps that wrap session tokens in EncryptedSharedPreferences interpret this as session expiry and force a relogin 2. Restored KeyMetadata authorizations differed from generation-time bytes - loadPersistedKeys rebuilt KeyMintAttestation with mostly null/empty fields, so toAuthorizations emitted a different tag set after reboot vs. at generateKey time - Apps that fingerprint metadata across keystore calls saw a "changed key" 3. certificate / certificateChain split could shift after restore - buildKeyEntryResponse called updateCertificateChain on the rebuilt metadata, which is allowed to repartition leaf vs. chain bytes - Apps with strict leaf fingerprint checks saw a "changed cert" 4. Touching ANY .xml under /data/adb/tricky_store wiped every cached key - ConfigObserver called clearAllGeneratedKeys() which also calls GeneratedKeyPersistence.deleteAll() - Editing keybox.xml (or any unrelated .xml) thus deleted every persisted key on disk - Even the keybox-cache argument does not justify wiping per-app keys: patched chains alone are stale, raw keypairs are not 5. SoftwareOperation NPE when restored keyParams missed PURPOSE tag - Init dereferenced keyPair!! before checking purpose, so a half-restored record crashed instead of producing a clean error Single on-disk format (FORMAT_VERSION = 3) covers everything: PKCS8 private key bytes for asymmetric, raw secret bytes for symmetric, plus the byte-identical KeyMetadata parcel snapshot so authorizations restore exactly. Earlier dev-only formats are silently skipped by the loader; the next generateKey for those aliases re-creates them in v3. ConfigObserver now calls invalidatePatchedChains() instead of clearAllGeneratedKeys() on .xml edits - only the chain cache is stale, not the underlying keypairs. Tested on OnePlus 13 (Android 16, KSU 3.2.4): - Apps survive force-stop + cold reboot without losing keystore state - Apps survive keybox.xml edits / replacements (touch, sed, cp -mv) - Tamper score still 4 (CONSISTENT) on Duck Detector - KeyAttestation chain output unchanged --- .../config/ConfigurationManager.kt | 9 +- .../keystore/Keystore2Interceptor.kt | 38 +++ .../keystore/shim/GeneratedKeyPersistence.kt | 135 +++++++- .../shim/KeyMintSecurityLevelInterceptor.kt | 290 ++++++++++++++---- .../keystore/shim/SoftwareOperation.kt | 54 +++- 5 files changed, 448 insertions(+), 78 deletions(-) diff --git a/app/src/main/java/org/matrix/TEESimulator/config/ConfigurationManager.kt b/app/src/main/java/org/matrix/TEESimulator/config/ConfigurationManager.kt index f5f6c4a..db1ed97 100644 --- a/app/src/main/java/org/matrix/TEESimulator/config/ConfigurationManager.kt +++ b/app/src/main/java/org/matrix/TEESimulator/config/ConfigurationManager.kt @@ -297,10 +297,15 @@ object ConfigurationManager { ) KeyBoxManager.invalidateCache(path) if (Build.VERSION.SDK_INT > Build.VERSION_CODES.R) { - // Clear cached keys possibly containing old certificates + // Drop only the patched cert chains so the next + // attestation request re-signs with the new keybox. + // Do NOT drop generatedKeys — that would destroy + // every alias/private key in memory and on disk, + // logging users out of any app that pinned a + // persisted keystore alias. org.matrix.TEESimulator.interception.keystore.shim .KeyMintSecurityLevelInterceptor - .clearAllGeneratedKeys("updating $file") + .invalidatePatchedChains("updating $file") } } } diff --git a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.kt b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.kt index a8405c0..cf61c43 100644 --- a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.kt +++ b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.kt @@ -210,6 +210,28 @@ object Keystore2Interceptor : AbstractKeystoreInterceptor() { } if (descriptor.alias == null) { + if (descriptor.domain == Domain.KEY_ID) { + // The probe pipeline (and some AOSP callers) switch follow-up + // operations to KEY_ID semantics after generateKey returns a + // KEY_ID descriptor. Without this branch, our software keys + // are invisible to KEY_ID-based getKeyEntry calls and the + // request falls through to the real keystore2 daemon, which + // legitimately responds with KEY_NOT_FOUND. Duck Detector's + // TimingSideChannelProbe captures that exception during its + // warmup phase and surfaces it as + // "Captured private binder exception during timing skip". + // Resolving by KEY_ID and returning the cached response keeps + // the call on the happy path, eliminating the warmup signal. + val info = KeyMintSecurityLevelInterceptor.findGeneratedKeyByKeyId( + callingUid, descriptor.nspace + ) + if (info?.response != null) { + SystemLogger.info( + "[TX_ID: $txId] Found generated response via KEY_ID nspace=${descriptor.nspace}" + ) + return InterceptorUtils.createTypedObjectReply(info.response) + } + } return TransactionResult.ContinueAndSkipPost } val keyId = KeyIdentifier(callingUid, descriptor.alias) @@ -387,9 +409,24 @@ object Keystore2Interceptor : AbstractKeystoreInterceptor() { ) KeyMintSecurityLevelInterceptor.attestationKeys.add(keyId) + // Snapshot metadata bytes for the same reason as the + // primary doSoftwareKeyGen path — loss-less restore + // after reboot. + val metadataBytesForPersist = response.metadata?.let { md -> + runCatching { + val parcel = android.os.Parcel.obtain() + try { + md.writeToParcel(parcel, 0) + parcel.marshall() + } finally { + parcel.recycle() + } + }.getOrNull() + } GeneratedKeyPersistence.save( keyId = keyId, keyPair = keyData.first, + secretKey = null, nspace = newNspace, securityLevel = response.metadata.keySecurityLevel, certChain = keyData.second, @@ -399,6 +436,7 @@ object Keystore2Interceptor : AbstractKeystoreInterceptor() { purposes = parsedParameters.purpose, digests = parsedParameters.digest, isAttestationKey = true, + metadataBytes = metadataBytesForPersist, ) return InterceptorUtils.createTypedObjectReply(response) diff --git a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/GeneratedKeyPersistence.kt b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/GeneratedKeyPersistence.kt index 3f2e76e..3508875 100644 --- a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/GeneratedKeyPersistence.kt +++ b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/GeneratedKeyPersistence.kt @@ -29,13 +29,47 @@ data class PersistedKeyData( val ecCurve: Int, val purposes: List, val digests: List, + /** PKCS#8-encoded private key for asymmetric records, empty for symmetric. */ val privateKeyBytes: ByteArray, val certChainBytes: List, + /** + * Byte-identical KeyMetadata parcel snapshot. Restoring authorizations + * directly from these bytes preserves tag count, order, and exact + * security-level annotations across reboots — the kind of structural + * details apps fingerprint to decide whether the alias is still + * "the same key". + */ + val metadataBytes: ByteArray, + /** + * Raw secret material for symmetric records (AES, HMAC, 3DES). Empty + * for asymmetric. Critical for AndroidX security crypto MasterKey + * (AES-GCM-256) — without this every reboot regenerates a fresh AES + * key and EncryptedSharedPreferences becomes undecryptable, which is + * what banking apps interpret as session expiry and force a relogin. + */ + val symmetricKeyBytes: ByteArray, + val symmetricAlgorithm: String, ) object GeneratedKeyPersistence { - private const val FORMAT_VERSION = 1 + /** + * Single source of truth for the on-disk format. Bump this every time + * the layout changes; older numbers are silently skipped on read so + * stale dev artifacts and pre-fix upstream files can't be partially + * rehydrated into broken in-memory state. + * + * History: + * 1 — original upstream layout (no metadata snapshot, no symmetric + * block; restored keys lose authorization tags and AES master + * keys altogether — apps relying on persisted keystore state + * across reboots get logged out) + * 2 — transitional dev-only format that added metadata but still + * missed the symmetric block; never shipped + * 3 — current: byte-identical KeyMetadata snapshot + raw symmetric + * key material so AES/HMAC keys survive reboots + */ + private const val FORMAT_VERSION = 3 private val PERSISTENCE_DIR = File(CONFIG_PATH, "persistent_keys") // Per-filename locks to prevent concurrent writes to the same key file @@ -47,7 +81,8 @@ object GeneratedKeyPersistence { fun save( keyId: KeyIdentifier, - keyPair: KeyPair, + keyPair: KeyPair?, + secretKey: javax.crypto.SecretKey?, nspace: Long, securityLevel: Int, certChain: List, @@ -57,7 +92,11 @@ object GeneratedKeyPersistence { purposes: List, digests: List, isAttestationKey: Boolean, + metadataBytes: ByteArray? = null, ) { + require(keyPair != null || secretKey != null) { + "Either keyPair or secretKey must be provided" + } val filename = keyFileName(keyId.uid, keyId.alias) val lock = getLockForKey(filename) SystemLogger.debug("[Persistence] Acquiring lock for $filename") @@ -87,7 +126,8 @@ object GeneratedKeyPersistence { out.writeInt(digests.size) digests.forEach { out.writeInt(it) } - val pkBytes = keyPair.private.encoded + // Asymmetric key block (empty for symmetric-only). + val pkBytes = keyPair?.private?.encoded ?: ByteArray(0) out.writeInt(pkBytes.size) out.write(pkBytes) @@ -97,6 +137,23 @@ object GeneratedKeyPersistence { out.writeInt(encoded.size) out.write(encoded) } + + // Metadata snapshot (always present, may be empty + // if the live KeyMetadata could not be marshalled). + val mdBytes = metadataBytes ?: ByteArray(0) + out.writeInt(mdBytes.size) + if (mdBytes.isNotEmpty()) out.write(mdBytes) + + // Symmetric key block (empty for asymmetric keys). + if (secretKey != null) { + val skBytes = secretKey.encoded + out.writeUTF(secretKey.algorithm) + out.writeInt(skBytes.size) + out.write(skBytes) + } else { + out.writeUTF("") + out.writeInt(0) + } } } catch (e: Exception) { tmpFile.delete() @@ -189,8 +246,17 @@ object GeneratedKeyPersistence { DataInputStream(BufferedInputStream(FileInputStream(file))).use { input -> val version = input.readInt() if (version != FORMAT_VERSION) { - SystemLogger.warning( - "Skipping ${file.name}: unknown format version $version" + // Old upstream files (v1) and dev-only intermediate + // files (v2) are missing the metadata snapshot + // and/or symmetric key block — restoring them + // would put broken state in memory (apps relying + // on those records get logged out). Skip and let + // the next generateKey re-create cleanly with the + // new format. Affected apps re-login once after + // upgrade, then never again. + SystemLogger.info( + "Skipping ${file.name}: legacy format version $version. " + + "It will be replaced on next generateKey for this alias." ) return@runCatching } @@ -212,7 +278,7 @@ object GeneratedKeyPersistence { val pkLen = requireBounds(input.readInt(), 8192, "pkLen") val pkBytes = ByteArray(pkLen) - input.readFully(pkBytes) + if (pkLen > 0) input.readFully(pkBytes) val certCount = requireBounds(input.readInt(), 10, "certCount") val certChainBytes = (0 until certCount).map { @@ -222,6 +288,17 @@ object GeneratedKeyPersistence { certBytes } + val metaLen = requireBounds(input.readInt(), 256 * 1024, "metaLen") + val metadataBytes = ByteArray(metaLen).also { + if (metaLen > 0) input.readFully(it) + } + + val skAlgo = input.readUTF() + val skLen = requireBounds(input.readInt(), 8192, "skLen") + val skBytes = ByteArray(skLen).also { + if (skLen > 0) input.readFully(it) + } + if (storedSecLevel == securityLevel) { result.add( PersistedKeyData( @@ -237,6 +314,9 @@ object GeneratedKeyPersistence { digests = digests, privateKeyBytes = pkBytes, certChainBytes = certChainBytes, + metadataBytes = metadataBytes, + symmetricKeyBytes = skBytes, + symmetricAlgorithm = skAlgo, ) ) } @@ -292,7 +372,7 @@ object GeneratedKeyPersistence { DataInputStream(BufferedInputStream(FileInputStream(existing))).use { input -> val version = input.readInt() if (version != FORMAT_VERSION) { - SystemLogger.warning("rePersist: unknown format version $version for $keyId") + SystemLogger.warning("rePersist: legacy format version $version for $keyId, will not re-persist (next generateKey replaces it)") return } readPersistedKeyData(input) @@ -303,10 +383,29 @@ object GeneratedKeyPersistence { return } - val keyPair = generatedKeyInfo.keyPair ?: return + val keyPair = generatedKeyInfo.keyPair + val secretKey = generatedKeyInfo.secretKey + if (keyPair == null && secretKey == null) { + SystemLogger.warning("rePersist: no key material for $keyId") + return + } + // Serialize the live KeyMetadata (now contains the user-installed cert + // chain via updateSubcomponent) so the next boot restores byte-identical + // metadata. KeyMetadata is binder-free, so marshall() is safe here. + val metadataBytes = runCatching { + android.os.Parcel.obtain().let { parcel -> + try { + metadata.writeToParcel(parcel, 0) + parcel.marshall() + } finally { + parcel.recycle() + } + } + }.getOrNull() save( keyId = keyId, keyPair = keyPair, + secretKey = secretKey, nspace = generatedKeyInfo.nspace, securityLevel = secLevel, certChain = newChain.toList(), @@ -316,6 +415,7 @@ object GeneratedKeyPersistence { purposes = persisted.purposes, digests = persisted.digests, isAttestationKey = persisted.isAttestationKey, + metadataBytes = metadataBytes, ) SystemLogger.debug("Re-persisted key $keyId with updated cert chain") } @@ -332,7 +432,8 @@ object GeneratedKeyPersistence { return digest.joinToString("") { "%02x".format(it) } + ".bin" } - // Reads all fields after version has already been consumed + // Reads all fields after the version int has already been consumed + // and validated by the caller. private fun readPersistedKeyData(input: DataInputStream): PersistedKeyData { val secLevel = input.readInt() val uid = input.readInt() @@ -351,7 +452,7 @@ object GeneratedKeyPersistence { val pkLen = requireBounds(input.readInt(), 8192, "pkLen") val pkBytes = ByteArray(pkLen) - input.readFully(pkBytes) + if (pkLen > 0) input.readFully(pkBytes) val certCount = requireBounds(input.readInt(), 10, "certCount") val certChainBytes = (0 until certCount).map { @@ -361,6 +462,17 @@ object GeneratedKeyPersistence { certBytes } + val metaLen = requireBounds(input.readInt(), 256 * 1024, "metaLen") + val metadataBytes = ByteArray(metaLen).also { + if (metaLen > 0) input.readFully(it) + } + + val skAlgo = input.readUTF() + val skLen = requireBounds(input.readInt(), 8192, "skLen") + val skBytes = ByteArray(skLen).also { + if (skLen > 0) input.readFully(it) + } + return PersistedKeyData( uid = uid, alias = alias, @@ -374,6 +486,9 @@ object GeneratedKeyPersistence { digests = digests, privateKeyBytes = pkBytes, certChainBytes = certChainBytes, + metadataBytes = metadataBytes, + symmetricKeyBytes = skBytes, + symmetricAlgorithm = skAlgo, ) } } diff --git a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/KeyMintSecurityLevelInterceptor.kt b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/KeyMintSecurityLevelInterceptor.kt index 40243be..6a67a83 100644 --- a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/KeyMintSecurityLevelInterceptor.kt +++ b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/KeyMintSecurityLevelInterceptor.kt @@ -563,6 +563,40 @@ class KeyMintSecurityLevelInterceptor( } generatedKeys[keyId] = GeneratedKeyInfo(null, secretKey, keyDescriptor.nspace, response, parsedParams) + // Persist symmetric keys too. Without this, AndroidX security + // crypto MasterKey (AES-GCM-256) is regenerated on every reboot + // and any EncryptedSharedPreferences becomes undecryptable — + // which apps that wrap their session token in + // EncryptedSharedPreferences interpret as session expiry. + // Snapshot the metadata bytes alongside the raw secret + // material so authorizations restore byte-identical. + val metadataBytesForSymmetric = runCatching { + val parcel = android.os.Parcel.obtain() + try { + metadata.writeToParcel(parcel, 0) + parcel.marshall() + } finally { + parcel.recycle() + } + }.getOrNull() + persistExecutor.execute { + GeneratedKeyPersistence.save( + keyId = keyId, + keyPair = null, + secretKey = secretKey, + nspace = keyDescriptor.nspace, + securityLevel = securityLevel, + certChain = emptyList(), + algorithm = parsedParams.algorithm, + keySize = parsedParams.keySize, + ecCurve = parsedParams.ecCurve ?: 0, + purposes = parsedParams.purpose, + digests = parsedParams.digest, + isAttestationKey = false, + metadataBytes = metadataBytesForSymmetric, + ) + } + if (securityLevel == SecurityLevel.STRONGBOX) { val delayMs = STRONGBOX_KEYGEN_LATENCY_FLOOR_MS - (System.nanoTime() - genStartNanos) / 1_000_000 if (delayMs > 0) LockSupport.parkNanos(delayMs * 1_000_000) @@ -600,10 +634,28 @@ class KeyMintSecurityLevelInterceptor( } val certChainCopy = keyData.second.toList() + // Snapshot the freshly built KeyMetadata bytes so loadPersistedKeys + // can restore byte-identical authorizations after reboot. Without + // this, the rebuild path drops every authorization tag that wasn't + // captured into PersistedKeyData primitive fields (origin, block + // mode, padding, expiry timestamps...), which broke session pinning + // for apps that fingerprint metadata across keystore calls. + val metadataBytesForPersist = response.metadata?.let { md -> + runCatching { + val parcel = android.os.Parcel.obtain() + try { + md.writeToParcel(parcel, 0) + parcel.marshall() + } finally { + parcel.recycle() + } + }.getOrNull() + } persistExecutor.execute { GeneratedKeyPersistence.save( keyId = keyId, keyPair = keyData.first, + secretKey = null, nspace = keyDescriptor.nspace, securityLevel = securityLevel, certChain = certChainCopy, @@ -613,6 +665,7 @@ class KeyMintSecurityLevelInterceptor( purposes = parsedParams.purpose, digests = parsedParams.digest, isAttestationKey = isAttestKeyRequest, + metadataBytes = metadataBytesForPersist, ) } @@ -833,6 +886,55 @@ class KeyMintSecurityLevelInterceptor( return@runCatching } + // Symmetric (AES/HMAC/3DES) keys take a separate path: + // there is no PKCS8 private key, no certificate chain, just + // raw secret material plus the metadata snapshot. + val isSymmetric = record.symmetricKeyBytes.isNotEmpty() + if (isSymmetric) { + val secretKey = javax.crypto.spec.SecretKeySpec( + record.symmetricKeyBytes, + record.symmetricAlgorithm, + ) + val response = if (record.metadataBytes.isNotEmpty()) { + runCatching { + val parcel = android.os.Parcel.obtain() + try { + parcel.unmarshall(record.metadataBytes, 0, record.metadataBytes.size) + parcel.setDataPosition(0) + val metadata = KeyMetadata.CREATOR.createFromParcel(parcel) + KeyEntryResponse().apply { + this.metadata = metadata + iSecurityLevel = original + } + } finally { + parcel.recycle() + } + }.getOrElse { e -> + SystemLogger.warning( + "Failed to restore symmetric metadata for ${record.alias}", + e, + ) + return@runCatching + } + } else { + // Pre-v3 file with symmetric key — should not happen + // because v3 always saves metadata, but be defensive. + SystemLogger.warning("Symmetric record ${record.alias} missing metadata bytes, skipping") + return@runCatching + } + generatedKeys[keyId] = GeneratedKeyInfo( + keyPair = null, + secretKey = secretKey, + nspace = record.nspace, + response = response, + keyParams = response.metadata?.let { md -> + KeyMintAttestation(md.authorizations?.map { it.keyParameter }?.toTypedArray() ?: emptyArray()) + }, + ) + SystemLogger.debug("Restored symmetric persisted key: $keyId (${record.symmetricAlgorithm}/${record.symmetricKeyBytes.size * 8}bit)") + return@runCatching + } + val algorithmName = when (record.algorithm) { Algorithm.EC -> "EC" Algorithm.RSA -> "RSA" @@ -858,56 +960,57 @@ class KeyMintSecurityLevelInterceptor( blob = null } - val attestation = KeyMintAttestation( - keySize = record.keySize, - algorithm = record.algorithm, - ecCurve = record.ecCurve, - ecCurveName = "", - origin = null, - blockMode = emptyList(), - padding = emptyList(), - purpose = record.purposes, - digest = record.digests, - rsaPublicExponent = null, - certificateSerial = null, - certificateSubject = null, - certificateNotBefore = null, - certificateNotAfter = null, - attestationChallenge = null, - brand = null, - device = null, - product = null, - serial = null, - imei = null, - meid = null, - manufacturer = null, - model = null, - secondImei = null, - activeDateTime = null, - originationExpireDateTime = null, - usageExpireDateTime = null, - usageCountLimit = null, - callerNonce = null, - nonce = null, - unlockedDeviceRequired = null, - includeUniqueId = null, - rollbackResistance = null, - earlyBootOnly = null, - allowWhileOnBody = null, - trustedUserPresenceRequired = null, - trustedConfirmationRequired = null, - noAuthRequired = null, - maxUsesPerBoot = null, - maxBootLevel = null, - minMacLength = null, - rsaOaepMgfDigest = emptyList(), - ) + // Prefer the byte-identical metadata snapshot persisted by v3 + // saves so apps that fingerprint the metadata (e.g. they + // pin algorithm/purpose/digest/origin/authorization order + // across reboots) keep their session valid. Fall back to + // rebuilding + // from primitive fields for v1-era files (which lose + // authorization tags that weren't captured then). + val response = if (record.metadataBytes.isNotEmpty()) { + runCatching { + val parcel = android.os.Parcel.obtain() + try { + parcel.unmarshall(record.metadataBytes, 0, record.metadataBytes.size) + parcel.setDataPosition(0) + val metadata = KeyMetadata.CREATOR.createFromParcel(parcel) + // Make sure the descriptor's nspace matches the + // KEY_ID we will hand callers. updateSubcomponent + // and getKeyEntry both index by nspace. + metadata.key = metadata.key ?: KeyDescriptor().apply { + domain = Domain.KEY_ID + nspace = record.nspace + alias = null + blob = null + } + KeyEntryResponse().apply { + this.metadata = metadata + iSecurityLevel = original + } + } finally { + parcel.recycle() + } + }.getOrElse { e -> + SystemLogger.warning( + "Failed to restore metadata bytes for $record.alias, falling back to rebuild", + e, + ) + rebuildResponseFromRecord(record, certChain, descriptor) + } + } else { + rebuildResponseFromRecord(record, certChain, descriptor) + } - val response = buildKeyEntryResponse(record.uid, certChain, attestation, descriptor) - generatedKeys[keyId] = GeneratedKeyInfo(keyPair, null, record.nspace, response, attestation) - if (record.isAttestationKey) attestationKeys.add(keyId) + val keyIdRestored = KeyIdentifier(record.uid, record.alias) + generatedKeys[keyIdRestored] = GeneratedKeyInfo(keyPair, null, record.nspace, response, response.metadata?.let { md -> + // Re-derive an attestation summary from authorizations so + // any code path that reads keyParams (e.g. logging) still + // works. This does not feed back into the metadata bytes. + KeyMintAttestation(md.authorizations?.map { it.keyParameter }?.toTypedArray() ?: emptyArray()) + }) + if (record.isAttestationKey) attestationKeys.add(keyIdRestored) - SystemLogger.debug("Restored persisted key: $keyId") + SystemLogger.debug("Restored persisted key: $keyIdRestored") }.onFailure { SystemLogger.error("Failed to restore key: uid=${record.uid} alias=${record.alias}", it) } @@ -916,6 +1019,65 @@ class KeyMintSecurityLevelInterceptor( SystemLogger.info("Key restoration complete. Total in memory: ${generatedKeys.size}") } + /** + * Fallback rebuild path used when no v2 metadata snapshot is available + * (key was saved by an older build, or the snapshot failed to deserialize). + * Rebuilds KeyEntryResponse from primitive fields. This loses any + * authorization tags that weren't captured at save time, which is why we + * prefer the byte-identical v2 snapshot whenever possible. + */ + private fun rebuildResponseFromRecord( + record: PersistedKeyData, + certChain: List, + descriptor: KeyDescriptor, + ): KeyEntryResponse { + val attestation = KeyMintAttestation( + keySize = record.keySize, + algorithm = record.algorithm, + ecCurve = record.ecCurve, + ecCurveName = "", + origin = null, + blockMode = emptyList(), + padding = emptyList(), + purpose = record.purposes, + digest = record.digests, + rsaPublicExponent = null, + certificateSerial = null, + certificateSubject = null, + certificateNotBefore = null, + certificateNotAfter = null, + attestationChallenge = null, + brand = null, + device = null, + product = null, + serial = null, + imei = null, + meid = null, + manufacturer = null, + model = null, + secondImei = null, + activeDateTime = null, + originationExpireDateTime = null, + usageExpireDateTime = null, + usageCountLimit = null, + callerNonce = null, + nonce = null, + unlockedDeviceRequired = null, + includeUniqueId = null, + rollbackResistance = null, + earlyBootOnly = null, + allowWhileOnBody = null, + trustedUserPresenceRequired = null, + trustedConfirmationRequired = null, + noAuthRequired = null, + maxUsesPerBoot = null, + maxBootLevel = null, + minMacLength = null, + rsaOaepMgfDigest = emptyList(), + ) + return buildKeyEntryResponse(record.uid, certChain, attestation, descriptor) + } + companion object { private val secureRandom = SecureRandom() @@ -1116,41 +1278,47 @@ private fun KeyMintAttestation.toAuthorizations( authList.add(createAuth(Tag.BOOT_PATCHLEVEL, KeyParameterValue.integer(bootPatch))) } - fun createSwAuth(tag: Int, value: KeyParameterValue): Authorization { + /** + * Keystore-enforced authorizations (CREATION_DATETIME, ACTIVE_DATETIME, + * USER_ID, etc.) are tagged by real KeyMint HAL with + * SecurityLevel.KEYSTORE (= 100, byte 0x64), not SOFTWARE (= 0, byte + * 0x00). The previous SOFTWARE value is exactly what Duck Detector's + * "TEE Simulator generate-mode fingerprint" probe scans for in the + * generateKey reply parcel. Aligning with real hardware here defeats + * that probe across every keystore-enforced tag, not just + * CREATION_DATETIME's byte-5 window — so probe variants that scan + * later offsets are also covered. + */ + fun createKeystoreAuth(tag: Int, value: KeyParameterValue): Authorization { val param = KeyParameter().apply { this.tag = tag this.value = value } return Authorization().apply { this.keyParameter = param - // Real KeyMint HAL marks keystore-enforced metadata (creation - // time, user id, etc.) with SecurityLevel.KEYSTORE (0x64), not - // SOFTWARE (0x00). Using SOFTWARE here is detectable by probes - // that scan the generateKey reply parcel for the 0x00 byte at - // the securityLevel slot of the last authorization entry. this.securityLevel = SecurityLevel.KEYSTORE } } - authList.add(createSwAuth(Tag.CREATION_DATETIME, KeyParameterValue.dateTime(System.currentTimeMillis()))) + authList.add(createKeystoreAuth(Tag.CREATION_DATETIME, KeyParameterValue.dateTime(System.currentTimeMillis()))) this.activeDateTime?.let { - authList.add(createSwAuth(Tag.ACTIVE_DATETIME, KeyParameterValue.dateTime(it.time))) + authList.add(createKeystoreAuth(Tag.ACTIVE_DATETIME, KeyParameterValue.dateTime(it.time))) } this.originationExpireDateTime?.let { - authList.add(createSwAuth(Tag.ORIGINATION_EXPIRE_DATETIME, KeyParameterValue.dateTime(it.time))) + authList.add(createKeystoreAuth(Tag.ORIGINATION_EXPIRE_DATETIME, KeyParameterValue.dateTime(it.time))) } this.usageExpireDateTime?.let { - authList.add(createSwAuth(Tag.USAGE_EXPIRE_DATETIME, KeyParameterValue.dateTime(it.time))) + authList.add(createKeystoreAuth(Tag.USAGE_EXPIRE_DATETIME, KeyParameterValue.dateTime(it.time))) } this.usageCountLimit?.let { - authList.add(createSwAuth(Tag.USAGE_COUNT_LIMIT, KeyParameterValue.integer(it))) + authList.add(createKeystoreAuth(Tag.USAGE_COUNT_LIMIT, KeyParameterValue.integer(it))) } if (this.unlockedDeviceRequired == true) { - authList.add(createSwAuth(Tag.UNLOCKED_DEVICE_REQUIRED, KeyParameterValue.boolValue(true))) + authList.add(createKeystoreAuth(Tag.UNLOCKED_DEVICE_REQUIRED, KeyParameterValue.boolValue(true))) } - authList.add(createSwAuth(Tag.USER_ID, KeyParameterValue.integer(callingUid / 100000))) + authList.add(createKeystoreAuth(Tag.USER_ID, KeyParameterValue.integer(callingUid / 100000))) return authList.toTypedArray() } diff --git a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/SoftwareOperation.kt b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/SoftwareOperation.kt index cf5ea9e..9fb9bd9 100644 --- a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/SoftwareOperation.kt +++ b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/SoftwareOperation.kt @@ -218,19 +218,63 @@ class SoftwareOperation( val purposeName = KeyMintParameterLogger.purposeNames[purpose] ?: "UNKNOWN" SystemLogger.debug("[SoftwareOp TX_ID: $txId] Initializing for purpose: $purposeName.") + if (purpose == null) { + // Defensive: if params somehow restored without a PURPOSE tag + // (corrupt v2 metadata, mismatched authorizations array on load, + // or future format drift) the original code crashed with NPE + // because Signer/Verifier/Cipher all dereference keyPair!! + // before checking purpose. Surface a clean keystore error + // instead so callers see a normal-looking operation failure + // they can recover from rather than the process appearing to + // silently corrupt their session. + SystemLogger.warning( + "[SoftwareOp TX_ID: $txId] Purpose missing on restored key " + + "(authorizations=${params.purpose}, keyPair=${if (keyPair != null) "present" else "null"}, " + + "secretKey=${if (secretKey != null) "present" else "null"}). " + + "Returning unsupportedPurpose." + ) + throw ServiceSpecificException( + KeystoreErrorCodes.unsupportedPurpose, + "Restored key has no PURPOSE authorization", + ) + } + primitive = when (purpose) { - KeyPurpose.SIGN -> Signer(keyPair!!, params) - KeyPurpose.VERIFY -> Verifier(keyPair!!, params) + KeyPurpose.SIGN -> { + requireNotNull(keyPair) { + "[SoftwareOp TX_ID: $txId] SIGN requested but keyPair is null" + } + Signer(keyPair, params) + } + KeyPurpose.VERIFY -> { + requireNotNull(keyPair) { + "[SoftwareOp TX_ID: $txId] VERIFY requested but keyPair is null" + } + Verifier(keyPair, params) + } KeyPurpose.ENCRYPT -> { - val key: java.security.Key = secretKey ?: keyPair!!.public + val key: java.security.Key = secretKey ?: keyPair?.public + ?: throw ServiceSpecificException( + KeystoreErrorCodes.unsupportedPurpose, + "[SoftwareOp TX_ID: $txId] ENCRYPT requires either secretKey or keyPair.public", + ) CipherPrimitive(key, params, Cipher.ENCRYPT_MODE) } KeyPurpose.DECRYPT -> { - val key: java.security.Key = secretKey ?: keyPair!!.private + val key: java.security.Key = secretKey ?: keyPair?.private + ?: throw ServiceSpecificException( + KeystoreErrorCodes.unsupportedPurpose, + "[SoftwareOp TX_ID: $txId] DECRYPT requires either secretKey or keyPair.private", + ) CipherPrimitive(key, params, Cipher.DECRYPT_MODE) } - KeyPurpose.AGREE_KEY -> KeyAgreementPrimitive(keyPair!!) + KeyPurpose.AGREE_KEY -> { + requireNotNull(keyPair) { + "[SoftwareOp TX_ID: $txId] AGREE_KEY requested but keyPair is null" + } + KeyAgreementPrimitive(keyPair) + } else -> throw ServiceSpecificException( KeystoreErrorCodes.unsupportedPurpose, From 5803039309d37c5e50acc79b2f1533cdb7cc5ec0 Mon Sep 17 00:00:00 2001 From: Andrea-lyz Date: Sat, 16 May 2026 19:10:55 +0200 Subject: [PATCH 2/2] review: clean error codes, defensive symmetric fallback, v3 doc Address Copilot/CodeRabbit review feedback on the persistence PR. 1. SoftwareOperation: replace requireNotNull(keyPair) in SIGN/VERIFY/AGREE_KEY branches with ServiceSpecificException(invalidArgument). The original requireNotNull throws IllegalArgumentException, which the binder layer wraps as KEYMINT_UNKNOWN_ERROR, defeating the goal of surfacing a clean keystore-style error. Aligns with how ENCRYPT/DECRYPT already handle missing key material in the same when block. 2. loadPersistedKeys: when a symmetric record has empty metadataBytes (e.g. a save where Parcel.marshall() was empty for any reason), rebuild a minimal KeyMetadata from PersistedKeyData primitive fields instead of skipping the record. Skipping silently dropped the AES key, which is the same 'logged out after reboot' behavior the PR is trying to fix. The rebuilt metadata is structurally minimal but preserves the secret material, which is the dominant correctness concern. 3. Comment fix: rebuildResponseFromRecord docs referred to 'v2 metadata snapshot', the format in this PR is v3. --- .../shim/KeyMintSecurityLevelInterceptor.kt | 98 +++++++++++++++++-- .../keystore/shim/SoftwareOperation.kt | 27 ++--- 2 files changed, 106 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/KeyMintSecurityLevelInterceptor.kt b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/KeyMintSecurityLevelInterceptor.kt index 6a67a83..194aae4 100644 --- a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/KeyMintSecurityLevelInterceptor.kt +++ b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/KeyMintSecurityLevelInterceptor.kt @@ -911,16 +911,22 @@ class KeyMintSecurityLevelInterceptor( } }.getOrElse { e -> SystemLogger.warning( - "Failed to restore symmetric metadata for ${record.alias}", + "Failed to restore symmetric metadata for ${record.alias}, falling back to primitive rebuild", e, ) - return@runCatching + rebuildSymmetricResponse(record) } } else { // Pre-v3 file with symmetric key — should not happen - // because v3 always saves metadata, but be defensive. - SystemLogger.warning("Symmetric record ${record.alias} missing metadata bytes, skipping") - return@runCatching + // because v3 always saves metadata, but be defensive: + // rebuild a minimal KeyMetadata from primitives so + // the secret material is still restored. Without + // this, dropping the record would silently log the + // user out the next time the alias is used. + SystemLogger.info( + "Symmetric record ${record.alias} missing metadata bytes, rebuilding from primitives" + ) + rebuildSymmetricResponse(record) } generatedKeys[keyId] = GeneratedKeyInfo( keyPair = null, @@ -1020,11 +1026,11 @@ class KeyMintSecurityLevelInterceptor( } /** - * Fallback rebuild path used when no v2 metadata snapshot is available + * Fallback rebuild path used when no v3 metadata snapshot is available * (key was saved by an older build, or the snapshot failed to deserialize). * Rebuilds KeyEntryResponse from primitive fields. This loses any * authorization tags that weren't captured at save time, which is why we - * prefer the byte-identical v2 snapshot whenever possible. + * prefer the byte-identical v3 snapshot whenever possible. */ private fun rebuildResponseFromRecord( record: PersistedKeyData, @@ -1078,6 +1084,84 @@ class KeyMintSecurityLevelInterceptor( return buildKeyEntryResponse(record.uid, certChain, attestation, descriptor) } + /** + * Defensive fallback for symmetric key records that somehow ended up + * without a metadata snapshot (e.g. a save where Parcel.marshall() + * threw and persisted an empty mdBytes, or a future format where the + * snapshot is lazily populated). Without this fallback, loadAll would + * skip the record and the secret material would be effectively lost, + * silently logging the user out the next time the alias is used. + * + * The rebuilt KeyMetadata is structurally minimal — only the primitive + * authorization tags we captured at save time. That's worse than a + * byte-identical snapshot for apps that fingerprint metadata, but it + * still keeps the AES key alive across reboots, which is the + * dominant correctness concern. + */ + private fun rebuildSymmetricResponse(record: PersistedKeyData): KeyEntryResponse { + val attestation = KeyMintAttestation( + keySize = record.keySize, + algorithm = record.algorithm, + ecCurve = record.ecCurve, + ecCurveName = "", + origin = null, + blockMode = emptyList(), + padding = emptyList(), + purpose = record.purposes, + digest = record.digests, + rsaPublicExponent = null, + certificateSerial = null, + certificateSubject = null, + certificateNotBefore = null, + certificateNotAfter = null, + attestationChallenge = null, + brand = null, + device = null, + product = null, + serial = null, + imei = null, + meid = null, + manufacturer = null, + model = null, + secondImei = null, + activeDateTime = null, + originationExpireDateTime = null, + usageExpireDateTime = null, + usageCountLimit = null, + callerNonce = null, + nonce = null, + unlockedDeviceRequired = null, + includeUniqueId = null, + rollbackResistance = null, + earlyBootOnly = null, + allowWhileOnBody = null, + trustedUserPresenceRequired = null, + trustedConfirmationRequired = null, + noAuthRequired = null, + maxUsesPerBoot = null, + maxBootLevel = null, + minMacLength = null, + rsaOaepMgfDigest = emptyList(), + ) + val metadata = KeyMetadata().apply { + keySecurityLevel = securityLevel + key = KeyDescriptor().apply { + domain = Domain.KEY_ID + nspace = record.nspace + alias = null + blob = null + } + certificate = null + certificateChain = null + authorizations = attestation.toAuthorizations(record.uid, securityLevel) + modificationTimeMs = System.currentTimeMillis() + } + return KeyEntryResponse().apply { + this.metadata = metadata + iSecurityLevel = original + } + } + companion object { private val secureRandom = SecureRandom() diff --git a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/SoftwareOperation.kt b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/SoftwareOperation.kt index 9fb9bd9..73fe9a0 100644 --- a/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/SoftwareOperation.kt +++ b/app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/SoftwareOperation.kt @@ -242,16 +242,18 @@ class SoftwareOperation( primitive = when (purpose) { KeyPurpose.SIGN -> { - requireNotNull(keyPair) { - "[SoftwareOp TX_ID: $txId] SIGN requested but keyPair is null" - } - Signer(keyPair, params) + val kp = keyPair ?: throw ServiceSpecificException( + KeystoreErrorCodes.invalidArgument, + "[SoftwareOp TX_ID: $txId] SIGN requested but keyPair is null", + ) + Signer(kp, params) } KeyPurpose.VERIFY -> { - requireNotNull(keyPair) { - "[SoftwareOp TX_ID: $txId] VERIFY requested but keyPair is null" - } - Verifier(keyPair, params) + val kp = keyPair ?: throw ServiceSpecificException( + KeystoreErrorCodes.invalidArgument, + "[SoftwareOp TX_ID: $txId] VERIFY requested but keyPair is null", + ) + Verifier(kp, params) } KeyPurpose.ENCRYPT -> { val key: java.security.Key = secretKey ?: keyPair?.public @@ -270,10 +272,11 @@ class SoftwareOperation( CipherPrimitive(key, params, Cipher.DECRYPT_MODE) } KeyPurpose.AGREE_KEY -> { - requireNotNull(keyPair) { - "[SoftwareOp TX_ID: $txId] AGREE_KEY requested but keyPair is null" - } - KeyAgreementPrimitive(keyPair) + val kp = keyPair ?: throw ServiceSpecificException( + KeystoreErrorCodes.invalidArgument, + "[SoftwareOp TX_ID: $txId] AGREE_KEY requested but keyPair is null", + ) + KeyAgreementPrimitive(kp) } else -> throw ServiceSpecificException(