Skip to content

Fix/align keystore2 interception model#157

Open
MhmRdd wants to merge 56 commits intoJingMatrix:mainfrom
MhmRdd:fix/align-keystore2-interception-model
Open

Fix/align keystore2 interception model#157
MhmRdd wants to merge 56 commits intoJingMatrix:mainfrom
MhmRdd:fix/align-keystore2-interception-model

Conversation

@MhmRdd
Copy link
Copy Markdown
Contributor

@MhmRdd MhmRdd commented Mar 18, 2026

No description provided.

MhmRdd added 21 commits March 18, 2026 18:24
In real AOSP keystore2, USER_ID is added at SecurityLevel.SOFTWARE
(see store_new_key in security_level.rs), since the application UID
is a software concept not enforced by the TEE. The previous code
assigned all authorizations the same TEE/StrongBox security level,
making generated keys distinguishable from real ones.
The IKeystoreOperation AIDL interface defines updateAad() for providing
additional authenticated data in AEAD modes (e.g. AES-GCM). The previous
SoftwareOperationBinder inherited the default stub which throws
UnsupportedOperationException, causing crashes for GCM operations.

Add updateAad to the CryptoPrimitive interface with proper implementations:
cipher.updateAAD() for CipherPrimitive, no-op for Signer/Verifier.
When operating in PATCH mode, the post-transaction hooks for both
generateKey and getKeyEntry were patching the certificate chain to
embed custom patch levels, but leaving the KeyMetadata.authorizations
array untouched. This created an inconsistency where the attestation
certificate reported one set of patch levels while the authorizations
(OS_PATCHLEVEL, VENDOR_PATCHLEVEL, BOOT_PATCHLEVEL) still contained
the real device values.

Add patchAuthorizations() utility to InterceptorUtils and apply it
in both the SecurityLevel post-generateKey hook and the KeystoreService
post-getKeyEntry hook.
The signing algorithm for the leaf certificate must match the signing
key's type (from keybox or attestation key), not the subject key's
algorithm. An EC attestation key signing an RSA subject key's
certificate would previously select SHA256withRSA (wrong) instead of
SHA256withECDSA, producing an invalid or failing certificate.
Software operations now track finalization state and reject calls after
finish/abort with INVALID_OPERATION_HANDLE, matching AOSP operation.rs
outcome tracking. Errors during update/updateAad also finalize the
operation.

SoftwareOperationBinder wraps all methods in synchronized blocks to
prevent concurrent access, matching AOSP's Mutex-protected
KeystoreOperation wrapper that returns OPERATION_BUSY.

Input data is validated against MAX_RECEIVE_DATA (32KB) on update,
updateAad, and finish to match the AOSP-enforced limit.

CryptoPrimitive gains getBeginParameters() for exposing begin-phase
output (e.g. GCM nonce/IV) via CreateOperationResponse.parameters.
The post-importKey hook previously only cleaned up cached data but never
patched the KeyMetadata returned from importKey. Imported asymmetric
keys with attestation chains now get the same certificate and
authorization patching treatment as generated keys.

Also remove the unconditional early return for imported keys in the
post-getKeyEntry handler. The existing chain-length check already
correctly handles keys without attestation chains, so the blanket
import-key skip was overly broad.
AOSP returns begin_result.params in CreateOperationResponse.parameters,
which contains the IV/nonce for AES-GCM encryption operations. Software
operations previously left this field null, so clients expecting the
server-generated IV from the response would not receive it.

CipherPrimitive now exposes cipher.iv as a NONCE KeyParameter via
getBeginParameters(), surfaced through SoftwareOperation.beginParameters
and into the CreateOperationResponse.
AOSP keystore2 reports all errors via ServiceSpecificException with
numeric codes: negative values for KeyMint ErrorCode (e.g. -28 for
INVALID_OPERATION_HANDLE, -30 for VERIFICATION_FAILED) and positive
values for Keystore2 ResponseCode (e.g. 16 for TOO_MUCH_DATA). The
binder framework serializes these as EX_SERVICE_SPECIFIC on the wire.

Previously, software operations threw plain Java exceptions
(IllegalStateException, SignatureException) which serialize differently
and produce unrecognizable error codes on the client side.

