From: gilles Date: Tue, 4 Sep 2018 13:04:42 +0000 (+0000) Subject: upon mda failure, smtpd would assume tempfail and retry. this is at odds X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=eb268927b2706e837d7a2e2831fb5d2a5f3b0ea2;p=openbsd upon mda failure, smtpd would assume tempfail and retry. this is at odds with the other MTA which assume a permfail unless the exit status is one of a specific set. make smtpd honour the same exit statuses as postfix. note that all errors that occur before the user mda is executed (fork, pipe and related) are still considered tempfail, only errors coming from the mda itself are handled as permfail. this commit is a temporary solution as i believe the SIGCHLD handler is way more complex than it should be and we'll simplify it after 6.4 is out. ok eric@ --- diff --git a/usr.sbin/smtpd/mda.c b/usr.sbin/smtpd/mda.c index 091d8506d1a..71fa3ac9695 100644 --- a/usr.sbin/smtpd/mda.c +++ b/usr.sbin/smtpd/mda.c @@ -1,4 +1,4 @@ -/* $OpenBSD: mda.c,v 1.133 2018/07/08 13:06:37 gilles Exp $ */ +/* $OpenBSD: mda.c,v 1.134 2018/09/04 13:04:42 gilles Exp $ */ /* * Copyright (c) 2008 Gilles Chehade @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -102,6 +103,7 @@ static const char *mda_user_to_text(const struct mda_user *); static struct mda_envelope *mda_envelope(const struct envelope *); static void mda_envelope_free(struct mda_envelope *); static struct mda_session * mda_session(struct mda_user *); +static const char *mda_sysexit_to_str(int); static struct tree sessions; static struct tree users; @@ -118,12 +120,14 @@ mda_imsg(struct mproc *p, struct imsg *imsg) struct deliver deliver; struct msg m; const void *data; - const char *error, *parent_error; + const char *error, *parent_error, *syserror; uint64_t reqid; size_t sz; char out[256], buf[LINE_MAX]; int n; enum lka_resp_status status; + enum mda_resp_status mda_status; + int mda_sysexit; switch (imsg->hdr.type) { case IMSG_MDA_LOOKUP_USERINFO: @@ -311,6 +315,8 @@ mda_imsg(struct mproc *p, struct imsg *imsg) case IMSG_MDA_DONE: m_msg(&m, imsg); m_get_id(&m, &reqid); + m_get_int(&m, (int *)&mda_status); + m_get_int(&m, (int *)&mda_sysexit); m_get_string(&m, &parent_error); m_end(&m); @@ -322,29 +328,48 @@ mda_imsg(struct mproc *p, struct imsg *imsg) out[0] = '\0'; if (imsg->fd != -1) mda_getlastline(imsg->fd, out, sizeof(out)); + /* * Choose between parent's description of error and * child's output, the latter having preference over * the former. */ error = NULL; - if (strcmp(parent_error, "exited okay") == 0) { - if (s->datafp || (s->io && io_queued(s->io))) + if (mda_status == MDA_OK) { + if (s->datafp || (s->io && io_queued(s->io))) { error = "mda exited prematurely"; + mda_status = MDA_TEMPFAIL; + } } else error = out[0] ? out : parent_error; + syserror = NULL; + if (mda_sysexit) { + syserror = mda_sysexit_to_str(mda_sysexit); + if (syserror) + error = syserror; + } + /* update queue entry */ - if (error) { + switch (mda_status) { + case MDA_TEMPFAIL: mda_queue_tempfail(e->id, error, ESC_OTHER_MAIL_SYSTEM_STATUS); (void)snprintf(buf, sizeof buf, "Error (%s)", error); mda_log(e, "TempFail", buf); - } - else { + break; + case MDA_PERMFAIL: + mda_queue_permfail(e->id, error, + ESC_OTHER_MAIL_SYSTEM_STATUS); + (void)snprintf(buf, sizeof buf, + "Error (%s)", error); + mda_log(e, "PermFail", buf); + break; + case MDA_OK: mda_queue_ok(e->id); mda_log(e, "Ok", "Delivered"); + break; } mda_done(s); return; @@ -836,3 +861,44 @@ mda_session(struct mda_user * u) return (s); } + +static const char * +mda_sysexit_to_str(int sysexit) +{ + switch (sysexit) { + case EX_USAGE: + return "command line usage error"; + case EX_DATAERR: + return "data format error"; + case EX_NOINPUT: + return "cannot open input"; + case EX_NOUSER: + return "user unknown"; + case EX_NOHOST: + return "host name unknown"; + case EX_UNAVAILABLE: + return "service unavailable"; + case EX_SOFTWARE: + return "internal software error"; + case EX_OSERR: + return "system resource problem"; + case EX_OSFILE: + return "critical OS file missing"; + case EX_CANTCREAT: + return "can't create user output file"; + case EX_IOERR: + return "input/output error"; + case EX_TEMPFAIL: + return "temporary failure"; + case EX_PROTOCOL: + return "remote error in protocol"; + case EX_NOPERM: + return "permission denied"; + case EX_CONFIG: + return "local configuration error"; + default: + break; + } + return NULL; +} + diff --git a/usr.sbin/smtpd/smtpd.c b/usr.sbin/smtpd/smtpd.c index 77fee038857..bc94e8ee55c 100644 --- a/usr.sbin/smtpd/smtpd.c +++ b/usr.sbin/smtpd/smtpd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: smtpd.c,v 1.302 2018/07/25 16:00:48 eric Exp $ */ +/* $OpenBSD: smtpd.c,v 1.303 2018/09/04 13:04:42 gilles Exp $ */ /* * Copyright (c) 2008 Gilles Chehade @@ -46,6 +46,7 @@ #include #include #include +#include #include #include @@ -336,7 +337,9 @@ parent_sig_handler(int sig, short event, void *p) case SIGCHLD: do { int len; - + enum mda_resp_status mda_status; + int mda_sysexit; + pid = waitpid(-1, &status, WNOHANG); if (pid <= 0) continue; @@ -346,13 +349,24 @@ parent_sig_handler(int sig, short event, void *p) fail = 1; len = asprintf(&cause, "terminated; signal %d", WTERMSIG(status)); + mda_status = MDA_TEMPFAIL; + mda_sysexit = 0; } else if (WIFEXITED(status)) { if (WEXITSTATUS(status) != 0) { fail = 1; len = asprintf(&cause, "exited abnormally"); - } else + mda_sysexit = WEXITSTATUS(status); + if (mda_sysexit == EX_OSERR || + mda_sysexit == EX_TEMPFAIL) + mda_status = MDA_TEMPFAIL; + else + mda_status = MDA_PERMFAIL; + } else { len = asprintf(&cause, "exited okay"); + mda_status = MDA_OK; + mda_sysexit = 0; + } } else /* WIFSTOPPED or WIFCONTINUED */ continue; @@ -395,12 +409,15 @@ parent_sig_handler(int sig, short event, void *p) log_debug("debug: smtpd: mda process done " "for session %016"PRIx64 ": %s", child->mda_id, cause); + m_create(p_pony, IMSG_MDA_DONE, 0, 0, child->mda_out); m_add_id(p_pony, child->mda_id); + m_add_int(p_pony, mda_status); + m_add_int(p_pony, mda_sysexit); m_add_string(p_pony, cause); m_close(p_pony); - /* free(cause); */ + break; case CHILD_ENQUEUE_OFFLINE: @@ -1238,6 +1255,8 @@ forkmda(struct mproc *p, uint64_t id, struct deliver *deliver) dsp->u.local.user); m_create(p_pony, IMSG_MDA_DONE, 0, 0, -1); m_add_id(p_pony, id); + m_add_int(p_pony, MDA_PERMFAIL); + m_add_int(p_pony, EX_NOUSER); m_add_string(p_pony, ebuf); m_close(p_pony); return; @@ -1266,6 +1285,8 @@ forkmda(struct mproc *p, uint64_t id, struct deliver *deliver) deliver->userinfo.username); m_create(p_pony, IMSG_MDA_DONE, 0, 0, -1); m_add_id(p_pony, id); + m_add_int(p_pony, MDA_PERMFAIL); + m_add_int(p_pony, EX_NOPERM); m_add_string(p_pony, ebuf); m_close(p_pony); return; @@ -1275,6 +1296,8 @@ forkmda(struct mproc *p, uint64_t id, struct deliver *deliver) (void)snprintf(ebuf, sizeof ebuf, "pipe: %s", strerror(errno)); m_create(p_pony, IMSG_MDA_DONE, 0, 0, -1); m_add_id(p_pony, id); + m_add_int(p_pony, MDA_TEMPFAIL); + m_add_int(p_pony, EX_OSERR); m_add_string(p_pony, ebuf); m_close(p_pony); return; @@ -1287,6 +1310,8 @@ forkmda(struct mproc *p, uint64_t id, struct deliver *deliver) (void)snprintf(ebuf, sizeof ebuf, "mkstemp: %s", strerror(errno)); m_create(p_pony, IMSG_MDA_DONE, 0, 0, -1); m_add_id(p_pony, id); + m_add_int(p_pony, MDA_TEMPFAIL); + m_add_int(p_pony, EX_OSERR); m_add_string(p_pony, ebuf); m_close(p_pony); close(pipefd[0]); @@ -1300,6 +1325,8 @@ forkmda(struct mproc *p, uint64_t id, struct deliver *deliver) (void)snprintf(ebuf, sizeof ebuf, "fork: %s", strerror(errno)); m_create(p_pony, IMSG_MDA_DONE, 0, 0, -1); m_add_id(p_pony, id); + m_add_int(p_pony, MDA_TEMPFAIL); + m_add_int(p_pony, EX_OSERR); m_add_string(p_pony, ebuf); m_close(p_pony); close(pipefd[0]); diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index 816227ceed4..bd3de8cb494 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: smtpd.h,v 1.557 2018/08/31 07:28:27 eric Exp $ */ +/* $OpenBSD: smtpd.h,v 1.558 2018/09/04 13:04:42 gilles Exp $ */ /* * Copyright (c) 2008 Gilles Chehade @@ -981,6 +981,12 @@ enum ca_resp_status { CA_FAIL }; +enum mda_resp_status { + MDA_OK, + MDA_TEMPFAIL, + MDA_PERMFAIL +}; + struct ca_cert_req_msg { uint64_t reqid; char name[HOST_NAME_MAX+1];