docs(crypto): key-storage migration design (design-only, gated)
Design for moving off the deprecated androidx.security:security-crypto to Tink Android-Keystore-backed storage, without ever losing the couple key. Covers: the load-bearing hazard (SecurePreferencesFactory.reset() silently WIPES the couple key on any read failure) which must be removed first; lazy dual-read + re-wrap + verify-then-clean migration; fail-closed (recover, never delete) failure matrix; consumer ordering (couple key last); staged RC rollout + content-free telemetry; test plan; open questions for the owner. No implementation ships until the owner approves this design (Batch 5.1 gate). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
null
parent
943507b72a
commit
a79db9474b
|
|
@ -0,0 +1,140 @@
|
|||
# Key-storage migration — design (DESIGN ONLY, not yet approved)
|
||||
|
||||
> **Status:** proposal for owner review. **No code ships from this doc** until the owner signs off —
|
||||
> it touches the E2EE root of trust, where a mistake means permanent, unrecoverable content loss.
|
||||
> Batch 5.1 of the roadmap (`make-a-plan-for-recursive-spindle.md`).
|
||||
|
||||
## Context / problem
|
||||
|
||||
All on-device secrets go through `data/local/SecurePreferencesFactory`, which uses
|
||||
**`androidx.security:security-crypto:1.0.0`** (`EncryptedSharedPreferences` + `MasterKeys`). That
|
||||
library is **deprecated and unmaintained by Google** (no fixes, no target-SDK updates). Six stores
|
||||
depend on it:
|
||||
|
||||
| Store | Holds | If lost… |
|
||||
|---|---|---|
|
||||
| `crypto/CoupleKeyStore` | **the couple AES-256 keyset** (Tink, JSON) + recovery phrase | **all E2EE content unreadable** — the crown jewel |
|
||||
| `crypto/UserKeyManager` | per-user ECIES P-256 private key | sealed-answer/keybox flows break until re-published |
|
||||
| `crypto/PendingAnswerKeyStore` | one-time per-answer keys, pre-reveal | a pending answer can't be revealed |
|
||||
| `data/local/RecoveryPhraseStore` | the recovery phrase | falls back to partner copy |
|
||||
| `data/local/PendingInviteStore` | inviter's code + phrase during pairing | pairing restart |
|
||||
| `data/repository/SharedPreferencesLocalAnswerRepository` | local answer drafts | drafts lost |
|
||||
|
||||
### The load-bearing hazard to fix along the way
|
||||
|
||||
`SecurePreferencesFactory.encryptedSharedPreferences()` catches any failure opening the store and
|
||||
calls **`reset()` — which deletes the file** — then recreates it empty. For draft/pending stores
|
||||
that's an acceptable "start over." **For `CoupleKeyStore` it is silent, permanent couple-key
|
||||
destruction**: a transient Keystore hiccup, an OS upgrade that invalidates the master key, or a
|
||||
botched migration would wipe the couple key with no prompt. The couple is partner-recoverable today
|
||||
(the partner holds the same key + phrase), so it's not *total* loss — but it turns a recoverable blip
|
||||
into a full re-pair/restore. **Any migration must remove this auto-wipe for the couple-key store** and
|
||||
replace it with a fail-closed path that preserves ciphertext and asks the user to recover, never
|
||||
deletes.
|
||||
|
||||
## Goals
|
||||
|
||||
1. Move off `androidx.security:security-crypto` to a **maintained** primitive.
|
||||
2. **Never lose the couple key** during or after migration — the overriding constraint.
|
||||
3. Zero user-visible disruption for the happy path (transparent, lazy migration).
|
||||
4. Keep the wire/at-rest format of *server-stored* data unchanged (this is purely on-device storage;
|
||||
Firestore `enc:v1:` ciphertext and the recovery-phrase-wrapped `wrappedCoupleKey` are untouched).
|
||||
|
||||
## Non-goals
|
||||
|
||||
- No change to the E2EE scheme itself (Tink AEAD, Argon2id recovery wrap, ECIES keyboxes) — see
|
||||
`SECURITY.md` / `docs/Engineering_Reference_Manual.md`.
|
||||
- No change to server storage or Firestore rules.
|
||||
- Not couple-key *rotation* / forward secrecy (separate roadmap item).
|
||||
|
||||
## Target design (recommended)
|
||||
|
||||
**Tink's Android-Keystore-backed keyset storage** (`AndroidKeysetManager` +
|
||||
`AndroidKeystoreKmsClient`), replacing `EncryptedSharedPreferences`:
|
||||
|
||||
- A single app master key in the AndroidKeyStore (`AndroidKeystoreKmsClient`, alias e.g.
|
||||
`closer_master_key`) wraps each stored Tink keyset; keysets persist in a plain `SharedPreferences`
|
||||
as Tink-encrypted blobs. This is Tink-native (we already depend on `tink-android`), maintained, and
|
||||
removes the `androidx.security` dependency.
|
||||
- Secrets that are raw strings (recovery phrase, invite phrase, local drafts) are wrapped with a
|
||||
dedicated Tink AEAD whose keyset is itself Keystore-master-wrapped (same mechanism), so nothing is
|
||||
stored in cleartext.
|
||||
- **Fail-closed**, not fail-wiped: if a keyset can't be decrypted, surface a recover-this-device flow
|
||||
(existing partner-assisted restore / recovery-phrase paths) — never delete.
|
||||
|
||||
**Alternative considered:** raw AndroidKeyStore AES-GCM + manual IV/blob management. Rejected — more
|
||||
bespoke crypto code to get wrong; Tink already gives us the KMS-client wrapper.
|
||||
|
||||
## Migration strategy
|
||||
|
||||
Transparent, lazy, per-store, one direction. For each key on read:
|
||||
|
||||
1. **Dual-read window.** New `SecureStoreV2` tries the new (Tink-Keystore) location first; on miss,
|
||||
falls back to reading the **old** `EncryptedSharedPreferences` value.
|
||||
2. **Re-wrap on read.** When a value is found only in the old store, decrypt it via the old lib and
|
||||
**write it to the new store**, then return it. (Lazy migration — no big-bang pass, no startup
|
||||
stall.) The old value is **left in place** until the migration is confirmed durable (see below).
|
||||
3. **Confirm, then clean.** Only after the new value has been successfully read back from the new
|
||||
store (a verify-read) is the old entry deleted. A crash between steps 2 and 3 is safe: next read
|
||||
re-does the re-wrap idempotently.
|
||||
4. **Never-lose-the-couple-key rule.** For `CoupleKeyStore` specifically: the old-store delete in
|
||||
step 3 is gated on a successful new-store round-trip AND is a no-op if the new value is absent.
|
||||
The `reset()`/auto-wipe path is removed for this store; a decrypt failure raises a typed
|
||||
`KeyUnavailable` that routes to recovery UI, not deletion.
|
||||
|
||||
### Failure matrix
|
||||
|
||||
| Situation | Behavior |
|
||||
|---|---|
|
||||
| New store has the value | Use it (fast path). |
|
||||
| Only old store has it | Re-wrap → new store → verify → (later) delete old. |
|
||||
| Neither has it, key expected | `KeyUnavailable` → recovery flow (partner/phrase). **No wipe.** |
|
||||
| New-store write fails mid-migration | Keep old value; return decrypted value from old; retry next read. |
|
||||
| Keystore master invalidated (OS upgrade / biometric change) | Detect, treat as `KeyUnavailable` → recovery; do NOT recreate empty. |
|
||||
|
||||
## Consumer ordering (lowest → highest risk)
|
||||
|
||||
1. `PendingInviteStore`, `SharedPreferencesLocalAnswerRepository`, `PendingAnswerKeyStore` —
|
||||
ephemeral/rebuildable; a wipe here is tolerable, so migrate first to shake out the mechanism.
|
||||
2. `UserKeyManager` (ECIES) — recoverable by re-publishing a fresh public key; medium risk.
|
||||
3. `RecoveryPhraseStore` — partner-recoverable; medium risk.
|
||||
4. **`CoupleKeyStore` last** — only after the mechanism is proven on the others, and only with the
|
||||
fail-closed (no-wipe) behavior in place.
|
||||
|
||||
## Staged rollout + telemetry
|
||||
|
||||
- Behind a Remote Config flag `key_storage_v2_enabled` (build the RC wrapper first — see
|
||||
`Future.md`, currently no wrapper exists). Default off; enable to a small % first.
|
||||
- **Content-free** telemetry only (respecting the analytics consent toggle): migration attempted /
|
||||
succeeded / fell-back / `KeyUnavailable` counts per store — never key material. Watch the
|
||||
`KeyUnavailable`-on-couple-key rate like a hawk; any nonzero blip is a rollback trigger.
|
||||
- Kill switch: flag off → `SecureStoreV2` reads new-then-old but stops *writing* new (freezes
|
||||
migration) without breaking either store.
|
||||
|
||||
## Test plan (before any staged rollout)
|
||||
|
||||
- Instrumented (real Keystore, can't be JVM-unit-tested): write via old lib → read via V2 →
|
||||
re-wrap → new-store round-trip → old entry cleaned only after verify.
|
||||
- **Fresh-device / process-death**: kill between re-wrap and cleanup; assert idempotent recovery, no
|
||||
loss.
|
||||
- **Keystore-invalidation simulation**: force a decrypt failure; assert `KeyUnavailable` + recovery
|
||||
route, assert the file is NOT deleted.
|
||||
- Full E2EE round-trip after migration (encrypt/decrypt a message, reveal a daily answer) on a
|
||||
migrated device — extends the existing QA "content is ciphertext at rest" pass.
|
||||
- Backward-compat: a device that never migrates (flag off) keeps working on the old store.
|
||||
|
||||
## Open questions for the owner
|
||||
|
||||
1. **Acceptable rollout %/duration** for the couple-key store, given partner-recoverability as the
|
||||
safety net?
|
||||
2. Keep the old `androidx.security` dependency for a full release cycle as the dual-read source, then
|
||||
remove — or remove sooner?
|
||||
3. Do we take the opportunity to **also** remove the `reset()` auto-wipe from the *other* stores now
|
||||
(independent, small, strictly-safer change), or bundle it all?
|
||||
|
||||
## Recommendation
|
||||
|
||||
Ship the **`reset()`-auto-wipe removal for `CoupleKeyStore` as a small, standalone, pre-migration
|
||||
fix** (it's strictly safer and independent of the library swap), then do the Tink-Keystore migration
|
||||
lazily behind the RC flag in the consumer order above, couple key last. Nothing here is implemented
|
||||
until this design is approved.
|
||||
Loading…
Reference in New Issue