Bob, I agree, the hdr->rewritten approach is not good.
I think the best approach here would be to not add any new entries
on incoming connections in the first place, but only keep updating
the existing ones (when the connection is incoming).
In addition to not whitelisting greylisted or blocked connection that
are logged, this would also prevent the case of double-whitelisting
the connections that are logged and whitelisted through other rules,
without any adverse side effects or unexpected behaviour.
Patch attached inline.
C.
On 2013-W10-3 13:47 -0700, Bob Beck wrote:
No constantine - the solution is to simply not use the "log" keyword
on such traffic
All of my boxen I run this on also rewite the traffic to (pool) of
mailservers so this is
not accurate.
Simply don't log the traffic you don't want spamlogd to see. the
*point* of spamlogd
is to ensure all continuing valid connections *stay* whitelisted.
On Wed, Mar 6, 2013 at 1:08 PM, Constantine A. Murenin <[email protected]> wrote:
> Hi,
>
> I've started using spamlogd, and since then, every single connection attempt
> results in the host being whitelisted.
>
> I log some `rdr-to 127.0.0.1 port spamd` connection attempts into pflog, and
> it would seem like spamlogd filter (for port 25) is picking up the original
> dport, not the rewritten one (with hdr->dport containing original port,
> too).
>
> Not sure of the correct solution, but one of the options is to look at the
> hdr->rewritten field, and only act if it is false. This might impact
> someone who does pf rewrites for sendmail itself, but at least it's not
> going to let all the spam in for someone who simply logs stuff up. A patch
> is attached.
>
> Cheers,
> Constantine.
Index: spamlogd.c
===================================================================
RCS file: /cvs/OpenBSD-CVS/src/libexec/spamlogd/spamlogd.c,v
retrieving revision 1.21
diff -u -d -p -8 -r1.21 spamlogd.c
--- spamlogd.c 18 Mar 2011 22:37:06 -0000 1.21
+++ spamlogd.c 6 Mar 2013 21:14:51 -0000
@@ -75,17 +75,17 @@ pcap_t *hpcap = NULL;
struct syslog_data sdata = SYSLOG_DATA_INIT;
time_t whiteexp = WHITEEXP;
extern char *__progname;
void logmsg(int , const char *, ...);
void sighandler_close(int);
int init_pcap(void);
void logpkt_handler(u_char *, const struct pcap_pkthdr *, const u_char *);
-int dbupdate(char *, char *);
+int dbupdate(char *, char *, int);
void usage(void);
void
logmsg(int pri, const char *msg, ...)
{
va_list ap;
va_start(ap, msg);
@@ -187,22 +187,22 @@ logpkt_handler(u_char *user, const struc
sizeof(ipstraddr));
}
if (ipstraddr[0] != '\0') {
if (hdr->dir == PF_IN)
logmsg(LOG_DEBUG,"inbound %s", ipstraddr);
else
logmsg(LOG_DEBUG,"outbound %s", ipstraddr);
- dbupdate(PATH_SPAMD_DB, ipstraddr);
+ dbupdate(PATH_SPAMD_DB, ipstraddr, hdr->dir == PF_IN);
}
}
int
-dbupdate(char *dbname, char *ip)
+dbupdate(char *dbname, char *ip, int updateonly)
{
HASHINFO hashinfo;
DBT dbk, dbd;
DB *db;
struct gdata gd;
time_t now;
int r;
struct in_addr ia;
@@ -227,16 +227,20 @@ dbupdate(char *dbname, char *ip)
/* add or update whitelist entry */
r = db->get(db, &dbk, &dbd, 0);
if (r == -1) {
logmsg(LOG_NOTICE, "db->get failed (%m)");
goto bad;
}
if (r) {
+ if (updateonly) {
+ logmsg(LOG_DEBUG,"ignoring %s", ip);
+ goto bad;
+ }
/* new entry */
memset(&gd, 0, sizeof(gd));
gd.first = now;
gd.bcount = 1;
gd.pass = now;
gd.expire = now + whiteexp;
memset(&dbk, 0, sizeof(dbk));
dbk.size = strlen(ip);
