first commit of a serie to cleanup, simplify and improve aliases resolution

which is probably the most complex code in smtpd right now. no longer use a
single list to hold aliases to be resolved and resolved aliases, and do not
use struct alias to hold resolved aliases. instead use a delivery list that
is a list of struct path, and populate it with resolved aliases.

idea discussed with jacekm@, this needs some testing to make sure it does
not introduce a regression with aliases. flush your queue and make clean.

4 files changed, 159 insertions, 126 deletions

diff --git a/usr.sbin/smtpd/forward.c b/usr.sbin/smtpd/forward.c
index 7c020ec1828..94df78af645 100644
--- a/usr.sbin/smtpd/forward.c
+++ b/usr.sbin/smtpd/forward.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: forward.c,v 1.14 2009/08/08 00:02:22 gilles Exp $	*/
+/*	$OpenBSD: forward.c,v 1.15 2009/11/08 19:38:26 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -84,7 +84,6 @@ forwards_get(int fd, struct aliaseslist *aliases)
 			buf = cp;
 			cp = p;
 
-			log_debug("\tforward: %s", buf);
 			if (! alias_parse(&alias, buf)) {
 				log_debug("bad entry in ~/.forward");
 				continue;
diff --git a/usr.sbin/smtpd/lka.c b/usr.sbin/smtpd/lka.c
index a076669296a..29b45fbfaab 100644
--- a/usr.sbin/smtpd/lka.c
+++ b/usr.sbin/smtpd/lka.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: lka.c,v 1.80 2009/11/05 12:26:19 gilles Exp $	*/
+/*	$OpenBSD: lka.c,v 1.81 2009/11/08 19:38:26 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -51,19 +51,21 @@ void		lka_dispatch_runner(int, short, void *);
 void		lka_dispatch_mta(int, short, void *);
 void		lka_setup_events(struct smtpd *);
 void		lka_disable_events(struct smtpd *);
-void		lka_expand_rcpt(struct smtpd *, struct aliaseslist *, struct lkasession *);
+void		lka_expand_pickup(struct smtpd *, struct lkasession *);
+int		lka_expand_resume(struct smtpd *, struct lkasession *);
 int		lka_resolve_alias(struct smtpd *, char *tag, struct path *, struct alias *);
 int		lka_verify_mail(struct smtpd *, struct path *);
 struct rule    *ruleset_match(struct smtpd *, char *, struct path *, struct sockaddr_storage *);
-int		lka_resolve_path(struct smtpd *, struct path *);
+int		lka_resolve_path(struct smtpd *, struct lkasession *, struct path *);
 struct lkasession *lka_session_init(struct smtpd *, struct submit_status *);
 void		lka_request_forwardfile(struct smtpd *, struct lkasession *, char *);
 void		lka_clear_aliaseslist(struct aliaseslist *);
+void		lka_clear_deliverylist(struct deliverylist *);
 int		lka_encode_credentials(char *, size_t, char *);
 size_t		lka_expand(char *, size_t, struct path *);
 void		lka_rcpt_action(struct smtpd *, char *, struct path *);
-int		lka_expand_rcpt_iteration(struct smtpd *, struct aliaseslist *, struct lkasession *);
 void		lka_session_destroy(struct smtpd *, struct lkasession *);
+void		lka_expansion_done(struct smtpd *, struct lkasession *);
 
 void
 lka_sig_handler(int sig, short event, void *p)
@@ -213,7 +215,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;
+			struct path *path;
 
 			IMSG_SIZE_CHECK(fwreq);
 
@@ -224,14 +226,19 @@ lka_dispatch_parent(int sig, short event, void *p)
 			fd = imsg.fd;
 			--lkasession->pending;
 
+			strlcpy(lkasession->path.pw_name, fwreq->pw_name,
+			    sizeof(lkasession->path.pw_name));
+			lkasession->path.flags |= F_PATH_FORWARDED;
+
 			/* 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;
+					lkasession->flags |= F_ERROR;					
 				}
 				close(fd);
-				lka_expand_rcpt(env, &lkasession->aliaseslist, lkasession);
+				lkasession->path.flags |= F_PATH_FORWARDED;
+				lka_expand_pickup(env, lkasession);
 				break;
 			}
 
@@ -239,17 +246,15 @@ lka_dispatch_parent(int sig, short event, void *p)
 			if (! fwreq->status) {
 				lkasession->ss.code = 530;
 				lkasession->flags |= F_ERROR;
-				lka_expand_rcpt(env, &lkasession->aliaseslist, lkasession);
+				lka_expand_pickup(env, 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.tag, &lkasession->message.recipient, &alias);
-			queue_submit_envelope(env, &lkasession->message);
-			lka_expand_rcpt(env, &lkasession->aliaseslist, lkasession);
+			/* no forward file, convert pw_name to a struct path ... */
+			path = path_dup(&lkasession->path);
+			strlcpy(path->pw_name, fwreq->pw_name, sizeof(path->pw_name));
+			TAILQ_INSERT_TAIL(&lkasession->deliverylist, path, entry);			
+			lka_expand_pickup(env, lkasession);
 			break;
 		}
 		default:
