commit 82206056ecfb4003651ef84ca27945fb33f818e8
parent b3b81916fc31048e89e418a5538d1f8812814479
Author: Christoph Lohmann <20h@r-36.net>
Date:   Sun, 14 Jun 2020 13:00:42 +0200
Finanlize errors, make debugging an option.
Diffstat:
| bmf-milter.c | | | 169 | ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------- | 
1 file changed, 121 insertions(+), 48 deletions(-)
diff --git a/bmf-milter.c b/bmf-milter.c
@@ -6,6 +6,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
+#include <fcntl.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -20,8 +21,14 @@
 
 #include "arg.h"
 
-struct Priv
-{
+/*
+ * In case any requires feature is not negotiable, simply do nothing,
+ * thus always return SMFI_CONTINUE. If we would return any FAIL, the
+ * message would be rejected.
+ */
+int donothing = 0, dodebug = 0;
+
+struct Priv {
 	/* read from execpipe[0], write to execpipe[1] */
 	int	execpipe[2];
 	int	execpid;
@@ -30,25 +37,41 @@ struct Priv
 #define MLFIPRIV ((struct Priv *) smfi_getpriv(ctx))
 
 sfsistat
-mlfi_cleanup(SMFICTX *ctx)
+mlfi_cleanup(SMFICTX *ctx, int iseom)
 {
 	struct Priv *priv = MLFIPRIV;
 	int retcode = -1;
 
-	fprintf(stderr, "mlfi_cleanup\n");
+	if (dodebug)
+		fprintf(stderr, "mlfi_cleanup\n");
 
 	if (priv == NULL)
 		return SMFIS_CONTINUE;
-
+	if (dodebug)
+		fprintf(stderr, "cleanup: closing execpipe[1]\n");
 	close(priv->execpipe[1]);
+	priv->execpipe[1] = -1;
+	if (dodebug)
+		fprintf(stderr, "cleanup: waitpid\n");
 	waitpid(priv->execpid, &retcode, 0);
-
-	if (retcode == 0) {
-		if (smfi_addheader(ctx, "X-Spam-Flag", "YES") == MI_FAILURE)
-			return SMFIS_TEMPFAIL;
+	if (dodebug)
+		fprintf(stderr, "cleanup: retcode = %d\n", retcode);
+
+	/*
+	 * smfi_addheader is only allowed in eom.
+	 */
+	if (iseom) {
+		if (retcode == 0) {
+			if (smfi_addheader(ctx, "X-Spam-Flag",
+						" YES") == MI_FAILURE) {
+				if (dodebug)
+					fprintf(stderr, "x-spam-flag failed\n");
+			}
+		}
 		if (smfi_addheader(ctx, "X-BMF-Processed",
-					"YES") == MI_FAILURE) {
-			return SMFIS_TEMPFAIL;
+					" YES") == MI_FAILURE) {
+			if (dodebug)
+				fprintf(stderr, "x-bmf-processed failed\n");
 		}
 	}
 
@@ -56,17 +79,32 @@ mlfi_cleanup(SMFICTX *ctx)
 }
 
 sfsistat
+mlfi_connect(SMFICTX *ctx, char *hostname, _SOCK_ADDR *hostaddr)
+{
+	if (dodebug)
+		fprintf(stderr, "mlfi_connect(%s)\n", hostname);
+
+	return SMFIS_CONTINUE;
+}
+
+sfsistat
 mlfi_helo(SMFICTX *ctx, char *helohost)
 {
 	struct Priv *priv;
 	char *ident;
-	int pid;
+	int pid, nullfd;
 
-	fprintf(stderr, "mlfi_helo(%s)\n", helohost);
+	if (dodebug)
+		fprintf(stderr, "mlfi_helo(%s)\n", helohost);
+
+	if (donothing) {
+		smfi_setpriv(ctx, NULL);
+		return SMFIS_CONTINUE;
+	}
 
 	priv = malloc(sizeof *priv);
 	if (priv == NULL)
-		return SMFIS_TEMPFAIL;
+		return SMFIS_CONTINUE;
 	memset(priv, '\0', sizeof *priv);
 
 	smfi_setpriv(ctx, priv);
@@ -74,14 +112,25 @@ mlfi_helo(SMFICTX *ctx, char *helohost)
 	if (pipe(priv->execpipe) < 0) {
 		free(priv);
 		smfi_setpriv(ctx, NULL);
-		return SMFIS_TEMPFAIL;
+		return SMFIS_CONTINUE;
 	}
 
 	switch((pid = fork())) {
 	case 0:
 		while(dup2(priv->execpipe[0], 0) < 0 && errno == EINTR);
 		close(priv->execpipe[1]);
-		if (execl("/usr/bin/bmf", "bmf", "-t", (char *)NULL) < 0) {
+
+		if (!dodebug) {
+			nullfd = open("/dev/null", O_WRONLY);
+			if (nullfd < 0) {
+				perror("open");
+				_exit(1);
+			}
+			while(dup2(priv->execpipe[0], 1) < 0 && errno == EINTR);
+			while(dup2(priv->execpipe[0], 2) < 0 && errno == EINTR);
+		}
+
+		if (execl("/usr/bin/bmf", "bmf", "-t", "-v", "-v", (char *)NULL) < 0) {
 			perror("execl");
 			_exit(1);
 		}
@@ -89,7 +138,7 @@ mlfi_helo(SMFICTX *ctx, char *helohost)
 	case -1:
 		free(priv);
 		smfi_setpriv(ctx, NULL);
-		return SMFIS_TEMPFAIL;
+		break;
 	default:
 		priv->execpid = pid;
 		close(priv->execpipe[0]);
@@ -104,7 +153,8 @@ mlfi_envfrom(SMFICTX *ctx, char *argv[])
 {
 	struct Priv *priv = MLFIPRIV;
 
-	fprintf(stderr, "mlfi_envfrom(%s)\n", argv[0]);
+	if (dodebug)
+		fprintf(stderr, "mlfi_envfrom(%s)\n", argv[0]);
 
 	dprintf(priv->execpipe[1], "From: %s\n", argv[0]);
 
@@ -116,7 +166,8 @@ mlfi_envrcpt(SMFICTX *ctx, char *argv[])
 {
 	struct Priv *priv = MLFIPRIV;
 
-	fprintf(stderr, "mfli_envrcpt(%s)\n", argv[0]);
+	if (dodebug)
+		fprintf(stderr, "mfli_envrcpt(%s)\n", argv[0]);
 
 	dprintf(priv->execpipe[1], "To: %s\n", argv[0]);
 
@@ -128,7 +179,8 @@ mlfi_header(SMFICTX *ctx, char *headerf, char *headerv)
 {
 	struct Priv *priv = MLFIPRIV;
 
-	fprintf(stderr, "mlfi_header(%s = '%s')\n", headerf, headerv);
+	if (dodebug)
+		fprintf(stderr, "mlfi_header(%s = '%s')\n", headerf, headerv);
 
 	dprintf(priv->execpipe[1], "%s: %s\n", headerf, headerv);
 
@@ -140,7 +192,8 @@ mlfi_eoh(SMFICTX *ctx)
 {
 	struct Priv *priv = MLFIPRIV;
 
-	fprintf(stderr, "mlfi_eoh\n");
+	if (dodebug)
+		fprintf(stderr, "mlfi_eoh\n");
 
 	dprintf(priv->execpipe[1], "\r\n");
 
@@ -153,9 +206,10 @@ mlfi_body(SMFICTX *ctx, unsigned char *bodyp, size_t bodylen)
         struct Priv *priv = MLFIPRIV;
 	int written;
 
-	fprintf(stderr, "mlfi_body(%ld bytes)\n", bodylen);
+	if (dodebug)
+		fprintf(stderr, "mlfi_body(%ld bytes)\n", bodylen);
 
-	for (int written = 0, rw = 0; written <= bodylen; written += rw)
+	for (int written = 0, rw = 0; written < bodylen; written += rw)
 		rw = write(priv->execpipe[1], bodyp+written, bodylen-written);
 
 	return SMFIS_CONTINUE;
@@ -164,15 +218,19 @@ mlfi_body(SMFICTX *ctx, unsigned char *bodyp, size_t bodylen)
 sfsistat
 mlfi_eom(SMFICTX *ctx)
 {
-	fprintf(stderr, "mlfi_eom\n");
-	return mlfi_cleanup(ctx);
+	if (dodebug)
+		fprintf(stderr, "mlfi_eom\n");
+
+	return mlfi_cleanup(ctx, 1);
 }
 
 sfsistat
 mlfi_abort(SMFICTX *ctx)
 {
-	fprintf(stderr, "mlfi_abort\n");
-	return mlfi_cleanup(ctx);
+	if (dodebug)
+		fprintf(stderr, "mlfi_abort\n");
+
+	return mlfi_cleanup(ctx, 0);
 }
 
 sfsistat
@@ -180,13 +238,22 @@ mlfi_close(SMFICTX *ctx)
 {
 	struct Priv *priv = MLFIPRIV;
 
-	fprintf(stderr, "mlfi_close\n");
+	if (dodebug)
+		fprintf(stderr, "mlfi_close\n");
 
 	if (priv != NULL) {
-		if (priv->execpipe[1] != 0)
+		if (priv->execpipe[1] > 0) {
+			if (dodebug)
+				fprintf(stderr, "Close execpipe[1]\n");
 			close(priv->execpipe[1]);
-		if (priv->execpid != 0)
+		}
+		if (priv->execpid > 0) {
+			if (dodebug)
+				fprintf(stderr, "Kill pid %d.\n", priv->execpid);
 			kill(priv->execpid, SIGKILL);
+		}
+		if (dodebug)
+			fprintf(stderr, "Free priv\n");
 		free(priv);
 		smfi_setpriv(ctx, NULL);
 	}
@@ -205,17 +272,24 @@ mlfi_negotiate(SMFICTX *ctx,
 		unsigned long *pf2,
 		unsigned long *pf3)
 {
-	fprintf(stderr, "mlfi_negotiate\n");
+	if (dodebug)
+		fprintf(stderr, "mlfi_negotiate\n");
 
 	/* milter actions */
 	*pf0 = 0;
-	if (f0 & SMFIF_ADDHDRS)
-		f0 |= SMFIF_ADDHDRS;
+	if (f0 & SMFIF_ADDHDRS) {
+		*pf0 |= SMFIF_ADDHDRS;
+	} else {
+		donothing = 1;
+	}
 
 	/* milter protocol steps */
-	*pf1 = f1 & (SMFIP_NOCONNECT|SMFIP_NOUNKNOWN|SMFIP_NODATA);
-	if (f1 & SMFIP_HDR_LEADSPC)
+	*pf1 = f1 & (SMFIP_NOUNKNOWN|SMFIP_NODATA);
+	if (f1 & SMFIP_HDR_LEADSPC) {
 		*pf1 |= SMFIP_HDR_LEADSPC;
+	} else {
+		donothing = 1;
+	}
 
 	/* future */
 	*pf2 = 0;
@@ -229,7 +303,7 @@ struct smfiDesc smfilter =
 	"BMF",		/* filter name */
 	SMFI_VERSION,	/* version code -- do not change */
 	SMFIF_ADDHDRS,  /* flags */
-	NULL,		/* connection info filter */
+	mlfi_connect,	/* connection info filter */
 	mlfi_helo,	/* SMTP HELO command filter */
 	mlfi_envfrom,	/* envelope sender filter */
 	mlfi_envrcpt,	/* envelope recipient filter */
@@ -259,11 +333,12 @@ main(int argc, char *argv[])
 	int timeout;
 
 	port = "inet:9957";
-	timeout = 0;
+	timeout = -1;
 
 	ARGBEGIN(argv0) {
 	case 'd':
 		smfi_setdbg(atoi(EARGF(argv0)));
+		dodebug = 1;
 		break;
 	case 'p':
 		port = EARGF(usage(argv0));
@@ -276,22 +351,20 @@ main(int argc, char *argv[])
 		return 1;
 	} ARGEND;
 
-	fprintf(stderr, "port = %s; timeout = %d;\n", port, timeout);
-
 	if (smfi_setconn(port) == MI_FAILURE) {
 		perror("smfi_setconn");
 		return 1;
 	}
+	if (dodebug)
+		fprintf(stderr, "port set to '%s'\n", port);
 
-	if (!strncasecmp(port, "unix:", 5)) {
-		unlink(port + 5);
-	} else if (!strncasecmp(port, "local:", 6)) {
-		unlink(port + 6);
-	}
-
-	if (smfi_settimeout(timeout) == MI_FAILURE) {
-		perror("smfi_settimout");
-		return 1;
+	if (timeout > 0) {
+		if (smfi_settimeout(timeout) == MI_FAILURE) {
+			perror("smfi_settimout");
+			return 1;
+		}
+		if (dodebug)
+			fprintf(stderr, "timeout set to %d\n", timeout);
 	}
 
 	if (smfi_register(smfilter) == MI_FAILURE) {