From 90c1176f93def1671494d2a2f7015ed0ccb44669 Mon Sep 17 00:00:00 2001 From: Bernd Rodler Date: Tue, 4 Aug 2026 12:35:08 +0200 Subject: [PATCH] fix(smime): banner slot can silently miss a resolved signature Found live, not from a test: sent a genuinely signed+encrypted message through the real composer, opened it in Sent, and the banner showed only "Encrypted message" - no signature row at all, despite both Sign and Encrypt having been checked and the body decrypting correctly. Root cause is a race, not a crypto bug. onRenderEmailBody (which fetches the blob, decrypts, verifies the inner signature, and persists the full status) and EmailBanner (a separate plugin UI slot) mount independently. The banner read persisted status exactly once, in a useEffect keyed only on email.id. If that read fired before the async decrypt+verify pipeline finished writing, the banner fell back to a header-derived guess: it can see from the OUTER envelope's Content-Type that a message is encrypted, but has no way to know it is ALSO signed, since that only becomes knowable after decryption completes. This is more than cosmetic. The same race could just as easily hide an INVALID signature - a tampered message or wrong signer - behind the generic "Encrypted message" banner, purely because of timing, with no indication anything needs attention. Fix: track whether the initial read came from a real persisted value or from the header-only fallback. Only in the fallback case, poll briefly (150ms x 20 = 3s) for the real result to land - the same pattern unlockNow already uses after a manual key unlock, generalized to the initial mount. Once persisted state exists, stop. Verified in the real browser: re-sent and re-opened the same signed+ encrypted Sent message after this fix, banner now shows both rows - "Decrypted" and "Valid signature by bernd.rodler@sandbox.vnc.de - self-signed" (amber, correctly, since the spike cert is self-signed and fix 1's selfSigned flag is doing its job). Two source assertions added to verify-fixes.mjs. 51 unit assertions, 28 round-trip assertions, all passing. Co-Authored-By: Claude Opus 4.8 --- vnc/plugins/smime/src/index.js | 20 ++++++++++++++++++++ vnc/plugins/smime/verify-fixes.mjs | 8 ++++++++ 2 files changed, 28 insertions(+) diff --git a/vnc/plugins/smime/src/index.js b/vnc/plugins/smime/src/index.js index 04b14776..ed755646 100644 --- a/vnc/plugins/smime/src/index.js +++ b/vnc/plugins/smime/src/index.js @@ -723,6 +723,7 @@ function EmailBanner(props) { (async () => { if (!email || !email.id) { setLoaded(true); return; } let s = await host.storage.get(VERIFY_PREFIX + email.id); + const fromPersisted = !!s; if (!s) { // No render-hook result yet — best-effort detect from headers/source. const ct = email.headers && (email.headers['Content-Type'] || email.headers['content-type']); @@ -732,6 +733,25 @@ function EmailBanner(props) { else if (det.type === 'detached-sig') s = { isSigned: true, unsupportedReason: 'detached signature' }; } if (alive) { setStatus(s || null); setLoaded(true); } + + // VNC: onRenderEmailBody runs concurrently with this component and does + // real async work (fetch blob, decrypt, verify the inner signature) + // before it persists the full status. If storage had nothing yet, what we + // just showed is a header-derived guess — for an encrypted message that + // guess cannot know whether it's ALSO signed, since that only becomes + // knowable after decryption. Without this poll, a signed+encrypted + // message can permanently show "Encrypted message" with no signature + // row at all, and — worse — an INVALID signature could go undetected on + // screen simply because this component read storage a moment too early. + // Once a real persisted value exists, stop. + if (!fromPersisted && s && s.isEncrypted) { + for (let i = 0; i < 20 && alive; i++) { + await new Promise((resolve) => setTimeout(resolve, 150)); + let next = null; + try { next = await host.storage.get(VERIFY_PREFIX + email.id); } catch { /* ignore */ } + if (next) { if (alive) setStatus(next); break; } + } + } })(); return () => { alive = false; }; }, [email && email.id]); diff --git a/vnc/plugins/smime/verify-fixes.mjs b/vnc/plugins/smime/verify-fixes.mjs index c987831f..82cb4b16 100644 --- a/vnc/plugins/smime/verify-fixes.mjs +++ b/vnc/plugins/smime/verify-fixes.mjs @@ -150,5 +150,13 @@ check('dead lockOnLogout setting removed from manifest', check('auth:observe still declared (B-09 safety)', mani.permissions.includes('auth:observe'), true); +// banner race fix (found live, 2026-08-04): the initial mount read must poll +// when it fell back to a header-only guess, so a signature that resolves a +// moment later (or an INVALID one) isn't silently missed on screen. +check('banner distinguishes a persisted read from the header fallback', + /const fromPersisted = !!s;/.test(idx), true); +check('banner polls after a fallback-only encrypted read', + /if \(!fromPersisted && s && s\.isEncrypted\)/.test(idx), true); + console.log(`\n${fail === 0 ? 'ALL PASS' : 'FAILURES'} — ${pass} passed, ${fail} failed\n`); process.exit(fail === 0 ? 0 : 1);