Address Copilot/CodeRabbit review feedback on the persistence PR.
1. SoftwareOperation: replace requireNotNull(keyPair) in SIGN/VERIFY/AGREE_KEY
branches with ServiceSpecificException(invalidArgument). The original
requireNotNull throws IllegalArgumentException, which the binder layer
wraps as KEYMINT_UNKNOWN_ERROR, defeating the goal of surfacing a
clean keystore-style error. Aligns with how ENCRYPT/DECRYPT already
handle missing key material in the same when block.
2. loadPersistedKeys: when a symmetric record has empty metadataBytes (e.g.
a save where Parcel.marshall() was empty for any reason), rebuild a
minimal KeyMetadata from PersistedKeyData primitive fields instead of
skipping the record. Skipping silently dropped the AES key, which is
the same 'logged out after reboot' behavior the PR is trying to fix.
The rebuilt metadata is structurally minimal but preserves the secret
material, which is the dominant correctness concern.
3. Comment fix: rebuildResponseFromRecord docs referred to 'v2 metadata
snapshot', the format in this PR is v3.
Five issues that together caused keystore-pinned apps to be silently
logged out across reboots and config changes. All flow from the same
root cause: GeneratedKeyPersistence loses information on save -> reload.
1. Symmetric keys (AES, HMAC, 3DES) were never persisted at all
- GeneratedKeyPersistence.save only accepted KeyPair, ignoring SecretKey
- AndroidX security MasterKey (AES-GCM-256) regenerated on every
reboot, making EncryptedSharedPreferences undecryptable
- Apps that wrap session tokens in EncryptedSharedPreferences
interpret this as session expiry and force a relogin
2. Restored KeyMetadata authorizations differed from generation-time bytes
- loadPersistedKeys rebuilt KeyMintAttestation with mostly null/empty
fields, so toAuthorizations emitted a different tag set after
reboot vs. at generateKey time
- Apps that fingerprint metadata across keystore calls saw a
"changed key"
3. certificate / certificateChain split could shift after restore
- buildKeyEntryResponse called updateCertificateChain on the rebuilt
metadata, which is allowed to repartition leaf vs. chain bytes
- Apps with strict leaf fingerprint checks saw a "changed cert"
4. Touching ANY .xml under /data/adb/tricky_store wiped every cached key
- ConfigObserver called clearAllGeneratedKeys() which also calls
GeneratedKeyPersistence.deleteAll()
- Editing keybox.xml (or any unrelated .xml) thus deleted every
persisted key on disk
- Even the keybox-cache argument does not justify wiping per-app keys:
patched chains alone are stale, raw keypairs are not
5. SoftwareOperation NPE when restored keyParams missed PURPOSE tag
- Init dereferenced keyPair!! before checking purpose, so a
half-restored record crashed instead of producing a clean error
Single on-disk format (FORMAT_VERSION = 3) covers everything: PKCS8
private key bytes for asymmetric, raw secret bytes for symmetric, plus
the byte-identical KeyMetadata parcel snapshot so authorizations
restore exactly. Earlier dev-only formats are silently skipped by the
loader; the next generateKey for those aliases re-creates them in v3.
ConfigObserver now calls invalidatePatchedChains() instead of
clearAllGeneratedKeys() on .xml edits - only the chain cache is
stale, not the underlying keypairs.
Tested on OnePlus 13 (Android 16, KSU 3.2.4):
- Apps survive force-stop + cold reboot without losing keystore state
- Apps survive keybox.xml edits / replacements (touch, sed, cp -mv)
- Tamper score still 4 (CONSISTENT) on Duck Detector
- KeyAttestation chain output unchanged
Duck Detector's 'TEE Simulator generate-mode fingerprint' probe scans the
generateKey reply parcel for a 16-byte marker where the securityLevel byte
is 0x00 (SOFTWARE). Real KeyMint HAL uses 0x64 (KEYSTORE=100) for
keystore-enforced metadata (creation time, user ID, etc.).
This single-line change aligns with real hardware behavior and defeats
the probe. Tested on OnePlus 13 (Android 16, KSU 3.2.4):
- Before: 'TEE Simulator generate-mode fingerprint: Matched' (score 50)
- After: 'No TEE Simulator generate-mode fingerprint observed' (score 4)
Reference: https://github.com/eltavine/Duck-Detector-Refactoring/commit/e368038
