- ensure greylist entry expiry is not updated until actually whitelisted

  to avoid keeping multiple grey entries around from a single host for
  extra time.
- make -G work
- paranoia and cleanup suggestions from deraadt@
ok millert@

2 files changed, 87 insertions, 77 deletions

diff --git a/libexec/spamd/grey.c b/libexec/spamd/grey.c
index 0c678b907e0..1015b25e226 100644
--- a/libexec/spamd/grey.c
+++ b/libexec/spamd/grey.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: grey.c,v 1.5 2004/02/26 08:52:58 beck Exp $	*/
+/*	$OpenBSD: grey.c,v 1.6 2004/02/28 00:03:59 beck Exp $	*/
 
 /*
  * Copyright (c) 2004 Bob Beck.  All rights reserved.
@@ -39,6 +39,7 @@
 
 #include "grey.h"
 
+extern time_t passtime, greyexp, whiteexp;
 extern struct syslog_data sdata;
 extern struct passwd *pw;
 extern FILE * grey;
@@ -48,10 +49,6 @@ size_t whitecount, whitealloc;
 char **whitelist;
 int pfdev;
 
-DB		*db;
-DBT		dbk, dbd;
-BTREEINFO	btreeinfo;
-
 /* borrowed from dhartmei.. */
 int
 address_valid_v4(const char *a)
