- fix a null deref which could happen after a couple iterations of the

  aliases/virtual domains resolution code.

- fix a logic bug which caused virtual domains not to be correctly
  handled after one iteration of the aliases resolution code.

- introduce a few helper functions to help clean up and simplify the
  lka code.

- simplify the IS_EXT/IS_MAILBOX/IS_RELAY macros so they manipulate a
  struct path * instead of the mess of dereferences we were passing them.

6 files changed, 130 insertions, 119 deletions

diff --git a/usr.sbin/smtpd/aliases.c b/usr.sbin/smtpd/aliases.c
index 99aa40b03af..076d7ef7099 100644
--- a/usr.sbin/smtpd/aliases.c
+++ b/usr.sbin/smtpd/aliases.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: aliases.c,v 1.20 2009/10/11 17:40:49 gilles Exp $	*/
+/*	$OpenBSD: aliases.c,v 1.21 2009/10/12 22:34:37 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -144,6 +144,9 @@ aliases_vdomain_exists(struct smtpd *env, struct map *map, char *hostname)
 	DB     *vtable;
 	char	strkey[MAX_LINE_SIZE];
 
+	if (map == NULL)
+		return 0;
+
 	vtable = dbopen(map->m_config, O_RDONLY, 0600, DB_HASH, NULL);
 	if (vtable == NULL) {
 		log_warn("aliases_vdomain_exists: dbopen");
@@ -177,6 +180,9 @@ aliases_virtual_exist(struct smtpd *env, struct map *map, struct path *path)
 	DB *aliasesdb;
 	char	strkey[MAX_LINE_SIZE];
 
+	if (map == NULL)
+		return 0;
+
 	aliasesdb = dbopen(map->m_config, O_RDONLY, 0600, DB_HASH, NULL);
 	if (aliasesdb == NULL) {
 		log_warn("aliases_virtual_exist: dbopen");
@@ -231,6 +237,9 @@ aliases_virtual_get(struct smtpd *env, struct map *map,
 	struct alias *nextalias;
 	char	strkey[MAX_LINE_SIZE];
 
+	if (map == NULL)
+		return 0;
+
 	aliasesdb = dbopen(map->m_config, O_RDONLY, 0600, DB_HASH, NULL);
 	if (aliasesdb == NULL) {
 		log_warn("aliases_virtual_get: dbopen");
diff --git a/usr.sbin/smtpd/lka.c b/usr.sbin/smtpd/lka.c
index 8377e739e57..7b952ab5134 100644
--- a/usr.sbin/smtpd/lka.c
+++ b/usr.sbin/smtpd/lka.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: lka.c,v 1.66 2009/10/11 17:40:49 gilles Exp $	*/
+/*	$OpenBSD: lka.c,v 1.67 2009/10/12 22:34:37 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -67,7 +67,11 @@ int		lka_expand_rcpt_iteration(struct smtpd *, struct aliaseslist *, struct lkas
 void		lka_rcpt_action(struct smtpd *, struct path *);
 void		lka_clear_aliaseslist(struct aliaseslist *);
 int		lka_encode_credentials(char *, size_t, char *);
+struct lkasession *lka_session_init(struct smtpd *, struct submit_status *);
+void		lka_request_forwardfile(struct smtpd *, struct lkasession *, char *);
 struct rule    *ruleset_match(struct smtpd *, struct path *, struct sockaddr_storage *);
+void		 queue_submit_envelope(struct smtpd *, struct message *);
+void		 queue_commit_envelopes(struct smtpd *, struct message*);
 
 void
 lka_sig_handler(int sig, short event, void *p)
@@ -217,6 +221,7 @@ lka_dispatch_parent(int sig, short event, void *p)
 			struct forward_req	*fwreq = imsg.data;
 			struct lkasession	key;
 			struct lkasession	*lkasession;
+			struct alias		 alias;
 
 			IMSG_SIZE_CHECK(fwreq);
 
@@ -227,55 +232,30 @@ lka_dispatch_parent(int sig, short event, void *p)
 			fd = imsg.fd;
 			--lkasession->pending;
 
-			if (fd == -1) {
-				if (! fwreq->status) {
-					lkasession->ss.code = 530;
-					lkasession->flags |= F_ERROR;
-				}
-				else {
-					struct alias *alias;
-					struct message message;
-
-					alias = calloc(1, sizeof(struct alias));
-					if (alias == NULL)
-						fatal("lka_dispatch_parent: calloc");
-
-					alias_parse(alias, fwreq->pw_name);
-
-					message = lkasession->message;
-
-					bzero(&message.recipient, sizeof(struct path));
-					strlcpy(message.recipient.domain, lkasession->path.domain,
-					    sizeof(message.recipient.domain));
-
-					lka_resolve_alias(env, &message.recipient, alias);
-					lka_rcpt_action(env, &message.recipient);
-
-					if (lka_expand(message.recipient.rule.r_value.path,
-						sizeof(message.recipient.rule.r_value.path),
-						&message.recipient) >=
-					    sizeof(message.recipient.rule.r_value.path)) {
-						log_debug("expansion failed...");
-					}
-
-					imsg_compose_event(env->sc_ievs[PROC_QUEUE],
-					    IMSG_QUEUE_SUBMIT_ENVELOPE, 0, 0, -1,
-					    &message, sizeof(struct message));
-
-					if (! lkasession->pending)
-						imsg_compose_event(env->sc_ievs[PROC_QUEUE],
-						    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1,
-						    &message, sizeof(struct message));
-					break;
-				}
-			}
-			else {
+			/* received a descriptor, we have a forward file ... */
+			if (fd != -1) {
 				if (! forwards_get(fd, &lkasession->aliaseslist)) {
 					lkasession->ss.code = 530;
 					lkasession->flags |= F_ERROR;
 				}
 				close(fd);
+				lka_expand_rcpt(env, &lkasession->aliaseslist, lkasession);
+				break;
+			}
+
+			/* did not receive a descriptor but expected one ... */
+			if (! fwreq->status) {
+				lkasession->ss.code = 530;
+				lkasession->flags |= F_ERROR;
+				lka_expand_rcpt(env, &lkasession->aliaseslist, lkasession);
+				break;
 			}
