From b62af95fa98188118939cff3b1d5d942bb20b934 Mon Sep 17 00:00:00 2001 From: Gilles Chehade <gilles@cvs.openbsd.org> Date: Tue, 3 Mar 2009 23:23:53 +0000 Subject: 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. --- usr.sbin/smtpd/forward.c | 35 +----- usr.sbin/smtpd/lka.c | 316 ++++++++++++++++++++++++++++++++++------------- usr.sbin/smtpd/smtpd.c | 62 +++++++++- usr.sbin/smtpd/smtpd.h | 29 ++++- 4 files changed, 323 insertions(+), 119 deletions(-) (limited to 'usr.sbin') 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); @@ -1182,6 +1196,52 @@ lockfail: return -1; } +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) { 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 *); -- cgit v1.2.3