Bug#1031196: spamass-milter: 'Could not retrieve sendmail macro "b"' with postfix: PATCH

[none]

Package: spamass-milter
Version: 0.4.0-2
Followup-For: Bug #1004485
X-Debbugs-Cc: debtr...@firemail.cc
Control: tags -1 patch

Dear maintainer,

In this patch I've included a fix for this problem, which includes a new option for the /etc/default config. By default it is commented out so that the behaviour doesn't change.

This introduces the -Y command line flag for "postfix compatibility mode" which explicitly sets default values for macros which postfix doesn't support, or doesn't support in the ENVRCPT context.

There's another hidden issue here: when using spamass-milter this way with postfix, it doesn't support the {auth_type} macro in the ENVRCPT context, which is handled by function mlfi_envrcpt(). The fallback there is to always assume unauthenticated sender. This is fine if you don't spamfilter mail from authenticated users. However it would be problematic in a scenario where someone wants to spamfilter incoming mail from authenticated users but has some spamassassin rules that change the score based on whether the sender is authenticated or not - which is the whole reason why all those bits of information are requested from the MTA using macros and included for spamassassin in the first place. You only see the 'b' macro and never the '{auth_type}' macro failing because 'b' is requested first.

Therefore I've made the -Y dependend on the -I flag ("don't filter mail by authenticated users"), to make this behaviour explicit and visible.

The patch doesn't fix the cut-off manpage part for -x (which might be more visible with this change), nor the broken indendation in some places, and uses code style of surrounding code.

On a side note:
> It goes away if I add b to milter_rcpt_macros, so presumably that should be added to the README.

This is wrong and does not work with postfix.



*** fix-1004485.patch
---
debian/spamass-milter.default | 4 ++
spamass-milter.1.in | 7 +++
spamass-milter.cpp | 81 +++++++++++++++++++++++++----------
3 files changed, 69 insertions(+), 23 deletions(-)

diff --git a/debian/spamass-milter.default b/debian/spamass-milter.default
index 8922fb5..d94f99a 100644
--- a/debian/spamass-milter.default
+++ b/debian/spamass-milter.default
@@ -17,6 +17,10 @@ OPTIONS="-u spamass-milter -i 127.0.0.1"
# Do not modify Subject:, Content-Type: or body.
#OPTIONS="${OPTIONS} -m"

+# Postfix compatibility mode for before-queue-filtering
+# only filter mail from unauthenticated senders
+#OPTIONS="${OPTIONS} -Y -I"
+
######################################
# If /usr/sbin/postfix is executable, the following are set by
# default. You can override them by uncommenting and changing them
diff --git a/spamass-milter.1.in b/spamass-milter.1.in
index dfe56ba..c10836a 100644
--- a/spamass-milter.1.in
+++ b/spamass-milter.1.in
@@ -23,6 +23,7 @@
.Op Fl r rejectmsg
.Op Fl u Ar defaultuser
.Op Fl x
+.Op Fl Y
.Op Fl S /path/to/sendmail
.Op Fl - Ar spamc flags ...
.Sh DESCRIPTION
@@ -221,6 +222,12 @@ Requires the
flag. The spamass-milter configuration process does its
best to find sendmail, but it is possible to override this compiled-in
setting via the
+.It Fl Y
+Postfix pre-queue compatibility mode. Uses default values for sendmail
+macros which Postfix doesn't support. Assumes all filtered messages
+originate from unauthenticated clients, therefore requires
+.Fl I
+flag.
.It Fl - Ar spamc flags ...
Pass all remaining options to spamc.
This allows you to connect to a remote spamd with
diff --git a/spamass-milter.cpp b/spamass-milter.cpp
index fdda5f0..e376df8 100644
--- a/spamass-milter.cpp
+++ b/spamass-milter.cpp
@@ -178,6 +178,7 @@ bool flag_expand = false; /* alias/virtusertable expansion */
bool ignore_authenticated_senders = false;
bool warnedmacro = false; /* have we logged that we couldn't fetch a macro? */
bool auth = false; /* don't scan authenticated users */
+bool postfix_compat = false; /* replace some sendmail macros with default values */

// {{{ main()

@@ -185,7 +186,7 @@ int
main(int argc, char* argv[])
{
int c, err = 0;
- const char *args = "afd:mMp:P:r:u:D:i:Ib:B:e:xS:R:C:";
+ const char *args = "afd:mMp:P:r:u:D:i:Ib:B:e:xS:R:C:Y";
char *sock = NULL;
bool dofork = false;
char *pidfilename = NULL;
@@ -281,6 +282,9 @@ main(int argc, char* argv[])
case 'x':
flag_expand = true;
break;
+ case 'Y':
+ postfix_compat = true;
+ break;
case '?':
err = 1;
break;
@@ -293,6 +297,12 @@ main(int argc, char* argv[])
err=1;
}

