Merge chore/code-hygiene: Phase 2 #7 Messages (E2EE crypto fix + UI bugs)

Fix: AES-GCM nonce reuse in e2ee.setup() (+ drop redundant key_check oracle,
backward-compatible); silent 404/429 send failures now toast; scroll/render-churn
in ChatThread + Messages; chatDock LS helper; deleted dead lock(). Verified: tsc,
re-review clean, localdev boots, self-E2E green (unlock + full-thread decrypt on a
real existing key, user entered the passphrase).
This commit is contained in:
npeter83 2026-07-11 20:48:38 +02:00
commit 01d55f3029
7 changed files with 38 additions and 16 deletions

View file

@ -2,9 +2,10 @@ import { useEffect, useRef, useState } from "react";
import { useTranslation } from "react-i18next"; import { useTranslation } from "react-i18next";
import { useMutation, useQueryClient } from "@tanstack/react-query"; import { useMutation, useQueryClient } from "@tanstack/react-query";
import { Send } from "lucide-react"; import { Send } from "lucide-react";
import { api, type Message, type MessageUser } from "../lib/api"; import { api, HttpError, type Message, type MessageUser } from "../lib/api";
import { useLiveQuery } from "../lib/useLiveQuery"; import { useLiveQuery } from "../lib/useLiveQuery";
import { relativeTime } from "../lib/format"; import { relativeTime } from "../lib/format";
import { notify } from "../lib/notifications";
import { partnerPub, useKeyState } from "../lib/messaging"; import { partnerPub, useKeyState } from "../lib/messaging";
import * as e2ee from "../lib/e2ee"; import * as e2ee from "../lib/e2ee";
import KeyGate from "./KeyGate"; import KeyGate from "./KeyGate";
@ -39,12 +40,15 @@ export default function ChatThread({
); );
const items = q.data?.items ?? []; const items = q.data?.items ?? [];
// Decrypt the thread when fresh server data arrives. Keyed on dataUpdatedAt (not `items`): the
// `?? []` fallback is a new array reference every render, which would otherwise re-run this
// effect (and setPlain a new object) on every render while data is loading — a render storm.
useEffect(() => { useEffect(() => {
if (isSystem || !ready) return; if (isSystem || !ready) return;
let cancelled = false; let cancelled = false;
(async () => { (async () => {
const out: Record<number, string> = {}; const out: Record<number, string> = {};
for (const m of items) { for (const m of q.data?.items ?? []) {
if (m.ciphertext && m.iv) { if (m.ciphertext && m.iv) {
try { try {
const pub = await partnerPub(partnerId); const pub = await partnerPub(partnerId);
@ -59,7 +63,8 @@ export default function ChatThread({
return () => { return () => {
cancelled = true; cancelled = true;
}; };
}, [items, ready, isSystem, partnerId, t]); // eslint-disable-next-line react-hooks/exhaustive-deps
}, [q.dataUpdatedAt, ready, isSystem, partnerId, t]);
// Opening / refreshing the thread marks incoming messages read, so clear the badges. // Opening / refreshing the thread marks incoming messages read, so clear the badges.
useEffect(() => { useEffect(() => {
@ -69,9 +74,12 @@ export default function ChatThread({
} }
}, [q.dataUpdatedAt, qc]); }, [q.dataUpdatedAt, qc]);
// Scroll to the newest message when the count changes (open / new message) — NOT on `plain`,
// which gets a fresh object on every poll's decrypt pass and would yank a scrolled-up reader
// back to the bottom even when nothing new arrived.
useEffect(() => { useEffect(() => {
bottomRef.current?.scrollIntoView({ block: "end" }); bottomRef.current?.scrollIntoView({ block: "end" });
}, [items.length, plain]); }, [items.length]);
const send = useMutation({ const send = useMutation({
mutationFn: async (text: string) => { mutationFn: async (text: string) => {
@ -84,6 +92,14 @@ export default function ChatThread({
qc.invalidateQueries({ queryKey: ["thread", partnerId] }); qc.invalidateQueries({ queryKey: ["thread", partnerId] });
qc.invalidateQueries({ queryKey: ["conversations"] }); qc.invalidateQueries({ queryKey: ["conversations"] });
}, },
onError: (e) => {
// The draft is kept (only cleared on success) so the user can retry. 400/409/422/500 already
// raised the global error dialog; 404 (recipient gone) and 429 (rate-limited) are "caller-
// handled" in api.req (no modal), so surface those here or the send fails silently.
if (e instanceof HttpError && (e.status === 404 || e.status === 429)) {
notify({ level: "error", message: e.detail || t("messages.sendFailed") });
}
},
}); });
const submit = () => { const submit = () => {

View file

@ -64,12 +64,15 @@ function ConversationList({
const items = q.data?.items ?? []; const items = q.data?.items ?? [];
const [previews, setPreviews] = useState<Record<number, string>>({}); const [previews, setPreviews] = useState<Record<number, string>>({});
// Decrypt conversation previews when fresh data arrives. Keyed on dataUpdatedAt (not `items`):
// the `?? []` fallback is a new array each render, which would otherwise spin this effect
// (and setPreviews) every render while the query is loading.
useEffect(() => { useEffect(() => {
if (!ready) return; if (!ready) return;
let cancelled = false; let cancelled = false;
(async () => { (async () => {
const out: Record<number, string> = {}; const out: Record<number, string> = {};
for (const c of items) { for (const c of q.data?.items ?? []) {
const m = c.last_message; const m = c.last_message;
if (m.kind === "user" && m.ciphertext && m.iv) { if (m.kind === "user" && m.ciphertext && m.iv) {
try { try {
@ -85,7 +88,8 @@ function ConversationList({
return () => { return () => {
cancelled = true; cancelled = true;
}; };
}, [items, ready]); // eslint-disable-next-line react-hooks/exhaustive-deps
}, [q.dataUpdatedAt, ready]);
function previewText(c: Conversation): string { function previewText(c: Conversation): string {
const m = c.last_message; const m = c.last_message;

View file

@ -17,6 +17,7 @@
"encrypted": "Verschlüsselte Nachricht", "encrypted": "Verschlüsselte Nachricht",
"encryptedHint": "Ende-zu-Ende-verschlüsselt", "encryptedHint": "Ende-zu-Ende-verschlüsselt",
"cantDecrypt": "Kann nicht entschlüsselt werden", "cantDecrypt": "Kann nicht entschlüsselt werden",
"sendFailed": "Nachricht konnte nicht gesendet werden. Bitte erneut versuchen.",
"systemReadOnly": "Dies ist eine automatische Nachricht.", "systemReadOnly": "Dies ist eine automatische Nachricht.",
"setupTitle": "Sichere Nachrichten einrichten", "setupTitle": "Sichere Nachrichten einrichten",
"setupBody": "Nachrichten sind Ende-zu-Ende-verschlüsselt. Wähle ein Passwort zum Schutz deines Schlüssels — er verlässt nie dein Gerät, sodass niemand (nicht einmal ein Admin) deine Unterhaltungen lesen kann.", "setupBody": "Nachrichten sind Ende-zu-Ende-verschlüsselt. Wähle ein Passwort zum Schutz deines Schlüssels — er verlässt nie dein Gerät, sodass niemand (nicht einmal ein Admin) deine Unterhaltungen lesen kann.",

View file

@ -17,6 +17,7 @@
"encrypted": "Encrypted message", "encrypted": "Encrypted message",
"encryptedHint": "End-to-end encrypted", "encryptedHint": "End-to-end encrypted",
"cantDecrypt": "Can't decrypt", "cantDecrypt": "Can't decrypt",
"sendFailed": "Couldn't send your message. Try again.",
"systemReadOnly": "This is an automated message.", "systemReadOnly": "This is an automated message.",
"setupTitle": "Set up secure messaging", "setupTitle": "Set up secure messaging",
"setupBody": "Messages are end-to-end encrypted. Choose a passphrase to protect your key — it never leaves your device, so no one (not even an admin) can read your conversations.", "setupBody": "Messages are end-to-end encrypted. Choose a passphrase to protect your key — it never leaves your device, so no one (not even an admin) can read your conversations.",

View file

@ -17,6 +17,7 @@
"encrypted": "Titkosított üzenet", "encrypted": "Titkosított üzenet",
"encryptedHint": "Végpontig titkosítva", "encryptedHint": "Végpontig titkosítva",
"cantDecrypt": "Nem fejthető vissza", "cantDecrypt": "Nem fejthető vissza",
"sendFailed": "Nem sikerült elküldeni az üzenetet. Próbáld újra.",
"systemReadOnly": "Ez egy automatikus üzenet.", "systemReadOnly": "Ez egy automatikus üzenet.",
"setupTitle": "Biztonságos üzenetküldés beállítása", "setupTitle": "Biztonságos üzenetküldés beállítása",
"setupBody": "Az üzenetek végpontig titkosítottak. Válassz egy jelszót a kulcsod védelméhez — sosem hagyja el az eszközödet, így senki (még az admin sem) olvashatja a beszélgetéseidet.", "setupBody": "Az üzenetek végpontig titkosítottak. Válassz egy jelszót a kulcsod védelméhez — sosem hagyja el az eszközödet, így senki (még az admin sem) olvashatja a beszélgetéseidet.",

View file

@ -3,6 +3,7 @@
// when a message arrives) and stays open across page navigation. The dock state (which chats // when a message arrives) and stays open across page navigation. The dock state (which chats
// are open + their minimised state) is persisted per user so it survives a reload. // are open + their minimised state) is persisted per user so it survives a reload.
import { useSyncExternalStore } from "react"; import { useSyncExternalStore } from "react";
import { LS } from "./storage";
export interface DockChat { export interface DockChat {
partnerId: number; partnerId: number;
@ -20,7 +21,7 @@ let userId: number | null = null;
const subs = new Set<() => void>(); const subs = new Set<() => void>();
function storageKey(): string | null { function storageKey(): string | null {
return userId != null ? `siftlode.chatDock.${userId}` : null; return userId != null ? LS.chatDock(userId) : null;
} }
function persist(): void { function persist(): void {

View file

@ -46,12 +46,6 @@ export function isUnlocked(): boolean {
return !!privKey; return !!privKey;
} }
export function lock(): void {
privKey = null;
convKeys.clear();
emitUnlock();
}
// --- IndexedDB (per-device persistence of the unlocked private key) --- // --- IndexedDB (per-device persistence of the unlocked private key) ---
const DB_NAME = "siftlode-e2ee"; const DB_NAME = "siftlode-e2ee";
const STORE = "privkeys"; const STORE = "privkeys";
@ -127,7 +121,11 @@ export async function setup(userId: number, passphrase: string): Promise<KeySetu
const wrapKey = await wrapKeyFrom(passphrase, salt); const wrapKey = await wrapKeyFrom(passphrase, salt);
const pkcs8 = await subtle.exportKey("pkcs8", kp.privateKey); const pkcs8 = await subtle.exportKey("pkcs8", kp.privateKey);
const wrapped = await subtle.encrypt({ name: "AES-GCM", iv: bsrc(wrapIv) }, wrapKey, bsrc(pkcs8)); const wrapped = await subtle.encrypt({ name: "AES-GCM", iv: bsrc(wrapIv) }, wrapKey, bsrc(pkcs8));
const keyCheck = await subtle.encrypt({ name: "AES-GCM", iv: bsrc(wrapIv) }, wrapKey, bsrc(enc.encode("ok"))); // key_check is retained for the upload schema but MUST use its own nonce: reusing wrapIv under
// wrapKey would be GCM nonce-reuse (leaks keystream + enables GHASH forgery). It is no longer a
// verification oracle either — unlock() authenticates the passphrase via the wrapped-key GCM tag.
const checkIv = crypto.getRandomValues(new Uint8Array(12));
const keyCheck = await subtle.encrypt({ name: "AES-GCM", iv: bsrc(checkIv) }, wrapKey, bsrc(enc.encode("ok")));
const spki = await subtle.exportKey("spki", kp.publicKey); const spki = await subtle.exportKey("spki", kp.publicKey);
privKey = await importPrivate(pkcs8); privKey = await importPrivate(pkcs8);
@ -149,8 +147,8 @@ export async function unlock(userId: number, passphrase: string, bundle: MyKeyBu
throw new Error("incomplete key bundle"); throw new Error("incomplete key bundle");
} }
const wrapKey = await wrapKeyFrom(passphrase, ub64(bundle.salt)); const wrapKey = await wrapKeyFrom(passphrase, ub64(bundle.salt));
// Wrong passphrase → AES-GCM auth tag fails here. // Wrong passphrase → the AES-GCM auth tag fails on this decrypt. (No separate key_check oracle:
await subtle.decrypt({ name: "AES-GCM", iv: bsrc(ub64(bundle.wrap_iv)) }, wrapKey, bsrc(ub64(bundle.key_check))); // that was redundant with this tag and formerly shared wrapIv — see setup().)
const pkcs8 = await subtle.decrypt( const pkcs8 = await subtle.decrypt(
{ name: "AES-GCM", iv: bsrc(ub64(bundle.wrap_iv)) }, { name: "AES-GCM", iv: bsrc(ub64(bundle.wrap_iv)) },
wrapKey, wrapKey,