diff --git a/crates/integration-tests/source.lua b/crates/integration-tests/source.lua index d053fc9f..859e31c4 100644 --- a/crates/integration-tests/source.lua +++ b/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, diff --git a/crates/integration-tests/src/main.rs b/crates/integration-tests/src/main.rs index ec8a1bf7..f982601f 100644 --- a/crates/integration-tests/src/main.rs +++ b/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, } " ); diff --git a/crates/kumod/src/smtp_server.rs b/crates/kumod/src/smtp_server.rs index 00347047..d7b25eb4 100644 --- a/crates/kumod/src/smtp_server.rs +++ b/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(()) } } diff --git a/docs/changelog/main.md b/docs/changelog/main.md index 476e7f73..e3eb4b8b 100644 --- a/docs/changelog/main.md +++ b/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.