3 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
737a3e5d5d |
security(sms): encrypt scheduled payloads, report real send status (E3, E7)
Some checks failed
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
Closes the two Phase E items I had left open and documented as open.
E3 -- scheduled message bodies were plaintext on disk.
Pending schedules must outlive the process so SmsAlarmReceiver can
send them when the alarm fires and so they survive a reboot, so
recipient and body go to SharedPreferences. MODE_PRIVATE is the right
primitive -- the file is UID-scoped -- but the contents were in the
clear, readable by anything running as the same UID and swept into
cloud backup by default. Same asset class as the inbox
(THREAT_MODEL.md T-I4).
Adds SmsScheduleCrypto: AES-256-GCM, fresh IV per value, key generated
inside the platform AndroidKeyStore and non-exportable. An attacker
with the prefs file but not the keystore gets ciphertext.
Deliberately NOT androidx.security.EncryptedSharedPreferences: that is
a Gradle dependency, and this crate compiles its Java with bare javac
against android.jar (see build.rs), so using it would mean a Gradle
build or a vendored jar. AndroidKeyStore and javax.crypto are both in
android.jar and give the property that matters.
The key is deliberately NOT user-authentication-bound: an alarm fires
while the device may be locked and the receiver must decrypt with no
user present. This protects against another app and against an
extracted backup, which is the threat in scope -- not against someone
holding an unlocked handset.
Fails CLOSED. If the keystore is unavailable, encrypt returns null and
schedule_sms errors rather than writing plaintext. A row that cannot
be decrypted -- wrong key after a reinstall, tampering, or written by
an older build -- is treated exactly like a missing row and skipped;
sending a garbled body would be worse than not sending.
E7 -- "sent" was a guess.
Both the sentIntent and deliveryIntent arguments were null, so nothing
could report back. send_sms returning Ok meant "the JNI call
returned", not that the radio accepted the message and certainly not
that it arrived -- and the UI rendered that as a tick. A send rejected
for no service, no SIM or a throttled radio was indistinguishable from
a delivered one.
Adds send_sms_tracked, which attaches real PendingIntents and returns
a correlating token, plus SmsSentReceiver to collect the platform
result and SendOutcome/SendReport to express it: Sent (radio accepted)
is now a different value from Delivered (handset acknowledged), and
failures carry the RESULT_ERROR_* code.
Three details worth recording:
- multipart takes ArrayList<PendingIntent>, one entry per part, so
the intent is repeated part_count times. Passing null here, as
before, meant no status for exactly the messages most likely to
fail: the long ones.
- the request code is derived from (token, kind), or the two intents
collide and the delivery report overwrites the send report.
- the broadcast is package-scoped and the receiver registered
NOT_EXPORTED, so another app cannot forge a delivery report.
If the receiver class is unavailable the send still goes out with null
intents: losing the status report is much better than losing the
message.
Also fixes A5 on the scheduled path. SmsAlarmReceiver still called
sendTextMessage directly, so a scheduled message over 160 GSM-7
characters -- or 70 with any emoji -- was silently truncated by the
carrier. It now divides and sends multipart, as the Rust send path has
since Phase A.
CI: two gates, both negative-tested by reverting the fix and confirming
they fail. One asserts schedule_sms never writes request.recipient or
request.body directly; the other asserts the send path still passes
sent/delivery intents in both the single-part and multipart calls.
THREAT_MODEL.md T-I4 and the delivery-confirmation row move from open to
fixed, with the residual risk stated: callers may still use the
untracked send_sms, which remains honest about meaning only "handed to
the platform".
Tests: robius-sms 25 -> 28.
Verified: 13/13 CI checks, clippy -D warnings clean on host and
aarch64-linux-android, both new Java classes javac-compile and dex.
NOT verified on a device. The keystore round-trip, the broadcast
delivery and the token correlation all need an emulator or handset;
there is still no CI runner on this repo.
|
||
|
|
a38c41c00a |
security(sms): stop persisting message bodies, throttle sends (Phase E)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
nigig-map / test (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
E1 -- the inbox was written to disk in plaintext. offline_store wrote every SMS body to app_data_dir/offline_store/sms_messages.json as pretty-printed JSON. SMS is the transport for OTPs, banking codes and M-Pesa confirmations, so that file was the user's complete authentication history sitting in app-private storage -- readable by anything running as the same UID, and included in backups. This repository already knew the answer. THREAT_MODEL.md T-I2 records "Raw SMS persisted to PSV file" as fixed in Phase 0, with raw_message omitted from save_to_disk() so it "lives in memory only". The SMS app then re-introduced the same defect at larger scale: the entire inbox rather than just M-Pesa messages, and with no retention limit until D6. OfflineSmsMessage.body is now #[serde(skip)]. Dropping the field rather than encrypting it is the deliberate choice: every consumer already reads the device provider FIRST and writes the cache second (nigig-sms fetch_from_device, and the mpesa and pay transaction pages), so the provider is the system of record and no body needs to survive a restart. Encryption would keep the plaintext reachable to anything holding the key. Not writing it removes the asset. Two consequences handled: sms_key() no longer hashes the body, since a reloaded row has an empty one and dedupe would otherwise never match its own cached entry and grow a duplicate per refresh; and the cached first paint shows a neutral placeholder rather than a blank preview for the instant before the provider read lands. E9 -- the Linux backend's dependencies were pure cost. robius-sms declared polkit =0.17.0 and gio =0.17.0 for target_os = "linux". sys/linux.rs references neither: all twelve functions return Err(PermanentlyUnavailable). Those two crates dragged in glib and proc-macro-error and were the origin of RUSTSEC-2024-0370 and RUSTSEC-2024-0429 for every consumer of this crate. Deleting the block removes 340 lines from Cargo.lock. polkit, gio, glib and proc-macro-error no longer appear in the workspace at all, which also closes the LGPL-2.1 linkage question outright rather than routing around it as Phase B did for nigig-build alone. E4 -- ROBIUS_SMS_BOOT_LIB was a code-injection vector. build.rs interpolated that environment variable straight into a Java string literal, which is then compiled, dexed and loaded at runtime with the app's full permissions. A value containing a quote closes the literal and injects arbitrary Java that runs on the device at boot. Build-time environment is not trusted input. Now validated against [A-Za-z0-9_]+ and the build fails loudly otherwise. Tested both ways: an exec payload is rejected, a legitimate name builds. E5 -- undefined behaviour in the dex loader. new_direct_byte_buffer was handed RECEIVER_BYTECODE.as_ptr() as *mut u8 -- a &'static [u8] in .rodata cast to a mutable pointer, when the API is documented as taking writable memory and InMemoryDexClassLoader may write through it. Now copies into an owned allocation and leaks it, which is correct rather than lazy: the buffer backs a ClassLoader cached in a OnceLock for the process lifetime. E6 -- two bindings for one native method. rustRestoreSchedules was both exported #[no_mangle] and registered dynamically via register_native_methods. Which one won was unspecified. Kept the dynamic one, because the class is loaded from an in-memory dex and is not on the JVM's search path, so symbol binding is not guaranteed to find it. E8 -- no send rate limiting. Nothing capped send rate, and the bulk UI exists to blast a scraped directory. Android's practical throttle is ~30 messages per 30 minutes per app, past which sends are silently dropped -- so an unthrottled batch both overspends and fails opaquely. Adds SendRateLimiter, a pure token bucket taking an explicit clock so it is unit-testable without sleeping, wired into the bulk sender. A 200-recipient blast now stops at 30 and says why. E10 -- robius-sms carried no license field, so cargo-deny needed a [[licenses.clarify]] override asserting one. Stated in the manifest; override removed. E2 was already satisfied by D6 (retention capped at 5,000). E7/E11 are documented rather than fixed, which is the honest status: delivery confirmation needs real PendingIntents plumbed through (A8 documents that Ok != delivered), and sender validation cannot be solved client-side. Both are now rows in THREAT_MODEL.md instead of findings in a markdown report -- along with T-I2b for E1 and T-I4 for E3, which is NOT done: scheduled message bodies are still plaintext in SharedPreferences. CI: adds a gate asserting OfflineSmsMessage.body keeps #[serde(skip)]. Removing that attribute silently resumes writing plaintext and nothing else would fail. Negative-tested. deny-nigig-build.toml drops to 2 exemptions (from 5 before Phase B). Tests: robius-sms 21 -> 25. Verified: 12/12 CI jobs, clippy -D warnings clean on host and aarch64-linux-android, cargo deny "advisories ok, bans ok, licenses ok, sources ok", clippy ratchet holds at 49. |
||
| cc05abdc71 | Initial commit |