Hi folks,
great work, but just want to add some quick comments which may
lead to changes ...
On Fri, Sep 26, 2008 at 02:07:09PM +0200, Manuel Badzong wrote:
[..]
> diff -ruN milter-greylist/Makefile.in mg_manu/Makefile.in
> --- milter-greylist/Makefile.in 2008-09-07 04:12:15.000000000 +0200
> +++ mg_manu/Makefile.in 2008-09-26 12:26:18.000000000 +0200
> @@ -56,10 +56,10 @@
> OBJ= milter-greylist.o pending.o sync.o dnsrbl.o list.o macro.o \
> conf_yacc.o dump_yacc.o conf.o autowhite.o dump.o spf.o \
> acl.o urlcheck.o stat.o clock.o geoip.o fd_pool.o prop.o \
> - ldapcheck.o dkimcheck.o p0f.o
> + ldapcheck.o dkimcheck.o p0f.o sa.o
> SRC= milter-greylist.c pending.c sync.c conf.c macro.c stat.c \
> clock.c autowhite.c dump.c spf.c acl.c dnsrbl.c list.c \
> - urlcheck.c geoip.c prop.c ldapcheck.c dkimcheck.c p0f.c
> + urlcheck.c geoip.c prop.c ldapcheck.c dkimcheck.c p0f.c sa.c
> GENSRC= conf_yacc.c conf_lex.c dump_yacc.c dump_lex.c
>
> VPATH= ${SRCDIR}
> diff -ruN milter-greylist/acl.c mg_manu/acl.c
> --- milter-greylist/acl.c 2008-09-23 20:55:11.000000000 +0200
> +++ mg_manu/acl.c 2008-09-26 11:25:58.000000000 +0200
> @@ -90,6 +90,9 @@
> #ifdef USE_DKIM
> #include "dkimcheck.h"
> #endif
> +#ifdef USE_SA
> +#include "sa.h"
> +#endif
> #include "macro.h"
> #include "clock.h"
> #include "milter-greylist.h"
> @@ -367,6 +370,14 @@
> acl_print_list, acl_add_list,
> NULL, acl_list_filter },
> #endif
> +#ifdef USE_SA
> + { AC_SA, MULTIPLE_OK, AS_DATA, "spamd",
> + AT_NONE, AC_NONE, AC_NONE, EXF_SA,
> + acl_print_null, NULL, NULL, sa_spam },
> + { AC_SASCORE, MULTIPLE_OK, AS_DATA, "spamd score",
> + AT_OPNUM, AC_NONE, AC_NONE, EXF_SA,
> + acl_print_opnum, acl_add_opnum, NULL, sa_score },
> +#endif
> };
>
> struct {
> diff -ruN milter-greylist/acl.h mg_manu/acl.h
> --- milter-greylist/acl.h 2008-09-07 02:13:34.000000000 +0200
> +++ mg_manu/acl.h 2008-09-26 11:06:05.000000000 +0200
> @@ -52,7 +52,7 @@
> typedef enum { AS_NONE, AS_RCPT, AS_DATA, AS_ANY, } acl_stage_t;
> typedef enum { AT_NONE, AT_STRING, AT_REGEX, AT_NETBLOCK, AT_OPNUM,
> AT_CLOCKSPEC, AT_DNSRBL, AT_URLCHECK, AT_MACRO,
> - AT_LIST, AT_PROP, AT_SPF, AT_DKIM,
> + AT_LIST, AT_PROP, AT_SPF, AT_DKIM,
> AT_LDAPCHECK } acl_data_type_t;
>
> typedef enum {
> @@ -108,6 +108,8 @@
> AC_P0F,
> AC_P0F_RE,
> AC_P0F_LIST,
> + AC_SA,
> + AC_SASCORE,
> } acl_clause_t;
>
> struct acl_clause;
> @@ -304,6 +306,7 @@
> struct acl_param *, struct mlfi_priv *);
> int acl_msgsize_cmp(acl_data_t *, acl_stage_t,
> struct acl_param *, struct mlfi_priv *);
> +int acl_opnum_cmp(int, enum operator, int);
> int myregexec(struct mlfi_priv *, acl_data_t *,
> struct acl_param *, const char *);
>
> @@ -340,4 +343,5 @@
> #define EXF_LDAPCHECK (1 << 27)
> #define EXF_DKIM (1 << 28)
> #define EXF_P0F (1 << 29)
> +#define EXF_SA (1 << 30)
> #endif /* _ACL_H_ */
> diff -ruN milter-greylist/conf.c mg_manu/conf.c
> --- milter-greylist/conf.c 2008-09-07 02:13:34.000000000 +0200
> +++ mg_manu/conf.c 2008-09-25 22:37:49.000000000 +0200
> @@ -78,6 +78,9 @@
> #ifdef USE_P0F
> #include "p0f.h"
> #endif
> +#ifdef USE_SA
> +#include "sa.h"
> +#endif
> #include "autowhite.h"
> #include "conf.h"
> #include "sync.h"
> @@ -467,5 +470,9 @@
> #ifdef USE_P0F
> c->c_p0fsock[0] = '\0';
> #endif
> +#ifdef USE_SA
> + c->c_sasock[0] = '\0';
> + c->c_sasocktype[0] = '\0';
> +#endif
> return;
> }
> diff -ruN milter-greylist/conf.h mg_manu/conf.h
> --- milter-greylist/conf.h 2008-09-07 02:13:34.000000000 +0200
> +++ mg_manu/conf.h 2008-09-25 20:57:08.000000000 +0200
> @@ -112,6 +112,10 @@
> #ifdef USE_P0F
> char c_p0fsock[QSTRLEN + 1];
> #endif
> +#ifdef USE_SA
> + char c_sasock[QSTRLEN + 1];
> + char c_sasocktype[QSTRLEN + 1];
> +#endif
> };
>
> /* c_forced flags */
> diff -ruN milter-greylist/conf_lex.l mg_manu/conf_lex.l
> --- milter-greylist/conf_lex.l 2008-09-07 18:50:16.000000000 +0200
> +++ mg_manu/conf_lex.l 2008-09-26 11:23:49.000000000 +0200
> @@ -100,6 +100,9 @@
> ldapcheck [Ll][Dd][Aa][Pp][Cc][Hh][Ee][Cc][Kk]
> p0fsock [Pp]0[Ff][Ss][Oo][Cc][Kk]
> p0f [Pp]0[Ff]
> +spamdsock [Ss][Pp][Aa][Mm][Dd][Ss][Oo][Cc][Kk]
> +spamdsockt [Ii][Nn][Ee][Tt]|[Uu][Nn][Ii][Xx]
> +spamd [Ss][Pp][Aa][Mm][Dd]
> openlist "{"
> closelist "}"
> nextln "\\".*"\n"
> @@ -230,6 +233,13 @@
> {ldapcheck} { return LDAPCHECK; }
> {p0fsock} { return P0FSOCK; }
> {p0f} { BEGIN(S_REGEX); return P0F; }
> +{spamdsock} { return SPAMDSOCK; }
> +{spamdsockt} {
> + strncpy(yylval.spamdsockt, yytext, QSTRLEN);
> + yylval.spamdsockt[QSTRLEN] = '\0';
> + return SPAMDSOCKT;
> + }
> +{spamd} { return SPAMD; }
> {pidfile} { return PIDFILE; }
> {dumpfile} { return GLDUMPFILE; }
> {subnetmatch} { return SUBNETMATCH; }
> diff -ruN milter-greylist/conf_yacc.y mg_manu/conf_yacc.y
> --- milter-greylist/conf_yacc.y 2008-09-07 02:13:34.000000000 +0200
> +++ mg_manu/conf_yacc.y 2008-09-26 11:23:07.000000000 +0200
> @@ -1,4 +1,4 @@
> -%token TNUMBER ADDR IPADDR IP6ADDR CIDR HELO FROM RCPT EMAIL PEER AUTOWHITE GREYLIST NOAUTH NOACCESSDB EXTENDEDREGEX NOSPF QUIET TESTMODE VERBOSE PIDFILE GLDUMPFILE QSTRING TDELAY SUBNETMATCH SUBNETMATCH6 SOCKET USER NODETACH REGEX REPORT NONE DELAYS NODELAYS ALL LAZYAW GLDUMPFREQ GLTIMEOUT DOMAIN DOMAINNAME SYNCADDR SYNCSRCADDR PORT ACL WHITELIST DEFAULT STAR DELAYEDREJECT DB NODRAC DRAC DUMP_NO_TIME_TRANSLATION LOGEXPIRED GLXDELAY DNSRBL LIST OPENLIST CLOSELIST BLACKLIST FLUSHADDR CODE ECODE MSG SM_MACRO UNSET URLCHECK RACL DACL GLHEADER BODY MAXPEEK STAT POSTMSG FORK GETPROP CLEAR PROP AUTH TLS SPF DKIMCHECK MSGSIZE RCPTCOUNT OP NO SLASH MINUS COMMA TIME GEOIPDB GEOIP PASS FAIL SOFTFAIL NEUTRAL UNKNWON ERROR SELF SPF_STATUS LDAPCONF LDAPCHECK P0FSOCK P0F
> +%token TNUMBER ADDR IPADDR IP6ADDR CIDR HELO FROM RCPT EMAIL PEER AUTOWHITE GREYLIST NOAUTH NOACCESSDB EXTENDEDREGEX NOSPF QUIET TESTMODE VERBOSE PIDFILE GLDUMPFILE QSTRING TDELAY SUBNETMATCH SUBNETMATCH6 SOCKET USER NODETACH REGEX REPORT NONE DELAYS NODELAYS ALL LAZYAW GLDUMPFREQ GLTIMEOUT DOMAIN DOMAINNAME SYNCADDR SYNCSRCADDR PORT ACL WHITELIST DEFAULT STAR DELAYEDREJECT DB NODRAC DRAC DUMP_NO_TIME_TRANSLATION LOGEXPIRED GLXDELAY DNSRBL LIST OPENLIST CLOSELIST BLACKLIST FLUSHADDR CODE ECODE MSG SM_MACRO UNSET URLCHECK RACL DACL GLHEADER BODY MAXPEEK STAT POSTMSG FORK GETPROP CLEAR PROP AUTH TLS SPF DKIMCHECK MSGSIZE RCPTCOUNT OP NO SLASH MINUS COMMA TIME GEOIPDB GEOIP PASS FAIL SOFTFAIL NEUTRAL UNKNWON ERROR SELF SPF_STATUS LDAPCONF LDAPCHECK P0FSOCK P0F SPAMDSOCK SPAMDSOCKT SPAMD
>
> %{
> #include "config.h"
> @@ -42,6 +42,9 @@
> #ifdef USE_P0F
> #include "p0f.h"
> #endif
> +#ifdef USE_SA
> +#include "sa.h"
> +#endif
> #include "stat.h"
> #include "clock.h"
> #include "spf.h"
> @@ -75,6 +78,7 @@
> char prop[QSTRLEN + 1];
> enum spf_status spf_status;
> enum spf_status dkim_status;
> + char spamdsockt[QSTRLEN + 1];
> }
> %type <ipaddr> IPADDR;
> %type <ip6addr> IP6ADDR;
> @@ -88,6 +92,7 @@
> %type <op> OP;
> %type <prop> PROP;
> %type <spf_status> SPF_STATUS;
> +%type <spamdsockt> SPAMDSOCKT;
>
> %%
> lines : lines netblock '\n'
> @@ -137,6 +142,7 @@
> | lines ldapcheckdef '\n'
> | lines ldapconfdef '\n'
> | lines p0fsockdef '\n'
> + | lines spamdsockdef '\n'
> | lines listdef '\n'
> | lines '\n'
> |
> @@ -392,6 +398,19 @@
> #endif
> }
> ;
> +spamdsockdef: SPAMDSOCK SPAMDSOCKT QSTRING {
> +#ifdef USE_SA
> + char path[QSTRLEN + 1];
> +
> + sa_sock_set($2, quotepath(path, $3, QSTRLEN));
> +#else
> + mg_log(LOG_INFO,
> + "spamassassin support not compiled in, "
> + "ignore line %d",
> + conf_line);
> +#endif
> + }
> + ;
> geoipdb: GEOIPDB QSTRING {
> #ifdef USE_GEOIP
> char path[QSTRLEN + 1];
> @@ -646,6 +665,8 @@
> | geoip_clause
> | prop_clause
> | propregex_clause
> + | spamd_clause
> + | spamd_score_clause
> ;
>
> acl_values: acl_value
> @@ -725,6 +746,31 @@
> #endif
> }
> ;
> +spamd_clause: SPAMD {
> +#ifdef USE_SA
> + acl_add_clause(AC_SA, NULL);
> +#else
> + mg_log(LOG_INFO,
> + "spamassassin support not compiled in, ignore line %d",
> + conf_line);
> +#endif
> + }
> + ;
> +spamd_score_clause: SPAMD OP TNUMBER {
> +#ifdef USE_SA
> + struct acl_opnum_data aond;
> +
> + aond.op = $2;
> + aond.num = atoi($3);
> +
> + acl_add_clause(AC_SASCORE, &aond);
> +#else
> + mg_log(LOG_INFO,
> + "spamassassin support not compiled in, ignore line %d",
> + conf_line);
> +#endif
> + }
> + ;
> geoip_clause: GEOIP QSTRING {
> #ifdef USE_GEOIP
> char ccode[IPADDRSTRLEN + 1];
> diff -ruN milter-greylist/configure.ac mg_manu/configure.ac
> --- milter-greylist/configure.ac 2008-09-07 02:13:34.000000000 +0200
> +++ mg_manu/configure.ac 2008-09-26 12:47:06.000000000 +0200
> @@ -427,6 +427,11 @@
> [ --enable-p0f Enable p0f support],
> [if test x$enableval = xyes; then CFLAGS=$CFLAGS" -DUSE_P0F"; fi])
>
> +# Check for SpamAssassin
> +AC_ARG_ENABLE(spamassassin,
> + [ --enable-spamassassin Enable SpamAssassin support],
> + [if test x$enableval = xyes; then CFLAGS=$CFLAGS" -DUSE_SA"; fi])
> +
> # Check for libmilter. For sendmail-8.12.1, -lsm is required too.
> # This uses a gross hack on the second AC_CHECK_LIB first argument, but using
> # [-lsm] in the optionnal 5th argument does not seems to help at all.
> diff -ruN milter-greylist/milter-greylist.c mg_manu/milter-greylist.c
> --- milter-greylist/milter-greylist.c 2008-09-23 20:55:11.000000000 +0200
> +++ mg_manu/milter-greylist.c 2008-09-25 20:21:11.000000000 +0200
> @@ -353,6 +353,9 @@
> priv->priv_p0f = NULL;
> p0f_lookup(priv);
> #endif
> +#ifdef USE_SA
> + priv->priv_sa = 0;
> +#endif
> return SMFIS_CONTINUE;
> }
>
> diff -ruN milter-greylist/milter-greylist.h mg_manu/milter-greylist.h
> --- milter-greylist/milter-greylist.h 2008-09-07 02:13:34.000000000 +0200
> +++ mg_manu/milter-greylist.h 2008-09-26 11:09:05.000000000 +0200
> @@ -213,6 +213,11 @@
> #ifdef USE_P0F
> char *priv_p0f;
> #endif
> +#ifdef USE_SA
> + int priv_sa;
> + int priv_sa_spam;
> + int priv_sa_score10;
> +#endif
> };
>
> sfsistat mlfi_connect(SMFICTX *, char *, _SOCK_ADDR *);
> diff -ruN milter-greylist/sa.c mg_manu/sa.c
> --- milter-greylist/sa.c 1970-01-01 01:00:00.000000000 +0100
> +++ mg_manu/sa.c 2008-09-26 12:51:49.000000000 +0200
> @@ -0,0 +1,429 @@
> +#ifdef USE_SA
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <netdb.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <sysexits.h>
> +#include <syslog.h>
> +
> +#include "acl.h"
> +#include "conf.h"
> +#include "queue.h"
> +#include "milter-greylist.h"
> +
> +#include "sa.h"
> +
> +#define SA_PORT "783"
> +#define SA_BUFLEN 1024
> +
> +#define SA_SPAMD_SPAMD "SPAMD/"
> +#define SA_SPAMD_VERSION "1.1"
> +#define SA_SPAMD_OK "EX_OK"
> +#define SA_SPAMD_SPAM "Spam: "
> +#define SA_SPAMD_TRUE "True"
> +#define SA_SPAMD_FALSE "False"
> +
> +#define SA_ERR_PROTO "sa spamd protocol error"
> +#define SA_ERR_VERSION "sa spamd protocol version mismatch"
> +#define SA_ERR_STATUS "sa spamd returned non-ok"
> +
> +static int sa_check(acl_data_t *, acl_stage_t, struct acl_param *, struct mlfi_priv *);
> +static void sa_rcvhdr(struct mlfi_priv *, char *, int);
> +static char *sa_trim(char *);
> +static int sa_read(int, char *, size_t);
> +static int sa_write(int, char *, size_t);
> +static int sa_socket(char *, char *);
> +static int sa_unix_socket(char *);
> +static int sa_inet_socket(char *, char *);
> +
> +static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
Not needed, see below ...
[..]
> +static void
> +sa_rcvhdr(priv, str, len)
> + struct mlfi_priv *priv;
> + char *str;
> + int len;
> +{
> + struct rcpt rcpt;
> + char ipstr[IPADDRSTRLEN];
> + char myhostname[ADDRLEN + 1];
> + char now[SA_BUFLEN];
> + time_t t;
> + struct tm *tm;
Add this here, explanation see below ...
+ struct tm tm_buf;
> +
> + iptostring(SA(&priv->priv_addr), priv->priv_addrlen, ipstr, sizeof(ipstr));
> +
> + if(gethostname(myhostname, SA_BUFLEN)) {
> + mg_log(LOG_WARNING, "sa gethostname failed");
> + strcpy(myhostname, "unknown");
> + }
> +
> + /*
> + * localtime isn't thread-safe.
> + */
> + t = time(NULL);
> + pthread_mutex_lock(&mutex);
> + tm = localtime(&t);
> + if (strftime(now, len, "%a, %d %b %Y %H:%M:%S %z", tm) == 0) {
> + mg_log(LOG_WARNING, "sa strftime failed");
> + now[0] = '\0';
> + }
> + pthread_mutex_unlock(&mutex);
I would suggest to use localtime_r() (gmtime_r(), respectively) which
makes the use of a mutex obsolete.
+ tm = localtime_r(&t, &tm_buf);
Another problem may arise with strftime formatstring: %z is GNU libc
specific which is probably not portable. Solaris, for instance, do not
have this format specifier.
Taking a look at the sendmail source code (arpadate.c) there is a
respectable and portable routine calculating the offset from the
difference of gmtime() and localtime().
But to make it short, such a beast may not be necessary in this case:
The timestamp in the artificial milter received line is not that
important, especially regarding the timezone. I would suggest to use
gmtime() with a static timezone offset +0000, the time information will
be correct anyway, e.g.
+ tm = gmtime_r(&t, &tm_buf);
+ if (strftime(now, len, "%a, %d %b %Y %H:%M:%S +0000", tm) == 0) {
I hope, this helps ;)
Regards,
JohannMessage
Re: [milter-greylist] spamassassin patch
2008-10-01 by Johann Klasek