From ce2232714731f1dd186f20d15a537367c5ac62a4 Mon Sep 17 00:00:00 2001 From: Enginex0 Date: Wed, 17 Jun 2026 10:38:32 +0100 Subject: [PATCH] refactor(keystore): strip unique-id at parse time Decide the effective generateKey params once via .let when the caller lacks gen_unique_id / REQUEST_UNIQUE_ID_ATTESTATION, instead of mutating var params/parsedParams deep in handleGenerateKey and re-parsing KeyMintAttestation a second time. isAttestKeyRequest now derives from the final parsedParams, closing the staleness flagged in PR #27 review r3308356496. Behavior is unchanged: no gate between the parse and the old strip site reads INCLUDE_UNIQUE_ID, and the && short-circuits so the permission lookups still run only when the tag is present. --- .../shim/KeyMintSecurityLevelInterceptor.kt | 62 +++++++++---------- 1 file changed, 29 insertions(+), 33 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 89336aa..8c67793 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 @@ -547,8 +547,35 @@ class KeyMintSecurityLevelInterceptor( SystemLogger.debug( "Handling generateKey ${keyDescriptor.alias}, attestKey=${attestationKey?.alias}" ) - var params = data.createTypedArray(KeyParameter.CREATOR)!! - var parsedParams = KeyMintAttestation(params) + // INCLUDE_UNIQUE_ID requires SELinux gen_unique_id OR + // REQUEST_UNIQUE_ID_ATTESTATION (AOSP security_level.rs:478-485). AOSP + // rejects with PERMISSION_DENIED when neither is held, but that breaks + // Google Wallet card binding and regresses Play Integrity: Wallet's + // generateKey carries the tag without the permission. Strip it at parse + // so the key generates and the attestation omits unique_id (pre-PR157 + // behavior). Deciding here keeps params/parsedParams val and derives + // isAttestKeyRequest from the effective parameters. + val params = + data.createTypedArray(KeyParameter.CREATOR)!!.let { raw -> + val stripUniqueId = + raw.any { it.tag == Tag.INCLUDE_UNIQUE_ID } && + !ConfigurationManager.checkSELinuxPermission( + callingPid, + "keystore_key", + "gen_unique_id", + ) && + !ConfigurationManager.hasPermissionForUid( + callingUid, + "android.permission.REQUEST_UNIQUE_ID_ATTESTATION", + ) + if (stripUniqueId) { + SystemLogger.debug( + "[TX_ID: $txId] Stripping INCLUDE_UNIQUE_ID for uid=$callingUid pid=$callingPid (no permission)" + ) + raw.filter { it.tag != Tag.INCLUDE_UNIQUE_ID }.toTypedArray() + } else raw + } + val parsedParams = KeyMintAttestation(params) val isAttestKeyRequest = parsedParams.isAttestKey() val hasDeviceIdAttestation = @@ -662,37 +689,6 @@ class KeyMintSecurityLevelInterceptor( return InterceptorUtils.createErrorReply(KEYMINT_CANNOT_ATTEST_IDS) } - // INCLUDE_UNIQUE_ID requires SELinux gen_unique_id OR - // android.permission.REQUEST_UNIQUE_ID_ATTESTATION (AOSP - // security_level.rs:478-485). AOSP returns PERMISSION_DENIED - // when neither is held — but doing so breaks Google Wallet - // card binding (Wallet's generateKey carries the tag without - // holding the permission, and Play Integrity also fails when - // unique_id ends up in the attestation). Silently strip the - // tag so the key generates normally and the resulting - // attestation simply omits the unique_id field. This mirrors - // the pre-PR157 behavior where the tag had no effect. - if (params.any { it.tag == Tag.INCLUDE_UNIQUE_ID }) { - val hasSELinux = - ConfigurationManager.checkSELinuxPermission( - callingPid, - "keystore_key", - "gen_unique_id", - ) - val hasAndroid = - ConfigurationManager.hasPermissionForUid( - callingUid, - "android.permission.REQUEST_UNIQUE_ID_ATTESTATION", - ) - if (!hasSELinux && !hasAndroid) { - SystemLogger.debug( - "[TX_ID: $txId] Stripping INCLUDE_UNIQUE_ID for uid=$callingUid pid=$callingPid (no permission)" - ) - params = params.filter { it.tag != Tag.INCLUDE_UNIQUE_ID }.toTypedArray() - parsedParams = KeyMintAttestation(params) - } - } - val isSymmetric = parsedParams.algorithm == Algorithm.AES || parsedParams.algorithm == Algorithm.HMAC ||