Warn when max message size misconfigured
This commit is contained in:
parent
300a3b2d13
commit
2e1443578a
4 changed files with 57 additions and 4 deletions
|
@ -4,6 +4,9 @@
|
||||||
|
|
||||||
* Add `--reply-code`, `--reply-status-code`, and `--reply-text` options to
|
* Add `--reply-code`, `--reply-status-code`, and `--reply-text` options to
|
||||||
allow customising the SMTP reply when rejecting spam.
|
allow customising the SMTP reply when rejecting spam.
|
||||||
|
* Log a warning and do not truncate message body when `--max-message-size` is
|
||||||
|
misconfigured (must be ≥ `spamc` max size as documented).
|
||||||
|
* Update dependencies in `Cargo.lock`.
|
||||||
|
|
||||||
## 0.1.2 (2020-06-07)
|
## 0.1.2 (2020-06-07)
|
||||||
|
|
||||||
|
|
|
@ -85,8 +85,8 @@ Maximum message size in bytes to pass to
|
||||||
.I BYTES
|
.I BYTES
|
||||||
must be equal to or greater than the max size configured for
|
must be equal to or greater than the max size configured for
|
||||||
.BR spamc ,
|
.BR spamc ,
|
||||||
else it is possible that the body of spam messages is truncated to the size
|
in order to ensure that SpamAssassin does not process messages truncated to the
|
||||||
configured for
|
size configured for
|
||||||
.BR spamc .
|
.BR spamc .
|
||||||
Defaults to the
|
Defaults to the
|
||||||
.B spamc
|
.B spamc
|
||||||
|
|
|
@ -185,7 +185,8 @@ fn handle_body(mut ctx: Context, bytes: &[u8]) -> milter::Result<Status> {
|
||||||
client.send_body_chunk(bytes)?;
|
client.send_body_chunk(bytes)?;
|
||||||
|
|
||||||
let max = config::get().max_message_size();
|
let max = config::get().max_message_size();
|
||||||
Ok(if client.bytes_written() >= max {
|
Ok(if client.bytes_written() > max {
|
||||||
|
client.skip_body();
|
||||||
let id = queue_id(&ctx.api)?;
|
let id = queue_id(&ctx.api)?;
|
||||||
verbose!("{}: skipping rest of message larger than {} bytes", id, max);
|
verbose!("{}: skipping rest of message larger than {} bytes", id, max);
|
||||||
Status::Skip
|
Status::Skip
|
||||||
|
|
|
@ -96,6 +96,7 @@ pub struct Client {
|
||||||
recipients: Vec<String>,
|
recipients: Vec<String>,
|
||||||
headers: HeaderMap,
|
headers: HeaderMap,
|
||||||
bytes: usize,
|
bytes: usize,
|
||||||
|
skipped: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Client {
|
impl Client {
|
||||||
|
@ -106,6 +107,7 @@ impl Client {
|
||||||
recipients: vec![],
|
recipients: vec![],
|
||||||
headers: HeaderMap::new(),
|
headers: HeaderMap::new(),
|
||||||
bytes: 0,
|
bytes: 0,
|
||||||
|
skipped: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -117,6 +119,10 @@ impl Client {
|
||||||
self.bytes
|
self.bytes
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn skip_body(&mut self) {
|
||||||
|
self.skipped = true;
|
||||||
|
}
|
||||||
|
|
||||||
pub fn connect(&mut self) -> Result<()> {
|
pub fn connect(&mut self) -> Result<()> {
|
||||||
self.process.connect()
|
self.process.connect()
|
||||||
}
|
}
|
||||||
|
@ -252,7 +258,7 @@ impl Client {
|
||||||
}
|
}
|
||||||
if !config.preserve_body() {
|
if !config.preserve_body() {
|
||||||
rewriter.rewrite_report_headers(id, actions)?;
|
rewriter.rewrite_report_headers(id, actions)?;
|
||||||
email::replace_body(id, email.body, actions, config)?;
|
replace_body(id, email.body, self.skipped, actions, config)?;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -280,6 +286,29 @@ fn reject_spam(id: &str, actions: &impl SetErrorReply, config: &Config) -> milte
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn replace_body(
|
||||||
|
id: &str,
|
||||||
|
body: &[u8],
|
||||||
|
skipped: bool,
|
||||||
|
actions: &impl ActionContext,
|
||||||
|
config: &Config,
|
||||||
|
) -> milter::Result<()> {
|
||||||
|
// Do not replace the message body when part of it was skipped. This only
|
||||||
|
// occurs when the milter’s max message size is less than that of `spamc`.
|
||||||
|
// This condition ensures message integrity in such a misconfigured setup.
|
||||||
|
if skipped {
|
||||||
|
eprintln!(
|
||||||
|
"{}: not replacing possibly truncated message body; \
|
||||||
|
please review max message size setting ({})",
|
||||||
|
id,
|
||||||
|
config.max_message_size()
|
||||||
|
);
|
||||||
|
Ok(())
|
||||||
|
} else {
|
||||||
|
email::replace_body(id, body, actions, config)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
@ -514,4 +543,24 @@ mod tests {
|
||||||
assert!(expected.contains(&a));
|
assert!(expected.contains(&a));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn client_process_skipped_body_not_replaced() {
|
||||||
|
let spamc = MockSpamc::with_output(
|
||||||
|
b"X-Spam-Flag: YES\r\nX-Spam-Level: *****\r\n\r\nReport".to_vec(),
|
||||||
|
);
|
||||||
|
let actions = MockActionContext::new();
|
||||||
|
let config = Default::default();
|
||||||
|
|
||||||
|
let mut client = Client::new(spamc, String::from("sender"));
|
||||||
|
client.skip_body();
|
||||||
|
|
||||||
|
let status = client.process("id", &actions, &config).unwrap();
|
||||||
|
|
||||||
|
assert_eq!(status, Status::Accept);
|
||||||
|
|
||||||
|
let called = actions.called.borrow();
|
||||||
|
assert!(called.contains(&Action::AddHeader("X-Spam-Level".into(), " *****".into())));
|
||||||
|
assert!(!called.contains(&Action::AppendBodyChunk(b"Report".to_vec())));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue