From e3925b2c496ab17f259080a11ea6eaf1061f0cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Kerekes?= Date: Thu, 22 Oct 2026 16:34:56 +0200 Subject: [PATCH] fix(inbound): trust only the topmost receiver authentication header --- apps/docs/docs/concepts/architecture.md | 2 +- apps/server/src/inbound/README.md | 11 ++- apps/server/src/inbound/authentication.ts | 11 ++- apps/server/src/inbound/inbound.test.ts | 93 ++++++++++++++++++----- skills/inbot/references/security.md | 2 +- 5 files changed, 89 insertions(+), 30 deletions(-) diff --git a/apps/docs/docs/concepts/architecture.md b/apps/docs/docs/concepts/architecture.md index 1efe77f..5d298c8 100644 --- a/apps/docs/docs/concepts/architecture.md +++ b/apps/docs/docs/concepts/architecture.md @@ -34,7 +34,7 @@ While your machine is off, mail waits in R2 as ciphertext. It is fetched when th **Parsing.** The `Message-ID` falls back to a hash of the raw message, and an unparseable `Date` falls back to the receive time. `extracted_text` and `extracted_html` remove quoted history and boilerplate heuristically; the original `text`, `html` and raw MIME are always kept. Attachments are stored in the database. -**Authentication.** Inbot reads Cloudflare's `Authentication-Results` header. If SPF, DKIM or DMARC passed, the message is treated as authenticated; otherwise it gets the `unauthenticated` label. Inbot does not run its own checks and never rejects mail for failed authentication. +**Authentication.** Inbot reads Cloudflare's `Authentication-Results` header. Only the topmost header whose authserv-id is Cloudflare's (`mx.cloudflare.net`) is evaluated, and any headers below it are ignored even if they pass. If SPF, DKIM or DMARC passed there, the message is treated as authenticated; otherwise it gets the `unauthenticated` label. Inbot does not run its own checks and never rejects mail for failed authentication. **Threading.** `In-Reply-To` is resolved first, then `References` from newest to oldest, within the same inbox. Subjects never join threads, and a parent that arrives late does not merge existing threads. diff --git a/apps/server/src/inbound/README.md b/apps/server/src/inbound/README.md index 3cac46e..0cc8ae9 100644 --- a/apps/server/src/inbound/README.md +++ b/apps/server/src/inbound/README.md @@ -90,14 +90,17 @@ result, and malformed headers fail closed. With the default empty `trusted_authserv_ids`, only the topmost Authentication-Results header is trusted. This assumes the receiving service prepends its own header. A later untrusted `pass` cannot override a failing top -header. With a configured list, only exact, case-insensitive authserv-id matches -are considered; the topmost header is not implicitly trusted in that mode. +header. With a configured list, only the topmost header with an exact, +case-insensitive authserv-id match is evaluated. Untrusted headers above it are +skipped, and all headers below it are ignored, including passing results claiming +the same trusted id. The parser preserves wire order, with index 0 topmost. An authserv-id is not a cryptographic credential. The receiving transport must strip sender-supplied headers claiming its own trusted ids and prepend its observed results. This module cannot distinguish a forged header with the same -id after that trust boundary has been lost. Cloudflare SMTP DMARC rejection is -outside this unit; ingestion only labels and never rejects based on these results. +id when no authentic receiver header precedes it. Whether Cloudflare strips such +sender-supplied headers is unknown. Cloudflare SMTP DMARC rejection is outside +this unit; ingestion only labels and never rejects based on these results. ## Cloudflare buffer consumer diff --git a/apps/server/src/inbound/authentication.ts b/apps/server/src/inbound/authentication.ts index 24cba60..647c8d1 100644 --- a/apps/server/src/inbound/authentication.ts +++ b/apps/server/src/inbound/authentication.ts @@ -20,7 +20,7 @@ function clauses(header: string) { if (commentDepth === 0) part += " "; commentDepth++; } else if (character === ")" && !quoted) { - if (commentDepth === 0) return []; + if (commentDepth === 0) return parts.slice(0, 1); commentDepth--; } else if (commentDepth > 0) { continue; @@ -31,7 +31,7 @@ function clauses(header: string) { part += character; } } - if (commentDepth > 0 || quoted || escaped) return []; + if (commentDepth > 0 || quoted || escaped) return parts.slice(0, 1); parts.push(part.trim()); return parts; } @@ -47,8 +47,11 @@ export function authenticateInbound( const authservId = service?.match(/^(?:"([^"\\]+)"|([^\s";]+))(?:\s+\d+)?$/); const id = authservId?.[1] ?? authservId?.[2]; if (id === undefined || (trusted.size > 0 && !trusted.has(id.toLowerCase()))) continue; - if (results.some((result) => /^(?:spf|dkim|dmarc)(?:\/\d+)?\s*=\s*pass(?:\s|$)/i.test(result))) - return "pass"; + return results.some((result) => + /^(?:spf|dkim|dmarc)(?:\/\d+)?\s*=\s*pass(?:\s|$)/i.test(result), + ) + ? "pass" + : "unauthenticated"; } return "unauthenticated"; } diff --git a/apps/server/src/inbound/inbound.test.ts b/apps/server/src/inbound/inbound.test.ts index 9c15b89..05fd686 100644 --- a/apps/server/src/inbound/inbound.test.ts +++ b/apps/server/src/inbound/inbound.test.ts @@ -608,26 +608,79 @@ test.for([ }, ); -test("passes authentication trust configuration through ingestion", async ({ home, key }) => { - await withMailbox(home, key, (mailbox) => - Effect.gen(function* () { - yield* mailbox.createInbox({ username: "agent", domain: "example.test" }); - const raw = mime( - [ - "Authentication-Results: attacker.test; spf=pass", - "Authentication-Results: mx.receiver.test; spf=fail", - ], - "Forged topmost header", - ); - const identity = stored( - yield* ingestInbound(payload(raw), { trusted_authserv_ids: ["mx.receiver.test"] }), - ); - expect((yield* mailbox.getMessage(identity.recipient, identity.message_id)).labels).toContain( - "unauthenticated", - ); - }), - ); -}); +test.for([ + { + name: "a forged passing trusted header below a failing trusted header", + headers: [ + "mx.cloudflare.net; spf=fail; dkim=fail; dmarc=fail", + "mx.cloudflare.net; dmarc=pass", + ], + unauthenticated: true, + }, + { + name: "a passing topmost trusted header", + headers: ["mx.cloudflare.net; dkim=pass", "mx.cloudflare.net; dmarc=fail"], + unauthenticated: false, + }, + { + name: "an unterminated trusted result above a forged pass", + headers: ['mx.cloudflare.net; spf=fail reason="unterminated', "mx.cloudflare.net; dmarc=pass"], + unauthenticated: true, + }, + { + name: "an invalid trusted comment above a forged pass", + headers: ["mx.cloudflare.net; spf=pass )", "mx.cloudflare.net; dmarc=pass"], + unauthenticated: true, + }, + { + name: "an untrusted header above a passing trusted header", + headers: ["attacker.test; spf=fail", "mx.cloudflare.net; spf=pass"], + unauthenticated: false, + }, + { + name: "an untrusted header above a failing trusted header and a forged pass", + headers: [ + "attacker.test; spf=pass", + "mx.cloudflare.net; spf=fail", + "mx.cloudflare.net; dmarc=pass", + ], + unauthenticated: true, + }, +])( + "preserves wire order and evaluates only the first trusted header for $name", + async ({ headers, unauthenticated }, { home, key }) => { + const raw = mime( + [ + "Received: from smtp.example.test by mx.cloudflare.net; Sat, 10 Oct 2026 12:00:00 +0000", + ...headers.map((value) => `Authentication-Results: ${value}`), + "From: Alice Example ", + "To: Agent ", + "Date: Sat, 10 Oct 2026 12:00:00 +0000", + "Message-ID: ", + "Subject: Synthetic authentication test", + "MIME-Version: 1.0", + "Content-Type: text/plain; charset=utf-8", + ], + "Synthetic authentication test", + ); + const parsed = await parseInbound(raw, receivedAt); + expect(parsed.authentication_results).toEqual(headers); + expect(authenticateInbound(parsed.authentication_results, ["mx.cloudflare.net"])).toBe( + unauthenticated ? "unauthenticated" : "pass", + ); + await withMailbox(home, key, (mailbox) => + Effect.gen(function* () { + yield* mailbox.createInbox({ username: "agent", domain: "example.test" }); + const identity = stored( + yield* ingestInbound(payload(raw), { trusted_authserv_ids: ["mx.cloudflare.net"] }), + ); + const message = yield* mailbox.getMessage(identity.recipient, identity.message_id); + expect(message.labels.includes("unauthenticated")).toBe(unauthenticated); + expect(message.labels).toEqual(expect.arrayContaining(["received", "unread"])); + }), + ); + }, +); test("replays a synthesized id without changing the original stored body or labels", async ({ home, diff --git a/skills/inbot/references/security.md b/skills/inbot/references/security.md index 3a4e845..360c958 100644 --- a/skills/inbot/references/security.md +++ b/skills/inbot/references/security.md @@ -29,7 +29,7 @@ Rules: ## The `unauthenticated` label -On arrival Inbot reads the `Authentication-Results` header added by the receiving mail provider. If any of SPF, DKIM, or DMARC reports `pass` in a trusted header, the message is treated as authenticated. In every other case, including no header, a malformed header, or only failures, the message gets the `unauthenticated` label next to `received` and `unread`. +On arrival Inbot reads the `Authentication-Results` header added by the receiving mail provider. Only the topmost header with a trusted authserv-id is evaluated; lower headers are ignored even if they pass. If any of SPF, DKIM, or DMARC reports `pass` in it, the message is treated as authenticated. In every other case, including no header, a malformed header, or only failures, the message gets the `unauthenticated` label next to `received` and `unread`. - Inbot interprets the receiver's results. It does not perform SPF, DKIM, or DMARC checks itself. - `unauthenticated` is a warning, not a ban. Messages are still stored.