mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(messages): decrypt residual DM cache writers to close ciphertext leak
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 <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -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",
|
||||
);
|
||||
});
|
||||
@@ -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<RelayEvent[]>(
|
||||
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<RelayEvent[]>(
|
||||
channelMessagesKey(channelId, options.currentPubkey),
|
||||
(current) => {
|
||||
if (!current) {
|
||||
return current;
|
||||
}
|
||||
|
||||
return mergeTimelineCacheMessages(current, event);
|
||||
},
|
||||
);
|
||||
return mergeTimelineCacheMessages(current, decrypted);
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
const handleMentionEvent = React.useEffectEvent((event: RelayEvent) => {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
@@ -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<RelayEvent[]>(
|
||||
channelMessagesKey(activeChannel.id, selfPubkey),
|
||||
(current = []) => mergeMessages(current, event),
|
||||
(current = []) => mergeMessages(current, decrypted),
|
||||
);
|
||||
} catch (error) {
|
||||
console.error("Failed to load ancestor event", eventId, error);
|
||||
|
||||
Reference in New Issue
Block a user