From 4d157aeecb3faeba55c3165c0794a7ffee02f0b4 Mon Sep 17 00:00:00 2001 From: Mario Fetka Date: Mon, 27 Jul 2026 07:05:23 +0200 Subject: [PATCH] Enforce outbound DKIM and SRS policy --- contrib/testing/README.md | 5 +- contrib/testing/smtp-relayhost-check.py | 7 +- docs/mail-authentication.md | 10 ++ docs/release-testing-0.7.md | 2 +- src/agents/smtp/outbound-policy.c | 9 ++ src/agents/smtp/outbound-policy.h | 2 + src/agents/smtp/smtpc.c | 119 ++++++++++++------- src/agents/smtp/smtpc.h | 1 + src/agents/smtp/tests/outbound-policy-test.c | 10 ++ 9 files changed, 121 insertions(+), 44 deletions(-) diff --git a/contrib/testing/README.md b/contrib/testing/README.md index ade1e96..2a40bce 100644 --- a/contrib/testing/README.md +++ b/contrib/testing/README.md @@ -30,7 +30,10 @@ one-off files in `/tmp`: relay hostname on non-standard ports. It proves verified STARTTLS without authentication, verified implicit TLS with optional authentication, a temporary `535` failure, durable Queue retention, credential repair and - explicit retry. The exact SMTP configuration is restored afterward. + explicit retry. Every probe enters through the trusted internal-relay + listener, as a Mailman return message would, and the captured external + message must contain Bongo's DKIM signature. The exact SMTP configuration + is restored afterward. - `smtp-internal-relay-check.py` temporarily lowers the trusted port 26 limits, verifies allowed and denied source addresses plus connection, recipient, message, and byte limits, then restores the exact SMTP diff --git a/contrib/testing/smtp-relayhost-check.py b/contrib/testing/smtp-relayhost-check.py index ad9965d..dfe5864 100755 --- a/contrib/testing/smtp-relayhost-check.py +++ b/contrib/testing/smtp-relayhost-check.py @@ -380,6 +380,10 @@ def perform_test() -> None: SMTP07.replace_smtp_configuration(original) if originally_active: restart_bongo() + # A delivery attempt may race with the first cleanup while the + # temporary relay configuration is still active. Once the + # original service is back, remove any remaining SMTP-09 entry. + cleanup_marked_queue() if fixture_started: SMTP4DEV.stop(FIXTURE_ROOT) PASSWORD_FILE.unlink(missing_ok=True) @@ -388,7 +392,8 @@ def perform_test() -> None: print( "SMTP-09 PASS hostname=localhost ports=12525/12465 " "starttls=no-auth implicit-tls=auth failure=retained recovery=delivered " - "dkim=verified queue-clean=yes config-restored=yes" + "trusted-port26=dkim-signed dkim=verified " + "queue-clean=yes config-restored=yes" ) diff --git a/docs/mail-authentication.md b/docs/mail-authentication.md index 4b87810..6ce67db 100644 --- a/docs/mail-authentication.md +++ b/docs/mail-authentication.md @@ -31,6 +31,16 @@ The relevant `smtp` configuration keys are: * `srs_forward`, `srs_reverse`, `srs_domain`, and `srs_secret_file` control SRS forwarding and bounce reversal. +Every delivery which leaves Bongo for an external SMTP peer is authenticated +before transmission. Local, authenticated, trusted-internal-relay and +Bongo-generated bounce traffic receives a Bongo DKIM signature. This includes +mail returned by Mailman over the restricted internal SMTP listener and mail +sent through a system or per-user relayhost. An unauthenticated incoming +message forwarded to another external domain receives an SRS envelope sender +instead; Bongo defers that delivery if SRS is disabled or unusable rather than +leaking the original sender unchanged. Internal Store delivery and configured +LMTP delivery do not add DKIM or SRS. + The SRS secret and DKIM private keys must not be readable by other users. SRS addresses are envelope addresses such as `SRS0=...@example.org`; SRS is not a message-header format. diff --git a/docs/release-testing-0.7.md b/docs/release-testing-0.7.md index 93b8c2a..0af0b06 100644 --- a/docs/release-testing-0.7.md +++ b/docs/release-testing-0.7.md @@ -136,7 +136,7 @@ inputs and outputs is absent from the ZIP. | SMTP-07 | Internet delivery prefers STARTTLS and follows opportunistic fallback | [PASS](test-evidence/0.7-r1.md#smtp-07) | | | | SMTP-08 | Required TLS and `REQUIRETLS` defer rather than downgrade | [PASS](test-evidence/0.7-r1.md#smtp-08) | | | | SMTP-27 | DANE takes precedence; MTA-STS enforce/testing/cache and MX wildcard policy through complete BIND/Unbound, Technitium/Technitium, and PowerDNS/PowerDNS Recursor DNS stacks | [PASS](test-evidence/0.7-r1.md#smtp-27) | | | -| SMTP-09 | Relayhost hostname, port, TLS, optional authentication, failure, and recovery | | | | +| SMTP-09 | Trusted port-26/Mailman-style return receives DKIM through relayhost hostname, port, TLS, optional authentication, failure, and recovery | | | | | SMTP-10 | Open-relay tests reject unauthenticated/untrusted third-party relay | | | | | SMTP-11 | `SIZE` exact limit, over-limit, absent size, and disk-space responses | | | | | SMTP-12 | `PIPELINING` ordering and error recovery | | | | diff --git a/src/agents/smtp/outbound-policy.c b/src/agents/smtp/outbound-policy.c index 6a2d2a6..d6159d3 100644 --- a/src/agents/smtp/outbound-policy.c +++ b/src/agents/smtp/outbound-policy.c @@ -34,6 +34,15 @@ SMTPOutboundIdentityTrusted(unsigned long flags, const char *auth_sender) auth_sender[0] != '-'; } +int +SMTPOutboundRequiresDKIM(unsigned long flags, const char *auth_sender, + const char *sender) +{ + return SMTPOutboundIdentityTrusted(flags, auth_sender) || + sender == NULL || sender[0] == '\0' || + (sender[0] == '-' && sender[1] == '\0'); +} + int SMTPOutboundNeedsSRS(unsigned long flags, const char *auth_sender, const char *sender) diff --git a/src/agents/smtp/outbound-policy.h b/src/agents/smtp/outbound-policy.h index 8b859f3..3be43ed 100644 --- a/src/agents/smtp/outbound-policy.h +++ b/src/agents/smtp/outbound-policy.h @@ -23,6 +23,8 @@ #define BONGO_SMTP_OUTBOUND_POLICY_H int SMTPOutboundIdentityTrusted(unsigned long flags, const char *auth_sender); +int SMTPOutboundRequiresDKIM(unsigned long flags, const char *auth_sender, + const char *sender); int SMTPOutboundNeedsSRS(unsigned long flags, const char *auth_sender, const char *sender); diff --git a/src/agents/smtp/smtpc.c b/src/agents/smtp/smtpc.c index 97532c1..52ba930 100755 --- a/src/agents/smtp/smtpc.c +++ b/src/agents/smtp/smtpc.c @@ -232,7 +232,7 @@ ParseClientMetadata(char *line, SMTPClient *queue) static BongoMailAuthStatus GetEnvelopeSender(const SMTPClient *queue, char *rewritten, size_t rewritten_size, - const char **sender) + const char **sender, BOOL externalTransport) { BOOL forwarded; @@ -241,11 +241,22 @@ GetEnvelopeSender(const SMTPClient *queue, char *rewritten, size_t rewritten_siz } if (queue->sender == NULL) return BONGO_MAILAUTH_PROTOCOL_ERROR; *sender = strcmp(queue->sender, "-") == 0 ? "" : queue->sender; + if (!externalTransport) { + return BONGO_MAILAUTH_OK; + } forwarded = SMTPOutboundNeedsSRS( (unsigned long) queue->flags, queue->authSender, queue->sender); - if (!MailAuth.srs_forward || !forwarded) { + if (!forwarded) { return BONGO_MAILAUTH_OK; } + /* + * An unauthenticated Internet message which is forwarded externally + * must not leave Bongo with its original envelope sender. Deferring is + * safer than silently breaking SPF/DMARC when SRS is disabled. + */ + if (!MailAuth.srs_forward) { + return BONGO_MAILAUTH_INVALID_ARGUMENT; + } if (MailAuth.srs_domain == NULL || MailAuth.srs_domain[0] == '\0' || MailAuth.srs_secret_file == NULL || MailAuth.srs_secret_file[0] == '\0') { return BONGO_MAILAUTH_INVALID_ARGUMENT; @@ -282,21 +293,21 @@ GenerateDKIMSignature(SMTPClient *queue) const char *domain; const char *at; char keyPath[XPL_MAX_PATH]; - char *messageLine = NULL; - unsigned long messageLineSize = 0; + unsigned char messageChunk[8192]; long long remaining; BongoDKIMSigner *signer = NULL; BongoMailAuthStatus status; + queue->dkimPrepared = TRUE; queue->hasDKIMSignature = FALSE; - /* External submission providers apply their own aligned DKIM signature. */ - if (!MailAuth.dkim_sign_outgoing || queue->hasExternalRelay) { + /* Sign local, authenticated, explicitly trusted relay mail, and DSNs. */ + if (!SMTPOutboundRequiresDKIM( + (unsigned long) queue->flags, queue->authSender, + queue->sender)) { return BONGO_MAILAUTH_OK; } - /* Sign only local, authenticated, or explicitly trusted relay mail. */ - if (!SMTPOutboundIdentityTrusted( - (unsigned long) queue->flags, queue->authSender)) { - return BONGO_MAILAUTH_OK; + if (!MailAuth.dkim_sign_outgoing) { + return BONGO_MAILAUTH_INVALID_ARGUMENT; } domain = MailAuth.dkim_signing_domain; if (domain == NULL || domain[0] == '\0') { @@ -327,27 +338,24 @@ GenerateDKIMSignature(SMTPClient *queue) goto cleanup; } remaining = atoll(&queue->line[5]); + if (remaining < 0) { + status = BONGO_MAILAUTH_PROTOCOL_ERROR; + goto cleanup; + } while (remaining > 0) { - size_t lineLength; - size_t consumed; + size_t chunk = remaining < (long long) sizeof(messageChunk) + ? (size_t) remaining : sizeof(messageChunk); - ConnReadToAllocatedBuffer(queue->conn, &messageLine, - &messageLineSize); - if (messageLine == NULL) { + if (ConnReadCount(queue->conn, messageChunk, (int) chunk) != + (int) chunk) { status = BONGO_MAILAUTH_IO_ERROR; goto cleanup; } - lineLength = strlen(messageLine); - status = BongoMailAuthDKIMSignerUpdate(signer, messageLine, lineLength); - if (status == BONGO_MAILAUTH_OK) { - status = BongoMailAuthDKIMSignerUpdate(signer, "\r\n", 2U); - } + status = BongoMailAuthDKIMSignerUpdate(signer, messageChunk, chunk); if (status != BONGO_MAILAUTH_OK) { goto cleanup; } - consumed = lineLength + 2U; - remaining -= remaining > (long long) consumed - ? (long long) consumed : remaining; + remaining -= (long long) chunk; } ConnReadAnswer(queue->conn, queue->line, CONN_BUFSIZE); if (atoi(queue->line) != 1000) { @@ -359,13 +367,27 @@ GenerateDKIMSignature(SMTPClient *queue) queue->hasDKIMSignature = status == BONGO_MAILAUTH_OK; cleanup: - if (messageLine != NULL) { - MemFree(messageLine); - } BongoMailAuthDKIMSignerFree(signer); return status; } +static BongoMailAuthStatus +PrepareExternalMessage(SMTPClient *queue) +{ + if (queue == NULL) { + return BONGO_MAILAUTH_INVALID_ARGUMENT; + } + if (queue->dkimPrepared) { + return queue->hasDKIMSignature + ? BONGO_MAILAUTH_OK : ( + SMTPOutboundNeedsSRS( + (unsigned long) queue->flags, queue->authSender, + queue->sender) + ? BONGO_MAILAUTH_OK : BONGO_MAILAUTH_PROTOCOL_ERROR); + } + return GenerateDKIMSignature(queue); +} + static int InspectQueueMessageHeaders(SMTPClient *queue) { @@ -904,8 +926,16 @@ DeliverRelay(SMTPClient *queue, RecipStruct *recipient, queue != NULL && queue->qID != NULL ? queue->qID : "(unknown)"); goto done; } + senderStatus = PrepareExternalMessage(queue); + if (senderStatus != BONGO_MAILAUTH_OK) { + Log(LOG_ERROR, + "Cannot DKIM-sign external relay message %s (status %d)", + queue->qID, (int)senderStatus); + goto done; + } senderStatus = GetEnvelopeSender( - queue, rewrittenSender, sizeof(rewrittenSender), &envelopeSender); + queue, rewrittenSender, sizeof(rewrittenSender), &envelopeSender, + TRUE); if (senderStatus != BONGO_MAILAUTH_OK) { Log(LOG_ERROR, "Cannot prepare relay envelope sender for message %s (status %d)", @@ -1627,7 +1657,8 @@ beginConversation: /* Apply SRS only to unauthenticated externally sourced messages which * Bongo is forwarding. Local submissions and bounces are not rewritten. */ if (GetEnvelopeSender(Queue, rewrittenSender, sizeof(rewrittenSender), - &envelopeSender) != BONGO_MAILAUTH_OK) { + &envelopeSender, !Remote->isLMTP) != + BONGO_MAILAUTH_OK) { Log(LOG_ERROR, "SRS forwarding failed for queue %s; delivery deferred", Queue->qID); @@ -1799,7 +1830,7 @@ beginConversation: Recip->Result = DELIVER_TRY_LATER; goto finalization; } - if (Queue->hasDKIMSignature) { + if (Queue->hasDKIMSignature && !Remote->isLMTP) { ConnWriteF(Remote->conn, "DKIM-Signature: %s\r\n", Queue->dkimSignature); } @@ -2039,17 +2070,6 @@ ProcessEntry(void *clientp, Connection *conn) g_array_free(Recipients, TRUE); return -1; } - { - BongoMailAuthStatus dkimStatus = GenerateDKIMSignature(Queue); - if (dkimStatus != BONGO_MAILAUTH_OK) { - Log(LOG_ERROR, - "DKIM signing preparation failed for queue %s (status %d)", - Queue->qID, (int) dkimStatus); - PreserveRemoteRecipientsForRetry(Queue, Recipients); - g_array_free(Recipients, TRUE); - return -1; - } - } g_array_sort(Recipients, (ArrayCompareFunc)RecipientCompare); /* now i can skip over any duplicates an do lookups once per remote domain */ @@ -2110,7 +2130,24 @@ ProcessEntry(void *clientp, Connection *conn) continue; } - DeliverMessage(Queue, &Remote, &CurrentRecip); + if (!Remote.isLMTP) { + BongoMailAuthStatus dkimStatus = + PrepareExternalMessage(Queue); + if (dkimStatus != BONGO_MAILAUTH_OK) { + Log(LOG_ERROR, + "DKIM signing preparation failed for queue %s " + "(status %d)", + Queue->qID, (int) dkimStatus); + CurrentRecip.Result = DELIVER_TRY_LATER; + ConnClose(Remote.conn); + ConnFree(Remote.conn); + Remote.conn = NULL; + } else { + DeliverMessage(Queue, &Remote, &CurrentRecip); + } + } else { + DeliverMessage(Queue, &Remote, &CurrentRecip); + } if (Remote.conn != NULL) { ConnClose(Remote.conn); ConnFree(Remote.conn); diff --git a/src/agents/smtp/smtpc.h b/src/agents/smtp/smtpc.h index e225201..2f09741 100644 --- a/src/agents/smtp/smtpc.h +++ b/src/agents/smtp/smtpc.h @@ -52,6 +52,7 @@ typedef struct { int flags; char line[CONN_BUFSIZE + 1]; char qID[16]; /* holds the queueid pulled during handshake */ + BOOL dkimPrepared; BOOL hasDKIMSignature; char dkimSignature[8192]; BOOL hasExternalRelay; diff --git a/src/agents/smtp/tests/outbound-policy-test.c b/src/agents/smtp/tests/outbound-policy-test.c index 72342c5..e03f78a 100644 --- a/src/agents/smtp/tests/outbound-policy-test.c +++ b/src/agents/smtp/tests/outbound-policy-test.c @@ -34,6 +34,16 @@ main(void) assert(SMTPOutboundIdentityTrusted( MSG_FLAG_SOURCE_EXTERNAL | MSG_FLAG_TRUSTED_RELAY, "-")); + assert(SMTPOutboundRequiresDKIM(0U, NULL, "sender@example.test")); + assert(SMTPOutboundRequiresDKIM( + MSG_FLAG_SOURCE_EXTERNAL, "test1", "sender@example.test")); + assert(SMTPOutboundRequiresDKIM( + MSG_FLAG_SOURCE_EXTERNAL | MSG_FLAG_TRUSTED_RELAY, "-", + "sender@example.test")); + assert(SMTPOutboundRequiresDKIM(MSG_FLAG_SOURCE_EXTERNAL, "-", "-")); + assert(!SMTPOutboundRequiresDKIM( + MSG_FLAG_SOURCE_EXTERNAL, "-", "sender@example.test")); + assert(SMTPOutboundNeedsSRS( MSG_FLAG_SOURCE_EXTERNAL, "-", "sender@example.test")); assert(!SMTPOutboundNeedsSRS(