diff --git a/lib/__tests__/email-headers.test.ts b/lib/__tests__/email-headers.test.ts index 874f5e6bd..edf78a8ec 100644 --- a/lib/__tests__/email-headers.test.ts +++ b/lib/__tests__/email-headers.test.ts @@ -333,3 +333,63 @@ describe('extractListHeaders', () => { expect(result.listPost).toBe(''); }); }); + +describe('parseAuthenticationResults - sender-controlled text', () => { + // Stalwart writes dkim (when signed), spf helo, spf mailfrom with the MAIL + // FROM address echoed in a comment and in smtp.mailfrom, iprev, then dmarc. + const stalwart = (mailFrom: string, dmarc: string) => + `mx.victim.example;\r\n\tspf=pass (mx.victim.example: domain of ${mailFrom} designates 203.0.113.9 as permitted sender) smtp.mailfrom=${mailFrom};\r\n\tiprev=pass policy.iprev=203.0.113.9;\r\n\tdmarc=${dmarc} header.from=corp.example policy.dmarc=none`; + + it('reads the honest header', () => { + const result = parseAuthenticationResults([stalwart('bounce@evil.example', 'fail')]); + expect(result.dmarc?.result).toBe('fail'); + expect(result.spf?.domain).toBe('bounce@evil.example'); + expect(result.iprev).toEqual({ result: 'pass', ip: '203.0.113.9' }); + expect(isAuthenticationSpoofed(result)).toBe(true); + }); + + it('ignores a method=result token inside the envelope local part', () => { + const result = parseAuthenticationResults([stalwart('dmarc=pass@evil.example', 'fail')]); + expect(result.dmarc?.result).toBe('fail'); + expect(result.spf?.domain).toBe('dmarc=pass@evil.example'); + expect(isAuthenticationSpoofed(result)).toBe(true); + }); + + it('does not split on a ; inside a quoted local part or a comment', () => { + const header = + 'mx.victim.example; spf=fail (domain of "x; dkim=pass"@evil.example; dmarc=pass) smtp.mailfrom="x; dmarc=pass header.from=corp.example"@evil.example; dmarc=fail header.from=corp.example'; + const result = parseAuthenticationResults(header); + expect(result.dkim).toBeUndefined(); + expect(result.dmarc?.result).toBe('fail'); + expect(result.spf?.result).toBe('fail'); + }); + + it('takes DKIM and DMARC only from the topmost (receiving server) header', () => { + const result = parseAuthenticationResults([ + stalwart('bounce@evil.example', 'none'), + 'forged.example; dkim=pass header.d=corp.example header.s=s1; dmarc=pass header.from=corp.example', + ]); + expect(result.dkim).toBeUndefined(); + expect(result.dmarc?.result).toBe('none'); + }); + + it('lets a lower header escalate SPF to a failure but never to a pass', () => { + const escalated = parseAuthenticationResults([ + 'mx.victim.example; spf=pass smtp.mailfrom=corp.example', + 'relay.example; spf=fail smtp.mailfrom=corp.example', + ]); + expect(escalated.spf?.result).toBe('fail'); + + const notUpgraded = parseAuthenticationResults([ + 'mx.victim.example; spf=softfail smtp.mailfrom=corp.example', + 'forged.example; spf=pass smtp.mailfrom=corp.example', + ]); + expect(notUpgraded.spf?.result).toBe('softfail'); + expect(notUpgraded.spf?.all).toBeUndefined(); + }); + + it('keeps the most severe DMARC verdict within one header', () => { + const result = parseAuthenticationResults('mx.example; dmarc=pass header.from=a.example; dmarc=fail header.from=a.example'); + expect(result.dmarc?.result).toBe('fail'); + }); +}); diff --git a/lib/email-headers.ts b/lib/email-headers.ts index 505046ff1..2128e6336 100644 --- a/lib/email-headers.ts +++ b/lib/email-headers.ts @@ -37,11 +37,107 @@ export function isAuthenticationSpoofed(auth?: AuthenticationResults): boolean { return false; } +interface ResInfo { + method: string; + result: string; + props: Record; +} + /** - * Parse Authentication-Results header to extract SPF, DKIM, DMARC results + * Split one Authentication-Results header into its `;`-separated parts + * (RFC 8601), dropping comments. A `;` inside a quoted string or a comment + * does not split: both can carry sender-chosen text such as the envelope + * address. */ -export function parseAuthenticationResults(header: string): AuthenticationResults { +function splitResinfo(header: string): string[] { + const parts: string[] = []; + let current = ''; + let depth = 0; + let quoted = false; + for (let i = 0; i < header.length; i++) { + const c = header[i]; + if (c === '\\' && (quoted || depth > 0)) { + if (depth === 0) current += c + (header[i + 1] ?? ''); + i++; + continue; + } + if (quoted) { + current += c; + if (c === '"') quoted = false; + continue; + } + if (c === '(') { + depth++; + continue; + } + if (depth > 0) { + if (c === ')' && --depth === 0) current += ' '; + continue; + } + if (c === '"') quoted = true; + if (c === ';') { + parts.push(current); + current = ''; + continue; + } + current += c; + } + parts.push(current); + return parts.map((part) => part.trim()).filter(Boolean); +} + +const METHOD_RE = /^([a-z0-9][a-z0-9_-]*)(?:\/\d+)?\s*=\s*([a-z]+)(?=\s|$)/i; +const PROP_RE = /\s*([^\s=]+)\s*=\s*("(?:[^"\\]|\\.)*"|\S*)/y; + +/** + * Read one resinfo: the method must open the part, so a `dmarc=pass` that + * appears inside a property value (say an envelope local part in + * `smtp.mailfrom=`) is never taken for a result. + */ +function parseResinfo(part: string): ResInfo | null { + const match = METHOD_RE.exec(part); + if (!match) return null; + const props: Record = {}; + const rest = part.slice(match[0].length); + PROP_RE.lastIndex = 0; + let prop: RegExpExecArray | null; + while ((prop = PROP_RE.exec(rest)) !== null && prop[0].length > 0) { + const key = prop[1].toLowerCase(); + let value = prop[2]; + if (value.startsWith('"')) value = value.slice(1, -1).replace(/\\(.)/g, '$1'); + if (!(key in props)) props[key] = value; + } + return { method: match[1].toLowerCase(), result: match[2].toLowerCase(), props }; +} + +function parseResinfos(header: string): ResInfo[] { + return splitResinfo(header) + .map(parseResinfo) + .filter((info): info is ResInfo => info !== null); +} + +const DMARC_SEVERITY: Record = { + fail: 3, + permerror: 2, + temperror: 2, + none: 1, + pass: 0, +}; + +/** + * Parse Authentication-Results headers into SPF, DKIM, DMARC results. + * + * Pass the headers in message order. The topmost one is the receiving + * server's own; anything below it may have been written by the sender, so + * DKIM, DMARC and iprev come from the topmost header only, and the others + * can only escalate SPF to a failure, never supply a pass. + */ +export function parseAuthenticationResults(headers: string | readonly string[]): AuthenticationResults { const results: AuthenticationResults = {}; + const list = typeof headers === 'string' ? [headers] : headers; + const perHeader = list.map(parseResinfos); + const own = perHeader[0] ?? []; + const foreign = perHeader.slice(1).flat(); type DkimResult = 'pass' | 'fail' | 'policy' | 'neutral' | 'temperror' | 'permerror'; type DmarcResult = 'pass' | 'fail' | 'none'; @@ -51,23 +147,27 @@ export function parseAuthenticationResults(header: string): AuthenticationResult // SPF result when the server evaluates multiple identities (HELO and MAIL // FROM). Collect them all so a hard fail on any identity isn't softened to // an ambiguous state recorded for another one. - const spfRegex = /spf=(\w+)(?:\s+\([^)]*\))?(?:\s+smtp\.(mailfrom|helo)=([^\s;]+))?/g; - const spfResults: SpfEntry[] = []; - let spfM: RegExpExecArray | null; - while ((spfM = spfRegex.exec(header)) !== null) { - spfResults.push({ - result: spfM[1] as SpfResult, - identity: spfM[2] as SpfEntry['identity'], - domain: spfM[3], - }); - } + const severity = (r: string) => SPF_SEVERITY[r as SpfResult] ?? -1; + const isFailure = (r: string) => severity(r) >= SPF_SEVERITY.temperror; + const toSpfEntry = (info: ResInfo): SpfEntry => { + const identity = info.props['smtp.mailfrom'] !== undefined + ? 'mailfrom' + : info.props['smtp.helo'] !== undefined ? 'helo' : undefined; + return { + result: info.result as SpfResult, + identity: identity as SpfEntry['identity'], + domain: identity ? info.props[`smtp.${identity}`] || undefined : undefined, + }; + }; + const spfResults: SpfEntry[] = [ + ...own.filter((info) => info.method === 'spf').map(toSpfEntry), + ...foreign.filter((info) => info.method === 'spf').map(toSpfEntry).filter((e) => isFailure(e.result)), + ]; if (spfResults.length > 0) { - const severity = (r: string) => SPF_SEVERITY[r as SpfResult] ?? -1; // MAIL FROM is the primary SPF identity. Another identity (HELO) may only // escalate the headline to a genuine failure state — a HELO `none` or // `neutral` must not downgrade a MAIL FROM `pass`, since most senders // publish no SPF record for their EHLO hostname. - const isFailure = (r: string) => severity(r) >= SPF_SEVERITY.temperror; let primary = spfResults.find((e) => e.identity === 'mailfrom') ?? spfResults[0]; for (const cur of spfResults) { @@ -82,32 +182,36 @@ export function parseAuthenticationResults(header: string): AuthenticationResult }; } - // Parse DKIM - const dkimMatch = header.match(/dkim=(\w+)(?:\s+header\.d=([^\s]+))?(?:\s+header\.s=([^\s]+))?/); - if (dkimMatch) { + const dkim = own.find((info) => info.method === 'dkim'); + if (dkim) { results.dkim = { - result: dkimMatch[1] as DkimResult, - domain: dkimMatch[2], - selector: dkimMatch[3] + result: dkim.result as DkimResult, + domain: dkim.props['header.d'], + selector: dkim.props['header.s'], }; } - // Parse DMARC - const dmarcMatch = header.match(/dmarc=(\w+)(?:\s+header\.from=([^\s]+))?(?:\s+policy\.dmarc=(\w+))?/); - if (dmarcMatch) { + // One DMARC verdict per message; should a header carry several, the most + // severe stands. + const dmarc = own + .filter((info) => info.method === 'dmarc') + .reduce( + (worst, info) => (!worst || (DMARC_SEVERITY[info.result] ?? -1) > (DMARC_SEVERITY[worst.result] ?? -1) ? info : worst), + undefined, + ); + if (dmarc) { results.dmarc = { - result: dmarcMatch[1] as DmarcResult, - domain: dmarcMatch[2], - policy: dmarcMatch[3] as DmarcPolicy | undefined + result: dmarc.result as DmarcResult, + domain: dmarc.props['header.from'], + policy: dmarc.props['policy.dmarc'] as DmarcPolicy | undefined, }; } - // Parse IP reverse lookup - const iprevMatch = header.match(/iprev=(\w+)(?:\s+policy\.iprev=([\d.]+))?/); - if (iprevMatch) { + const iprev = own.find((info) => info.method === 'iprev'); + if (iprev) { results.iprev = { - result: iprevMatch[1] as 'pass' | 'fail', - ip: iprevMatch[2] + result: iprev.result as 'pass' | 'fail', + ip: iprev.props['policy.iprev'], }; } diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index bb9f975b1..35d5e70cf 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -2261,10 +2261,9 @@ export class JMAPClient implements IJMAPClient { const authResultsHeader = headersRecord['Authentication-Results']; if (authResultsHeader) { - // Multiple Authentication-Results headers (or multiple SPF identities in - // one header) must all be considered so the most severe result wins. - const value = Array.isArray(authResultsHeader) ? authResultsHeader.join('; ') : authResultsHeader; - email.authenticationResults = parseAuthenticationResults(value); + // Keep the headers apart and in message order: the topmost one is the + // receiving server's own, the rest may be forged by the sender. + email.authenticationResults = parseAuthenticationResults(authResultsHeader); } for (const headerName of ['X-Spam-Score', 'X-Spam-Status', 'X-Spam-Result', 'X-Rspamd-Score']) {