@@ -337,59 +342,55 @@ lka_dispatch_mfa(int sig, short event, void *p)
 		case IMSG_LKA_RCPT: {
 			struct submit_status	*ss = imsg.data;
 			struct lkasession	*lkasession;
-			int ret = 0;
+			struct path		*path;
 
 			IMSG_SIZE_CHECK(ss);
 
-			if (IS_RELAY(ss->u.path)) {
-				ss->code = 250;
-				queue_submit_envelope(env, &ss->msg);
-				queue_commit_envelopes(env, &ss->msg);
-				break;
-			}
+			ss->code = 250;
+			path = &ss->u.path;
 
-			if (! lka_resolve_path(env, &ss->u.path)) {
+			lkasession = lka_session_init(env, ss);
+			if (! lka_resolve_path(env, lkasession, path)) {
 				ss->code = 530;
 				imsg_compose_event(iev, IMSG_LKA_RCPT, 0, 0, -1,
 				    ss, sizeof(*ss));
+				lka_session_destroy(env, lkasession);
 				break;
 			}
 
-			ss->code = 250;
-			lkasession = lka_session_init(env, ss);
-			if (lkasession->path.flags & F_PATH_ACCOUNT) {
-				log_debug("lka_dispatch_mfa: path is not expandable");
-				lka_request_forwardfile(env, lkasession, lkasession->path.user);
+			if (path->flags & F_PATH_ACCOUNT) {
+				lka_request_forwardfile(env, lkasession, path->pw_name);
 				break;
 			}
-			else if (lkasession->path.flags & F_PATH_ALIAS) {
-				log_debug("lka_dispatch_mfa: path is aliases-expandable");
-				ret = aliases_get(env,
-				    lkasession->path.rule.r_amap,
-				    &lkasession->aliaseslist,
-				    lkasession->path.user);
-			}
-			else if (lkasession->path.flags & F_PATH_VIRTUAL) {
-				log_debug("lka_dispatch_mfa: path is virtual-expandable");
-				ret = aliases_virtual_get(env, lkasession->path.cond->c_map,
-				    &lkasession->aliaseslist, &lkasession->path);
-			}
-			else
-				fatal("lka_dispatch_mfa: path with illegal flag");
 
-			log_debug("lka_dispatch_mfa: expanded to %d envelopes", ret);
+			if (path->flags & F_PATH_RELAY) {
+				path = path_dup(&ss->u.path);
+				TAILQ_INSERT_TAIL(&lkasession->deliverylist, path, entry);
+			}
 
-			if (ret == 0) {
-				/* No aliases ... */
-				ss->code = 530;
-				imsg_compose_event(iev, IMSG_LKA_RCPT, 0, 0,
-				    -1, ss, sizeof(*ss));
-				lka_session_destroy(env, lkasession);
-				break;
+			if (path->flags & F_PATH_ALIAS) {
+				if (! aliases_get(env, ss->u.path.rule.r_amap,
+					&lkasession->aliaseslist, ss->u.path.user)) {
+					ss->code = 530;
+					imsg_compose_event(iev, IMSG_LKA_RCPT, 0, 0, -1,
+					    ss, sizeof(*ss));
+					lka_session_destroy(env, lkasession);
+					break;
+				}
 			}
 
-			lka_expand_rcpt(env, &lkasession->aliaseslist, lkasession);
+			if (path->flags & F_PATH_VIRTUAL) {
+				if (! aliases_virtual_get(env, ss->u.path.cond->c_map,
+					&lkasession->aliaseslist, &ss->u.path)) {
+					ss->code = 530;
+					imsg_compose_event(iev, IMSG_LKA_RCPT, 0, 0, -1,
+					    ss, sizeof(*ss));
+					lka_session_destroy(env, lkasession);
+					break;
+				}
+			}
 
+			lka_expand_pickup(env, lkasession);
 			break;
 		}
 		default:
@@ -904,17 +905,15 @@ lka_resolve_alias(struct smtpd *env, char *tag, struct path *path, struct alias
 }
 
 void
-lka_expand_rcpt(struct smtpd *env, struct aliaseslist *aliases, struct lkasession *lkasession)
+lka_expand_pickup(struct smtpd *env, struct lkasession *lkasession)
 {
 	int	ret;
-	struct alias	*alias;
-	struct message	message;
 
 	ret = 0;
 	while (! (lkasession->flags & F_ERROR) &&
 	    ! lkasession->pending && lkasession->iterations < 5) {
 		++lkasession->iterations;
-		ret = lka_expand_rcpt_iteration(env, &lkasession->aliaseslist, lkasession);
+		ret = lka_expand_resume(env, lkasession);
 		if (ret == -1) {
 			lkasession->ss.code = 530;
 			lkasession->flags |= F_ERROR;
@@ -927,32 +926,11 @@ lka_expand_rcpt(struct smtpd *env, struct aliaseslist *aliases, struct lkasessio
 	if (lkasession->pending)
 		return;
 
-	if (lkasession->flags & F_ERROR) {
-		lka_clear_aliaseslist(&lkasession->aliaseslist);
-		imsg_compose_event(env->sc_ievs[PROC_MFA], IMSG_LKA_RCPT, 0, 0,
-		    -1, &lkasession->ss, sizeof(struct submit_status));
-	}
-	else if (TAILQ_FIRST(&lkasession->aliaseslist) == NULL) {
-		queue_commit_envelopes(env, &lkasession->message);
-	}
-	else {
-		message = lkasession->message;
-
-		while ((alias = TAILQ_FIRST(&lkasession->aliaseslist)) != NULL) {
-			lka_resolve_alias(env, message.tag, &message.recipient, alias);
-			queue_submit_envelope(env, &message);
-
-			TAILQ_REMOVE(&lkasession->aliaseslist, alias, entry);
-			free(alias);
-		}
-		queue_commit_envelopes(env, &message);
-	}
-
-	lka_session_destroy(env, lkasession);
+	lka_expansion_done(env, lkasession);
 }
 
 int
-lka_expand_rcpt_iteration(struct smtpd *env, struct aliaseslist *aliases, struct lkasession *lkasession)
+lka_expand_resume(struct smtpd *env, struct lkasession *lkasession)
 {
 	u_int8_t done = 1;
 	struct alias *rmalias = NULL;
@@ -962,70 +940,67 @@ lka_expand_rcpt_iteration(struct smtpd *env, struct aliaseslist *aliases, struct
 
 	lkasessionpath = &lkasession->path;
 	rmalias = NULL;
-	TAILQ_FOREACH(alias, aliases, entry) {
+	TAILQ_FOREACH(alias, &lkasession->aliaseslist, entry) {
+		struct path path;
+
 		if (rmalias) {
-			TAILQ_REMOVE(aliases, rmalias, entry);
+			TAILQ_REMOVE(&lkasession->aliaseslist, rmalias, entry);
 			free(rmalias);
 			rmalias = NULL;
 		}
-		
-		if (alias->type == ALIAS_ADDRESS) {
-			respath = &alias->u.path;
-			lka_rcpt_action(env, lkasession->message.tag, respath);
-			lka_resolve_path(env, respath);
-
-			if (alias->u.path.flags & F_PATH_VIRTUAL) {
-				if (aliases_virtual_get(env,
-					respath->cond->c_map,
-					aliases, respath)) {
-					rmalias = alias;
-					done = 0;
-				}
-			}
+		rmalias = alias;
 
-			if (alias->u.path.flags & F_PATH_ALIAS) {
-				if (aliases_get(env, lkasessionpath->rule.r_amap,
-					aliases, alias->u.username)) {
-					done = 0;
-					rmalias = alias;
-				}
-			}
+		if (! lka_resolve_alias(env, lkasession->message.tag, &path, alias))
+			return -1;
+		path.flags = lkasessionpath->flags;
+
+		respath = path_dup(&path);
+		if (! lka_resolve_path(env, lkasession, respath))
+			return -1;
+
+		if (respath->flags & F_PATH_ACCOUNT) {
+			if (respath->flags & F_PATH_FORWARDED)
+				TAILQ_INSERT_TAIL(&lkasession->deliverylist, respath, entry);
+			else
+				lka_request_forwardfile(env, lkasession, respath->pw_name);
 		}
-		
-		else if (alias->type == ALIAS_USERNAME) {
+
+		else if (respath->flags & F_PATH_ALIAS) {
 			if (aliases_get(env, lkasessionpath->rule.r_amap,
-				aliases, alias->u.username)) {
+				&lkasession->aliaseslist, respath->user))
 				done = 0;
-				rmalias = alias;
-			}
-			else {
-				lka_request_forwardfile(env, lkasession,
-				    alias->u.username);
+		}
+
+		else if (respath->flags & F_PATH_VIRTUAL) {
+			if (aliases_virtual_get(env, lkasessionpath->cond->c_map,
+				&lkasession->aliaseslist, respath))
 				done = 0;
-				rmalias = alias;
-			}
 		}
+
 	}
+
 	if (rmalias) {
-		TAILQ_REMOVE(aliases, rmalias, entry);
+		TAILQ_REMOVE(&lkasession->aliaseslist, rmalias, entry);
 		free(rmalias);
-		rmalias = NULL;
 	}
 
-	if (!done && lkasession->iterations == 5)
+	if (!done && lkasession->iterations == 5) {
 		return -1;
+	}
 
-	if (TAILQ_FIRST(aliases) == NULL)
+	if (TAILQ_FIRST(&lkasession->aliaseslist) == NULL)
 		return 0;
 
 	return 1;
 }
 
 int
-lka_resolve_path(struct smtpd *env, struct path *path){
-
-	if (path->cond == NULL)
+lka_resolve_path(struct smtpd *env, struct lkasession *lkasession, struct path *path)
+{
+	if (IS_RELAY(*path)) {
+		path->flags |= F_PATH_RELAY;
 		return 1;
+	}
 
 	switch (path->cond->c_type) {
 	case C_ALL:
@@ -1034,11 +1009,8 @@ lka_resolve_path(struct smtpd *env, struct path *path){
 		char username[MAXLOGNAME];
 		struct passwd *pw;
 
-		/* recipient is to be relayed, no need for more processing */
-		if (IS_RELAY(*path))
-			return 1;
-
 		lowercase(username, path->user, sizeof(username));
+
 		if (aliases_exist(env, path->rule.r_amap, username)) {
 			path->flags |= F_PATH_ALIAS;
 			return 1;
@@ -1116,6 +1088,17 @@ lka_clear_aliaseslist(struct aliaseslist *aliaseslist)
 	}
 }
 
+void
+lka_clear_deliverylist(struct deliverylist *deliverylist)
+{
+	struct path *path;
+
+	while ((path = TAILQ_FIRST(deliverylist)) != NULL) {
+		TAILQ_REMOVE(deliverylist, path, entry);
+		free(path);
+	}
+}
+
 int
 lka_encode_credentials(char *dst, size_t size, char *user)
 {
@@ -1153,6 +1136,7 @@ lka_session_init(struct smtpd *env, struct submit_status *ss)
 	lkasession->ss = *ss;
 	
 	TAILQ_INIT(&lkasession->aliaseslist);
+	TAILQ_INIT(&lkasession->deliverylist);
 	SPLAY_INSERT(lkatree, &env->lka_sessions, lkasession);
 
 	return lkasession;
@@ -1177,4 +1161,36 @@ lka_request_forwardfile(struct smtpd *env, struct lkasession *lkasession, char *
 	++lkasession->pending;
 }
 
+void
+lka_expansion_done(struct smtpd *env, struct lkasession *lkasession)
+{
+	struct message message;
+	struct path *path;
+
+	if (lkasession->flags & F_ERROR) {
+		lka_clear_aliaseslist(&lkasession->aliaseslist);
+		lka_clear_deliverylist(&lkasession->deliverylist);
+		imsg_compose_event(env->sc_ievs[PROC_MFA], IMSG_LKA_RCPT, 0, 0,
+		    -1, &lkasession->ss, sizeof(struct submit_status));
+	}
+	else if (TAILQ_FIRST(&lkasession->deliverylist) == NULL) {
+		queue_commit_envelopes(env, &lkasession->message);
+	}
+	else {
+		message = lkasession->message;
+
+		while ((path = TAILQ_FIRST(&lkasession->deliverylist)) != NULL) {
+			message.recipient = *path;
+			queue_submit_envelope(env, &message);
+
+			TAILQ_REMOVE(&lkasession->deliverylist, path, entry);
+			free(path);
+		}
+		queue_commit_envelopes(env, &message);
+	}
+
+	lka_session_destroy(env, lkasession);
+}
+
+
 SPLAY_GENERATE(lkatree, lkasession, nodes, lkasession_cmp);
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index 6a220a84ad4..6d6d5080aaf 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.h,v 1.153 2009/11/05 10:27:24 gilles Exp $	*/
+/*	$OpenBSD: smtpd.h,v 1.154 2009/11/08 19:38:26 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -346,6 +346,7 @@ enum path_flags {
 	F_PATH_FORWARDED = 0x10,
 	F_PATH_ACCOUNT = 0x20,
 	F_PATH_AUTHENTICATED = 0x40,
+	F_PATH_RELAY = 0x80,
 };
 
 struct path {
@@ -362,6 +363,7 @@ struct path {
 		char filter[MAXPATHLEN];
 	}				 u;
 };
+TAILQ_HEAD(deliverylist, path);
 
 enum alias_type {
 	ALIAS_USERNAME,
@@ -725,6 +727,7 @@ struct lkasession {
 
 	struct path			 path;
 	struct aliaseslist		 aliaseslist;
+	struct deliverylist    		 deliverylist;
 	u_int8_t			 iterations;
 	u_int32_t			 pending;
 	enum lkasession_flags		 flags;
@@ -980,3 +983,4 @@ void		 lowercase(char *, char *, size_t);
 void		 message_set_errormsg(struct message *, char *, ...);
 char		*message_get_errormsg(struct message *);
 void		 sa_set_port(struct sockaddr *, int);
+struct path	*path_dup(struct path *);
diff --git a/usr.sbin/smtpd/util.c b/usr.sbin/smtpd/util.c
index 0aa3fc80369..806e46ad908 100644
--- a/usr.sbin/smtpd/util.c
+++ b/usr.sbin/smtpd/util.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.c,v 1.27 2009/09/15 16:50:06 jacekm Exp $	*/
+/*	$OpenBSD: util.c,v 1.28 2009/11/08 19:38:26 gilles Exp $	*/
 
 /*
  * Copyright (c) 2000,2001 Markus Friedl.  All rights reserved.
@@ -418,3 +418,17 @@ sa_set_port(struct sockaddr *sa, int port)
 	memcpy(sa, res->ai_addr, res->ai_addrlen);
 	freeaddrinfo(res);
 }
+
+struct path *
+path_dup(struct path *path)
+{
+	struct path *pathp;
+
+	pathp = calloc(sizeof(struct path), 1);
+	if (pathp == NULL)
+		fatal("calloc");
+
+	*pathp = *path;
+
+	return pathp;
+}