Replace all error paths with ServiceSpecificException using the correct
AOSP error codes. Add ServiceSpecificException stub to the stub module.
The AOSP KeyMint reference implementation uses "CN=Android Keystore Key"
(lowercase 's') as the default certificate subject when no
CERTIFICATE_SUBJECT tag is provided. The previous value used camelCase
"KeyStore" which differs from real hardware output.
AOSP add_required_parameters (security_level.rs) sets default
CERTIFICATE_NOT_BEFORE to 0 (Unix epoch) and CERTIFICATE_NOT_AFTER to
253402300799000 (9999-12-31T23:59:59 UTC, the RFC 5280 GeneralizedTime
maximum). Since TEESimulator intercepts before add_required_parameters
runs, the defaults must match what keystore2 would have injected.

Previously, notBefore defaulted to the current time and notAfter to
one year from now, producing certificates with drastically different
validity periods than real hardware.

Also add null-safe default for RSA public exponent using
RSAKeyGenParameterSpec.F4 (65537), preventing potential NPE when the
caller omits RSA_PUBLIC_EXPONENT.
AOSP add_required_parameters (security_level.rs) only adds
ATTESTATION_APPLICATION_ID to the key parameters when an
ATTESTATION_CHALLENGE tag is present. The previous code unconditionally
included it in every softwareEnforced list, violating the Android Key
Attestation specification.

Pass KeyMintAttestation params to buildSoftwareEnforcedList and gate
the ATTESTATION_APPLICATION_ID entry on attestationChallenge != null.
When overriding a hardware attestation key in the post-getKeyEntry hook,
the response metadata's key.nspace was not updated to the new randomly
assigned value. This caused subsequent createOperation calls to fail
because the nspace in the cached GeneratedKeyInfo did not match the
descriptor the client received.

Also patch the authorizations (patch levels) in this path, matching
the behavior already applied to non-attestation hardware keys.
On Windows with core.autocrlf=true, shell scripts were checked out
with CRLF line endings and packaged as-is into the flashable zip.
Android's sh interpreter cannot parse CRLF scripts, causing
"syntax error: unexpected word" on module installation.

Force *.sh and module/daemon to always use LF regardless of platform.
For EC keys, callers often provide only EC_CURVE (e.g. P-256) without
an explicit KEY_SIZE tag. The parser defaulted keySize to 0, causing
the attestation teeEnforced list and KeyMetadata authorizations to
report keySize=0 instead of the correct value (e.g. 256 for P-256).

Add deriveKeySizeFromCurve() that maps EcCurve constants to their
corresponding key sizes as a fallback when KEY_SIZE is absent.
createOperation now validates the requested purpose against the key's
allowed purposes before creating a SoftwareOperation. Mismatched
purposes return INCOMPATIBLE_PURPOSE (-3) via ServiceSpecificException,
matching AOSP enforcements.rs authorize_create behavior. Previously,
mismatched purposes caused SoftwareOperation constructor to throw an
unrelated error, which fell through as KEY_NOT_FOUND.

Also wrap SoftwareOperation creation in runCatching so any construction
failure returns a proper ServiceSpecificException error reply instead
of propagating as an unhandled exception.

generateKey now rejects caller-provided CREATION_DATETIME with
INVALID_ARGUMENT (20), matching AOSP add_required_parameters which
explicitly forbids callers from specifying this tag.

Add createServiceSpecificErrorReply utility to InterceptorUtils for
writing ServiceSpecificException to binder reply parcels.
ResponseCode::TOO_MUCH_DATA is 21 in AOSP, not 16. Add remaining
AOSP error code constants (KEY_EXPIRED, KEY_NOT_YET_VALID,
CALLER_NONCE_PROHIBITED, INVALID_ARGUMENT, PERMISSION_DENIED,
KEY_NOT_FOUND) for use in operation enforcement.

Add onFinishCallback to SoftwareOperation for USAGE_COUNT_LIMIT
enforcement on successful finish.
Add parsing for ACTIVE_DATETIME, ORIGINATION_EXPIRE_DATETIME,
USAGE_EXPIRE_DATETIME, USAGE_COUNT_LIMIT, CALLER_NONCE, and
UNLOCKED_DEVICE_REQUIRED to KeyMintAttestation. These are needed
both for reflecting them in the attestation extension and for
enforcing key policies during createOperation.
Add CALLER_NONCE, ACTIVE_DATETIME, ORIGINATION_EXPIRE_DATETIME,
USAGE_EXPIRE_DATETIME, USAGE_COUNT_LIMIT, and UNLOCKED_DEVICE_REQUIRED
to the teeEnforced AuthorizationList in the attestation extension when
present in the key generation parameters. Previously these tags were
silently dropped, allowing detection by generating a key with these
constraints and observing they're absent from the attestation.
Software-generated keys now enforce the same operation policies as
AOSP keystore2's authorize_create():

