Milter Authenticated users not honoring "Spam List Skip If Authenticated" setting

[alexskynet]

We have a scenario where mobile users send mail through Postfix using DB SASL authentication

Sometimes clients get listed IPs so their mails go into spam mode.

We used to set "Spam List Skip If Authenticated = yes" to avoid this, but since we changed to milter mode (mta=msmail) the setting is not honored anymore (it was honored with mta=postifix).

I'm not a milter expert, but I can see Postifx logs whit lines like:

May 20 09:39:35 mailserver postfix/smtpd[12970]: 41223EE82C: client=host-[obfuscated}, sasl_method=PLAIN, sasl_username=username@mydomain.com

Looking into MSMail.pm around line 404 I see the function that loads milter data into the @metadata array but no A field is managed (that should be the AUTH method) or maybe even passed from Postfix (I added some logs to see what data si retrieved).

It looks only O, S and R fields are populated

Furthermore in Message.pm around line 750, where the code checks for authenticate users , it looks like postfix and exim mta are the only supported ones.

MS-Milter docs say nothing about this, so how is it supposed to work?

[github-actions]

Thank you for submitting your first issue to MailScanner! We will respond to you soon!

Sounds like we need to capture the A field here.

[alexskynet]

Thank you @shawniverson

In MSMilter around line 239 in encrypt_callback function I can see access to the data coming from postfix SASL:

if (defined($symbols->{'M'}->{'{auth_type}'}) && defined($symbols->{'M'}->{'{auth_authen}'})) {
${$message_ref} .= "\t(Authenticated sender: " . $symbols->{'M'}->{'{auth_authen}'} . ")\n";
MailScanner::Log::DebugLog("envrcpt_callback: Authenticated sender: " . $symbols->{'M'}->{'{auth_authen}'});
}

The code seeks for a M field (Mechanism I suppose)
This code may work fine for detecting auth senders, but it is not passed back in @metadata array
I may be wrong in catching the A field maybe we need this M field to get sure.
I supposed A since in Message.pm around line 755 in postfix detection for auth users I see

if ($metadata =~ m/^Asasl_method=(PLAIN|LOGIN)$/) {

So here it looks for A field

[alexskynet]

Ok I managed to have it working.

This is WIP but the patch works for me using milter with Postfix:

Here are the changes:

MSMilter

Here I capture the A field an push it to the message metadata

238         }
239         if (defined($symbols->{'M'}->{'{auth_type}'}) && defined($symbols->{'M'}->{'{auth_authen}'})) {
240             ${$message_ref} .= "\t(Authenticated sender: " . $symbols->{'M'}->{'{auth_authen}'} . ")\n";
241             MailScanner::Log::DebugLog("envrcpt_callback: Authenticated sender: " . $symbols->{'M'}->{'{auth_authen}'});
242                 ${$message_ref} = 'Asasl_method=' . $symbols->{'M'}->{'{auth_type}'} . "\nAsasl_username=" .$symbols->{'M'}->{'{auth_authen}'}."\n" . ${$message_ref};
243                 # MailScanner::Log::InfoLog("MILTER  authenticated  ");
244         } else {

Msmail.pm

Get the A field and push it into the @metadata

434         } elsif ($recdata =~ /^E</) {
435             $recdata =~ s/^E<//;
436             $recdata =~ s/\>$//;
437             push @{$message->{metadata}}, "E$recdata";
438             MailScanner::Log::DebugLog("MSMail: ReadQf: from = $recdata");
439             $pos = tell $RQf
440         } elsif ($recdata =~ /^Asasl/) {
441                # MailScanner::Log::InfoLog(">>> MILTER: $recdata");
442                 push @{$message->{metadata}}, "$recdata";
443                 MailScanner::Log::DebugLog("MSMail: ReadQf: from = $recdata");
444                 $pos = tell $RQf
445          } else  {
446             last;
447         }

Message.pm

This patch only works for milter in conjuction with postfix. I suppose. It may need to be reworked for Exim

749   my $isauthenticated = 0;
750   if ( ( MailScanner::Config::Value('mta') == "postfix" ||  MailScanner::Config::Value('mta') == "msmail" ) &&
751          MailScanner::Config::Value('spamlistskipifauthenticated')) {
752     # MailScanner::Log::InfoLog(Dumper($metadata));
753     # Test if sender is authenticated on mta
754     foreach my $metadata (@{$this->{metadata}}) {
755       #Postfix
756       if ($metadata =~ m/^Asasl_method=(PLAIN|LOGIN)$/) {
757         MailScanner::Log::InfoLog("Sender was authenticated - Not checking RBLs");
758         $isauthenticated = 1;
759       }
760     }
761   } elsif (MailScanner::Config::Value('mta') == "exim" && MailScanner::Config::Value('spamlistskipifauthenticated')) {

Now my MailScanner honors the skipping of lists check for auth users

Hope this may help

[alexskynet]

Another idea:

In MSMilter

      240             ${$message_ref} .= "\t(Authenticated sender: " . $symbols->{'M'}->{'{auth_authen}'} . ")\n";   

here we expose the auth username to the world and this may be a security issue.

This may be done in postfix with the:

smtpd_sasl_authenticated_header = yes

directive.

I propose MS should never override a MTA setting causing information leak

[alexskynet]

PROPOSAL

What about adding to MS a setting like
Auto Whitelist Auth Users = 1

and then in Message.pm

755 #Postfix
756 if ($metadata =~ m/^Asasl_method=(PLAIN|LOGIN)$/) {
757 MailScanner::Log::InfoLog("Sender was authenticated - Not checking RBLs");
758 $isauthenticated = 1;
759 $iswhitelisted = 1;
760 }

[alexskynet]

Here are the final patches

This works for postfix and postfix as milter and the new setting "Auto whitelist Auth Users" also works for Exim

[EDIT} Added the new configuration option in ConfigDefs.pl

ATTACHMENT REMOVED SEE LATER VERSION

[alexskynet]

Previous code may have a problem since some users using pipelining complain about an empty CR header added in message header metadata.
The users complain about this to happen only starting from second message in pipeline batch so I suspect the sasl header gets empty since the user already authenticated so no sasl_auth is detected.
I have no evidence of the problem untill now.
Really not sure about this complain (it seems to happen only using pipelining and never with single messages) and how to fix it, anyway adapted the code to be compliant to other field capture and changed regexp to also remove endline char in detection.
Maybe @shawniverson may take a look and suggest what to fix if any.
Any suggestion is welcome.
Here are modified patches

ATTACHMENT REMOVED SEE LATER VERSION

[Edit sorrry forgot to add MSMail.patch now it is included too]

[alexskynet]

Ok Guys
Fixed the header bug
Here are the final patches to enable both features skipRBL for Auth users and optional autowhitelist

patches.zip