glts/spamassassin-milter
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Revisit header rewriter, prepend X-Spam- headers

Browse source
This commit is contained in:
David Bürgin 2021-08-02 18:41:36 +02:00
parent 53e66eba0d
commit f95e89caaa
9 changed files with 193 additions and 98 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
CHANGELOG.md
View file

@ -2,9 +2,12 @@
## 0.2.0 (unreleased)
* Invoke `spamc` using the absolute path `/usr/bin/spamc`. To customise this,
set the environment variable `SPAMASSASSIN_MILTER_SPAMC` to the desired path
when building the application.
* (defaults change) Invoke `spamc` using the absolute path `/usr/bin/spamc`
(instead of any executable named `spamc` in the search path). To customise
this, set the environment variable `SPAMASSASSIN_MILTER_SPAMC` to the
desired path when building the application.
* Revise header rewriting logic. Placement of `X-Spam-` headers now more
closely mirrors that received from SpamAssassin.
* Update dependencies.
## 0.1.6 (2021-05-17)

47
src/client.rs
View file

@ -333,7 +333,7 @@ fn replace_body(
#[cfg(test)]
mod tests {
use super::*;
use std::{cell::RefCell, collections::HashSet};
use std::cell::RefCell;
#[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
struct MockSpamc {
@ -380,6 +380,7 @@ mod tests {
#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
enum Action {
AddHeader(String, String),
InsertHeader(usize, String, String),
ReplaceHeader(String, usize, Option<String>),
AppendBodyChunk(Vec<u8>),
SetErrorReply(String, Option<String>, Vec<String>),
@ -403,6 +404,12 @@ mod tests {
))
}
fn insert_header(&self, index: usize, name: &str, value: &str) -> milter::Result<()> {
Ok(self.called.borrow_mut().push(
Action::InsertHeader(index, name.to_owned(), value.to_owned())
))
}
fn replace_header(&self, name: &str, index: usize, value: Option<&str>) -> milter::Result<()> {
Ok(self.called.borrow_mut().push(
Action::ReplaceHeader(name.to_owned(), index, value.map(|v| v.to_owned()))
@ -427,10 +434,6 @@ mod tests {
unimplemented!();
}
fn insert_header(&self, _: usize, _: &str, _: &str) -> milter::Result<()> {
unimplemented!();
}
fn quarantine(&self, _: &str) -> milter::Result<()> {
unimplemented!();
}
@ -524,15 +527,14 @@ mod tests {
assert_eq!(status, Status::Reject);
let called = actions.called.borrow();
assert_eq!(called.len(), 1);
assert_eq!(
called.first().unwrap(),
&Action::SetErrorReply(
let expected = [
Action::SetErrorReply(
"550".into(),
Some("5.7.1".into()),
vec!["Spam message refused".into()]
)
);
vec!["Spam message refused".into()],
),
];
assert_eq!(called.as_slice(), expected.as_ref());
}
#[test]
@ -552,17 +554,14 @@ mod tests {
assert_eq!(status, Status::Accept);
let called = actions.called.borrow();
assert_eq!(called.len(), 4);
let mut expected = HashSet::new();
expected.insert(Action::AddHeader("X-Spam-Flag".into(), " YES".into()));
expected.insert(Action::ReplaceHeader("X-Spam-Level".into(), 1, Some(" *****".into())));
expected.insert(Action::ReplaceHeader("X-Spam-Report".into(), 1, None));
expected.insert(Action::AppendBodyChunk(b"Report".to_vec()));
for a in called.iter() {
assert!(expected.contains(&a));
}
let expected = [
Action::ReplaceHeader("X-Spam-Level".into(), 1, None),
Action::InsertHeader(0, "X-Spam-Level".into(), " *****".into()),
Action::InsertHeader(0, "X-Spam-Flag".into(), " YES".into()),
Action::ReplaceHeader("x-spam-report".into(), 1, None),
Action::AppendBodyChunk(b"Report".to_vec()),
];
assert_eq!(called.as_slice(), expected.as_ref());
}
#[test]
@ -581,7 +580,7 @@ mod tests {
assert_eq!(status, Status::Accept);
let called = actions.called.borrow();
assert!(called.contains(&Action::AddHeader("X-Spam-Level".into(), " *****".into())));
assert!(called.contains(&Action::InsertHeader(0, "X-Spam-Level".into(), " *****".into())));
assert!(!called.contains(&Action::AppendBodyChunk(b"Report".to_vec())));
}
}

11
src/collections.rs
View file

@ -77,10 +77,6 @@ where
Self { map: StrVecMap::new() }
}
pub fn iter(&self) -> impl Iterator<Item = &E> {
self.map.keys()
}
pub fn contains<Q: AsRef<str>>(&self, key: Q) -> bool {
self.map.contains_key(key)
}
@ -148,9 +144,8 @@ mod tests {
assert!(set.insert("KEY2"));
assert!(!set.insert("key1"));
let mut iter = set.iter();
assert_eq!(iter.next(), Some(&"KEY1"));
assert_eq!(iter.next(), Some(&"KEY2"));
assert_eq!(iter.next(), None);
assert!(set.contains("key1"));
assert!(set.contains("key2"));
assert!(!set.contains("key3"));
}
}

173
src/email.rs
View file

@ -111,17 +111,6 @@ pub fn is_spam_assassin_header(name: &str) -> bool {
pub type HeaderMap = StrVecMap<String, String>;
pub type HeaderSet<'e> = StrVecSet<&'e str>;
// Selected subset of ‘X-Spam-’ headers for which we assume responsibility.
pub static SPAM_ASSASSIN_HEADERS: Lazy<HeaderSet<'static>> = Lazy::new(|| {
let mut h = HeaderSet::new();
h.insert("X-Spam-Checker-Version");
h.insert("X-Spam-Flag");
h.insert("X-Spam-Level");
h.insert("X-Spam-Status");
h.insert("X-Spam-Report");
h
});
pub static REWRITE_HEADERS: Lazy<HeaderSet<'static>> = Lazy::new(|| {
let mut h = HeaderSet::new();
h.insert("Subject");
@ -145,6 +134,7 @@ pub static REPORT_HEADERS: Lazy<HeaderSet<'static>> = Lazy::new(|| {
pub struct HeaderRewriter<'a> {
original: HeaderMap,
processed: HeaderSet<'a>,
prepend: Option<bool>,
spam_flag: Option<bool>,
spam_assassin_mods: Vec<HeaderMod<'a>>,
rewrite_mods: Vec<HeaderMod<'a>>,
@ -157,6 +147,7 @@ impl<'a> HeaderRewriter<'a> {
Self {
original,
processed: HeaderSet::new(),
prepend: None,
spam_flag: None,
spam_assassin_mods: vec![],
rewrite_mods: vec![],
@ -166,36 +157,59 @@ impl<'a> HeaderRewriter<'a> {
}
pub fn process_header(&mut self, name: &'a str, value: &'a str) {
// Assumes that the value is normalised to using CRLF line breaks, and
// includes leading whitespace.
// The very first header seen determines if ‘X-Spam-’ headers are
// prepended or appended to the existing header.
self.prepend.get_or_insert_with(|| is_spam_assassin_header(name));
if name.eq_ignore_ascii_case("X-Spam-Flag") {
self.spam_flag.get_or_insert_with(|| value.trim().eq_ignore_ascii_case("YES"));
}
if is_spam_assassin_header(name) {
if let Some(m) = self.convert_to_header_mod(name, value) {
if let Some(m) = self.convert_to_header_mod_substitute(name, value) {
self.spam_assassin_mods.push(m);
}
} else if REWRITE_HEADERS.contains(name) {
if let Some(m) = self.convert_to_header_mod(name, value) {
if let Some(m) = self.convert_to_header_mod_update(name, value) {
self.rewrite_mods.push(m);
}
} else if REPORT_HEADERS.contains(name) {
if let Some(m) = self.convert_to_header_mod(name, value) {
if let Some(m) = self.convert_to_header_mod_update(name, value) {
self.report_mods.push(m);
}
}
}
fn convert_to_header_mod(&mut self, name: &'a str, value: &'a str) -> Option<HeaderMod<'a>> {
fn convert_to_header_mod_substitute(
&mut self,
name: &'a str,
value: &'a str,
) -> Option<HeaderMod<'a>> {
if !self.processed.insert(name) {
return None;
}
let prepend = self.prepend.unwrap();
Some(match self.original.get(name) {
None => HeaderMod::Add { name, value, prepend },
Some(_) => HeaderMod::Replace { name, value, prepend },
})
}
fn convert_to_header_mod_update(
&mut self,
name: &'a str,
value: &'a str,
) -> Option<HeaderMod<'a>> {
if !self.processed.insert(name) {
return None;
}
match self.original.get(name) {
None => Some(HeaderMod::Add { name, value }),
None => Some(HeaderMod::Add { name, value, prepend: false }),
Some(original_value) => {
if original_value != value {
Some(HeaderMod::Replace { name, value })
Some(HeaderMod::Modify { name, value })
} else {
None
}
@ -212,12 +226,18 @@ impl<'a> HeaderRewriter<'a> {
id: &str,
actions: &impl ActionContext,
) -> milter::Result<()> {
execute_mods(id, self.spam_assassin_mods.iter(), actions, self.config)?;
if self.prepend.unwrap_or(false) {
// Prepend ‘X-Spam-’ headers in reverse order, so that they appear
// in the order received from SpamAssassin.
execute_mods(id, self.spam_assassin_mods.iter().rev(), actions, self.config)?;
} else {
execute_mods(id, self.spam_assassin_mods.iter(), actions, self.config)?;
}
// Delete certain incoming SpamAssassin headers not returned by
// SpamAssassin, to get rid of foreign `X-Spam-Flag` etc. headers.
let deletions = SPAM_ASSASSIN_HEADERS.iter()
.filter(|n| self.original.contains_key(n) && !self.processed.contains(n))
// Delete all incoming ‘X-Spam-’ headers not returned by SpamAssassin to
// get rid of foreign `X-Spam-Flag` etc. headers.
let deletions = self.original.keys()
.filter(|n| is_spam_assassin_header(n) && !self.processed.contains(n))
.map(|name| HeaderMod::Delete { name })
.collect::<Vec<_>>();
execute_mods(id, deletions.iter(), actions, self.config)
@ -279,8 +299,9 @@ pub fn replace_body(
/// only on the first instance of headers occurring multiple times.
#[derive(Clone, Copy, Debug)]
enum HeaderMod<'a> {
Add { name: &'a str, value: &'a str },
Replace { name: &'a str, value: &'a str },
Add { name: &'a str, value: &'a str, prepend: bool },
Replace { name: &'a str, value: &'a str, prepend: bool },
Modify { name: &'a str, value: &'a str },
Delete { name: &'a str },
}
@ -290,21 +311,45 @@ impl HeaderMod<'_> {
// The milter library is smart enough to treat the name in a
// case-insensitive manner, eg ‘Subject’ may replace ‘sUbject’.
match self {
Add { name, value } => actions.add_header(name, &ensure_lf(value)),
Replace { name, value } => actions.replace_header(name, 1, Some(&ensure_lf(value))),
Delete { name } => actions.replace_header(name, 1, None),
match *self {
Add { name, value, prepend } => add_header(actions, name, value, prepend),
Replace { name, value, prepend } => {
delete_header(actions, name)?;
add_header(actions, name, value, prepend)?;
Ok(())
}
Modify { name, value } => actions.replace_header(name, 1, Some(&ensure_lf(value))),
Delete { name } => delete_header(actions, name),
}
}
}
fn add_header(
actions: &impl ActionContext,
name: &str,
value: &str,
prepend: bool,
) -> milter::Result<()> {
if prepend {
actions.insert_header(0, name, &ensure_lf(value))
} else {
actions.add_header(name, &ensure_lf(value))
}
}
fn delete_header(actions: &impl ActionContext, name: &str) -> milter::Result<()> {
actions.replace_header(name, 1, None)
}
impl Display for HeaderMod<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
use HeaderMod::*;
match self {
Add { name, .. } => write!(f, "add header \"{}\"", name),
Replace { name, .. } => write!(f, "replace header \"{}\"", name),
Replace { name, .. } | Modify { name, .. } => {
write!(f, "replace header \"{}\"", name)
}
Delete { name } => write!(f, "delete header \"{}\"", name),
}
}
@ -417,9 +462,10 @@ mod tests {
let mut mods = rewriter.spam_assassin_mods.into_iter();
match mods.next().unwrap() {
HeaderMod::Add { name, value } => {
HeaderMod::Add { name, value, prepend } => {
assert_eq!(name, "X-Spam-Flag");
assert_eq!(value, " NO");
assert_eq!(prepend, true);
}
_ => panic!(),
}
@ -427,24 +473,75 @@ mod tests {
}
#[test]
fn header_rewriter_replaces_different_values() {
fn header_rewriter_adds_and_replaces_headers() {
let mut headers = HeaderMap::new();
headers.insert(String::from("x-spam-level"), String::from(" ***"));
headers.insert(String::from("x-spam-report"), String::from(" original"));
headers.insert(String::from("subject"), String::from(" original"));
headers.insert(String::from("to"), String::from(" recipient@gluet.ch"));
let config = Default::default();
let mut rewriter = HeaderRewriter::new(headers, &config);
rewriter.process_header("X-Spam-Level", " ***");
rewriter.process_header("X-Spam-Report", " new");
rewriter.process_header("X-Spam-Level", " *****");
rewriter.process_header("X-Spam-Report", " report");
rewriter.process_header("Subject", " [SPAM] 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 } => {
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, " new");
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());
}
#[test]
fn header_rewriter_adds_appended_headers() {
let headers = HeaderMap::new();
let config = Default::default();
let mut rewriter = HeaderRewriter::new(headers, &config);
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!(),
}
}
}

2
tests/common/mod.rs
View file

@ -83,7 +83,7 @@ where
// Crude handling of the `spamc` client protocol: strip off everything
// before and including the first "\r\n\r\n".
let i = msg.find("\r\n\r\n").expect("spamc protocol header missing");
msg.replace_range(..i + 4, "");
msg.drain(..i + 4);
match f(msg) {
// Again very basic handling of the `spamd` server protocol: add a

5
tests/ham_message.lua
View file

@ -67,8 +67,9 @@ local err = mt.eom(conn)
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_ACCEPT)
assert(mt.eom_check(conn, MT_HDRCHANGE, "X-Spam-Checker-Version", " MyChecker 1.0.0"))
assert(mt.eom_check(conn, MT_HDRADD, "X-Spam-Custom", " Custom-Value"))
assert(mt.eom_check(conn, MT_HDRINSERT, "X-Spam-Custom", " Custom-Value"))
assert(mt.eom_check(conn, MT_HDRDELETE, "X-Spam-Checker-Version"))
assert(mt.eom_check(conn, MT_HDRINSERT, "X-Spam-Checker-Version", " MyChecker 1.0.0"))
assert(mt.eom_check(conn, MT_HDRDELETE, "X-Spam-Report"))
local err = mt.disconnect(conn)

26
tests/ham_message.rs
View file

@ -10,20 +10,18 @@ fn ham_message() {
let config = builder.build();
let server = spawn_mock_spamd_server(SPAMD_PORT, |ham| {
Ok(ham
.replacen(
"X-Spam-Checker-Version: BogusChecker 1.0.0\r\n",
"X-Spam-Checker-Version: MyChecker 1.0.0\r\n",
1,
)
.replacen("X-Spam-Report: Bogus report\r\n", "", 1)
.replacen(
"\r\n\r\n",
"\r\n\
X-Spam-Custom: Custom-Value\r\n\
\r\n",
1,
))
let mut ham = ham
.replacen("X-Spam-Checker-Version: BogusChecker 1.0.0\r\n", "", 1)
.replacen("X-Spam-Report: Bogus report\r\n", "", 1);
// Prepend replaced and newly added SpamAssassin headers.
ham.insert_str(
0,
"X-Spam-Checker-Version: MyChecker 1.0.0\r\n\
X-Spam-Custom: Custom-Value\r\n",
);
Ok(ham)
});
let miltertest = spawn_miltertest_runner(file!());

5
tests/spam_message.lua
View file

@ -67,11 +67,12 @@ local err = mt.eom(conn)
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_ACCEPT)
assert(mt.eom_check(conn, MT_HDRCHANGE, "Subject", " [SPAM] Test message"))
assert(mt.eom_check(conn, MT_HDRCHANGE, "X-Spam-Checker-Version", " MyChecker 1.0.0"))
assert(mt.eom_check(conn, MT_HDRDELETE, "X-Spam-Checker-Version"))
assert(mt.eom_check(conn, MT_HDRADD, "X-Spam-Checker-Version", " MyChecker 1.0.0"))
assert(mt.eom_check(conn, MT_HDRADD, "X-Spam-Flag", " YES"))
assert(mt.eom_check(conn, MT_HDRADD, "X-Spam-Custom", " Custom-Value"))
assert(mt.eom_check(conn, MT_HDRDELETE, "X-Spam-Report"))
assert(mt.eom_check(conn, MT_HDRCHANGE, "Subject", " [SPAM] Test message"))
assert(mt.eom_check(conn, MT_HDRADD, "Content-Type", " multipart/mixed; ..."))
assert(mt.eom_check(conn, MT_BODYCHANGE, "Spam detection software has identified ..."))

13
tests/spam_message.rs
View file

@ -11,21 +11,22 @@ fn spam_message() {
let server = spawn_mock_spamd_server(SPAMD_PORT, |spam| {
let mut spam = spam
.replacen("Subject: Test message\r\n", "Subject: [SPAM] Test message\r\n", 1)
.replacen(
"X-Spam-Checker-Version: BogusChecker 1.0.0\r\n",
"X-Spam-Checker-Version: MyChecker 1.0.0\r\n",
1,
)
.replacen("X-Spam-Checker-Version: BogusChecker 1.0.0\r\n", "", 1)
.replacen("X-Spam-Report: Bogus report\r\n", "", 1)
.replacen(
"\r\n\r\n",
"\r\n\
X-Spam-Checker-Version: MyChecker 1.0.0\r\n\
X-Spam-Flag: YES\r\n\
X-Spam-Custom: Custom-Value\r\n\
Content-Type: multipart/mixed; ...\r\n\
\r\n",
1,
)
.replacen(
"Subject: Test message\r\n",
"Subject: [SPAM] Test message\r\n",
1,
);
// Replace the message body with the SpamAssassin ‘report’.