Refactor header handling, bump version to 0.0.3

This commit is contained in:
David Bürgin 2020-02-14 09:03:38 +01:00
parent de915de167
commit 1432560004
24 changed files with 359 additions and 197 deletions

2
Cargo.lock generated
View file

@ -170,7 +170,7 @@ checksum = "2439c63f3f6139d1b57529d16bc3b8bb855230c8efcc5d3a896c8bea7c3b1e84"
[[package]]
name = "spamassassin-milter"
version = "0.0.2"
version = "0.0.3"
dependencies = [
"chrono",
"clap",

View file

@ -1,6 +1,6 @@
[package]
name = "spamassassin-milter"
version = "0.0.2" # remember to update html_root_url
version = "0.0.3" # remember to update html_root_url and VERSION
description = "Milter for spam filtering with SpamAssassin"
authors = ["David Bürgin <dbuergin@gluet.ch>"]
edition = "2018"

View file

@ -28,13 +28,13 @@ configuration and integration approaches.
This application can be used as a replacement for [spamass-milt]; it has a
reduced feature set, but it should be satisfactory for a personal mail server
setup. SpamAssassin Milter has been used in such a setup together with Postfix,
SpamAssassin, and for delivery [Dovecot] with LMTP and the [Sieve] plugin.
SpamAssassin, and for delivery [Dovecot] with LMTP and the [Sieve plugin].
[Apache SpamAssassin]: https://spamassassin.apache.org
[Postfix]: http://www.postfix.org
[spamass-milt]: https://savannah.nongnu.org/projects/spamass-milt/
[Dovecot]: https://dovecot.org
[Sieve]: https://doc.dovecot.org/configuration_manual/sieve/
[Sieve plugin]: https://doc.dovecot.org/configuration_manual/sieve/
## Building
@ -119,6 +119,16 @@ and the server.
The main SpamAssassin configuration file is `/etc/spamassassin/local.conf`. See
`perldoc Mail::SpamAssassin::Conf` for detailed information.
The phrase flagged as spam, which appears throughout this documentation,
refers to whether SpamAssassin has marked a message as being spam by adding the
header `X-Spam-Flag: YES`. A message being spam or ham (not spam) is a binary
property. The classification threshold can be configured by setting
`required_score` as follows:
```
required_score 9.0
```
By default, SpamAssassin creates reports for messages it recognises as spam.
These reports replace the message body, that is, the message body is rewritten
to present a report instead of the original message, and the original message is
@ -176,8 +186,8 @@ Finally, a pitfall of `spamc` deserves highlighting: `spamc` by default tries to
resist failure to an extent that it will not indicate failure even if it cannot
connect to SpamAssassin server at all (apart from warnings logged to syslog)! If
it cannot connect to the server, it simply echoes what it received, and so masks
the error condition. This mechanism is labelled safe fallback and should be
disabled once the system is set up and working well. Set the following flag:
the error condition. This behaviour is labelled safe fallback and is perhaps
best disabled once the system is set up. Set the following flag:
```
--no-safe-fallback

View file

@ -1,4 +1,4 @@
.TH SPAMASSASSIN-MILTER 8 2020-02-09
.TH SPAMASSASSIN-MILTER 8 2020-02-14
.SH NAME
spamassassin-milter \- milter for spam filtering with SpamAssassin
.SH SYNOPSIS

View file

@ -39,7 +39,7 @@ fn handle_negotiate(
}
}
let req_protocol_opts = ProtocolOpts::SKIP;
let req_protocol_opts = ProtocolOpts::SKIP | ProtocolOpts::HEADER_LEADING_SPACE;
assert!(actions.contains(req_actions), "required milter actions not supported");
assert!(protocol_opts.contains(req_protocol_opts), "required milter protocol options not supported");
@ -130,6 +130,10 @@ fn handle_data(mut ctx: Context<Connection>) -> milter::Result<Status> {
let client_ip = conn.client_ip.to_string();
// Note that when SpamAssassin reports are enabled (`report_safe 1`), the
// forged 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()?;
client.send_envelope_recipients()?;
client.send_forged_received_header(

View file

@ -23,6 +23,7 @@ pub struct Spamc {
}
impl Spamc {
// TODO Is the name enough? Do we need a path, eg when run as root?
const SPAMC_PROGRAM: &'static str = "spamc";
pub fn new(spamc_args: &'static [String]) -> Self {
@ -35,7 +36,7 @@ impl Spamc {
impl Process for Spamc {
fn connect(&mut self) -> Result<()> {
// `Command::spawn` always succeeds when spamc can be invoked, even if
// `Command::spawn` always succeeds when `spamc` can be invoked, even if
// logically the command is invalid, eg if it uses non-existing options.
let child = Command::new(Spamc::SPAMC_PROGRAM)
.args(self.spamc_args)
@ -50,17 +51,20 @@ impl Process for Spamc {
fn writer(&mut self) -> &mut dyn Write {
let spamc = self.spamc.as_mut().expect("spamc process not started");
spamc.stdin.as_mut().unwrap()
}
fn finish(&mut self) -> Result<Vec<u8>> {
let spamc = self.spamc.take().expect("spamc process not started");
let output = spamc.wait_with_output()?;
// TODO put signal/exit code in message
if output.status.success() {
Ok(output.stdout)
} else {
Err(Error::IoError(String::from("spamc returned error exit code")))
Err(Error::Io(String::from("spamc returned error exit code")))
}
}
@ -71,10 +75,12 @@ impl Process for Spamc {
impl Drop for Spamc {
fn drop(&mut self) {
// Kill spamc: a child process will continue to run even after its
// `Child` handle has gone out of scope.
// Kill (and wait on) `spamc`: a child process will continue to run even
// after its `Child` handle has gone out of scope.
if let Some(spamc) = self.spamc.as_mut() {
// The results are no longer of interest at this point.
let _ = spamc.kill();
let _ = spamc.wait();
}
}
}
@ -83,7 +89,7 @@ pub struct Client {
process: Box<dyn Process>,
sender: String,
recipients: Vec<String>,
headers: HeaderMap<'static>, // `'static` keys in canonical form
headers: HeaderMap,
bytes: usize,
}
@ -98,23 +104,23 @@ impl Client {
}
}
pub fn bytes_written(&self) -> usize {
self.bytes
pub fn add_recipient(&mut self, recipient: String) {
self.recipients.push(recipient);
}
pub fn add_recipient(&mut self, rcpt: String) {
self.recipients.push(rcpt);
pub fn bytes_written(&self) -> usize {
self.bytes
}
pub fn connect(&mut self) -> Result<()> {
self.process.connect()
}
// Implementation note: the send operations can all fail with an IO error
// and so return a Result that is then simply unwrapped with `?` in the
// `callbacks` module. That is, in normal circumstances we dont expect this
// to occur, only in abnormal circumstances such as when wrong flags where
// passed to spamc!
// Implementation note: The send operations all may fail with an I/O error,
// and so return a `Result<()>` that is then unceremoniously unwrapped with
// `?` in the callback functions. This is acceptable, because we dont
// expect this to happen in normal circumstances, only when something is
// wrong with `spamc` configuration or operation.
pub fn send_envelope_sender(&mut self) -> Result<()> {
let buf = format!("X-Envelope-From: {}\r\n", self.sender);
@ -163,22 +169,17 @@ impl Client {
}
pub fn send_header(&mut self, name: &str, value: &str) -> Result<()> {
// Record selected SpamAssassin headers, but never pass any on.
if email::is_spam_assassin_header(name) {
if let Some(canonical_name) = email::SPAM_ASSASSIN_HEADERS.get(name) {
self.headers.insert_if_absent(canonical_name, email::ensure_crlf(value));
}
return Ok(());
}
// Remember that the value includes leading whitespace, as weve
// requested in the milter protocol options. This lets us keep
// whitespace exactly as is.
let value = email::ensure_crlf(value);
let buf = format!("{}: {}\r\n", name, value);
let buf = format!("{}:{}\r\n", name, value);
if let Some(canonical_name) = email::REWRITE_HEADERS.get(name)
.or_else(|| email::REPORT_HEADERS.get(name))
if email::is_spam_assassin_header(name)
|| email::REWRITE_HEADERS.contains(name)
|| email::REPORT_HEADERS.contains(name)
{
self.headers.insert_if_absent(canonical_name, value);
self.headers.insert_if_absent(name, value);
}
self.bytes += self.process.writer().write(buf.as_bytes())?;
@ -207,6 +208,7 @@ impl Client {
let output = match self.process.finish() {
Ok(output) => output,
Err(e) => {
// TODO revise error message (not failed to wait?)
eprintln!("{}: failed to wait for spamc to exit: {}", id, e);
return Ok(Status::Tempfail);
}
@ -215,7 +217,7 @@ impl Client {
let email = match Email::parse(&output) {
Ok(email) => email,
Err(e) => {
eprintln!("{}: {}", id, e);
eprintln!("{}: invalid response from spamc: {}", id, e);
return Ok(Status::Tempfail);
}
};
@ -243,7 +245,6 @@ impl Client {
}
}
verbose!(config, "{}: finished processing", id);
Ok(Status::Accept)
}
}
@ -392,8 +393,8 @@ mod tests {
let spamc = MockSpamc::new();
let mut client = Client::new(spamc, String::from("sender"));
client.send_header("name1", "value1").unwrap();
client.send_header("name2", "value2\n\tcontinued").unwrap();
client.send_header("name1", " value1").unwrap();
client.send_header("name2", " value2\n\tcontinued").unwrap();
client.send_eoh().unwrap();
client.send_body_chunk(b"body").unwrap();
@ -404,17 +405,6 @@ mod tests {
);
}
#[test]
fn client_send_no_spam_assassin_headers() {
let spamc = MockSpamc::new();
let mut client = Client::new(spamc, String::from("sender"));
client.send_header("x-spam-flag", "YES").unwrap();
client.send_header("x-spam-bogus", "x").unwrap();
assert_eq!(client.bytes_written(), 0);
}
#[test]
fn client_process_invalid_response() {
let spamc = MockSpamc::with_output(b"invalid message response".to_vec());
@ -439,13 +429,16 @@ mod tests {
let status = client.process("id", &actions, &config).unwrap();
assert_eq!(status, Status::Reject);
let called = actions.called.borrow();
assert_eq!(called.len(), 1);
assert_eq!(
actions.called.borrow().first(),
Some(&Action::SetErrorReply(
called.first().unwrap(),
&Action::SetErrorReply(
"550".into(),
Some("5.7.1".into()),
vec!["Spam message refused".into()]
))
)
);
}
@ -458,8 +451,8 @@ mod tests {
let config = Default::default();
let mut client = Client::new(spamc, String::from("sender"));
client.send_header("x-spam-level", "*").unwrap();
client.send_header("x-spam-report", "...").unwrap();
client.send_header("x-spam-level", " *").unwrap();
client.send_header("x-spam-report", " ...").unwrap();
let status = client.process("id", &actions, &config).unwrap();
@ -469,8 +462,8 @@ mod tests {
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::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()));

View file

@ -1,38 +1,43 @@
use std::mem;
/// An array map with ASCII-case-insensitive `&str` keys.
// Design note: Header handling requires ASCII-case-insensitive map keys. This
// complexity could have been pushed out to the map key type (a newtype
// implementing `Hash` and `Eq`) while using an ordinary `HashMap`. Weve found
// it simpler to extract the complexity into these custom collections instead,
// and use plain strings in the application logic.
/// A vector map with ASCII-case-insensitive `AsRef<str>` keys.
#[derive(Clone, Debug, Default)]
pub struct StrVecMap<'k, V> {
entries: Vec<(&'k str, V)>,
pub struct StrVecMap<K, V> {
entries: Vec<(K, V)>,
}
impl<'k, V> StrVecMap<'k, V> {
impl<K, V> StrVecMap<K, V>
where
K: AsRef<str>,
{
pub fn new() -> Self {
Self { entries: Default::default() }
}
pub fn iter(&self) -> impl Iterator<Item = &(&str, V)> {
self.entries.iter()
pub fn iter(&self) -> impl Iterator<Item = (&K, &V)> {
self.entries.iter().map(|e| (&e.0, &e.1))
}
pub fn keys(&self) -> impl Iterator<Item = &str> {
pub fn keys(&self) -> impl Iterator<Item = &K> {
self.iter().map(|e| e.0)
}
pub fn contains_key(&self, key: &str) -> bool {
self.iter().any(|e| e.0.eq_ignore_ascii_case(key))
self.iter().any(|e| e.0.as_ref().eq_ignore_ascii_case(key))
}
pub fn get(&self, key: &str) -> Option<&V> {
self.iter().find(|e| e.0.eq_ignore_ascii_case(key)).map(|e| &e.1)
self.iter().find(|e| e.0.as_ref().eq_ignore_ascii_case(key)).map(|e| e.1)
}
pub fn get_key_value(&self, key: &str) -> Option<(&str, &V)> {
self.iter().find(|e| e.0.eq_ignore_ascii_case(key)).map(|e| (e.0, &e.1))
}
pub fn insert(&mut self, key: &'k str, value: V) -> Option<V> {
match self.entries.iter_mut().find(|e| e.0.eq_ignore_ascii_case(key)) {
pub fn insert(&mut self, key: K, value: V) -> Option<V> {
match self.entries.iter_mut().find(|e| e.0.as_ref().eq_ignore_ascii_case(key.as_ref())) {
Some(e) => Some(mem::replace(&mut e.1, value)),
None => {
self.entries.push((key, value));
@ -40,29 +45,39 @@ impl<'k, V> StrVecMap<'k, V> {
}
}
}
}
pub fn insert_if_absent(&mut self, key: &'k str, value: V) -> Option<V> {
if self.contains_key(key) {
// Ad-hoc implementation, only does `to_owned` on the lookup key if the key is
// not yet present.
impl<V> StrVecMap<String, V> {
pub fn insert_if_absent<K>(&mut self, key: K, value: V) -> Option<V>
where
K: AsRef<str>,
{
if self.contains_key(key.as_ref()) {
Some(value)
} else {
self.entries.push((key, value));
self.entries.push((key.as_ref().to_owned(), value));
None
}
}
}
/// An array set containing ASCII-case-insensitive `&str` elements.
/// A vector set containing ASCII-case-insensitive `AsRef<str>` elements.
#[derive(Clone, Debug, Default)]
pub struct StrVecSet<'e> {
map: StrVecMap<'e, ()>,
pub struct StrVecSet<E> {
map: StrVecMap<E, ()>,
}
impl<'e> StrVecSet<'e> {
impl<E> StrVecSet<E>
where
E: AsRef<str>,
{
pub fn new() -> Self {
Default::default()
Self { map: StrVecMap::new() }
}
pub fn iter(&self) -> impl Iterator<Item = &str> {
pub fn iter(&self) -> impl Iterator<Item = &E> {
self.map.keys()
}
@ -70,11 +85,7 @@ impl<'e> StrVecSet<'e> {
self.map.contains_key(key)
}
pub fn get(&self, key: &str) -> Option<&str> {
self.map.get_key_value(key).map(|(k, _)| k)
}
pub fn insert(&mut self, key: &'e str) -> bool {
pub fn insert(&mut self, key: E) -> bool {
self.map.insert(key, ()).is_none()
}
}
@ -88,17 +99,17 @@ mod tests {
let mut map = StrVecMap::new();
map.insert("KEY1", 1);
assert_eq!(map.contains_key("key1"), true);
assert_eq!(map.contains_key("key2"), false);
assert!(map.contains_key("key1"));
assert!(!map.contains_key("key2"));
}
#[test]
fn map_get_key_value() {
fn map_get() {
let mut map = StrVecMap::new();
map.insert("KEY1", 1);
assert_eq!(map.get_key_value("key1"), Some(("KEY1", &1)));
assert_eq!(map.get_key_value("key2"), None);
assert_eq!(map.get("key1"), Some(&1));
assert_eq!(map.get("key2"), None);
}
#[test]
@ -110,8 +121,8 @@ mod tests {
assert_eq!(map.insert("key1", 3), Some(1));
let mut iter = map.iter();
assert_eq!(iter.next(), Some(&("KEY1", 3)));
assert_eq!(iter.next(), Some(&("KEY2", 2)));
assert_eq!(iter.next(), Some((&"KEY1", &3)));
assert_eq!(iter.next(), Some((&"KEY2", &2)));
assert_eq!(iter.next(), None);
}
@ -124,31 +135,22 @@ mod tests {
assert_eq!(map.insert_if_absent("key1", 3), Some(3));
let mut iter = map.iter();
assert_eq!(iter.next(), Some(&("KEY1", 1)));
assert_eq!(iter.next(), Some(&("KEY2", 2)));
assert_eq!(iter.next(), Some((&String::from("KEY1"), &1)));
assert_eq!(iter.next(), Some((&String::from("KEY2"), &2)));
assert_eq!(iter.next(), None);
}
#[test]
fn set_get() {
let mut set = StrVecSet::new();
set.insert("KEY1");
assert_eq!(set.get("key1"), Some("KEY1"));
assert_eq!(set.get("key2"), None);
}
#[test]
fn set_insert() {
let mut set = StrVecSet::new();
assert_eq!(set.insert("KEY1"), true);
assert_eq!(set.insert("KEY2"), true);
assert_eq!(set.insert("key1"), false);
assert!(set.insert("KEY1"));
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(), Some(&"KEY1"));
assert_eq!(iter.next(), Some(&"KEY2"));
assert_eq!(iter.next(), None);
}
}

View file

@ -92,7 +92,7 @@ impl Default for ConfigBuilder {
trusted_networks: Default::default(),
auth_untrusted: Default::default(),
spamc_args: Default::default(),
max_message_size: 512_000,
max_message_size: 512_000, // same as in `spamc`
dry_run: Default::default(),
reject_spam: Default::default(),
preserve_headers: Default::default(),

View file

@ -80,14 +80,13 @@ fn parse_header_line(bytes: &[u8]) -> Result<Header<'_>> {
return Err(Error::ParseEmail);
}
let ascii_whitespace: &[_] = &[' ', '\t'];
let value = (&value[1..]).trim_start_matches(ascii_whitespace);
let value = &value[1..];
Ok(Header { name, value })
}
pub fn ensure_crlf(s: &str) -> String {
// For symmetry, ensure existing occurrences of b"\r\n" remain unchanged.
// For symmetry, ensure existing occurrences of "\r\n" remain unchanged.
s.split('\n')
.map(|line| match line.as_bytes().last() {
Some(&last) if last == b'\r' => &line[..(line.len() - 1)],
@ -108,8 +107,9 @@ pub fn is_spam_assassin_header(name: &str) -> bool {
name[..cmp::min(prefix.len(), name.len())].eq_ignore_ascii_case(prefix)
}
pub type HeaderMap<'k> = StrVecMap<'k, String>; // values use CRLF line breaks
pub type HeaderSet<'e> = StrVecSet<'e>;
// Values use CRLF line breaks and include leading whitespace.
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(|| {
@ -137,12 +137,13 @@ pub static REPORT_HEADERS: Lazy<HeaderSet<'static>> = Lazy::new(|| {
h
});
/// A header rewriter going between incoming headers, and headers returned from
/// the SpamAssassin service. Operates only on the first occurrence of headers
/// with the same name.
/// A header rewriter that processes headers returned by SpamAssassin, and
/// computes and applies modifications by referring back to the original set of
/// headers. The rewriter operates only on the first occurrence of headers with
/// the same name.
#[derive(Clone, Debug)]
pub struct HeaderRewriter<'a> {
original: HeaderMap<'a>,
original: HeaderMap,
processed: HeaderSet<'a>,
spam_assassin_mods: Vec<HeaderMod<'a>>,
rewrite_mods: Vec<HeaderMod<'a>>,
@ -151,7 +152,7 @@ pub struct HeaderRewriter<'a> {
}
impl<'a> HeaderRewriter<'a> {
pub fn new(original: HeaderMap<'a>, config: &'a Config) -> Self {
pub fn new(original: HeaderMap, config: &'a Config) -> Self {
Self {
original,
processed: HeaderSet::new(),
@ -163,7 +164,8 @@ 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.
// Assumes that the value is normalised to using CRLF line breaks, and
// includes leading whitespace.
if is_spam_assassin_header(name) {
if let Some(m) = self.convert_to_header_mod(name, value) {
self.spam_assassin_mods.push(m);
@ -201,18 +203,27 @@ impl<'a> HeaderRewriter<'a> {
self.spam_assassin_mods.iter().any(|m| match m {
Add { name, value } | Replace { name, value } => {
name.eq_ignore_ascii_case("X-Spam-Flag") && value.eq_ignore_ascii_case("YES")
name.eq_ignore_ascii_case("X-Spam-Flag") && value.trim().eq_ignore_ascii_case("YES")
}
_ => false,
})
}
pub fn rewrite_report_headers(
pub fn rewrite_spam_assassin_headers(
&self,
id: &str,
actions: &impl ActionContext,
) -> milter::Result<()> {
execute_mods(id, self.report_mods.iter(), actions, self.config)
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))
.map(|name| HeaderMod::Delete { name })
.collect::<Vec<_>>();
execute_mods(id, deletions.iter(), actions, self.config)
}
pub fn rewrite_rewrite_headers(
@ -223,20 +234,12 @@ impl<'a> HeaderRewriter<'a> {
execute_mods(id, self.rewrite_mods.iter(), actions, self.config)
}
pub fn rewrite_spam_assassin_headers(
pub fn rewrite_report_headers(
&self,
id: &str,
actions: &impl ActionContext,
) -> milter::Result<()> {
execute_mods(id, self.spam_assassin_mods.iter(), actions, self.config)?;
// Delete incoming SpamAssassin headers not returned by SpamAssassin.
let deletions = SPAM_ASSASSIN_HEADERS.iter()
.filter(|n| self.original.contains_key(n) && !self.processed.contains(n))
.map(|name| HeaderMod::Delete { name })
.collect::<Vec<_>>();
execute_mods(id, deletions.iter(), actions, self.config)
execute_mods(id, self.report_mods.iter(), actions, self.config)
}
}
@ -273,8 +276,8 @@ pub fn replace_body(
})
}
// Header rewriting modification operation, operates only on the first of
// headers occurring multiple times (index = 1).
/// A header rewriting modification operation. These are intended to operate
/// only on the first instance of headers occurring multiple times.
#[derive(Clone, Copy, Debug)]
enum HeaderMod<'a> {
Add { name: &'a str, value: &'a str },
@ -286,8 +289,8 @@ impl HeaderMod<'_> {
fn execute(&self, actions: &impl ActionContext) -> milter::Result<()> {
use HeaderMod::*;
// Experiment shows that the milter library is smart enough to treat the
// name in case-insensitive manner, eg Subject may replace sUbject.
// 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))),
@ -352,7 +355,7 @@ mod tests {
assert_eq!(parse_header_line(b":empty name"), Err(Error::ParseEmail));
assert_eq!(parse_header_line(b"\t : whitespace name"), Err(Error::ParseEmail));
assert_eq!(parse_header_line(b"name:value"), Ok(Header { name: "name", value: "value" }));
assert_eq!(parse_header_line(b"name: value"), Ok(Header { name: "name", value: "value" }));
assert_eq!(parse_header_line(b"name: value"), Ok(Header { name: "name", value: " value" }));
assert_eq!(
parse_header_line(b"name:\r\n\tvalue"),
Ok(Header { name: "name", value: "\r\n\tvalue" })
@ -394,30 +397,30 @@ mod tests {
#[test]
fn header_rewriter_flags_spam() {
let config = Default::default();
let mut headers = HeaderMap::new();
headers.insert("x-spam-flag", String::from("no"));
headers.insert(String::from("x-spam-flag"), String::from(" no"));
let config = Default::default();
let mut rewriter = HeaderRewriter::new(headers, &config);
rewriter.process_header("X-Spam-Flag", "YES");
rewriter.process_header("X-Spam-Flag", " YES");
assert!(rewriter.is_flagged_spam());
}
#[test]
fn header_rewriter_processes_first_occurrence_only() {
let config = Default::default();
let headers = HeaderMap::new();
let config = Default::default();
let mut rewriter = HeaderRewriter::new(headers, &config);
rewriter.process_header("X-Spam-Flag", "NO");
rewriter.process_header("X-Spam-Flag", "YES");
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 } => {
assert_eq!(name, "X-Spam-Flag");
assert_eq!(value, "NO");
assert_eq!(value, " NO");
}
_ => panic!(),
}
@ -426,20 +429,20 @@ mod tests {
#[test]
fn header_rewriter_replaces_different_values() {
let config = Default::default();
let mut headers = HeaderMap::new();
headers.insert("x-spam-level", String::from("***"));
headers.insert("x-spam-report", String::from("original"));
headers.insert(String::from("x-spam-level"), String::from(" ***"));
headers.insert(String::from("x-spam-report"), String::from(" original"));
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", " new");
let mut mods = rewriter.spam_assassin_mods.into_iter();
match mods.next().unwrap() {
HeaderMod::Replace { name, value } => {
assert_eq!(name, "X-Spam-Report");
assert_eq!(value, "new");
assert_eq!(value, " new");
}
_ => panic!(),
}

View file

@ -10,8 +10,8 @@ pub type Result<T> = result::Result<T, Error>;
pub enum Error {
ParseEmail,
// For our purposes it is enough to record just the error message of I/O
// errors, no need to keep the full `io::Error` around.
IoError(String),
// errors, no need to keep the `io::Error` itself around.
Io(String),
}
impl Display for Error {
@ -19,26 +19,22 @@ impl Display for Error {
use Error::*;
match self {
ParseEmail => write!(f, "failed to parse email response"),
IoError(msg) => msg.fmt(f),
ParseEmail => write!(f, "failed to parse email"),
Io(msg) => msg.fmt(f),
}
}
}
impl error::Error for Error {
fn source(&self) -> Option<&(dyn error::Error + 'static)> {
None
}
}
impl error::Error for Error {}
impl From<io::Error> for Error {
fn from(error: io::Error) -> Self {
Error::IoError(format!("{}", error)) // just record the error message
Error::Io(format!("{}", error)) // just record the error message
}
}
impl From<Error> for milter::Error {
fn from(mine: Error) -> Self {
milter::Error::Custom(mine.into())
fn from(error: Error) -> Self {
milter::Error::Custom(error.into())
}
}

View file

@ -1,6 +1,6 @@
//! The SpamAssassin Milter application library.
#![doc(html_root_url = "https://docs.rs/spamassassin-milter/0.0.2")]
#![doc(html_root_url = "https://docs.rs/spamassassin-milter/0.0.3")]
#![macro_use]
macro_rules! verbose {
@ -31,9 +31,9 @@ use milter::Milter;
pub const MILTER_NAME: &str = "SpamAssassin Milter";
/// The current version string of SpamAssassin Milter.
pub const VERSION: &str = "0.0.2";
pub const VERSION: &str = "0.0.3";
/// Starts SpamAssassin Milter at the given socket using the supplied
/// Starts SpamAssassin Milter listening on the given socket using the supplied
/// configuration.
///
/// This is a blocking call.

View file

@ -88,27 +88,28 @@ fn main() {
fn build_config(matches: &ArgMatches<'_>) -> Result<Config, String> {
let mut config = Config::builder();
if let Some(s) = matches.value_of(ARG_MAX_MESSAGE_SIZE) {
match s.parse() {
if let Some(bytes) = matches.value_of(ARG_MAX_MESSAGE_SIZE) {
match bytes.parse() {
Ok(bytes) => {
config.set_max_message_size(bytes);
}
Err(_) => {
return Err(format!("invalid max message size \"{}\"", s));
return Err(format!("invalid max message size \"{}\"", bytes));
}
}
}
if let Some(t) = matches.values_of(ARG_TRUSTED_NETWORKS) {
if let Some(nets) = matches.values_of(ARG_TRUSTED_NETWORKS) {
config.set_has_trusted_networks(true);
for n in t.filter(|n| !n.trim().is_empty()) {
match n.parse().or_else(|_| n.parse::<IpAddr>().map(From::from)) {
for net in nets.filter(|n| !n.trim().is_empty()) {
// Both `ipnet::IpNet` and `std::net::IpAddr` are supported.
match net.parse().or_else(|_| net.parse::<IpAddr>().map(From::from)) {
Ok(net) => {
config.add_trusted_network(net);
}
Err(_) => {
return Err(format!("invalid trusted network address \"{}\"", n));
return Err(format!("invalid trusted network address \"{}\"", net));
}
}
}
@ -133,8 +134,8 @@ fn build_config(matches: &ArgMatches<'_>) -> Result<Config, String> {
config.set_verbose(true);
}
if let Some(s) = matches.values_of(ARG_SPAMC_ARGS) {
config.set_spamc_args(s.map(|arg| arg.to_owned()).collect());
if let Some(spamc_args) = matches.values_of(ARG_SPAMC_ARGS) {
config.set_spamc_args(spamc_args.map(|a| a.to_owned()).collect());
};
Ok(config.build())

View file

@ -5,7 +5,7 @@ use spamassassin_milter::*;
#[test]
fn authenticated_sender() {
let config = Config::builder().build();
let config = Default::default();
let miltertest = spawn_miltertest_runner(file!());

View file

@ -62,7 +62,7 @@ where
match f(s) {
// Again very basic handling of the spamd server protocol: add a forged
// protocol header terminated with "\r\n\r\n".
// protocol header terminated with "\r\n\r\n". (Currently not used.)
Ok(ham) => format!(
"{}\r\nContent-length: {}\r\nSpam: False ; 4.0 / 5.0\r\n\r\n{}",
SPAMD_PROTOCOL_OK,

View file

@ -47,12 +47,18 @@ assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "Date", os.date("%a, %d %b %Y %H:%M:%S %Z"))
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "X-Spam-Checker-Version", "BogusChecker 1.0.0")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "X-Spam-Report", "Bogus report")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.eoh(conn)
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.bodystring(conn, "Test message body")
local err = mt.bodystring(conn, "Hello, we would like to invite you to ...")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
@ -60,5 +66,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_HDRDELETE, "X-Spam-Report"))
local err = mt.disconnect(conn)
assert(err == nil, err)

View file

@ -9,7 +9,27 @@ fn ham_flow() {
builder.set_spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
let config = builder.build();
let server = spawn_mock_spamd_server(SPAMD_PORT, Ok);
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 miltertest = spawn_miltertest_runner(file!());
run("inet:3333@localhost", config).expect("milter execution failed");

View file

@ -48,6 +48,8 @@ local err = mt.header(conn, "Date", os.date("%a, %d %b %Y %H:%M:%S %Z"))
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
-- TODO Add headers here to experiment
local err = mt.eoh(conn)
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)

View file

@ -10,8 +10,8 @@ use spamassassin_milter::*;
#[ignore] // use option `--include-ignored` to run
fn live() {
// Without `spamc_args` set, `spamc` will try to connect to the default
// `spamd` port 783 (see also /etc/services).
let config = Config::builder().build();
// `spamd` port 783 (see also `/etc/services`).
let config = Default::default();
let miltertest = spawn_miltertest_runner(file!());

View file

@ -5,7 +5,7 @@ use spamassassin_milter::*;
#[test]
fn loopback_connection() {
let config = Config::builder().build();
let config = Default::default();
let miltertest = spawn_miltertest_runner(file!());

69
tests/reject_spam.lua Normal file
View file

@ -0,0 +1,69 @@
-- A spam message is rejected with an SMTP error reply.
conn = mt.connect("inet:3333@localhost")
assert(conn, "could not open connection")
local err = mt.conninfo(conn, "client.gluet.ch", "123.123.123.123")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.helo(conn, "mail.gluet.ch")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.mailfrom(conn, "from@gluet.ch")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.rcptto(conn, "to@gluet.ch")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
SMFIC_DATA = string.byte("T") -- SMFIC_DATA not exported by miltertest
local err = mt.macro(conn, SMFIC_DATA,
"i", "1234567ABC",
"j", "localhost",
"_", "client.gluet.ch [123.123.123.123]",
"{tls_version}", "TLSv1.2",
"v", "Postfix 3.3.0")
assert(err == nil, err)
local err = mt.data(conn)
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "From", "from@gluet.ch")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "To", "to@gluet.ch")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "Subject", "Test message")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "Message-ID", string.format("<%06d@gluet.ch>", math.random(999999)))
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "Date", os.date("%a, %d %b %Y %H:%M:%S %Z"))
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.eoh(conn)
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.bodystring(conn, "Test message body")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
-- A `miltertest` (or milter protocol?) pitfall. Even though we return the
-- `SMFIR_REJECT` status in the application code, because we use a custom error
-- reply, we must check for `SMFIR_REPLYCODE` instead.
local err = mt.eom(conn)
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_REPLYCODE)
assert(mt.eom_check(conn, MT_SMTPREPLY, "550", "5.7.1", "Spam message refused"))
local err = mt.disconnect(conn)
assert(err == nil, err)

24
tests/reject_spam.rs Normal file
View file

@ -0,0 +1,24 @@
mod common;
pub use common::*; // `pub` only to silence unused code warnings
use spamassassin_milter::*;
#[test]
fn reject_spam() {
let mut builder = Config::builder();
builder.set_reject_spam(true);
builder.set_spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
let config = builder.build();
let server = spawn_mock_spamd_server(SPAMD_PORT, |spam| {
Err(spam.replacen("\r\n\r\n", "\r\nX-Spam-Flag: YES\r\n\r\n", 1))
});
let miltertest = spawn_miltertest_runner(file!());
run("inet:3333@localhost", config).expect("milter execution failed");
let exit_code = miltertest.join().expect("panic in miltertest runner");
assert!(exit_code.success(), "miltertest returned error exit code");
server.join().expect("panic in mock spamd server");
}

View file

@ -50,12 +50,15 @@ assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "X-Spam-Checker-Version", "BogusChecker 1.0.0")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.header(conn, "X-Spam-Report", "Bogus report")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.eoh(conn)
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
local err = mt.bodystring(conn, "You have just won a billion dollars!!!!")
local err = mt.bodystring(conn, "HI!!! You have won a BILLION dollars!!!!")
assert(err == nil, err)
assert(mt.getreply(conn) == SMFIR_CONTINUE)
@ -63,9 +66,13 @@ 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_BODYCHANGE))
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_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_HDRADD, "Content-Type", " multipart/mixed; ..."))
assert(mt.eom_check(conn, MT_BODYCHANGE, "Spam detection software has identified ..."))
local err = mt.disconnect(conn)
assert(err == nil, err)

View file

@ -9,15 +9,36 @@ fn spam_flow() {
builder.set_spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
let config = builder.build();
let server = spawn_mock_spamd_server(SPAMD_PORT, |s| {
Err(
s.replacen("Subject: Test message\r\n", "Subject: [SPAM] Test message\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\
\r\n", 1)
)
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-Report: Bogus report\r\n",
"",
1,
)
.replacen(
"\r\n\r\n",
"\r\n\
X-Spam-Flag: YES\r\n\
X-Spam-Custom: Custom-Value\r\n\
Content-Type: multipart/mixed; ...\r\n\
\r\n",
1,
);
// Replace the message body with the report …
spam.replace_range(
(spam.find("\r\n\r\n").unwrap() + 4)..,
"Spam detection software has identified ..."
);
Err(spam)
});
let miltertest = spawn_miltertest_runner(file!());

View file

@ -6,7 +6,7 @@ use spamassassin_milter::*;
#[test]
fn spamc_connection_error() {
let mut builder = Config::builder();
// spamc always works even if it cannot actually reach spamd!
// `spamc` always works even if it cannot actually reach `spamd`!
// `--no-safe-fallback` prevents this masking of connection errors.
// TODO Also try with non-existing flag, behaviour is different?
builder.set_spamc_args(vec![