Milter-greylist

Hello all,

   Today I've finally succeeded in integration of milter-greylist
with Sun/Oracle Messaging Server (via milter plugin). There were
some problems with support of inet type of sockets (the chown
problem that was recently discussed and fixed), and also the
milter complained about missing {if_addr} values.

   The Messaging Server uses {client_addr} instead, and their
documentation states so, that the milters should be converted
to use it. And for clarity (and to avoid noise in logs) I did.
Resulting patch against 4.4.3 tarball is attached - in the couple
of places that the code uses {if_addr}, we also try to use
{client_addr} if the first attempt returned null. Complain
about absent values only if both are null.

   I am not sure if this behaviour should be default or flagged
(at compile-time or via command-line), I just made it default
as suits this particular mail-server :)

HTH,
//Jim Klimov

Jim Klimov <jimklimov@...> wrote:

>    I am not sure if this behaviour should be default or flagged
> (at compile-time or via command-line), I just made it default
> as suits this particular mail-server :)

You test {if_addr} and fall back to {client_addr} every time the information is
needed, which seems suboptimal. You could add a config option (which would
default to {if_addr}) for that. Something like:
macro_map "{if_addr}" "{client_addr}"

We would then have changes like this:

- ip = smfi_getsymval(priv->priv_ctx, "{if_addr}");
+ ip = smfi_getsymval(priv->priv_ctx, conf.c_macro_map.if_addr);

With the config code change below (not tested, not even built):
Index: conf.c
===================================================================
RCS file: /cvsroot/milter-greylist/conf.c,v
retrieving revision 1.71
diff -U 4 -r1.71 conf.c
--- conf.c      19 Jan 2013 16:01:15 -0000      1.71
+++ conf.c      20 Jul 2013 03:54:12 -0000
@@ -489,6 +489,7 @@
        c->c_spamdsocktype[0] = '\0';
 #endif
        c->c_syncmaxqlen = SYNC_MAXQLEN;
        (void)memset(&c->c_localaddr, 0, sizeof(c->c_localaddr));
+       c->c_macro_map.if_addr = "{if_addr}";
        return;
 }
Index: conf.h
===================================================================
RCS file: /cvsroot/milter-greylist/conf.h,v
retrieving revision 1.55
diff -U 4 -r1.55 conf.h
--- conf.h      19 Jan 2013 16:01:15 -0000      1.55
+++ conf.h      20 Jul 2013 03:54:12 -0000
@@ -119,8 +119,11 @@
        char c_spamdsocktype[QSTRLEN + 1];
 #endif
        struct sockaddr_storage c_localaddr;
        int c_fixldapcheck;
+       struct {
+               char if_addr[QSTRLEN + 1];
+       } c_macro_map;
 };
 
 /* c_forced flags */
 #define C_GLNONE       0x00000
Index: conf_lex.l
===================================================================
RCS file: /cvsroot/milter-greylist/conf_lex.l,v
retrieving revision 1.105
diff -U 4 -r1.105 conf_lex.l
--- conf_lex.l  19 May 2013 05:53:34 -0000      1.105
+++ conf_lex.l  20 Jul 2013 03:54:12 -0000
@@ -142,8 +142,10 @@
 spamdsockt     [Ii][Nn][Ee][Tt]|[Uu][Nn][Ii][Xx]
 spamd          [Ss][Pp][Aa][Mm][Dd]
 ratelimit      [Rr][Aa][Tt][Ee][Ll][Ii][Mm][Ii][Tt]
 key            [Kk][Ee][Yy]
+macro_map      [Mm][Aa][Cc][Rr][Oo]_[Mm][Aa][Pp]
+if_addr                "\"{if_addr}\""
 openlist       "{"
 closelist      "}"
 nextln         "\\".*"\n"
 star           "*"
@@ -344,8 +346,10 @@
 <S_LOGFAC>{none}       { BEGIN(0); return NONE; }
 {ratelimit}            { return RATELIMIT; }
 {data}                 { return DATA; }
 {key}          { return KEY; }
+{macro_map}    { return MACRO_MAP; }
+{if_addr}      { return IF_ADDR; }
 {report}       { return REPORT; }
 <INITIAL>{none}        { return NONE; }
 {stat}         { return STAT; }
 {delays}       { return DELAYS; }
Index: conf_yacc.y
===================================================================
RCS file: /cvsroot/milter-greylist/conf_yacc.y,v
retrieving revision 1.120
diff -U 4 -r1.120 conf_yacc.y
--- conf_yacc.y 19 May 2013 05:53:34 -0000      1.120
+++ conf_yacc.y 20 Jul 2013 03:54:12 -0000
@@ -17,8 +17,9 @@
 %token SPAMDSOCK SPAMDSOCKT SPAMD DOMAINEXACT ADDHEADER NOLOG LDAPBINDDN 
 %token LDAPBINDPW TARPIT TARPIT_SCOPE SESSION COMMAND MX RATELIMIT KEY
 %token DOMATCH DATA LOCALADDR ADDFOOTER CONTINUE FIXLDAPCHECK SUBJTAG
 %token NOENCODE NOESCAPE TSIG NSUPDATE SERVERS RNAME RVALUE TTL CLASS TYPE
+%token MACRO_MAP IF_ADDR
 
 %{
 #include "config.h"
 
@@ -176,8 +177,9 @@
        |       lines listdef '\n'
        |       lines domainexact '\n'
        |       lines syncmaxqlen '\n'
        |       lines ratelimitdef '\n'
+       |       lines macromapdef '\n'
        |       lines '\n'
        |
        ;
 netblock:      ADDR IPADDR CIDR{
@@ -998,8 +1000,13 @@
                                           quotepath(key, $8, QSTRLEN));
                }
        ;
 
+macromapdef:   MACRO_MAP IF_ADDR QSTRING {
+                       quotepath(conf.c_macro_map.if_addr, $3, QSTRLEN);
+               }
+       ;
+
 access_list:   ACL GREYLIST  acl_entry { 
                        acl_register_entry_last(AS_RCPT, A_GREYLIST);
                }
        |       ACL WHITELIST acl_entry { 


-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

Hmmm... it seems that the two options are not identical (not
that it is fatal in the example I see): in the added headers,
the original {if_addr} displayed the MTA's address, like:
"not delayed by $mta_hostname $mta_ip"

With {client_addr} in its place, this displays the MTA host
name and, confusingly but not fatally, the sender's IP address.

So I guess my patch is not academically correct :)
I see the macro is also used in p0f; I wonder if the substitution
would break anything (worse than user reports) there?.. Did not
check further.

//Jim