- Missing PURPOSE rejected with INVALID_ARGUMENT (-38)
- Incompatible PURPOSE rejected with INCOMPATIBLE_PURPOSE (-3)
- Forced operations rejected with PERMISSION_DENIED (6)
- ACTIVE_DATETIME in future rejected with KEY_NOT_YET_VALID (-24)
- ORIGINATION_EXPIRE past rejected with KEY_EXPIRED (-25) for SIGN/ENCRYPT
- USAGE_EXPIRE past rejected with KEY_EXPIRED (-25) for DECRYPT/VERIFY
- CALLER_NONCE without permission rejected with CALLER_NONCE_PROHIBITED (-55)
- USAGE_COUNT_LIMIT enforced on finish via callback; key deleted on exhaustion

Store KeyMintAttestation in GeneratedKeyInfo so enforcement checks can
access the original key parameters during createOperation.
handleCreateOperation only accepted Domain.KEY_ID descriptors, rejecting
Domain.APP with ContinueAndSkipPost. Native callers and the Android
framework can call createOperation with Domain.APP + alias, which was
being forwarded to hardware where the software-generated key doesn't
exist, resulting in KEY_NOT_FOUND for all operation enforcement tests.

Add alias-based lookup from generatedKeys when domain is APP, matching
AOSP's create_operation which resolves all domain types via database.
AOSP rejects VERIFY and ENCRYPT for asymmetric keys (EC/RSA) at the HAL
level with UNSUPPORTED_PURPOSE (-2), distinct from INCOMPATIBLE_PURPOSE
(-3) which applies when the purpose isn't in the key's authorized list.
Add algorithm-level check before purpose-list check.

Fix USAGE_COUNT_LIMIT tracking: use nspace comparison instead of
identity comparison for key lookup, and check exhaustion before creating
the operation to return KEY_NOT_FOUND when the limit is reached.
@Enginex0
Copy link
Copy Markdown
Contributor

You obviously pushed all my commits here, he said he had a different architecture in mind, so it's of no use at all

And some things will conflict

@JingMatrix JingMatrix linked an issue Mar 18, 2026 that may be closed by this pull request
@MhmRdd
Copy link
Copy Markdown
Contributor Author

MhmRdd commented Mar 18, 2026

You obviously pushed all my commits here, he said he had a different architecture in mind, so it's of no use at all

And some things will conflict

1- I don't even know you, so i have no clue with what you meant by "pushed all your commits"
2- "different architecture": i don't see any other options than these two:

  • intercepting binder requests of keystore
  • intercepting binder requests of hardware HAL
    The second option is not yet studied due to changes made by vendors.

@JingMatrix
Copy link
Copy Markdown
Owner

@MhmRdd If what @Enginex0 said is true, we should properly credit to him, using Co-authored-by: for certain commits.

I will start reviewing when the PR is ready. It seems to take a long time.

@MhmRdd
Copy link
Copy Markdown
Contributor Author

MhmRdd commented Mar 18, 2026

Cool, i don't mind anyway although I'm sure those were made manually here by comparing AOSP, there are still a lot of changes i need to do with local tests.

@JingMatrix
Copy link
Copy Markdown
Owner

@Enginex0 Could you provide some proofs why you claim that certains commits credit to you ?

…age tracking

Reject ATTESTATION_ID_SERIAL, ATTESTATION_ID_IMEI, ATTESTATION_ID_MEID,
and DEVICE_UNIQUE_ATTESTATION in generateKey with CANNOT_ATTEST_IDS
(-66), since normal apps lack READ_PRIVILEGED_PHONE_STATE.

Fix createOperation to merge key params with operation params when
constructing SoftwareOperation: use the key's algorithm/digest for
crypto, but the operation's purpose. Previously, missing DIGEST in
operation params caused signature algorithm resolution to fail.

Fix USAGE_COUNT_LIMIT by resolving KeyIdentifier during key lookup
instead of re-searching generatedKeys by nspace inside the let block.
@Enginex0
Copy link
Copy Markdown
Contributor

