Prefer internal LMTP over outbound relays

contrib/testing/README.md

@@ -57,8 +57,10 @@ one-off files in `/tmp`:
 - `smtp-lmtp-delivery-check.py` starts a loopback-only, status-aware LMTP
   daemon, temporarily maps `lmtp.smtp20.test` to it, and submits a
   Mailman-style message through trusted port 26. It requires the original
-  envelope plus an unsigned, non-SRS internal LMTP message, then restores the
-  exact SMTP configuration and active service.
+  envelope plus an unsigned, non-SRS internal LMTP message while an
+  unreachable global relayhost is enabled. This proves that internal LMTP
+  routing takes precedence, after which the test restores the exact SMTP
+  configuration and active service.
 - `smtp-client-interoperability-check.py` repeats real delivery with curl,
   swaks, Xeams Email Sender, GNU Mailutils, and paced Telnet input. It covers
   authenticated STARTTLS submission plus unauthenticated local delivery on

contrib/testing/smtp-lmtp-delivery-check.py

@@ -280,6 +280,20 @@ def perform_test() -> None:
     ]
     transports.append(mapping)
     test_smtp["lmtp_transports"] = transports
+    # Deliberately enable an unreachable global relay.  LMTP transport
+    # mappings are internal routes and must take precedence over it.
+    test_smtp.update(
+        {
+            "use_relay_host": True,
+            "relay_host": "127.0.0.1",
+            "relay_port": LMTP_PORT + 1,
+            "relay_tls_security_level": "none",
+            "relay_tls_wrapper_mode": False,
+            "relay_tls_ca_file": "",
+            "relay_username": "",
+            "relay_password_file": "",
+        }
+    )
 
     capture = LMTPCapture()
     server = ThreadingLMTPServer((LMTP_HOST, LMTP_PORT), LMTPHandler)
@@ -335,7 +349,8 @@ def perform_test() -> None:
         raise SMTP20Error("SMTP configuration was not restored exactly")
     print(
         "SMTP-20 PASS ingress=trusted-internal-smtp transport=lmtp "
-        "envelope=original dkim=absent srs=absent duplicate=no "
+        "relayhost=bypassed envelope=original dkim=absent srs=absent "
+        "duplicate=no "
         f"queue-clean={'yes' if queue_clean else 'no'} "
         "config-restored=yes service-restored=active"
     )

docs/test-evidence/0.7-r1.md

@@ -2166,13 +2166,14 @@ that AUTH is available only on the encrypted session, authenticates as the
 envelope sender, and submits to `test2@bongo.test`. It then consumes the
 streaming Store LIST response completely before reading objects, finds the
 unique marker in `test2`'s real `/mail/INBOX`, and verifies the From, To,
-Message-ID, marker header, and body. More than one matching Store object is a
-hard failure.
+Message-ID, marker header, and body. It also rejects any `DKIM-Signature` or
+SRS return path: this is an internal Store delivery, not an external SMTP
+delivery. More than one matching Store object is a hard failure.
 
 The installed-service run reported:
 
 ```text
-SMTP-06 PASS host=127.0.0.1:587 tls=TLSv1.3 auth=test1 recipient=test2@bongo.test store-inbox=yes duplicate=no queue-clean=yes store-clean=yes
+SMTP-06 PASS host=127.0.0.1:587 tls=TLSv1.3 auth=test1 recipient=test2@bongo.test store-inbox=yes duplicate=no dkim=absent srs=absent queue-clean=yes store-clean=yes
 ```
 
 The cleanup path deleted the exact Store object, removed any matching Queue

src/agents/smtp/smtpc.c

@@ -923,7 +923,8 @@ DeliverRelay(SMTPClient *queue, RecipStruct *recipient,
         ((route->username == NULL || route->username[0] == '\0') !=
          (route->password == NULL || route->password[0] == '\0'))) {
         Log(LOG_ERROR, "Invalid relay route for message %s",
-            queue != NULL && queue->qID != NULL ? queue->qID : "(unknown)");
+            queue != NULL && queue->qID[0] != '\0'
+                ? queue->qID : "(unknown)");
         goto done;
     }
     senderStatus = PrepareExternalMessage(queue);
@@ -2099,10 +2100,6 @@ ProcessEntry(void *clientp, Connection *conn)
             /* A malformed address from an older/imported queue entry was
              * rejected while the envelope was parsed.  Preserve that result
              * and report it below without attempting DNS or relay delivery. */
-        } else if (Queue->hasExternalRelay) {
-            DeliverExternalRelay(Queue, &CurrentRecip);
-        } else if (MailAuth.use_relay_host) {
-            DeliverGlobalRelay(Queue, &CurrentRecip);
         } else {
             Remote.isLMTP = FindLMTPTransport(
                 &CurrentRecip, Remote.transportHost,
@@ -2111,6 +2108,15 @@ ProcessEntry(void *clientp, Connection *conn)
                 Remote.conn = ConnectLMTP(
                     &CurrentRecip, Remote.transportHost,
                     Remote.transportPort);
+                if (Remote.conn == NULL) continue;
+                DeliverMessage(Queue, &Remote, &CurrentRecip);
+                ConnClose(Remote.conn);
+                ConnFree(Remote.conn);
+                Remote.conn = NULL;
+            } else if (Queue->hasExternalRelay) {
+                DeliverExternalRelay(Queue, &CurrentRecip);
+            } else if (MailAuth.use_relay_host) {
+                DeliverGlobalRelay(Queue, &CurrentRecip);
             } else {
                 /* Direct RFC 5321 MX delivery always uses SMTP port 25.
                  * Preserve it in the transport metadata as well as the
@@ -2124,34 +2130,28 @@ ProcessEntry(void *clientp, Connection *conn)
                     Remote.originalNextHopDomain,
                     sizeof(Remote.originalNextHopDomain),
                     &Remote.mxDnssecAuthenticated);
-            }
-            if (!Remote.conn) {
-                /* There was an error looking up or connecting to the remote server. */
-                continue;
-            }
-
-            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;
+                if (Remote.conn == NULL) {
+                    /* The MX lookup or connection failed. */
+                    continue;
+                }
+                {
+                    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;
+                    } else {
+                        DeliverMessage(Queue, &Remote, &CurrentRecip);
+                    }
+                }
+                if (Remote.conn != NULL) {
                     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);
-                Remote.conn = NULL;
             }
         }