Clean up changelog, identifiers in format strings

CHANGELOG.md

@@ -1,5 +1,13 @@
 # SpamAssassin Milter changelog
 
+## 0.4.1 (unreleased)
+
+### Added
+
+* Add `--synth-relay` option to allow inserting the synthesised internal relay
+  header (the MTA’s `Received` header) at a different position than at the very
+  beginning.
+
 ## 0.4.0 (2023-01-29)
 
 The minimum supported Rust version is now 1.61.0.

Cargo.lock

@@ -2,6 +2,15 @@
 # It is not intended for manual editing.
 version = 3
 
+[[package]]
+name = "aho-corasick"
+version = "0.7.20"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cc936419f96fa211c1b9166887b38e5e40b19958e5b895be7c1f93adec7071ac"
+dependencies = [
+ "memchr",
+]
+
 [[package]]
 name = "android_system_properties"
 version = "0.1.5"
@@ -468,6 +477,23 @@ dependencies = [
  "getrandom",
 ]
 
+[[package]]
+name = "regex"
+version = "1.7.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "48aaa5748ba571fb95cd2c85c09f629215d3a6ece942baa100950af03a34f733"
+dependencies = [
+ "aho-corasick",
+ "memchr",
+ "regex-syntax",
+]
+
+[[package]]
+name = "regex-syntax"
+version = "0.6.28"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "456c603be3e8d448b072f410900c09faf164fbce2d480456f50eea6e25f9c848"
+
 [[package]]
 name = "scratch"
 version = "1.0.3"
