fix(intercept): use ContinueAndSkipPost for KEY_ID createOperation NOT FOUND#26
Conversation
…T FOUND When handleCreateOperation receives a Domain.KEY_ID request for a key not in our generatedKeys cache, it correctly forwards to the real HAL. However, it previously returned TransactionResult.Continue, which lets the post-handler run. The post-handler unconditionally registers an OperationInterceptor on the IKeystoreOperation binder returned by real keystore2. This interceptor then interferes with the caller's operation (intercepting finish/abort/updateAad calls). On devices where vendor daemons (e.g. fingerprint calibration) use Domain.KEY_ID for their hardware-backed keys, this causes operation failures — the OperationInterceptor races with the immediate finish/abort call and may reject updateAad with INVALID_TAG if the operation is not GCM mode. Fix: return ContinueAndSkipPost (matching the existing Domain.APP NOT FOUND path) so the post-handler never runs for operations on keys we don't own. When the key IS in generatedKeys, we never reach this return — we proceed to create a SoftwareOperation directly in pre-transact. Symptom: OnePlus engineering mode ultrasonic fingerprint calibration hash retrieval fails with module enabled, works with module disabled. Signed-off-by: Andrea-lyz <Andrea-lyz@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR modifies how the KeyMint security level interceptor handles failed KeyId lookups. When a requested KeyId cannot be resolved from in-memory generated keys, the interceptor now skips post-transact interception by returning ChangesKeyId Lookup Failure Handling
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates KeyMint security-level interception behavior so that a “key not found” path forwards to HAL while skipping post-interception handling.
Changes:
- Return
TransactionResult.ContinueAndSkipPostinstead ofTransactionResult.Continuewhen the key entry is missing and the call is forwarded to HAL.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -317,7 +317,7 @@ class KeyMintSecurityLevelInterceptor( | |||
| entry ?: run { | |||
| trackAndEnforceOpLimit(callingUid, txId)?.let { return it } | |||
| SystemLogger.info("[TX_ID: $txId] createOperation KeyId(${keyDescriptor.nspace}) NOT FOUND for uid=$callingUid. Forwarding to HAL.") | |||
When handleCreateOperation receives a Domain.KEY_ID request for a key not in our generatedKeys cache, it correctly forwards to the real HAL. However, it previously returned TransactionResult.Continue, which lets the post-handler run. The post-handler unconditionally registers an OperationInterceptor on the IKeystoreOperation binder returned by real keystore2. This interceptor then interferes with the caller's operation (intercepting finish/abort/updateAad calls).
On devices where vendor daemons (e.g. fingerprint calibration) use Domain.KEY_ID for their hardware-backed keys, this causes operation failures.
Fix: return ContinueAndSkipPost (matching the existing Domain.APP NOT FOUND path) so the post-handler never runs for operations on keys we don't own.
Symptom: OnePlus engineering mode ultrasonic fingerprint calibration hash retrieval fails with module enabled, works with module disabled.
Summary by CodeRabbit