From adbd1731d56099233e7b0d1cf0d76c0369db0457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20B=C3=BCrgin?= Date: Wed, 4 Aug 2021 15:06:28 +0200 Subject: [PATCH] Special case X-Spam-Prev-, doc --- README.md | 66 +++++++++--------- spamassassin-milter.8 | 2 +- src/client.rs | 6 +- src/email.rs | 157 ++++++++++++++++++++++-------------------- 4 files changed, 119 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index b060c72..b85635e 100644 --- a/README.md +++ b/README.md @@ -37,46 +37,46 @@ Postfix, and for delivery [Dovecot] with LMTP and the [Sieve plugin]. [Dovecot]: https://dovecot.org [Sieve plugin]: https://doc.dovecot.org/configuration_manual/sieve -## Building - -This project is a [Rust] package. Build it with Cargo as usual. - -The program `spamc`, which is used for communication with SpamAssassin server, -must be executable and located in the milter’s search path. - -As a milter, this package requires the libmilter C library to be available. Be -sure to install the libmilter shared library and header files. - -The shared library is discovered using the pkg-config program. If your -distribution does not install a pkg-config metadata file for libmilter, try -using the provided `milter.pc` file. Put it on the pkg-config path as follows: - -``` -PKG_CONFIG_PATH=. cargo build -``` - -The integration tests rely on the `miltertest` utility. Make sure `miltertest` -is available and can be executed when running the integration tests. (Until -recently, `miltertest` had a serious bug that prevents most integration tests in -this package from completing; make sure you use an up-to-date version of -`miltertest`.) - -The minimum supported Rust version is 1.42.0. - -[Rust]: https://www.rust-lang.org - ## Installation -SpamAssassin Milter can be installed using Cargo. The program will be installed -in the local installation’s `bin` directory as usual: +SpamAssassin Milter is a [Rust] project. It can be installed using Cargo as +usual: ``` cargo install --locked spamassassin-milter ``` -Again, if your distribution does not provide pkg-config metadata, try using the -`milter.pc` file included in this package. Save `milter.pc` to some directory, -then run the above install command with `PKG_CONFIG_PATH` set to that directory. +As a milter, this package requires the libmilter C library to be available. Be +sure to install the libmilter shared library provided by your distribution. + +The shared library is discovered using the pkg-config program. If your +distribution does not install pkg-config metadata for libmilter, try using the +provided `milter.pc` file. Put this file on the pkg-config path when running any +Cargo command: + +``` +PKG_CONFIG_PATH=. cargo build +``` + +SpamAssassin Milter uses the `spamc` program for communication with SpamAssassin +server. By default, `/usr/bin/spamc` is used as the executable’s path. To +override this, set the environment variable `SPAMASSASSIN_MILTER_SPAMC` to the +desired path when building the application. + +The minimum supported Rust version is 1.42.0. + +[Rust]: https://www.rust-lang.org + +### Building + +The prerequisites mentioned above apply to building and testing the package, +too. + +Additionally, the integration tests rely on the third-party `miltertest` +utility. Make sure `miltertest` is available and can be executed when running +the integration tests. (Until recently, `miltertest` had a serious bug that +prevents most integration tests in this package from completing; make sure you +use an up-to-date version of `miltertest`.) ## Usage diff --git a/spamassassin-milter.8 b/spamassassin-milter.8 index 0c3bcef..47d9517 100644 --- a/spamassassin-milter.8 +++ b/spamassassin-milter.8 @@ -57,7 +57,7 @@ received from SpamAssassin server. is a light-weight integration component, enabling use of SpamAssassin with a milter-capable MTA. Users are advised to familiarize themselves with the setup and configuration -options of the components participating, namely, the SpamAssassin programs +options of the components involved, namely, the SpamAssassin programs .B spamd (SpamAssassin server) and .BR spamc , diff --git a/src/client.rs b/src/client.rs index 3c6fa85..497f7b0 100644 --- a/src/client.rs +++ b/src/client.rs @@ -471,7 +471,7 @@ mod tests { assert_eq!(client.bytes_written(), 48); assert_eq!( as_mock_spamc(client.process.as_ref()).buf, - Vec::from(b"name1: value1\r\nname2: value2\r\n\tcontinued\r\n\r\nbody" as &[_]) + b"name1: value1\r\nname2: value2\r\n\tcontinued\r\n\r\nbody".as_ref() ); } @@ -492,12 +492,12 @@ mod tests { assert_eq!( as_mock_spamc(client.process.as_ref()).buf, - Vec::from(format!( + format!( "X-Envelope-From: {}\r\n\ X-Envelope-To: {},\r\n\ \t{}\r\n", sender, recipient1, recipient2 - ).as_bytes()) + ).as_bytes() ); } diff --git a/src/email.rs b/src/email.rs index b09f75a..245d429 100644 --- a/src/email.rs +++ b/src/email.rs @@ -6,7 +6,6 @@ use crate::{ use milter::ActionContext; use once_cell::sync::Lazy; use std::{ - cmp, fmt::{self, Display, Formatter}, str, }; @@ -51,7 +50,8 @@ fn header_lines(header: &[u8]) -> Vec<&[u8]> { let mut start = i; while i < header.len() { - // Assume line endings are always encoded as b"\r\n". + // Assume line endings are always encoded as b"\r\n" since that is what + // the client sent to SpamAssassin earlier. if header[i] == b'\r' && i + 1 < header.len() && header[i + 1] == b'\n' { if i + 2 < header.len() && (header[i + 2] == b' ' || header[i + 2] == b'\t') { i += 3; @@ -73,7 +73,10 @@ fn header_lines(header: &[u8]) -> Vec<&[u8]> { } fn parse_header_line(bytes: &[u8]) -> Result> { + // This assumes that headers received back from SpamAssassin are valid + // UTF-8, which is plausible since the client only sent UTF-8 earlier. let line = str::from_utf8(bytes).map_err(|_| Error::ParseEmail)?; + let (name, value) = line.split_at(line.find(':').ok_or(Error::ParseEmail)?); if name.trim().is_empty() { @@ -101,10 +104,8 @@ pub fn ensure_lf(s: &str) -> String { } pub fn is_spam_assassin_header(name: &str) -> bool { - let prefix = b"X-Spam-"; - let name = name.as_bytes(); - - name[..cmp::min(prefix.len(), name.len())].eq_ignore_ascii_case(prefix) + let prefix = "X-Spam-"; + matches!(name.get(..prefix.len()), Some(s) if s.eq_ignore_ascii_case(prefix)) } // Values use CRLF line breaks and include leading whitespace. @@ -166,21 +167,21 @@ impl<'a> HeaderRewriter<'a> { } if is_spam_assassin_header(name) { - if let Some(m) = self.convert_to_header_mod_substitute(name, value) { + if let Some(m) = self.convert_to_x_spam_header_mod(name, value) { self.spam_assassin_mods.push(m); } } else if REWRITE_HEADERS.contains(name) { - if let Some(m) = self.convert_to_header_mod_update(name, value) { + if let Some(m) = self.convert_to_header_mod(name, value) { self.rewrite_mods.push(m); } } else if REPORT_HEADERS.contains(name) { - if let Some(m) = self.convert_to_header_mod_update(name, value) { + if let Some(m) = self.convert_to_header_mod(name, value) { self.report_mods.push(m); } } } - fn convert_to_header_mod_substitute( + fn convert_to_x_spam_header_mod( &mut self, name: &'a str, value: &'a str, @@ -190,17 +191,27 @@ impl<'a> HeaderRewriter<'a> { } let prepend = self.prepend.unwrap(); - Some(match self.original.get(name) { - None => HeaderMod::Add { name, value, prepend }, - Some(_) => HeaderMod::Replace { name, value, prepend }, - }) + match self.original.get(name) { + None => Some(HeaderMod::Add { name, value, prepend }), + Some(original_value) => { + // Special case ‘X-Spam-Prev-’ headers: if already present, + // modify them in place or leave them be. Other ‘X-Spam-’ + // headers are replaced, ie stripped and re-added. + let prev = "X-Spam-Prev-"; + if matches!(name.get(..prev.len()), Some(s) if s.eq_ignore_ascii_case(prev)) { + if original_value != value { + Some(HeaderMod::Modify { name, value }) + } else { + None + } + } else { + Some(HeaderMod::Replace { name, value, prepend }) + } + } + } } - fn convert_to_header_mod_update( - &mut self, - name: &'a str, - value: &'a str, - ) -> Option> { + fn convert_to_header_mod(&mut self, name: &'a str, value: &'a str) -> Option> { if !self.processed.insert(name) { return None; } @@ -297,7 +308,7 @@ pub fn replace_body( /// A header rewriting modification operation. These are intended to operate /// only on the first instance of headers occurring multiple times. -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] enum HeaderMod<'a> { Add { name: &'a str, value: &'a str, prepend: bool }, Replace { name: &'a str, value: &'a str, prepend: bool }, @@ -460,16 +471,16 @@ mod tests { rewriter.process_header("X-Spam-Flag", " NO"); rewriter.process_header("X-Spam-Flag", " YES"); - let mut mods = rewriter.spam_assassin_mods.into_iter(); - match mods.next().unwrap() { - HeaderMod::Add { name, value, prepend } => { - assert_eq!(name, "X-Spam-Flag"); - assert_eq!(value, " NO"); - assert_eq!(prepend, true); - } - _ => panic!(), - } - assert!(mods.next().is_none()); + assert_eq!( + rewriter.spam_assassin_mods, + [ + HeaderMod::Add { + name: "X-Spam-Flag", + value: " NO", + prepend: true, + }, + ] + ); } #[test] @@ -477,6 +488,7 @@ mod tests { let mut headers = HeaderMap::new(); headers.insert(String::from("x-spam-level"), String::from(" ***")); headers.insert(String::from("subject"), String::from(" original")); + headers.insert(String::from("x-spam-prev-subject"), String::from(" very original")); headers.insert(String::from("to"), String::from(" recipient@gluet.ch")); let config = Default::default(); @@ -484,45 +496,39 @@ mod tests { rewriter.process_header("X-Spam-Level", " *****"); rewriter.process_header("X-Spam-Report", " report"); rewriter.process_header("Subject", " [SPAM] original"); + rewriter.process_header("X-Spam-Prev-Subject", " very original"); rewriter.process_header("From", " sender@gluet.ch"); rewriter.process_header("To", " recipient@gluet.ch"); - let mut mods = rewriter.spam_assassin_mods.into_iter(); - match mods.next().unwrap() { - HeaderMod::Replace { name, value, prepend } => { - assert_eq!(name, "X-Spam-Level"); - assert_eq!(value, " *****"); - assert_eq!(prepend, true); - } - _ => panic!(), - } - match mods.next().unwrap() { - HeaderMod::Add { name, value, prepend } => { - assert_eq!(name, "X-Spam-Report"); - assert_eq!(value, " report"); - assert_eq!(prepend, true); - } - _ => panic!(), - } - assert!(mods.next().is_none()); - - let mut mods = rewriter.rewrite_mods.into_iter(); - match mods.next().unwrap() { - HeaderMod::Modify { name, value } => { - assert_eq!(name, "Subject"); - assert_eq!(value, " [SPAM] original"); - } - _ => panic!(), - } - match mods.next().unwrap() { - HeaderMod::Add { name, value, prepend } => { - assert_eq!(name, "From"); - assert_eq!(value, " sender@gluet.ch"); - assert_eq!(prepend, false); - } - _ => panic!(), - } - assert!(mods.next().is_none()); + assert_eq!( + rewriter.spam_assassin_mods, + [ + HeaderMod::Replace { + name: "X-Spam-Level", + value: " *****", + prepend: true, + }, + HeaderMod::Add { + name: "X-Spam-Report", + value: " report", + prepend: true, + }, + ] + ); + assert_eq!( + rewriter.rewrite_mods, + [ + HeaderMod::Modify { + name: "Subject", + value: " [SPAM] original", + }, + HeaderMod::Add { + name: "From", + value: " sender@gluet.ch", + prepend: false, + }, + ] + ); } #[test] @@ -534,14 +540,15 @@ mod tests { rewriter.process_header("Received", " from localhost by ..."); rewriter.process_header("X-Spam-Flag", " YES"); - let mut mods = rewriter.spam_assassin_mods.into_iter(); - match mods.next().unwrap() { - HeaderMod::Add { name, value, prepend } => { - assert_eq!(name, "X-Spam-Flag"); - assert_eq!(value, " YES"); - assert_eq!(prepend, false); - } - _ => panic!(), - } + assert_eq!( + rewriter.spam_assassin_mods, + [ + HeaderMod::Add { + name: "X-Spam-Flag", + value: " YES", + prepend: false, + }, + ] + ); } }