@@ -537,6 +563,7 @@ dependencies = [
  "ipnet",
  "once_cell",
  "rand",
+ "regex",
  "signal-hook",
  "signal-hook-tokio",
  "tokio",

Cargo.toml

@@ -18,6 +18,7 @@ futures = "0.3.25"
 indymilter = "0.2.0"
 ipnet = "2.7.1"
 once_cell = "1.17.0"
+regex = "1.7.1"
 signal-hook = "0.3.14"
 signal-hook-tokio = { version = "0.3.1", features = ["futures-v0_3"] }
 tokio = { version = "1.24.2", features = ["fs", "io-util", "macros", "net", "process", "rt", "rt-multi-thread", "sync"] }

spamassassin-milter.8

@@ -148,6 +148,32 @@ when rejecting a message flagged as spam.
 For multiline replies, use an ASCII newline character as the line separator.
 The default reply text is “Spam message refused”.
 .TP
+.BR \-\-synth-relay " \fIPOS\fR"
+Synthesize the internal relay header after position
+.IR POS .
+.I POS
+may be either an integer, in which case the relay header will be inserted after
+skipping
+.I POS
+header fields, or a regular expression, in which case the relay header will be
+inserted after skipping all header fields matching
+.IR POS .
+.IP
+This option is useful in situations where the default position of the
+synthesized internal relay header (the MTA’s
+.B Received
+header,
+.IR https://cwiki.apache.org/confluence/display/SPAMASSASSIN/TrustedRelays )
+at the very beginning of the message header is not appropriate.
+For example, when other milters insert
+.B Authentication-Results
+headers for SpamAssassin to consume, SpamAssassin will not trust these headers
+unless they appear before the internal relay header; in this case setting
+.I POS
+to
+.RB “ "^(?i)authentication-results:" ”
+achieves proper placement of the relay header after such header fields.
+.TP
 .BR \-t ", " \-\-trusted-networks " \fINETS\fR"
 Trust connections coming from the IP networks or addresses
 .IR NETS .

src/callbacks.rs

@@ -16,7 +16,8 @@
 
 use crate::{
     client::{Client, ReceivedInfo, Spamc},
-    config::Config,
+    config::{Config, SynthRelayPosition},
+    error::Result,
 };
 use byte_strings::c_str;
 use chrono::Local;
@@ -90,30 +91,27 @@ pub fn make_callbacks(config: Config) -> Callbacks<Connection> {
     let config = Arc::new(config);
     let config_connect = config.clone();
     let config_mail = config.clone();
+    let config_data = config.clone();
+    let config_header = config.clone();
+    let config_eoh = config.clone();
     let config_body = config.clone();
     let config_eom = config.clone();
 
     Callbacks::new()
-        .on_negotiate(move |cx, _, _| {
-            Box::pin(handle_negotiate(config.clone(), cx))
-        })
+        .on_negotiate(move |cx, _, _| Box::pin(handle_negotiate(config.clone(), cx)))
         .on_connect(move |cx, _, socket_info| {
             Box::pin(handle_connect(config_connect.clone(), cx, socket_info))
         })
         .on_helo(|cx, helo_host| Box::pin(handle_helo(cx, helo_host)))
-        .on_mail(move |cx, smtp_args| {
-            Box::pin(handle_mail(config_mail.clone(), cx, smtp_args))
-        })
+        .on_mail(move |cx, smtp_args| Box::pin(handle_mail(config_mail.clone(), cx, smtp_args)))
         .on_rcpt(|cx, smtp_args| Box::pin(handle_rcpt(cx, smtp_args)))
-        .on_data(|cx| Box::pin(handle_data(cx)))
-        .on_header(|cx, name, value| Box::pin(handle_header(cx, name, value)))
-        .on_eoh(|cx| Box::pin(handle_eoh(cx)))
-        .on_body(move |cx, chunk| {
-            Box::pin(handle_body(config_body.clone(), cx, chunk))
-        })
-        .on_eom(move |cx| {
-            Box::pin(handle_eom(config_eom.clone(), cx))
+        .on_data(move |cx| Box::pin(handle_data(config_data.clone(), cx)))
+        .on_header(move |cx, name, value| {
+            Box::pin(handle_header(config_header.clone(), cx, name, value))
         })
+        .on_eoh(move |cx| Box::pin(handle_eoh(config_eoh.clone(), cx)))
+        .on_body(move |cx, chunk| Box::pin(handle_body(config_body.clone(), cx, chunk)))
+        .on_eom(move |cx| Box::pin(handle_eom(config_eom.clone(), cx)))
         .on_abort(|cx| Box::pin(handle_abort(cx)))
         .on_close(|cx| Box::pin(handle_close(cx)))
 }
@@ -190,7 +188,12 @@ async fn handle_mail(
     let spamc = Spamc::new(config.spamc_args());
     let sender = smtp_args[0].to_string_lossy();
 
-    conn.client = Some(Client::new(spamc, sender.into()));
+    conn.client = Some(Client::new(
+        spamc,
+        conn.client_ip,
+        conn.helo_host.as_deref(),
+        sender.into(),
+    ));
 
     Status::Continue
 }
@@ -206,7 +209,7 @@ async fn handle_rcpt(context: &mut Context<Connection>, smtp_args: Vec<CString>)
     Status::Continue
 }
 
-async fn handle_data(context: &mut Context<Connection>) -> Status {
+async fn handle_data(config: Arc<Config>, context: &mut Context<Connection>) -> Status {
     let conn = context.data.connection();
     let client = conn.client.as_mut().unwrap();
 
@@ -217,51 +220,89 @@ async fn handle_data(context: &mut Context<Connection>) -> Status {
         return Status::Tempfail;
     }
 
-    // Note that when SpamAssassin reports are enabled (`report_safe 1`), the
-    // synthesised headers below are ‘leaked’ to users in the sense that they
-    // are included inside the email MIME attachment in the new message body.
-
-    ok_or_tempfail!(client.send_envelope_sender().await);
-    ok_or_tempfail!(client.send_envelope_recipients().await);
-
-    let info = ReceivedInfo {
-        client_ip: conn.client_ip,
-        helo_host: conn.helo_host.as_deref(),
-        client_name_addr: context.macros.get_string(c_str!("_")),
-        my_hostname: context.macros.get_string(c_str!("j")),
-        mta: context.macros.get_string(c_str!("v")),
-        tls: context.macros.get_string(c_str!("{tls_version}")),
-        auth: context.macros.get_string(c_str!("{auth_authen}")),
-        queue_id: &id,
-        date_time: Local::now().to_rfc2822(),
-    };
-
-    ok_or_tempfail!(client.send_synthesized_received_header(info).await);
+    if config.synth_relay().is_none() {
+        ok_or_tempfail!(synthesize_relay_header(&config, &context.macros, client).await);
+    }
 
     Status::Continue
 }
 
-async fn handle_header(context: &mut Context<Connection>, name: CString, value: CString) -> Status {
+async fn handle_header(
+    config: Arc<Config>,
+    context: &mut Context<Connection>,
+    name: CString,
+    value: CString,
+) -> Status {
     let conn = context.data.connection();
     let client = conn.client.as_mut().unwrap();
 
     let name = name.to_string_lossy();
     let value = value.to_string_lossy();
 
+    if let Some(pos) = config.synth_relay() {
+        if !client.is_synth_relay_inserted() {
+            match pos {
+                SynthRelayPosition::Index(i) => {
+                    if *i == client.header_count() {
+                        ok_or_tempfail!(synthesize_relay_header(&config, &context.macros, client).await);
+                    }
+                }
+                SynthRelayPosition::Regex(regex) => {
+                    if !regex.is_match(&format!("{}:{}", name, value)) {
+                        ok_or_tempfail!(synthesize_relay_header(&config, &context.macros, client).await);
+                    }
+                }
+            }
+        }
+    }
+
     ok_or_tempfail!(client.send_header(&name, &value).await);
 
     Status::Continue
 }
 
-async fn handle_eoh(context: &mut Context<Connection>) -> Status {
+async fn handle_eoh(config: Arc<Config>, context: &mut Context<Connection>) -> Status {
     let conn = context.data.connection();
     let client = conn.client.as_mut().unwrap();
 
+    if config.synth_relay().is_some() && !client.is_synth_relay_inserted() {
+        ok_or_tempfail!(synthesize_relay_header(&config, &context.macros, client).await);
+    }
+
     ok_or_tempfail!(client.send_eoh().await);
 
     Status::Continue
 }
 
+async fn synthesize_relay_header(config: &Config, macros: &Macros, client: &mut Client) -> Result<()> {
+    let id = macros.queue_id();
+
+    if config.synth_relay().is_some() {
+        verbose!(config, "{id}: inserting synthetic relay header at index {}", client.header_count());
+    }
+
+    // Note that when SpamAssassin reports are enabled (`report_safe 1`), the
+    // synthesised headers below are ‘leaked’ to users in the sense that they
+    // are included inside the email MIME attachment in the new message body.
+
+    client.send_envelope_sender().await?;
+    client.send_envelope_recipients().await?;
+
+    let info = ReceivedInfo {
+        client_name_addr: macros.get_string(c_str!("_")),
+        my_hostname: macros.get_string(c_str!("j")),
+        mta: macros.get_string(c_str!("v")),
+        tls: macros.get_string(c_str!("{tls_version}")),
+        auth: macros.get_string(c_str!("{auth_authen}")),
+        queue_id: &id,
+        date_time: Local::now().to_rfc2822(),
+    };
+
+    client.send_synthesized_received_header(info).await?;
+
+    Ok(())
+}
+
 async fn handle_body(
     config: Arc<Config>,
     context: &mut Context<Connection>,

src/client.rs

@@ -124,9 +124,7 @@ impl Process for Spamc {
     }
 }
 
-pub struct ReceivedInfo<'helo, 'macros> {
-    pub client_ip: IpAddr,
-    pub helo_host: Option<&'helo str>,
+pub struct ReceivedInfo<'macros> {
     pub client_name_addr: Option<Cow<'macros, str>>,
     pub my_hostname: Option<Cow<'macros, str>>,
     pub mta: Option<Cow<'macros, str>>,
@@ -138,19 +136,32 @@ pub struct ReceivedInfo<'helo, 'macros> {
 
 pub struct Client {
     process: Box<dyn Process + Send>,
+    client_ip: IpAddr,
+    helo_host: Option<String>,
     sender: Box<str>,
     recipients: Vec<Box<str>>,
+    synth_relay_inserted: bool,
+    header_count: usize,
     headers: HeaderMap,
     bytes: usize,
     skipped: bool,
 }
 
 impl Client {
-    pub fn new(process: impl Process + Send + 'static, sender: Box<str>) -> Self {
+    pub fn new(
+        process: impl Process + Send + 'static,
+        client_ip: IpAddr,
+        helo_host: Option<&str>,
+        sender: Box<str>,
+    ) -> Self {
         Self {
             process: Box::new(process),
+            client_ip,
+            helo_host: helo_host.map(|s| s.to_owned()),
             sender,
             recipients: vec![],
+            synth_relay_inserted: false,
+            header_count: 0,
             headers: HeaderMap::new(),
             bytes: 0,
             skipped: false,
@@ -161,6 +172,14 @@ impl Client {
         self.recipients.push(recipient);
     }
 
+    pub fn is_synth_relay_inserted(&self) -> bool {
+        self.synth_relay_inserted
+    }
+
+    pub fn header_count(&self) -> usize {
+        self.header_count
+    }
+
     pub fn bytes_written(&self) -> usize {
         self.bytes
     }
@@ -191,16 +210,13 @@ impl Client {
         Ok(())
     }
 
-    pub async fn send_synthesized_received_header(
-        &mut self,
-        info: ReceivedInfo<'_, '_>,
-    ) -> Result<()> {
+    pub async fn send_synthesized_received_header(&mut self, info: ReceivedInfo<'_>) -> Result<()> {
         // Sending this ‘Received’ header is crucial: Milters don’t see the
         // MTA’s own ‘Received’ header. However, SpamAssassin draws a lot of
         // information from that header. So we make one up and send it along.
 
-        let client_ip = info.client_ip.to_string();
-        let helo_host = info.helo_host.unwrap_or(&client_ip);
+        let client_ip = self.client_ip.to_string();
+        let helo_host = self.helo_host.as_deref().unwrap_or(&client_ip);
 
         let client_name_addr = info.client_name_addr.as_deref().unwrap_or(&client_ip);
         let my_hostname = info.my_hostname.as_deref().unwrap_or("localhost");
@@ -229,6 +245,8 @@ impl Client {
         self.process.writer().write_all(buf.as_bytes()).await?;
         self.bytes += buf.len();
 
+        self.synth_relay_inserted = true;
+
         Ok(())
     }
 
@@ -248,6 +266,8 @@ impl Client {
         self.process.writer().write_all(buf.as_bytes()).await?;
         self.bytes += buf.len();
 
+        self.header_count += 1;
+
         Ok(())
     }
 
@@ -364,7 +384,7 @@ mod tests {
     use super::*;
     use byte_strings::c_str;
     use indymilter::{ActionError, IntoCString, SmtpReplyError};
-    use std::{ffi::CString, result, sync::Mutex};
+    use std::{ffi::CString, net::Ipv4Addr, result, sync::Mutex};
 
     #[derive(Debug, Default)]
     struct MockSpamc {
@@ -551,13 +571,15 @@ mod tests {
         }
     }
 
+    const IP: IpAddr = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4));
+    const HELO: &str = "helo";
     const ID: &str = "NONE";
 
     #[tokio::test]
     async fn client_send_writes_bytes() {
         let spamc = MockSpamc::new();
 
-        let mut client = Client::new(spamc, "sender".into());
+        let mut client = Client::new(spamc, IP, Some(HELO), "sender".into());
         client.send_header("name1", " value1").await.unwrap();
         client.send_header("name2", " value2\n\tcontinued").await.unwrap();
         client.send_eoh().await.unwrap();
@@ -578,7 +600,7 @@ mod tests {
         let recipient1 = "<recipient1@gluet.ch>";
         let recipient2 = "<recipient2@gluet.ch>";
 
-        let mut client = Client::new(spamc, sender.into());
+        let mut client = Client::new(spamc, IP, Some(HELO), sender.into());
         client.add_recipient(recipient1.into());
         client.add_recipient(recipient2.into());
 
@@ -600,7 +622,7 @@ mod tests {
     async fn client_process_invalid_response() {
         let spamc = MockSpamc::with_output(b"invalid message response".to_vec());
 
-        let client = Client::new(spamc, "sender".into());
+        let client = Client::new(spamc, IP, Some(HELO), "sender".into());
 
         let mut reply = MockSmtpReply::new();
         let actions = MockEomActions::new();
@@ -615,7 +637,7 @@ mod tests {
     async fn client_process_reject_spam() {
         let spamc = MockSpamc::with_output(b"X-Spam-Flag: YES\r\n\r\n".to_vec());
 
-        let client = Client::new(spamc, "sender".into());
+        let client = Client::new(spamc, IP, Some(HELO), "sender".into());
 
         let mut reply = MockSmtpReply::new();
         let actions = MockEomActions::new();
@@ -640,7 +662,7 @@ mod tests {
             b"X-Spam-Flag: YES\r\nX-Spam-Level: *****\r\n\r\nReport".to_vec(),
         );
 
-        let mut client = Client::new(spamc, "sender".into());
+        let mut client = Client::new(spamc, IP, Some(HELO), "sender".into());
 
         client.send_header("x-spam-level", " *").await.unwrap();
         client.send_header("x-spam-report", " ...").await.unwrap();
@@ -672,7 +694,7 @@ mod tests {
             b"X-Spam-Flag: YES\r\nX-Spam-Level: *****\r\n\r\nReport".to_vec(),
         );
 
-        let mut client = Client::new(spamc, "sender".into());
+        let mut client = Client::new(spamc, IP, Some(HELO), "sender".into());
         client.skip_body();
 
         let mut reply = MockSmtpReply::new();

src/config.rs

@@ -15,15 +15,17 @@
 // this program. If not, see https://www.gnu.org/licenses/.
 
 use ipnet::IpNet;
-use std::{collections::HashSet, net::IpAddr};
+use regex::{Error, Regex};
+use std::{collections::HashSet, net::IpAddr, str::FromStr};
 
 /// A builder for SpamAssassin Milter configuration objects.
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug)]
 pub struct ConfigBuilder {
     use_trusted_networks: bool,
     trusted_networks: HashSet<IpNet>,
     auth_untrusted: bool,
     spamc_args: Vec<String>,
+    synth_relay: Option<SynthRelayPosition>,
     max_message_size: usize,
     dry_run: bool,
     reject_spam: bool,
@@ -61,6 +63,11 @@ impl ConfigBuilder {
         self
     }
 
+    pub fn synth_relay(mut self, value: SynthRelayPosition) -> Self {
+        self.synth_relay = Some(value);
+        self
+    }
+
     pub fn max_message_size(mut self, value: usize) -> Self {
         self.max_message_size = value;
         self
@@ -125,6 +132,7 @@ impl ConfigBuilder {
             trusted_networks: self.trusted_networks,
             auth_untrusted: self.auth_untrusted,
             spamc_args: self.spamc_args,
+            synth_relay: self.synth_relay,
             max_message_size: self.max_message_size,
             dry_run: self.dry_run,
             reject_spam: self.reject_spam,
@@ -149,6 +157,7 @@ impl Default for ConfigBuilder {
             trusted_networks: Default::default(),
             auth_untrusted: Default::default(),
             spamc_args: Default::default(),
+            synth_relay: Default::default(),
             max_message_size: 512_000,  // same as in `spamc`
             dry_run: Default::default(),
             reject_spam: Default::default(),
@@ -166,12 +175,13 @@ impl Default for ConfigBuilder {
 }
 
 /// A configuration object for SpamAssassin Milter.
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug)]
 pub struct Config {
     use_trusted_networks: bool,
     trusted_networks: HashSet<IpNet>,
     auth_untrusted: bool,
     spamc_args: Vec<String>,
+    synth_relay: Option<SynthRelayPosition>,
     max_message_size: usize,
     dry_run: bool,
     reject_spam: bool,
@@ -204,6 +214,10 @@ impl Config {
         &self.spamc_args
     }
 
+    pub fn synth_relay(&self) -> Option<&SynthRelayPosition> {
+        self.synth_relay.as_ref()
+    }
+
     pub fn max_message_size(&self) -> usize {
         self.max_message_size
     }
@@ -247,6 +261,25 @@ impl Default for Config {
     }
 }
 
+/// The position of the synthesised relay header.
+#[derive(Clone, Debug)]
+pub enum SynthRelayPosition {
+    /// Insert header after skipping *n* headers.
+    Index(usize),
+    /// Insert header after skipping headers matching regex.
+    Regex(Regex),
+}
+
+impl FromStr for SynthRelayPosition {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        s.parse::<usize>()
+            .map(Self::Index)
+            .or_else(|_| s.parse::<Regex>().map(Self::Regex))
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/lib.rs

@@ -51,7 +51,7 @@ mod config;
 mod email;
 mod error;
 
-pub use crate::config::{Config, ConfigBuilder};
+pub use crate::config::{Config, ConfigBuilder, SynthRelayPosition};
 use indymilter::IntoListener;
 use std::{future::Future, io};
 

src/main.rs

@@ -136,6 +136,7 @@ Options:
   -C, --reply-code <CODE>           Reply code when rejecting messages
   -S, --reply-status-code <CODE>    Status code when rejecting messages
   -R, --reply-text <MSG>            Reply text when rejecting messages
+      --synth-relay <POS>           Synthesize relay header after position
   -t, --trusted-networks <NETS>     Trust connections from these networks
   -v, --verbose                     Enable verbose operation logging
   -V, --version                     Print version information
@@ -205,6 +206,13 @@ fn parse_args() -> Result<(Socket, Config), Box<dyn Error>> {
 
                 config = config.reply_text(msg);
             }
+            "--synth-relay" => {
+                let arg = args.next().ok_or_else(missing_value)??;
+                let pos = arg.parse()
+                    .map_err(|_| format!("invalid value for synthesized relay header position: \"{arg}\""))?;
+
+                config = config.synth_relay(pos);
+            }
             "-t" | "--trusted-networks" => {
                 let arg = args.next().ok_or_else(missing_value)??;