feat(interception): add hardening fixes from pre-PR157 backup
Ported challenge length validation, forced op rejection, system transaction skip, dead interceptor blocking, and createErrorReply. Removed size guards that broke G10. Fixed F14 error code to KeyMint space (-38).
This commit is contained in:
@@ -355,15 +355,14 @@ static sp<BinderStub> g_stub_instance = nullptr;
|
|||||||
|
|
||||||
namespace {
|
namespace {
|
||||||
|
|
||||||
/**
|
|
||||||
* @brief Analyses a binder transaction. If the target is monitored,
|
|
||||||
* hijacks the transaction by rewriting its destination to our BinderStub.
|
|
||||||
* @param txn_data Pointer to the transaction data within the ioctl buffer.
|
|
||||||
*/
|
|
||||||
void inspectAndRewriteTransaction(binder_transaction_data *txn_data) {
|
void inspectAndRewriteTransaction(binder_transaction_data *txn_data) {
|
||||||
if (!txn_data || txn_data->target.ptr == 0)
|
if (!txn_data || txn_data->target.ptr == 0)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
|
// Skip system transactions (PING, INTERFACE, DUMP) to avoid latency detectors
|
||||||
|
if (txn_data->code > 0x00ffffffu && txn_data->code != intercept::kBackdoorCode)
|
||||||
|
return;
|
||||||
|
|
||||||
bool hijack = false;
|
bool hijack = false;
|
||||||
ThreadTransactionInfo info;
|
ThreadTransactionInfo info;
|
||||||
|
|
||||||
@@ -613,9 +612,15 @@ bool BinderInterceptor::processInterceptedTransaction(uint64_t tx_id, sp<BBinder
|
|||||||
Parcel pre_req, pre_resp;
|
Parcel pre_req, pre_resp;
|
||||||
writeTransactionData(pre_req, tx_id, target, code, flags, request);
|
writeTransactionData(pre_req, tx_id, target, code, flags, request);
|
||||||
|
|
||||||
if (callback->transact(intercept::kPreTransact, pre_req, &pre_resp) != OK) {
|
status_t pre_status = callback->transact(intercept::kPreTransact, pre_req, &pre_resp);
|
||||||
LOGW("[TX_ID: %" PRIu64 "] Pre-transaction callback failed. Forwarding original call.", tx_id);
|
if (pre_status != OK) {
|
||||||
return false; // Callback failed, proceed as if not intercepted
|
if (callback->pingBinder() != OK) {
|
||||||
|
LOGE("[TX_ID: %" PRIu64 "] Interceptor DEAD. Blocking to prevent attestation leak.", tx_id);
|
||||||
|
result = DEAD_OBJECT;
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
LOGW("[TX_ID: %" PRIu64 "] Pre-transaction callback failed (not dead). Forwarding.", tx_id);
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
int32_t action = pre_resp.readInt32();
|
int32_t action = pre_resp.readInt32();
|
||||||
|
|||||||
@@ -17,6 +17,18 @@ data class KeyIdentifier(val uid: Int, val alias: String)
|
|||||||
/** A collection of utility functions to support binder interception. */
|
/** A collection of utility functions to support binder interception. */
|
||||||
object InterceptorUtils {
|
object InterceptorUtils {
|
||||||
|
|
||||||
|
private const val EX_SERVICE_SPECIFIC = -8
|
||||||
|
|
||||||
|
fun createErrorReply(errorCode: Int): BinderInterceptor.TransactionResult.OverrideReply {
|
||||||
|
val parcel = Parcel.obtain().apply {
|
||||||
|
writeInt(EX_SERVICE_SPECIFIC)
|
||||||
|
writeString(null)
|
||||||
|
writeInt(0)
|
||||||
|
writeInt(errorCode)
|
||||||
|
}
|
||||||
|
return BinderInterceptor.TransactionResult.OverrideReply(parcel)
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Uses reflection to get the integer transaction code for a given method name from a Stub
|
* Uses reflection to get the integer transaction code for a given method name from a Stub
|
||||||
* class. This is necessary for older Android versions where codes are not public constants.
|
* class. This is necessary for older Android versions where codes are not public constants.
|
||||||
|
|||||||
+26
-24
@@ -22,6 +22,7 @@ import java.security.spec.PKCS8EncodedKeySpec
|
|||||||
import java.util.concurrent.ConcurrentHashMap
|
import java.util.concurrent.ConcurrentHashMap
|
||||||
import java.util.concurrent.Executors
|
import java.util.concurrent.Executors
|
||||||
import org.matrix.TEESimulator.attestation.AttestationBuilder
|
import org.matrix.TEESimulator.attestation.AttestationBuilder
|
||||||
|
import org.matrix.TEESimulator.attestation.AttestationConstants
|
||||||
import org.matrix.TEESimulator.attestation.AttestationPatcher
|
import org.matrix.TEESimulator.attestation.AttestationPatcher
|
||||||
import org.matrix.TEESimulator.attestation.KeyMintAttestation
|
import org.matrix.TEESimulator.attestation.KeyMintAttestation
|
||||||
import org.matrix.TEESimulator.config.ConfigurationManager
|
import org.matrix.TEESimulator.config.ConfigurationManager
|
||||||
@@ -70,7 +71,7 @@ class KeyMintSecurityLevelInterceptor(
|
|||||||
GENERATE_KEY_TRANSACTION -> {
|
GENERATE_KEY_TRANSACTION -> {
|
||||||
logTransaction(txId, transactionNames[code]!!, callingUid, callingPid)
|
logTransaction(txId, transactionNames[code]!!, callingUid, callingPid)
|
||||||
|
|
||||||
if (!shouldSkip) return handleGenerateKey(callingUid, callingPid, data)
|
if (!shouldSkip) return handleGenerateKey(txId, callingUid, callingPid, data)
|
||||||
}
|
}
|
||||||
CREATE_OPERATION_TRANSACTION -> {
|
CREATE_OPERATION_TRANSACTION -> {
|
||||||
logTransaction(txId, transactionNames[code]!!, callingUid, callingPid)
|
logTransaction(txId, transactionNames[code]!!, callingUid, callingPid)
|
||||||
@@ -268,17 +269,19 @@ class KeyMintSecurityLevelInterceptor(
|
|||||||
|
|
||||||
val opParams = data.createTypedArray(KeyParameter.CREATOR)!!
|
val opParams = data.createTypedArray(KeyParameter.CREATOR)!!
|
||||||
val parsedOpParams = KeyMintAttestation(opParams)
|
val parsedOpParams = KeyMintAttestation(opParams)
|
||||||
data.readBoolean() // forced: no-op for sw ops
|
val forced = data.readBoolean()
|
||||||
|
|
||||||
val keyParams = generatedKeyInfo.keyParams
|
|
||||||
|
|
||||||
val requestedPurpose = parsedOpParams.purpose.firstOrNull()
|
val requestedPurpose = parsedOpParams.purpose.firstOrNull()
|
||||||
if (requestedPurpose == null) {
|
if (requestedPurpose == null) {
|
||||||
return InterceptorUtils.createServiceSpecificErrorReply(
|
return InterceptorUtils.createServiceSpecificErrorReply(KEYMINT_INVALID_ARGUMENT)
|
||||||
KeystoreErrorCode.INVALID_ARGUMENT
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (forced) {
|
||||||
|
return InterceptorUtils.createServiceSpecificErrorReply(PERMISSION_DENIED)
|
||||||
|
}
|
||||||
|
|
||||||
|
val keyParams = generatedKeyInfo.keyParams
|
||||||
|
|
||||||
val algorithm = keyParams.algorithm
|
val algorithm = keyParams.algorithm
|
||||||
val isAsymmetric = algorithm == Algorithm.EC || algorithm == Algorithm.RSA
|
val isAsymmetric = algorithm == Algorithm.EC || algorithm == Algorithm.RSA
|
||||||
val unsupported =
|
val unsupported =
|
||||||
@@ -408,7 +411,7 @@ class KeyMintSecurityLevelInterceptor(
|
|||||||
* Handles the `generateKey` transaction. Based on the configuration for the calling UID, it
|
* 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.
|
* either generates a key in software or lets the call pass through to the hardware.
|
||||||
*/
|
*/
|
||||||
private fun handleGenerateKey(callingUid: Int, callingPid: Int, data: Parcel): TransactionResult {
|
private fun handleGenerateKey(txId: Long, callingUid: Int, callingPid: Int, data: Parcel): TransactionResult {
|
||||||
return runCatching {
|
return runCatching {
|
||||||
data.enforceInterface(IKeystoreSecurityLevel.DESCRIPTOR)
|
data.enforceInterface(IKeystoreSecurityLevel.DESCRIPTOR)
|
||||||
val keyDescriptor = data.readTypedObject(KeyDescriptor.CREATOR)!!
|
val keyDescriptor = data.readTypedObject(KeyDescriptor.CREATOR)!!
|
||||||
@@ -418,21 +421,26 @@ class KeyMintSecurityLevelInterceptor(
|
|||||||
)
|
)
|
||||||
|
|
||||||
val params = data.createTypedArray(KeyParameter.CREATOR)!!
|
val params = data.createTypedArray(KeyParameter.CREATOR)!!
|
||||||
|
val parsedParams = KeyMintAttestation(params)
|
||||||
|
|
||||||
// Caller-provided CREATION_DATETIME is not allowed.
|
val challenge = parsedParams.attestationChallenge
|
||||||
|
if (challenge != null && challenge.size > AttestationConstants.CHALLENGE_LENGTH_LIMIT) {
|
||||||
|
SystemLogger.warning("[TX_ID: $txId] Rejecting oversized attestation challenge: ${challenge.size} bytes (max ${AttestationConstants.CHALLENGE_LENGTH_LIMIT})")
|
||||||
|
return InterceptorUtils.createErrorReply(KEYMINT_INVALID_INPUT_LENGTH)
|
||||||
|
}
|
||||||
|
|
||||||
if (params.any { it.tag == Tag.CREATION_DATETIME }) {
|
if (params.any { it.tag == Tag.CREATION_DATETIME }) {
|
||||||
return@runCatching InterceptorUtils.createServiceSpecificErrorReply(
|
SystemLogger.warning("[TX_ID: $txId] Rejecting CREATION_DATETIME in generateKey params")
|
||||||
INVALID_ARGUMENT
|
return InterceptorUtils.createErrorReply(INVALID_ARGUMENT)
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Device ID attestation requires READ_PRIVILEGED_PHONE_STATE.
|
|
||||||
val hasDeviceIdTags =
|
val hasDeviceIdTags =
|
||||||
params.any {
|
params.any {
|
||||||
it.tag == Tag.ATTESTATION_ID_SERIAL ||
|
it.tag == Tag.ATTESTATION_ID_SERIAL ||
|
||||||
it.tag == Tag.ATTESTATION_ID_IMEI ||
|
it.tag == Tag.ATTESTATION_ID_IMEI ||
|
||||||
it.tag == Tag.ATTESTATION_ID_MEID ||
|
it.tag == Tag.ATTESTATION_ID_MEID ||
|
||||||
it.tag == Tag.DEVICE_UNIQUE_ATTESTATION
|
it.tag == Tag.DEVICE_UNIQUE_ATTESTATION ||
|
||||||
|
it.tag == Tag.ATTESTATION_ID_SECOND_IMEI
|
||||||
}
|
}
|
||||||
if (
|
if (
|
||||||
hasDeviceIdTags &&
|
hasDeviceIdTags &&
|
||||||
@@ -441,13 +449,9 @@ class KeyMintSecurityLevelInterceptor(
|
|||||||
"android.permission.READ_PRIVILEGED_PHONE_STATE",
|
"android.permission.READ_PRIVILEGED_PHONE_STATE",
|
||||||
)
|
)
|
||||||
) {
|
) {
|
||||||
return@runCatching InterceptorUtils.createServiceSpecificErrorReply(
|
return InterceptorUtils.createErrorReply(CANNOT_ATTEST_IDS)
|
||||||
CANNOT_ATTEST_IDS
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// INCLUDE_UNIQUE_ID requires SELinux gen_unique_id OR Android
|
|
||||||
// REQUEST_UNIQUE_ID_ATTESTATION (security_level.rs:478-485).
|
|
||||||
if (params.any { it.tag == Tag.INCLUDE_UNIQUE_ID }) {
|
if (params.any { it.tag == Tag.INCLUDE_UNIQUE_ID }) {
|
||||||
val hasSELinux =
|
val hasSELinux =
|
||||||
ConfigurationManager.checkSELinuxPermission(
|
ConfigurationManager.checkSELinuxPermission(
|
||||||
@@ -461,13 +465,9 @@ class KeyMintSecurityLevelInterceptor(
|
|||||||
"android.permission.REQUEST_UNIQUE_ID_ATTESTATION",
|
"android.permission.REQUEST_UNIQUE_ID_ATTESTATION",
|
||||||
)
|
)
|
||||||
if (!hasSELinux && !hasAndroid) {
|
if (!hasSELinux && !hasAndroid) {
|
||||||
return@runCatching InterceptorUtils.createServiceSpecificErrorReply(
|
return InterceptorUtils.createServiceSpecificErrorReply(PERMISSION_DENIED)
|
||||||
PERMISSION_DENIED
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
val parsedParams = KeyMintAttestation(params)
|
|
||||||
val isAttestKeyRequest = parsedParams.isAttestKey()
|
val isAttestKeyRequest = parsedParams.isAttestKey()
|
||||||
|
|
||||||
val forceGenerate =
|
val forceGenerate =
|
||||||
@@ -878,6 +878,8 @@ class KeyMintSecurityLevelInterceptor(
|
|||||||
|
|
||||||
@Volatile var teeFunctional = false
|
@Volatile var teeFunctional = false
|
||||||
|
|
||||||
|
private const val KEYMINT_INVALID_INPUT_LENGTH = -21
|
||||||
|
private const val KEYMINT_INVALID_ARGUMENT = -38
|
||||||
private const val INVALID_ARGUMENT = 20
|
private const val INVALID_ARGUMENT = 20
|
||||||
private const val PERMISSION_DENIED = 6
|
private const val PERMISSION_DENIED = 6
|
||||||
private const val SECURE_HW_COMMUNICATION_FAILED = -49
|
private const val SECURE_HW_COMMUNICATION_FAILED = -49
|
||||||
|
|||||||
Reference in New Issue
Block a user