Oh okay, it's sitting right there on my repo fork, these were done a few hours ago @fatalcoder also contributed at some point with some Pr as well

MhmRdd added 6 commits March 19, 2026 03:08
AES and HMAC keys were failing in GENERATE mode because
doSoftwareGeneration only handled asymmetric key pairs. Generate
symmetric keys via javax.crypto.KeyGenerator and return KeyMetadata
without certificates (symmetric keys have no cert chain).

Store SecretKey in GeneratedKeyInfo alongside KeyPair. Update
SoftwareOperation and CipherPrimitive to accept either key type.
Software key generation completes in ~8ms, while real TEE hardware
(QTEE, Trustonic) takes 55-75ms. Add TeeLatencySimulator that models
three independent latency components:

1. Base crypto delay (log-normal, algorithm-dependent): EC ~45ms,
   RSA ~55ms, AES ~30ms. Log-normal matches the positive skew
   observed in real hardware measurements.
2. Kernel/binder transit noise (exponential, mean 5ms): models IPC
   scheduling jitter with occasional spikes.
3. TEE scheduler jitter (Gaussian, stddev 3ms): models non-deterministic
   TrustZone world-switch cost.

A per-boot session bias (Gaussian, stddev 8ms) shifts the distribution
to prevent cross-session fingerprinting. The actual generation time is
subtracted so faster CPUs get more padding naturally.

Distribution parameters derived from observed QTEE (7 sessions, 56
runs) and Trustonic (7 sessions, 56 runs) timing profiles.
Restore the concurrent CompletableFuture race between TEE hardware
and software generation. The original KEY_NOT_FOUND issue was caused
by the TEE-generated key not being cached in generatedKeys, making
getKeyEntry unable to find it through our pre-hook.

Cache the patched TEE result in generatedKeys after the hardware path
succeeds, so getKeyEntry returns it correctly regardless of which UID
namespace the hardware stored it under.
deleteKey was intercepting TEE-generated keys (found via teeResponses)
and returning success without forwarding to real hardware, leaving
orphaned keys in the keystore2 database. Only skip hardware for
software-generated keys that exist solely in our generatedKeys cache.
@MhmRdd
Copy link
Copy Markdown
Contributor Author

MhmRdd commented Mar 19, 2026

Screenshot_2026-03-19-05-01-11-77_84d3000e3f4017145260f7618db1d683.jpg

@MhmRdd
Copy link
Copy Markdown
Contributor Author

MhmRdd commented Mar 19, 2026

GG

@MhmRdd MhmRdd marked this pull request as ready for review March 19, 2026 04:01
MhmRdd added 4 commits March 19, 2026 05:49
listEntries and listEntriesBatched inject software-generated keys into
results, but getNumberOfEntries returned only the hardware database
count. An app verifying count == list.size would detect the mismatch.

Intercept getNumberOfEntries in the post-hook, add the count of
software keys matching the caller UID to the hardware count.
deleteKey only resolved keys by alias (Domain::APP). Keys deleted
via Domain::KEY_ID with nspace were skipped, leaving stale entries
in generatedKeys and teeResponses. Resolve by nspace via
findGeneratedKeyByKeyId for KEY_ID domain deletes.
Enginex0 added a commit to Enginex0/TEESimulator-RS that referenced this pull request Mar 19, 2026
Enginex0 added a commit to Enginex0/TEESimulator-RS that referenced this pull request Mar 21, 2026
Full diff analysis against upstream's 50 commits revealed 8 functional
gaps after v5.0. These are detectable by conformance tests or detector
apps inspecting KeyMetadata authorizations and operation semantics.

KeyMetadata authorizations:
- Add 9 TEE-enforced tags (CALLER_NONCE, MIN_MAC_LENGTH, ROLLBACK_RESISTANCE,
  EARLY_BOOT_ONLY, ALLOW_WHILE_ON_BODY, TRUSTED_USER_PRESENCE_REQUIRED,
  TRUSTED_CONFIRMATION_REQUIRED, MAX_USES_PER_BOOT, MAX_BOOT_LEVEL)
- Fix CREATION_DATETIME to SOFTWARE security level via createSwAuth
- Add SOFTWARE-enforced date enforcement, USAGE_COUNT_LIMIT, UNLOCKED_DEVICE_REQUIRED

