fix(controller): scope log broadcast (M4) + ADB key sign/verify-only (S3)#10
Merged
Merged
Conversation
…(S3) Two small Phase 1 security hardening fixes (no behaviour change for users). - M4 (IIABWatchdog.broadcastLog): the ACTION_LOG_MESSAGE broadcast carried log text as an implicit broadcast (no setPackage), leaking it to any app and crashing on API 34+ for dynamic receivers. Scope it to our own package, like every other broadcast in the app already does. - S3 (IIABAdbManager key spec): the ADB identity key was generated with ENCRYPT | DECRYPT and non-randomized encryption padding on top of SIGN | VERIFY. The key is only used to sign/verify the ADB handshake (no Cipher uses this alias), so restrict the spec to SIGN | VERIFY and drop the encryption paddings / setRandomizedEncryptionRequired(false). Alias kept at v3 so existing devices are not forced to re-pair (the change hardens newly generated keys). No new tests: both are framework-bound legacy line-fixes with no extractable pure logic; verified by inspection + compile + the CI lint/test gate.
luisguzman-adfa
force-pushed
the
fix/phase1-security-m4-s3
branch
from
3c079f4 to
969c78c
Compare
luisguzman-adfa
added a commit
to luisguzman-adfa/iiab-android
that referenced
this pull request
Jun 23, 2026
S3 (PR appdevforall#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).
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
Two small Phase 1 (security hardening) fixes, no behaviour change for users. Surgical line-fixes — no refactor needed.
Changes
IIABWatchdog.broadcastLog: theACTION_LOG_MESSAGEbroadcast carried log text as an implicit broadcast (nosetPackage), leaking it to any installed app and crashing on API 34+ for dynamic receivers. Now scoped to our own package — matching every other broadcast in the app (WatchdogService,IIABAdbManager), which already do this.IIABAdbManagerkey spec: the ADB identity key was generated withENCRYPT | DECRYPTand non-randomized encryption padding on top ofSIGN | VERIFY. Verified that noCipheruses this alias (the key only signs/verifies the ADB handshake), so the spec is nowSIGN | VERIFYonly (encryption paddings andsetRandomizedEncryptionRequired(false)removed). Alias kept atv3so existing devices are not forced to re-pair ADB — the change hardens newly generated keys.Why no tests / no refactor
Both are framework-bound legacy line-fixes (Android
KeyGenParameterSpec/Contextbroadcast) with no extractable pure logic, so there is nothing JVM-unit-testable to add. Verified by inspection + compile + the CI lint/test gate. A refactor would add risk for no benefit; the surrounding code is already small and clean.Notes
Independent of PR #9 (S1) — touches different files. Docs:
controller/docs/TECH_DEBT_PLAN.mdprogress log updated (M4, S3 done).