+
+			/* no forward file, convert pw_name to a struct alias ... */
+			alias_parse(&alias, fwreq->pw_name);
+
+			/* then resolve alias and get back to expanding the aliases list */
+			lka_resolve_alias(env, &lkasession->message.recipient, &alias);
 			lka_expand_rcpt(env, &lkasession->aliaseslist, lkasession);
 			break;
 		}
@@ -363,61 +343,38 @@ lka_dispatch_mfa(int sig, short event, void *p)
 		}
 		case IMSG_LKA_RCPT: {
 			struct submit_status	*ss = imsg.data;
-			struct message		message;
 			struct lkasession	*lkasession;
-			struct forward_req	 fwreq;
-			int ret;
+			int ret = 0;
 
 			IMSG_SIZE_CHECK(ss);
 
-			ss->code = 530;
-			
-			if (IS_RELAY(ss->u.path.rule.r_action)) {
+			if (IS_RELAY(ss->u.path)) {
 				ss->code = 250;
-				message = ss->msg;
-				message.recipient = ss->u.path;
-				imsg_compose_event(env->sc_ievs[PROC_QUEUE],
-				    IMSG_QUEUE_SUBMIT_ENVELOPE, 0, 0, -1,
-				    &message, sizeof(struct message));
-				imsg_compose_event(env->sc_ievs[PROC_QUEUE],
-				    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1,
-				    &message, sizeof(struct message));
+				queue_submit_envelope(env, &ss->msg);
+				queue_commit_envelopes(env, &ss->msg);
 				break;
 			}
 
 			if (! lka_resolve_path(env, &ss->u.path)) {
+				ss->code = 530;
 				imsg_compose_event(iev, IMSG_LKA_RCPT, 0, 0, -1,
 				    ss, sizeof(*ss));
 				break;
 			}
 
 			ss->code = 250;
-
-			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;
-			lkasession->ss = *ss;
-
-			TAILQ_INIT(&lkasession->aliaseslist);
-
-			SPLAY_INSERT(lkatree, &env->lka_sessions, lkasession);
-
-			ret = 0;
+			lkasession = lka_session_init(env, ss);
 			if (lkasession->path.flags & F_PATH_ACCOUNT) {
-				fwreq.id = lkasession->id;
-				(void)strlcpy(fwreq.pw_name, ss->u.path.pw_name, sizeof(fwreq.pw_name));
-				imsg_compose_event(env->sc_ievs[PROC_PARENT], IMSG_PARENT_FORWARD_OPEN, 0, 0, -1,
-				    &fwreq, sizeof(fwreq));
-				++lkasession->pending;
+				log_debug("F_PATH_ACCOUNT");
+				lka_request_forwardfile(env, lkasession, lkasession->path.user);
 				break;
 			}
 			else if (lkasession->path.flags & F_PATH_ALIAS) {
+				log_debug("F_PATH_ALIAS");
 				ret = aliases_get(env, &lkasession->aliaseslist, lkasession->path.user);
 			}
 			else if (lkasession->path.flags & F_PATH_VIRTUAL) {
+				log_debug("F_PATH_VIRTUAL");
 				ret = aliases_virtual_get(env, lkasession->path.cond->c_match,
 				    &lkasession->aliaseslist, &lkasession->path);
 			}
@@ -878,6 +835,8 @@ lka_expand(char *buf, size_t len, struct path *path)
 int
 lka_resolve_alias(struct smtpd *env, struct path *path, struct alias *alias)
 {
+	bzero(path, sizeof(struct path));
+
 	switch (alias->type) {
 	case ALIAS_USERNAME:
 		log_debug("USERNAME: %s", alias->u.username);
@@ -894,6 +853,8 @@ lka_resolve_alias(struct smtpd *env, struct path *path, struct alias *alias)
 				sizeof(path->domain)) >= sizeof(path->domain))
 				return 0;
 		}
+		log_debug("RESOLVED TO %s@%s", path->user, path->domain);
+		lka_rcpt_action(env, path);
 		break;
 
 	case ALIAS_FILENAME:
@@ -913,12 +874,21 @@ lka_resolve_alias(struct smtpd *env, struct path *path, struct alias *alias)
 
 	case ALIAS_ADDRESS:
 		log_debug("ADDRESS: %s@%s", alias->u.path.user, alias->u.path.domain);
+
 		*path = alias->u.path;
+		lka_rcpt_action(env, path);
 		break;
 	case ALIAS_INCLUDE:
 		fatalx("lka_resolve_alias: unexpected type");
 		break;
 	}