Symmetric key support:
- Generate AES/HMAC keys in software via javax.crypto.KeyGenerator
- GeneratedKeyInfo expanded with nullable keyPair + secretKey fields
- CipherPrimitive accepts java.security.Key for symmetric operations
- SoftwareOperation routes ENCRYPT/DECRYPT to secretKey when available

Operation compliance:
- beginParameters property replaces manual IV wrapping for GCM
- KeyAgreementPrimitive for ECDH AGREE_KEY operations
- handleCreateOperation wrapped in runCatching (crash prevention)
- SECURE_HW_COMMUNICATION_FAILED on software gen failure

Certificate patching:
- Import key cert chain + authorization patching in onPostTransact
- patchAuthorizations added to post-generateKey PATCH mode path
Enginex0 added a commit to Enginex0/TEESimulator-RS that referenced this pull request Mar 21, 2026
…tency

AUTO mode now races TEE hardware against software generation via
CompletableFuture. If TEE succeeds, the cert chain is patched and
cached in teeResponses before returning, making attestation
stress-resilient. If TEE fails, software fallback is used.

ConfigurationManager no longer resolves AUTO at config time; it
passes Mode.AUTO through to KeyMintSecurityLevelInterceptor for
runtime dispatch. shouldPatch() returns true for both PATCH and
AUTO modes. TEE status file persistence removed entirely.

Aligns handleGenerateKey with upstream PR JingMatrix#157 three-way dispatch:
forceGenerate, raceTeePatch, or hardware forwarding with post-patch.

Hardware keygen rate limiting removed (replaced by raceTeePatch for
AUTO, plain Continue for PATCH). Attest key override in
Keystore2Interceptor now patches authorizations and uses null-safe
nspace assignment.
Enginex0 added a commit to Enginex0/TEESimulator-RS that referenced this pull request Mar 21, 2026
Our reimplementation of PR 157's logic had a silent divergence causing
G10 to still fail under binder stress. Instead of hunting line-by-line,
replace all shared Kotlin/Java/C++ files with PR 157's exact proven
versions that pass all 63 conformance tests. Our Rust-exclusive files
(NativeCertGen, GeneratedKeyPersistence, native-certgen crate) remain
in the repo but are dormant until re-wired in a follow-up commit.
Enginex0 added a commit to Enginex0/TEESimulator-RS that referenced this pull request Mar 21, 2026
Commit 93f937e introduced G2-specific fixes (ratio dropped from 5.00x
to 2.10x) that were lost when resetting to upstream PR JingMatrix#157 at 6c270ac.

Restores: try-catch safety in BinderInterceptor.onTransact, shouldPatch
early-exit in getKeyEntry post-transact, safe parcel reads (!! to ?:)
at 6 sites, teeResponses cache population in generateKey/importKey
post-transact, uncaught exception handler in App.kt, and removes the
pingBinder liveness check that added ~1.8x overhead per pre-transact.
@JingMatrix
Copy link
Copy Markdown
Owner

This pull-request contains too many commits, I will audit them manually, via the branch merge.

A new branch wip_157 is created to keep a backup of the original commits.

@MhmRdd
Copy link
Copy Markdown
Contributor Author

MhmRdd commented Mar 21, 2026

There are some few more changes left that i haven't pushed yet, since i didn't have time to review issues reported.
Maybe I'll push and report problems found by members who tested it.

@JingMatrix
Copy link
Copy Markdown
Owner

@MhmRdd No problem, currently I am only merging commits (audited) that are safe to me.
Experimental commits will be delayed, you can keep working on it.

@JingMatrix
Copy link
Copy Markdown
Owner

Changes related to OperationInterceptor are better viewed under a separated pull-request, since they can be categorized as non-implemented features.

@MhmRdd, have you finished these changes (related to OperationInterceptor) ?
If so, @XiaoTong6666 could help to first review them by creating a more systematic pr on this subject, and then I will start to audit / review it.

Currently, I will prioritize merging commits addressing bugs I left in my previous commits.

Some other experimental features, such as TEE status refactor and simulated latency, will be discussed later.

@0x7F00
Copy link
Copy Markdown

0x7F00 commented Mar 27, 2026

Screenshot_2026-03-28-03-00-48-17_9e8df3d0c7c1f50248b6ee043a653d26 The test result on native test is Conventional Tests (8).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Detected by DuckDetector

4 participants