Commit graph

3 commits

Author SHA1 Message Date
nigig-ci
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.
2026-08-02 07:58:44 +00:00
nigig-ci
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.
2026-08-01 04:58:46 +00:00
cc05abdc71 Initial commit 2026-07-26 19:38:26 +03:00