@@ -105,12 +102,14 @@ configure_pf(char **addrs, int count)
 	if (pipe(pdes) != 0) {
 		syslog_r(LOG_INFO, &sdata, "pipe failed (%m)");
 		free(fdpath);
+		fdpath = NULL;
 		return(-1);
 	}
 	switch (pid = fork()) {
 	case -1:
 		syslog_r(LOG_INFO, &sdata, "fork failed (%m)");
 		free(fdpath);
+		fdpath = NULL;
 		close(pdes[0]);
 		close(pdes[1]);
 		return(-1);
@@ -128,10 +127,12 @@ configure_pf(char **addrs, int count)
 
 	/* parent */
 	free(fdpath);
+	fdpath = NULL;
 	close(pdes[0]);
 	pf = fdopen(pdes[1], "w");
 	if (pf == NULL) {
 		syslog_r(LOG_INFO, &sdata, "fdopen failed (%m)");
+		close(pdes[1]);
 		return(-1);
 	}
 	for (i = 0; i < count; i++)
@@ -179,9 +180,12 @@ addwhiteaddr(char *addr)
 int
 greyscan(char *dbname)
 {
+	BTREEINFO	btreeinfo;
+	DBT		dbk, dbd;
+	DB		*db;
+	struct gdata	gd;
+	int		r;
 	time_t now = time(NULL);
-	struct gdata gd;
-	int r;
 
 	/* walk db, expire, and whitelist */
 
@@ -199,6 +203,7 @@ greyscan(char *dbname)
 
 		if ((dbk.size < 1) || dbd.size != sizeof(struct gdata)) {
 			db->close(db);
+			db = NULL;
 			return(-1);
 		}
 		memcpy(&gd, dbd.data, sizeof(gd));
@@ -214,9 +219,10 @@ greyscan(char *dbname)
 			if (db->del(db, &dbk, 0)) {
 				db->sync(db, 0);
 				db->close(db);
+				db = NULL;
 				return(-1);
 			}
-		} else if (gd.pass < now) {
+		} else if (gd.pass  <= now) {
 			int tuple = 0;
 			char *cp;
 
@@ -242,6 +248,7 @@ greyscan(char *dbname)
 				dbk.size = strlen(a);
 				dbk.data = a;
 				memset(&dbd, 0, sizeof(dbd));
+				gd.expire = now + whiteexp;
 				dbd.size = sizeof(gd);
 				dbd.data = &gd;
 				if (db->put(db, &dbk, &dbd, 0))
@@ -268,10 +275,13 @@ greyscan(char *dbname)
 int
 greyupdate(char *dbname, char *ip, char *from, char *to)
 {
-	char *key = NULL;
-	struct gdata gd;
-	time_t now;
-	int r;
+	BTREEINFO	btreeinfo;
+	DBT		dbk, dbd;
+	DB		*db;
+	char		*key = NULL;
+	struct gdata	gd;
+	time_t		now;
+	int		r;
 
 	now = time(NULL);
 
@@ -294,8 +304,8 @@ greyupdate(char *dbname, char *ip, char *from, char *to)
 		memset(&gd, 0, sizeof(gd));
 		gd.first = now;
 		gd.bcount = 1;
-		gd.pass = now + GREYEXP;
-		gd.expire = now + GREYEXP;
+		gd.pass = now + greyexp;
+		gd.expire = now + greyexp;
 		memset(&dbk, 0, sizeof(dbk));
 		dbk.size = strlen(key);
 		dbk.data = key;
@@ -316,10 +326,8 @@ greyupdate(char *dbname, char *ip, char *from, char *to)
 		}
 		memcpy(&gd, dbd.data, sizeof(gd));
 		gd.bcount++;
-		if (gd.first + PASSTIME < now) {
+		if (gd.first + passtime < now)
 			gd.pass = now;
-			gd.expire = now + WHITEEXP;
-		}
 		memset(&dbk, 0, sizeof(dbk));
 		dbk.size = strlen(key);
 		dbk.data = key;
@@ -333,11 +341,13 @@ greyupdate(char *dbname, char *ip, char *from, char *to)
 			fprintf(stderr, "updated %s\n", key);
 	}
 	free(key);
+	key = NULL;
 	db->close(db);
 	db = NULL;
 	return(0);
  bad:
 	free(key);
+	key = NULL;
 	db->close(db);
 	db = NULL;
 	return(-1);
@@ -401,9 +411,14 @@ greyreader(void)
 void
 greyscanner(void)
 {
+	int i;
+
 	for (;;) {
 		sleep(DB_SCAN_INTERVAL);
-		greyscan(PATH_SPAMD_DB);
+		i = greyscan(PATH_SPAMD_DB);
+		if (i == -1)
+			syslog_r(LOG_NOTICE, &sdata, "scan of %s failed",
+			    PATH_SPAMD_DB);
 	}
 	/* NOTREACHED */
 }
@@ -444,13 +459,14 @@ greywatcher(void)
 
 	if (!debug) {
 		if (daemon(1, 1) == -1)
-			err(1, "fork");
+			err(1, "daemon");
 	}
 
 	pid = fork();
-	if (pid == -1)
+	switch(pid) {
+	case -1:
 		err(1, "fork");
-	if (pid == 0) {
+	case 0:
 		/*
 		 * child, talks to jailed spamd over greypipe,
 		 * updates db. has no access to pf.
@@ -458,14 +474,16 @@ greywatcher(void)
 		close(pfdev);
 		setproctitle("(%s update)", PATH_SPAMD_DB);
 		greyreader();
-	} else {
-		/*
-		 * parent, scans db periodically for changes and updates
-		 * pf whitelist table accordingly.
-		 */
-		fclose(grey);
-		setproctitle("(pf <spamd-white> update)");
-		greyscanner();
+		/* NOTREACHED */
+		_exit(1);
 	}
-	return(0);
+	/*
+	 * parent, scans db periodically for changes and updates
+	 * pf whitelist table accordingly.
+	 */
+	fclose(grey);
+	setproctitle("(pf <spamd-white> update)");
+	greyscanner();
+	/* NOTREACHED */
+	exit(1);
 }
diff --git a/libexec/spamd/spamd.c b/libexec/spamd/spamd.c
index 2ece4fef9e0..4b8239abce1 100644
--- a/libexec/spamd/spamd.c
+++ b/libexec/spamd/spamd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: spamd.c,v 1.54 2004/02/26 08:18:56 deraadt Exp $	*/
+/*	$OpenBSD: spamd.c,v 1.55 2004/02/28 00:03:59 beck Exp $	*/
 
 /*
  * Copyright (c) 2002 Theo de Raadt.  All rights reserved.
@@ -74,7 +74,6 @@ struct con {
 	int il;
 	char rend[5];	/* any chars in here causes input termination */
 
-	int obufalloc;
 	char *obuf;
 	char *lists;
 	size_t osize;
@@ -148,19 +147,15 @@ grow_obuf(struct con *cp, int off)
 {
 	char *tmp;
 
-	if (!cp->obufalloc)
-		cp->obuf = NULL;
 	tmp = realloc(cp->obuf, cp->osize + 8192);
 	if (tmp == NULL) {
 		free(cp->obuf);
 		cp->obuf = NULL;
 		cp->osize = 0;
-		cp->obufalloc = 0;
 		return (NULL);
 	} else {
 		cp->osize += 8192;
 		cp->obuf = tmp;
-		cp->obufalloc = 1;
 		return (cp->obuf + off);
 	}
 }
@@ -447,11 +442,10 @@ build_reply(struct con *cp)
 
 	matches = cp->blacklists;
 	if (matches == NULL) {
-		if (cp->osize)
-			free(cp->obuf);
+		free(cp->obuf);
 		cp->obuf = NULL;
 		cp->osize = 0;
-		goto bad;
+		goto nomatch;
 	}
 	for (; *matches; matches++) {
 		int used = 0;
@@ -474,12 +468,8 @@ build_reply(struct con *cp)
 			cp->obuf[off] = '\0';
 		}
 	}