+
+	if (lka_expand(path->rule.r_value.path, sizeof(struct path), path) >=
+		sizeof(struct path)) {
+		log_debug("expansion failed...");
+		return 0;
+	}
+
 	return 1;
 }
 
@@ -952,34 +922,20 @@ lka_expand_rcpt(struct smtpd *env, struct aliaseslist *aliases, struct lkasessio
 		    -1, &lkasession->ss, sizeof(struct submit_status));
 	}
 	else if (TAILQ_FIRST(&lkasession->aliaseslist) == NULL) {
-		message = lkasession->message;
-		message.recipient = lkasession->path;
-		imsg_compose_event(env->sc_ievs[PROC_QUEUE],
-		    IMSG_QUEUE_SUBMIT_ENVELOPE, 0, 0, -1,
-		    &message, sizeof(struct message));
-		imsg_compose_event(env->sc_ievs[PROC_QUEUE],
-		    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1,
-		    &message, sizeof(struct message));
+		queue_submit_envelope(env, &lkasession->message);
+		queue_commit_envelopes(env, &lkasession->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(env, &message.recipient, alias);
-			lka_rcpt_action(env, &message.recipient);
-			
-			imsg_compose_event(env->sc_ievs[PROC_QUEUE],
-			    IMSG_QUEUE_SUBMIT_ENVELOPE, 0, 0, -1,
-			    &message, sizeof(struct message));
-			
+			queue_submit_envelope(env, &message);
+
 			TAILQ_REMOVE(&lkasession->aliaseslist, alias, entry);
 			free(alias);
 		}
-		imsg_compose_event(env->sc_ievs[PROC_QUEUE],
-		    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1, &message,
-		    sizeof(struct message));
+		queue_commit_envelopes(env, &message);
 	}
 	SPLAY_REMOVE(lkatree, &env->lka_sessions, lkasession);
 	free(lkasession);
