From 7d9cda442ece9311e329b361c33a9f703eeccd20 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 23 Jun 2026 13:14:13 -0400 Subject: [PATCH] fix(messages): decrypt residual DM cache writers to close ciphertext leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cache-population paths bypassed makeDmIngestDecryptor and wrote raw NIP-44 v2 ciphertext into the rendered DM timeline bucket, the same leak class as the identity-load cold-start race. useLoadMissingAncestors fetched a missing thread ancestor and merged it raw — deterministically reachable by deep-linking to a reply whose parent is older than the window. useLiveChannelUpdates' dual-write (a belt-and-suspenders against the useChannelSubscription connect window, PR #410) merged the raw live event; on an id collision the last writer wins, so a raw event arriving after the decrypting path could clobber the decrypted copy with ciphertext until the 5-min staleTime. Both now route the event through makeDmIngestDecryptor before merge — a no-op outside a 2-party DM, so uniform across channel types. The dual-write is kept (option a) rather than dropped (option b) because its connect-window race protection is real coverage the decrypting subscription does not provide during that window. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../channels/useLiveChannelUpdates.test.mjs | 92 ++++++++++++++++++ .../channels/useLiveChannelUpdates.ts | 39 +++++--- .../messages/useLoadMissingAncestors.test.mjs | 96 +++++++++++++++++++ .../messages/useLoadMissingAncestors.ts | 11 ++- 4 files changed, 224 insertions(+), 14 deletions(-) create mode 100644 desktop/src/features/channels/useLiveChannelUpdates.test.mjs create mode 100644 desktop/src/features/messages/useLoadMissingAncestors.test.mjs diff --git a/desktop/src/features/channels/useLiveChannelUpdates.test.mjs b/desktop/src/features/channels/useLiveChannelUpdates.test.mjs new file mode 100644 index 000000000..c66815ae2 --- /dev/null +++ b/desktop/src/features/channels/useLiveChannelUpdates.test.mjs @@ -0,0 +1,92 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { channelMessagesKey } from "@/features/messages/lib/messageQueryKeys"; +import { makeDmIngestDecryptor } from "@/features/messages/lib/dmCrypto"; +import { mergeTimelineCacheMessages } from "@/features/messages/hooks"; + +// Minimal valid NIP-44 v2 envelope (see messageQueryKeys.test.mjs). +const V2_CIPHERTEXT_LIVE = + "AgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + +const DM_CHANNEL_LIVE = { + id: "dm-live-channel-id", + channelType: "dm", + participantPubkeys: ["a".repeat(64), "b".repeat(64)], +}; +const SELF_LIVE = "a".repeat(64); +const PEER_LIVE = "b".repeat(64); + +function liveDmEvent(content) { + return { + id: "live".padEnd(64, "0"), + pubkey: PEER_LIVE, + created_at: 6_000, + kind: 9, + tags: [["h", DM_CHANNEL_LIVE.id]], + content, + sig: "mocksig".repeat(20).slice(0, 128), + }; +} + +// Mirror useLiveChannelUpdates' handleIncomingMessage timeline-cache write: +// decrypt via makeDmIngestDecryptor, then merge under the `if (!current)` +// guard. RED form (no decrypt) merges the raw event. +async function liveTimelineWrite(store, dmChannel, currentPubkey, event) { + const key = JSON.stringify( + channelMessagesKey(event.tags[0][1], currentPubkey), + ); + const [decrypted] = await makeDmIngestDecryptor( + dmChannel, + currentPubkey, + )([event]); + const current = store.get(key); + if (!current) { + return key; + } + store.set(key, mergeTimelineCacheMessages(current, decrypted)); + return key; +} + +test("live dual-write decrypts a DM event so it cannot clobber the decrypted copy with ciphertext", async () => { + const store = new Map(); + const key = JSON.stringify(channelMessagesKey(DM_CHANNEL_LIVE.id, SELF_LIVE)); + + // The decrypting useChannelSubscription seeds the bucket with plaintext-X. + store.set(key, [{ ...liveDmEvent("dinner at 7?"), content: "dinner at 7?" }]); + + // The live dual-write then fires for the SAME event id, carrying raw + // ciphertext. On the id collision the last writer wins — so without + // decryption this would replace plaintext-X with ciphertext-X. + await liveTimelineWrite( + store, + DM_CHANNEL_LIVE, + SELF_LIVE, + liveDmEvent(V2_CIPHERTEXT_LIVE), + ); + + const cached = store.get(key); + assert.equal(cached.length, 1, "id collision keeps a single row"); + assert.notEqual( + cached[0].content, + V2_CIPHERTEXT_LIVE, + "the live dual-write must not clobber plaintext with raw ciphertext", + ); +}); + +test("live dual-write never SEEDS an absent DM bucket (guard preserved)", async () => { + const store = new Map(); + // Bucket not yet seeded by the decrypting path: the guard returns early, so + // even a ciphertext event must not create a bucket here. + const key = await liveTimelineWrite( + store, + DM_CHANNEL_LIVE, + SELF_LIVE, + liveDmEvent(V2_CIPHERTEXT_LIVE), + ); + assert.equal( + store.has(key), + false, + "an absent bucket is never seeded by the dual-write", + ); +}); diff --git a/desktop/src/features/channels/useLiveChannelUpdates.ts b/desktop/src/features/channels/useLiveChannelUpdates.ts index 4007c0df1..186f71cd8 100644 --- a/desktop/src/features/channels/useLiveChannelUpdates.ts +++ b/desktop/src/features/channels/useLiveChannelUpdates.ts @@ -3,6 +3,7 @@ import { useQueryClient } from "@tanstack/react-query"; import { channelsQueryKey } from "@/features/channels/hooks"; import { mergeTimelineCacheMessages } from "@/features/messages/hooks"; +import { makeDmIngestDecryptor } from "@/features/messages/lib/dmCrypto"; import { channelMessagesKey } from "@/features/messages/lib/messageQueryKeys"; import { getChannelIdFromTags, @@ -248,25 +249,37 @@ export function useLiveChannelUpdates( // Merge into the timeline cache for the active channel. // useChannelSubscription also writes to this cache, but there's a - // race window where it hasn't connected yet. Writes are idempotent - // (mergeTimelineCacheMessages deduplicates by event ID). + // race window where it hasn't connected yet (PR #410). Writes are + // idempotent (mergeTimelineCacheMessages deduplicates by event ID). // // Keyed on the same selfPubkey (currentPubkey) as useChannelMessagesQuery // so this write lands in the identity-scoped bucket the renderer reads — // not a stale 2-element key that would orphan the event. The `if (!current)` // guard means this only appends to an already-populated cache, so it never - // seeds a DM bucket with the still-ciphertext event ahead of the decrypting - // subscription path. - queryClient.setQueryData( - channelMessagesKey(channelId, options.currentPubkey), - (current) => { - if (!current) { - return current; - } + // seeds a DM bucket ahead of the decrypting subscription path. + // + // Decrypt before merge: a DM body is NIP-44 v2 ciphertext, and this path + // catches the same CHANNEL_EVENT_KINDS/#h events as the decrypting + // useChannelSubscription. Without decryption a raw event arriving here + // *after* the decrypting path wrote plaintext-X would CLOBBER it on the + // id collision (mergeMessagesWithNormalizer keeps the last writer). The + // decryptor is a no-op outside a 2-party DM, so this is uniform/safe. + const dmChannel = dmChannelMap.get(channelId) ?? null; + void makeDmIngestDecryptor( + dmChannel, + options.currentPubkey, + )([event]).then(([decrypted]) => { + queryClient.setQueryData( + channelMessagesKey(channelId, options.currentPubkey), + (current) => { + if (!current) { + return current; + } - return mergeTimelineCacheMessages(current, event); - }, - ); + return mergeTimelineCacheMessages(current, decrypted); + }, + ); + }); }); const handleMentionEvent = React.useEffectEvent((event: RelayEvent) => { diff --git a/desktop/src/features/messages/useLoadMissingAncestors.test.mjs b/desktop/src/features/messages/useLoadMissingAncestors.test.mjs new file mode 100644 index 000000000..300b503b1 --- /dev/null +++ b/desktop/src/features/messages/useLoadMissingAncestors.test.mjs @@ -0,0 +1,96 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { channelMessagesKey } from "@/features/messages/lib/messageQueryKeys"; +import { + decryptIngestedContent, + makeDmIngestDecryptor, +} from "@/features/messages/lib/dmCrypto"; +import { mergeMessages } from "@/features/messages/hooks"; + +// base64(0x02 + 98 zero bytes) — minimal valid NIP-44 v2 envelope, so +// looksLikeNip44V2 treats it as an encrypted DM body to decrypt, not legacy +// plaintext. Matches the fixture in messageQueryKeys.test.mjs. +const V2_CIPHERTEXT = + "AgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + +const DM_CHANNEL = { + id: "dm-channel-id", + channelType: "dm", + participantPubkeys: ["a".repeat(64), "b".repeat(64)], +}; +const SELF = "a".repeat(64); +const PEER = "b".repeat(64); + +function ancestorEvent(content) { + return { + id: "anc".padEnd(64, "0"), + pubkey: PEER, + created_at: 4_000, + kind: 9, + tags: [["h", DM_CHANNEL.id]], + content, + sig: "mocksig".repeat(20).slice(0, 128), + }; +} + +// Mirror useLoadMissingAncestors' fetched-ancestor cache write: decrypt the +// fetched event, then mergeMessages into the channel cache. RED form (no +// decrypt) writes the raw event; the fix routes it through the decryptor. +async function loadAncestorIntoCache(store, channel, selfPubkey, event) { + const decryptIngested = makeDmIngestDecryptor(channel, selfPubkey); + const key = JSON.stringify(channelMessagesKey(channel.id, selfPubkey)); + const [decrypted] = await decryptIngested([event]); + const current = store.get(key) ?? []; + store.set(key, mergeMessages(current, decrypted)); + return key; +} + +test("missing DM ancestor is decrypted before it lands in the rendered cache, never raw ciphertext", async () => { + const store = new Map(); + + // A fetched ancestor whose body is valid v2 ciphertext. Routed through the + // ingest decryptor with a resolved identity, the rendered cache must NOT end + // holding the raw ciphertext. + const key = await loadAncestorIntoCache( + store, + DM_CHANNEL, + SELF, + ancestorEvent(V2_CIPHERTEXT), + ); + + const cached = store.get(key); + assert.equal(cached.length, 1, "the ancestor is cached"); + assert.notEqual( + cached[0].content, + V2_CIPHERTEXT, + "raw NIP-44 v2 ciphertext must never be written into the rendered DM cache", + ); +}); + +test("decryptIngestedContent turns a valid-v2 ancestor body into the decrypted plaintext", async () => { + // Independent proof that the decryptor TRANSFORMS valid-v2 ciphertext (not a + // no-op passthrough), with an injected decrypt standing in for Tauri NIP-44. + const content = await decryptIngestedContent( + ancestorEvent(V2_CIPHERTEXT), + PEER, + async () => "decrypted ancestor body", + ); + assert.equal(content, "decrypted ancestor body"); +}); + +test("missing ancestor in a non-DM channel is passed through unchanged", async () => { + const store = new Map(); + const streamChannel = { + id: "stream-channel-id", + channelType: "stream", + participantPubkeys: [], + }; + // Outside a 2-party DM the decryptor is an identity no-op: a v2-shaped body + // here is NOT an encrypted DM, so it must pass through verbatim. + const key = await loadAncestorIntoCache(store, streamChannel, SELF, { + ...ancestorEvent(V2_CIPHERTEXT), + tags: [["h", streamChannel.id]], + }); + assert.equal(store.get(key)[0].content, V2_CIPHERTEXT); +}); diff --git a/desktop/src/features/messages/useLoadMissingAncestors.ts b/desktop/src/features/messages/useLoadMissingAncestors.ts index c80cf940b..c0337d175 100644 --- a/desktop/src/features/messages/useLoadMissingAncestors.ts +++ b/desktop/src/features/messages/useLoadMissingAncestors.ts @@ -3,6 +3,7 @@ import { useQueryClient } from "@tanstack/react-query"; import { channelMessagesKey } from "@/features/messages/lib/messageQueryKeys"; import { mergeMessages } from "@/features/messages/hooks"; +import { makeDmIngestDecryptor } from "@/features/messages/lib/dmCrypto"; import { getChannelIdFromTags, getThreadReference, @@ -78,6 +79,8 @@ export function useLoadMissingAncestors( let isCancelled = false; + const decryptIngested = makeDmIngestDecryptor(activeChannel, selfPubkey); + void Promise.all( [...missingAncestorIds].map(async (eventId) => { try { @@ -90,9 +93,15 @@ export function useLoadMissingAncestors( return; } + // Decrypt before caching: a DM ancestor is a NIP-44 v2 ciphertext + // body, so it must route through the same decryptor as every other + // ingest site or it lands raw in the rendered bucket (the decryptor + // is a no-op outside a 2-party DM, so this is uniform/safe). + const [decrypted] = await decryptIngested([event]); + queryClient.setQueryData( channelMessagesKey(activeChannel.id, selfPubkey), - (current = []) => mergeMessages(current, event), + (current = []) => mergeMessages(current, decrypted), ); } catch (error) { console.error("Failed to load ancestor event", eventId, error);