Fix a long standing issue where ~/.forward files were opened by user _smtpd

causing them not to be handled when a user's homedir is set to mode 0700. I
still need to do some cleanup and make sure it works as it should, but this
diff provides better behavior than what we had.

4 files changed, 323 insertions, 119 deletions

diff --git a/usr.sbin/smtpd/forward.c b/usr.sbin/smtpd/forward.c
index ac2757a95fb..fb5decbdd45 100644
--- a/usr.sbin/smtpd/forward.c
+++ b/usr.sbin/smtpd/forward.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: forward.c,v 1.12 2009/02/22 11:44:29 form Exp $	*/
+/*	$OpenBSD: forward.c,v 1.13 2009/03/03 23:23:52 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -35,43 +35,20 @@
 #include "smtpd.h"
 
 int
-forwards_get(struct aliaseslist *aliases, char *username)
+forwards_get(int fd, struct aliaseslist *aliases)
 {
 	FILE *fp;
 	struct alias alias;
 	struct alias *aliasp;
-	char pathname[MAXPATHLEN];
 	char *buf, *lbuf, *p, *cp;
 	size_t len;
-	struct stat sb;
-	struct passwd *pw;
 	size_t nbaliases = 0;
 	int quoted;
 
-	pw = safe_getpwnam(username);
-	if (pw == NULL)
-		return 0;
-
-	if (! bsnprintf(pathname, sizeof(pathname), "%s/.forward", pw->pw_dir))
-		return 0;
-
-	fp = fopen(pathname, "r");
+	fp = fdopen(fd, "r");
 	if (fp == NULL)
 		return 0;
 
-	log_debug("+ opening forward file %s", pathname);
-	/* make sure ~/ is not writable by anyone but owner */
-	if (stat(pw->pw_dir, &sb) == -1)
-		goto bad;
-	if (sb.st_uid != pw->pw_uid || sb.st_mode & (S_IWGRP|S_IWOTH))
-		goto bad;
-
-	/* make sure ~/.forward is not writable by anyone but owner */
-	if (fstat(fileno(fp), &sb) == -1)
-		goto bad;
-	if (sb.st_uid != pw->pw_uid || sb.st_mode & (S_IWGRP|S_IWOTH))
-		goto bad;
-
 	lbuf = NULL;
 	while ((buf = fgetln(fp, &len))) {
 		if (buf[len - 1] == '\n')
@@ -130,10 +107,4 @@ forwards_get(struct aliaseslist *aliases, char *username)
 	free(lbuf);
 	fclose(fp);
 	return (nbaliases);
-
-bad:
-	log_debug("+ forward file error, probably bad perms/mode");
-	if (fp != NULL)
-		fclose(fp);
-	return (0);
 }
