EHLO,

Recently I moved my mailserver to a new HD resulting in temporary 
inaccessibility of my maildir (temporarily root owned because of 
recursive copy), leaving smtpd running thinking nothing of it.
This resulted in a few mails being PERMFAIL.

I talked to gilles@ about this and he's somewhat inclined to agree with
me that this is a TEMPFAIL situation. Even if the directory where the
mail is supposed to be delivered is permanently not accessible I'd
argue that we should keep this a TEMPFAIL, because an admin is inclined
to spot this misconfiguration mistake earlier this way (e.g. via mailq
monitoring) and mail won't get lost.

The diff below fixes this for mail.maildir and mail.mboxfile.
For mail.maildir I left the cases that appeared to be obvious
misconfiguration as PERMFAIL, but might be worth to convert these to
EX_TEMPFAIL as well.

Note that mail.lmtp already uses EX_TEMPFAIL in all cases.

While here I also removed the mkdirs from utils.c, which got me
confused at first (it's also part of mail.maildir.c and not used
anywhere else).

thoughts? OK?

martijn@

Index: mail.maildir.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/mail.maildir.c,v
retrieving revision 1.12
diff -u -p -r1.12 mail.maildir.c
--- mail.maildir.c      3 Jul 2019 03:24:03 -0000       1.12
+++ mail.maildir.c      25 Sep 2019 13:21:36 -0000
@@ -31,6 +31,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <time.h>
+#include <sysexits.h>
 #include <unistd.h>
 
 #define        MAILADDR_ESCAPE         "!#$%&'*/?^`{|}~"
@@ -93,8 +94,11 @@ maildir_mkdirs(const char *dirname)
        char    pathname[PATH_MAX];
        char    *subdirs[] = { "cur", "tmp", "new" };
 
-       if (mkdirs(dirname, 0700) == -1 && errno != EEXIST)
-               err(1, NULL);
+       if (mkdirs(dirname, 0700) == -1 && errno != EEXIST) {
+               if (errno == EINVAL || errno == ENAMETOOLONG)
+                       err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
+       }
 
        for (i = 0; i < nitems(subdirs); ++i) {
                ret = snprintf(pathname, sizeof pathname, "%s/%s", dirname,
@@ -102,7 +106,7 @@ maildir_mkdirs(const char *dirname)
                if (ret < 0 || (size_t)ret >= sizeof pathname)
                        errc(1, ENAMETOOLONG, "%s/%s", dirname, subdirs[i]);
                if (mkdir(pathname, 0700) == -1 && errno != EEXIST)
-                       err(1, NULL);
+                       err(EX_TEMPFAIL, NULL);
        }
 }
 
@@ -178,9 +182,9 @@ maildir_engine(const char *dirname, int 
 
        fd = open(tmp, O_CREAT | O_EXCL | O_WRONLY, 0600);
        if (fd == -1)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
        if ((fp = fdopen(fd, "w")) == NULL)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        while ((linelen = getline(&line, &linesize, stdin)) != -1) {
                line[strcspn(line, "\n")] = '\0';
@@ -194,19 +198,19 @@ maildir_engine(const char *dirname, int 
        }
        free(line);
        if (ferror(stdin))
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        if (fflush(fp) == EOF ||
            ferror(fp) ||
            fsync(fd) == -1 ||
            fclose(fp) == EOF)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        (void)snprintf(new, sizeof new, "%s/new/%s",
            is_junk ? junkpath : dirname, filename);
 
        if (rename(tmp, new) == -1)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        exit(0);
 }
@@ -223,8 +227,10 @@ mkdirs_component(const char *path, mode_
                if (mkdir(path, mode | S_IWUSR | S_IXUSR) == -1)
                        return 0;
        }
-       else if (!S_ISDIR(sb.st_mode))
+       else if (!S_ISDIR(sb.st_mode)) {
+               errno = ENOTDIR;
                return 0;
+       }
 
        return 1;
 }
