Enforce outbound DKIM and SRS policy

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

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"
     )
 
 

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.

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 |  |  |  |

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)

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);
 

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);

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;

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(