The INCLUDE_UNIQUE_ID gate in handleGenerateKey (introduced as part of the
PR157 AOSP-compliance work) returns PERMISSION_DENIED when the caller
holds neither SELinux gen_unique_id nor REQUEST_UNIQUE_ID_ATTESTATION.
This breaks Google Wallet card binding on real devices: Wallet's
generateKey carries INCLUDE_UNIQUE_ID without holding the permission, so
its attestation key request is rejected and Wallet surfaces the failure
as "this phone does not meet the security requirements for Google
Wallet". Symptoms reported by users: clearing GMS data only helps for a
few seconds before the state regresses; no card can be added.
Naive removal of the gate is not safe: AttestationBuilder honours
`includeUniqueId == true` by computing an HMAC-SHA256 unique_id and
embedding it in the attestation extension. With the gate gone, GMS
attestation flows that include the tag end up with a unique_id in the
extension that Play Integrity flags as inconsistent for the caller,
turning all three integrity verdicts red.
The fix here splits the difference: when the permission check fails,
silently strip the INCLUDE_UNIQUE_ID tag from the KeyParameter array
(and re-parse `parsedParams`) instead of rejecting the request. The key
generates normally, AttestationBuilder takes the
`else { ByteArray(0) }` branch, and the resulting attestation simply
omits the unique_id field, matching pre-PR157 behaviour, where the
tag effectively had no effect.
Verified on a device that previously failed Wallet binding on the
PR157 baseline:
- Play Integrity: BASIC + DEVICE + STRONG all pass.
- Google Wallet: card binding completes successfully.
- Calls that DO hold the permission are unaffected (still emit
unique_id as before).
The rest of the PR157 compliance work (CALLER_NONCE handling,
AuthorizeCreate ordering, USAGE_COUNT_LIMIT counters, effectiveParams
merging) is preserved.
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
Andrea-lyz