Review doc, revise error handling and config builder

This commit is contained in:
David Bürgin 2020-02-17 15:06:25 +01:00
parent 1432560004
commit 02b7e2ad64
15 changed files with 145 additions and 79 deletions

View file

@ -1,6 +1,6 @@
# SpamAssassin Milter
SpamAssassin Milter is a milter application that filters email through
SpamAssassin Milter is a [milter] application that filters email through
SpamAssassin server using the `spamc` client. It is a light-weight component
that serves to integrate [Apache SpamAssassin] with a milter-capable MTA (mail
server) such as [Postfix]. Its task is thus helping combat spam on email sites.
@ -30,6 +30,7 @@ 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].
[milter]: https://crates.io/crates/milter
[Apache SpamAssassin]: https://spamassassin.apache.org
[Postfix]: http://www.postfix.org
[spamass-milt]: https://savannah.nongnu.org/projects/spamass-milt/
@ -54,7 +55,7 @@ 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.
Note that until recently `miltertest` had a bad bug that prevents most
Note that 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`.
@ -99,9 +100,9 @@ SpamAssassin server (`spamd`) and client (`spamc`). You can get started with
just picking a socket and things should just work. Some integration options are
discussed in subsequent sections.
First-time users may wish to run SpamAssassin Milter with the `--dry-run` option
before going live. Combined with the `--verbose` option, this gives accurate
insight into the changes that SpamAssassin Milter would apply.
New users may wish to run SpamAssassin Milter with the `--dry-run` option before
going live. Combined with the `--verbose` option, this gives accurate insight
into the changes that SpamAssassin Milter would apply.
```
spamassassin-milter --dry-run --verbose inet:3000@localhost
@ -122,7 +123,7 @@ The main SpamAssassin configuration file is `/etc/spamassassin/local.conf`. See
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
property. The classification threshold can be adjusted by setting
`required_score` as follows:
```
@ -220,7 +221,7 @@ A further component that can be useful with SpamAssassin Milter is a
those.
As an example, in case [Dovecot] does mail delivery with [LMTP], enable the
Sieve plugin for the LMTP protocol, then setup a global Sieve script that files
Sieve plugin for the LMTP protocol, then set up a global Sieve script that files
messages flagged as spam into the Junk folder:
```

View file

