diff --git a/src/db/sqlite/connection.ts b/src/db/sqlite/connection.ts index cae06ee..30ccb24 100644 --- a/src/db/sqlite/connection.ts +++ b/src/db/sqlite/connection.ts @@ -1060,6 +1060,15 @@ function runMigrations(d: DatabaseSync): void { d.prepare("UPDATE users SET dm_key_seen_at = ? WHERE dm_key_binding IS NOT NULL").run(Date.now()); } + // Handover asks, so a restart doesn't lose one (GRYT-2023). + d.exec(`CREATE TABLE IF NOT EXISTS dm_key_asks ( + server_user_id TEXT NOT NULL, + binding TEXT NOT NULL, + device_name TEXT NOT NULL, + created_at INTEGER NOT NULL, + PRIMARY KEY (server_user_id, binding) + )`); + // The MLS person key binding (GRYT-1515). Checked against the DM key binding's // signer when it's published, then stored whole like that one. if (!hasColumn(d, "users", "person_key_binding")) { diff --git a/src/db/sqlite/dmKeyAsks.ts b/src/db/sqlite/dmKeyAsks.ts new file mode 100644 index 0000000..ae77a4e --- /dev/null +++ b/src/db/sqlite/dmKeyAsks.ts @@ -0,0 +1,38 @@ +import { getSqliteDb } from "./connection"; + +/** A device with its own key asking to take over the account's DM key here (GRYT-2023). */ +export interface DmKeyAsk { + binding: string; + deviceName: string; + createdAt: number; +} + +/** More than this and the oldest goes. One account asking from five devices at once is already odd. */ +export const MAX_DM_KEY_ASKS = 5; + +/** Asking again keeps the first time, so it can't be used to keep an ask fresh forever. */ +export async function addDmKeyAsk(serverUserId: string, binding: string, deviceName: string, at: number): Promise { + const d = getSqliteDb(); + d.prepare( + `INSERT INTO dm_key_asks (server_user_id, binding, device_name, created_at) VALUES (?, ?, ?, ?) + ON CONFLICT (server_user_id, binding) DO UPDATE SET device_name = excluded.device_name`, + ).run(serverUserId, binding, deviceName, at); + d.prepare( + `DELETE FROM dm_key_asks WHERE server_user_id = ? AND binding NOT IN + (SELECT binding FROM dm_key_asks WHERE server_user_id = ? ORDER BY created_at DESC LIMIT ?)`, + ).run(serverUserId, serverUserId, MAX_DM_KEY_ASKS); +} + +/** The asks still open, oldest first. Expired ones are deleted on the way. */ +export async function listDmKeyAsks(serverUserId: string, maxAgeMs: number, now = Date.now()): Promise { + const d = getSqliteDb(); + d.prepare("DELETE FROM dm_key_asks WHERE server_user_id = ? AND created_at <= ?").run(serverUserId, now - maxAgeMs); + const rows = d + .prepare("SELECT binding, device_name, created_at FROM dm_key_asks WHERE server_user_id = ? ORDER BY created_at") + .all(serverUserId) as { binding: string; device_name: string; created_at: number }[]; + return rows.map((r) => ({ binding: r.binding, deviceName: r.device_name, createdAt: Number(r.created_at) })); +} + +export async function removeDmKeyAsk(serverUserId: string, binding: string): Promise { + getSqliteDb().prepare("DELETE FROM dm_key_asks WHERE server_user_id = ? AND binding = ?").run(serverUserId, binding); +} diff --git a/src/db/sqlite/index.ts b/src/db/sqlite/index.ts index 8323708..4491569 100644 --- a/src/db/sqlite/index.ts +++ b/src/db/sqlite/index.ts @@ -1,5 +1,6 @@ export { initSqlite } from "./connection"; export * from "./users"; +export * from "./dmKeyAsks"; export * from "./mergeGuest"; export * from "./messages"; export * from "./threads"; diff --git a/src/db/sqlite/mergeGuest.test.ts b/src/db/sqlite/mergeGuest.test.ts index c77fc68..79a6261 100644 --- a/src/db/sqlite/mergeGuest.test.ts +++ b/src/db/sqlite/mergeGuest.test.ts @@ -251,7 +251,7 @@ describe("merging a guest into an account that is already a member", () => { "users.server_user_id", "roles.server_user_id", "conversation_members.server_user_id", "mentions.server_user_id", "read_markers.server_user_id", "refresh_tokens.server_user_id", "mls_devices.server_user_id", "mls_key_packages.server_user_id", "mls_welcomes.server_user_id", - "mls_removed_devices.server_user_id", "push_devices.server_user_id", + "mls_removed_devices.server_user_id", "push_devices.server_user_id", "dm_key_asks.server_user_id", ]); const listed = new Set(AUTHOR_COLUMNS.map(([t, c]) => `${t}.${c}`)); const tables = db.prepare(`SELECT name FROM sqlite_master WHERE type = 'table'`).all() as { name: string }[]; diff --git a/src/db/sqlite/mergeGuest.ts b/src/db/sqlite/mergeGuest.ts index d2981ee..5037f52 100644 --- a/src/db/sqlite/mergeGuest.ts +++ b/src/db/sqlite/mergeGuest.ts @@ -169,6 +169,8 @@ function mergeInTransaction( db.prepare(`UPDATE refresh_tokens SET revoked = 1 WHERE gryt_user_id = ? AND revoked = 0`).run(guestGrytUserId); // The phone registers again as the account when it reconnects. db.prepare(`DELETE FROM push_devices WHERE server_user_id = ?`).run(from); + // A guest's handover asks were for the guest's key; the device asks again as the account. + db.prepare(`DELETE FROM dm_key_asks WHERE server_user_id = ?`).run(from); db.prepare(`DELETE FROM users WHERE server_user_id = ?`).run(from); return { guestServerUserId: from, accountServerUserId: to, ownerMoved, conversationIds }; diff --git a/src/socket/handlers/dmKeys.test.ts b/src/socket/handlers/dmKeys.test.ts index 2fc9047..301a110 100644 --- a/src/socket/handlers/dmKeys.test.ts +++ b/src/socket/handlers/dmKeys.test.ts @@ -7,6 +7,7 @@ import { after, before, describe, it } from "node:test"; import { initSqlite } from "../../db/sqlite/connection"; import { createServerConfigIfNotExists, setServerRole } from "../../db/sqlite/servers"; import { getUserByServerId, setUserDmKeyReset, setUserDmKeySeen, upsertUser } from "../../db/sqlite/users"; +import { listDmKeyAsks } from "../../db/sqlite/dmKeyAsks"; import type { Clients } from "../../types"; import { buildMemberList } from "../utils/clients"; import { KEY_RESET_DELAY_MS, registerDmKeyHandlers } from "./dmKeys"; @@ -299,17 +300,19 @@ describe("one identity per account (GRYT-1868)", () => { for (const m of [frank, frankPhone, stranger]) { io.sockets.sockets.set(m.clientId, { emit: (event: string, body: unknown) => (heard[m.clientId] ??= []).push([event, body]) }); } + await publish(frankPhone, { binding: await (await signer()).binding() }); + delete heard[frankPhone.clientId]; const binding = await (await signer()).binding(); - frank.handlers["dm:key:handover:ask"]({ binding, deviceName: "Mac" }); + await call(frank, "dm:key:handover:ask", { binding, deviceName: "Mac" }); - assert.deepEqual(heard[frankPhone.clientId], [["dm:key:handover:asked", { binding, deviceName: "Mac", askerId: frank.clientId }]]); + assert.deepEqual(heard[frankPhone.clientId]?.[0], ["dm:key:handover:asked", { binding, deviceName: "Mac", askerId: binding }]); assert.equal(heard[stranger.clientId], undefined, "nobody else's devices hear it"); - assert.equal(heard[frank.clientId], undefined, "the asker doesn't ask itself"); + assert.ok(!heard[frank.clientId]?.some((e) => (e as unknown[])[0] === "dm:key:handover:asked"), "the asker doesn't ask itself"); - stranger.handlers["dm:key:handover:deny"]({ askerId: frank.clientId }); - assert.equal(heard[frank.clientId], undefined, "only the account's own devices can say no"); - frankPhone.handlers["dm:key:handover:deny"]({ askerId: frank.clientId }); - assert.deepEqual(heard[frank.clientId], [["dm:key:handover:denied", {}]]); + await stranger.handlers["dm:key:handover:deny"]({ askerId: binding }); + assert.ok(!heard[frank.clientId]?.some((e) => (e as unknown[])[0] === "dm:key:handover:denied"), "only the account's own devices can say no"); + await frankPhone.handlers["dm:key:handover:deny"]({ askerId: binding }); + assert.deepEqual(heard[frank.clientId]?.at(-1), ["dm:key:handover:denied", { binding }]); }); it("hands a request to a device that connects after it was made, until it's answered", async () => { @@ -317,18 +320,19 @@ describe("one identity per account (GRYT-1868)", () => { const heard: Record = {}; const listen = (m: Member) => io.sockets.sockets.set(m.clientId, { emit: (event: string, body: unknown) => (heard[m.clientId] ??= []).push([event, body]) }); listen(ida); + await publish(ida, { binding: await (await signer()).binding() }); const binding = await (await signer()).binding(); - ida.handlers["dm:key:handover:ask"]({ binding, deviceName: "Mac" }); + await call(ida, "dm:key:handover:ask", { binding, deviceName: "Mac" }); const phone = await connect("Ida's phone"); clientsInfo[phone.clientId].serverUserId = ida.serverUserId; listen(phone); - phone.handlers["dm:key:handover:pending"]({}); - assert.deepEqual(heard[phone.clientId], [["dm:key:handover:asked", { binding, deviceName: "Mac", askerId: ida.clientId }]]); + await phone.handlers["dm:key:handover:pending"]({}); + assert.deepEqual(heard[phone.clientId], [["dm:key:handover:asked", { binding, deviceName: "Mac", askerId: binding }]]); - phone.handlers["dm:key:handover:deny"]({ askerId: ida.clientId }); + await phone.handlers["dm:key:handover:deny"]({ askerId: binding }); heard[phone.clientId] = []; - phone.handlers["dm:key:handover:pending"]({}); + await phone.handlers["dm:key:handover:pending"]({}); assert.deepEqual(heard[phone.clientId], [], "an answered request isn't handed out again"); }); @@ -374,8 +378,27 @@ describe("one identity per account (GRYT-1868)", () => { delete clientsInfo[asking.clientId]; const heard: unknown[] = []; io.sockets.sockets.set(jill.clientId, { emit: (event: string, body: unknown) => heard.push([event, body]) }); - jill.handlers["dm:key:handover:pending"]({}); - assert.deepEqual(heard, [["dm:key:handover:asked", { binding: next, deviceName: "iPhone", askerId: asking.clientId }]], "an ask outlives the asker's connection"); + await jill.handlers["dm:key:handover:pending"]({}); + assert.deepEqual(heard, [["dm:key:handover:asked", { binding: next, deviceName: "iPhone", askerId: next }]], "an ask outlives the asker's connection"); + assert.deepEqual((await listDmKeyAsks(jill.serverUserId, KEY_RESET_DELAY_MS)).map((a) => a.deviceName), ["iPhone"], "and it's in the database, not in memory"); + }); + + it("drops the ask when the reset is stopped or the handover is approved", async () => { + const max = await connect("Max"); + const [pc, phone, mac] = [await signer(), await signer(), await signer()]; + const current = await pc.binding(); + await publish(max, { binding: current }); + await call(max, "dm:key:handover:ask", { binding: await phone.binding(), deviceName: "iPhone" }); + const proof = await gryt.signKeyHandover({ newBinding: current, scope, identityPrivateKey: pc.privateKey, identityPublicJwk: pc.publicJwk }); + await call(max, "dm:key:reset:cancel", { proof }); + assert.deepEqual(await listDmKeyAsks(max.serverUserId, KEY_RESET_DELAY_MS), [], "\"that wasn't me\" takes the ask with it"); + + const next = await mac.binding(); + await call(max, "dm:key:handover:ask", { binding: next, deviceName: "Mac" }); + const handover = await gryt.signKeyHandover({ newBinding: next, scope, identityPrivateKey: pc.privateKey, identityPublicJwk: pc.publicJwk }); + assert.equal((await publish(max, { binding: next, replace: true, handover })).ok, true); + assert.deepEqual(await listDmKeyAsks(max.serverUserId, KEY_RESET_DELAY_MS), []); + assert.equal((await getUserByServerId(max.serverUserId))?.dm_key_holder, "Mac", "the new holder is named after the asker"); }); it("hands over at once when no device with the current key has been here for 30 days", async () => { diff --git a/src/socket/handlers/dmKeys.ts b/src/socket/handlers/dmKeys.ts index e71c340..12728eb 100644 --- a/src/socket/handlers/dmKeys.ts +++ b/src/socket/handlers/dmKeys.ts @@ -1,5 +1,5 @@ import type { HandlerContext, EventHandlerMap } from "./types"; -import { getUserByServerId, setUserDmKeyBinding, setUserDmKeyReset, setUserDmKeySeen } from "../../db"; +import { addDmKeyAsk, getUserByServerId, listDmKeyAsks, removeDmKeyAsk, setUserDmKeyBinding, setUserDmKeyReset, setUserDmKeySeen } from "../../db"; import { dmBindingIdentity, handoverValid, KEY_RESET_DELAY_MS, keyResetDueAt } from "../../services/personKeyBinding"; import { pushNotify } from "../../services/push"; import { checkRateLimit, type RateLimitRule } from "../../utils/rateLimiter"; @@ -22,16 +22,9 @@ export { KEY_RESET_DELAY_MS }; /** Every connect republishes, so this only stops a write per connect. */ const SEEN_WRITE_EVERY_MS = 60 * 60 * 1000; -interface HandoverAsk { binding: string; deviceName: string; askerId: string; at: number } -/* Per server and account, the requests still waiting, so a phone that connects late gets them. */ -const pendingAsks = new Map(); - -function liveAsks(key: string, now = Date.now()): HandoverAsk[] { - const live = (pendingAsks.get(key) ?? []).filter((a) => now - a.at < KEY_RESET_DELAY_MS); - if (live.length) pendingAsks.set(key, live); - else pendingAsks.delete(key); - return live; -} +/* An ask is named by the asking key, not a socket id, so it means the same after a restart. Older + clients read that name as `askerId`. */ +const asked = (ask: { binding: string; deviceName: string }) => ({ binding: ask.binding, deviceName: ask.deviceName, askerId: ask.binding }); /** A binding is derived from a seed and a scope, so it is sent on arrival and then never again. Anything faster is a client with a loop in it. */ @@ -102,13 +95,12 @@ export function registerDmKeyHandlers(ctx: HandlerContext): EventHandlerMap { } } - const askKey = `${serverId}:${serverUserId}`; await setUserDmKeyBinding(serverUserId, binding); if (user.dm_key_reset_binding) await setUserDmKeyReset(serverUserId, null, null); // Approving publishes the asker's key, so the name is the asker's. - const asker = liveAsks(askKey).find((a) => a.binding === binding); + const asker = binding ? (await listDmKeyAsks(serverUserId, KEY_RESET_DELAY_MS)).find((a) => a.binding === binding) : undefined; if (binding) await setUserDmKeySeen(serverUserId, Date.now(), asker ? asker.deviceName || null : deviceName); - if (pendingAsks.has(askKey)) pendingAsks.set(askKey, liveAsks(askKey).filter((a) => a.binding !== binding)); + if (asker) await removeDmKeyAsk(serverUserId, asker.binding); reply({ ok: true, changed: true }); // The list is deduped on its contents and the binding is in that hash, so @@ -127,18 +119,18 @@ export function registerDmKeyHandlers(ctx: HandlerContext): EventHandlerMap { if (!serverUserId || typeof binding !== "string" || binding.length > MAX_BINDING_BYTES || !COMPACT_JWT.test(binding)) return; if (!checkRateLimit("dm:key:handover:ask", serverUserId, getClientIp(), RL_DM_KEY).allowed) return; const deviceName = typeof payload?.deviceName === "string" ? payload.deviceName.slice(0, 64) : ""; - const key = `${serverId}:${serverUserId}`; - pendingAsks.set(key, [...liveAsks(key).filter((a) => a.askerId !== clientId).slice(-4), { binding, deviceName, askerId: clientId, at: Date.now() }]); + const user = await getUserByServerId(serverUserId); + const [had, sent] = user?.dm_key_binding ? await Promise.all([dmBindingIdentity(user.dm_key_binding), dmBindingIdentity(binding)]) : [null, null]; + if (!user || !sent || sent === had) return reply({ ok: false }); + + await addDmKeyAsk(serverUserId, binding, deviceName, Date.now()); for (const [cid, ci] of Object.entries(clientsInfo)) { if (cid !== clientId && ci.serverUserId === serverUserId) { - io.sockets.sockets.get(cid)?.emit("dm:key:handover:asked", { binding, deviceName, askerId: clientId }); + io.sockets.sockets.get(cid)?.emit("dm:key:handover:asked", asked({ binding, deviceName })); } } pushNotify(clientsInfo, [serverUserId], "approve", `handover:${serverUserId}`, { evenIfPresent: true }); - const user = await getUserByServerId(serverUserId); - const [had, sent] = user?.dm_key_binding ? await Promise.all([dmBindingIdentity(user.dm_key_binding), dmBindingIdentity(binding)]) : [null, null]; - if (!user || !sent || sent === had) return reply({ ok: false }); // Same rule as a reset: asking again doesn't restart the clock. if (user.dm_key_reset_binding !== binding || !user.dm_key_reset_at) { await setUserDmKeyReset(serverUserId, binding, Date.now() + KEY_RESET_DELAY_MS); @@ -151,12 +143,12 @@ export function registerDmKeyHandlers(ctx: HandlerContext): EventHandlerMap { }, /** A device that just connected asks for requests made while it was away, even if the asker has gone since. */ - "dm:key:handover:pending": () => { + "dm:key:handover:pending": async () => { const serverUserId = clientsInfo[clientId]?.serverUserId; if (!serverUserId) return; - for (const ask of liveAsks(`${serverId}:${serverUserId}`)) { - if (ask.askerId === clientId) continue; - io.sockets.sockets.get(clientId)?.emit("dm:key:handover:asked", { binding: ask.binding, deviceName: ask.deviceName, askerId: ask.askerId }); + // The asking device gets its own ask back too, and ignores it: only a device with the current key answers. + for (const ask of await listDmKeyAsks(serverUserId, KEY_RESET_DELAY_MS)) { + io.sockets.sockets.get(clientId)?.emit("dm:key:handover:asked", asked(ask)); } }, @@ -189,19 +181,21 @@ export function registerDmKeyHandlers(ctx: HandlerContext): EventHandlerMap { return reply({ ok: false, error: "proof_required" }); } await setUserDmKeyReset(serverUserId, null, null); + await removeDmKeyAsk(serverUserId, user.dm_key_reset_binding); reply({ ok: true }); emitToAccount(serverUserId, "dm:key:reset:pending", { at: null }); broadcastMemberList(io, clientsInfo, serverId); }, /** "That isn't mine": tells the asking device, so it stops waiting. */ - "dm:key:handover:deny": (payload: { askerId?: string }) => { + "dm:key:handover:deny": async (payload: { askerId?: string }) => { const serverUserId = clientsInfo[clientId]?.serverUserId; - const askerId = payload?.askerId; - const key = `${serverId}:${serverUserId}`; - if (!serverUserId || typeof askerId !== "string" || !liveAsks(key).some((a) => a.askerId === askerId)) return; - pendingAsks.set(key, liveAsks(key).filter((a) => a.askerId !== askerId)); - io.sockets.sockets.get(askerId)?.emit("dm:key:handover:denied", {}); + const binding = payload?.askerId; + if (!serverUserId || typeof binding !== "string") return; + if (!(await listDmKeyAsks(serverUserId, KEY_RESET_DELAY_MS)).some((a) => a.binding === binding)) return; + await removeDmKeyAsk(serverUserId, binding); + // To every device on the account: the one whose key this is stops waiting. + emitToAccount(serverUserId, "dm:key:handover:denied", { binding }); }, }; }