feat(crypto): escrow the sealed-reveal key under the couple key (heals the second-device gap)
The per-user ECIES keypair lived on exactly one phone. A second device — a new
phone, a reinstall — had no private key, so every sealed reveal died there. Worse
than dying: it reported "waiting for partner". The user isn't waiting for anything;
their key is gone. They'd never report that bug accurately, because the app told
them the wrong story.
Fix: escrow, not multi-device fan-out. The private keyset is stored encrypted under
the COUPLE key (AAD = uid) at users/{uid}/devices/primary/secure/escrow; a device
holding the couple key — 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.
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 keyset — is material the
partner already has. It adds zero capability to anyone. It's owner-only anyway:
the escrow is a SUBDOC because the parent devices/primary must stay
partner-readable (they seal to the public key) and rules can't gate one field.
That distinction is now the mutation-checked test — making the escrow inherit the
parent's read rule fails exactly "the PARTNER cannot read the escrow".
Corrected the KDoc while I was in there: it claimed a second sign-in "overwrites
users/{uid}/devices/primary". It doesn't — every publish site is existence-gated,
so device 1 keeps working. The real failure was narrower and quieter than the
documentation said, which is its own small lesson about trusting comments.
Heal-forward on Home load (migrateProfileFields shape: idempotent, best-effort,
never blocks Home) so existing users escrow on next launch. Residual case
documented rather than hidden: a user whose only device dies BEFORE ever escrowing
keeps today's behaviour — that key never left the phone and nothing can fix it
retroactively.
10 new rules tests (151 total, mutation-checked). Android suite green, assemble
clean. Rules deploy + the live wipe→recover→sealed-reveal proof still pending.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
5716aead8f
commit
3c74aec82d
|
|
@ -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()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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) }
|
||||
|
|
|
|||
|
|
@ -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}", () => {
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue