diff --git a/app/src/main/java/app/closer/crypto/SealedRevealManager.kt b/app/src/main/java/app/closer/crypto/SealedRevealManager.kt index e2e92bda..7eaf7b6e 100644 --- a/app/src/main/java/app/closer/crypto/SealedRevealManager.kt +++ b/app/src/main/java/app/closer/crypto/SealedRevealManager.kt @@ -1,6 +1,7 @@ package app.closer.crypto import app.closer.data.remote.FirestoreDeviceKeyDataSource +import com.google.crypto.tink.KeysetHandle import app.closer.data.remote.FirestoreReleaseKeyDataSource import javax.inject.Inject import javax.inject.Singleton @@ -24,7 +25,10 @@ class SealedRevealManager @Inject constructor( private val releaseKeyEncryptor: ReleaseKeyEncryptor, private val sealedAnswerEncryptor: SealedAnswerEncryptor, private val deviceKeyDataSource: FirestoreDeviceKeyDataSource, - private val releaseKeyDataSource: FirestoreReleaseKeyDataSource + private val releaseKeyDataSource: FirestoreReleaseKeyDataSource, + private val encryptionManager: CoupleEncryptionManager, + private val fieldEncryptor: FieldEncryptor, + private val crashReporter: app.closer.core.crash.CrashReporter ) { /** @@ -90,11 +94,13 @@ class SealedRevealManager @Inject constructor( recipientUserId = userId ) ?: return null - // loadPrivateKey — NOT getOrCreatePrivateKey. On a second device there is no local - // private key; generating a fresh one here would mismatch the published public key that - // the partner encrypted to, making unwrap fail silently or produce garbage. Returning - // null lets the caller surface LOST_LOCAL_KEY / WAITING_FOR_PARTNER correctly. - val myPrivateKey = userKeyManager.loadPrivateKey() ?: return null + // loadPrivateKey — NOT getOrCreatePrivateKey. Generating a fresh keypair here would mismatch + // the published public key the partner sealed to, turning a recoverable state into garbage. + // On a device that never had the key (a reinstall / second phone), try the couple-key escrow + // first: that is exactly the case this used to fail silently on. + val myPrivateKey = userKeyManager.loadPrivateKey() + ?: restorePrivateKeyFromEscrow(userId, coupleId) + ?: return null val oneTimeKey = releaseKeyEncryptor.unwrapFromSender( keyboxB64 = keybox, recipientPrivateKey = myPrivateKey, @@ -212,4 +218,37 @@ class SealedRevealManager @Inject constructor( val publicKeyB64 = userKeyManager.publicKeyB64(privateKey) deviceKeyDataSource.publishPublicKey(userId, publicKeyB64) } + + /** + * Escrow this device's ECIES private key under the couple key, once, so a future device can pick + * up where this one left off instead of silently losing every sealed reveal. + * + * Heal-forward, in the shape of `migrateProfileFields`: idempotent, best-effort, and it must never + * block or break the caller (Home). A device that can't publish today publishes tomorrow; existing + * users heal on their next Home load. + */ + suspend fun ensurePrivateKeyEscrowed(userId: String, coupleId: String) { + runCatching { + val aead = encryptionManager.aeadFor(coupleId) ?: return + if (deviceKeyDataSource.getPrivateKeyEscrow(userId) != null) return + val keysetJson = userKeyManager.exportPrivateKey() ?: return + // AAD = uid: binds the escrow to its owner, so a blob can't be replayed onto another user. + val encrypted = fieldEncryptor.encrypt(keysetJson, aead, userId) + deviceKeyDataSource.publishPrivateKeyEscrow(userId, encrypted) + }.onFailure { crashReporter.recordException(it) } + } + + /** + * Adopt the escrowed keypair on a device that has the couple key but no ECIES key of its own. + * Returns null when there's no escrow or it can't be read — the caller then reports the honest + * LOST_LOCAL_KEY rather than pretending we're still waiting for the partner. + */ + private suspend fun restorePrivateKeyFromEscrow(userId: String, coupleId: String): KeysetHandle? = + runCatching { + val blob = deviceKeyDataSource.getPrivateKeyEscrow(userId) ?: return null + val aead = encryptionManager.aeadFor(coupleId) ?: return null + val keysetJson = fieldEncryptor.decrypt(blob, aead, userId) ?: return null + if (!userKeyManager.importPrivateKey(keysetJson)) return null + userKeyManager.loadPrivateKey() + }.onFailure { crashReporter.recordException(it) }.getOrNull() } diff --git a/app/src/main/java/app/closer/crypto/UserKeyManager.kt b/app/src/main/java/app/closer/crypto/UserKeyManager.kt index afe4b8d5..64ea0c28 100644 --- a/app/src/main/java/app/closer/crypto/UserKeyManager.kt +++ b/app/src/main/java/app/closer/crypto/UserKeyManager.kt @@ -23,21 +23,32 @@ import javax.inject.Singleton * [UserKeySetupManager]. Only the public keyset JSON is base64-encoded — no secret * material ever leaves the device. * - * KNOWN LIMITATION — Single-device only: - * One keypair per user, stored only on the device that created it. If a user signs - * in on a second device, that device generates a NEW keypair and overwrites - * users/{uid}/devices/primary with the new public key. Any sealed answers whose - * one-time keys were wrapped for the OLD public key become permanently undecryptable - * on both devices (partner encrypted to stale key; private key is on original device). + * SECOND DEVICE — healed by escrow (was: "single-device only"): + * The keypair is generated per device and only the PUBLIC half is published, so a second device + * used to be stuck: it had no private key, and every sealed reveal died there. * - * Symptoms: "This answer cannot be revealed from this device" on the new device, - * and the original device can no longer complete the reveal (its pending key was - * removed after key release, but the partner encrypted to the new key). + * The old note here claimed a second sign-in "overwrites users/{uid}/devices/primary" — it does + * not. Every publish site is existence-gated ([SealedRevealManager.ensurePublicKeyPublished] and + * the two datasource helpers all no-op when a key is already published), so device 1 keeps working. + * The real failure was narrower and quieter: device 2 had no private key, so + * `decryptPartnerAnswer` returned null and the UI reported WAITING_FOR_PARTNER — a lie, and one + * the user could never report accurately. * - * Fix path (not implemented): multi-device key distribution, e.g. key agreement - * via iCloud/Google Drive backup or a per-device public key stored under - * users/{uid}/devices/{deviceId} so partners encrypt to ALL of the user's known - * public keys simultaneously. + * The fix is escrow, not multi-device fan-out: [exportPrivateKey] is stored encrypted under the + * COUPLE key at users/{uid}/devices/primary/secure/escrow, and a device that holds the couple key + * (any device that has recovered, which is a precondition for reading anything at all) imports it + * and becomes crypto-identical to the original. Fan-out — sealing every release key to N public + * keys, plus device lifecycle and pruning — buys the same outcome for an order of magnitude more + * protocol, and this product has no multi-device story to justify it. + * + * Why escrow leaks nothing: the only party besides the owner who can hold the couple key is the + * partner, and everything this keypair protects (release keys for the partner's OWN answers, + * restore keyboxes of the SHARED couple keyset) is material the partner already has. The escrow + * sits in an owner-only `secure/` subdoc regardless, and the server stays blind either way. + * + * Residual, documented not hidden: a user whose only device dies BEFORE ever publishing an escrow + * keeps the old behaviour — their pre-escrow sealed history stays locked. Nothing can fix that + * retroactively; the key never left that phone. */ @Singleton class UserKeyManager @Inject constructor( @@ -67,6 +78,25 @@ class UserKeyManager @Inject constructor( */ fun publicKeyB64(privateKey: KeysetHandle): String = publicKeyB64Companion(privateKey) + /** + * The private keyset, serialised, for escrow under the couple key. Never log or transmit this in + * the clear — the only legitimate caller encrypts it immediately. + */ + fun exportPrivateKey(): String? = runCatching { + prefs.getString(PRIVATE_KEY_PREF, null) + }.getOrNull() + + /** + * Adopt an escrowed keypair. Replaces the local key only on a successful parse (atomic, like + * every other key store here) — a corrupt escrow must leave this device exactly as it was rather + * than destroying a key that might still work. + */ + fun importPrivateKey(keysetJson: String): Boolean = runCatching { + val handle = CleartextKeysetHandle.read(JsonKeysetReader.withString(keysetJson)) + savePrivateKey(handle) + true + }.getOrDefault(false) + private fun savePrivateKey(handle: KeysetHandle) { val baos = ByteArrayOutputStream() CleartextKeysetHandle.write(handle, JsonKeysetWriter.withOutputStream(baos)) diff --git a/app/src/main/java/app/closer/data/remote/FirestoreDeviceKeyDataSource.kt b/app/src/main/java/app/closer/data/remote/FirestoreDeviceKeyDataSource.kt index 3ac6b858..49b2f819 100644 --- a/app/src/main/java/app/closer/data/remote/FirestoreDeviceKeyDataSource.kt +++ b/app/src/main/java/app/closer/data/remote/FirestoreDeviceKeyDataSource.kt @@ -36,6 +36,33 @@ class FirestoreDeviceKeyDataSource @Inject constructor( suspend fun getPublicKey(userId: String): String? = deviceRef(userId).get().await().getString("publicKey") + /** + * The owner's ECIES private keyset, encrypted under the COUPLE key, in an owner-only subdoc. + * + * It lives in `secure/` rather than as a field on `devices/primary` because that document is + * partner-readable (they need the public key to seal to). Rules can't gate individual fields, so + * the escrow gets its own document with an owner-only read. Belt and braces: even if the partner + * could read it, they hold the couple key it's encrypted under — and everything it protects is + * already theirs. The server can read neither. + */ + suspend fun publishPrivateKeyEscrow(userId: String, encryptedKeyset: String) { + escrowRef(userId) + .set( + mapOf( + "encryptedPrivateKey" to encryptedKeyset, + "updatedAt" to System.currentTimeMillis() + ), + SetOptions.merge() + ) + .await() + } + + suspend fun getPrivateKeyEscrow(userId: String): String? = + escrowRef(userId).get().await().getString("encryptedPrivateKey") + + private fun escrowRef(userId: String) = + deviceRef(userId).collection("secure").document("escrow") + private fun deviceRef(userId: String) = db.collection(FirestoreCollections.USERS) .document(userId) diff --git a/app/src/main/java/app/closer/ui/home/HomeViewModel.kt b/app/src/main/java/app/closer/ui/home/HomeViewModel.kt index 463a3d2d..a5c15f12 100644 --- a/app/src/main/java/app/closer/ui/home/HomeViewModel.kt +++ b/app/src/main/java/app/closer/ui/home/HomeViewModel.kt @@ -151,6 +151,12 @@ class HomeViewModel @Inject constructor( val uid = authRepository.currentUserId uid?.let { launch { runCatching { sealedRevealManager.ensurePublicKeyPublished(it) } } } val couple = uid?.let { runCatching { coupleRepository.getCoupleForUser(it) }.getOrNull() } + // Escrow this device's sealed-reveal key under the couple key, once. Heal-forward for + // existing users; needs the couple (the key it's encrypted under). Best-effort and + // off the critical path — Home must never fail because of it. + if (uid != null && couple != null) { + launch { sealedRevealManager.ensurePrivateKeyEscrowed(uid, couple.id) } + } val partnerUser = couple?.userIds?.firstOrNull { it != uid }?.let { partnerId -> runCatching { userRepository.getUser(partnerId) } .onFailure { Log.w(TAG, "Could not load partner profile", it) } diff --git a/firestore-tests/rules.test.ts b/firestore-tests/rules.test.ts index ba56b0a8..077442c0 100644 --- a/firestore-tests/rules.test.ts +++ b/firestore-tests/rules.test.ts @@ -185,6 +185,107 @@ describe("users/{uid}/entitlements/{doc}", () => { }); }); +// ── users/{uid}/devices/{deviceId} + the private-key escrow ───────────────── + +describe("users/{uid}/devices/{deviceId}", () => { + // The parent doc is deliberately PARTNER-READABLE: they need the public key to seal answers to. + // The escrow subdoc must NOT be — rules can't gate a single field, which is exactly why the + // escrowed private key lives in its own document. + const ESCROW = "enc:v1:YWJjZGVm"; + + beforeEach(async () => { + await seedCouple(); + await seedUser(UID_A, COUPLE_ID); + await seedUser(UID_B, COUPLE_ID); + await seedUser(UID_C); + }); + + test("owner can publish their public key — allowed", async () => { + await assertSucceeds(setDoc(doc(alice().firestore(), `users/${UID_A}/devices/primary`), { + deviceId: "primary", + publicKey: "pub:v1:AAAA", + platform: "android", + updatedAt: 1, + })); + }); + + test("a paired partner can READ the public key (they seal to it) — allowed", async () => { + await testEnv.withSecurityRulesDisabled(async (ctx) => { + await setDoc(doc(ctx.firestore(), `users/${UID_A}/devices/primary`), { + deviceId: "primary", publicKey: "pub:v1:AAAA", platform: "android", updatedAt: 1, + }); + }); + await assertSucceeds(getDoc(doc(bob().firestore(), `users/${UID_A}/devices/primary`))); + }); + + test("owner can read their own escrow — allowed", async () => { + await testEnv.withSecurityRulesDisabled(async (ctx) => { + await setDoc(doc(ctx.firestore(), `users/${UID_A}/devices/primary/secure/escrow`), { + encryptedPrivateKey: ESCROW, updatedAt: 1, + }); + }); + await assertSucceeds(getDoc(doc(alice().firestore(), `users/${UID_A}/devices/primary/secure/escrow`))); + }); + + test("the PARTNER cannot read the escrow — denied", async () => { + // The whole reason the escrow is a subdoc: partner-readable parent, owner-only secret. + await testEnv.withSecurityRulesDisabled(async (ctx) => { + await setDoc(doc(ctx.firestore(), `users/${UID_A}/devices/primary/secure/escrow`), { + encryptedPrivateKey: ESCROW, updatedAt: 1, + }); + }); + await assertFails(getDoc(doc(bob().firestore(), `users/${UID_A}/devices/primary/secure/escrow`))); + }); + + test("an outsider cannot read the escrow — denied", async () => { + await testEnv.withSecurityRulesDisabled(async (ctx) => { + await setDoc(doc(ctx.firestore(), `users/${UID_A}/devices/primary/secure/escrow`), { + encryptedPrivateKey: ESCROW, updatedAt: 1, + }); + }); + await assertFails(getDoc(doc(charlie().firestore(), `users/${UID_A}/devices/primary/secure/escrow`))); + }); + + test("owner can write a ciphertext escrow — allowed", async () => { + await assertSucceeds(setDoc(doc(alice().firestore(), `users/${UID_A}/devices/primary/secure/escrow`), { + encryptedPrivateKey: ESCROW, + updatedAt: 1, + })); + }); + + test("owner cannot escrow a PLAINTEXT private key — denied", async () => { + // The one thing that must never reach the server in the clear. + await assertFails(setDoc(doc(alice().firestore(), `users/${UID_A}/devices/primary/secure/escrow`), { + encryptedPrivateKey: '{"primaryKeyId":123,"key":[...]}', + updatedAt: 1, + })); + }); + + test("the partner cannot write the owner's escrow — denied", async () => { + await assertFails(setDoc(doc(bob().firestore(), `users/${UID_A}/devices/primary/secure/escrow`), { + encryptedPrivateKey: ESCROW, + updatedAt: 1, + })); + }); + + test("junk keys on the escrow — denied", async () => { + await assertFails(setDoc(doc(alice().firestore(), `users/${UID_A}/devices/primary/secure/escrow`), { + encryptedPrivateKey: ESCROW, + updatedAt: 1, + hasPremium: true, + })); + }); + + test("nobody can delete an escrow — denied", async () => { + await testEnv.withSecurityRulesDisabled(async (ctx) => { + await setDoc(doc(ctx.firestore(), `users/${UID_A}/devices/primary/secure/escrow`), { + encryptedPrivateKey: ESCROW, updatedAt: 1, + }); + }); + await assertFails(deleteDoc(doc(alice().firestore(), `users/${UID_A}/devices/primary/secure/escrow`))); + }); +}); + // ── users/{uid}/notification_queue/{id} ────────────────────────────────────── describe("users/{uid}/notification_queue/{id}", () => { diff --git a/firestore.rules b/firestore.rules index 733ba4b7..39aa976f 100644 --- a/firestore.rules +++ b/firestore.rules @@ -363,6 +363,18 @@ service cloud.firestore { && request.resource.data.publicKey.matches('^pub:v1:.*') && request.resource.data.keys().hasOnly(['deviceId', 'publicKey', 'platform', 'updatedAt']); allow delete: if false; + + // The owner's ECIES private keyset, escrowed encrypted under the COUPLE key so a later device + // can pick up sealed reveals instead of losing them forever. OWNER-ONLY, unlike the parent + // doc (which the partner must read to seal to the public key) — rules can't gate a single + // field, so the escrow gets its own document. Ciphertext-only, and the server can't read it. + match /secure/{secureDoc} { + allow read: if isOwner(uid); + allow create, update: if isOwner(uid) + && isCiphertext(request.resource.data.encryptedPrivateKey) + && request.resource.data.keys().hasOnly(['encryptedPrivateKey', 'updatedAt']); + allow delete: if false; + } } }