MailScanner / v5

MailScanner v5
GNU General Public License v2.0
182 stars 58 forks source link

Milter: empty line in added Received-header causes trouble #618

Closed dneuhaeuser closed 1 year ago

dneuhaeuser commented 1 year ago

I'm trying to get MailScanner 5.4.4-1 running in Milter mode.

System: Ubuntu 22.04.1 LTS Postfix: 3.6.4

Apparently there is a problem with the Received-Header added by MSMilter. I intercepted and looked at a queue file in /var/spool/MailScanner/milterin

There is an empty line (marked below) in the middle of the Received-Header:

O<him@recipient.de>
S<me@sender.de>
Received: from mail.sender.de (mail.sender.de [89.244.xx.xxx])
        (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
**empty line**
        by mx.kluthe.local (MailScanner Milter) with SMTP id CDCB15C005
        for <him@recipient.de>; Fr, 21 Okt 2022 21:51:27 +0200 (CEST)

This obviously causes Mailscanner to truncate all lines after the empty line leading to all kind of issues because of missing headers:

mx MailScanner[3654671]: Message 196F35C030.A0BAE from 127.0.0.1 (me@sender.de) to him@recipient.de is not spam... MISSING_DATE 1.36, MISSING_FROM 1.00, MISSING_HEADERS 1.02, MISSING_MID 0.50, MISSING_SUBJECT 1.80, UNPARSEABLE_RELAY 0.00

I can reproduce this every time with milter mode. Without milter (classic HOLD method) Mailscanner is working perfectly.

For a test I edited the queue file, removed the empty line and copied it back to milterin. The mail was processed with no problem then.

msapiro commented 1 year ago

On 10/21/22 14:50, Dennis Neuhaeuser wrote:

I'm trying to get MailScanner 5.4.4-1 running in Milter mode.

System: Ubuntu 22.04.1 LTS Postfix: 3.6.4

Apparently there is a problem with the Received-Header added by MSMilter. I intercepted and looked at a queue file in /var/spool/MailScanner/milterin

There is an empty line in the middle of the Received-Header:

It looks like a bug in the code beginning at https://github.com/MailScanner/v5/blob/master/common/usr/sbin/MSMilter#L231

Line 232 adds the line (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))

Then, if the 'if' statements at lines 235 and 239 are both false, nothing more is added except another '\n' at line 244.

I think something like this (not tested)

--- /usr/sbin/MSMilter 2022-07-03 10:09:04.000000000 -0700 +++ MSMilter 2022-10-21 15:52:39.506030473 -0700 @@ -233,15 +233,13 @@ MailScanner::Log::DebugLog("envrcpt_callback: ssl/tls detected"); } if (!defined($symbols->{'H'}->{'{cert_subject}'})) {

  • ${$message_ref} .= "\t(no client certificate requested)";
  • ${$message_ref} .= "\t(no client certificate requested)\n"; MailScanner::Log::DebugLog("envrcpt_callback: no client certificate requested"); } if (defined($symbols->{'M'}->{'{auth_type}'}) && defined($symbols->{'M'}->{'{auth_authen}'})) { ${$message_ref} .= "\t(Authenticated sender)\n"; MailScanner::Log::DebugLog("envrcpt_callback: Authenticated sender: " . $symbols->{'M'}->{'{auth_authen}'}); ${$message_ref} = "A{'M'}->{'{auth_type}'} . ">\n" . ${$message_ref};
  • } else {
  • ${$message_ref} .= "\n"; } ${$message_ref} .= "\tby " . hostname . ' (MailScanner Milter) with SMTP id '; }

will fix it.

-- Mark Sapiro @.***> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

shawniverson commented 1 year ago

Looks good, good catch

dneuhaeuser commented 1 year ago

I can confirm the suggested patch works for me!

dneuhaeuser commented 1 year ago

see PR #619

shawniverson commented 1 year ago

This one is an important fix, so I will prepare a updated release.

shawniverson commented 1 year ago

v5.4.5-2 published

shawniverson commented 1 year ago

Found a critical bug upstream in a perl module, please jump ahead to v5.4.5-3 for testing.

shawniverson commented 1 year ago

@dneuhaeuser can we close this issue? Working?

dneuhaeuser commented 1 year ago

yes, problem solved.