[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Nmh-workers] dot locking broken?
From: |
Marcin Cieslak |
Subject: |
[Nmh-workers] dot locking broken? |
Date: |
Tue, 10 Feb 2015 14:18:05 +0000 |
Hi,
I am not a regular nmh user, but I noticed the following:
Out of curiosity I have enabled --with-locking=dot
on my system (I am currently testing nmh and mmh)
and I found out that it probably does not work at
all when it comes to spool locking on my FreeBSD.
First, looks like that MAILGROUP=1 does not
get set in the config.h because we no longer
use "LOCKTYPE" configure variable.
This way "inc" is also not installed setgid mail.
This is fixed by:
diff --git a/configure.ac b/configure.ac
index 02f0a04..158255f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -306,7 +306,7 @@ AS_IF([test x"$DISABLE_SETGID_MAIL" != x -a
x"$DISABLE_SETGID_MAIL" != x0],
dnl If mailspool is not world-writable and dotlockfile is not setgid,
dnl we need to #define MAILGROUP to 1 and make inc setgid.
-if test x"$LOCKTYPE" = x"dot" -a x"$nmh_cv_mailspool_world_writable" = x"no"
-a x"$nmh_cv_dotlockfile_setgid" != x"yes" ; then
+if test x"$with_locking" = x"dot" -a x"$nmh_cv_mailspool_world_writable" =
x"no" -a x"$nmh_cv_dotlockfile_setgid" != x"yes" ; then
dnl do we really need both of these?
AC_DEFINE([MAILGROUP],[1],
[Define to 1 if you need to make `inc' set-group-id because your mail
spool is not world writable. There are no guarantees as to the safety of doing
this, but this #define will add some extra security checks.])dnl
diff --git a/sbr/lock_file.c b/sbr/lock_file.c
index f4d3902..a8a9811 100644
Once this is fixed, inc fails to lock my spool:
(those messages appear in 1 sec intervals as we "wait"
to obtain the lock)
inc: unable to create temporary file in /home/sapern/Mail
inc: unable to create temporary file in /home/sapern/Mail
inc: unable to create temporary file in /home/sapern/Mail
inc: unable to create temporary file in /home/sapern/Mail
inc: unable to lock and fopen /var/mail/sapern
Let's first fix the error message:
--- a/sbr/lock_file.c
+++ b/sbr/lock_file.c
@@ -561,7 +561,7 @@ lockit (struct lockinfo *li)
curlock = li->curlock;
if ((tmpfile = m_mktemp(li->tmplock, &fd, NULL)) == NULL) {
- advise(NULL, "unable to create temporary file in %s", get_temp_dir());
+ advise(NULL, "unable to create temporary file in %s", li->tmplock);
return -1;
}
Now we get those:
inc: unable to create temporary file in /var/mail/,LCK.XXXXXX
inc: unable to create temporary file in /var/mail/,LCK.XXXXXX
inc: unable to create temporary file in /var/mail/,LCK.XXXXXX
inc: unable to create temporary file in /var/mail/,LCK.XXXXXX
inc: unable to lock and fopen /var/mail/sapern
So it looks like something goes wrong with permissions,
my spool is:
drwxrwxr-x 2 root mail 512 Feb 10 14:05 /var/mail
And my inc is:
-rwxr-sr-x 1 root mail 527575 Feb 10 14:12 /usr/local/nmh/bin/inc
I think that using setgid(getgid()) to drop
privileges before we need them is incorrect
- they get dropped permanently.
diff --git a/uip/inc.c b/uip/inc.c
index 8b9fe7a..b3f4dd0 100644
--- a/uip/inc.c
+++ b/uip/inc.c
@@ -129,8 +129,8 @@ static FILE *pf = NULL;
#ifdef MAILGROUP
static int return_gid;
#define TRYDROPGROUPPRIVS() DROPGROUPPRIVS()
-#define DROPGROUPPRIVS() setgid(getgid())
-#define GETGROUPPRIVS() setgid(return_gid)
+#define DROPGROUPPRIVS() setegid(getgid())
+#define GETGROUPPRIVS() setegid(return_gid)
#define SAVEGROUPPRIVS() return_gid = getegid()
#else
/* define *GROUPPRIVS() as null; this avoids having lots of "#ifdef
MAILGROUP"s */
With setegid() now in place I seem to be able to finally
run "inc".
Few remarks:
I think that when m_mktemp() fails we should fail with
adios(), since there is no way things get magically
better by waiting.
Preliminary patch for mmh: http://thread.gmane.org/gmane.mail.mmh/31
I was also wondering if we should give the user
to abort waiting for a lock with ^C.
This is a preliminary patch for mmh: http://thread.gmane.org/gmane.mail.mmh/32
I hope the above makes some sense...
//Marcin
- [Nmh-workers] dot locking broken?,
Marcin Cieslak <=
Re: [Nmh-workers] dot locking broken?, Marcin Cieslak, 2015/02/13