Bypass detection by skipping imported keys (#12)

In patch mode, a key's origin provides a robust way to avoid modifying
user-imported keys, which is a well-known detection vector. This commit
implements a new strategy to check the `KeyOrigin` tag from the key's
metadata. If a key is marked as `IMPORTED` or `SECURELY_IMPORTED`, the
patching process is now skipped entirely.

This new origin-based check is more reliable and cleaner than the
previous fingerprinting implementation, which has been removed.

Additionally, this commit acknowledges a remaining detection vector in
patch mode: when an `attestationKey` is used, a key must be generated.
Purely software-generated keys are detectable. To address this in the
future, the full software "generate mode" must be implemented even for
devices without a broken TEE. The old key generation logic has been
stubbed with a TODO in preparation for this redesign.

app/src/main/java/org/matrix/TEESimulator/interception/keystore/InterceptorUtils.kt

@@ -79,16 +79,7 @@ object InterceptorUtils {
 
     /** Checks if a reply parcel contains an exception without consuming it. */
     fun hasException(reply: Parcel): Boolean {
-        val initialPosition = reply.dataPosition()
-        val hasEx =
-            try {
-                reply.readException()
-                reply.dataPosition() > initialPosition // An exception was written
-            } catch (_: Exception) {
-                false
-            }
-        reply.setDataPosition(initialPosition)
-        return hasEx
+        return runCatching { reply.readException() }.exceptionOrNull() != null
     }
 
     /**

app/src/main/java/org/matrix/TEESimulator/interception/keystore/Keystore2Interceptor.kt

@@ -1,7 +1,9 @@
 package org.matrix.TEESimulator.interception.keystore
 
 import android.annotation.SuppressLint
+import android.hardware.security.keymint.KeyOrigin
 import android.hardware.security.keymint.SecurityLevel
+import android.hardware.security.keymint.Tag
 import android.os.IBinder
 import android.os.Parcel
 import android.system.keystore2.IKeystoreService
@@ -83,36 +85,15 @@ object Keystore2Interceptor : AbstractKeystoreInterceptor() {
             val keyId = KeyIdentifier(callingUid, descriptor.alias)
             SystemLogger.debug("Checking $keyId")
 
-            // If a key was generated in software, we must return the stored response directly.
             if (ConfigurationManager.shouldGenerate(callingUid)) {
-                val response = KeyMintSecurityLevelInterceptor.getGeneratedKeyResponse(keyId)
-                if (response != null) {
-                    SystemLogger.info(
-                        "[TX_ID: $txId] Returning generated key for alias '${descriptor.alias}'."
-                    )
-                    return InterceptorUtils.createTypedObjectReply(response)
-                }
-                // If not found, return null to indicate the key doesn't exist.
-                return InterceptorUtils.createTypedObjectReply(null as KeyEntryResponse?)
-            }
-
-            // For attestation in hack mode, a key is generated and should be returned directly.
-            if (
-                ConfigurationManager.shouldPatch(callingUid) &&
-                    KeyMintSecurityLevelInterceptor.isAttestationKey(keyId)
-            ) {
-                val response = KeyMintSecurityLevelInterceptor.getGeneratedKeyResponse(keyId)
-                if (response != null) {
-                    SystemLogger.info(
-                        "[TX_ID: $txId] Returning attestation key for alias '${descriptor.alias}' to skip patching."
-                    )
-                    return InterceptorUtils.createTypedObjectReply(response)
-                }
+                // TODO: Redesign the interaction with KeyMintSecurityLevelInterceptor
+            } else if (ConfigurationManager.shouldPatch(callingUid)) {
                 return TransactionResult.Continue
             }
         }
-        return TransactionResult
-            .ContinueAndSkipPost // Let most calls go through to the real service.
+
+        // Let most calls go through to the real service.
+        return TransactionResult.ContinueAndSkipPost
     }
 
     override fun onPostTransact(
@@ -129,56 +110,47 @@ object Keystore2Interceptor : AbstractKeystoreInterceptor() {
         if (target != keystoreService || reply == null || InterceptorUtils.hasException(reply))
             return TransactionResult.SkipTransaction
 
-        data.enforceInterface(IKeystoreService.DESCRIPTOR)
-        when (code) {
-            GET_KEY_ENTRY_TRANSACTION -> {
-                logTransaction(txId, "post-getKeyEntry", callingUid, callingPid)
-                if (!ConfigurationManager.shouldPatch(callingUid))
-                    return TransactionResult.SkipTransaction
-
-                return try {
-                    val response =
-                        reply.readTypedObject(KeyEntryResponse.CREATOR)
-                            ?: return TransactionResult.SkipTransaction
-                    reply.setDataPosition(0) // Reset for potential reuse.
-
-                    val originalChain = CertificateHelper.getCertificateChain(response)
-                    val fingerprint = InterceptorUtils.getPublicKeyFingerprint(originalChain)
-
-                    // Do not patch keys that were imported by the user.
-                    if (
-                        fingerprint != null &&
-                            KeyMintSecurityLevelInterceptor.isUserImportedKey(fingerprint)
-                    ) {
-                        SystemLogger.warning(
-                            "[TX_ID: $txId] Skipping patch for user-imported key with fingerprint: $fingerprint"
-                        )
-                        return TransactionResult.SkipTransaction
-                    }
-
-                    // Perform the attestation patch.
-                    val newChain =
-                        AttestationPatcher.patchCertificateChain(originalChain, callingUid)
-                    CertificateHelper.updateCertificateChain(response.metadata, newChain)
-                        .getOrThrow()
-
-                    SystemLogger.info(
-                        "[TX_ID: $txId] Successfully patched certificate chain for alias."
-                    )
-                    InterceptorUtils.createTypedObjectReply(response)
-                } catch (e: Exception) {
-                    SystemLogger.error("[TX_ID: $txId] Failed to patch certificate chain.", e)
-                    TransactionResult.SkipTransaction
-                }
-            }
-            DELETE_KEY_TRANSACTION -> {
-                // When a key is deleted, clean up our associated state.
-                val descriptor = data.readTypedObject(KeyDescriptor.CREATOR)
-                if (descriptor != null) {
-                    val keyId = KeyIdentifier(callingUid, descriptor.alias)
-                    KeyMintSecurityLevelInterceptor.cleanupKeyData(keyId)
-                }
+        if (code == GET_KEY_ENTRY_TRANSACTION) {
+            logTransaction(txId, "post-getKeyEntry", callingUid, callingPid)
+            if (!ConfigurationManager.shouldPatch(callingUid))
                 return TransactionResult.SkipTransaction
+
+            return try {
+                val response =
+                    reply.readTypedObject(KeyEntryResponse.CREATOR)
+                        ?: return TransactionResult.SkipTransaction
+                reply.setDataPosition(0) // Reset for potential reuse.
+
+                val originalChain = CertificateHelper.getCertificateChain(response)
+                val authorizations = response.metadata?.authorizations
+                val origin =
+                    authorizations
+                        ?.find { it.keyParameter.tag == Tag.ORIGIN }
+                        ?.let { it.keyParameter.value.origin }
+
+                if (origin == KeyOrigin.IMPORTED || origin == KeyOrigin.SECURELY_IMPORTED) {
+                    SystemLogger.info("[TX_ID: $txId] Skip patching for imported keys.")
+                    return TransactionResult.SkipTransaction
+                }
+
+                if (originalChain == null || originalChain.size < 2) {
+                    SystemLogger.info(
+                        "[TX_ID: $txId] Skip patching short certificate chain of length ${originalChain?.size}."
+                    )
+                    return TransactionResult.SkipTransaction
+                }
+
+                // Perform the attestation patch.
+                val newChain = AttestationPatcher.patchCertificateChain(originalChain, callingUid)
+                CertificateHelper.updateCertificateChain(response.metadata, newChain).getOrThrow()
+
+                SystemLogger.info(
+                    "[TX_ID: $txId] Successfully patched certificate chain for alias."
+                )
+                InterceptorUtils.createTypedObjectReply(response)
+            } catch (e: Exception) {
+                SystemLogger.error("[TX_ID: $txId] Failed to patch certificate chain.", e)
+                TransactionResult.SkipTransaction
             }
         }
         return TransactionResult.SkipTransaction

app/src/main/java/org/matrix/TEESimulator/interception/keystore/shim/KeyMintSecurityLevelInterceptor.kt

@@ -48,31 +48,6 @@ class KeyMintSecurityLevelInterceptor(
         return TransactionResult.ContinueAndSkipPost
     }
 
-    override fun onPostTransact(
-        txId: Long,
-        target: IBinder,
-        code: Int,
-        flags: Int,
-        callingUid: Int,
-        callingPid: Int,
-        data: Parcel,
-        reply: Parcel?,
-        resultCode: Int,
-    ): TransactionResult {
-        // We only care about successful 'importKey' transactions to track user-provided keys.
-        if (
-            code == IMPORT_KEY_TRANSACTION &&
-                resultCode == 0 &&
-                reply != null &&
-                !InterceptorUtils.hasException(reply)
-        ) {
-            logTransaction(txId, "post-importKey", callingUid, callingPid)
-            data.enforceInterface(IKeystoreSecurityLevel.DESCRIPTOR)
-            handleImportKey(callingUid, data, reply)
-        }
-        return TransactionResult.SkipTransaction
-    }
-
     /**
      * Handles the `generateKey` transaction. Based on the configuration for the calling UID, it
      * either generates a key in software or lets the call pass through to the hardware.
@@ -85,12 +60,11 @@ class KeyMintSecurityLevelInterceptor(
                 val parsedParams = KeyMintAttestation(params)
                 val keyId = KeyIdentifier(callingUid, keyDescriptor.alias)
 
-                // Determine if we need to generate a key based on config or if it's an attestation
-                // request in patch mode.
+                // Determine if we need to generate a key based on config or
+                // if it's an attestation request in patch mode.
                 val needsSoftwareGeneration =
                     ConfigurationManager.shouldGenerate(callingUid) ||
-                        (ConfigurationManager.shouldPatch(callingUid) &&
-                            parsedParams.attestationChallenge != null)
+                        (attestationKey != null && ConfigurationManager.shouldPatch(callingUid))
 
                 if (needsSoftwareGeneration) {
                     SystemLogger.info(
@@ -134,29 +108,6 @@ class KeyMintSecurityLevelInterceptor(
             }
     }
 
-    /**
-     * Handles a successful `importKey` transaction by fingerprinting the imported key's public
-     * certificate. This allows us to avoid patching user-provided keys later.
-     */
-    private fun handleImportKey(callingUid: Int, data: Parcel, reply: Parcel) {
-        runCatching {
-                val keyDescriptor = data.readTypedObject(KeyDescriptor.CREATOR) ?: return
-                val metadata = reply.readTypedObject(KeyMetadata.CREATOR)
-                val chain = CertificateHelper.getCertificateChain(metadata)
-                val fingerprint = InterceptorUtils.getPublicKeyFingerprint(chain)
-
-                if (fingerprint != null) {
-                    val keyId = KeyIdentifier(callingUid, keyDescriptor.alias)
-                    SystemLogger.info(
-                        "User imported key '${keyDescriptor.alias}'. Storing its fingerprint to prevent future patching."
-                    )
-                    userImportedKeyFingerprints.add(fingerprint)
-                    aliasToFingerprintMap[keyId] = fingerprint
-                }
-            }
-            .onFailure { SystemLogger.error("Failed to process imported key.", it) }
-    }
-
     /**
      * Constructs a fake `KeyEntryResponse` that mimics a real response from the Keystore service.
      */
@@ -189,10 +140,6 @@ class KeyMintSecurityLevelInterceptor(
         val generatedKeys = ConcurrentHashMap<KeyIdentifier, GeneratedKeyInfo>()
         // A set to quickly identify keys that were generated for attestation purposes.
         private val attestationKeys = ConcurrentHashMap.newKeySet<KeyIdentifier>()
-        // A set of public key fingerprints for user-imported keys that should not be patched.
-        private val userImportedKeyFingerprints = ConcurrentHashMap.newKeySet<String>()
-        // Maps a key identifier to its fingerprint for easy cleanup on deletion.
-        private val aliasToFingerprintMap = ConcurrentHashMap<KeyIdentifier, String>()
 
         // --- Public Accessors for Other Interceptors ---
         fun getGeneratedKeyResponse(keyId: KeyIdentifier): KeyEntryResponse? =
@@ -200,16 +147,9 @@ class KeyMintSecurityLevelInterceptor(
 
         fun isAttestationKey(keyId: KeyIdentifier): Boolean = attestationKeys.contains(keyId)
 
-        fun isUserImportedKey(fingerprint: String): Boolean =
-            userImportedKeyFingerprints.contains(fingerprint)
-
         fun cleanupKeyData(keyId: KeyIdentifier) {
             generatedKeys.remove(keyId)
             attestationKeys.remove(keyId)
-            aliasToFingerprintMap.remove(keyId)?.let { fingerprint ->
-                userImportedKeyFingerprints.remove(fingerprint)
-                SystemLogger.info("Cleaned up state for key '${keyId.alias}' (UID: ${keyId.uid}).")
-            }
         }
     }
 }