@@ -991,7 +947,6 @@ lka_expand_rcpt_iteration(struct smtpd *env, struct aliaseslist *aliases, struct
 	u_int8_t done = 1;
 	struct alias *rmalias = NULL;
 	struct alias *alias;
-	struct forward_req fwreq;
 	struct path *lkasessionpath;
 
 	lkasessionpath = &lkasession->path;
@@ -1004,7 +959,8 @@ lka_expand_rcpt_iteration(struct smtpd *env, struct aliaseslist *aliases, struct
 		}
 		
 		if (alias->type == ALIAS_ADDRESS) {
-			if (aliases_virtual_get(env, lkasessionpath->cond->c_match, aliases, &alias->u.path)) {
+			lka_rcpt_action(env, &alias->u.path);
+			if (aliases_virtual_get(env, alias->u.path.cond->c_match, aliases, &alias->u.path)) {
 				rmalias = alias;
 				done = 0;
 			}
@@ -1016,12 +972,8 @@ lka_expand_rcpt_iteration(struct smtpd *env, struct aliaseslist *aliases, struct
 				rmalias = alias;
 			}
 			else {
+				lka_request_forwardfile(env, lkasession, alias->u.username);
 				done = 0;
-				fwreq.id = lkasession->id;
-				(void)strlcpy(fwreq.pw_name, alias->u.username, sizeof(fwreq.pw_name));
-				imsg_compose_event(env->sc_ievs[PROC_PARENT], IMSG_PARENT_FORWARD_OPEN, 0, 0, -1,
-				    &fwreq, sizeof(fwreq));
-				++lkasession->pending;
 				rmalias = alias;
 			}
 		}
@@ -1042,8 +994,7 @@ lka_expand_rcpt_iteration(struct smtpd *env, struct aliaseslist *aliases, struct
 }
 
 int
-lka_resolve_path(struct smtpd *env, struct path *path)
-{
+lka_resolve_path(struct smtpd *env, struct path *path){
 	switch (path->cond->c_type) {
 	case C_DOM: {
 		char username[MAXLOGNAME];
@@ -1070,7 +1021,6 @@ lka_resolve_path(struct smtpd *env, struct path *path)
 		return 1;
 	}
 	case C_VDOM: {
-
 		if (aliases_virtual_exist(env, path->cond->c_match, path)) {
 			path->flags |= F_PATH_VIRTUAL;
 			return 1;
@@ -1150,4 +1100,36 @@ lka_encode_credentials(char *dst, size_t size, char *user)
 	return 1;
 }
 
+struct lkasession *
+lka_session_init(struct smtpd *env, struct submit_status *ss)
+{
+	struct lkasession *lkasession;
+
+	lkasession = calloc(1, sizeof(struct lkasession));
+	if (lkasession == NULL)
+		fatal("lka_session_init: calloc");
+
+	lkasession->id = queue_generate_id();
+	lkasession->path = ss->u.path;
+	lkasession->message = ss->msg;
+	lkasession->ss = *ss;
+	
+	TAILQ_INIT(&lkasession->aliaseslist);
+	SPLAY_INSERT(lkatree, &env->lka_sessions, lkasession);
+
+	return lkasession;
+}
+
+void
+lka_request_forwardfile(struct smtpd *env, struct lkasession *lkasession, char *username)
+{
+	struct forward_req	 fwreq;
+
+	fwreq.id = lkasession->id;
+	(void)strlcpy(fwreq.pw_name, username, sizeof(fwreq.pw_name));
+	imsg_compose_event(env->sc_ievs[PROC_PARENT], IMSG_PARENT_FORWARD_OPEN, 0, 0, -1,
+	    &fwreq, sizeof(fwreq));
+	++lkasession->pending;
+}
+
 SPLAY_GENERATE(lkatree, lkasession, nodes, lkasession_cmp);
diff --git a/usr.sbin/smtpd/mfa.c b/usr.sbin/smtpd/mfa.c
index d0708f51b32..40d364114df 100644
--- a/usr.sbin/smtpd/mfa.c
+++ b/usr.sbin/smtpd/mfa.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mfa.c,v 1.40 2009/10/07 18:19:39 gilles Exp $	*/
+/*	$OpenBSD: mfa.c,v 1.41 2009/10/12 22:34:37 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -434,6 +434,7 @@ mfa_test_rcpt(struct smtpd *env, struct message *m)
 	ss.u.path = m->session_rcpt;
 	ss.ss = m->session_ss;
 	ss.msg = *m;
+	ss.msg.recipient = m->session_rcpt;
 	ss.flags = m->flags;
 
 	strip_source_route(ss.u.path.user, sizeof(ss.u.path.user));
diff --git a/usr.sbin/smtpd/queue.c b/usr.sbin/smtpd/queue.c
index 035d552da60..53af5771912 100644
--- a/usr.sbin/smtpd/queue.c
+++ b/usr.sbin/smtpd/queue.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: queue.c,v 1.70 2009/09/15 16:50:06 jacekm Exp $	*/
+/*	$OpenBSD: queue.c,v 1.71 2009/10/12 22:34:37 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -56,6 +56,9 @@ int		queue_remove_layout_envelope(char *, struct message *);
 int		queue_commit_layout_message(char *, struct message *);
 int		queue_open_layout_messagefile(char *, struct message *);
 
+void		queue_submit_envelope(struct smtpd *, struct message *);
+void	        queue_commit_envelopes(struct smtpd *, struct message*);
+
 void
 queue_sig_handler(int sig, short event, void *p)
 {
@@ -410,8 +413,8 @@ queue_dispatch_lka(int sig, short event, void *p)
 			messagep->id = queue_generate_id();
 			ss.id = messagep->session_id;
 
-			if (IS_MAILBOX(messagep->recipient.rule.r_action) ||
-			    IS_EXT(messagep->recipient.rule.r_action))
+			if (IS_MAILBOX(messagep->recipient) ||
+			    IS_EXT(messagep->recipient))
 				messagep->type = T_MDA_MESSAGE;
 			else
 				messagep->type = T_MTA_MESSAGE;
@@ -612,3 +615,19 @@ queue_purge(char *queuepath)
 
 	qwalk_close(q);
 }
+
+void
+queue_submit_envelope(struct smtpd *env, struct message *message)
+{
+	imsg_compose_event(env->sc_ievs[PROC_QUEUE],
+	    IMSG_QUEUE_SUBMIT_ENVELOPE, 0, 0, -1,
+	    message, sizeof(struct message));
+}
+
+void
+queue_commit_envelopes(struct smtpd *env, struct message *message)
+{
+	imsg_compose_event(env->sc_ievs[PROC_QUEUE],
+	    IMSG_QUEUE_COMMIT_ENVELOPES, 0, 0, -1,
+	    message, sizeof(struct message));
+}
diff --git a/usr.sbin/smtpd/runner.c b/usr.sbin/smtpd/runner.c
index 8174c467efe..3d8f72234cf 100644
--- a/usr.sbin/smtpd/runner.c
+++ b/usr.sbin/smtpd/runner.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: runner.c,v 1.67 2009/09/04 19:11:32 jacekm Exp $	*/
+/*	$OpenBSD: runner.c,v 1.68 2009/10/12 22:34:37 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -982,8 +982,7 @@ batch_record(struct smtpd *env, struct message *messagep)
 		    sizeof(batchp->hostname));
 
 		if (batchp->type != T_BOUNCE_BATCH) {
-			if (IS_MAILBOX(path->rule.r_action) ||
-			    IS_EXT(path->rule.r_action)) {
+			if (IS_MAILBOX(*path) || IS_EXT(*path)) {
 				batchp->type = T_MDA_BATCH;
 			}
 			else {
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index e4770bd45e9..d8b3c83f3a3 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.h,v 1.146 2009/10/11 17:40:49 gilles Exp $	*/
+/*	$OpenBSD: smtpd.h,v 1.147 2009/10/12 22:34:37 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -314,9 +314,10 @@ enum action_type {
 	A_FILENAME,
 	A_EXT
 };
-#define IS_MAILBOX(x)	((x) == A_MAILDIR || (x) == A_MBOX || (x) == A_FILENAME)
-#define IS_RELAY(x)	((x) == A_RELAY || (x) == A_RELAYVIA)
-#define IS_EXT(x)	((x) == A_EXT)
+
+#define IS_MAILBOX(x)	((x).rule.r_action == A_MAILDIR || (x).rule.r_action == A_MBOX || (x).rule.r_action == A_FILENAME)
+#define IS_RELAY(x)	((x).rule.r_action == A_RELAY || (x).rule.r_action == A_RELAYVIA)
+#define IS_EXT(x)	((x).rule.r_action == A_EXT)
 
 struct rule {
 	TAILQ_ENTRY(rule)		 r_entry;