diff --git a/app/src/main/java/app/closer/crypto/CoupleEncryptionManager.kt b/app/src/main/java/app/closer/crypto/CoupleEncryptionManager.kt index 88dc94e9..072de384 100644 --- a/app/src/main/java/app/closer/crypto/CoupleEncryptionManager.kt +++ b/app/src/main/java/app/closer/crypto/CoupleEncryptionManager.kt @@ -33,7 +33,8 @@ data class SetupResult( @Singleton class CoupleEncryptionManager @Inject constructor( private val keyStore: CoupleKeyStore, - private val keyManager: RecoveryKeyManager + private val keyManager: RecoveryKeyManager, + private val fieldEncryptor: FieldEncryptor ) { /** * Generates a new couple keyset + recovery phrase but does NOT store them yet — @@ -97,13 +98,13 @@ class CoupleEncryptionManager @Inject constructor( fun isUnlocked(coupleId: String): Boolean = keyStore.hasKeyset(coupleId) /** - * Re-wraps the locally-held keyset with a new phrase and returns the new WrappedKey - * so the caller can persist it to Firestore. The old phrase is NOT required. - * Returns failure if no local keyset exists for [coupleId]. + * Re-wraps the locally-held keyset with a new phrase and returns the new WrappedKey so the caller + * can persist it. The old phrase is NOT required. * - * ⚠️ The new phrase only lives on THIS device. The partner's locally-stored phrase becomes stale - * (the phrase is never on the server in plaintext, so it can't propagate) — see the warning on - * [CoupleRepository.changeRecoveryPhrase]. Currently unwired; don't expose without partner re-share. + * Low-level: this ONLY makes the wrap. It does not tell the partner, and their stored phrase does + * not update itself — publishing this wrap on its own is exactly the desync landmine. The safe + * caller is the phrase-change handshake ([preparePhraseChange] → ack → [rewrapForAckedPhrase]), + * which only re-wraps once every device has confirmed it can read the new phrase. */ suspend fun rewrapWithNewPhrase( coupleId: String, @@ -116,6 +117,111 @@ class CoupleEncryptionManager @Inject constructor( } } + // ── Recovery-phrase change (two-phase handshake) ───────────────────────────── + // + // The phrase is one per-couple secret that BOTH partners hold locally and the server never sees in + // plaintext. So a change can't just re-wrap: the partner's stored copy would go stale, their + // Settings → Security would reveal a phrase that no longer unwraps anything, and the "ask your + // partner" recovery path — the whole reason the partner holds it — would hand over a dud. + // + // Instead the new phrase travels the only channel that is both private and available: encrypted + // under the couple key the partner already holds. And the WRAP moves last: + // + // phase 1 publish encrypted phrase + phraseGeneration (old phrase still valid) + // phase 2 each device confirms it can read it (ack) (old phrase still valid) + // phase 3 re-wrap under the new phrase + phraseWrapGeneration + // + // Until phase 3 the old phrase unwraps everything, so an abandoned change is a no-op rather than a + // couple that can't recover. A device stores the new phrase only when the wrap is actually made + // from it — the stored phrase and the wrap never disagree. + + /** Everything phase 1 needs, before anything is persisted. */ + data class PreparedPhraseChange( + val newPhrase: String, + val encryptedPhrase: String, + val newGeneration: Long + ) + + /** + * Phase 1: mint a fresh phrase and seal it to the couple's own key. Persists nothing locally — + * this device keeps using the OLD phrase until the handshake completes, which is what lets a + * rotation land mid-handshake without wrapping the key under a phrase the partner lacks. + */ + suspend fun preparePhraseChange(coupleId: String, currentGeneration: Long): Result = + withContext(Dispatchers.Default) { + runCatching { + val aead = requireAead(coupleId) + val phrase = RecoveryKeyManager.normalizePhrase(keyManager.generateRecoveryPhrase()) + PreparedPhraseChange( + newPhrase = phrase, + encryptedPhrase = fieldEncryptor.encrypt(phrase, aead, coupleId), + newGeneration = currentGeneration + 1 + ) + } + } + + /** + * Read a published phrase. Any member's device can: it is sealed to the couple key, and the Tink + * keyring retains older keys, so a blob published before a rotation still opens afterwards. + */ + fun readPendingPhrase(couple: Couple): String? { + val blob = couple.encryptedPhraseSync ?: return null + val aead = aeadFor(couple.id) ?: return null + return fieldEncryptor.decrypt(blob, aead, couple.id) + } + + /** Phase 3 material: the CURRENT local keyset, re-wrapped under the acked phrase. */ + suspend fun rewrapForAckedPhrase(coupleId: String, phrase: String): Result = + rewrapWithNewPhrase(coupleId, phrase) + + /** Phase 3 local commit: the wrap is now made from this phrase, so store it as THE phrase. */ + fun commitPhraseChange(coupleId: String, phrase: String, generation: Long) { + keyStore.storeRecoveryPhrase(coupleId, phrase) + keyStore.storePhraseGeneration(coupleId, generation) + } + + sealed interface PhraseSyncResult { + /** Nothing pending, or this device is already in step. */ + data object UpToDate : PhraseSyncResult + /** A change is published and readable here; [generation] should be acked. */ + data class ReadyToAck(val generation: Long) : PhraseSyncResult + /** The wrap now uses the new phrase and this device stored it. */ + data object Adopted : PhraseSyncResult + /** Published but unreadable here (no couple key yet) — recovery flows handle this device. */ + data object Unreadable : PhraseSyncResult + data class Failed(val cause: Throwable) : PhraseSyncResult + } + + /** + * The partner/rotation-independent half of the handshake, run on every couple emission. + * + * Ordering matters and is asserted by tests: phrase BEFORE rotation. Rotation adoption unwraps the + * wrap with the locally-stored phrase, so if a completed phrase change is waiting, the phrase must + * be picked up first or the unwrap uses a retired phrase and fails. + */ + fun syncPhraseIfNeeded(couple: Couple, uid: String): PhraseSyncResult { + val coupleId = couple.id + if (!keyStore.hasKeyset(coupleId)) return PhraseSyncResult.UpToDate + if (couple.phraseGeneration <= 0L) return PhraseSyncResult.UpToDate + + val localGeneration = keyStore.loadPhraseGeneration(coupleId) + val wrapMoved = couple.phraseWrapGeneration > localGeneration + val needsAck = (couple.phraseAckedBy[uid] ?: 0L) < couple.phraseGeneration + + if (!wrapMoved && !needsAck) return PhraseSyncResult.UpToDate + + val phrase = readPendingPhrase(couple) ?: return PhraseSyncResult.Unreadable + + // The wrap already moved to this phrase — store it, the two must never disagree. + if (wrapMoved) { + return runCatching { + commitPhraseChange(coupleId, phrase, couple.phraseWrapGeneration) + PhraseSyncResult.Adopted + }.getOrElse { PhraseSyncResult.Failed(it) } + } + return PhraseSyncResult.ReadyToAck(couple.phraseGeneration) + } + fun deleteKeyset(coupleId: String) = keyStore.deleteKeyset(coupleId) // ── Couple-key rotation (phase 1: rotate-forward) ──────────────────────────── diff --git a/app/src/main/java/app/closer/crypto/CoupleKeyStore.kt b/app/src/main/java/app/closer/crypto/CoupleKeyStore.kt index a9185461..e2d91f45 100644 --- a/app/src/main/java/app/closer/crypto/CoupleKeyStore.kt +++ b/app/src/main/java/app/closer/crypto/CoupleKeyStore.kt @@ -67,6 +67,18 @@ class CoupleKeyStore @Inject constructor( prefs.edit().putLong(keyGenerationKey(coupleId), generation).apply() } + /** + * The phrase generation the LOCALLY STORED phrase corresponds to. Kept in lockstep with the wrap: + * the stored phrase must always be the one that actually unwraps `wrappedCoupleKey`, otherwise + * Settings → Security reveals a phrase that doesn't work and "ask your partner" hands over a dud. + */ + fun loadPhraseGeneration(coupleId: String): Long = + prefs.getLong(phraseGenerationKey(coupleId), 0L) + + fun storePhraseGeneration(coupleId: String, generation: Long) { + prefs.edit().putLong(phraseGenerationKey(coupleId), generation).apply() + } + fun loadKeyset(coupleId: String): KeysetHandle? = load(prefKey(coupleId)) @@ -92,6 +104,7 @@ class CoupleKeyStore @Inject constructor( .remove(prefKey(coupleId)) .remove(recoveryPhraseKey(coupleId)) .remove(keyGenerationKey(coupleId)) + .remove(phraseGenerationKey(coupleId)) .apply() aeadCache.remove(coupleId) } @@ -109,6 +122,7 @@ class CoupleKeyStore @Inject constructor( private fun invitePhrasePrefKey(inviteCode: String) = "phrase_invite_$inviteCode" private fun recoveryPhraseKey(coupleId: String) = "recovery_phrase_$coupleId" private fun keyGenerationKey(coupleId: String) = "key_generation_$coupleId" + private fun phraseGenerationKey(coupleId: String) = "phrase_generation_$coupleId" private fun serialize(handle: KeysetHandle): String { val baos = ByteArrayOutputStream() diff --git a/app/src/main/java/app/closer/data/remote/FirestoreCoupleDataSource.kt b/app/src/main/java/app/closer/data/remote/FirestoreCoupleDataSource.kt index a88b7bad..27d57361 100644 --- a/app/src/main/java/app/closer/data/remote/FirestoreCoupleDataSource.kt +++ b/app/src/main/java/app/closer/data/remote/FirestoreCoupleDataSource.kt @@ -60,22 +60,86 @@ class FirestoreCoupleDataSource @Inject constructor( coupleRef(coupleId).set(data).await() } - suspend fun updateWrappedKey(coupleId: String, wrappedKey: RecoveryKeyManager.WrappedKey) { - coupleRef(coupleId).set( - mapOf( - "wrappedCoupleKey" to wrappedKey.cipherB64, - "kdfSalt" to wrappedKey.saltB64, - "kdfParams" to wrappedKey.params - ), - SetOptions.merge() - ).await() - } /** * Rotation write: the new wrap AND the generation bump land atomically, so the partner can never * observe a bumped generation pointing at the old wrap (or vice versa). Rules enforce the * generation is strictly increasing. */ + // ── Recovery-phrase change handshake ──────────────────────────────────────── + // + // Three distinct writes, deliberately kept separate so each is a narrow, checkable shape in the + // rules (a single fat write would need one clause to mean three things): + // 1. publishPhraseSync — the changer publishes the new phrase, encrypted under the couple key + // 2. ackPhraseSync — each device confirms it can read that blob + // 3. completePhraseChange — the wrap is re-made under the new phrase, once everyone has acked + // + // The wrap is NOT touched until step 3, which is what keeps this safe: until every device has the + // new phrase, the OLD phrase still unwraps the key, so an interrupted change is a no-op rather + // than a couple that can't recover. + + /** Phase 1: publish the new phrase (ciphertext) + bump the generation. Leaves the wrap alone. */ + suspend fun publishPhraseSync(coupleId: String, encryptedPhrase: String, newGeneration: Long) { + coupleRef(coupleId).set( + mapOf( + "encryptedPhraseSync" to encryptedPhrase, + "phraseGeneration" to newGeneration + ), + SetOptions.merge() + ).await() + } + + /** Phase 2: this device confirms it can read the pending phrase. Writes only its own ack. */ + suspend fun ackPhraseSync(coupleId: String, uid: String, generation: Long) { + coupleRef(coupleId).set( + mapOf("phraseAckedBy" to mapOf(uid to generation)), + SetOptions.merge() + ).await() + } + + /** + * Phase 3: re-wrap under the new phrase, in a TRANSACTION. + * + * The transaction is load-bearing, not caution. This write replaces `wrappedCoupleKey` with a wrap + * of the keyset THIS device holds — so if a rotation lands between our read and our write, we would + * publish a wrap of the pre-rotation keyset and roll the rotation back, stranding the partner on + * content encrypted with a key the published wrap no longer contains. Re-reading `keyGeneration` + * inside the transaction and aborting when it moved makes that unrepresentable; the caller simply + * retries on the next couple emission with a current keyset. + * + * Also re-checks the handshake itself (generation unchanged, still unwrapped, still fully acked) so + * two devices completing at once converge instead of double-writing. + */ + suspend fun completePhraseChange( + coupleId: String, + wrappedKey: RecoveryKeyManager.WrappedKey, + phraseGeneration: Long, + expectedKeyGeneration: Long + ): Boolean = db.runTransaction { txn -> + val snap = txn.get(coupleRef(coupleId)) + val liveKeyGeneration = snap.getLong("keyGeneration") ?: 0L + val livePhraseGeneration = snap.getLong("phraseGeneration") ?: 0L + val liveWrapGeneration = snap.getLong("phraseWrapGeneration") ?: 0L + val stale = liveKeyGeneration != expectedKeyGeneration || + livePhraseGeneration != phraseGeneration || + liveWrapGeneration >= phraseGeneration + if (stale) { + false + } else { + txn.set( + coupleRef(coupleId), + mapOf( + "wrappedCoupleKey" to wrappedKey.cipherB64, + "kdfSalt" to wrappedKey.saltB64, + "kdfParams" to wrappedKey.params, + "phraseWrapGeneration" to phraseGeneration + ), + SetOptions.merge() + ) + true + } + }.await() + suspend fun rotateWrappedKey( coupleId: String, wrappedKey: RecoveryKeyManager.WrappedKey, @@ -180,9 +244,20 @@ class FirestoreCoupleDataSource @Inject constructor( wrappedCoupleKey = getString("wrappedCoupleKey"), kdfSalt = getString("kdfSalt"), kdfParams = getString("kdfParams"), - keyGeneration = getLong("keyGeneration") ?: 0L + keyGeneration = getLong("keyGeneration") ?: 0L, + encryptedPhraseSync = getString("encryptedPhraseSync"), + phraseGeneration = getLong("phraseGeneration") ?: 0L, + phraseAckedBy = phraseAcks(), + phraseWrapGeneration = getLong("phraseWrapGeneration") ?: 0L ) + /** `phraseAckedBy` as uid → generation, tolerating any junk shape (absent/garbage = no acks). */ + @Suppress("UNCHECKED_CAST") + private fun DocumentSnapshot.phraseAcks(): Map = + ((get("phraseAckedBy") as? Map) ?: emptyMap()) + .mapNotNull { (uid, v) -> (v as? Number)?.let { uid to it.toLong() } } + .toMap() + private fun DocumentSnapshot.millisOrNull(field: String): Long? = when (val raw = get(field)) { is Number -> raw.toLong() is Timestamp -> raw.toDate().time diff --git a/app/src/main/java/app/closer/data/repository/CoupleRepositoryImpl.kt b/app/src/main/java/app/closer/data/repository/CoupleRepositoryImpl.kt index d5a6d6ce..d7213ca6 100644 --- a/app/src/main/java/app/closer/data/repository/CoupleRepositoryImpl.kt +++ b/app/src/main/java/app/closer/data/repository/CoupleRepositoryImpl.kt @@ -67,13 +67,43 @@ class CoupleRepositoryImpl @Inject constructor( if (coupleId != null) encryptionManager.deleteKeyset(coupleId) } - // ⚠️ Unwired (no UI caller). Changing the phrase desyncs the PARTNER's locally-stored phrase — - // it's never on the server in plaintext, so the new phrase can't propagate to their device, and - // their "ask your partner" recovery copy would then be wrong. See CoupleRepository KDoc + SECURITY.md - // before exposing this. (TODO: if wired, also force the partner to re-save the new phrase.) - override suspend fun changeRecoveryPhrase(coupleId: String, newPhrase: String): Result = runCatching { - val newWrapped = encryptionManager.rewrapWithNewPhrase(coupleId, newPhrase).getOrElse { throw it } - coupleDataSource.updateWrappedKey(coupleId, newWrapped) + // ── Recovery-phrase change (three phases; see CoupleRepository + CoupleEncryptionManager) ── + + override suspend fun startPhraseChange(coupleId: String): Result = runCatching { + // Server-authoritative generation, for the same reason rotation needs one: a cached value + // makes us re-publish the number already on the doc, which is not a change, so no partner + // ever learns to adopt it (C-ROTATE-001). + val couple = coupleDataSource.getCoupleByIdFromServer(coupleId) ?: error("couple not found") + val prepared = encryptionManager + .preparePhraseChange(coupleId, couple.phraseGeneration) + .getOrElse { throw it } + coupleDataSource.publishPhraseSync(coupleId, prepared.encryptedPhrase, prepared.newGeneration) + // Nothing stored locally: this device keeps the OLD phrase until the wrap actually moves. + prepared.newPhrase + } + + override suspend fun ackPhraseSync(coupleId: String, uid: String, generation: Long): Result = + runCatching { coupleDataSource.ackPhraseSync(coupleId, uid, generation) } + + override suspend fun completePhraseChange(coupleId: String): Result = runCatching { + val couple = coupleDataSource.getCoupleByIdFromServer(coupleId) ?: error("couple not found") + if (!couple.phraseChangePending() || !couple.phraseAckedByAll()) return@runCatching false + + val phrase = encryptionManager.readPendingPhrase(couple) ?: return@runCatching false + val wrapped = encryptionManager.rewrapForAckedPhrase(coupleId, phrase).getOrElse { throw it } + + // The transaction re-checks keyGeneration: this wrap is of the keyset THIS device holds, so a + // rotation landing in between would make it a rollback of the rotation, stranding the partner. + val written = coupleDataSource.completePhraseChange( + coupleId = coupleId, + wrappedKey = wrapped, + phraseGeneration = couple.phraseGeneration, + expectedKeyGeneration = couple.keyGeneration + ) + // Only once the server accepted does the new phrase become THE phrase here — the stored phrase + // must always be the one that unwraps the published wrap. + if (written) encryptionManager.commitPhraseChange(coupleId, phrase, couple.phraseGeneration) + written } override suspend fun rotateCoupleKey(coupleId: String): Result = runCatching { diff --git a/app/src/main/java/app/closer/domain/model/Couple.kt b/app/src/main/java/app/closer/domain/model/Couple.kt index dcd5d482..9f05097e 100644 --- a/app/src/main/java/app/closer/domain/model/Couple.kt +++ b/app/src/main/java/app/closer/domain/model/Couple.kt @@ -17,5 +17,27 @@ data class Couple( val kdfSalt: String? = null, val kdfParams: String? = null, // Bumped by couple-key rotation; devices compare it against their local generation to adopt. 0 = never rotated. - val keyGeneration: Long = 0L -) + val keyGeneration: Long = 0L, + + // ── Recovery-phrase change handshake (see CoupleRepository.changeRecoveryPhrase) ── + // The phrase is a per-couple secret BOTH partners hold locally and is never on the server in + // plaintext — so changing it can't just re-wrap and hope: the partner's copy would go stale and + // their "ask your partner" recovery would hand over a wrong phrase. These three fields carry a + // handshake that keeps the stored phrase and the wrap in lockstep on both devices. + /** The new phrase, encrypted under the CURRENT couple keyset (AAD=coupleId). Server stays blind. */ + val encryptedPhraseSync: String? = null, + /** Monotonic. Bumped when a new phrase is PUBLISHED (phase 1). 0 = phrase never changed. */ + val phraseGeneration: Long = 0L, + /** uid → the phraseGeneration that device has confirmed it can read. Phase 3 waits for all members. */ + val phraseAckedBy: Map = emptyMap(), + /** The generation the CURRENT wrap actually corresponds to. Advanced at phase 3 (the re-wrap). */ + val phraseWrapGeneration: Long = 0L +) { + /** True while a published phrase change is still waiting on someone to confirm they can read it. */ + fun phraseChangePending(): Boolean = phraseGeneration > phraseWrapGeneration + + /** Every member has confirmed the pending phrase — the re-wrap (phase 3) may proceed. */ + fun phraseAckedByAll(): Boolean = + phraseGeneration > 0 && userIds.isNotEmpty() && + userIds.all { (phraseAckedBy[it] ?: 0L) >= phraseGeneration } +} diff --git a/app/src/main/java/app/closer/domain/repository/CoupleRepository.kt b/app/src/main/java/app/closer/domain/repository/CoupleRepository.kt index c987689a..ea1cbe21 100644 --- a/app/src/main/java/app/closer/domain/repository/CoupleRepository.kt +++ b/app/src/main/java/app/closer/domain/repository/CoupleRepository.kt @@ -15,18 +15,29 @@ interface CoupleRepository { suspend fun leaveCouple(userId: String): Result /** - * ⚠️ CURRENTLY UNWIRED — no UI calls this (verified 2026-06-29). Re-wraps the couple key under a - * NEW phrase and uploads a new `wrappedCoupleKey`, then re-saves the new phrase on THIS device only. + * Start a recovery-phrase change (phase 1 of 3): mint a fresh phrase, publish it encrypted under + * the couple's own key, and bump `phraseGeneration`. Returns the new phrase so the UI can show it + * for saving. * - * DO NOT wire this to a UI without first solving partner re-sharing. The recovery phrase is the same - * for both partners and is never on the server in plaintext, so there is no way to push the new phrase - * to the partner's device. After a change, the partner's locally-stored phrase (and their Settings → - * Security reveal) is STALE — it no longer matches `wrappedCoupleKey`, which silently breaks the - * primary "lost your phrase? ask your partner" recovery path. If you expose a change-phrase feature, - * you MUST also force the partner to re-save the new phrase (e.g. re-share + re-confirm on their - * device). See SECURITY.md (recovery) and the Engineering Manual landmine "recovery-phrase change desync". + * The wrap is deliberately NOT touched here. The phrase is one secret both partners hold locally + * and the server never sees in plaintext, so re-wrapping immediately would leave the partner's + * stored copy stale and their "ask your partner" recovery handing over a dud — the desync this + * handshake exists to prevent. The old phrase keeps working until every device has confirmed it + * can read the new one ([ackPhraseSync]) and the wrap is re-made ([completePhraseChange]); an + * abandoned change is therefore a no-op, not a broken couple. */ - suspend fun changeRecoveryPhrase(coupleId: String, newPhrase: String): Result + suspend fun startPhraseChange(coupleId: String): Result + + /** Phase 2: this device confirms it can read the pending phrase. Nothing is stored yet. */ + suspend fun ackPhraseSync(coupleId: String, uid: String, generation: Long): Result + + /** + * Phase 3: once every member has acked, re-wrap under the new phrase and store it locally. + * Transactional against a concurrent rotation (a stale re-wrap would roll the rotation back). + * Returns false when another device got there first or the state moved — both benign, retried + * on the next couple emission. + */ + suspend fun completePhraseChange(coupleId: String): Result /** * Rotate the couple key forward (phase 1): new key becomes primary, old keys stay readable, the diff --git a/app/src/main/java/app/closer/domain/usecase/CoupleKeyRotationAdopter.kt b/app/src/main/java/app/closer/domain/usecase/CoupleKeyRotationAdopter.kt index 8d73d790..513dd0ac 100644 --- a/app/src/main/java/app/closer/domain/usecase/CoupleKeyRotationAdopter.kt +++ b/app/src/main/java/app/closer/domain/usecase/CoupleKeyRotationAdopter.kt @@ -2,6 +2,7 @@ package app.closer.domain.usecase import app.closer.core.crash.CrashReporter import app.closer.crypto.CoupleEncryptionManager +import app.closer.domain.model.Couple import app.closer.domain.repository.CoupleRepository import kotlinx.coroutines.flow.catch import javax.inject.Inject @@ -41,7 +42,15 @@ class CoupleKeyRotationAdopter @Inject constructor( suspend fun watch(uid: String) { coupleRepository.observeCoupleForUser(uid) .catch { crashReporter.recordException(it) } - .collect { couple -> couple?.let { adoptForCouple(it) } } + .collect { couple -> + couple?.let { + // Phrase FIRST: rotation adoption unwraps the published wrap with the phrase stored + // on this device, so a completed phrase change has to be picked up before the + // unwrap — otherwise it runs with a retired phrase and fails. + advancePhraseChange(it, uid) + adoptForCouple(it) + } + } } /** Startup entry point: resolves the couple itself. Returns the outcome for callers that care. */ @@ -50,9 +59,36 @@ class CoupleKeyRotationAdopter @Inject constructor( .onFailure { crashReporter.recordException(it) } .getOrNull() ?: return CoupleEncryptionManager.AdoptionResult.UpToDate + advancePhraseChange(couple, uid) return adoptForCouple(couple) } + /** + * Drive this device's part of a recovery-phrase change: read + ack a published phrase, store it + * once the wrap has moved to it, and — when everyone has acked — perform the re-wrap. + * + * Every step is idempotent and best-effort: a device that can't read the phrase yet (no couple key) + * simply does nothing and the OLD phrase stays valid for everyone, which is the whole point of + * moving the wrap last. + */ + suspend fun advancePhraseChange(couple: Couple, uid: String) { + when (val result = encryptionManager.syncPhraseIfNeeded(couple, uid)) { + is CoupleEncryptionManager.PhraseSyncResult.ReadyToAck -> + coupleRepository.ackPhraseSync(couple.id, uid, result.generation) + .onFailure { crashReporter.recordException(it) } + is CoupleEncryptionManager.PhraseSyncResult.Failed -> + crashReporter.recordException(result.cause) + else -> Unit + } + // Whoever sees the last ack completes it — including the partner, so an offline changer can't + // leave the couple showing a phrase the wrap doesn't honour. The transaction inside makes the + // double-completion race a no-op for the loser. + if (couple.phraseChangePending() && couple.phraseAckedByAll()) { + coupleRepository.completePhraseChange(couple.id) + .onFailure { crashReporter.recordException(it) } + } + } + /** For callers that already hold the couple (Home) — no redundant read. */ suspend fun adoptForCouple(couple: app.closer.domain.model.Couple): CoupleEncryptionManager.AdoptionResult = when (val result = encryptionManager.adoptRotationIfNeeded(couple)) { diff --git a/app/src/main/java/app/closer/ui/settings/SecurityScreen.kt b/app/src/main/java/app/closer/ui/settings/SecurityScreen.kt index 0de5559d..0d1e14b0 100644 --- a/app/src/main/java/app/closer/ui/settings/SecurityScreen.kt +++ b/app/src/main/java/app/closer/ui/settings/SecurityScreen.kt @@ -81,7 +81,13 @@ data class SecurityUiState( val canRotateKey: Boolean = false, val isRotatingKey: Boolean = false, // One-shot outcome message for the rotation action; consumed by the UI after showing. - val rotateKeyMessage: String? = null + val rotateKeyMessage: String? = null, + /** Changing the phrase only needs the couple key on this device (the new phrase is sealed to it). */ + val canChangePhrase: Boolean = false, + val isChangingPhrase: Boolean = false, + /** The freshly minted phrase, shown once the handshake's first step is published. */ + val newPhrase: String? = null, + val changePhraseMessage: String? = null ) @HiltViewModel @@ -103,9 +109,15 @@ class SecurityViewModel @Inject constructor( // Whether the couple key itself is present on this device (set up), independent of the phrase. private val _coupleKeyPresent = MutableStateFlow(false) private val _rotation = MutableStateFlow(RotationState()) + private val _phraseChange = MutableStateFlow(PhraseChangeState()) private var coupleId: String? = null data class RotationState(val isRotating: Boolean = false, val message: String? = null) + data class PhraseChangeState( + val isChanging: Boolean = false, + val newPhrase: String? = null, + val message: String? = null + ) // Account type is fixed for the life of this screen: only an email/password account (signed in, not // Google, not anonymous) has a password to change. @@ -122,13 +134,16 @@ class SecurityViewModel @Inject constructor( } } + // combine() tops out at 5 flows; pair the two action states so the typed overload still applies. + private val _actions = combine(_rotation, _phraseChange) { rotation, phraseChange -> rotation to phraseChange } + val uiState: StateFlow = combine( settingsRepository.settings, _recoveryPhrase, _storedPhrase, _coupleKeyPresent, - _rotation - ) { settings, phrase, stored, keyPresent, rotation -> + _actions + ) { settings, phrase, stored, keyPresent, (rotation, phraseChange) -> SecurityUiState( biometricLoginEnabled = settings.biometricLoginEnabled, canChangePassword = canChangePassword, @@ -139,7 +154,13 @@ class SecurityViewModel @Inject constructor( // SAME phrase, which is exactly what lets the partner adopt it with no new ceremony. canRotateKey = keyPresent && stored != null, isRotatingKey = rotation.isRotating, - rotateKeyMessage = rotation.message + rotateKeyMessage = rotation.message, + // Only the couple key is needed: the new phrase travels sealed to it. A device without the + // phrase can still change it — the handshake gives everyone the new one. + canChangePhrase = keyPresent, + isChangingPhrase = phraseChange.isChanging, + newPhrase = phraseChange.newPhrase, + changePhraseMessage = phraseChange.message ) }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), SecurityUiState()) @@ -165,6 +186,44 @@ class SecurityViewModel @Inject constructor( _rotation.update { it.copy(message = null) } } + /** + * Phase 1 of the phrase change: publish a fresh phrase, sealed to the couple key, and show it. + * + * The old phrase keeps working until both devices confirm they can read the new one and the wrap + * is re-made (phases 2-3, driven by the couple-doc watcher). So the honest promise here is "saved + * on your phone, takes over once your partner's phone confirms" — not "changed". + */ + fun startPhraseChange() { + val id = coupleId ?: return + if (_phraseChange.value.isChanging) return + _phraseChange.value = PhraseChangeState(isChanging = true) + viewModelScope.launch { + coupleRepository.startPhraseChange(id) + .onSuccess { phrase -> _phraseChange.value = PhraseChangeState(newPhrase = phrase) } + .onFailure { e -> + crashReporter.recordException(e) + // Nothing was written, or only the (inert) publish was — the old phrase still works + // either way, so retrying is always safe. + _phraseChange.value = PhraseChangeState( + message = "Couldn't change the phrase — your current one still works. Please try again." + ) + } + } + } + + fun consumeNewPhrase() { + _phraseChange.update { it.copy(newPhrase = null) } + // The phrase the user now holds becomes the stored one once the handshake completes; re-read + // so Settings shows the truth rather than a stale reveal. + viewModelScope.launch { + coupleId?.let { _storedPhrase.value = encryptionManager.recoveryPhrase(it) } + } + } + + fun consumeChangePhraseMessage() { + _phraseChange.update { it.copy(message = null) } + } + fun setBiometricLogin(enabled: Boolean) { viewModelScope.launch { settingsRepository.setBiometricLogin(enabled) } } @@ -255,6 +314,66 @@ fun SecurityScreen( ) } + state.newPhrase?.let { phrase -> + AlertDialog( + onDismissRequest = { viewModel.consumeNewPhrase() }, + title = { Text("Your new recovery phrase", color = SettingsInk) }, + text = { + Column(verticalArrangement = Arrangement.spacedBy(12.dp)) { + Text( + phrase, + style = MaterialTheme.typography.titleMedium, + color = SettingsInk, + fontWeight = FontWeight.SemiBold + ) + Text( + "Write this somewhere safe. It's the only thing that can unlock your history if you " + + "both lose your phones.", + style = MaterialTheme.typography.bodySmall, + color = SettingsMuted + ) + Text( + "Your old phrase still works until your partner's phone has this one — that usually " + + "takes a moment. You can always see it again here in Security.", + style = MaterialTheme.typography.bodySmall, + color = SettingsMuted + ) + } + }, + confirmButton = { + TextButton(onClick = { viewModel.consumeNewPhrase() }) { Text("I've saved it") } + }, + containerColor = SettingsSoft + ) + } + + var showChangePhraseConfirm by remember { mutableStateOf(false) } + if (showChangePhraseConfirm) { + AlertDialog( + onDismissRequest = { showChangePhraseConfirm = false }, + title = { Text("Get a new recovery phrase?", color = SettingsInk) }, + text = { + Text( + "We'll give you a brand-new phrase and show it to you. Your partner's phone picks it up " + + "on its own the next time they open the app — you don't have to read it to them.\n\n" + + "Your current phrase keeps working until their phone has the new one, so nothing can " + + "get lost in between.", + style = MaterialTheme.typography.bodySmall, + color = SettingsMuted + ) + }, + confirmButton = { + TextButton(onClick = { showChangePhraseConfirm = false; viewModel.startPhraseChange() }) { + Text("Get a new phrase") + } + }, + dismissButton = { + TextButton(onClick = { showChangePhraseConfirm = false }) { Text("Cancel") } + }, + containerColor = SettingsSoft + ) + } + // One-shot rotation outcome, surfaced in this screen's own idiom (same as the Copy toast). state.rotateKeyMessage?.let { message -> LaunchedEffect(message) { @@ -421,6 +540,30 @@ fun SecurityScreen( } } + // Change recovery phrase — the two-phase handshake (publish sealed → partner acks → + // re-wrap). Only needs the couple key here: the new phrase travels sealed to it, so a + // device that lost its phrase can still get everyone onto a fresh one. + if (state.canChangePhrase) { + Divider(modifier = Modifier.padding(horizontal = 16.dp), thickness = 0.5.dp) + Row( + modifier = Modifier + .fillMaxWidth() + .clickable(enabled = !state.isChangingPhrase) { showChangePhraseConfirm = true } + .padding(horizontal = 16.dp, vertical = 14.dp) + .alpha(if (state.isChangingPhrase) 0.5f else 1f), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(12.dp) + ) { + Icon(CloserGlyphs.Lock, contentDescription = null, tint = SettingsMuted) + Text( + text = if (state.isChangingPhrase) "Getting a new phrase…" else "Change recovery phrase", + style = MaterialTheme.typography.bodyLarge, + color = SettingsInk, + modifier = Modifier.weight(1f) + ) + } + } + // Help-my-partner-restore entry — a manual way to reach the consent screen if the // "Help your partner restore" notification never arrived. Only offered when we hold the // couple key (you can't hand over a key you don't have). The screen shows "no request diff --git a/app/src/test/java/app/closer/crypto/CoupleKeyRotationTest.kt b/app/src/test/java/app/closer/crypto/CoupleKeyRotationTest.kt index 8d4c8f51..0d3b5745 100644 --- a/app/src/test/java/app/closer/crypto/CoupleKeyRotationTest.kt +++ b/app/src/test/java/app/closer/crypto/CoupleKeyRotationTest.kt @@ -41,7 +41,7 @@ class CoupleKeyRotationTest { private val keyManager = RecoveryKeyManager() private val keyStore: CoupleKeyStore = mockk(relaxUnitFun = true) - private val manager = CoupleEncryptionManager(keyStore, keyManager) + private val manager = CoupleEncryptionManager(keyStore, keyManager, FieldEncryptor()) private val phrase = "hunt down gear east lead over drop live more baby" private val coupleId = "c1" diff --git a/app/src/test/java/app/closer/domain/model/PhraseHandshakeStateTest.kt b/app/src/test/java/app/closer/domain/model/PhraseHandshakeStateTest.kt new file mode 100644 index 00000000..0ffdcecc --- /dev/null +++ b/app/src/test/java/app/closer/domain/model/PhraseHandshakeStateTest.kt @@ -0,0 +1,61 @@ +package app.closer.domain.model + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The two predicates that decide when a recovery-phrase change may finish. + * + * Getting these wrong is not a cosmetic bug: completing early re-wraps the couple key under a phrase + * the partner's device hasn't got, so their stored phrase silently stops unwrapping anything and the + * "ask your partner to read you the phrase" recovery path hands over a dud. The rules enforce this + * too — these keep the client from even trying. + */ +class PhraseHandshakeStateTest { + + private val couple = Couple(id = "c1", userIds = listOf("a", "b")) + + @Test + fun `no change published means nothing is pending`() { + assertFalse(couple.phraseChangePending()) + assertFalse(couple.phraseAckedByAll()) + } + + @Test + fun `a published change is pending until the wrap catches up`() { + val published = couple.copy(phraseGeneration = 1, phraseWrapGeneration = 0) + assertTrue(published.phraseChangePending()) + // Once the re-wrap lands, the generations agree and the handshake is over. + assertFalse(published.copy(phraseWrapGeneration = 1).phraseChangePending()) + } + + @Test + fun `every member must ack — one is not enough`() { + val onlyOne = couple.copy(phraseGeneration = 1, phraseAckedBy = mapOf("a" to 1L)) + assertFalse("one ack must not unlock the re-wrap", onlyOne.phraseAckedByAll()) + assertTrue(onlyOne.copy(phraseAckedBy = mapOf("a" to 1L, "b" to 1L)).phraseAckedByAll()) + } + + @Test + fun `a stale ack from an older generation does not count`() { + // Publishing doesn't clear old acks, so generation-1 acks must not satisfy generation 2 — + // otherwise the second change completes without anyone confirming they can read it. + val stale = couple.copy(phraseGeneration = 2, phraseAckedBy = mapOf("a" to 1L, "b" to 1L)) + assertFalse(stale.phraseAckedByAll()) + assertTrue(stale.copy(phraseAckedBy = mapOf("a" to 2L, "b" to 2L)).phraseAckedByAll()) + } + + @Test + fun `an ack from someone who isn't a member cannot stand in for a partner`() { + val impostor = couple.copy(phraseGeneration = 1, phraseAckedBy = mapOf("a" to 1L, "zz" to 1L)) + assertFalse(impostor.phraseAckedByAll()) + } + + @Test + fun `an unpaired couple never reports all-acked`() { + // Guards the vacuous-truth trap: all() over an empty member list is true. + val noMembers = Couple(id = "c1", userIds = emptyList(), phraseGeneration = 1) + assertFalse(noMembers.phraseAckedByAll()) + } +} diff --git a/app/src/test/java/app/closer/domain/usecase/CoupleKeyRotationAdopterTest.kt b/app/src/test/java/app/closer/domain/usecase/CoupleKeyRotationAdopterTest.kt index 0ad1acf7..931f0ad1 100644 --- a/app/src/test/java/app/closer/domain/usecase/CoupleKeyRotationAdopterTest.kt +++ b/app/src/test/java/app/closer/domain/usecase/CoupleKeyRotationAdopterTest.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals +import org.junit.Before import org.junit.Test /** @@ -32,6 +33,14 @@ class CoupleKeyRotationAdopterTest { private val adopter = CoupleKeyRotationAdopter(coupleRepository, encryptionManager, crashReporter) + @Before + fun noPhraseChangePending() { + // These tests are about rotation adoption. The watcher also drives the phrase handshake on + // every emission, so give it a quiet default; the handshake has its own tests. + every { encryptionManager.syncPhraseIfNeeded(any(), any()) } returns + CoupleEncryptionManager.PhraseSyncResult.UpToDate + } + private val gen1 = Couple(id = "c1", keyGeneration = 1L) private val gen2 = Couple(id = "c1", keyGeneration = 2L) diff --git a/firestore-tests/rules.test.ts b/firestore-tests/rules.test.ts index cc24ae71..ba56b0a8 100644 --- a/firestore-tests/rules.test.ts +++ b/firestore-tests/rules.test.ts @@ -424,18 +424,238 @@ describe("couples/{coupleId}", () => { await assertFails(deleteDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`))); }); + // ── Recovery-phrase change handshake ─────────────────────────────────────── + // + // The phrase is one per-couple secret both partners hold locally; the server never sees it in + // plaintext. The invariant these protect: THE WRAP AND EVERY DEVICE'S STORED PHRASE MUST NEVER + // DISAGREE. The client trusts `phraseWrapGeneration` to mean "the wrap now uses the new phrase — + // store it", so the rules must make that field unforgeable; otherwise one write makes the partner + // overwrite their working phrase with one that unwraps nothing, permanently and silently. + describe("recovery-phrase change handshake", () => { + const PHRASE_BLOB = "enc:v1:" + "AAAA"; + + /** A couple mid-handshake: phrase published at `gen`, acked by whoever `acks` says. */ + async function seedPending(gen: number, acks: Record, extra: object = {}) { + await testEnv.withSecurityRulesDisabled(async (ctx) => { + await setDoc(doc(ctx.firestore(), `couples/${COUPLE_ID}`), { + ...COUPLE_DOC, + encryptedPhraseSync: PHRASE_BLOB, + phraseGeneration: gen, + phraseAckedBy: acks, + ...extra, + }); + }); + } + + // ── phase 1: publish ── + test("a member can publish a new phrase — allowed", async () => { + await assertSucceeds(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + encryptedPhraseSync: PHRASE_BLOB, + phraseGeneration: 1, + })); + }); + + test("publishing plaintext instead of ciphertext — denied", async () => { + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + encryptedPhraseSync: "just the words in the clear", + phraseGeneration: 1, + })); + }); + + test("publishing without advancing the generation — denied", async () => { + await seedPending(3, {}); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + encryptedPhraseSync: "enc:v1:BBBB", + phraseGeneration: 3, + })); + }); + + test("an outsider cannot publish a phrase — denied", async () => { + await assertFails(updateDoc(doc(charlie().firestore(), `couples/${COUPLE_ID}`), { + encryptedPhraseSync: PHRASE_BLOB, + phraseGeneration: 1, + })); + }); + + // ── phase 2: ack ── + test("a member can ack the published generation — allowed", async () => { + await seedPending(1, {}); + await assertSucceeds(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + phraseAckedBy: { [UID_A]: 1 }, + })); + }); + + test("a member cannot forge the PARTNER's ack — denied", async () => { + // The attack this blocks: forging Bob's ack lets phase 3 fire before his device has the new + // phrase, leaving him with a phrase that unwraps nothing. + await seedPending(1, {}); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + phraseAckedBy: { [UID_B]: 1 }, + })); + }); + + test("a member cannot ack a generation that isn't published — denied", async () => { + await seedPending(1, {}); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + phraseAckedBy: { [UID_A]: 2 }, + })); + }); + + test("a member cannot delete the partner's ack — denied", async () => { + await seedPending(1, { [UID_A]: 1, [UID_B]: 1 }); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + phraseAckedBy: { [UID_A]: 1 }, + })); + }); + + // ── phase 3: complete ── + test("completing after BOTH acked — allowed", async () => { + // Also the regression for the defaulting bug: this couple has never rotated, so keyGeneration + // is ABSENT. Reading it without a default is an error and denied every first phrase change. + await seedPending(1, { [UID_A]: 1, [UID_B]: 1 }); + await assertSucceeds(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "wrapped-under-new-phrase", + kdfSalt: "s2", + kdfParams: "p2", + phraseWrapGeneration: 1, + })); + }); + + test("completing when only ONE partner acked — denied", async () => { + await seedPending(1, { [UID_A]: 1 }); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "wrapped-under-new-phrase", + kdfSalt: "s2", + kdfParams: "p2", + phraseWrapGeneration: 1, + })); + }); + + test("completing with NO acks — denied", async () => { + await seedPending(1, {}); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "wrapped-under-new-phrase", + kdfSalt: "s2", + kdfParams: "p2", + phraseWrapGeneration: 1, + })); + }); + + test("completing with a stale ack from a previous generation — denied", async () => { + // Phase 1 doesn't clear old acks, so a generation-1 ack must not satisfy generation 2. + await seedPending(2, { [UID_A]: 1, [UID_B]: 1 }); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "w", + kdfSalt: "s", + kdfParams: "p", + phraseWrapGeneration: 2, + })); + }); + + // ── the one-write weapon ── + test("moving phraseWrapGeneration ALONE, with no re-wrap — denied", async () => { + // THE critical hole: the partner's device treats phraseWrapGeneration as proof the wrap moved + // and overwrites its stored phrase. On its own it must be unforgeable. + await seedPending(1, { [UID_A]: 1, [UID_B]: 1 }); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + phraseWrapGeneration: 1, + })); + }); + + test("a rotation cannot smuggle phraseWrapGeneration along — denied", async () => { + // Forging the "everything is consistent" signal inside a legitimate rotation would stop the + // partner's client from ever noticing the pending change. + await seedPending(1, {}); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "rotated", + kdfSalt: "s", + kdfParams: "p", + keyGeneration: 1, + phraseWrapGeneration: 1, + })); + }); + + test("completing cannot also move the key — denied", async () => { + await seedPending(1, { [UID_A]: 1, [UID_B]: 1 }); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "w", + kdfSalt: "s", + kdfParams: "p", + phraseWrapGeneration: 1, + keyGeneration: 1, + })); + }); + + test("phase 1 and phase 3 cannot be combined into one write — denied", async () => { + await seedPending(1, { [UID_A]: 1, [UID_B]: 1 }); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + encryptedPhraseSync: "enc:v1:CCCC", + phraseGeneration: 2, + wrappedCoupleKey: "w", + kdfSalt: "s", + kdfParams: "p", + phraseWrapGeneration: 2, + })); + }); + + test("phraseWrapGeneration cannot be rolled back — denied", async () => { + await seedPending(5, { [UID_A]: 5, [UID_B]: 5 }, { phraseWrapGeneration: 5 }); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "w", + kdfSalt: "s", + kdfParams: "p", + phraseWrapGeneration: 3, + })); + }); + + test("an outsider cannot complete — denied", async () => { + await seedPending(1, { [UID_A]: 1, [UID_B]: 1 }); + await assertFails(updateDoc(doc(charlie().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "w", + kdfSalt: "s", + kdfParams: "p", + phraseWrapGeneration: 1, + })); + }); + }); + // The v0→v1→v2 client-side migration flow is gone: couples are created at // encryptionVersion 2 server-side, and the only key-material update a client may // make is the recovery-wrap re-key (wrappedCoupleKey/kdfSalt/kdfParams together). describe("recovery wrap re-key", () => { - test("a member can re-wrap the couple key — allowed", async () => { - await assertSucceeds(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + // C-ROTATE-001: a bare re-wrap USED to be allowed, and that was the hole. The wrap is what every + // device's stored phrase must match, so it may only change for a reason the partner's device can + // see: a key rotation (keyGeneration advances) or a completed phrase handshake. A silent re-wrap + // strands the partner on content they can't read, with a phrase that unwraps nothing. + test("a member cannot re-wrap the couple key with no reason — denied", async () => { + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { wrappedCoupleKey: "new-wrapped-key", kdfSalt: "new-salt", kdfParams: "argon2id-v2", })); }); + test("a member can re-wrap as part of a key rotation — allowed", async () => { + await assertSucceeds(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "rotated-wrapped-key", + kdfSalt: "new-salt", + kdfParams: "argon2id-v2", + keyGeneration: 1, + })); + }); + + test("a rotation cannot roll keyGeneration backwards — denied", async () => { + await testEnv.withSecurityRulesDisabled(async (ctx) => { + await setDoc(doc(ctx.firestore(), `couples/${COUPLE_ID}`), { ...COUPLE_DOC, keyGeneration: 5 }); + }); + await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { + wrappedCoupleKey: "old-wrapped-key", + kdfSalt: "s", + kdfParams: "p", + keyGeneration: 4, + })); + }); + test("a member cannot change encryptionVersion alongside the wrap — denied", async () => { await assertFails(updateDoc(doc(alice().firestore(), `couples/${COUPLE_ID}`), { wrappedCoupleKey: "new-wrapped-key", diff --git a/firestore.rules b/firestore.rules index 498aa5b0..733ba4b7 100644 --- a/firestore.rules +++ b/firestore.rules @@ -174,13 +174,63 @@ service cloud.firestore { .hasOnly(['answerKeyReleased', 'updatedAt']); } + // ── Recovery-phrase change handshake (3 narrow write shapes) ────────────────────── + // The phrase is a per-couple secret both partners hold locally; the server never sees it in + // plaintext. A change therefore travels as ciphertext under the couple's own key, and the WRAP is + // re-made only after every member confirms they can read it — until then the old phrase still + // works, so an abandoned change is a no-op instead of an unrecoverable couple. + + /** Phase 1: publish the encrypted phrase + bump the generation. The wrap is untouched here. */ + function isPublishingPhraseSync() { + return request.resource.data.diff(resource.data).affectedKeys().hasOnly( + ['encryptedPhraseSync', 'phraseGeneration'] + ) + && isCiphertext(request.resource.data.encryptedPhraseSync) + // A CHANGED blob must advance the generation — the same lesson as C-ROTATE-001: an unchanged + // value is absent from affectedKeys(), so without this a re-publish could go unnoticed. + && request.resource.data.phraseGeneration is int + && request.resource.data.phraseGeneration > resource.data.get('phraseGeneration', 0); + } + + /** Phase 2: a member acks ONLY their own uid, and only the generation currently published. */ + function isAckingPhraseSync() { + return request.resource.data.diff(resource.data).affectedKeys().hasOnly(['phraseAckedBy']) + && request.resource.data.phraseAckedBy[request.auth.uid] == resource.data.phraseGeneration + && request.resource.data.phraseAckedBy.diff( + resource.data.get('phraseAckedBy', {}) + ).affectedKeys().hasOnly([request.auth.uid]); + } + + /** Phase 3: the wrap catches up to a published, fully-acked phrase. Key generation must NOT move. */ + function isCompletingPhraseChange() { + // The re-wrap IS the completion. Without this, advancing phraseWrapGeneration on its own passes + // (once acks exist) and tells both devices to store a phrase the wrap was never made from — + // leaving the couple with a phrase that unwraps nothing. Caught by the rules tests. + return request.resource.data.diff(resource.data).affectedKeys().hasAny(['wrappedCoupleKey']) + && request.resource.data.phraseWrapGeneration is int + && request.resource.data.phraseWrapGeneration == resource.data.get('phraseGeneration', 0) + && request.resource.data.phraseWrapGeneration > resource.data.get('phraseWrapGeneration', 0) + // The key must NOT move here: this write re-wraps the SAME keyset under a new phrase. Both + // sides need .get() defaults — couples are created without keyGeneration, and reading a + // never-written field is an error (which would deny every first phrase change). + && request.resource.data.get('keyGeneration', 0) == resource.data.get('keyGeneration', 0) + // The wrap may only move once BOTH devices have confirmed they can read the new phrase. + // Without this the rules would take a client's word for it, and a client that completes + // early leaves the partner holding a phrase that unwraps nothing — the exact disaster this + // handshake exists to prevent. Acks live in resource.data (this write can't touch them). + && resource.data.get('phraseAckedBy', {}).get(resource.data.userIds[0], 0) + == resource.data.get('phraseGeneration', 0) + && resource.data.get('phraseAckedBy', {}).get(resource.data.userIds[1], 0) + == resource.data.get('phraseGeneration', 0); + } + function isUpdatingRecoveryWrap() { return request.resource.data.encryptionVersion >= 1 && request.resource.data.wrappedCoupleKey is string && request.resource.data.kdfSalt is string && request.resource.data.kdfParams is string && request.resource.data.diff(resource.data).affectedKeys().hasOnly([ - 'wrappedCoupleKey', 'kdfSalt', 'kdfParams', 'keyGeneration' + 'wrappedCoupleKey', 'kdfSalt', 'kdfParams', 'keyGeneration', 'phraseWrapGeneration' ]) // A CHANGED wrap is a key rotation, and a rotation MUST advance keyGeneration: that bump is // the only signal the partner's device has to adopt the new key. Publishing a new wrap while @@ -190,8 +240,16 @@ service cloud.firestore { // affectedKeys(), which is what let the stale write through before. && ( !request.resource.data.diff(resource.data).affectedKeys().hasAny(['wrappedCoupleKey']) + // Lawful reason #1: a key rotation — keyGeneration advances (C-ROTATE-001). A rotation + // must not smuggle phraseWrapGeneration along; that combination is caught by the + // phraseWrapGeneration guard below, which only accepts a genuine phase 3. || (request.resource.data.keyGeneration is int && request.resource.data.keyGeneration > resource.data.get('keyGeneration', 0)) + // Lawful reason #2: completing a recovery-phrase change — phraseWrapGeneration catches up + // to the published, fully-acked phraseGeneration. The wrap changes but the KEY doesn't, so + // there is no rotation to signal; the phrase handshake is the signal. Anything else that + // changes the wrap is still rejected, which is what keeps a silent re-wrap impossible. + || isCompletingPhraseChange() ) // Monotonic like encryptionVersion: never downgradeable, so a replayed or stale rotation // write can't roll the couple back to an older wrap. @@ -199,6 +257,14 @@ service cloud.firestore { !request.resource.data.diff(resource.data).affectedKeys().hasAny(['keyGeneration']) || (request.resource.data.keyGeneration is int && request.resource.data.keyGeneration > resource.data.get('keyGeneration', 0)) + ) + // phraseWrapGeneration is the signal partners' devices trust to mean "the wrap now uses the + // new phrase — store it". It must therefore ONLY move as part of a real phase 3: on its own + // it is a one-write weapon that makes the partner overwrite their working phrase with one + // that unwraps nothing. Being in the allowlist above is not permission to move it. + && ( + !request.resource.data.diff(resource.data).affectedKeys().hasAny(['phraseWrapGeneration']) + || isCompletingPhraseChange() ); } @@ -364,6 +430,8 @@ service cloud.firestore { && ( isUpdatingCoupleRhythm() || isUpdatingRecoveryWrap() + || isPublishingPhraseSync() + || isAckingPhraseSync() ); // Delete: server-only (admin SDK only). Admin SDK bypasses rules. diff --git a/functions/src/couples/onCoupleKeyRotated.test.ts b/functions/src/couples/onCoupleKeyRotated.test.ts index d41f2fed..0c48c045 100644 --- a/functions/src/couples/onCoupleKeyRotated.test.ts +++ b/functions/src/couples/onCoupleKeyRotated.test.ts @@ -1,4 +1,4 @@ -import { isKeyGenerationIncrease } from './onCoupleKeyRotated' +import { isKeyGenerationIncrease, isPhrasePublished } from './onCoupleKeyRotated' describe('isKeyGenerationIncrease', () => { it('fires on a genuine rotation (0 → 1, and every later bump)', () => { @@ -23,3 +23,23 @@ describe('isKeyGenerationIncrease', () => { expect(isKeyGenerationIncrease(null, 'high')).toBe(false) }) }) + +describe('isPhrasePublished', () => { + it('fires when a new recovery phrase is published (phase 1)', () => { + expect(isPhrasePublished(undefined, 1)).toBe(true) + expect(isPhrasePublished(0, 1)).toBe(true) + expect(isPhrasePublished(2, 3)).toBe(true) + }) + + it('ignores the acks and the completion — only the publish needs the partner in the app', () => { + // Phase 2 (acks) and phase 3 (the re-wrap) leave phraseGeneration alone, so they must not alert: + // the partner is already acting, and re-alerting them would be noise on a security channel. + expect(isPhrasePublished(1, 1)).toBe(false) + expect(isPhrasePublished(undefined, undefined)).toBe(false) + }) + + it('never fires on a downgrade or a redelivered stale event', () => { + expect(isPhrasePublished(5, 4)).toBe(false) + expect(isPhrasePublished(1, undefined)).toBe(false) + }) +}) diff --git a/functions/src/couples/onCoupleKeyRotated.ts b/functions/src/couples/onCoupleKeyRotated.ts index e68c06d0..1249c1af 100644 --- a/functions/src/couples/onCoupleKeyRotated.ts +++ b/functions/src/couples/onCoupleKeyRotated.ts @@ -21,25 +21,59 @@ export function isKeyGenerationIncrease(before: unknown, after: unknown): boolea return a > b } +/** + * Pure edge guard for a newly PUBLISHED recovery phrase (phase 1 of the change handshake). + * + * Deliberately keyed on `phraseGeneration`, not `phraseWrapGeneration`: the publish is the moment the + * partner needs to act on, and their ack is what lets the change finish at all. Same shape as + * isKeyGenerationIncrease so the two read alike. + */ +export function isPhrasePublished(before: unknown, after: unknown): boolean { + const b = typeof before === 'number' ? before : 0 + const a = typeof after === 'number' ? after : 0 + return a > b +} + export const onCoupleKeyRotated = onDocumentUpdated('couples/{coupleId}', async (event) => { const before = event.data?.before.data() const after = event.data?.after.data() - if (!isKeyGenerationIncrease(before?.keyGeneration, after?.keyGeneration)) return + + const rotated = isKeyGenerationIncrease(before?.keyGeneration, after?.keyGeneration) + const phrasePublished = isPhrasePublished(before?.phraseGeneration, after?.phraseGeneration) + if (!rotated && !phrasePublished) return const { coupleId } = event.params const db = admin.firestore() const userIds = (after?.userIds ?? []) as string[] if (userIds.length === 0) return - logger.log(`[onCoupleKeyRotated] couple=${coupleId} generation=${after?.keyGeneration}`) + // A rotation and a phrase publish are separate write shapes (the rules forbid combining them), so + // exactly one of these is true per event. + const alert = rotated + ? { + type: 'couple_key_rotated', + title: '🔑 Security update', + body: 'Your shared key was rotated. Open the app and everything continues as normal.', + } + : { + // Not merely informational: the change CANNOT complete until the partner's app opens and + // confirms it can read the new phrase. This push is what gets them there. And if they didn't + // expect a phrase change, this is how they find out — the was-this-you philosophy again. + type: 'recovery_phrase_changed', + title: '🔑 Security update', + body: 'Your recovery phrase was changed. Open the app to finish saving the new one.', + } + + logger.log( + `[onCoupleKeyRotated] couple=${coupleId} ` + + `${rotated ? `generation=${after?.keyGeneration}` : `phraseGeneration=${after?.phraseGeneration}`}` + ) // Per-member isolation: one failed send must not cost the other member their alert. const results = await Promise.allSettled( userIds.map((uid) => queueAndPush(db, uid, { - type: 'couple_key_rotated', - title: '🔑 Security update', - body: 'Your shared key was rotated. Open the app and everything continues as normal.', + ...alert, coupleId, bypassQuietHours: true, })