@ -3,14 +3,14 @@
spamassassin-milter \- milter for spam filtering with SpamAssassin
.SH SYNOPSIS
.B spamassassin-milter
[\fB\-a\fR]
[\fB\-B\fR]
[\fB\-H\fR]
[\fB\-n\fR]
[\fB\-r\fR]
.RB [ \-a ]
.RB [ \-B ]
.RB [ \-H ]
.RB [ \-n ]
.RB [ \-r ]
[\fB\-s\fR \fIBYTES\fR]
[\fB\-t\fR \fINETS\fR]
[\fB\-v\fR]
.RB [ \-v ]
.IR SOCKET
[\fB\-\-\fR \fISPAMC_ARGS\fR...]
.SH DESCRIPTION
@ -18,11 +18,11 @@ spamassassin-milter \- milter for spam filtering with SpamAssassin
is a milter that filters email through SpamAssassin server using the
.B spamc
client.
It reads the response from SpamAssassin and adds its
It reads the response from SpamAssassin and adds its diagnostic
.B X-Spam-
headers to the message, and can optionally apply header and body rewriting to
messages flagged as spam, or reject such messages at the SMTP level.
A message flagged as spam is a message with a header
A message \*(lqflagged as spam\*(rq is a message with a header
.BR "X-Spam-Flag: YES" .
.PP
The mandatory
@ -30,32 +30,31 @@ The mandatory
argument specifies the listening socket to open.
.I SOCKET
can be either an IPv4/IPv6 TCP socket in the form
.BR inet: "\fIPORT\fR" @ "\fIHOST\fR"
.BI inet: PORT @ HOST
or
.BR inet6: "\fIPORT\fR" @ "\fIHOST\fR"
.BI inet6: PORT @ HOST
(for example,
.BR inet:3000@localhost ),
or a UNIX domain socket in the form
.BR unix: "\fIPATH\fR"
.BI unix: PATH
(for example,
.BR unix:/run/spamassassin-milter.sock ).
After the options and argument, additional arguments
.IR SPAMC_ARGS ...
.I SPAMC_ARGS
listed after
.B \-\-
are gathered as arguments to pass to the
.B spamc
invocation.
.B spamassassin-milter
has reasonable defaults and if run with no options, will apply modifications
uses reasonable defaults and if run with no options, will apply modifications
received from SpamAssassin server.
.\" 3) description of larger context, spamc spamd mta (eg spamc config file!)
.PP
.B spamassassin-milter
is a light-weight integration component, enabling use of SpamAssassin with a
milter-capable MTA.
As such, users must first be familiar with the setup and configuration options
of the components participating, namely, the SpamAssassin programs
Users are advised to familiarize themselves with the setup and configuration
options of the components participating, namely, the SpamAssassin programs
.B spamd
(SpamAssassin server) and
.BR spamc ,
@ -112,8 +111,8 @@ their values replaced with the values received from SpamAssassin, if necessary.
.TP
.BR \-r ", " \-\-reject-spam
Reject messages flagged as spam at the SMTP level.
Rejection results in a permanent SMTP error reply being returned to the client,
and the message is not delivered.
Rejection results in a permanent SMTP error being returned to the client, and
the message is not delivered.
.TP
.BR \-t ", " \-\-trusted-networks " \fINETS\fR"
Trust connections coming from the IP networks or addresses
@ -128,11 +127,12 @@ If this option is not used, all connections from loopback addresses are trusted.
.BR \-v ", " \-\-verbose
Enable verbose operation logging.
If this option is not used, only unexpected error conditions are logged (that
is, printed to stderr).
is, printed to standard error).
.TP
.BR \-V ", " \-\-version
Print version information.
.SH SEE ALSO
.BR spamc (1),
.BR spamassassin (1p),
.BR spamd (8p),
.BR spamc (1)
.BR Mail::SpamAssassin::Conf (3pm),
.BR spamd (8p)

View file

@ -46,7 +46,10 @@ fn handle_negotiate(
ctx.api.request_macros(Stage::Connect, "")?;
ctx.api.request_macros(Stage::Helo, "")?;
ctx.api.request_macros(Stage::Mail, "{auth_authen}")?;
ctx.api.request_macros(
Stage::Mail,
if config::get().auth_untrusted() { "" } else { "{auth_authen}" },
)?;
ctx.api.request_macros(Stage::Rcpt, "")?;
ctx.api.request_macros(Stage::Data, "i j _ {tls_version} v")?;
ctx.api.request_macros(Stage::Eoh, "")?;
@ -109,9 +112,10 @@ fn handle_mail(mut ctx: Context<Connection>, smtp_args: Vec<&str>) -> milter::Re
#[on_rcpt(rcpt_callback)]
fn handle_rcpt(mut ctx: Context<Connection>, smtp_args: Vec<&str>) -> milter::Result<Status> {
let conn = ctx.data.borrow_mut().unwrap();
let client = conn.client.as_mut().unwrap();
let recipient = smtp_args[0].to_owned();
conn.client.as_mut().unwrap().add_recipient(recipient);
client.add_recipient(recipient);
Ok(Status::Continue)
}

View file

@ -7,6 +7,7 @@ use milter::{ActionContext, SetErrorReply, Status};
use std::{
any::Any,
io::Write,
os::unix::process::ExitStatusExt,
process::{Child, Command, Stdio},
};
@ -23,7 +24,6 @@ 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 {
@ -60,11 +60,16 @@ impl Process for Spamc {
let output = spamc.wait_with_output()?;
// TODO put signal/exit code in message
if output.status.success() {
Ok(output.stdout)
} else {
Err(Error::Io(String::from("spamc returned error exit code")))
Err(match output.status.code() {
Some(code) => Error::Io(format!("spamc exited with status code {}", code)),
None => Error::Io(format!(
"spamc terminated by signal {}",
output.status.signal().unwrap()
)),
})
}
}
@ -169,9 +174,8 @@ impl Client {
}
pub fn send_header(&mut self, name: &str, value: &str) -> Result<()> {
// Remember that the value includes leading whitespace, as weve
// requested in the milter protocol options. This lets us keep
// whitespace exactly as is.
// As requested during milter protocol negotiation, the value includes
// leading whitespace. This lets us pass on whitespace exactly as is.
let value = email::ensure_crlf(value);
let buf = format!("{}:{}\r\n", name, value);
@ -208,8 +212,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);
eprintln!("{}: failed to complete spamc communication: {}", id, e);
return Ok(Status::Tempfail);
}
};
@ -405,6 +408,32 @@ mod tests {
);
}
#[test]
fn client_send_envelope_addresses() {
let spamc = MockSpamc::new();
let sender = "<sender@gluet.ch>";
let recipient1 = "<recipient1@gluet.ch>";
let recipient2 = "<recipient2@gluet.ch>";
let mut client = Client::new(spamc, String::from(sender));
client.add_recipient(String::from(recipient1));
client.add_recipient(String::from(recipient2));
client.send_envelope_sender().unwrap();
client.send_envelope_recipients().unwrap();
assert_eq!(
as_mock_spamc(client.process.as_ref()).buf,
Vec::from(format!(
"X-Envelope-From: {}\r\n\
X-Envelope-To: {},\r\n\
\t{}\r\n",
sender, recipient1, recipient2
).as_bytes())
);
}
#[test]
fn client_process_invalid_response() {
let spamc = MockSpamc::with_output(b"invalid message response".to_vec());
@ -422,7 +451,7 @@ mod tests {
let spamc = MockSpamc::with_output(b"X-Spam-Flag: YES\r\n\r\n".to_vec());
let actions = MockActionContext::new();
let mut builder = Config::builder();
builder.set_reject_spam(true);
builder.reject_spam(true);
let config = builder.build();
let client = Client::new(spamc, String::from("sender"));

View file

@ -28,12 +28,12 @@ where
self.iter().map(|e| e.0)
}
pub fn contains_key(&self, key: &str) -> bool {
self.iter().any(|e| e.0.as_ref().eq_ignore_ascii_case(key))
pub fn contains_key<Q: AsRef<str>>(&self, key: Q) -> bool {
self.iter().any(|e| e.0.as_ref().eq_ignore_ascii_case(key.as_ref()))
}
pub fn get(&self, key: &str) -> Option<&V> {
self.iter().find(|e| e.0.as_ref().eq_ignore_ascii_case(key)).map(|e| e.1)
pub fn get<Q: AsRef<str>>(&self, key: Q) -> Option<&V> {
self.iter().find(|e| e.0.as_ref().eq_ignore_ascii_case(key.as_ref())).map(|e| e.1)
}
pub fn insert(&mut self, key: K, value: V) -> Option<V> {
@ -81,7 +81,7 @@ where
self.map.keys()
}
pub fn contains(&self, key: &str) -> bool {
pub fn contains<Q: AsRef<str>>(&self, key: Q) -> bool {
self.map.contains_key(key)
}

View file

@ -5,6 +5,7 @@ use std::{collections::HashSet, net::IpAddr};
/// A builder for SpamAssassin Milter configuration objects.
#[derive(Clone, Debug)]
pub struct ConfigBuilder {
milter_debug_level: i32,
has_trusted_networks: bool,
trusted_networks: HashSet<IpNet>,
auth_untrusted: bool,
@ -18,7 +19,12 @@ pub struct ConfigBuilder {
}
impl ConfigBuilder {
pub fn set_has_trusted_networks(&mut self, value: bool) -> &mut Self {
pub fn milter_debug_level(&mut self, value: i32) -> &mut Self {
self.milter_debug_level = value;
self
}
pub fn has_trusted_networks(&mut self, value: bool) -> &mut Self {
self.has_trusted_networks = value;
self
}
@ -29,48 +35,49 @@ impl ConfigBuilder {
self
}
pub fn set_auth_untrusted(&mut self, value: bool) -> &mut Self {
pub fn auth_untrusted(&mut self, value: bool) -> &mut Self {
self.auth_untrusted = value;
self
}
pub fn set_spamc_args(&mut self, value: Vec<String>) -> &mut Self {
pub fn spamc_args(&mut self, value: Vec<String>) -> &mut Self {
self.spamc_args = value;
self
}
pub fn set_max_message_size(&mut self, value: usize) -> &mut Self {
pub fn max_message_size(&mut self, value: usize) -> &mut Self {
self.max_message_size = value;
self
}
pub fn set_dry_run(&mut self, value: bool) -> &mut Self {
pub fn dry_run(&mut self, value: bool) -> &mut Self {
self.dry_run = value;
self
}
pub fn set_reject_spam(&mut self, value: bool) -> &mut Self {
pub fn reject_spam(&mut self, value: bool) -> &mut Self {
self.reject_spam = value;
self
}
pub fn set_preserve_headers(&mut self, value: bool) -> &mut Self {
pub fn preserve_headers(&mut self, value: bool) -> &mut Self {
self.preserve_headers = value;
self
}
pub fn set_preserve_body(&mut self, value: bool) -> &mut Self {
pub fn preserve_body(&mut self, value: bool) -> &mut Self {
self.preserve_body = value;
self
}
pub fn set_verbose(&mut self, value: bool) -> &mut Self {
pub fn verbose(&mut self, value: bool) -> &mut Self {
self.verbose = value;
self
}
pub fn build(self) -> Config {
Config {
milter_debug_level: self.milter_debug_level,
has_trusted_networks: self.has_trusted_networks,
trusted_networks: self.trusted_networks,
auth_untrusted: self.auth_untrusted,
@ -88,6 +95,7 @@ impl ConfigBuilder {
impl Default for ConfigBuilder {
fn default() -> Self {
Self {
milter_debug_level: Default::default(),
has_trusted_networks: Default::default(),
trusted_networks: Default::default(),
auth_untrusted: Default::default(),
@ -105,6 +113,7 @@ impl Default for ConfigBuilder {
/// A configuration object for SpamAssassin Milter.
#[derive(Clone, Debug)]
pub struct Config {
milter_debug_level: i32,
has_trusted_networks: bool,
trusted_networks: HashSet<IpNet>,
auth_untrusted: bool,
@ -122,6 +131,10 @@ impl Config {
Default::default()
}
pub fn milter_debug_level(&self) -> i32 {
self.milter_debug_level
}
pub fn has_trusted_networks(&self) -> bool {
self.has_trusted_networks
}

View file

@ -222,7 +222,6 @@ impl<'a> HeaderRewriter<'a> {
.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)
}

View file

@ -58,6 +58,8 @@ pub const VERSION: &str = "0.0.3";
/// }
/// ```
pub fn run(socket: &str, config: Config) -> milter::Result<()> {
milter::set_debug_level(config.milter_debug_level());
config::init(config);
Milter::new(socket)

View file

@ -1,10 +1,11 @@
use clap::{App, Arg, ArgMatches};
use clap::{App, Arg, ArgMatches, Error, ErrorKind, Result};
use spamassassin_milter::Config;
use std::{net::IpAddr, process};
const ARG_AUTH_UNTRUSTED: &str = "AUTH_UNTRUSTED";
const ARG_DRY_RUN: &str = "DRY_RUN";
const ARG_MAX_MESSAGE_SIZE: &str = "MAX_MESSAGE_SIZE";
const ARG_MILTER_DEBUG_LEVEL: &str = "MILTER_DEBUG_LEVEL";
const ARG_PRESERVE_BODY: &str = "PRESERVE_BODY";
const ARG_PRESERVE_HEADERS: &str = "PRESERVE_HEADERS";
const ARG_REJECT_SPAM: &str = "REJECT_SPAM";
@ -31,6 +32,13 @@ fn main() {
.long("max-message-size")
.value_name("BYTES")
.help("Maximum message size to process"))
.arg(Arg::with_name(ARG_MILTER_DEBUG_LEVEL)
.long("milter-debug-level")
.value_name("LEVEL")
.possible_values(&["0", "1", "2", "3", "4", "5", "6"])
.help("Set the milter library debug level")
.hidden(true) // not documented for now
.hide_possible_values(true))
.arg(Arg::with_name(ARG_PRESERVE_BODY)
.short("B")
.long("preserve-body")
@ -66,9 +74,8 @@ fn main() {
let socket = matches.value_of(ARG_SOCKET).unwrap();
let config = match build_config(&matches) {
Ok(config) => config,
Err(msg) => {
eprintln!("error: {}", msg);
process::exit(1);
Err(e) => {
e.exit();
}
};
@ -85,22 +92,25 @@ fn main() {
}
}
fn build_config(matches: &ArgMatches<'_>) -> Result<Config, String> {
fn build_config(matches: &ArgMatches<'_>) -> Result<Config> {
let mut config = Config::builder();
if let Some(bytes) = matches.value_of(ARG_MAX_MESSAGE_SIZE) {
match bytes.parse() {
Ok(bytes) => {
config.set_max_message_size(bytes);
config.max_message_size(bytes);
}
Err(_) => {
return Err(format!("invalid max message size \"{}\"", bytes));
return Err(Error::with_description(
&format!("Invalid value for max message size: \"{}\"", bytes),
ErrorKind::InvalidValue,
));
}
}
}
if let Some(nets) = matches.values_of(ARG_TRUSTED_NETWORKS) {
config.set_has_trusted_networks(true);
config.has_trusted_networks(true);
for net in nets.filter(|n| !n.trim().is_empty()) {
// Both `ipnet::IpNet` and `std::net::IpAddr` are supported.
@ -109,33 +119,40 @@ fn build_config(matches: &ArgMatches<'_>) -> Result<Config, String> {
config.add_trusted_network(net);
}
Err(_) => {
return Err(format!("invalid trusted network address \"{}\"", net));
return Err(Error::with_description(
&format!("Invalid value for trusted network address: \"{}\"", net),
ErrorKind::InvalidValue,
));
}
}
}
}
if matches.is_present(ARG_AUTH_UNTRUSTED) {
config.set_auth_untrusted(true);
config.auth_untrusted(true);
}
if matches.is_present(ARG_DRY_RUN) {
config.set_dry_run(true);
config.dry_run(true);
}
if matches.is_present(ARG_PRESERVE_BODY) {
config.set_preserve_body(true);
config.preserve_body(true);
}
if matches.is_present(ARG_PRESERVE_HEADERS) {
config.set_preserve_headers(true);
config.preserve_headers(true);
}
if matches.is_present(ARG_REJECT_SPAM) {
config.set_reject_spam(true);
config.reject_spam(true);
}
if matches.is_present(ARG_VERBOSE) {
config.set_verbose(true);
config.verbose(true);
}
if let Some(level) = matches.value_of(ARG_MILTER_DEBUG_LEVEL) {
config.milter_debug_level(level.parse().unwrap());
}
if let Some(spamc_args) = matches.values_of(ARG_SPAMC_ARGS) {
config.set_spamc_args(spamc_args.map(|a| a.to_owned()).collect());
config.spamc_args(spamc_args.map(|a| a.to_owned()).collect());
};
Ok(config.build())

View file

@ -25,8 +25,9 @@ where
let listener = TcpListener::bind(socket_addr).unwrap();
// This server handles only a single connection, so that we can `join`
// this thread in the tests and detect panics.
// TODO What if the connection never comes? Timeout?
// this thread in the tests and detect panics. If the connection never
// comes but `miltertest` still succeeds, this will cause the test to
// hang.
match listener.accept() {
Ok((mut stream, _)) => {
let mut buf = Vec::new();

View file

@ -6,7 +6,7 @@ use spamassassin_milter::*;
#[test]
fn ham_flow() {
let mut builder = Config::builder();
builder.set_spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
builder.spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
let config = builder.build();
let server = spawn_mock_spamd_server(SPAMD_PORT, |ham| {

View file

@ -6,8 +6,8 @@ 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)]);
builder.reject_spam(true);
builder.spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
let config = builder.build();
let server = spawn_mock_spamd_server(SPAMD_PORT, |spam| {

View file

@ -6,8 +6,8 @@ use spamassassin_milter::*;
#[test]
fn skip_large_message() {
let mut builder = Config::builder();
builder.set_max_message_size(512);
builder.set_spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
builder.max_message_size(512);
builder.spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
let config = builder.build();
let server = spawn_mock_spamd_server(SPAMD_PORT, Ok);

View file

@ -6,7 +6,7 @@ use spamassassin_milter::*;
#[test]
fn spam_flow() {
let mut builder = Config::builder();
builder.set_spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
builder.spamc_args(vec![format!("--port={}", SPAMD_PORT)]);
let config = builder.build();
let server = spawn_mock_spamd_server(SPAMD_PORT, |spam| {

View file

@ -9,7 +9,7 @@ fn spamc_connection_error() {
// `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![
builder.spamc_args(vec![
String::from("--no-safe-fallback"),
format!("--port={}", SPAMD_PORT),
]);