diff --git a/usr.sbin/smtpd/lka.c b/usr.sbin/smtpd/lka.c
index 1bdd443e031..1de64937c12 100644
--- a/usr.sbin/smtpd/lka.c
+++ b/usr.sbin/smtpd/lka.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: lka.c,v 1.28 2009/02/24 21:40:51 jacekm Exp $	*/
+/*	$OpenBSD: lka.c,v 1.29 2009/03/03 23:23:52 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -55,13 +55,12 @@ int		aliases_exist(struct smtpd *, char *);
 int		aliases_get(struct smtpd *, struct aliaseslist *, char *);
 int		lka_resolve_alias(struct path *, struct alias *);
 int		lka_parse_include(char *);
-int		forwards_get(struct aliaseslist *, char *);
 int		lka_check_source(struct smtpd *, struct map *, struct sockaddr_storage *);
 int		lka_match_mask(struct sockaddr_storage *, struct netaddr *);
 int		aliases_virtual_get(struct smtpd *, struct aliaseslist *, struct path *);
 int		aliases_virtual_exist(struct smtpd *, struct path *);
 int		lka_resolve_path(struct smtpd *, struct path *);
-int		lka_expand_aliases(struct smtpd *, struct aliaseslist *, struct path *);
+int		lka_expand_aliases(struct smtpd *, struct aliaseslist *, struct lkasession *);
 void		lka_rcpt_action(struct smtpd *, struct path *);
 
 void
@@ -113,6 +112,101 @@ lka_dispatch_parent(int sig, short event, void *p)
 			break;
 
 		switch (imsg.hdr.type) {
+		case IMSG_PARENT_FORWARD_OPEN: {
+			int fd;
+			int ret;
+			struct forward_req	*fwreq;
+			struct lkasession	key;
+			struct lkasession	*lkasession;
+			struct alias		*alias;
+			struct message		message;
+
+			fwreq = imsg.data;
+
+			key.id = fwreq->id;
+			lkasession = SPLAY_FIND(lkatree, &env->lka_sessions, &key);
+			if (lkasession == NULL)
+				fatal("lka_dispatch_parent: lka session is gone");
+			fd = imsg_get_fd(ibuf, &imsg);
+			--lkasession->pending;
+
+			if (! lkasession->pending && lkasession->flags & F_ERROR) {
+				log_debug("error in lka session");
+				/* we detected an error and this is last imsg */
+				//XXXXX clear aliaseslist and return temp fail
+				break;
+			}
+
+			if (fd == -1) {
+				if (fwreq->pw_name[0] != '\0') {
+					log_debug("error in forward open");
+					/* error id local, return a temporary fail */
+					break;
+				}
+			}
+			else if (! forwards_get(fd, &lkasession->aliaseslist)) {
+				lkasession->flags |= F_ERROR;
+			}
+
+			ret = 0;
+			while (! lkasession->pending && lkasession->iterations < 5) {
+				++lkasession->iterations;
+				ret = lka_expand_aliases(env, &lkasession->aliaseslist, lkasession);
+				if (lkasession->pending) {
+					if (ret == -1)
+						lkasession->flags |= F_ERROR;
+					break;
+				}
+
+				if (ret <= 0)
+					break;
+			}
+
+			if (lkasession->pending)
+				break;
+
+			if (ret < 0) {
+				log_debug("loop detected");
+				while ((alias = TAILQ_FIRST(&lkasession->aliaseslist)) != NULL) {
+					TAILQ_REMOVE(&lkasession->aliaseslist, alias, entry);
+					free(alias);
+				}
+				break;
+			}
+
+			if (ret == 0) {
+				log_debug("expansion resulted in empty list");
+				message = lkasession->message;
+				message.recipient = lkasession->path;
+				imsg_compose(env->sc_ibufs[PROC_QUEUE],
+				    IMSG_QUEUE_SUBMIT_ENVELOPE, 0, 0, -1,
+				    &message, sizeof(struct message));
+				imsg_compose(env->sc_ibufs[PROC_QUEUE],
+				    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1,
+				    &message, sizeof(struct message));
+			}
+			else {
+				log_debug("a list of aliases is available");
+				message = lkasession->message;
+				while ((alias = TAILQ_FIRST(&lkasession->aliaseslist)) != NULL) {
+					bzero(&message.recipient, sizeof(struct path));
+					
+					lka_resolve_alias(&message.recipient, alias);
+					lka_rcpt_action(env, &message.recipient);
+					
+					imsg_compose(env->sc_ibufs[PROC_QUEUE],
+					    IMSG_QUEUE_SUBMIT_ENVELOPE, 0, 0, -1,
+					    &message, sizeof(struct message));
+					
+					TAILQ_REMOVE(&lkasession->aliaseslist, alias, entry);
+					free(alias);
+				}
+				imsg_compose(env->sc_ibufs[PROC_QUEUE],
+				    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1, &message,
+				    sizeof(struct message));
+			}
+			break;
+		}
 		default:
 			log_debug("parent_dispatch_lka: unexpected imsg %d",
 			    imsg.hdr.type);
