fix mbox locking by having it done by mail.local(8) which in turns uses

lockspool(1). this means our mbox delivery follows a code path that has
become almost identical to external mda deliveries. this is the first
of a serie of diffs actually...

lockspool(1) suggestion by deraadt@, mail.local(8) idea by jacekm@, and
fix and testing by me

3 files changed, 66 insertions, 45 deletions

diff --git a/usr.sbin/smtpd/smtpd.c b/usr.sbin/smtpd/smtpd.c
index a296a4a702c..8b4ab17ed4a 100644
--- a/usr.sbin/smtpd/smtpd.c
+++ b/usr.sbin/smtpd/smtpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.c,v 1.38 2009/03/01 21:58:53 jacekm Exp $	*/
+/*	$OpenBSD: smtpd.c,v 1.39 2009/03/03 15:47:27 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -942,52 +942,69 @@ parent_mailbox_init(struct passwd *pw, char *pathname)
 int
 parent_mailbox_open(char *path, struct passwd *pw, struct batch *batchp)
 {
-	int fd;
-	int mode = O_CREAT|O_APPEND|O_RDWR|O_EXLOCK|O_NONBLOCK|O_NOFOLLOW;
+	pid_t pid;
+	int pipefd[2];
+	struct mdaproc *mdaproc;
+	char sender[MAX_PATH_SIZE];
+
+	/* This can never happen, but better safe than sorry. */
+	if (! bsnprintf(sender, MAX_PATH_SIZE, "%s@%s",
+		batchp->message.sender.user,
+		batchp->message.sender.domain)) {
+		batchp->message.status |= S_MESSAGE_PERMFAILURE;
+		return -1;
+	}
 
 	if (! parent_mailbox_init(pw, path)) {
 		batchp->message.status |= S_MESSAGE_TEMPFAILURE;
 		return -1;
 	}
 
-	fd = open(path, mode, 0600);
-	if (fd == -1) {
-		/* XXX - this needs to be discussed ... */
-		switch (errno) {
-		case ENOTDIR:
-		case ENOENT:
-		case EACCES:
-		case ELOOP:
-		case EROFS:
-		case EDQUOT:
-		case EINTR:
-		case EIO:
-		case EMFILE:
-		case ENFILE:
-		case ENOSPC:
-			goto tempfail;
-		case EWOULDBLOCK:
-			batchp->message.status |= S_MESSAGE_LOCKFAILURE;
-			goto tempfail;
-		default:
-			batchp->message.status |= S_MESSAGE_PERMFAILURE;
-		}
+	log_debug("executing mail.local");
+
+	if (socketpair(AF_UNIX, SOCK_STREAM, 0, pipefd) == -1) {
+		batchp->message.status |= S_MESSAGE_PERMFAILURE;
+		return -1;
+	}
+
+	/* raise privileges because mail.local needs root to
+	 * deliver to user mailboxes.
+	 */
+	if (seteuid(0) == -1)
+		fatal("privraise failed");
+
+	pid = fork();
+	if (pid == -1) {
+		close(pipefd[0]);
+		close(pipefd[1]);
+		batchp->message.status |= S_MESSAGE_PERMFAILURE;
 		return -1;
 	}
 
-	if (! secure_file(fd, path, pw)) {
-		log_warnx("refusing delivery to unsecure path: %s", path);
-		goto tempfail;
+	if (pid == 0) {
+		setproctitle("mail.local");
+
+		close(pipefd[0]);
+		close(STDOUT_FILENO);
+		close(STDERR_FILENO);
+		dup2(pipefd[1], 0);
+
+		execlp(PATH_MAILLOCAL, "mail.local", "-f", sender, pw->pw_name, (void *)NULL);
+		_exit(1);
 	}
 
-	return fd;
+	if (seteuid(pw->pw_uid) == -1)
+		fatal("privdrop failed");
 
-tempfail:
-	if (fd != -1)
-		close(fd);
+	mdaproc = calloc(1, sizeof (struct mdaproc));
+	if (mdaproc == NULL)
+		fatal("calloc");
+	mdaproc->pid = pid;
 
-	batchp->message.status |= S_MESSAGE_TEMPFAILURE;
-	return -1;
+	SPLAY_INSERT(mdaproctree, &batchp->env->mdaproc_queue, mdaproc);
+
+	close(pipefd[1]);
+	return pipefd[0];
 }
 
 int
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index a5614b3e983..7467dc1e0fa 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.h,v 1.79 2009/03/01 21:58:53 jacekm Exp $	*/
+/*	$OpenBSD: smtpd.h,v 1.80 2009/03/03 15:47:27 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -48,6 +48,8 @@
 #define SMTPD_SESSION_TIMEOUT	 300
 #define SMTPD_BACKLOG		 5
 
+#define	PATH_MAILLOCAL		"/usr/libexec/mail.local"
+
 #define	DIRHASH_BUCKETS		 4096
 
 #define PATH_SPOOL		"/var/spool/smtpd"
diff --git a/usr.sbin/smtpd/store.c b/usr.sbin/smtpd/store.c
index ce9b713f572..312e026f6f9 100644
--- a/usr.sbin/smtpd/store.c
+++ b/usr.sbin/smtpd/store.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: store.c,v 1.14 2009/02/23 00:51:32 chl Exp $	*/
+/*	$OpenBSD: store.c,v 1.15 2009/03/03 15:47:28 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -114,16 +114,18 @@ store_write_header(struct batch *batchp, struct message *messagep, FILE *fp,
 	bzero(addrbuf, sizeof (addrbuf));
 	inet_ntop(messagep->session_ss.ss_family, p, addrbuf, sizeof (addrbuf));
 
-	if (batchp->type & T_DAEMON_BATCH) {
-		if (fprintf(fp, "From %s@%s %s\n", "MAILER-DAEMON",
-			batchp->env->sc_hostname, timebuf) == -1) {
-			return 0;
+	if (messagep->recipient.rule.r_action != A_MBOX) {
+		if (batchp->type & T_DAEMON_BATCH) {
+			if (fprintf(fp, "From %s@%s %s\n", "MAILER-DAEMON",
+				batchp->env->sc_hostname, timebuf) == -1) {
+				return 0;
+			}
+		}
+		else {
+			if (fprintf(fp, "From %s@%s %s\n",
+				messagep->sender.user, messagep->sender.domain, timebuf) == -1)
+				return 0;
 		}
-	}
-	else {
-		if (fprintf(fp, "From %s@%s %s\n",
-			messagep->sender.user, messagep->sender.domain, timebuf) == -1)
-			return 0;
 	}
 	
 	if (fprintf(fp, "Received: from %s (%s [%s%s])\n"