diff --git a/packages/shell/src/main/collab/sharing-engine.ts b/packages/shell/src/main/collab/sharing-engine.ts index 27921b74..3c1150cb 100644 --- a/packages/shell/src/main/collab/sharing-engine.ts +++ b/packages/shell/src/main/collab/sharing-engine.ts @@ -585,7 +585,11 @@ export class SharingEngine { async resolveSiblingRoster(): Promise { const { VaultPropertiesStore } = await import("../vault/vault-properties-store"); const props = await VaultPropertiesStore.open(this.#session.ydocStore); - return props.devices().listActive(); + // LAN-2b — verified under THIS identity's key. A forged roster row here + // would be sealed an entity DEK, so the read path is where it has to be + // caught: this is the consumer 10.3c turned from "LAN admission" into + // "every entity key in the vault". + return props.devices().listActive(this.#session.identity.publicKey); } /** diff --git a/packages/shell/src/main/collab/sibling-wrap-fanout.test.ts b/packages/shell/src/main/collab/sibling-wrap-fanout.test.ts index 1ffa92cc..55bf8a9c 100644 --- a/packages/shell/src/main/collab/sibling-wrap-fanout.test.ts +++ b/packages/shell/src/main/collab/sibling-wrap-fanout.test.ts @@ -14,6 +14,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { bytesToBase64 } from "../credentials/crypto"; +import { signAddDeviceRecord } from "../pairing/devices-store"; import { EntitiesRepository } from "../storage/entities-repo"; import { LoopbackRelayPort, type RelayPort } from "../sync/relay-port"; import { VaultSession } from "../vault/session"; @@ -67,14 +68,21 @@ describe("SharingEngine.fanOutEntityWrapToSiblings", () => { async function rosterSibling(x25519PubB64: string): Promise { const props = await VaultPropertiesStore.open(owner.ydocStore); - props.devices().add({ - deviceEd25519Pub: "sibling-ed25519-pub", - deviceX25519Pub: x25519PubB64, - deviceLabel: "Second device", - addedAt: Date.now(), - addedBy: owner.identity.publicKeyBase64, - sig: "test-signature", - }); + // LAN-2b — really signed under the owner's identity. This fixture used to + // plant `sig: "test-signature"`, which is precisely the forged-row shape + // the read path now rejects: a roster the vault owner never vouched for. + props.devices().add( + signAddDeviceRecord( + { + deviceEd25519Pub: "sibling-ed25519-pub", + deviceX25519Pub: x25519PubB64, + deviceLabel: "Second device", + addedAt: Date.now(), + addedBy: owner.identity.publicKeyBase64, + }, + owner.exposeIdentityForPairing().secretKey, + ), + ); // The store persists through an observer, and the engine re-opens the // doc from disk rather than sharing this instance — let the write land. await new Promise((resolve) => setTimeout(resolve, 50)); diff --git a/packages/shell/src/main/index.ts b/packages/shell/src/main/index.ts index 1700a74d..10f78792 100644 --- a/packages/shell/src/main/index.ts +++ b/packages/shell/src/main/index.ts @@ -2218,7 +2218,15 @@ void app.whenReady().then(async () => { .then(({ VaultPropertiesStore }) => VaultPropertiesStore.open(session.ydocStore)) .then((props) => { // Guard against a vault switch completing mid-resolve. - if (getActiveVaultSession() === session) lanDevices = props.devices(); + if (getActiveVaultSession() !== session) return; + // LAN-2b — bind the verifying key HERE, where the session that + // owns the roster is known, so the handshake's view of "who is + // rostered" is signature-verified rather than whatever the + // properties doc happens to hold. Adapting rather than handing + // the store over keeps the consumer's no-arg interface. + const store = props.devices(); + const userEd25519Pub = session.identity.publicKey; + lanDevices = { listActive: () => store.listActive(userEd25519Pub) }; }) .catch(() => { lanDevices = null; diff --git a/packages/shell/src/main/ipc/pairing-handlers.ts b/packages/shell/src/main/ipc/pairing-handlers.ts index b05d936a..7ee6d5e6 100644 --- a/packages/shell/src/main/ipc/pairing-handlers.ts +++ b/packages/shell/src/main/ipc/pairing-handlers.ts @@ -433,7 +433,12 @@ export function registerPairingHandlers(options: PairingHandlersOptions): () => // separate Y.Doc handle for the same id, so it cannot see a write that // has only happened in this one. That is precisely why re-reading the // roster after pairing still reported zero devices. - options.onDevicesChanged?.(active?.props.devices().listActive() ?? []); + // LAN-2b — verified under this vault's own identity key. This feeds + // LAN admission and the 10.3c DEK backfill, so an unverifiable row + // must never reach either. + options.onDevicesChanged?.( + active ? active.props.devices().listActive(active.session.identity.publicKey) : [], + ); } catch (error) { console.warn("[brainstorm] pairing-devices main-process hook failed:", error); } diff --git a/packages/shell/src/main/pairing/devices-store.test.ts b/packages/shell/src/main/pairing/devices-store.test.ts index a83ae7e5..565b4e0d 100644 --- a/packages/shell/src/main/pairing/devices-store.test.ts +++ b/packages/shell/src/main/pairing/devices-store.test.ts @@ -192,12 +192,44 @@ describe("DevicesStore", () => { store.add(r1); store.add(r2); store.revoke("dev-A", 1_800_000_000); - const active = store.listActive(); + const active = store.listActive(user.pub); expect(active.length).toBe(1); expect(active[0]?.deviceEd25519Pub).toBe("dev-B"); expect(store.list().length).toBe(2); // list() still surfaces both. }); + it("listActive() drops a record whose signature does not verify (LAN-2b)", () => { + const doc = freshDoc(); + const store = new DevicesStore(doc); + const user = freshUserPair(); + const addedBy = Buffer.from(user.pub).toString("base64"); + const good = signAddDeviceRecord(makeInput({ deviceEd25519Pub: "dev-good", addedBy }), user.sec); + // Signed by SOMEONE ELSE — the shape a forged roster row takes. Before + // 10.3c this bought LAN admission; now it would be sealed every entity DEK. + const attacker = freshUserPair(); + const forged = signAddDeviceRecord( + makeInput({ deviceEd25519Pub: "dev-forged", addedBy }), + attacker.sec, + ); + store.add(good); + store.add(forged); + + const active = store.listActive(user.pub); + expect(active.map((r) => r.deviceEd25519Pub)).toEqual(["dev-good"]); + // `list()` stays unfiltered so Settings can still show the bad row to remove. + expect(store.list().length).toBe(2); + }); + + it("listActive() returns nothing when the verifying key is wrong — fail closed", () => { + const doc = freshDoc(); + const store = new DevicesStore(doc); + const user = freshUserPair(); + const addedBy = Buffer.from(user.pub).toString("base64"); + store.add(signAddDeviceRecord(makeInput({ deviceEd25519Pub: "dev-A", addedBy }), user.sec)); + + expect(store.listActive(freshUserPair().pub)).toEqual([]); + }); + it("rejects malformed records on add()", () => { const doc = freshDoc(); const store = new DevicesStore(doc); diff --git a/packages/shell/src/main/pairing/devices-store.ts b/packages/shell/src/main/pairing/devices-store.ts index 8a5cb22d..cc9f1bb4 100644 --- a/packages/shell/src/main/pairing/devices-store.ts +++ b/packages/shell/src/main/pairing/devices-store.ts @@ -225,9 +225,36 @@ export class DevicesStore { * rotation operation (the "decoupled access change from key * rotation" decision recorded in OQ-27). The contract here is just: * a fresh wrap for an entity skips a revoked device entirely. + * + * **LAN-2b — every row is signature-verified here, and the key is a + * REQUIRED parameter rather than an option, so a new call site cannot + * silently opt out of verification.** `verifyAddDeviceRecord` existed + * from the start but only `vault-validate` ever called it, so the two + * consumers that decide real access — LAN admission and, since 10.3c, + * the entity-DEK fan-out — trusted whatever the roster happened to + * contain. 10.3c is what makes that matter: before it, a forged roster + * row bought LAN admission; now it buys **every entity DEK in the + * vault**, sealed to a key of the attacker's choosing. + * + * Fail-closed: a row whose signature does not verify under this + * identity is dropped, not returned-and-flagged, because both callers + * would otherwise have to remember to check. `list()` is deliberately + * left unfiltered — Settings must still be able to SHOW a bad row so + * the user can remove it. */ - listActive(): SignedAddDeviceRecord[] { - return this.list().filter((r) => r.revokedAt === undefined); + listActive(userEd25519Pub: Uint8Array): SignedAddDeviceRecord[] { + const kept: SignedAddDeviceRecord[] = []; + for (const record of this.list()) { + if (record.revokedAt !== undefined) continue; + if (!verifyAddDeviceRecord(record, userEd25519Pub)) { + console.warn( + `[devices] dropped an unverifiable roster record for ${record.deviceEd25519Pub.slice(0, 12)}… — signature does not verify under this identity`, + ); + continue; + } + kept.push(record); + } + return kept; } private readArray(): SignedAddDeviceRecord[] {