+ if (postfix_compat && !ignore_authenticated_senders)
+ {
+ fprintf(stderr, "-Y flag requires -I\n");
+ err=1;
+ }
+
/* remember the remainer of the arguments so we can pass them to spamc */
spamc_argc = argc - optind;
spamc_argv = argv + optind;
@@ -302,7 +312,7 @@ main(int argc, char* argv[])
cout << "SpamAssassin Sendmail Milter Plugin" << endl;
cout << "Usage: spamass-milter -p socket [-b|-B bucket] [-d xx[,yy...]] [-D host]" << endl;
cout << " [-e defaultdomain] [-f] [-i networks] [-I] [-m] [-M]" << endl;
- cout << " [-P pidfile] [-r nn] [-u defaultuser] [-x] [-a]" << endl;
+ cout << " [-P pidfile] [-r nn] [-u defaultuser] [-x] [-Y] [-a]" << endl;
cout << " [-C rejectcode] [ -R rejectmsg ]" << endl;
cout << " [-- spamc args ]" << endl;
cout << " -p socket: path to create socket" << endl;
@@ -327,6 +337,7 @@ main(int argc, char* argv[])
cout << " -u defaultuser: pass the recipient's username to spamc.\n"
" Uses 'defaultuser' if there are multiple recipients." << endl;
cout << " -x: pass email address through alias and virtusertable expansion." << endl;
+ cout << " -Y: use default values for sendmail macros not supported by postfix." << endl;
cout << " -a: don't scan messages over an authenticated connection." << endl;
cout << " -- spamc args: pass the remaining flags to spamc." << endl;

@@ -934,22 +945,28 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcpt)
char date[32];

/* RFC 822 date. */
- macro_b = smfi_getsymval(ctx, const_cast<char *>("b"));
- if (!macro_b)
+ /* Postfix doesn't support the 'get current timestamp' macro. */
+ if (!postfix_compat)
+ macro_b = smfi_getsymval(ctx, const_cast<char *>("b"));
+ if (postfix_compat || !macro_b)
{
time_t tval;
time(&tval);
strftime(date, sizeof(date), "%a, %d %b %Y %H:%M:%S %z", localtime(&tval));
macro_b = date;
- warnmacro("b", "ENVRCPT");
+ if (!postfix_compat)
+ warnmacro("b", "ENVRCPT");
}

/* queue ID */
- macro_i = smfi_getsymval(ctx, const_cast<char *>("i"));
- if (!macro_i)
+ /* Postfix doesn't assign a queue id at rcpt stage yet (sendmail does) */
+ if (!postfix_compat)
+ macro_i = smfi_getsymval(ctx, const_cast<char *>("i"));
+ if (postfix_compat || !macro_i)
{
macro_i = "unknown";
- warnmacro("i", "ENVRCPT");
+ if (!postfix_compat)
+ warnmacro("i", "ENVRCPT");
}

/* FQDN of this site */
@@ -961,26 +978,38 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcpt)
}

/* Protocol used to receive the message */
- macro_r = smfi_getsymval(ctx, const_cast<char *>("r"));
- if (!macro_r)
+ /* Postfix doesn't support the 'protocol type' macro, assume SMTP */
+ if (!postfix_compat)
+ macro_r = smfi_getsymval(ctx, const_cast<char *>("r"));
+ if (postfix_compat || !macro_r)
{
macro_r = "SMTP";
- warnmacro("r", "ENVRCPT");
+ if (!postfix_compat)
+ warnmacro("r", "ENVRCPT");
}

/* Authenticated bits Information */
- macro_auth_type = smfi_getsymval(ctx, "{auth_type}");
- if (!macro_auth_type)
+ /* Postfix doesn't support the 'authentication type' macro at RCPT stage */
+ /* Assume no auth, as only mails by unauth senders are filtered. */
+ if (!postfix_compat)
+ macro_auth_type = smfi_getsymval(ctx, "{auth_type}");
+ if (postfix_compat || !macro_auth_type)
{
macro_auth_type = "";
- warnmacro("auth_type", "ENVRCPT");
+ if (!postfix_compat)
+ warnmacro("auth_type", "ENVRCPT");
}

