return 550 for relay_to=false, log_oob=true case

When `log_arf` or `log_oob` are set to true with `relay_to=false`, we
now return a 550 error response for messages that are not ARF or OOB
reports.  Previously, we would return a 250 response and silently drop
the message in this case, which gave the false impression that it was
accepted for relaying.

Expand integration test to explicitly assert that the right things
are allowed/denied/relayed/parsed.

crates/integration-tests/source.lua

@@ -4,13 +4,20 @@ local kumo = require 'kumo'
 local TEST_DIR = os.getenv 'KUMOD_TEST_DIR'
 local SINK_PORT = tonumber(os.getenv 'KUMOD_SMTP_SINK_PORT')
 local WEBHOOK_PORT = os.getenv 'KUMOD_WEBHOOK_PORT'
+local LISTENER_MAP = os.getenv 'KUMOD_LISTENER_DOMAIN_MAP'
 
 kumo.on('init', function()
   kumo.configure_accounting_db_path(TEST_DIR .. '/accounting.db')
 
+  local relay_hosts = { '0.0.0.0/0' }
+  local RELAY_HOSTS = os.getenv 'KUMOD_RELAY_HOSTS'
+  if RELAY_HOSTS then
+    relay_hosts = kumo.json_parse(RELAY_HOSTS)
+  end
+
   kumo.start_esmtp_listener {
     listen = '127.0.0.1:0',
-    relay_hosts = { '0.0.0.0/0' },
+    relay_hosts = relay_hosts,
   }
 
   kumo.start_http_listener {
@@ -79,6 +86,14 @@ if WEBHOOK_PORT then
 end
 
 kumo.on('get_listener_domain', function(domain, listener, conn_meta)
+  if LISTENER_MAP then
+    local map = kumo.json_parse(LISTENER_MAP)
+    local params = map[domain]
+    if params then
+      return kumo.make_listener_domain(params)
+    end
+  end
+
   return kumo.make_listener_domain {
     relay_to = true,
     log_oob = true,

crates/integration-tests/src/main.rs

@@ -19,6 +19,7 @@ mod test {
     use kumo_log_types::RecordType::{Bounce, Delivery, Reception, TransientFailure};
     use mailparsing::DecodedBody;
     use rfc5321::*;
+    use serde_json::json;
     use std::collections::BTreeMap;
     use std::time::Duration;
 
@@ -577,11 +578,28 @@ Some(
         Ok(())
     }
 
+    fn json_string(v: serde_json::Value) -> String {
+        serde_json::to_string(&v).unwrap()
+    }
+
     #[tokio::test]
     async fn log_oob_arf() -> anyhow::Result<()> {
-        let mut daemon = DaemonWithMaildir::start()
-            .await
-            .context("DaemonWithMaildir::start")?;
+        let mut daemon = DaemonWithMaildir::start_with_env(vec![
+            (
+                "KUMOD_LISTENER_DOMAIN_MAP",
+                &json_string(json!({
+                    "example.com": {
+                        "relay_to": false,
+                        "log_oob": true,
+                        "log_arf": true,
+                    }
+
+                })),
+            ),
+            ("KUMOD_RELAY_HOSTS", &json_string(json!([]))),
+        ])
+        .await
+        .context("DaemonWithMaildir::start")?;
 
         eprintln!("sending message");
         let mut client = daemon.smtp_client().await.context("make smtp_client")?;
@@ -600,6 +618,40 @@ Some(
             anyhow::ensure!(response.code == 250);
         }
 
+        {
+            // and verify that relaying for !report is not allowed
+            let body = generate_message_text(1024, 78);
+            let response = MailGenParams {
+                body: Some(&body),
+                ..Default::default()
+            }
+            .send(&mut client)
+            .await
+            .unwrap_err();
+            k9::snapshot!(
+                response,
+                r#"
+Rejected(
+    Response {
+        code: 550,
+        enhanced_code: Some(
+            EnhancedStatusCode {
+                class: 5,
+                subject: 7,
+                detail: 1,
+            },
+        ),
+        content: "relaying not permitted",
+        command: Some(
+            ".\r
+",
+        ),
+    },
+)
+"#
+            );
+        }
+
         daemon
             .wait_for_maildir_count(2, Duration::from_secs(10))
             .await;
@@ -613,15 +665,12 @@ Some(
             "
 DeliverySummary {
     source_counts: {
-        Reception: 1,
-        Delivery: 2,
+        Reception: 2,
         OOB: 1,
         Feedback: 1,
+        Rejection: 1,
     },
-    sink_counts: {
-        Reception: 2,
-        Delivery: 2,
-    },
+    sink_counts: {},
 }
 "
         );
@@ -629,8 +678,8 @@ DeliverySummary {
             daemon.source.accounting_stats()?,
             "
 AccountingStats {
-    received: 1,
-    delivered: 2,
+    received: 2,
+    delivered: 0,
 }
 "
         );

crates/kumod/src/smtp_server.rs

@@ -1456,6 +1456,8 @@ impl SmtpServer {
         // get to work on logging and injecting into the queues
 
         let mut messages = vec![];
+        let mut was_arf_or_oob = false;
+
         for message in accepted_messages {
             if self.params.trace_headers.supplemental_header {
                 let mut object = json!({
@@ -1511,6 +1513,13 @@ impl SmtpServer {
                     message.save().await?;
                 }
             }
+
+            if relay_disposition.log_arf && matches!(message.parse_rfc5965(), Ok(Some(_))) {
+                was_arf_or_oob = true;
+            } else if relay_disposition.log_oob && matches!(message.parse_rfc3464(), Ok(Some(_))) {
+                was_arf_or_oob = true;
+            }
+
             log_disposition(LogDisposition {
                 kind: RecordType::Reception,
                 msg: message.clone(),
@@ -1539,14 +1548,20 @@ impl SmtpServer {
             }
         }
 
-        if !messages.is_empty() {
-            for (queue_name, msg) in messages {
-                QueueManager::insert(&queue_name, msg).await?;
-            }
+        let relayed_any = !messages.is_empty();
+
+        for (queue_name, msg) in messages {
+            QueueManager::insert(&queue_name, msg).await?;
+        }
+
+        if !relayed_any && !was_arf_or_oob {
+            self.write_response(550, "5.7.1 relaying not permitted")
+                .await?;
+        } else {
+            let ids = ids.join(" ");
+            self.write_response(250, format!("OK ids={ids}")).await?;
         }
 
-        let ids = ids.join(" ");
-        self.write_response(250, format!("OK ids={ids}")).await?;
         Ok(())
     }
 }

docs/changelog/main.md

@@ -103,3 +103,7 @@
   and sending to domain(s) with an MTA-STS policy.
 * If a TSA rule suspends a ready queue in response to a 421-before-MAIL-FROM,
   the contents of the corresponding ready queue could get stuck
+* When `log_arf` or `log_oob` are set to true with `relay_to=false`, we now return
+  a 550 error response for messages that are not ARF or OOB reports.
+  Previously, we would return a 250 response and silently drop the message in this case,
+  which gave the false impression that it was accepted for relaying.