fix(controller): restore ADB keystore capabilities broken by S3#21
Merged
Conversation
S3 (PR #10) re-scoped the ADB identity key to SIGN|VERIFY only, on the assumption that ENCRYPT|DECRYPT + the NONE/PKCS1 encryption paddings + setRandomizedEncryptionRequired(false) were unused attack surface. That assumption was wrong. The ADB connection runs over TLS; the libadb (MuntashirAkon:libadb-android) / conscrypt handshake signs with this key via a raw RSA operation (Cipher "RSA/ECB/NoPadding"), which REQUIRES PURPOSE_ENCRYPT plus ENCRYPTION_PADDING_NONE (and PKCS1) and setRandomizedEncryptionRequired(false). With the S3 key spec, a freshly generated key made the keystore reject the handshake op with KeyStoreException "INCOMPATIBLE_PADDING_MODE", so every ADB connection failed with an SSLHandshakeException (confirmed via logcat). - Restore PURPOSE_SIGN|VERIFY|ENCRYPT|DECRYPT + setEncryptionPaddings(NONE, PKCS1) + setRandomizedEncryptionRequired(false) on the key spec. - Bump KEY_ALIAS iiab_adb_key_v3 -> v4 so devices that already generated the broken SIGN|VERIFY-only v3 key regenerate a working one (one-time ADB re-pair). - Add a comment marking these capabilities as required so they are not stripped again, and withdraw S3 from the tech-debt register (it was a misdiagnosis).
…rt-s3 # Conflicts: # controller/docs/TECH_DEBT_PLAN.md
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
1 participant
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.
Summary
Reverts the keystore change from S3 (PR #10), which broke every ADB connection. Confirmed via logcat.
S3 re-scoped the ADB identity key to
SIGN | VERIFYonly, assumingENCRYPT|DECRYPT+ theNONE/PKCS1encryption paddings +setRandomizedEncryptionRequired(false)were unused attack surface. That assumption was wrong.Root cause (from logcat)
The ADB connection runs over TLS, and the libadb (
MuntashirAkon:libadb-android:3.1.1) / conscrypt handshake signs with this key via a raw RSA op (Cipher "RSA/ECB/NoPadding",CryptoUpcalls.rsaSignDigestWithPrivateKey). On a freshly generated S3 key the keystore rejects it:So those capabilities are required by the TLS handshake, not vestigial.
Changes (
IIABAdbManager)PURPOSE_SIGN | VERIFY | ENCRYPT | DECRYPT+setEncryptionPaddings(ENCRYPTION_PADDING_NONE, ENCRYPTION_PADDING_RSA_PKCS1)+setRandomizedEncryptionRequired(false).KEY_ALIASiiab_adb_key_v3→v4so devices that already generated the broken SIGN|VERIFY-only v3 key regenerate a working one. One-time cost: ADB must be re-paired on affected devices.TECH_DEBT_PLAN.md: withdraw S3 from the register — it was a misdiagnosis, not real debt.Notes
No unit test (AndroidKeyStore + TLS handshake, device-only). Validation is on-device: ADB connects again in Sync/Share. The rest of Phase 1 (S1/M4/D6/D2/D12/D11) is unaffected and stands.