@@ -178,10 +272,11 @@ lka_dispatch_mfa(int sig, short event, void *p)
 		}
 		case IMSG_LKA_RCPT: {
 			struct submit_status	*ss;
-			struct alias		*alias;
-			struct aliaseslist	 aliases;
 			struct message		 message;
-			int expret;
+			struct lkasession	*lkasession;
+			struct alias		*alias;
+			struct forward_req	 fwreq;
+			int ret;
 
 			ss = imsg.data;
 			ss->code = 530;
@@ -207,18 +302,71 @@ lka_dispatch_mfa(int sig, short event, void *p)
 
 			ss->code = 250;
 
-			TAILQ_INIT(&aliases);
-				
-			expret = lka_expand_aliases(env, &aliases, &ss->u.path);
-			if (expret < 0) {
-				log_debug("loop detected, rejecting recipient");
+			lkasession = calloc(1, sizeof(struct lkasession));
+			if (lkasession == NULL)
+				fatal("lka_dispatch_mfa: calloc");
+			lkasession->id = queue_generate_id();
+			lkasession->path = ss->u.path;
+			lkasession->message = ss->msg;
+			TAILQ_INIT(&lkasession->aliaseslist);
+
+			SPLAY_INSERT(lkatree, &env->lka_sessions, lkasession);
+			log_debug("LKA SESSION !");
+
+			ret = 0;
+			if (lkasession->path.flags & F_ACCOUNT) {
+				fwreq.id = lkasession->id;
+				(void)strlcpy(fwreq.pw_name, ss->u.path.pw_name, sizeof(fwreq.pw_name));
+				imsg_compose(env->sc_ibufs[PROC_PARENT], IMSG_PARENT_FORWARD_OPEN, 0, 0, -1,
+				    &fwreq, sizeof(fwreq));
+				++lkasession->pending;
+				break;
+			}
+			else if (lkasession->path.flags & F_ALIAS) {
+				ret = aliases_get(env, &lkasession->aliaseslist, lkasession->path.user);
+			}
+			else if (lkasession->path.flags & F_VIRTUAL) {
+				ret = aliases_virtual_get(env, &lkasession->aliaseslist, &lkasession->path);
+			}
+			else
+				fatal("lka_dispatch_mfa: path with illegal flag");
+
+			if (ret == 0) {
+				/* No aliases ... */
+				log_debug("expansion resulted in empty list");
 				ss->code = 530;
-				imsg_compose(ibuf, IMSG_LKA_RCPT, 0, 0, -1,
-				    ss, sizeof(*ss));
+				imsg_compose(ibuf, IMSG_LKA_RCPT, 0, 0,
+				    -1, ss, sizeof(*ss));
 				break;
 			}
 
-			if (expret == 0) {
+			ret = 0;
+			while (! lkasession->pending && lkasession->iterations < 5) {
+				++lkasession->iterations;
+				ret = lka_expand_aliases(env, &lkasession->aliaseslist, lkasession);
+				if (lkasession->pending) {
+					if (ret == -1)
+						lkasession->flags |= F_ERROR;
+					break;
+				}
+
+				if (ret <= 0)
+					break;
+			}
+
+			if (lkasession->pending)
+				break;
+
+			if (ret < 0) {
+				log_debug("detected a loop");
+				while ((alias = TAILQ_FIRST(&lkasession->aliaseslist)) != NULL) {
+					TAILQ_REMOVE(&lkasession->aliaseslist, alias, entry);
+					free(alias);
+				}
+				break;
+			}
+
+			if (ret == 0) {
 				log_debug("expansion resulted in empty list");
 				if (! (ss->u.path.flags & F_ACCOUNT)) {
 					ss->code = 530;
@@ -234,27 +382,27 @@ lka_dispatch_mfa(int sig, short event, void *p)
 				imsg_compose(env->sc_ibufs[PROC_QUEUE],
 				    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1,
 				    &message, sizeof(struct message));
-				break;
 			}
-
-			log_debug("a list of aliases is available");
-			message = ss->msg;
-			while ((alias = TAILQ_FIRST(&aliases)) != NULL) {
-				bzero(&message.recipient, sizeof(struct path));
-
-				lka_resolve_alias(&message.recipient, alias);
-				lka_rcpt_action(env, &message.recipient);
-
+			else {
+				log_debug("a list of aliases is available");
+				message = ss->msg;
+				while ((alias = TAILQ_FIRST(&lkasession->aliaseslist)) != NULL) {
+					bzero(&message.recipient, sizeof(struct path));
+					
+					lka_resolve_alias(&message.recipient, alias);
+					lka_rcpt_action(env, &message.recipient);
+					
+					imsg_compose(env->sc_ibufs[PROC_QUEUE],
+					    IMSG_QUEUE_SUBMIT_ENVELOPE, 0, 0, -1,
+					    &message, sizeof(struct message));
+					
+					TAILQ_REMOVE(&lkasession->aliaseslist, alias, entry);
+					free(alias);
+				}
 				imsg_compose(env->sc_ibufs[PROC_QUEUE],
-				    IMSG_QUEUE_SUBMIT_ENVELOPE, 0, 0, -1,
-				    &message, sizeof(struct message));
-
-				TAILQ_REMOVE(&aliases, alias, entry);
-				free(alias);
+				    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1, &message,
+				    sizeof(struct message));
 			}
-			imsg_compose(env->sc_ibufs[PROC_QUEUE],
-			    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1, &message,
-			    sizeof(struct message));
 			break;
 		}
 		default:
@@ -576,6 +724,7 @@ lka(struct smtpd *env)
 #endif
 
 	event_init();
+	SPLAY_INIT(&env->lka_sessions);
 
 	signal_set(&ev_sigint, SIGINT, lka_sig_handler, env);
 	signal_set(&ev_sigterm, SIGTERM, lka_sig_handler, env);
@@ -825,70 +974,52 @@ lka_resolve_alias(struct path *path, struct alias *alias)
 }
 
 int
-lka_expand_aliases(struct smtpd *env, struct aliaseslist *aliases, struct path *path)
+lka_expand_aliases(struct smtpd *env, struct aliaseslist *aliases, struct lkasession *lkasession)
 {
-	u_int8_t done = 0;
-	size_t iterations = 5;
+	u_int8_t done = 1;
 	struct alias *rmalias = NULL;
-	int ret;
 	struct alias *alias;
-	
-	log_debug("RESOLVE ALIASES/.FORWARD FILES");
-	log_debug("path->user: %s", path->user);
-	log_debug("path->domain: %s", path->domain);
-
-	if (path->flags & F_ACCOUNT)
-		ret = forwards_get(aliases, path->pw_name);
-	else if (path->flags & F_ALIAS)
-		ret = aliases_get(env, aliases, path->user);
-	else if (path->flags & F_VIRTUAL)
-		ret = aliases_virtual_get(env, aliases, path);
-	else
-		fatalx("lka_expand_aliases: invalid path type");
-
-	if (! ret)
-		return 0;
-
-	while (!done && iterations--) {
-		done = 1;
-		rmalias = NULL;
-		TAILQ_FOREACH(alias, aliases, entry) {
-			if (rmalias) {
-				TAILQ_REMOVE(aliases, rmalias, entry);
-				free(rmalias);
-				rmalias = NULL;
-			}
+	struct forward_req fwreq;
 
-			if (alias->type == ALIAS_ADDRESS) {
-				if (aliases_virtual_get(env, aliases, &alias->u.path)) {
-					done = 0;
-					rmalias = alias;
-				}
-			}
-			
-			else if (alias->type == ALIAS_USERNAME) {
-				if (aliases_get(env, aliases, alias->u.username) ||
-				    forwards_get(aliases, alias->u.username)) {
-					rmalias = alias;
-					done = 0;
-				}
-			}
-		}
+	rmalias = NULL;
+	TAILQ_FOREACH(alias, aliases, entry) {
 		if (rmalias) {
 			TAILQ_REMOVE(aliases, rmalias, entry);
 			free(rmalias);
 			rmalias = NULL;
 		}
+		
+		if (alias->type == ALIAS_ADDRESS) {
+			if (aliases_virtual_get(env, aliases, &alias->u.path)) {
+				rmalias = alias;
+				done = 0;
+			}
+		}
+		
+		else if (alias->type == ALIAS_USERNAME) {
+			if (aliases_get(env, aliases, alias->u.username)) {
+				done = 0;
+				rmalias = alias;
+			}
+			else {
+				done = 0;
+				fwreq.id = lkasession->id;
+				(void)strlcpy(fwreq.pw_name, alias->u.username, sizeof(fwreq.pw_name));
+				imsg_compose(env->sc_ibufs[PROC_PARENT], IMSG_PARENT_FORWARD_OPEN, 0, 0, -1,
+				    &fwreq, sizeof(fwreq));
+				++lkasession->pending;
+				rmalias = alias;
+			}
+		}
+	}
+	if (rmalias) {
+		TAILQ_REMOVE(aliases, rmalias, entry);
+		free(rmalias);
+		rmalias = NULL;
 	}
 
-	/* Loop detected, empty list */
-	if (!done) {
-		while ((alias = TAILQ_FIRST(aliases)) != NULL) {
-			TAILQ_REMOVE(aliases, alias, entry);
-			free(alias);
-		}
+	if (!done && lkasession->iterations == 5)
 		return -1;
-	}
 
 	if (TAILQ_FIRST(aliases) == NULL)
 		return 0;
@@ -968,3 +1099,20 @@ lka_rcpt_action(struct smtpd *env, struct path *path)
 	path->rule.r_action = A_RELAY;
 	return;
 }
+
+int
+lkasession_cmp(struct lkasession *s1, struct lkasession *s2)
+{
+	/*
+	 * do not return u_int64_t's
+	 */
+	if (s1->id < s2->id)
+		return (-1);
+
+	if (s1->id > s2->id)
+		return (1);
+
+	return (0);
+}
+
+SPLAY_GENERATE(lkatree, lkasession, nodes, lkasession_cmp);
diff --git a/usr.sbin/smtpd/smtpd.c b/usr.sbin/smtpd/smtpd.c
index 8b4ab17ed4a..0c8c3281427 100644
--- a/usr.sbin/smtpd/smtpd.c
+++ b/usr.sbin/smtpd/smtpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.c,v 1.39 2009/03/03 15:47:27 gilles Exp $	*/
+/*	$OpenBSD: smtpd.c,v 1.40 2009/03/03 23:23:52 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -65,6 +65,7 @@ int		parent_mailfile_rename(struct batch *, struct path *);
 int		parent_maildir_open(char *, struct passwd *, struct batch *);
 int		parent_maildir_init(struct passwd *, char *);
 int		parent_external_mda(char *, struct passwd *, struct batch *);
+int		parent_forward_open(char *);
 int		check_child(pid_t, const char *);
 int		setup_spool(uid_t, gid_t);
 
@@ -190,6 +191,19 @@ parent_dispatch_lka(int fd, short event, void *p)
 			break;
 
 		switch (imsg.hdr.type) {
+		case IMSG_PARENT_FORWARD_OPEN: {
+			int ret;
+			struct forward_req *fwreq;
+
+			fwreq = imsg.data;
+			ret = parent_forward_open(fwreq->pw_name);
+			if (ret == -1)
+				if (errno == ENOENT)
+					fwreq->pw_name[0] = '\0';
+			log_debug("parent will return fd %d", fd);
+			imsg_compose(ibuf, IMSG_PARENT_FORWARD_OPEN, 0, 0, ret, fwreq, sizeof(*fwreq));
+			break;
+		}
 		default:
 			log_debug("parent_dispatch_lka: unexpected imsg %d",
 			    imsg.hdr.type);
@@ -1183,6 +1197,52 @@ lockfail:
 }
 
 int
+parent_forward_open(char *username)
+{
+	struct passwd *pw;
+	struct stat sb;
+	char pathname[MAXPATHLEN];
+	int fd;
+
+	pw = safe_getpwnam(username);
+	if (pw == NULL)
+		return -1;
+
+	if (! bsnprintf(pathname, sizeof (pathname), "%s/.forward", pw->pw_dir))
+		return -1;
+
+	fd = open(pathname, O_RDONLY);
+	if (fd == -1) {
+		if (errno == ENOENT)
+			goto clear;
+		return -1;
+	}
+
+	/* make sure ~/ is not writable by anyone but owner */
+	if (stat(pw->pw_dir, &sb) == -1)
+		goto clearlog;
+
+	if (sb.st_uid != pw->pw_uid || sb.st_mode & (S_IWGRP|S_IWOTH))
+		goto clearlog;
+
+	/* make sure ~/.forward is not writable by anyone but owner */
+	if (fstat(fd, &sb) == -1)
+		goto clearlog;
+
+	if (sb.st_uid != pw->pw_uid || sb.st_mode & (S_IWGRP|S_IWOTH))
+		goto clearlog;
+
+	return fd;
+
+clearlog:
+	log_info("cannot process forward file for user %s due to wrong permissions", username);
+
+clear:
+	username[0] = '\0';
+	return -1;
+}
+
+int
 mdaproc_cmp(struct mdaproc *s1, struct mdaproc *s2)
 {
 	if (s1->pid < s2->pid)
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index 7467dc1e0fa..b0b4dd425c2 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.h,v 1.80 2009/03/03 15:47:27 gilles Exp $	*/
+/*	$OpenBSD: smtpd.h,v 1.81 2009/03/03 23:23:52 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -197,6 +197,7 @@ enum imsg_type {
 	IMSG_BATCH_APPEND,
 	IMSG_BATCH_CLOSE,
 
+	IMSG_PARENT_FORWARD_OPEN,
 	IMSG_PARENT_MAILBOX_OPEN,
 	IMSG_PARENT_MESSAGE_OPEN,
 	IMSG_PARENT_MAILBOX_RENAME,
@@ -619,6 +620,27 @@ struct session {
 
 };
 
+struct forward_req {
+	u_int64_t			 id;
+	char				 pw_name[MAXLOGNAME];
+};
+
+enum lkasession_flags {
+	F_ERROR		= 0x1
+};
+
+struct lkasession {
+	SPLAY_ENTRY(lkasession)		 nodes;
+	u_int64_t			 id;
+
+	struct path			 path;
+	struct aliaseslist		 aliaseslist;
+	u_int8_t			 iterations;
+	u_int32_t			 pending;
+	enum lkasession_flags		 flags;
+	struct message			 message;
+};
+
 struct smtpd {
 #define SMTPD_OPT_VERBOSE			 0x00000001
 #define SMTPD_OPT_NOACTION			 0x00000002
@@ -648,6 +670,7 @@ struct smtpd {
 
 	SPLAY_HEAD(batchtree, batch)		batch_queue;
 	SPLAY_HEAD(mdaproctree, mdaproc)	mdaproc_queue;
+	SPLAY_HEAD(lkatree, lkasession)		lka_sessions;
 };
 
 struct s_parent {
@@ -753,7 +776,7 @@ int		 getmxbyname(char *, char ***);
 
 
 /* forward.c */
-int forwards_get(struct aliaseslist *, char *);
+int forwards_get(int, struct aliaseslist *);
 
 
 /* imsg.c */
@@ -779,6 +802,8 @@ void	 imsg_clear(struct imsgbuf *);
 
 /* lka.c */
 pid_t		 lka(struct smtpd *);
+int		 lkasession_cmp(struct lkasession *, struct lkasession *);
+SPLAY_PROTOTYPE(lkatree, lkasession, nodes, lkasession_cmp);
 
 /* mfa.c */
 pid_t		 mfa(struct smtpd *);