From bcefae8e55a394c6f128db22fe3f706d2cee1c4e Mon Sep 17 00:00:00 2001 From: Sivert <316918682+grytchat@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:31:12 +0100 Subject: [PATCH 1/2] Handover asks live in the database, so a restart doesn't lose one (GRYT-2023) pendingAsks was an in-memory Map. Restarting a server for 1.12.7 dropped the handover Sivert's Mac had asked for on pp.sivert.io, with its device name, and the device holding the key never heard about it again. Asks now go in a dm_key_asks table: one row per account and asking key, with the device name and when it first asked. They're named by the asking binding instead of a socket id, which a restart also throws away. That name still goes out as askerId, so older clients keep working. Approving, saying no or stopping the reset removes the row, and so does expiry after 30 days. A no goes to every device on the account, with the binding, and only the device that asked acts on it. Co-Authored-By: Claude Opus 5.5 --- src/db/sqlite/connection.ts | 9 +++++ src/db/sqlite/dmKeyAsks.ts | 38 +++++++++++++++++++++ src/db/sqlite/index.ts | 1 + src/socket/handlers/dmKeys.test.ts | 51 ++++++++++++++++++++-------- src/socket/handlers/dmKeys.ts | 54 +++++++++++++----------------- 5 files changed, 109 insertions(+), 44 deletions(-) create mode 100644 src/db/sqlite/dmKeyAsks.ts 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/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 }); }, }; } From 698ed1c3450e46576017ef8b8e48cc583431e315 Mon Sep 17 00:00:00 2001 From: Sivert <316918682+grytchat@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:40:33 +0100 Subject: [PATCH 2/2] Guest merge drops the guest's handover asks (GRYT-2023) The merge test checks every column that can hold a server user id. A guest's asks were for the guest's own key, so they're deleted like its push devices, and the device asks again as the account. Co-Authored-By: Claude Opus 5.5 --- src/db/sqlite/mergeGuest.test.ts | 2 +- src/db/sqlite/mergeGuest.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) 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 };