- macro_auth_ssf = smfi_getsymval(ctx, "{auth_ssf}");
- if (!macro_auth_ssf)
+ /* Postfix doesn't support the
+ 'authenticated connection encryption mechanism strength in bits' macro
+ */
+ if (!postfix_compat)
+ macro_auth_ssf = smfi_getsymval(ctx, "{auth_ssf}");
+ if (postfix_compat || !macro_auth_ssf)
{
macro_auth_ssf = "";
- warnmacro("auth_ssf", "ENVRCPT");
+ if (!postfix_compat)
+ warnmacro("auth_ssf", "ENVRCPT");
}

/* Sendmail currently cannot pass us the {s} macro, but
@@ -988,8 +1017,10 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcpt)
fixed. Until that day, use the value remembered by
mlfi_helo()
*/
- macro_s = smfi_getsymval(ctx, const_cast<char *>("s"));
- if (!macro_s)
+ /* Postfix doesn't support the 'sender host name' macro either */
+ if (!postfix_compat)
+ macro_s = smfi_getsymval(ctx, const_cast<char *>("s"));
+ if (postfix_compat || !macro_s)
macro_s = sctx->helo;
if (!macro_s)
macro_s = "nohelo";
@@ -1003,8 +1034,11 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcpt)
}

/* Sendmail .cf version */
- macro_Z = smfi_getsymval(ctx, const_cast<char *>("Z"));
- if (!macro_Z)
+ /* Postfix doesn't support the 'sendmail config file version' macro */
+ /* Instead of taking a default value, leave out this part of the header. */
+ if (!postfix_compat)
+ macro_Z = smfi_getsymval(ctx, const_cast<char *>("Z"));
+ if (!postfix_compat && !macro_Z)
{
macro_Z = "8.13.0";
warnmacro("Z", "ENVRCPT");
@@ -1030,8 +1064,9 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcpt)
}
rec_header+=(string)")\r\n\t";
}
- rec_header+=(string)"by "+macro_j+" ("+macro_v+"/"+macro_Z+") with "+
- macro_r+" id "+macro_i+"\r\n\t"+
+ rec_header+=(string)"by "+macro_j+" ("+macro_v+
+ (postfix_compat?"":(string)"/"+macro_Z)+
+ ") with "+macro_r+" id "+macro_i+"\r\n\t"+
macro_b+"\r\n\t"+
"(envelope-from "+assassin->from()+");\r\n";

--
2.30.2



-- System Information:
Debian Release: 11.5
APT prefers stable-updates
APT policy: (500, 'stable-updates'), (500, 'stable-security'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 5.10.0-19-amd64 (SMP w/1 CPU thread)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages spamass-milter depends on:
ii adduser 3.118
ii libc6 2.31-13+deb11u5
ii libgcc-s1 10.2.1-6
ii libmilter1.0.1 8.15.2-22
ii libstdc++6 10.2.1-6
ii spamc 3.4.6-1

Versions of packages spamass-milter recommends:
ii postfix 3.5.13-0+deb11u1
ii spamassassin 3.4.6-1

spamass-milter suggests no packages.

-- Configuration Files:
/etc/default/spamass-milter changed [not included]

-- no debconf information

[Don Armstrong]

Control: tag -1 moreinfo - patch

On Mon, 13 Feb 2023, none wrote:
> In this patch I've included a fix for this problem, which includes a
> new option for the /etc/default config. By default it is commented out
> so that the behaviour doesn't change.
>
> This introduces the -Y command line flag for "postfix compatibility
> mode" which explicitly sets default values for macros which postfix
> doesn't support, or doesn't support in the ENVRCPT context.

It's not clear to me why we'd add this flag. If the macros aren't
supported, the code already falls back to reasonable defaults. The only
change this seems to make is to silence warnings (and breaks the macros
that postfix already supports).

Am I missing something?

> You only see the 'b' macro and never the '{auth_type}' macro failing
> because 'b' is requested first.

That's because warnmacro only warns on the first warning. You're
probably missing the correct configuration in your main.cf:

# spamass-milter configuration
smtpd_milters = unix:/spamass/spamass.sock
# milter macros useful for spamass-milter
milter_connect_macros = j {daemon_name} v {if_name} _
milter_data_macros = j i {auth_type} {daemon_name} v {if_name} _
milter_rcpt_macros = j {auth_type} {daemon_name} v {if_name} _



--
Don Armstrong https://www.donarmstrong.com

Taxes are not levied for the benefit of the taxed.
-- Robert Heinlein _Time Enough For Love_ p250