Align Keystore2 Operation Interceptions - #171
Closed
XiaoTong6666 wants to merge 18 commits into
Closed
Conversation
Owner
|
In commit c129a8a, please use my comment styles, which are from the file There is no need to repeat this reference url. I have put it at the beginning. |
Owner
|
Commit cdf1fac seems not suitable for current pr. |
Author
This commit combines the parser and metadata plumbing that the later software-operation work depends on: - derive KEY_SIZE from EC_CURVE when KEY_SIZE is absent - parse enforcement tags from key generation parameters into KeyMintAttestation - add patchAuthorizations() to rewrite patch-level authorizations alongside patched metadata - add updateAad() plumbing for software-backed operations It also widens GeneratedKeyInfo to carry parsed key parameters for the follow-on createOperation enforcement work. (cherry picked from commit e767649) (cherry picked from commit 4bc4713) (cherry picked from commit 45ebf9a) (cherry picked from commit c263ab1)
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. (cherry picked from commit 962cef6)
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. (cherry picked from commit da452cf)
Replace ad-hoc operation exceptions with ServiceSpecificException so the software-backed binder path returns AOSP-compatible error codes. This commit also folds in the follow-up error-code cleanup: - set TOO_MUCH_DATA to the correct keystore2 response value (21) - add the missing AOSP error-code constants used by the software operation path - align finish/update/updateAad failure propagation with the later usage-limit and onFinishCallback flow (cherry picked from commit 0ebdded) (cherry picked from commit 60d978d)
Software-generated keys now enforce the same operation policies as AOSP keystore2 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. This commit combines four steps from the same authorize_create evolution path: - 3078ea9 introduced the main AOSP authorize_create enforcement flow. - 50cd77f added the earlier purpose validation and caller-provided CREATION_DATETIME rejection that were later folded into the aligned validation path. - 07c98bc contributed the follow-up fixes around operation parameter handling and usage tracking that now live in the final createOperation enforcement implementation. - 2bc46be refined the unsupported-purpose and usage-count-limit enforcement edge cases. (cherry picked from commit 3078ea9) (cherry picked from commit 50cd77f) (cherry picked from commit 07c98bc) (cherry picked from commit 2bc46be)
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. (cherry picked from commit 890ee70)
…hecks This commit combines two related pieces of plumbing around the keystore2 interception path: - filter binder transaction codes at native registration time to avoid unnecessary Java round-trips on unintercepted calls - add READ_PRIVILEGED_PHONE_STATE-based permission checks before rejecting device ID attestation tags during generateKey The native filtering applies to IKeystoreService, IKeystoreSecurityLevel, and IKeystoreOperation registrations. The permission gating adds ConfigurationManager.hasPermissionForUid(), the IPackageManager checkPermission() stub, and the generateKey-side device-ID attestation validation path. (cherry picked from commit ca3fcbc) (cherry picked from commit ed98768)
AOSP enforcements.rs rejects AGREE_KEY for any algorithm that is not EC, not just RSA. Restructure the unsupported purpose check to match the exact authorize_create decision tree. (cherry picked from commit 509d157)
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. (cherry picked from commit d3bf3c8)
(cherry picked from commit 492d6dc)
…tions (cherry picked from commit 25538da)
(cherry picked from commit d839b69)
(cherry picked from commit 7a7e362)
Patched certificate chains now update KeyMetadata.authorizations, and createOperation bookkeeping follows the later aligned key-resolution path. - PATCH-mode certificate updates now patch authorizations alongside the certificate chain - createOperation usage tracking now resolves counters by the resolved key id - this builds on the earlier Domain.APP key-resolution path already introduced in the existing createOperation history This commit combines follow-up steps from the same metadata and createOperation alignment path: - 45ebf9a patched authorizations alongside certificate chains in PATCH mode. - 07c98bc contributed the follow-up fixes around operation parameter handling and usage tracking that now live in the aligned createOperation bookkeeping path. (cherry picked from commit 45ebf9a) (cherry picked from commit 07c98bc) Co-authored-by: Mohammed Riad <52679407+MhmRdd@users.noreply.github.com>
Software-generated KeyMetadata now skips OS/VENDOR/BOOT patch level authorizations when the configured value is DO_NOT_REPORT, matching the existing certificate-patching behavior and the later aligned metadata semantics. - OS_PATCHLEVEL is omitted when configured to not report - VENDOR_PATCHLEVEL is omitted when configured to not report - BOOT_PATCHLEVEL is omitted when configured to not report This commit aligns the generated metadata path with the same patch-level hiding semantics already applied when patching certificate chains. (cherry picked from commit 492d6dc) Co-authored-by: Mohammed Riad <52679407+MhmRdd@users.noreply.github.com>
Follow the comment style used on main: drop the per-line "file.rs: l=NNN" citations and the internal F# finding-code prefixes, keeping AOSP references as prose or whole-file cs.android.com links. While cross-checking the comments against AOSP, correct two small mismatches: - add ATTESTATION_ID_SECOND_IMEI to the device-ID attestation tag set, to match is_device_id_attestation_tag in utils.rs - remove the unused updateCertificateChain(metadata, chain) overload that had displaced the callingUid KDoc from the real function
Revert four commits that drifted outside this PR's scope. They stay in the branch history so they can be reused in a more fitting change later: - Filter intercepted transaction codes and gate device ID attestation checks - Fix silent error paths, challenge error code, and null handling - Add dir class to sepolicy and crash safety for binder interceptors - Hide DO_NOT_REPORT patch levels in generated KeyMetadata Also remove the leftover patch-level authorization rewriting (InterceptorUtils.patchAuthorizations and its call sites) and the now-unused PERMISSION_DENIED code, both of which concern generateKey/attestation metadata rather than the operation interception this PR is about.
Owner
|
Close as TEESimulator 4.0 has solved this in a completely new foundation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes: