From 0d7903fa953304a8193884b221242d4de81f8fdf Mon Sep 17 00:00:00 2001 From: Gilles Chehade Date: Sun, 8 Mar 2009 17:54:21 +0000 Subject: ~/.forward files handling was fixed recently so that it is the privileged process that does the opening, this commit does some cleanup, and fixes a bug I experienced today which was caused by a use-after-free. I did some testing to make sure a user cannot cause smtpd to deadlock, or loop, with broken setups (self-referencing forwards/aliases, empty files, broken files...), but if you are playing with aliases/forwards PLEASE let me know of any bug you run into. --- usr.sbin/smtpd/lka.c | 200 +++++++++++++++++++------------------------------ usr.sbin/smtpd/mfa.c | 8 +- usr.sbin/smtpd/smtpd.c | 3 +- usr.sbin/smtpd/smtpd.h | 45 ++++++----- 4 files changed, 104 insertions(+), 152 deletions(-) diff --git a/usr.sbin/smtpd/lka.c b/usr.sbin/smtpd/lka.c index 5e04bf671d6..521e33d446f 100644 --- a/usr.sbin/smtpd/lka.c +++ b/usr.sbin/smtpd/lka.c @@ -1,4 +1,4 @@ -/* $OpenBSD: lka.c,v 1.31 2009/03/04 00:00:40 gilles Exp $ */ +/* $OpenBSD: lka.c,v 1.32 2009/03/08 17:54:20 gilles Exp $ */ /* * Copyright (c) 2008 Pierre-Yves Ritschard @@ -60,7 +60,8 @@ 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 lkasession *); +void lka_expand_rcpt(struct smtpd *, struct aliaseslist *, struct lkasession *); +int lka_expand_rcpt_iteration(struct smtpd *, struct aliaseslist *, struct lkasession *); void lka_rcpt_action(struct smtpd *, struct path *); void lka_clear_aliaseslist(struct aliaseslist *); @@ -115,12 +116,9 @@ lka_dispatch_parent(int sig, short event, void *p) 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; @@ -133,73 +131,18 @@ lka_dispatch_parent(int sig, short event, void *p) if (fd == -1) { if (fwreq->pw_name[0] != '\0') { - lkasession->ss->code = 530; + lkasession->ss.code = 530; lkasession->flags |= F_ERROR; } } else { if (! forwards_get(fd, &lkasession->aliaseslist)) { - lkasession->ss->code = 530; + lkasession->ss.code = 530; lkasession->flags |= F_ERROR; } close(fd); } - - ret = 0; - while (! (lkasession->flags & F_ERROR) && - ! lkasession->pending && lkasession->iterations < 5) { - ++lkasession->iterations; - ret = lka_expand_aliases(env, &lkasession->aliaseslist, lkasession); - if (ret == -1) { - lkasession->ss->code = 530; - lkasession->flags |= F_ERROR; - } - - if (lkasession->pending || ret <= 0) - break; - } - - if (lkasession->pending) - break; - - if (lkasession->flags & F_ERROR) { - lka_clear_aliaseslist(&lkasession->aliaseslist); - imsg_compose(ibuf, IMSG_LKA_RCPT, 0, 0, - -1, lkasession->ss, sizeof(*lkasession->ss)); - } - else 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)); - } - SPLAY_REMOVE(lkatree, &env->lka_sessions, lkasession); - free(lkasession); + lka_expand_rcpt(env, &lkasession->aliaseslist, lkasession); break; } default: @@ -267,9 +210,8 @@ lka_dispatch_mfa(int sig, short event, void *p) } case IMSG_LKA_RCPT: { struct submit_status *ss; - struct message message; + struct message message; struct lkasession *lkasession; - struct alias *alias; struct forward_req fwreq; int ret; @@ -303,11 +245,11 @@ lka_dispatch_mfa(int sig, short event, void *p) lkasession->id = queue_generate_id(); lkasession->path = ss->u.path; lkasession->message = ss->msg; - lkasession->ss = ss; + lkasession->ss = *ss; + TAILQ_INIT(&lkasession->aliaseslist); SPLAY_INSERT(lkatree, &env->lka_sessions, lkasession); - log_debug("LKA SESSION !"); ret = 0; if (lkasession->path.flags & F_ACCOUNT) { @@ -336,60 +278,8 @@ lka_dispatch_mfa(int sig, short event, void *p) break; } - ret = 0; - while (! lkasession->pending && lkasession->iterations < 5) { - ++lkasession->iterations; - ret = lka_expand_aliases(env, &lkasession->aliaseslist, lkasession); - if (ret == -1) { - lkasession->ss->code = 530; - lkasession->flags |= F_ERROR; - } + lka_expand_rcpt(env, &lkasession->aliaseslist, lkasession); - if (lkasession->pending || ret <= 0) - break; - } - - if (lkasession->pending) - break; - - if (lkasession->flags & F_ERROR) { - lka_clear_aliaseslist(&lkasession->aliaseslist); - imsg_compose(ibuf, IMSG_LKA_RCPT, 0, 0, - -1, lkasession->ss, sizeof(*lkasession->ss)); - } - else if (ret == 0) { - log_debug("expansion resulted in empty list"); - message = ss->msg; - message.recipient = ss->u.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 = 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_COMMIT_ENVELOPES, 0, 0, -1, &message, - sizeof(struct message)); - } - SPLAY_REMOVE(lkatree, &env->lka_sessions, lkasession); - free(lkasession); break; } default: @@ -960,8 +850,72 @@ lka_resolve_alias(struct path *path, struct alias *alias) return 1; } +void +lka_expand_rcpt(struct smtpd *env, struct aliaseslist *aliases, 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); + if (ret == -1) { + lkasession->ss.code = 530; + lkasession->flags |= F_ERROR; + } + + if (lkasession->pending || ret <= 0) + break; + } + + if (lkasession->pending) + return; + + if (lkasession->flags & F_ERROR) { + lka_clear_aliaseslist(&lkasession->aliaseslist); + imsg_compose(env->sc_ibufs[PROC_MFA], IMSG_LKA_RCPT, 0, 0, + -1, &lkasession->ss, sizeof(struct submit_status)); + } + else 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)); + } + SPLAY_REMOVE(lkatree, &env->lka_sessions, lkasession); + free(lkasession); +} + int -lka_expand_aliases(struct smtpd *env, struct aliaseslist *aliases, struct lkasession *lkasession) +lka_expand_rcpt_iteration(struct smtpd *env, struct aliaseslist *aliases, struct lkasession *lkasession) { u_int8_t done = 1; struct alias *rmalias = NULL; @@ -1005,8 +959,10 @@ lka_expand_aliases(struct smtpd *env, struct aliaseslist *aliases, struct lkases rmalias = NULL; } - if (!done && lkasession->iterations == 5) + if (!done && lkasession->iterations == 5) { + log_debug("loop detected"); return -1; + } if (TAILQ_FIRST(aliases) == NULL) return 0; diff --git a/usr.sbin/smtpd/mfa.c b/usr.sbin/smtpd/mfa.c index 412bd322762..52187013c6a 100644 --- a/usr.sbin/smtpd/mfa.c +++ b/usr.sbin/smtpd/mfa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: mfa.c,v 1.15 2009/02/22 19:07:33 chl Exp $ */ +/* $OpenBSD: mfa.c,v 1.16 2009/03/08 17:54:20 gilles Exp $ */ /* * Copyright (c) 2008 Gilles Chehade @@ -214,14 +214,12 @@ mfa_dispatch_lka(int sig, short event, void *p) struct submit_status *ss; ss = imsg.data; - if (ss->msg.flags & F_MESSAGE_ENQUEUED) { + if (ss->msg.flags & F_MESSAGE_ENQUEUED) imsg_compose(env->sc_ibufs[PROC_CONTROL], IMSG_MFA_RCPT, 0, 0, -1, ss, sizeof(*ss)); - } - else { + else imsg_compose(env->sc_ibufs[PROC_SMTP], IMSG_MFA_RCPT, 0, 0, -1, ss, sizeof(*ss)); - } break; } default: diff --git a/usr.sbin/smtpd/smtpd.c b/usr.sbin/smtpd/smtpd.c index 98b4bdade42..6d3217e8683 100644 --- a/usr.sbin/smtpd/smtpd.c +++ b/usr.sbin/smtpd/smtpd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: smtpd.c,v 1.41 2009/03/04 00:00:40 gilles Exp $ */ +/* $OpenBSD: smtpd.c,v 1.42 2009/03/08 17:54:20 gilles Exp $ */ /* * Copyright (c) 2008 Gilles Chehade @@ -200,7 +200,6 @@ parent_dispatch_lka(int fd, short event, void *p) if (ret == -1) if (errno == ENOENT) fwreq->pw_name[0] = '\0'; - log_debug("parent will return fd %d", ret); imsg_compose(ibuf, IMSG_PARENT_FORWARD_OPEN, 0, 0, ret, fwreq, sizeof(*fwreq)); break; } diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index 5390f76aa43..23ad369074f 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: smtpd.h,v 1.83 2009/03/06 23:45:00 gilles Exp $ */ +/* $OpenBSD: smtpd.h,v 1.84 2009/03/08 17:54:20 gilles Exp $ */ /* * Copyright (c) 2008 Gilles Chehade @@ -620,28 +620,6 @@ 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 submit_status *ss; -}; - struct smtpd { #define SMTPD_OPT_VERBOSE 0x00000001 #define SMTPD_OPT_NOACTION 0x00000002 @@ -737,6 +715,27 @@ struct message_recipient { struct message msg; }; +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 submit_status ss; +}; /* aliases.c */ int aliases_exist(struct smtpd *, char *); -- cgit v1.2.3