Handover asks live in the database, so a restart doesn't lose one (GRYT-2023)

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")) {

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<void> {
+  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<DmKeyAsk[]> {
+  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<void> {
+  getSqliteDb().prepare("DELETE FROM dm_key_asks WHERE server_user_id = ? AND binding = ?").run(serverUserId, binding);
+}

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";

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 }[];

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 };

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,36 +300,39 @@ 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 () => {
     const ida = await connect("Ida");
     const heard: Record<string, unknown[]> = {};
     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 () => {

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<string, HandoverAsk[]>();
-
-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 });
     },
   };
 }