On Mon, 25 May 2020 16:04:25 -0600, Bob Beck wrote:
> getlock()'s behaviour changes in the case of a writeable mail spool. if we
> want to keep supporting this, I we can modify the pledge as follows:
I thought we decided not to adjust the pledge when I brought it up
last time. Here's the diff I had in my tree to remove support for
world-writable spool dirs.
- todd
Index: libexec/mail.local/locking.c
===================================================================
RCS file: /cvs/src/libexec/mail.local/locking.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 locking.c
--- libexec/mail.local/locking.c 9 Feb 2020 14:59:20 -0000 1.14
+++ libexec/mail.local/locking.c 9 Feb 2020 15:07:22 -0000
@@ -33,7 +33,6 @@
#include <fcntl.h>
#include <pwd.h>
#include <syslog.h>
-#include <time.h>
#include <unistd.h>
#include <limits.h>
#include <errno.h>
@@ -57,9 +56,8 @@ rellock(void)
int
getlock(const char *name, struct passwd *pw)
{
- struct stat sb, fsb;
+ struct stat sb;
int lfd=-1;
- char buf[8*1024];
int tries = 0;
(void)snprintf(lpath, sizeof lpath, "%s/%s.lock",
@@ -67,58 +65,8 @@ getlock(const char *name, struct passwd
if (stat(_PATH_MAILDIR, &sb) != -1 &&
(sb.st_mode & S_IWOTH) == S_IWOTH) {
- /*
- * We have a writeable spool, deal with it as
- * securely as possible.
- */
- time_t ctim = -1;
-
- seteuid(pw->pw_uid);
- if (lstat(lpath, &sb) != -1)
- ctim = sb.st_ctime;
- while (1) {
- /*
- * Deal with existing user.lock files
- * or directories or symbolic links that
- * should not be here.
- */
- if (readlink(lpath, buf, sizeof buf-1) != -1) {
- if (lstat(lpath, &sb) != -1 &&
- S_ISLNK(sb.st_mode)) {
- seteuid(sb.st_uid);
- unlink(lpath);
- seteuid(pw->pw_uid);
- }
- goto again;
- }
- if ((lfd = open(lpath, O_CREAT|O_WRONLY|O_EXCL|O_EXLOCK,
- S_IRUSR|S_IWUSR)) != -1)
- break;
-again:
- if (tries > 10) {
- mwarn("%s: %s", lpath, strerror(errno));
- seteuid(0);
- return(-1);
- }
- if (tries > 9 &&
- (lfd = open(lpath, O_WRONLY|O_EXLOCK, 0)) != -1) {
- if (fstat(lfd, &fsb) != -1 &&
- lstat(lpath, &sb) != -1) {
- if (fsb.st_dev == sb.st_dev &&
- fsb.st_ino == sb.st_ino &&
- ctim == fsb.st_ctime ) {
- seteuid(fsb.st_uid);
- baditem(lpath);
- seteuid(pw->pw_uid);
- }
- }
- close(lfd);
- }
- sleep(1U << tries);
- tries++;
- continue;
- }
- seteuid(0);
+ mwarn("%s: will not deliver to world-writable spool",
+ _PATH_MAILDIR);
} else {
/*
* Only root can write the spool directory.
@@ -136,25 +84,6 @@ again:
}
}
return(lfd);
-}
-
-void
-baditem(char *path)
-{
- char npath[PATH_MAX];
- int fd;
-
- if (unlink(path) == 0)
- return;
- snprintf(npath, sizeof npath, "%s/mailXXXXXXXXXX", _PATH_MAILDIR);
- if ((fd = mkstemp(npath)) == -1)
- return;
- close(fd);
- if (rename(path, npath) == -1)
- unlink(npath);
- else
- mwarn("nasty spool item %s renamed to %s", path, npath);
- /* XXX if we fail to rename, another attempt will happen later */
}
void
Index: libexec/mail.local/mail.local.8
===================================================================
RCS file: /cvs/src/libexec/mail.local/mail.local.8,v
retrieving revision 1.31
diff -u -p -u -r1.31 mail.local.8
--- libexec/mail.local/mail.local.8 16 Sep 2014 21:28:51 -0000 1.31
+++ libexec/mail.local/mail.local.8 9 Feb 2020 15:18:48 -0000
@@ -77,19 +77,18 @@ is prepended to any line in the message
.Dq "From\&\ "
delimiter line.
.Pp
-Significant efforts have been made to ensure that
+Significant effort has been made to ensure that
.Nm
-acts as securely as possible if the spool directory is mode 1777 or 755.
-The default of mode 755 is more secure, but it prevents mail clients from using
-.Pa username.lock
-style locking.
-The use of 1777 is more flexible in an NFS shared-spool environment,
-so many sites use it.
-However, it does carry some risks, such as attackers filling the spool disk.
-Some of these problems may be alleviated
-by making the spool a separate filesystem, and placing quotas on it.
-The use of any mode other than 1777 and 755 for the spool directory is
-recommended against but may work properly.
+acts as securely as possible.
+It will only deliver to a mail spool directory that is not world-writable.
+The default mode of
+.Pa /var/mail
+on
+.Ox
+is 755, which prevents non-root processes from creating mail spool files.
+The MTA is expected to either create the mail spool file itself, or call
+.Nm
+as root.
.Pp
The mailbox is always locked using
.Xr flock 2
