[enhancement]: Add option to filter out reports and rename report.analysis.forward to report.analysis.discard

[33KK]

What happened?

No emails are forwarded to the intended recipient, only successfully parsed reports shouldn't be.

How can we reproduce the problem?

Send an email to an address assigned to an account with inbound report analysis enabled. No emails, including non-report emails, will ever come through if forwarding is disabled.

mail-server/crates/smtp/src/inbound/data.rs

Lines 329 to 346 in a491c63

	if is_report {
	if !rc.analysis.forward {
	self.server.analyze_report(
	mail_parser::Message {
	html_body: parsed_message.html_body,
	text_body: parsed_message.text_body,
	attachments: parsed_message.attachments,
	parts: parsed_message
	.parts
	.into_iter()
	.map(|p| p.into_owned())
	.collect(),
	raw_message: b"".into(),
	},
	self.data.session_id,
	);
	self.data.messages_sent += 1;
	return (b"250 2.0.0 Message queued for delivery.\r\n"[..]).into();

Version

v0.11.x

What database are you using?

None

What blob storage are you using?

None

Where is your directory located?

None

What operating system are you using?

None

Relevant log output

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[mdecimus]

This is done for performance reasons to avoid parsing each received message to see whether it is a report or not. That is why report detection is a simple check of the recipient address. You should choose a unique report address that does not receive any other type of messages, or enable forwarding.

[33KK]

@mdecimus The defaults set postmaster@*, abuse@* and dmarc@* as the intercepted addresses, hence why this is a bug. My server got suspended because I couldn't get any emails from the hoster. abuse@* and postmaster@* are not purely dedicated to reports. I don't want to see reports that are already processed by the mail server, but I need to get all the other emails. The popup in the settings web ui also specifically mentioned reports.

[mdecimus]

It is just postmaster@*, the webadmin incorrectly lists abuse and dmarc as defaults but I have fixed that. In any case, Stalwart forwards all reports by default so perhaps you manually disabled report forwarding?

[33KK]

I don't want my postmaster address bombarded with useless reports I can't even read through the email client. I really don't care if emails come 50ms slower to it. I don't get your performance argument

[mdecimus]

Then change the default address to something else that is not postmaster? Postmaster is the safe default for most setups.

[33KK]

Or perhaps fix the obvious issue? Nowhere does it say that it any non-report emails will just be sent to the abyss. Nor does it make sense to do it this way. Your "performance" argument makes absolutely zero sense. It makes no difference if reports are delivered 5ms later, it makes no difference if non-reports to addresses listed in report analysis are delivered 5ms later. There's no issue with performance either way, nor is "performance" necessary here in the first place. This is a bug, simple as that.

[mdecimus]

I repeat, this is not a bug, by default report.analysis.forward is set to true (see the docs) so no messages are dropped. When you set this setting to false (again, not the default), messages are not forwarded because it means that the address is exclusive for report so after analysis the message is discarded. Just make sure you have report.analysis.forward set to true and you won't miss any messages.

[33KK]

😕 ?? The "intended behavior" you're describing is nonsensical. Especially considering you use postmaster as a default and the popup doesn't say "IF FORWARDING IS DISABLED YOU WILL LOSE ALL EMAILS 🚨 🚨 🚨 🚨 🚨 🚨 🚨 ". In what world is postmaster exclusively for automated reporting?

[33KK]

The popup reads to me "if the email is analyzed as a report it will not be forwarded", not "DANGER 🚨 🚨 🚨 🚨 🚨 "

[mdecimus]

Makes sense, I'll add "IF FORWARDING IS DISABLED YOU WILL LOSE ALL EMAILS 🚨 🚨 🚨 🚨 🚨 🚨 🚨 " and "DANGER 🚨 🚨 🚨 🚨 🚨 " to the documentation and the webadmin.

[33KK]

Why not just make it actually deliver non-reports though? How would this affect performance? Was it ever an actual issue or is the performance problem theoretical? As far as I can tell it would only stall the queue_message task for a few ms and only for emails sent to report addresses.

[33KK]

I genuinely doubt just awaiting on the analyze_report would have any real impact on performance assuming queue_message doesn't stall the entire server, and if it does it would be a better idea to fix that, rather than doing whatever this is for some reason...

I've looked at it a bit, and I guess it would stall the SMTP connection, but a report is literally only ever sent daily and probably using a separate connection. And the analysis barely does much work in the first place...

[mdecimus]

What you are proposing is already possible by setting report.analysis.forward to true (which is the default). It does not delay message delivery as the report is analyzed in a new thread.
What I agree is that a warning should be added to the documentation and webadmin explaining that if forwarding is set to false then everything that is not a report will be dropped.

[33KK]

As I said before, I want an option to only skip forwarding reports. I still want to get non-reports, I still want to get reports that failed to get analyzed if that happens. This is a behavior that was described on the webadmin and the one that I would like and expect (hence why I consider this a bug, and your proposed solution more of a workaround than a fix).

[33KK]

Also, tokio::spawn is not necessarily on a new thread and isn't intended for blocking work anyway, which seems to be most of what analyze_report is doing.

[33KK]

Sorry if I came off rude. I am not in a great mental state, and this also lost me money.

[mdecimus]

No worries, I'll add a note to the documentation and keep this issue open for the functionality you need.

[andreymal]

@mdecimus by the way, the value for report.analysis.addresses is set only after the spam rules are updated (crates/common/src/manager/boot.rs:306)

So if report.analysis.addresses is manually cleared after the spam rules are updated, the default value is actually empty (crates/common/src/config/smtp/report.rs:86 doesn't have a fallback value) and the webadmin still shows the wrong default value

(In general, the way the webadmin shows default values is fundamentally incorrect, there is no way to determine what values the configuration actually has without reading the source code carefully...)

[mdecimus]

by the way, the value for report.analysis.addresses is set only after the spam rules are updated (crates/common/src/manager/boot.rs:306)

No, it is set when there is no spam-filter.version key defined which means that Stalwart is running for the first time. Although the spam rules are fetched first, the list of keys to insert is merged.

So if report.analysis.addresses is manually cleared after the spam rules are updated,

As explained above, those default settings are only written when config.value("version.spam-filter") returns None. That code won't be executed ever again unless you manually delete the version.spam-filter key from the database.

crates/common/src/config/smtp/report.rs:86 doesn't have a fallback value

A fallback value is not used as we need to allow admins to disable this setting.

and the webadmin still shows the wrong default value.

I've just fixed this (but not yet pushed the fix).

In general, the way the webadmin shows default values is fundamentally incorrect, there is no way to determine what values the configuration actually has without reading the source code carefully

The webadmin should display the exact default value that the mail server is using when that key is not present. If that is not the case for a setting then it is a bug in the webadmin.

[andreymal]

If that is not the case for a setting then it is a bug in the webadmin.

Many of such bugs have been found in the last few weeks, and more are yet to be found...