diff --git a/controller/app/src/main/java/org/iiab/controller/IIABAdbManager.java b/controller/app/src/main/java/org/iiab/controller/IIABAdbManager.java index daf27eb..e28bfa0 100644 --- a/controller/app/src/main/java/org/iiab/controller/IIABAdbManager.java +++ b/controller/app/src/main/java/org/iiab/controller/IIABAdbManager.java @@ -51,14 +51,16 @@ private IIABAdbManager(Context context) { Log.i(TAG, "Generating new RSA V3 key pair in AndroidKeyStore..."); KeyPairGenerator kpg = KeyPairGenerator.getInstance(KeyProperties.KEY_ALGORITHM_RSA, "AndroidKeyStore"); + // S3: the ADB identity key is only ever used to sign/verify the + // connection handshake (no Cipher uses this alias), so scope it to + // SIGN | VERIFY. The previous spec also granted ENCRYPT | DECRYPT + // with non-randomized encryption padding, which is unused attack + // surface and a crypto anti-pattern. KeyGenParameterSpec spec = new KeyGenParameterSpec.Builder( KEY_ALIAS, - KeyProperties.PURPOSE_SIGN | KeyProperties.PURPOSE_VERIFY | - KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT) + KeyProperties.PURPOSE_SIGN | KeyProperties.PURPOSE_VERIFY) .setDigests(KeyProperties.DIGEST_NONE, KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA512) .setSignaturePaddings(KeyProperties.SIGNATURE_PADDING_RSA_PKCS1) - .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE, KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1) - .setRandomizedEncryptionRequired(false) .setKeySize(2048) .setCertificateSubject(new X500Principal("CN=" + this.deviceName)) .setCertificateSerialNumber(BigInteger.ONE) diff --git a/controller/app/src/main/java/org/iiab/controller/IIABWatchdog.java b/controller/app/src/main/java/org/iiab/controller/IIABWatchdog.java index 5023e1a..c2ecccf 100644 --- a/controller/app/src/main/java/org/iiab/controller/IIABWatchdog.java +++ b/controller/app/src/main/java/org/iiab/controller/IIABWatchdog.java @@ -225,6 +225,10 @@ private static void setRapidGrowthFlag(Context context, boolean enabled) { private static void broadcastLog(Context context, String message) { Intent intent = new Intent(ACTION_LOG_MESSAGE); intent.putExtra(EXTRA_MESSAGE, message); + // M4: scope to our own package. An implicit broadcast would leak the log + // text to any installed app, and broadcasting to dynamic receivers without + // a package is rejected on API 34+ (would crash). Matches the other senders. + intent.setPackage(context.getPackageName()); context.sendBroadcast(intent); } } diff --git a/controller/docs/TECH_DEBT_PLAN.md b/controller/docs/TECH_DEBT_PLAN.md index 28b82f7..241a30e 100644 --- a/controller/docs/TECH_DEBT_PLAN.md +++ b/controller/docs/TECH_DEBT_PLAN.md @@ -35,7 +35,12 @@ _Last updated: 2026-06-17. Tracks remediation work against the findings below. I - New pure domain rule `org.iiab.controller.sync.domain.SyncCredentialValidator` (strict user/host charsets, port range, control-char-free password, `isSafeConfigValue` for config lines). Unit-tested (`SyncCredentialValidatorTest`) plus three new malicious-payload cases in `SyncHandshakeHelperTest`. - Validation applied at the untrusted boundary (`SyncHandshakeHelper.parsePayload` -> `null` on invalid) and defensively in `RsyncManager` (server config + client URL paths), with a new `rsync_error_invalid_credentials` string (en + es). The validator is the reusable contract the remaining injection fixes (**S4**, **D2**) can build on. -**Phase 1 — Security hardening: IN PROGRESS.** **S1** done (above); remaining Phase 1 targets: **D2**, **D6**, **S3**, **M4**, **D12**, **S4**. +**M4 + S3 — Phase 1 security one-liners: DONE** (PR `fix/phase1-security-m4-s3`) +- **M4** (`IIABWatchdog.broadcastLog`): the `ACTION_LOG_MESSAGE` broadcast carrying log text was implicit (no `setPackage`), leaking it to any installed app and crashing on API 34+. Now scoped to our own package, matching every other broadcast in the app. +- **S3** (`IIABAdbManager` key spec): the ADB identity key was created with `ENCRYPT | DECRYPT` + non-randomized encryption padding on top of sign/verify. Verified no `Cipher` uses this alias, so the spec is now `SIGN | VERIFY` only (encryption paddings / `setRandomizedEncryptionRequired(false)` removed). Alias kept at `v3` (non-disruptive: hardens new key generation without forcing existing devices to re-pair ADB). +- No new unit tests: both are framework-bound legacy line-fixes with no extractable pure logic; verified by inspection + compile + CI lint. + +**Phase 1 — Security hardening: IN PROGRESS.** Done so far: **S1** (PR #9), **M4**, **S3**. Remaining: **D2**, **D6**, **D12**, **S4**, **D11**. ## 1. Executive summary