Files
SRCmail/vnc/plugins/smime/verify-fixes.mjs
T
Bernd RodlerandClaude Opus 4.8 90c1176f93 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 <noreply@anthropic.com>
2026-08-04 12:35:08 +02:00

163 lines
9.5 KiB
JavaScript

// Standalone proof for the two VNC hardening fixes. Reimplements only the
// decision logic under test (no pkijs/DOM needed) so it runs with plain node.
// node vnc/plugins/smime/verify-fixes.mjs
import { readFileSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { dirname, join } from 'node:path';
const here = dirname(fileURLToPath(import.meta.url));
let pass = 0, fail = 0;
const check = (name, got, want) => {
const ok = got === want;
console.log(`${ok ? ' PASS' : ' FAIL'} ${name}${ok ? '' : ` (got ${JSON.stringify(got)}, want ${JSON.stringify(want)})`}`);
ok ? pass++ : fail++;
};
// ── Fix 1: auto-import gate ─────────────────────────────────────────
// Mirrors the guard order in index.js maybeAutoImportSigner.
function wouldImport(status, autoImport = true) {
if (autoImport === false) return false;
const cert = status && status.signerCert;
if (!cert || !status.signatureValid || !cert.email) return false;
if (status.signerEmailMatch !== true) return false;
if (status.selfSigned) return false;
return true;
}
const cert = { email: 'a@b.com', fingerprint: 'ff' };
console.log('\nFix 1 — certificate auto-import gate');
check('CA-signed, address matches -> import',
wouldImport({ signerCert: cert, signatureValid: true, signerEmailMatch: true, selfSigned: false }), true);
check('THE ATTACK: self-signed, address matches -> REFUSE',
wouldImport({ signerCert: cert, signatureValid: true, signerEmailMatch: true, selfSigned: true }), false);
check('address mismatch -> REFUSE',
wouldImport({ signerCert: cert, signatureValid: true, signerEmailMatch: false, selfSigned: false }), false);
check('signerEmailMatch undefined (no From) -> REFUSE (fail closed)',
wouldImport({ signerCert: cert, signatureValid: true, signerEmailMatch: undefined, selfSigned: false }), false);
check('invalid signature -> REFUSE',
wouldImport({ signerCert: cert, signatureValid: false, signerEmailMatch: true, selfSigned: false }), false);
check('setting off -> REFUSE',
wouldImport({ signerCert: cert, signatureValid: true, signerEmailMatch: true, selfSigned: false }, false), false);
// ── Fix 2 (finding 3): CRLF stripping ───────────────────────────────
function stripCrlf(value) {
return String(value).replace(/[\r\n]+[ \t]*/g, ' ');
}
console.log('\nFinding 3 — CRLF header sanitisation');
check('BCC injection via display name',
stripCrlf('Evil\r\nBcc: attacker@evil.com'), 'Evil Bcc: attacker@evil.com');
check('bare LF', stripCrlf('a\nb'), 'a b');
check('bare CR', stripCrlf('a\rb'), 'a b');
check('folded continuation collapsed', stripCrlf('a\r\n\tb'), 'a b');
check('multiple injected headers',
stripCrlf('x\r\nBcc: a@b.c\r\nReply-To: d@e.f'), 'x Bcc: a@b.c Reply-To: d@e.f');
check('clean value untouched', stripCrlf('Normal Subject'), 'Normal Subject');
check('non-ASCII untouched', stripCrlf('Grüße büro'), 'Grüße büro');
// ── Finding 2: content-encryption allowlist ─────────────────────────
const ALLOW = new Map([
['2.16.840.1.101.3.4.1.2', { name: 'AES-128-CBC', authenticated: false }],
['2.16.840.1.101.3.4.1.22', { name: 'AES-192-CBC', authenticated: false }],
['2.16.840.1.101.3.4.1.42', { name: 'AES-256-CBC', authenticated: false }],
['2.16.840.1.101.3.4.1.6', { name: 'AES-128-GCM', authenticated: true }],
['2.16.840.1.101.3.4.1.26', { name: 'AES-192-GCM', authenticated: true }],
['2.16.840.1.101.3.4.1.46', { name: 'AES-256-GCM', authenticated: true }],
]);
const accepts = (oid) => ALLOW.has(oid);
const authed = (oid) => ALLOW.get(oid)?.authenticated ?? null;
console.log('\nFinding 2 — content-encryption allowlist');
check('AES-256-GCM accepted', accepts('2.16.840.1.101.3.4.1.46'), true);
check('AES-256-GCM is authenticated', authed('2.16.840.1.101.3.4.1.46'), true);
check('AES-128-CBC accepted (RFC 5751 interop)', accepts('2.16.840.1.101.3.4.1.2'), true);
check('AES-128-CBC NOT authenticated', authed('2.16.840.1.101.3.4.1.2'), false);
check('3DES-CBC REFUSED', accepts('1.2.840.113549.3.7'), false);
check('DES-CBC REFUSED', accepts('1.3.14.3.2.7'), false);
check('RC2-CBC REFUSED', accepts('1.2.840.113549.3.2'), false);
check('unknown OID REFUSED', accepts('1.2.3.4.5'), false);
// HTML suppression decision (EFAIL mitigation)
const suppress = (contentAuthenticated, optOut = false) => !contentAuthenticated && !optOut;
check('GCM -> HTML rendered', suppress(true), false);
check('CBC -> HTML suppressed by default', suppress(false), true);
check('CBC + explicit opt-out -> HTML rendered', suppress(false, true), false);
// ── Finding 5: parser resource limits ───────────────────────────────
const MAX_DEPTH = 20, MAX_PARTS = 500;
// Mirrors the bounded recursion in parseEntity: past MAX_DEPTH a multipart is
// treated as a leaf; past MAX_PARTS siblings are dropped.
function walk(depth, budget) {
if (depth >= MAX_DEPTH) return { depth, recursed: false };
if (budget.parts >= MAX_PARTS) return { depth, recursed: false };
budget.parts += 1;
return walk(depth + 1, budget);
}
console.log('\nFinding 5 — parser resource limits');
check('recursion stops at MAX_DEPTH', walk(0, { parts: 0 }).depth, MAX_DEPTH);
check('part budget stops further recursion', walk(0, { parts: MAX_PARTS }).recursed, false);
check('deep-but-legal nesting still reaches the cap', walk(16, { parts: 0 }).depth, MAX_DEPTH);
// ── Source assertions: guard against silent regression ──────────────
console.log('\nSource assertions');
const idx = readFileSync(join(here, 'src/index.js'), 'utf8');
const mb = readFileSync(join(here, 'src/mime-builder.js'), 'utf8');
check('index.js checks signerEmailMatch !== true', idx.includes('status.signerEmailMatch !== true'), true);
check('index.js checks selfSigned', /if \(status\.selfSigned\)/.test(idx), true);
check('formatHeader sanitises its value', /function formatHeader\(name, rawValue\)[\s\S]{0,80}stripCrlf\(rawValue\)/.test(mb), true);
check('attachment Content-Type sanitised', mb.includes('stripCrlf(att.contentType)'), true);
check('Content-ID sanitised', mb.includes('stripCrlf(att.cid)'), true);
// Every header assembled outside formatHeader must use a literal or a sanitised value.
const bypass = [...mb.matchAll(/lines\.push\(`([A-Za-z-]+): ([^`]*)`\)/g)]
.filter(([, , v]) => /\$\{/.test(v) && !/stripCrlf|encodeHeaderValue|boundary|altBoundary|disposition/.test(v));
check('no unsanitised interpolated headers remain', bypass.length, 0);
if (bypass.length) bypass.forEach(([m]) => console.log(' >>', m));
const dec = readFileSync(join(here, 'src/smime-decrypt.js'), 'utf8');
check('allowlist gate runs before any key use',
dec.indexOf('checkContentEncryption(envelopedData)') < dec.indexOf('unlockedKeys.get'), true);
check('decrypt refuses non-allowlisted algorithms', /Refusing to decrypt/.test(dec), true);
check('no legacy CBC OID appears in the decrypt allowlist',
/1\.2\.840\.113549\.3\.7|1\.3\.14\.3\.2\.7|1\.2\.840\.113549\.3\.2/.test(dec), false);
check('normal decrypt path uses the NATIVE engine',
/if \(!useLiner\)[\s\S]{0,120}nativeEngine\(\)/.test(dec), true);
check('liner engine reachable only via useLiner',
(dec.match(/getLinerCryptoEngine\(\)/g) || []).length, 1);
check('index.js suppresses HTML for unauthenticated content',
idx.includes('suppressHtml') && idx.includes('result.contentAuthenticated'), true);
// finding 5
const mp = readFileSync(join(here, 'src/mime-parse.js'), 'utf8');
const det = readFileSync(join(here, 'src/smime-detect.js'), 'utf8');
check('mime-parse caps depth/parts/bytes',
/MAX_DEPTH/.test(mp) && /MAX_PARTS/.test(mp) && /MAX_BYTES/.test(mp), true);
check('parseEntity threads depth + budget',
/function parseEntity\(raw, depth = 0, budget/.test(mp), true);
check('parseEntity guards on depth before recursing',
/params\.boundary && depth < MAX_DEPTH/.test(mp), true);
check('no unbounded .map(parseEntity) left', /\.map\(parseEntity\)/.test(mp), false);
check('both bodyStructure walkers are depth-capped',
(det.match(/depth < MAX_WALK_DEPTH/g) || []).length, 2);
// finding 4 hardening
check('lockOnLogout opt-out removed from wipe paths',
/settings\(\)\.lockOnLogout === false\) return/.test(idx), false);
check('exit wipe registered (pagehide + beforeunload)',
idx.includes("addEventListener('pagehide'") && idx.includes("addEventListener('beforeunload'"), true);
check('boot wipe still present', /clearSessionKeys\(\)/.test(idx), true);
const mani = JSON.parse(readFileSync(join(here, 'manifest.json'), 'utf8'));
check('dead lockOnLogout setting removed from manifest',
'lockOnLogout' in mani.settingsSchema, false);
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);