From 8fcff3fa4b42c8eabcc0d124b1f18ebd4a08c812 Mon Sep 17 00:00:00 2001 From: Mario Fetka Date: Mon, 27 Jul 2026 19:37:20 +0200 Subject: [PATCH] smtp: align BDAT error recovery with Postfix --- contrib/testing/smtp-bdat-check.py | 228 +++++++++++++++++++++++++++++ src/agents/smtp/smtpd.c | 47 +++++- 2 files changed, 273 insertions(+), 2 deletions(-) create mode 100755 contrib/testing/smtp-bdat-check.py diff --git a/contrib/testing/smtp-bdat-check.py b/contrib/testing/smtp-bdat-check.py new file mode 100755 index 0000000..be1c2ec --- /dev/null +++ b/contrib/testing/smtp-bdat-check.py @@ -0,0 +1,228 @@ +#!/usr/bin/env python3 +# This program is free software, licensed under the terms of the GNU GPL. +# See the Bongo COPYING file for full details. +# Copyright (c) 2026 Bongo Project contributors + +"""Verify Postfix-aligned SMTP CHUNKING/BDAT behavior.""" + +from __future__ import annotations + +import importlib.util +import os +from pathlib import Path +import socket +import sys +import time + + +TOKEN = f"smtp15-{os.getpid()}-{int(time.time())}" +TOKEN_PREFIX = b"smtp15-" +TIMEOUT = float(os.environ.get("BONGO_TEST_TIMEOUT", "30")) +ALLOW_LIVE = os.environ.get("BONGO_ALLOW_LIVE_SMTP_TEST") == "1" + + +class SMTP15Error(RuntimeError): + """Raised when SMTP CHUNKING loses data or protocol synchronization.""" + + +def load_helpers(): + path = Path(__file__).with_name("smtp-pipelining-check.py") + specification = importlib.util.spec_from_file_location( + "bongo_smtp15_helpers", path + ) + if specification is None or specification.loader is None: + raise SMTP15Error(f"cannot load SMTP test helpers from {path}") + module = importlib.util.module_from_spec(specification) + sys.modules[specification.name] = module + specification.loader.exec_module(module) + module.TOKEN_PREFIX = TOKEN_PREFIX + return module + + +HELPERS = load_helpers() + + +def message(label: str) -> bytes: + marker = f"{TOKEN}-{label}" + return ( + f"From: SMTP-15 \r\n" + f"To: {HELPERS.USER}@{HELPERS.DOMAIN}\r\n" + f"Subject: SMTP-15 BDAT {marker}\r\n" + f"Message-ID: <{marker}@{HELPERS.DOMAIN}>\r\n" + f"X-Bongo-Test: {marker}\r\n" + "\r\n" + f"Chunk boundary fixture {marker}.\r\n" + ".BDAT has no SMTP dot transparency.\r\n" + ).encode("ascii") + + +def envelope(client) -> list[bytes]: + capabilities = client.command("EHLO smtp15-client.example", 250) + client.command("MAIL FROM:", 250) + client.command( + f"RCPT TO:<{HELPERS.USER}@{HELPERS.DOMAIN}>", 250 + ) + return capabilities + + +def bdat(client, payload: bytes, last: bool, expected: int) -> list[bytes]: + suffix = " LAST" if last else "" + client.socket.sendall( + f"BDAT {len(payload)}{suffix}\r\n".encode("ascii") + payload + ) + return client.require(expected, f"BDAT {len(payload)}{suffix}") + + +def wait_for_messages(store, expected: dict[bytes, bytes]) -> list[str]: + deadline = time.monotonic() + TIMEOUT + while time.monotonic() < deadline: + identifiers = HELPERS.store_token_documents(store) + if len(identifiers) > len(expected): + raise SMTP15Error( + f"BDAT messages delivered more than once: {identifiers!r}" + ) + if len(identifiers) == len(expected): + found: set[bytes] = set() + for identifier in identifiers: + stored = store.Read(identifier) + for marker, submitted in expected.items(): + if marker in stored: + if not stored.endswith(submitted): + raise SMTP15Error( + f"{marker!r} was not stored byte-exactly" + ) + found.add(marker) + break + if found == set(expected): + return identifiers + time.sleep(0.25) + raise SMTP15Error("BDAT messages did not reach the Store") + + +def test_bad_order_is_bounded() -> None: + client = HELPERS.SMTPConnection() + try: + client.require(220, "bad-order greeting") + capabilities = client.command("EHLO smtp15-order.example", 250) + if b"CHUNKING" not in capabilities: + raise SMTP15Error("EHLO did not advertise CHUNKING") + if b"BINARYMIME" in capabilities: + raise SMTP15Error("EHLO incorrectly advertised BINARYMIME") + bdat(client, b"ABCD", False, 503) + client.command("NOOP", 250) + client.command("QUIT", 221) + finally: + client.close() + + +def test_malformed_closes() -> None: + client = HELPERS.SMTPConnection() + try: + client.require(220, "malformed greeting") + envelope(client) + client.socket.sendall(b"BDAT not-a-size\r\nNOOP\r\n") + client.require(521, "malformed BDAT") + client.socket.settimeout(2.0) + try: + trailing = client.reader.readline(1) + except socket.timeout as error: + raise SMTP15Error( + "malformed BDAT did not close the connection" + ) from error + if trailing: + raise SMTP15Error( + f"server parsed bytes after malformed BDAT: {trailing!r}" + ) + finally: + client.close() + + +def test_null_chunk_discard() -> None: + client = HELPERS.SMTPConnection() + try: + client.require(220, "null-chunk greeting") + envelope(client) + bdat(client, b"", False, 551) + bdat(client, b"DROP", False, 551) + bdat(client, b"", True, 551) + client.command("NOOP", 250) + client.command("QUIT", 221) + finally: + client.close() + + +def test_delivery() -> dict[bytes, bytes]: + multi = message("multi") + zero_last = message("zero-last") + + client = HELPERS.SMTPConnection() + try: + client.require(220, "multi-chunk greeting") + envelope(client) + boundaries = (17, 73) + bdat(client, multi[: boundaries[0]], False, 250) + bdat( + client, + multi[boundaries[0] : boundaries[1]], + False, + 250, + ) + bdat(client, multi[boundaries[1] :], True, 250) + + envelope(client) + bdat(client, zero_last, False, 250) + bdat(client, b"", True, 250) + client.command("QUIT", 221) + finally: + client.close() + + return { + f"{TOKEN}-multi".encode("ascii"): multi, + f"{TOKEN}-zero-last".encode("ascii"): zero_last, + } + + +def main() -> int: + if not ALLOW_LIVE: + raise SMTP15Error( + "set BONGO_ALLOW_LIVE_SMTP_TEST=1 for the live delivery test" + ) + if not HELPERS.PASSWORD: + raise SMTP15Error("BONGO_TEST_PASSWORD is required") + + store = HELPERS.open_store() + try: + HELPERS.cleanup(store) + test_bad_order_is_bounded() + test_malformed_closes() + test_null_chunk_discard() + expected = test_delivery() + identifiers = wait_for_messages(store, expected) + for identifier in identifiers: + store.Delete(identifier) + HELPERS.cleanup(store) + if HELPERS.store_token_documents(store) or HELPERS.queue_token_ids(): + raise SMTP15Error("test content remains after cleanup") + finally: + try: + HELPERS.cleanup(store) + finally: + store.Quit() + + print( + "SMTP-15 PASS advertised=chunking/no-binarymime " + "bad-order=payload-drained/resynchronized " + "malformed=521/connection-closed " + "null-nonlast=551/discard-until-last " + "multi-chunk=byte-exact zero-last=accepted " + "duplicate=no queue-clean=yes store-clean=yes" + ) + return 0 + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except (OSError, SMTP15Error, ValueError) as error: + print(f"SMTP-15 FAIL: {type(error).__name__}: {error}") + raise SystemExit(1) diff --git a/src/agents/smtp/smtpd.c b/src/agents/smtp/smtpd.c index eba6167..40b8672 100644 --- a/src/agents/smtp/smtpd.c +++ b/src/agents/smtp/smtpd.c @@ -289,6 +289,7 @@ typedef struct BOOL IsEHLO; /* used for RFC 3848 */ BOOL HasSPFResult; BOOL NMAPMessageWrite; + BOOL BdatDiscard; FILE *BdatMessage; uint64_t BdatLength; BongoSPFResponse SPFResult; @@ -902,6 +903,7 @@ SMTPClearTransaction(ConnectionStruct *client, unsigned long next_state) client->RcptCommandCount = 0; client->MsgFlags = MSG_FLAG_ENCODING_NONE; client->HasSPFResult = FALSE; + client->BdatDiscard = FALSE; memset(&client->SPFResult, 0, sizeof(client->SPFResult)); if (client->BdatMessage != NULL) { fclose(client->BdatMessage); @@ -919,7 +921,8 @@ SMTPResetTransaction(ConnectionStruct *client, unsigned long next_state) { char reply[BUFSIZE + 1]; - if (client->State >= STATE_FROM && client->State <= STATE_BDAT) { + if (client->State >= STATE_FROM && client->State <= STATE_BDAT && + !client->BdatDiscard) { if (NMAPSendCommand(client->nmap.conn, "QABRT\r\n", 7) < 0 || NMAPReadResponse(client->nmap.conn, reply, sizeof(reply), TRUE) != 1000) return FALSE; @@ -973,7 +976,8 @@ EndClientConnection (ConnectionStruct * Client) * closing it lets the queue discard the incomplete entry. */ FreeInternalConnection(Client->nmap); } else if (Client->Flags >= STATE_FROM && - Client->Flags <= STATE_BDAT) { + Client->Flags <= STATE_BDAT && + !Client->BdatDiscard) { if (NMAPSendCommand(Client->nmap.conn, "QABRT\r\n", 7) >= 0 && NMAPReadResponse(Client->nmap.conn, Reply, sizeof(Reply), TRUE) == 1000) { @@ -2189,6 +2193,26 @@ HandleConnection (void *param) ConnFlush(Client->client.conn); return EndClientConnection(Client); } + if (Client->BdatDiscard) { + if (ConnDiscard64(Client->client.conn, ChunkSize) < 0) + return EndClientConnection(Client); + if (LastChunk) { + SMTPClearTransaction( + Client, + IsAuthed ? STATE_AUTH : STATE_HELO); + } + count = snprintf( + Answer, sizeof(Answer), + "551 5.0.0 Discarded %" PRIu64 + " bytes after earlier BDAT error\r\n", + ChunkSize); + if (count < 0 || + (size_t)count >= sizeof(Answer)) + return EndClientConnection(Client); + ConnWrite(Client->client.conn, Answer, + (size_t)count); + break; + } if (!Client->IsEHLO || (Client->State != STATE_TO && Client->State != STATE_BDAT)) { @@ -2203,6 +2227,21 @@ HandleConnection (void *param) MSG503BADORDER_LEN); break; } + if (ChunkSize == 0U && !LastChunk) { + if (!SMTPResetTransaction( + Client, + IsAuthed ? STATE_AUTH : STATE_HELO)) + return EndClientConnection(Client); + Client->State = STATE_BDAT; + Client->BdatDiscard = TRUE; + NullSender = FALSE; + TooManyNullSenderRecips = FALSE; + ConnWrite( + Client->client.conn, + "551 5.7.1 Null BDAT request\r\n", + sizeof("551 5.7.1 Null BDAT request\r\n") - 1U); + break; + } if (Client->State == STATE_TO) { Client->BdatMessage = tmpfile(); if (Client->BdatMessage == NULL) { @@ -2233,6 +2272,10 @@ HandleConnection (void *param) if (!SMTPResetTransaction( Client, IsAuthed ? STATE_AUTH : STATE_HELO)) return EndClientConnection(Client); + if (!LastChunk) { + Client->State = STATE_BDAT; + Client->BdatDiscard = TRUE; + } NullSender = FALSE; TooManyNullSenderRecips = FALSE; ConnWrite(Client->client.conn, MSG552MSGTOOBIG,