-bad:
-	/* Out of memory, or no match. give generic reply */
-	if (cp->obuf != NULL && cp->obufalloc) {
-		free(cp->obuf);
-		cp->obuf = NULL;
-	}
+nomatch:
+	/* No match. give generic reply */
 	if (cp->blacklists != NULL)
 		asprintf(&cp->obuf,
 		    "%s-Sorry %s\n"
@@ -490,22 +480,24 @@ bad:
 	else
 		asprintf(&cp->obuf,
 		    "450 Temporary failure, please try again later.\r\n");
-	if (cp->obuf == NULL) {
-		/* we're having a really bad day.. */
-		cp->obufalloc = 0; /* know not to free or mangle */
-		cp->obuf = "450 Try again\n";
-	} else
+	if (cp->obuf != NULL)
 		cp->osize = strlen(cp->obuf) + 1;
+	else
+		cp->osize = 0;
+	return;
+bad:
+	if (cp->obuf != NULL) {
+		free(cp->obuf);
+		cp->obuf = NULL;
+		cp->osize = 0;
+	}
 }
 
 void
 doreply(struct con *cp)
 {
-	if (reply) {
-		if (!cp->obufalloc)
-			errx(1, "shouldn't happen");
+	if (reply)
 		snprintf(cp->obuf, cp->osize, "%s %s\n", nreply, reply);
-	}
 	build_reply(cp);
 }
 
@@ -538,10 +530,8 @@ initcon(struct con *cp, int fd, struct sockaddr_in *sin)
 	char *tmp;
 
 	time(&t);
-	if (cp->obufalloc) {
-		free(cp->obuf);
-		cp->obuf = NULL;
-	}
+	free(cp->obuf);
+	cp->obuf = NULL;
 	if (cp->blacklists)
 		free(cp->blacklists);
 	bzero(cp, sizeof(struct con));
@@ -560,14 +550,12 @@ initcon(struct con *cp, int fd, struct sockaddr_in *sin)
 	strlcpy(cp->addr, inet_ntoa(sin->sin_addr), sizeof(cp->addr));
 	tmp = strdup(ctime(&t));
 	if (tmp == NULL)
-		tmp = "some time";
-	else
-		tmp[strlen(tmp) - 1] = '\0'; /* nuke newline */
+		err(1, "malloc");
+	tmp[strlen(tmp) - 1] = '\0'; /* nuke newline */
 	snprintf(cp->obuf, cp->osize,
-	    "220 %s ESMTP %s; %s\r\n",
-	    hostname, spamd, tmp);
-	if (tmp != NULL)
-		free(tmp);
+		 "220 %s ESMTP %s; %s\r\n",
+		 hostname, spamd, tmp);
+	free(tmp);
 	cp->op = cp->obuf;
 	cp->ol = strlen(cp->op);
 	cp->w = t + cp->stutter;
@@ -597,7 +585,7 @@ closecon(struct con *cp)
 		free(cp->blacklists);
 		cp->blacklists = NULL;
 	}
-	if (cp->osize > 0 && cp->obufalloc) {
+	if (cp->osize > 0) {
 		free(cp->obuf);
 		cp->obuf = NULL;
 		cp->osize = 0;
@@ -1006,17 +994,10 @@ main(int argc, char *argv[])
 			err(1, "pipe");
 
 		pid = fork();
-		if (pid == -1)
+		switch(pid) {
+		case -1:
 			err(1, "fork");
-		if (pid != 0) {
-			/* parent - run greylister */
-			close(greypipe[1]);
-			grey = fdopen(greypipe[0], "r");
-			if (grey == NULL)
-				err(1, "fdopen");
-			return(greywatcher());
-			/* NOTREACHED */
-		} else {
+		case 0:
 			/* child - continue */
 			close(greypipe[0]);
 			grey = fdopen(greypipe[1], "w");
@@ -1024,9 +1005,18 @@ main(int argc, char *argv[])
 				warn("fdopen");
 				_exit(1);
 			}
+			goto jail;
 		}
+		/* parent - run greylister */
+		close(greypipe[1]);
+		grey = fdopen(greypipe[0], "r");
+		if (grey == NULL)
+			err(1, "fdopen");
+		return(greywatcher());
+		/* NOTREACHED */
 	}
 
+jail:
 	if (chroot("/var/empty") == -1 || chdir("/") == -1) {
 		syslog(LOG_ERR, "cannot chdir to /var/empty.");
 		exit(1);
@@ -1048,7 +1038,7 @@ main(int argc, char *argv[])
 
 	if (debug == 0) {
 		if (daemon(1, 1) == -1)
-			err(1, "fork");
+			err(1, "daemon");
 	} else
 		printf("listening for incoming connections.\n");
 	syslog_r(LOG_WARNING, &sdata, "listening for incoming connections.");
@@ -1068,7 +1058,9 @@ main(int argc, char *argv[])
 
 		if (max > omax) {
 			free(fdsr);
+			fdsr = NULL;
 			free(fdsw);
+			fdsr = NULL;
 			fdsr = (fd_set *)calloc(howmany(max+1, NFDBITS),
 			    sizeof(fd_mask));
 			if (fdsr == NULL)