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 091d850..71fa3ac 100644 (file)
--- 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 <gilles@poolp.org>
@@ -35,6 +35,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sysexits.h>
 #include <time.h>
 #include <unistd.h>
 #include <limits.h>
@@ -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 77fee03..bc94e8e 100644 (file)
--- 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 <gilles@poolp.org>
@@ -46,6 +46,7 @@
 #include <limits.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sysexits.h>
 #include <time.h>
 #include <unistd.h>
 
@@ -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 816227c..bd3de8c 100644 (file)
--- 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 <gilles@poolp.org>
@@ -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];