@@ -238,12 +244,16 @@ mkdirs(const char *path, mode_t mode)
        const char      *p;
 
        /* absolute path required */
-       if (*path != '/')
+       if (*path != '/') {
+               errno = EINVAL;
                return 0;
+       }
 
        /* make sure we don't exceed PATH_MAX */
-       if (strlen(path) >= sizeof buf)
+       if (strlen(path) >= sizeof buf) {
+               errno = ENAMETOOLONG;
                return 0;
+       }
 
        memset(buf, 0, sizeof buf);
        for (p = path; *p; p++) {
Index: mail.mboxfile.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/mail.mboxfile.c,v
retrieving revision 1.2
diff -u -p -r1.2 mail.mboxfile.c
--- mail.mboxfile.c     28 Jun 2019 13:32:50 -0000      1.2
+++ mail.mboxfile.c     25 Sep 2019 13:21:36 -0000
@@ -26,6 +26,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sysexits.h>
 #include <unistd.h>
 
 static void    mboxfile_engine(const char *sender, const char *filename);
@@ -73,10 +74,10 @@ mboxfile_engine(const char *sender, cons
 
        fd = open(filename, O_CREAT | O_APPEND | O_WRONLY | O_EXLOCK, 0600);
        if (fd == -1)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        if ((fp = fdopen(fd, "w")) == NULL)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        fprintf(fp, "From %s %s", sender, ctime(&now));
        while ((linelen = getline(&line, &linesize, stdin)) != -1) {
@@ -89,11 +90,11 @@ mboxfile_engine(const char *sender, cons
        fprintf(fp, "\n");
        free(line);
        if (ferror(stdin))
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        if (fflush(fp) == EOF ||
            ferror(fp) ||
            fsync(fd) == -1 ||
            fclose(fp) == EOF)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 }
Index: smtpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
retrieving revision 1.639
diff -u -p -r1.639 smtpd.h
--- smtpd.h     20 Sep 2019 17:46:05 -0000      1.639
+++ smtpd.h     25 Sep 2019 13:21:36 -0000
@@ -1673,7 +1673,6 @@ void addargs(arglist *, char *, ...)
        __attribute__((format(printf, 2, 3)));
 int bsnprintf(char *, size_t, const char *, ...)
        __attribute__((format (printf, 3, 4)));
-int mkdirs(char *, mode_t);
 int safe_fclose(FILE *);
 int hostname_match(const char *, const char *);
 int mailaddr_match(const struct mailaddr *, const struct mailaddr *);
Index: util.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/util.c,v
retrieving revision 1.147
diff -u -p -r1.147 util.c
--- util.c      28 Aug 2019 19:46:20 -0000      1.147
+++ util.c      25 Sep 2019 13:21:36 -0000
@@ -183,65 +183,6 @@ bsnprintf(char *str, size_t size, const 
 }
 
 
-static int
-mkdirs_component(char *path, mode_t mode)
-{
-       struct stat     sb;
-
-       if (stat(path, &sb) == -1) {
-               if (errno != ENOENT)
-                       return 0;
-               if (mkdir(path, mode | S_IWUSR | S_IXUSR) == -1)
-                       return 0;
-       }
-       else if (!S_ISDIR(sb.st_mode))Index: mail.maildir.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/mail.maildir.c,v
retrieving revision 1.12
diff -u -p -r1.12 mail.maildir.c
--- mail.maildir.c      3 Jul 2019 03:24:03 -0000       1.12
+++ mail.maildir.c      25 Sep 2019 13:21:36 -0000
@@ -31,6 +31,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <time.h>
+#include <sysexits.h>
 #include <unistd.h>
 
 #define        MAILADDR_ESCAPE         "!#$%&'*/?^`{|}~"
@@ -93,8 +94,11 @@ maildir_mkdirs(const char *dirname)
        char    pathname[PATH_MAX];
        char    *subdirs[] = { "cur", "tmp", "new" };
 
-       if (mkdirs(dirname, 0700) == -1 && errno != EEXIST)
-               err(1, NULL);
+       if (mkdirs(dirname, 0700) == -1 && errno != EEXIST) {
+               if (errno == EINVAL || errno == ENAMETOOLONG)
+                       err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
+       }
 
        for (i = 0; i < nitems(subdirs); ++i) {
                ret = snprintf(pathname, sizeof pathname, "%s/%s", dirname,
@@ -102,7 +106,7 @@ maildir_mkdirs(const char *dirname)
                if (ret < 0 || (size_t)ret >= sizeof pathname)
                        errc(1, ENAMETOOLONG, "%s/%s", dirname, subdirs[i]);
                if (mkdir(pathname, 0700) == -1 && errno != EEXIST)
-                       err(1, NULL);
+                       err(EX_TEMPFAIL, NULL);
        }
 }
 
@@ -178,9 +182,9 @@ maildir_engine(const char *dirname, int 
 
        fd = open(tmp, O_CREAT | O_EXCL | O_WRONLY, 0600);
        if (fd == -1)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
        if ((fp = fdopen(fd, "w")) == NULL)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        while ((linelen = getline(&line, &linesize, stdin)) != -1) {
                line[strcspn(line, "\n")] = '\0';
@@ -194,19 +198,19 @@ maildir_engine(const char *dirname, int 
        }
        free(line);
        if (ferror(stdin))
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        if (fflush(fp) == EOF ||
            ferror(fp) ||
            fsync(fd) == -1 ||
            fclose(fp) == EOF)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        (void)snprintf(new, sizeof new, "%s/new/%s",
            is_junk ? junkpath : dirname, filename);
 
        if (rename(tmp, new) == -1)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        exit(0);
 }
@@ -223,8 +227,10 @@ mkdirs_component(const char *path, mode_
                if (mkdir(path, mode | S_IWUSR | S_IXUSR) == -1)
                        return 0;
        }
-       else if (!S_ISDIR(sb.st_mode))
+       else if (!S_ISDIR(sb.st_mode)) {
+               errno = ENOTDIR;
                return 0;
+       }
 
        return 1;
 }
@@ -238,12 +244,16 @@ mkdirs(const char *path, mode_t mode)
        const char      *p;
 
        /* absolute path required */
-       if (*path != '/')
+       if (*path != '/') {
+               errno = EINVAL;
                return 0;
+       }
 
        /* make sure we don't exceed PATH_MAX */
-       if (strlen(path) >= sizeof buf)
+       if (strlen(path) >= sizeof buf) {
+               errno = ENAMETOOLONG;
                return 0;
+       }
 
        memset(buf, 0, sizeof buf);
        for (p = path; *p; p++) {
Index: mail.mboxfile.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/mail.mboxfile.c,v
retrieving revision 1.2
diff -u -p -r1.2 mail.mboxfile.c
--- mail.mboxfile.c     28 Jun 2019 13:32:50 -0000      1.2
+++ mail.mboxfile.c     25 Sep 2019 13:21:36 -0000
@@ -26,6 +26,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sysexits.h>
 #include <unistd.h>
 
 static void    mboxfile_engine(const char *sender, const char *filename);
@@ -73,10 +74,10 @@ mboxfile_engine(const char *sender, cons
 
        fd = open(filename, O_CREAT | O_APPEND | O_WRONLY | O_EXLOCK, 0600);
        if (fd == -1)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        if ((fp = fdopen(fd, "w")) == NULL)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        fprintf(fp, "From %s %s", sender, ctime(&now));
        while ((linelen = getline(&line, &linesize, stdin)) != -1) {
@@ -89,11 +90,11 @@ mboxfile_engine(const char *sender, cons
        fprintf(fp, "\n");
        free(line);
        if (ferror(stdin))
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 
        if (fflush(fp) == EOF ||
            ferror(fp) ||
            fsync(fd) == -1 ||
            fclose(fp) == EOF)
-               err(1, NULL);
+               err(EX_TEMPFAIL, NULL);
 }
Index: smtpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
retrieving revision 1.639
diff -u -p -r1.639 smtpd.h
--- smtpd.h     20 Sep 2019 17:46:05 -0000      1.639
+++ smtpd.h     25 Sep 2019 13:21:36 -0000
@@ -1673,7 +1673,6 @@ void addargs(arglist *, char *, ...)
        __attribute__((format(printf, 2, 3)));
 int bsnprintf(char *, size_t, const char *, ...)
        __attribute__((format (printf, 3, 4)));
-int mkdirs(char *, mode_t);
 int safe_fclose(FILE *);
 int hostname_match(const char *, const char *);
 int mailaddr_match(const struct mailaddr *, const struct mailaddr *);
Index: util.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/util.c,v
retrieving revision 1.147
diff -u -p -r1.147 util.c
--- util.c      28 Aug 2019 19:46:20 -0000      1.147
+++ util.c      25 Sep 2019 13:21:36 -0000
@@ -183,65 +183,6 @@ bsnprintf(char *str, size_t size, const 
 }
 
 
-static int
-mkdirs_component(char *path, mode_t mode)
-{
-       struct stat     sb;
-
-       if (stat(path, &sb) == -1) {
-               if (errno != ENOENT)
-                       return 0;
-               if (mkdir(path, mode | S_IWUSR | S_IXUSR) == -1)
-                       return 0;
-       }
-       else if (!S_ISDIR(sb.st_mode))
-               return 0;
-
-       return 1;
-}
-
-int
-mkdirs(char *path, mode_t mode)
-{
-       char     buf[PATH_MAX];
-       int      i = 0;
-       int      done = 0;
-       char    *p;
-
-       /* absolute path required */
-       if (*path != '/')
-               return 0;
-
-       /* make sure we don't exceed PATH_MAX */
-       if (strlen(path) >= sizeof buf)
-               return 0;
-
-       memset(buf, 0, sizeof buf);
-       for (p = path; *p; p++) {
-               if (*p == '/') {
-                       if (buf[0] != '\0')
-                               if (!mkdirs_component(buf, mode))
-                                       return 0;
-                       while (*p == '/')
-                               p++;
-                       buf[i++] = '/';
-                       buf[i++] = *p;
-                       if (*p == '\0' && ++done)
-                               break;
-                       continue;
-               }
-               buf[i++] = *p;
-       }
-       if (!done)
-               if (!mkdirs_component(buf, mode))
-                       return 0;
-
-       if (chmod(path, mode) == -1)
-               return 0;
-
-       return 1;
-}
-
 int
 ckdir(const char *path, mode_t mode, uid_t owner, gid_t group, int create)
 {

-               return 0;
-
-       return 1;
-}
-
-int
-mkdirs(char *path, mode_t mode)
-{
-       char     buf[PATH_MAX];
-       int      i = 0;
-       int      done = 0;
-       char    *p;
-
-       /* absolute path required */
-       if (*path != '/')
-               return 0;
-
-       /* make sure we don't exceed PATH_MAX */
-       if (strlen(path) >= sizeof buf)
-               return 0;
-
-       memset(buf, 0, sizeof buf);
-       for (p = path; *p; p++) {
-               if (*p == '/') {
-                       if (buf[0] != '\0')
-                               if (!mkdirs_component(buf, mode))
-                                       return 0;
-                       while (*p == '/')
-                               p++;
-                       buf[i++] = '/';
-                       buf[i++] = *p;
-                       if (*p == '\0' && ++done)
-                               break;
-                       continue;
-               }
-               buf[i++] = *p;
-       }
-       if (!done)
-               if (!mkdirs_component(buf, mode))
-                       return 0;
-
-       if (chmod(path, mode) == -1)
-               return 0;
-
-       return 1;
-}
-
 int
 ckdir(const char *path, mode_t mode, uid_t owner, gid_t group, int create)
 {

Reply via email to