September 25, 2019 3:23 PM, "Martijn van Duren"
<[email protected]> wrote:
> EHLO,
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?
I'm generally ok with making mda's less strict when it comes to errors
that can genuinely happen from a misconfiguration. I don't think we'll
convert all errors because by buffering on our side in hope that we're
going to recover somehow, we're also delaying the permanent failure on
the sending host which is not always desirable. A lot of errors can be
made less strict for sure, we need to address them case by case.
The mkdirs from utils.c were left-overs from when we converted maildir
to be a real mda rather than an smtpd builtin, good riddance.
diff reads ok to me
> 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)
> {
