I'm simply logging greylisted connections;
it's spamlogd that whitelists them just because they're logged.
It doesn't make sense that logging greylisted or blacklisted connections
would immediately turn them into being whitelisted by spamlogd.
Same goes for logging connections that are already whitelisted
through rules and tables other than <spamd-white>.
Why would you want them whitelisted several times?
My second patch fixes these issues, without breaking valid setups.
Yes, one could log stuff into different pflog interfaces, but
I don't understand why pf.conf `pass in ... log ... port smtp ...`
is effectively redefined to mean `add <spamd-white>`
when spamlogd is running, even when connections are redirected
to spamd for stuttering or greylisting.
That's not something that seems reasonable, and an update-if-exists
logic would make so much more sense.
C.
On 2013-W10-3 14:56 -0700, Bob Beck wrote:
No constatine - that is not the best approach. if you are whitelisting
grelisted connections
or blacklisted connections that are blocked you have your pf.conf or
spamlogd setup wrong.
On Wed, Mar 6, 2013 at 2:54 PM, Constantine A. Murenin <c...@cns.su> wrote:
> 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 <c...@cns.su> 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);
>
