From fb23518e741ca7ef726e1db4b609e59667dd006b Mon Sep 17 00:00:00 2001 From: Mario Fetka Date: Thu, 30 Jul 2026 08:02:58 +0200 Subject: [PATCH] Rollback failed multi-message IMAP copies --- contrib/testing/imap-overquota-check.py | 375 ++++++++++++++++++++++++ src/agents/imap/copy.c | 48 ++- 2 files changed, 419 insertions(+), 4 deletions(-) create mode 100755 contrib/testing/imap-overquota-check.py diff --git a/contrib/testing/imap-overquota-check.py b/contrib/testing/imap-overquota-check.py new file mode 100755 index 0000000..b5669ae --- /dev/null +++ b/contrib/testing/imap-overquota-check.py @@ -0,0 +1,375 @@ +#!/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 RFC 9208 DELETED-STORAGE and hard-quota IMAP operations.""" + +from __future__ import annotations + +import os +import re +import socket +import ssl +import subprocess +import sys +import time + + +HOST = os.environ.get("BONGO_TEST_HOST", "127.0.0.1") +PORT = int(os.environ.get("BONGO_TEST_IMAPS_PORT", "993")) +TIMEOUT = float(os.environ.get("BONGO_TEST_TIMEOUT", "10")) +USERNAME = os.environ.get("BONGO_TEST_USER", "test1") +PASSWORD = os.environ.get("BONGO_TEST_PASSWORD", "") +ADMIN = os.environ.get("BONGO_TEST_ADMIN", "/usr/bin/bongo-admin") +ALLOW_LIVE = os.environ.get("BONGO_ALLOW_LIVE_USER_TEST") == "1" +TAGGED = re.compile(rb"^([A-Za-z0-9]+) (OK|NO|BAD)(?: .*)?\r\n$") +STATUS_RESPONSE = re.compile(rb"^\* STATUS .+ \((.*)\)\r\n$", re.IGNORECASE) +APPEND_UID = re.compile(rb"\[APPENDUID [0-9]+ ([0-9]+)\]", re.IGNORECASE) + + +class IMAPOverQuotaError(RuntimeError): + """Raised when the live hard-quota contract is not met.""" + + +def quote(value: str) -> str: + return '"' + value.replace("\\", "\\\\").replace('"', '\\"') + '"' + + +def run_admin(*arguments: str) -> str: + completed = subprocess.run( + ["sudo", "-n", ADMIN, *arguments], + check=False, + capture_output=True, + text=True, + ) + if completed.returncode: + detail = completed.stderr.strip() or completed.stdout.strip() + raise IMAPOverQuotaError( + f"bongo-admin {' '.join(arguments)} failed: {detail}" + ) + return completed.stdout + + +def read_admin_quota() -> tuple[int, int]: + output = run_admin("user", "quota", USERNAME) + used_match = re.search(r"^Used: ([0-9]+) bytes$", output, re.MULTILINE) + limit_match = re.search( + r"^Quota: (?:(unlimited)|([0-9]+) bytes)$", output, re.MULTILINE + ) + if not used_match or not limit_match: + raise IMAPOverQuotaError( + f"cannot parse bongo-admin quota output: {output!r}" + ) + limit = 0 if limit_match.group(1) else int(limit_match.group(2)) + return int(used_match.group(1)), limit + + +def set_admin_quota(limit: int) -> None: + run_admin( + "user", + "quota", + USERNAME, + "unlimited" if limit == 0 else str(limit), + ) + + +class IMAPConnection: + def __init__(self) -> None: + raw = socket.create_connection((HOST, PORT), timeout=TIMEOUT) + context = ssl.create_default_context() + context.check_hostname = False + context.verify_mode = ssl.CERT_NONE + context.minimum_version = ssl.TLSVersion.TLSv1_2 + self.socket = context.wrap_socket(raw, server_hostname=HOST) + self.socket.settimeout(TIMEOUT) + self.reader = self.socket.makefile("rb") + self.counter = 0 + greeting = self.line() + if not greeting.startswith(b"* OK "): + raise IMAPOverQuotaError( + f"unexpected IMAP greeting: {greeting!r}" + ) + + def close(self) -> None: + try: + self.reader.close() + finally: + self.socket.close() + + def line(self) -> bytes: + line = self.reader.readline(65538) + if not line: + raise IMAPOverQuotaError( + "connection closed while waiting for IMAP response" + ) + if len(line) > 65536 or not line.endswith(b"\r\n"): + raise IMAPOverQuotaError(f"invalid IMAP response line: {line!r}") + return line + + def response( + self, tag: str, expected: tuple[str, ...], lines: list[bytes] + ) -> list[bytes]: + while True: + line = self.line() + match = TAGGED.match(line) + if match is not None and match.group(1).decode("ascii") == tag: + status = match.group(2).decode("ascii") + if status not in expected: + raise IMAPOverQuotaError( + f"{tag} expected {expected}, received {line!r}" + ) + return lines + [line] + lines.append(line) + + def command( + self, command: str, expected: tuple[str, ...] = ("OK",) + ) -> list[bytes]: + self.counter += 1 + tag = f"O{self.counter:04d}" + self.socket.sendall(f"{tag} {command}\r\n".encode("utf-8")) + return self.response(tag, expected, []) + + def append( + self, + mailbox: str, + payload: bytes, + expected: tuple[str, ...] = ("OK",), + ) -> list[bytes]: + self.counter += 1 + tag = f"O{self.counter:04d}" + self.socket.sendall( + f"{tag} APPEND {quote(mailbox)} {{{len(payload)}}}\r\n".encode( + "ascii" + ) + ) + continuation = self.line() + if not continuation.startswith(b"+"): + raise IMAPOverQuotaError( + f"APPEND did not request its literal: {continuation!r}" + ) + self.socket.sendall(payload + b"\r\n") + return self.response(tag, expected, [continuation]) + + +def status_values(lines: list[bytes]) -> dict[str, int]: + for line in lines: + match = STATUS_RESPONSE.match(line) + if match is None: + continue + tokens = match.group(1).decode("ascii").split() + if len(tokens) % 2: + raise IMAPOverQuotaError(f"malformed STATUS response: {line!r}") + try: + return { + tokens[index].upper(): int(tokens[index + 1]) + for index in range(0, len(tokens), 2) + } + except ValueError as error: + raise IMAPOverQuotaError( + f"non-numeric STATUS response: {line!r}" + ) from error + raise IMAPOverQuotaError(f"STATUS response missing from {lines!r}") + + +def append_uid(lines: list[bytes]) -> int: + for line in lines: + match = APPEND_UID.search(line) + if match is not None: + return int(match.group(1)) + raise IMAPOverQuotaError(f"APPENDUID missing from {lines!r}") + + +def require_overquota(lines: list[bytes], operation: str) -> None: + tagged = lines[-1] if lines else b"" + if not re.search(rb" NO \[OVERQUOTA\] ", tagged, re.IGNORECASE): + raise IMAPOverQuotaError( + f"{operation} did not return tagged NO [OVERQUOTA]: {lines!r}" + ) + + +def units(value: int) -> int: + return value // 1024 + (value % 1024 != 0) + + +def message(token: str, number: int, size: int = 8191) -> bytes: + prefix = ( + f"From: imap22@bongo.test\r\n" + f"To: {USERNAME}@bongo.test\r\n" + f"Subject: IMAP-22 quota {token} message {number}\r\n" + f"Message-ID: \r\n" + "\r\n" + ).encode("ascii") + if len(prefix) >= size: + raise IMAPOverQuotaError("configured fixture size is too small") + return prefix + b"Q" * (size - len(prefix)) + + +def require_safe_environment() -> None: + if not ALLOW_LIVE: + raise IMAPOverQuotaError( + "set BONGO_ALLOW_LIVE_USER_TEST=1 for the disposable live account" + ) + if not PASSWORD: + raise IMAPOverQuotaError("BONGO_TEST_PASSWORD must be set") + + +def main() -> int: + require_safe_environment() + baseline, original_limit = read_admin_quota() + token = f"{os.getpid():x}-{time.time_ns():x}" + source = f"Bongo 07 Quota Source {token}" + target = f"Bongo 07 Quota Target {token}" + moved = f"Bongo 07 Quota Moved {token}" + mailboxes: list[str] = [] + client: IMAPConnection | None = None + first = message(token, 1) + second = message(token, 2) + rejected = message(token, 3) + + try: + set_admin_quota(0) + client = IMAPConnection() + client.command(f"LOGIN {quote(USERNAME)} {quote(PASSWORD)}") + for mailbox in (source, target, moved): + client.command(f"CREATE {quote(mailbox)}") + mailboxes.append(mailbox) + first_uid = append_uid(client.append(source, first)) + append_uid(client.append(source, second)) + + used, limit = read_admin_quota() + expected_used = baseline + len(first) + len(second) + if (used, limit) != (expected_used, 0): + raise IMAPOverQuotaError( + f"fixture usage is {used}/{limit}, expected {expected_used}/0" + ) + + client.command(f"SELECT {quote(source)}") + client.command("STORE 1 +FLAGS.SILENT (\\Deleted)") + client.command("UNSELECT") + rev1 = status_values( + client.command( + f"STATUS {quote(source)} (MESSAGES DELETED-STORAGE SIZE)" + ) + ) + if rev1 != { + "MESSAGES": 2, + "DELETED-STORAGE": units(len(first)), + "SIZE": len(first) + len(second), + }: + raise IMAPOverQuotaError( + f"rev1 DELETED-STORAGE is incorrect: {rev1!r}" + ) + client.command("ENABLE IMAP4rev2") + rev2 = status_values( + client.command( + f"STATUS {quote(source)} " + "(MESSAGES DELETED DELETED-STORAGE SIZE)" + ) + ) + if rev2 != { + "MESSAGES": 2, + "DELETED": 1, + "DELETED-STORAGE": units(len(first)), + "SIZE": len(first) + len(second), + }: + raise IMAPOverQuotaError( + f"rev2 DELETED-STORAGE is incorrect: {rev2!r}" + ) + client.command(f"SELECT {quote(source)}") + client.command("STORE 1 -FLAGS.SILENT (\\Deleted)") + + set_admin_quota(expected_used) + overquota_append = client.append(source, rejected, ("NO",)) + require_overquota(overquota_append, "APPEND") + if status_values( + client.command(f"STATUS {quote(source)} (MESSAGES SIZE)") + ) != {"MESSAGES": 2, "SIZE": len(first) + len(second)}: + raise IMAPOverQuotaError("failed APPEND changed the source mailbox") + if read_admin_quota()[0] != expected_used: + raise IMAPOverQuotaError("failed APPEND changed quota usage") + + overquota_uid_copy = client.command( + f"UID COPY {first_uid} {quote(target)}", ("NO",) + ) + require_overquota(overquota_uid_copy, "UID COPY") + if status_values( + client.command(f"STATUS {quote(target)} (MESSAGES SIZE)") + ) != {"MESSAGES": 0, "SIZE": 0}: + raise IMAPOverQuotaError("failed UID COPY changed its target") + + set_admin_quota(expected_used + len(first)) + overquota_multi_copy = client.command( + f"COPY 1:2 {quote(target)}", ("NO",) + ) + require_overquota(overquota_multi_copy, "multi-message COPY") + if status_values( + client.command(f"STATUS {quote(target)} (MESSAGES SIZE)") + ) != {"MESSAGES": 0, "SIZE": 0}: + raise IMAPOverQuotaError( + "failed multi-message COPY left a partial destination" + ) + if read_admin_quota()[0] != expected_used: + raise IMAPOverQuotaError( + "failed multi-message COPY did not roll back quota usage" + ) + + set_admin_quota(expected_used) + client.command(f"UID MOVE {first_uid} {quote(moved)}") + if status_values( + client.command(f"STATUS {quote(source)} (MESSAGES SIZE)") + ) != {"MESSAGES": 1, "SIZE": len(second)}: + raise IMAPOverQuotaError("same-root MOVE did not remove its source") + if status_values( + client.command(f"STATUS {quote(moved)} (MESSAGES SIZE)") + ) != {"MESSAGES": 1, "SIZE": len(first)}: + raise IMAPOverQuotaError("same-root MOVE did not create its target") + if read_admin_quota()[0] != expected_used: + raise IMAPOverQuotaError("same-root MOVE changed quota usage") + client.command("NOOP") + client.command("CLOSE") + + for mailbox in reversed(mailboxes): + client.command(f"DELETE {quote(mailbox)}") + mailboxes.remove(mailbox) + client.command("LOGOUT") + finally: + if client is not None: + try: + client.command("CLOSE", ("OK", "NO", "BAD")) + except (IMAPOverQuotaError, OSError): + pass + for mailbox in reversed(mailboxes): + try: + client.command(f"DELETE {quote(mailbox)}", ("OK", "NO")) + except (IMAPOverQuotaError, OSError): + pass + try: + client.close() + except OSError: + pass + set_admin_quota(original_limit) + + restored_used, restored_limit = read_admin_quota() + if (restored_used, restored_limit) != (baseline, original_limit): + raise IMAPOverQuotaError( + "cleanup did not restore the original state: " + f"{restored_used}/{restored_limit}, expected {baseline}/{original_limit}" + ) + print( + "IMAP-22 PASS " + "deleted-storage=rev1/rev2/exact-rounded " + "append=overquota/no-partial " + "copy=uid-overquota/multi-atomic-rollback " + "move=same-root-at-limit/no-growth " + "session=reusable cleanup=yes quota-restored=yes" + ) + return 0 + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except (OSError, IMAPOverQuotaError) as error: + print(f"IMAP-22 FAIL: {error}", file=sys.stderr) + raise SystemExit(1) diff --git a/src/agents/imap/copy.c b/src/agents/imap/copy.c index 04ae1cc..4434d9a 100644 --- a/src/agents/imap/copy.c +++ b/src/agents/imap/copy.c @@ -30,6 +30,7 @@ typedef struct { uint32_t *source; uint32_t *target; + uint64_t *targetGuid; size_t count; size_t capacity; } CopyUidMapping; @@ -39,6 +40,7 @@ CopyUidMappingFree(CopyUidMapping *mapping) { MemFree(mapping->source); MemFree(mapping->target); + MemFree(mapping->targetGuid); memset(mapping, 0, sizeof(*mapping)); } @@ -51,7 +53,8 @@ CopyUidMappingInitialize(CopyUidMapping *mapping, size_t capacity) return STATUS_MEMORY_ERROR; mapping->source = MemMalloc(capacity * sizeof(*mapping->source)); mapping->target = MemMalloc(capacity * sizeof(*mapping->target)); - if (!mapping->source || !mapping->target) { + mapping->targetGuid = MemMalloc(capacity * sizeof(*mapping->targetGuid)); + if (!mapping->source || !mapping->target || !mapping->targetGuid) { CopyUidMappingFree(mapping); return STATUS_MEMORY_ERROR; } @@ -71,16 +74,41 @@ CopyUidMappingContains(const CopyUidMapping *mapping, uint32_t source) } static long -CopyUidMappingAdd(CopyUidMapping *mapping, uint32_t source, uint32_t target) +CopyUidMappingAdd(CopyUidMapping *mapping, uint32_t source, uint32_t target, + uint64_t targetGuid) { if (CopyUidMappingContains(mapping, source)) return STATUS_CONTINUE; if (mapping->count >= mapping->capacity) return STATUS_MEMORY_ERROR; mapping->source[mapping->count] = source; mapping->target[mapping->count] = target; + mapping->targetGuid[mapping->count] = targetGuid; mapping->count++; return STATUS_CONTINUE; } +/* + * RFC 9051 requires a failed COPY to restore the destination mailbox. Store + * COPY is intentionally a one-message transaction, so remove every target + * committed earlier in this IMAP command if a later message fails. + */ +static long +RollbackCopiedMessages(ImapSession *session, CopyUidMapping *mapping) +{ + while (mapping->count > 0) { + long ccode; + size_t index = mapping->count - 1; + + if (NMAPSendCommandF(session->store.conn, "PURGE %" PRIx64 "\r\n", + mapping->targetGuid[index]) == -1) + return STATUS_NMAP_COMM_ERROR; + ccode = NMAPReadResponse(session->store.conn, NULL, 0, 0); + if (ccode != 1000 && ccode != 4220) + return CheckForNMAPCommError(ccode); + mapping->count--; + } + return STATUS_CONTINUE; +} + __inline static long CopyMessageRangeToTarget(ImapSession *session, MessageInformation *message, unsigned long rangeCount, uint64_t target, @@ -110,7 +138,8 @@ CopyMessageRangeToTarget(ImapSession *session, MessageInformation *message, if (!IMAPUidplusParseCreated(response, &target_guid, &target_uid)) return STATUS_NMAP_PROTOCOL_ERROR; - ccode = CopyUidMappingAdd(mapping, message->uid, target_uid); + ccode = CopyUidMappingAdd(mapping, message->uid, target_uid, + target_guid); if (ccode != STATUS_CONTINUE) return ccode; count--; if (count > 0) { @@ -191,7 +220,8 @@ MoveMessageRangeToTarget(ImapSession *session, MessageInformation *message, &target_uid) || moved_guid != message->guid || source_uid != message->uid) return STATUS_NMAP_PROTOCOL_ERROR; - ccode = CopyUidMappingAdd(mapping, source_uid, target_uid); + ccode = CopyUidMappingAdd(mapping, source_uid, target_uid, + moved_guid); if (ccode != STATUS_CONTINUE) return ccode; count--; if (count > 0) { @@ -452,6 +482,11 @@ ImapCommandCopy(void *param) session->folder.selected.messageCount); if (ccode == STATUS_CONTINUE) ccode = HandleCopy(session, FALSE, &mapping, &targetUidValidity); + if (ccode != STATUS_CONTINUE && mapping.count > 0) { + long rollback = RollbackCopiedMessages(session, &mapping); + + if (rollback != STATUS_CONTINUE) ccode = rollback; + } if (ccode == STATUS_CONTINUE) ccode = SendCopyOk(session, "COPY", &mapping, targetUidValidity); CopyUidMappingFree(&mapping); @@ -472,6 +507,11 @@ ImapCommandUidCopy(void *param) session->folder.selected.messageCount); if (ccode == STATUS_CONTINUE) ccode = HandleCopy(session, TRUE, &mapping, &targetUidValidity); + if (ccode != STATUS_CONTINUE && mapping.count > 0) { + long rollback = RollbackCopiedMessages(session, &mapping); + + if (rollback != STATUS_CONTINUE) ccode = rollback; + } if (ccode == STATUS_CONTINUE) ccode = SendCopyOk(session, "UID COPY", &mapping, targetUidValidity); CopyUidMappingFree(&mapping);