fix(interception): resolve B3, C2, F1 and harden AUTO mode
B3: AttestationPatcher now accepts optional notBefore/notAfter overrides so the PATCH path honors CERTIFICATE_NOT_BEFORE instead of inheriting the real TEE's epoch 0. C2: getKeymasterVersion delegates to getAttestVersion directly, ensuring attestationVersion == keymasterVersion regardless of cache source. F1: Remove incorrect EC+DECRYPT guard in AuthorizeCreate that returned UNSUPPORTED_PURPOSE instead of INCOMPATIBLE_PURPOSE. AUTO mode: Replace volatile teeFunctional boolean with AtomicReference tri-state (null/true/false) so the first race winner locks the path for all subsequent requests, preventing mixed attestation under concurrency.
This commit is contained in:
@@ -16,6 +16,7 @@ import org.matrix.TEESimulator.logging.SystemLogger
|
||||
import org.matrix.TEESimulator.pki.KeyBox
|
||||
import org.matrix.TEESimulator.pki.KeyBoxManager
|
||||
import org.matrix.TEESimulator.util.toHex
|
||||
import java.util.Date
|
||||
|
||||
/**
|
||||
* Handles the modification (patching) of Android Key Attestation extensions within certificates.
|
||||
@@ -36,7 +37,12 @@ object AttestationPatcher {
|
||||
* @return A new, cryptographically valid, patched certificate chain. Returns the original chain
|
||||
* on any failure.
|
||||
*/
|
||||
fun patchCertificateChain(originalChain: Array<Certificate>?, uid: Int): Array<Certificate> {
|
||||
fun patchCertificateChain(
|
||||
originalChain: Array<Certificate>?,
|
||||
uid: Int,
|
||||
notBefore: Date? = null,
|
||||
notAfter: Date? = null,
|
||||
): Array<Certificate> {
|
||||
if (originalChain.isNullOrEmpty()) {
|
||||
SystemLogger.error("Attempted to patch a null or empty certificate chain for UID $uid.")
|
||||
return originalChain ?: emptyArray()
|
||||
@@ -63,6 +69,8 @@ object AttestationPatcher {
|
||||
keybox,
|
||||
originalLeaf.sigAlgName,
|
||||
uid,
|
||||
notBefore,
|
||||
notAfter,
|
||||
)
|
||||
|
||||
// 4. Construct the NEW, VALID chain by prepending the patched leaf to the keybox's
|
||||
@@ -111,17 +119,27 @@ object AttestationPatcher {
|
||||
keybox: KeyBox,
|
||||
sigAlgName: String,
|
||||
uid: Int,
|
||||
notBefore: Date? = null,
|
||||
notAfter: Date? = null,
|
||||
): Certificate {
|
||||
// The issuer of our new leaf is the subject of the first certificate in our custom keybox
|
||||
// chain.
|
||||
val newIssuer = X509CertificateHolder(keybox.certificates[0].encoded).subject
|
||||
|
||||
val effectiveNotBefore = notBefore ?: originalLeafHolder.notBefore
|
||||
val effectiveNotAfter = notAfter ?: originalLeafHolder.notAfter
|
||||
if (notBefore != null || notAfter != null) {
|
||||
SystemLogger.debug(
|
||||
"Overriding cert dates: notBefore=${effectiveNotBefore} (was ${originalLeafHolder.notBefore}), notAfter=${effectiveNotAfter} (was ${originalLeafHolder.notAfter})"
|
||||
)
|
||||
}
|
||||
|
||||
val builder =
|
||||
X509v3CertificateBuilder(
|
||||
newIssuer,
|
||||
originalLeafHolder.serialNumber,
|
||||
originalLeafHolder.notBefore,
|
||||
originalLeafHolder.notAfter,
|
||||
effectiveNotBefore,
|
||||
effectiveNotAfter,
|
||||
originalLeafHolder.subject,
|
||||
originalLeafHolder.subjectPublicKeyInfo,
|
||||
)
|
||||
|
||||
-2
@@ -29,8 +29,6 @@ object AuthorizeCreate {
|
||||
) {
|
||||
return KeystoreErrorCodes.unsupportedPurpose
|
||||
}
|
||||
if (algo == Algorithm.EC && purpose == KeyPurpose.DECRYPT)
|
||||
return KeystoreErrorCodes.unsupportedPurpose
|
||||
if (algo == Algorithm.RSA && purpose == KeyPurpose.AGREE_KEY)
|
||||
return KeystoreErrorCodes.unsupportedPurpose
|
||||
return null
|
||||
|
||||
+31
-9
@@ -20,11 +20,13 @@ import java.security.SecureRandom
|
||||
import java.security.cert.Certificate
|
||||
import java.security.cert.CertificateFactory
|
||||
import java.security.spec.PKCS8EncodedKeySpec
|
||||
import java.util.Date
|
||||
import java.util.concurrent.CompletableFuture
|
||||
import java.util.concurrent.ConcurrentHashMap
|
||||
import java.util.concurrent.ConcurrentLinkedDeque
|
||||
import java.util.concurrent.Executors
|
||||
import java.util.concurrent.atomic.AtomicInteger
|
||||
import java.util.concurrent.atomic.AtomicReference
|
||||
import java.util.concurrent.locks.LockSupport
|
||||
import org.matrix.TEESimulator.attestation.AttestationBuilder
|
||||
import org.matrix.TEESimulator.attestation.AttestationConstants
|
||||
@@ -57,6 +59,10 @@ class KeyMintSecurityLevelInterceptor(
|
||||
val keyParams: KeyMintAttestation? = null,
|
||||
)
|
||||
|
||||
// null = undecided, true = TEE works (use PATCH), false = TEE broken (use GENERATE)
|
||||
// Instance field so TRUSTED_ENVIRONMENT and STRONGBOX decide independently
|
||||
val teePathDecision = AtomicReference<Boolean?>(null)
|
||||
|
||||
private val activeOps = ConcurrentHashMap<Int, ConcurrentLinkedDeque<SoftwareOperation>>()
|
||||
private val recentOps = ConcurrentHashMap<Int, ConcurrentLinkedDeque<Long>>()
|
||||
|
||||
@@ -204,12 +210,18 @@ class KeyMintSecurityLevelInterceptor(
|
||||
CertificateHelper.getCertificateChain(metadata)
|
||||
?: return TransactionResult.SkipTransaction
|
||||
if (originalChain.size > 1) {
|
||||
val newChain = AttestationPatcher.patchCertificateChain(originalChain, callingUid)
|
||||
|
||||
// Cache the newly patched chain to ensure consistency across subsequent API calls.
|
||||
// Read the request parcel to extract keyDescriptor and cert date params.
|
||||
data.enforceInterface(IKeystoreSecurityLevel.DESCRIPTOR)
|
||||
val keyDescriptor = data.readTypedObject(KeyDescriptor.CREATOR)
|
||||
?: return TransactionResult.SkipTransaction
|
||||
data.readTypedObject(KeyDescriptor.CREATOR) // skip attestationKey
|
||||
val keyParams = data.createTypedArray(KeyParameter.CREATOR)
|
||||
val certNotBefore = keyParams?.find { it.tag == Tag.CERTIFICATE_NOT_BEFORE }?.value?.dateTime?.let { Date(it) }
|
||||
val certNotAfter = keyParams?.find { it.tag == Tag.CERTIFICATE_NOT_AFTER }?.value?.dateTime?.let { Date(it) }
|
||||
|
||||
val newChain = AttestationPatcher.patchCertificateChain(originalChain, callingUid, certNotBefore, certNotAfter)
|
||||
|
||||
// Cache the newly patched chain to ensure consistency across subsequent API calls.
|
||||
val key = metadata.key
|
||||
?: return TransactionResult.SkipTransaction
|
||||
val keyId = KeyIdentifier(callingUid, keyDescriptor.alias)
|
||||
@@ -471,9 +483,12 @@ class KeyMintSecurityLevelInterceptor(
|
||||
|
||||
val isAuto = ConfigurationManager.isAutoMode(callingUid)
|
||||
|
||||
if (isAuto) SystemLogger.debug("AUTO dispatch: teePathDecision=${teePathDecision.get()} for ${keyDescriptor.alias}")
|
||||
|
||||
when {
|
||||
forceGenerate -> doSoftwareKeyGen(callingUid, keyDescriptor, attestationKey, parsedParams, keyId, isAttestKeyRequest)
|
||||
isAuto && !teeFunctional -> raceTeePatch(callingUid, keyDescriptor, attestationKey, params, parsedParams, keyId, isAttestKeyRequest)
|
||||
isAuto && teePathDecision.get() == null -> raceTeePatch(callingUid, keyDescriptor, attestationKey, params, parsedParams, keyId, isAttestKeyRequest)
|
||||
isAuto && teePathDecision.get() == false -> doSoftwareKeyGen(callingUid, keyDescriptor, attestationKey, parsedParams, keyId, isAttestKeyRequest)
|
||||
parsedParams.attestationChallenge != null -> TransactionResult.Continue
|
||||
else -> {
|
||||
cleanupKeyData(keyId)
|
||||
@@ -633,12 +648,14 @@ class KeyMintSecurityLevelInterceptor(
|
||||
return try {
|
||||
val teeMetadata = threadA.join()
|
||||
threadB.cancel(true)
|
||||
teeFunctional = true
|
||||
SystemLogger.info("AUTO: TEE succeeded for ${keyDescriptor.alias}, marked functional.")
|
||||
teePathDecision.compareAndSet(null, true)
|
||||
SystemLogger.info("AUTO: TEE succeeded, path locked to PATCH for ${keyDescriptor.alias}")
|
||||
|
||||
val originalChain = CertificateHelper.getCertificateChain(teeMetadata)
|
||||
if (originalChain != null && originalChain.size > 1) {
|
||||
val newChain = AttestationPatcher.patchCertificateChain(originalChain, callingUid)
|
||||
val newChain = AttestationPatcher.patchCertificateChain(
|
||||
originalChain, callingUid, parsedParams.certificateNotBefore, parsedParams.certificateNotAfter
|
||||
)
|
||||
CertificateHelper.updateCertificateChain(teeMetadata, newChain).getOrThrow()
|
||||
teeMetadata.authorizations =
|
||||
InterceptorUtils.patchAuthorizations(teeMetadata.authorizations, callingUid)
|
||||
@@ -653,7 +670,13 @@ class KeyMintSecurityLevelInterceptor(
|
||||
|
||||
InterceptorUtils.createTypedObjectReply(teeMetadata)
|
||||
} catch (_: Exception) {
|
||||
SystemLogger.info("AUTO: TEE failed for ${keyDescriptor.alias}, using software result.")
|
||||
if (teePathDecision.get() == true) {
|
||||
threadB.cancel(true)
|
||||
SystemLogger.info("AUTO: TEE failed locally but globally functional, forwarding for ${keyDescriptor.alias}")
|
||||
return TransactionResult.Continue
|
||||
}
|
||||
teePathDecision.compareAndSet(null, false)
|
||||
SystemLogger.info("AUTO: TEE failed, path locked to GENERATE for ${keyDescriptor.alias}")
|
||||
try {
|
||||
threadB.join()
|
||||
} catch (e: Exception) {
|
||||
@@ -870,7 +893,6 @@ class KeyMintSecurityLevelInterceptor(
|
||||
|
||||
companion object {
|
||||
private val secureRandom = SecureRandom()
|
||||
@Volatile var teeFunctional = false
|
||||
|
||||
// Maximum alias length to prevent binder buffer exhaustion (Issue #109)
|
||||
// Binder buffer is ~1MB; 256KB provides 4x safety margin for transaction overhead
|
||||
|
||||
@@ -387,9 +387,17 @@ object AndroidDeviceUtils {
|
||||
if (securityLevel == SecurityLevel.STRONGBOX) {
|
||||
return 300
|
||||
}
|
||||
return DeviceAttestationService.CachedAttestationData?.attestVersion
|
||||
val cached = DeviceAttestationService.CachedAttestationData?.attestVersion
|
||||
val version = cached
|
||||
?: attestVersionMap[Build.VERSION.SDK_INT]
|
||||
?: 400 // Default to a recent version
|
||||
val source = when {
|
||||
cached != null -> "cache"
|
||||
attestVersionMap.containsKey(Build.VERSION.SDK_INT) -> "map"
|
||||
else -> "default"
|
||||
}
|
||||
SystemLogger.debug("attestVersion=$version source=$source securityLevel=$securityLevel")
|
||||
return version
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -398,10 +406,7 @@ object AndroidDeviceUtils {
|
||||
* @param securityLevel The security level, used to determine the correct attestation version.
|
||||
* @return The appropriate Keymaster or KeyMint version number.
|
||||
*/
|
||||
fun getKeymasterVersion(securityLevel: Int): Int {
|
||||
val attestVersion = getAttestVersion(securityLevel)
|
||||
return if (attestVersion >= 100) attestVersion else 41 // Keymaster 4.1 for older versions
|
||||
}
|
||||
fun getKeymasterVersion(securityLevel: Int): Int = getAttestVersion(securityLevel)
|
||||
|
||||
// --- APEX and Module Hash Properties ---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user