smtp: align BDAT error recovery with Postfix

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 <sender@example.test>\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:<sender@example.test>", 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)

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,