Discussion:
[Mimedefang] Privilege escalation via PID file manipulation
Michael Orlitzky
2017-08-31 14:08:46 UTC
Permalink
== Summary ==

The MIMEDefang daemons should create their PID files before dropping
privileges. This represents a minor security issue; additional factors
are needed to make it exploitable.


== Details ==

The purpose of the PID file is to hold the PID of the running daemon,
so that later it can be stopped, restarted, or otherwise signalled
(many daemons reload their configurations in response to a SIGHUP).
To fulfil that purpose, the contents of the PID file need to be
trustworthy. If the PID file is writable by a non-root user, then he
can replace its contents with the PID of a root process. Afterwards,
any attempt to signal the PID contained in the PID file will instead
signal a root process chosen by the non-root user (a vulnerability).

This is commonly exploitable through init scripts that are run as root
and which blindly trust the contents of their PID files. Examples of
said init scripts can be found in the MIMEDefang source tree:

* examples/init-script.in
* redhat/mimedefang-init.in


== Exploitation ==

An example of a problematic scenario involving an init script would be,

1. I run "/etc/init.d/mimedefang start" to start the daemon.

2. mimedefang drops to the "defang" user.

3. mimedefang writes its PID file, now owned by the "defang" user.

4. Someone compromises the daemon.

5. The attacker is generally limited in what he can do because the
daemon doesn't run as root. However, he can write "1" into the
PID file, and he does.

6. I run "/etc/init.d/mimedefang stop" to stop the daemon while I
investigate the weird behavior resulting from the hack.

7. The machine reboots, because I killed PID 1 (this is normally
restricted to root).


== Resolution ==

The problem is usually avoided by creating the PID file as root, before
dropping privileges.
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list ***@lists.roaringpenguin.com
http://
Dianne Skoll
2017-08-31 15:15:29 UTC
Permalink
Hi,
Post by Michael Orlitzky
The MIMEDefang daemons should create their PID files before dropping
privileges. This represents a minor security issue; additional factors
are needed to make it exploitable.
I have made a patch to open the PID files as root. However, since the
process has to keep the file descriptor open in order not to lose the
file lock, it doesn't completely eliminate the chance of an exploit.

I will post the patch in a little while, once I have thoroughly tested it.

Regards,

Dianne.
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list ***@lists.roaringpenguin.com
http://lis
Dianne Skoll
2017-08-31 15:24:58 UTC
Permalink
Here's a patch that should apply against MIMEDefang 2.80.

Again, I cannot see any way to completely close this hole as long as
we're holding an fcnrtl(F_SETLCK)-style lock, since the descriptor
must be kept open. I do as much as I can to mitigate this by
destroying the variable containing the fd, but an attacker could
pretty quickly discover which fd is pointing to the lock file.

Since an exploit requires compromising the daemon, I would say this
is not a super-urgent problem.

Regards,

Dianne.

diff --git a/Changelog b/Changelog
index da1a867..d056e4f 100644
--- a/Changelog
+++ b/Changelog
@@ -2,6 +2,11 @@ WARNING: Before upgrading MIMEDefang, please search this file for
*** NOTE INCOMPATIBILITY ** to see if anything has changed that might
affect your filter.

+2017-08-31 Dianne Skoll <***@roaringpenguin.com>
+
+ * Make mimedefang and mimedefang-multiplexor write their PID files
+ as root to avoid an unprivileged user tampering with the pidfiles.
+
2017-07-24 Dianne Skoll <***@roaringpenguin.com>

* MIMEDefang 2.80 RELEASED
diff --git a/mimedefang-multiplexor.c b/mimedefang-multiplexor.c
index 13b77b9..3dbf6e0 100644
--- a/mimedefang-multiplexor.c
+++ b/mimedefang-multiplexor.c
@@ -566,6 +566,7 @@ main(int argc, char *argv[], char **env)
int c;
int n;
char *pidfile = NULL;
+ int pidfile_fd = -1;
char *user = NULL;
char *options;
int facility = LOG_MAIL;
@@ -919,6 +920,17 @@ main(int argc, char *argv[], char **env)
strcat((char *) Settings.sockName, "/mimedefang-multiplexor.sock");
}

+ /* Open the pidfile as root. We'll lock it later in the grandchild */
+ if (pidfile) {
+ pidfile_fd = open(pidfile, O_RDWR|O_CREAT, 0666);
+ if (pidfile_fd < 0) {
+ syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+ exit(EXIT_FAILURE);
+ }
+ /* It needs to be world-readable */
+ fchmod(pidfile_fd, 0644);
+ }
+
/* Drop privileges */
if (user) {
pw = getpwnam(user);
@@ -1134,7 +1146,7 @@ main(int argc, char *argv[], char **env)
}

for (i=0; i<256; i++) {
- if (i == unpriv_sock || i == sock || i == Pipe[0] || i == Pipe[1]) continue;
+ if (i == pidfile_fd || i == unpriv_sock || i == sock || i == Pipe[0] || i == Pipe[1]) continue;
(void) close(i);
}

@@ -1155,10 +1167,12 @@ main(int argc, char *argv[], char **env)

/* Write pid */
if (pidfile) {
- if (write_and_lock_pidfile(pidfile) < 0) {
+ if (write_and_lock_pidfile(pidfile, pidfile_fd) < 0) {
exit(1);
}
free(pidfile);
+ /* Forget the fd */
+ pidfile_fd = -1;
}

/* Set up SIGHUP handler */
diff --git a/mimedefang.c b/mimedefang.c
index 7e74137..5d545c4 100644
--- a/mimedefang.c
+++ b/mimedefang.c
@@ -2331,6 +2331,7 @@ main(int argc, char **argv)
int got_p_option = 0;
int kidpipe[2];
char kidmsg[256];
+ int pidfile_fd = -1;

mode_t socket_umask = 077;
mode_t file_umask = 077;
@@ -2611,6 +2612,17 @@ main(int argc, char **argv)
exit(EXIT_FAILURE);
}

+ /* Open the pidfile as root. We'll lock it later in the grandchild */
+ if (pidfile) {
+ pidfile_fd = open(pidfile, O_RDWR|O_CREAT, 0666);
+ if (pidfile_fd < 0) {
+ syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+ exit(EXIT_FAILURE);
+ }
+ /* It needs to be world-readable */
+ fchmod(pidfile_fd, 0644);
+ }
+
/* Look up user */
if (user) {
pw = getpwnam(user);
@@ -2715,12 +2727,14 @@ main(int argc, char **argv)

/* Write pid */
if (pidfile) {
- if (write_and_lock_pidfile(pidfile) < 0) {
+ if (write_and_lock_pidfile(pidfile, pidfile_fd) < 0) {
/* Signal the waiting parent */
REPORT_FAILURE("Cannot lock pidfile: Is another copy running?", 45);
exit(1);
}
free(pidfile);
+ /* Forget the fd */
+ pidfile_fd = -1;
}

(void) closelog();
diff --git a/mimedefang.h b/mimedefang.h
index 381316d..608c2e6 100644
--- a/mimedefang.h
+++ b/mimedefang.h
@@ -66,7 +66,7 @@ extern int make_listening_socket(char const *str, int backlog, int must_be_unix)
extern void do_delay(char const *sleepstr);
extern int is_localhost(struct sockaddr *);
extern int remove_local_socket(char const *str);
-extern int write_and_lock_pidfile(char const *pidfile);
+extern int write_and_lock_pidfile(char const *pidfile, int fd);
#ifdef EMBED_PERL
extern int make_embedded_interpreter(char const *progPath,
char const *subFilter,
diff --git a/utils.c b/utils.c
index 7d4f9c1..1ca3db6 100644
--- a/utils.c
+++ b/utils.c
@@ -1300,9 +1300,8 @@ free_debug(void *ctx, void *x, char const *fname, int line)
#endif

int
-write_and_lock_pidfile(char const *pidfile)
+write_and_lock_pidfile(char const *pidfile, int fd)
{
- int fd;
struct flock fl;
char buf[64];

@@ -1311,11 +1310,6 @@ write_and_lock_pidfile(char const *pidfile)
fl.l_start = 0;
fl.l_len = 0;

- fd = open(pidfile, O_RDWR|O_CREAT, 0666);
- if (fd < 0) {
- syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
- return -1;
- }
if (fcntl(fd, F_SETLK, &fl) < 0) {
syslog(LOG_ERR, "Could not lock PID file %s: %m. Is another copy running?", pidfile);
return -1;
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list ***@lists.roaringpenguin.com
http://lis
Michael Orlitzky
2017-08-31 15:38:25 UTC
Permalink
Post by Dianne Skoll
Here's a patch that should apply against MIMEDefang 2.80.
Wow, that was fast, thanks.
Post by Dianne Skoll
Again, I cannot see any way to completely close this hole as long as
we're holding an fcnrtl(F_SETLCK)-style lock, since the descriptor
must be kept open. I do as much as I can to mitigate this by
destroying the variable containing the fd, but an attacker could
pretty quickly discover which fd is pointing to the lock file.
You'll have to forgive the stupid question since I'm not a regular user
of MIMEDefang, but what's the purpose of the file lock? Is it to prevent
multiple daemons from running at the same time when they're not managed
by an init system?
Post by Dianne Skoll
Since an exploit requires compromising the daemon, I would say this
is not a super-urgent problem.
Agreed there, thanks again for the quick fix.
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list ***@lists.roaringpenguin.com
http://lists.roaringpenguin.com/mailman/l
Dianne Skoll
2017-08-31 15:55:48 UTC
Permalink
On Thu, 31 Aug 2017 11:38:25 -0400
Post by Michael Orlitzky
You'll have to forgive the stupid question since I'm not a regular
user of MIMEDefang, but what's the purpose of the file lock? Is it to
prevent multiple daemons from running at the same time when they're
not managed by an init system?
Yep. In the days of systemd and the like, this is probably not
necessary, but not everyone runs systemd.

If people do use systemd or whatever, then they'd start mimedefang and
mimedefang-multiplexor without the options that create the pidfiles
and let systemd manage the processes.

Regards,

Dianne.
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list ***@lists.roaringpenguin.com
http://lists.roaringpenguin.com/mailman/li
Michael Orlitzky
2017-08-31 16:11:05 UTC
Permalink
Post by Dianne Skoll
Post by Michael Orlitzky
You'll have to forgive the stupid question since I'm not a regular
user of MIMEDefang, but what's the purpose of the file lock? Is it to
prevent multiple daemons from running at the same time when they're
not managed by an init system?
Yep. In the days of systemd and the like, this is probably not
necessary, but not everyone runs systemd.
Hmmm, in that case, maybe the PID file is being reused for a purpose
that it isn't really suited for? The contents of the PID file are
slightly sensitive, since init scripts tend to trust them -- but the
contents of a lock file aren't. Would it make more sense to have a
separate lock file, whose only purpose is to prevent multiple daemons
from starting (and not to provide info to an init system)?
Post by Dianne Skoll
If people do use systemd or whatever, then they'd start mimedefang and
mimedefang-multiplexor without the options that create the pidfiles
and let systemd manage the processes.
Yeah, this is pretty much only an issue for traditional SysV-style init
systems, because trying to answer "can I trust the contents of this PID
file?" is next to impossible in portable shell script.
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list ***@lists.roaringpenguin.com
http://lists.roaringpenguin.com/mailman/listinfo/
Dianne Skoll
2017-08-31 17:01:33 UTC
Permalink
On Thu, 31 Aug 2017 12:11:05 -0400
Post by Michael Orlitzky
Hmmm, in that case, maybe the PID file is being reused for a purpose
that it isn't really suited for? The contents of the PID file are
slightly sensitive, since init scripts tend to trust them -- but the
contents of a lock file aren't. Would it make more sense to have a
separate lock file, whose only purpose is to prevent multiple daemons
from starting (and not to provide info to an init system)?
That makes sense. I'll do it that way.

Thanks for alerting me to this.

Regards,

Dianne.
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list ***@lists.roaringpenguin.com
http://lists.roaringpenguin.
Dianne Skoll
2017-08-31 20:04:20 UTC
Permalink
Hi,

The patch I posted earlier does not completely fix the problem.

True, the pid file is owned by root, but it's created in a directory
owned by defang, so there's still a way for the "defang" user to
subvert this.

I will have a patch by tomorrow that separates out the pid file (which
will be root-owned in a root-owned directory) from the lock file
(which can be defang-owned in a defang-owned directory.)

Regards,

Dianne.
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list ***@lists.roaringpenguin.com
http://lists.r
Dianne Skoll
2017-08-31 20:42:54 UTC
Permalink
Hi,

This is a much more extensive patch, but I believe it does finally
close the hole if you keep your PID files in a root-owned directory.

Please test this; I plan on releasing 2.81 tomorrow.

Regards,

Dianne.


diff --git a/Changelog b/Changelog
index da1a867..b9f09fa 100644
--- a/Changelog
+++ b/Changelog
@@ -2,6 +2,18 @@ WARNING: Before upgrading MIMEDefang, please search this file for
*** NOTE INCOMPATIBILITY ** to see if anything has changed that might
affect your filter.

+2017-08-31 Dianne Skoll <***@roaringpenguin.com>
+
+ * Make mimedefang and mimedefang-multiplexor write their PID files
+ as root to avoid an unprivileged user tampering with the pidfiles.
+
+ *** NOTE INCOMPATIBILITY ***
+
+ You should move your PID files out of the MIMEDefang spool directory
+ and into a standard root-owned directory like /var/run. Use the -o
+ option to create lock files in the spool directory. The sample
+ init scripts have been updated to reflect this.
+
2017-07-24 Dianne Skoll <***@roaringpenguin.com>

* MIMEDefang 2.80 RELEASED
diff --git a/examples/init-script.in b/examples/init-script.in
index 346efca..8da803f 100755
--- a/examples/init-script.in
+++ b/examples/init-script.in
@@ -10,8 +10,10 @@
RETVAL=0
prog='mimedefang'
SPOOLDIR='@SPOOLDIR@'
-PID="$SPOOLDIR/$prog.pid"
-MXPID="$SPOOLDIR/$prog-multiplexor.pid"
+PID="/var/run/$prog.pid"
+MXPID="/var/run/$prog-multiplexor.pid"
+LOCK="$SPOOLDIR/$prog.lock"
+MXLOCK="$SPOOLDIR/$prog-multiplexor.lock"

# These lines keep SpamAssassin happy. Not needed if you
# aren't using SpamAssassin.
@@ -229,7 +231,7 @@ start_it() {
else
EMBEDFLAG=""
fi
- $PROGDIR/$prog-multiplexor -p $MXPID \
+ $PROGDIR/$prog-multiplexor -p $MXPID -o $MXLOCK \
$EMBEDFLAG \
`[ -n "$SPOOLDIR" ] && echo "-z $SPOOLDIR"` \
`[ -n "$FILTER" ] && echo "-f $FILTER"` \
@@ -269,7 +271,7 @@ start_it() {
# Start mimedefang
printf "%-60s" "Starting $prog: "
rm -f $SOCKET > /dev/null 2>&1
- $PROGDIR/$prog -P $PID -R $LOOPBACK_RESERVED_CONNECTIONS \
+ $PROGDIR/$prog -P $PID -o $LOCK -R $LOOPBACK_RESERVED_CONNECTIONS \
-m $MX_SOCKET \
`[ -n "$SPOOLDIR" ] && echo "-z $SPOOLDIR"` \
`[ -n "$MX_USER" ] && echo "-U $MX_USER"` \
diff --git a/mimedefang-multiplexor.8.in b/mimedefang-multiplexor.8.in
index 980b57d..3505e48 100644
--- a/mimedefang-multiplexor.8.in
+++ b/mimedefang-multiplexor.8.in
@@ -117,7 +117,18 @@ for the format of \fIsocket\fR.
.TP
.B \-p \fIfileName\fR
Causes \fBmimedefang-multiplexor\fR to write its process-ID (after
-becoming a daemon) to the specified file.
+becoming a daemon) to the specified file. The file will be owned
+by root.
+
+.TP
+.B \-o \fIfileName\fR
+Causes \fbmimedefang-multiplexor\fR to use \fIfileName\fR as a lock
+file to avoid multiple instances from running. If you supply
+\fB\-p\fR but not \fB\-o\fR, then \fbmimedefang-multiplexor\fR
+constructs a lock file by appending ".lock" to the pid file. However,
+this is less secure than having a root-owned pid file in a root-owned
+directory and a lock file writable by the user named by the \fB\-U\fR
+option. (The lock file must be writable by the \fB\-U\fR user.)

.TP
.B \-f \fIfilter_path\fR
diff --git a/mimedefang-multiplexor.c b/mimedefang-multiplexor.c
index 13b77b9..2e44f12 100644
--- a/mimedefang-multiplexor.c
+++ b/mimedefang-multiplexor.c
@@ -56,6 +56,12 @@
static void limit_mem_usage(unsigned long rss, unsigned long as);
#endif

+static char *pidfile = NULL;
+static char *lockfile = NULL;
+
+/* Number of file descriptors to close when forking */
+#define CLOSEFDS 256
+
/* Weird case, but hey... */
#if defined(HAVE_WAIT3) && !defined(HAVE_SETRLIMIT)
#include <sys/resource.h>
@@ -346,6 +352,7 @@ static int get_hourly_history_totals(int cmd, time_t now, int hours, int *total,

#define NUM_FREE_SLAVES (SlaveCount[STATE_IDLE] + SlaveCount[STATE_STOPPED])
#define NUM_RUNNING_SLAVES (SlaveCount[STATE_IDLE] + SlaveCount[STATE_BUSY] + SlaveCount[STATE_KILLED])
+#define REPORT_FAILURE(msg) do { if (kidpipe[1] >= 0) { write(kidpipe[1], "E" msg, strlen(msg)+1); } else { fprintf(stderr, "%s\n", msg); } } while(0)

/**********************************************************************
* %FUNCTION: state_name
@@ -496,6 +503,7 @@ usage(void)
fprintf(stderr, " -v -- Print version and exit\n");
fprintf(stderr, " -t filename -- Log statistics to filename\n");
fprintf(stderr, " -p filename -- Write process-ID in filename\n");
+ fprintf(stderr, " -o file -- Use specified file as a lock file\n");
fprintf(stderr, " -T -- Log statistics to syslog\n");
fprintf(stderr, " -u -- Flush stats file after each write\n");
fprintf(stderr, " -Z -- Accept and process status updates from busy slaves\n");
@@ -565,10 +573,13 @@ main(int argc, char *argv[], char **env)
int sock, unpriv_sock;
int c;
int n;
- char *pidfile = NULL;
+ int pidfile_fd = -1;
+ int lockfile_fd = -1;
char *user = NULL;
char *options;
int facility = LOG_MAIL;
+ int kidpipe[2] = {-1, -1};
+ char kidmsg[256];

time_t now;

@@ -593,13 +604,13 @@ main(int argc, char *argv[], char **env)
if (getuid() != geteuid()) {
fprintf(stderr, "ERROR: %s is NOT intended to run suid! Exiting.\n",
argv[0]);
- exit(1);
+ exit(EXIT_FAILURE);
}

if (getgid() != getegid()) {
fprintf(stderr, "ERROR: %s is NOT intended to run sgid! Exiting.\n",
argv[0]);
- exit(1);
+ exit(EXIT_FAILURE);
}

Settings.minSlaves = 0;
@@ -636,9 +647,9 @@ main(int argc, char *argv[], char **env)
Settings.debugSlaveScheduling = 0;

#ifndef HAVE_SETRLIMIT
- options = "GAa:Tt:um:x:y:r:i:b:c:s:hdlf:p:w:F:W:U:S:q:Q:I:DEO:X:Y:N:vZP:z:";
+ options = "GAa:Tt:um:x:y:r:i:b:c:s:hdlf:p:o:w:F:W:U:S:q:Q:I:DEO:X:Y:N:vZP:z:";
#else
- options = "GAa:Tt:um:x:y:r:i:b:c:s:hdlf:p:w:F:W:U:S:q:Q:L:R:M:I:DEO:X:Y:N:vZP:z:";
+ options = "GAa:Tt:um:x:y:r:i:b:c:s:hdlf:p:o:w:F:W:U:S:q:Q:L:R:M:I:DEO:X:Y:N:vZP:z:";
#endif
while((c = getopt(argc, argv, options)) != -1) {
switch(c) {
@@ -669,7 +680,7 @@ main(int argc, char *argv[], char **env)
break;
case 'v':
printf("mimedefang-multiplexor version %s\n", VERSION);
- exit(0);
+ exit(EXIT_SUCCESS);

case 'E':
#ifdef EMBED_PERL
@@ -830,6 +841,16 @@ main(int argc, char *argv[], char **env)
exit(EXIT_FAILURE);
}
break;
+ case 'o':
+ /* Use this as our lock file */
+ if (lockfile != NULL) free(lockfile);
+
+ lockfile = strdup(optarg);
+ if (!lockfile) {
+ fprintf(stderr, "%s: Out of memory\n", argv[0]);
+ exit(EXIT_FAILURE);
+ }
+ break;
case 'f':
/* Filter program */
if (optarg[0] != '/') {
@@ -919,6 +940,17 @@ main(int argc, char *argv[], char **env)
strcat((char *) Settings.sockName, "/mimedefang-multiplexor.sock");
}

+ /* Open the pidfile as root. We'll write the pid later on in the grandchild */
+ if (pidfile) {
+ pidfile_fd = open(pidfile, O_RDWR|O_CREAT, 0666);
+ if (pidfile_fd < 0) {
+ syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+ exit(EXIT_FAILURE);
+ }
+ /* It needs to be world-readable */
+ fchmod(pidfile_fd, 0644);
+ }
+
/* Drop privileges */
if (user) {
pw = getpwnam(user);
@@ -964,6 +996,63 @@ main(int argc, char *argv[], char **env)
Settings.maxRecipokPerDomain = 0;
}

+ /* Daemonize */
+ if (!nodaemon) {
+ /* Set up a pipe so child can report back when it's happy */
+ if (pipe(kidpipe) < 0) {
+ perror("pipe");
+ exit(EXIT_FAILURE);
+ }
+ i = fork();
+ if (i < 0) {
+ fprintf(stderr, "%s: fork() failed\n", argv[0]);
+ exit(EXIT_FAILURE);
+ } else if (i != 0) {
+ /* parent */
+ /* Wait for a message from kid */
+ close(kidpipe[1]);
+ i = read(kidpipe[0], kidmsg, sizeof(kidmsg) - 1);
+ if (i < 0) {
+ fprintf(stderr, "Error reading message from child: %s\n",
+ strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+ /* Zero-terminate the string */
+ kidmsg[i] = 0;
+ if (i == 1 && kidmsg[0] == 'X') {
+ /* Child indicated successful startup */
+ exit(EXIT_SUCCESS);
+ }
+ if (i > 1 && kidmsg[0] == 'E') {
+ /* Child indicated error */
+ fprintf(stderr, "Error from child: %s\n", kidmsg+1);
+ exit(EXIT_FAILURE);
+ }
+ /* Unknown status from child */
+ fprintf(stderr, "Unknown reply from child: %s\n", kidmsg);
+ exit(EXIT_FAILURE);
+ }
+ setsid();
+ signal(SIGHUP, SIG_IGN);
+ i = fork();
+ if (i < 0) {
+ fprintf(stderr, "%s: fork() failed\n", argv[0]);
+ exit(EXIT_FAILURE);
+ } else if (i != 0) {
+ exit(EXIT_SUCCESS);
+ }
+
+ }
+
+ /* Do the locking */
+ if (pidfile || lockfile) {
+ if ( (lockfile_fd = write_and_lock_pidfile(pidfile, lockfile, pidfile_fd)) < 0) {
+ REPORT_FAILURE("Cannot lock lockfile: Is another copy running?");
+ exit(EXIT_FAILURE);
+ }
+ pidfile_fd = -1;
+ }
+
/* Initialize history buckets */
init_history();

@@ -983,16 +1072,18 @@ main(int argc, char *argv[], char **env)
AllSlaves = calloc(Settings.maxSlaves, sizeof(Slave));

if (!AllSlaves) {
- fprintf(stderr, "%s: Unable to allocate memory for slaves\n",
- argv[0]);
+ REPORT_FAILURE("Unable to allocate memory for slaves");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}

/* Make an event selector */
es = Event_CreateSelector();
if (!es) {
- fprintf(stderr, "%s: Could not create event selector: %s\n",
- argv[0], strerror(errno));
+ REPORT_FAILURE("Could not create event selector");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}

@@ -1051,21 +1142,21 @@ main(int argc, char *argv[], char **env)

if (sock < 0) {
if (sock == -2) {
- fprintf(stderr, "%s: Argument to -s option must be a\nUNIX-domain socket, not a TCP socket.\n", argv[0]);
+ REPORT_FAILURE("Argument to -s option must be a UNIX-domain socket, not a TCP socket.");
} else {
- fprintf(stderr, "%s: Unable to create listening socket on path %s\n",
- argv[0],
- Settings.sockName);
+ REPORT_FAILURE("Unable to create listening socket.");
}
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}
set_cloexec(sock);

/* Set up an accept loop */
if (!EventTcp_CreateAcceptor(es, sock, handleAccept)) {
- fprintf(stderr, "%s: Could not make accept() handler: %s\n",
- argv[0],
- strerror(errno));
+ REPORT_FAILURE("Could not make accept() handler");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}

@@ -1077,16 +1168,16 @@ main(int argc, char *argv[], char **env)
Settings.listenBacklog, 0);
umask(file_umask);
if (unpriv_sock < 0) {
- fprintf(stderr, "%s: Unable to create listening socket on path %s\n",
- argv[0],
- Settings.unprivSockName);
+ REPORT_FAILURE("Unable to create unprivileged listening socket");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}
set_cloexec(unpriv_sock);
if (!EventTcp_CreateAcceptor(es, unpriv_sock, handleUnprivAccept)) {
- fprintf(stderr, "%s: Could not make accept() handler: %s\n",
- argv[0],
- strerror(errno));
+ REPORT_FAILURE("Could not make accept() handler");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}
}
@@ -1096,7 +1187,9 @@ main(int argc, char *argv[], char **env)

/* Set up pipe for signal handler for slave termination notification*/
if (pipe(Pipe) < 0) {
- perror("pipe");
+ REPORT_FAILURE("pipe() call failed");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}

@@ -1106,42 +1199,28 @@ main(int argc, char *argv[], char **env)
/* Create event handler for pipe */
if (!Event_AddHandler(es, Pipe[0],
EVENT_FLAG_READABLE, handlePipe, NULL)) {
- fprintf(stderr, "%s: Could not make pipe handler: %s\n",
- argv[0],
- strerror(errno));
+ REPORT_FAILURE("Could not make pipe handler");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}

- /* Daemonize */
- if (!nodaemon) {
- i = fork();
- if (i < 0) {
- fprintf(stderr, "%s: fork() failed\n", argv[0]);
- exit(EXIT_FAILURE);
- } else if (i != 0) {
- /* parent */
- exit(EXIT_SUCCESS);
- }
- setsid();
- signal(SIGHUP, SIG_IGN);
- i = fork();
- if (i < 0) {
- fprintf(stderr, "%s: fork() failed\n", argv[0]);
- exit(EXIT_FAILURE);
- } else if (i != 0) {
- exit(EXIT_SUCCESS);
+ /* Close files */
+ for (i=0; i<CLOSEFDS; i++) {
+ /* Don't close stdin/stdout/stderr if we are not a daemon */
+ if (nodaemon && i < 3) {
+ continue;
}
- }
-
- for (i=0; i<256; i++) {
- if (i == unpriv_sock || i == sock || i == Pipe[0] || i == Pipe[1]) continue;
+ if (i == kidpipe[0] || i == kidpipe[1] || i == lockfile_fd || i == unpriv_sock || i == sock || i == Pipe[0] || i == Pipe[1]) continue;
(void) close(i);
}

- /* Direct stdin/stdout/stderr to /dev/null */
- open("/dev/null", O_RDWR);
- open("/dev/null", O_RDWR);
- open("/dev/null", O_RDWR);
+ /* Direct stdin/stdout/stderr to /dev/null if we are a daemon */
+ if (!nodaemon) {
+ open("/dev/null", O_RDWR);
+ open("/dev/null", O_RDWR);
+ open("/dev/null", O_RDWR);
+ }

/* Syslog if required */
if (Settings.syslog_label) {
@@ -1153,20 +1232,14 @@ main(int argc, char *argv[], char **env)
/* Keep track of our pid */
ParentPid = getpid();

- /* Write pid */
- if (pidfile) {
- if (write_and_lock_pidfile(pidfile) < 0) {
- exit(1);
- }
- free(pidfile);
- }
-
/* Set up SIGHUP handler */
act.sa_handler = hupHandler;
sigemptyset(&act.sa_mask);
act.sa_flags = SA_RESTART;
if (sigaction(SIGHUP, &act, NULL) < 0) {
- syslog(LOG_ERR, "sigaction failed - exiting: %m");
+ REPORT_FAILURE("sigaction failed - exiting.");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}

@@ -1175,7 +1248,9 @@ main(int argc, char *argv[], char **env)
sigemptyset(&act.sa_mask);
act.sa_flags = SA_RESTART;
if (sigaction(SIGINT, &act, NULL) < 0) {
- syslog(LOG_ERR, "sigaction failed - exiting: %m");
+ REPORT_FAILURE("sigaction failed - exiting.");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}

@@ -1207,10 +1282,12 @@ main(int argc, char *argv[], char **env)

/* Set signal handler for SIGCHLD */
if (set_sigchld_handler() < 0) {
- syslog(LOG_ERR, "sigaction failed - exiting: %m");
+ REPORT_FAILURE("sigaction failed - exiting.");
#ifdef EMBED_PERL
if (Settings.useEmbeddedPerl) term_embedded_interpreter();
#endif
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}

@@ -1267,6 +1344,10 @@ main(int argc, char *argv[], char **env)
}
}

+ /* Tell the waiting parent that everything is fine */
+ write(kidpipe[1], "X", 1);
+ close(kidpipe[1]);
+
/* And loop... */
while(1) {
if (Event_HandleEvent(es) < 0) {
@@ -2599,7 +2680,7 @@ activateSlave(Slave *s, char const *reason)
if (Settings.useEmbeddedPerl) {
run_embedded_filter();
term_embedded_interpreter();
- exit(0);
+ exit(EXIT_SUCCESS);
}
#endif
if (Settings.wantStatusReports) {
@@ -3174,9 +3255,15 @@ sigterm(int sig)
/* Only the parent process should handle SIGTERM */
if (ParentPid != getpid()) {
syslog(LOG_WARNING, "Child process received SIGTERM before signal disposition could be reset! Exiting!");
- exit(1);
+ exit(EXIT_FAILURE);
}

+ if (pidfile) {
+ unlink(pidfile);
+ }
+ if (lockfile) {
+ unlink(lockfile);
+ }
if (DOLOG) {
if (sig) {
syslog(LOG_INFO, "Received SIGTERM: Stopping slaves and terminating");
@@ -3217,7 +3304,7 @@ sigterm(int sig)
#ifdef EMBED_PERL
if (Settings.useEmbeddedPerl) term_embedded_interpreter();
#endif
- exit(1);
+ exit(EXIT_FAILURE);
}
if (j != 9) {
sleep(1);
@@ -3247,7 +3334,7 @@ sigterm(int sig)
#ifdef EMBED_PERL
if (Settings.useEmbeddedPerl) term_embedded_interpreter();
#endif
- exit(1);
+ exit(EXIT_FAILURE);
}
if (j != 9) {
sleep(1);
diff --git a/mimedefang.8.in b/mimedefang.8.in
index 3151098..e0f45ca 100644
--- a/mimedefang.8.in
+++ b/mimedefang.8.in
@@ -139,7 +139,18 @@ the directory containing the failed message using syslog.
.TP
.B \-P \fIfileName\fR
Causes \fBmimedefang\fR to write its process-ID (after
-becoming a daemon) to the specified file.
+becoming a daemon) to the specified file. The file will be
+owned by root.
+
+.TP
+.B \-o \fIfileName\fR
+Causes \fbmimedefang\fR to use \fIfileName\fR as a lock file to avoid
+multiple instances from running. If you supply \fB\-P\fR but not
+\fB\-o\fR, then \fbmimedefang\fR constructs a lock file by appending
+".lock" to the pid file. However, this is less secure than having a
+root-owned pid file in a root-owned directory and a lock file writable
+by the user named by the \fB\-U\fR option. (The lock file must be
+writable by the \fB\-U\fR user.)

.TP
.B \-R \fInum\fR
diff --git a/mimedefang.c b/mimedefang.c
index 7e74137..b311838 100644
--- a/mimedefang.c
+++ b/mimedefang.c
@@ -109,6 +109,8 @@ extern int find_syslog_facility(char const *facility_name);
#define MD_SMFI_TRY(func, args) do { if (func args != MI_SUCCESS) syslog(LOG_WARNING, "%s: %s returned MI_FAILURE", data->qid, #func); } while (0)

char *scan_body = NULL;
+static char *pidfile = NULL;
+static char *lockfile = NULL;

#define KEY_FILE CONFDIR "/mimedefang-ip-key"

@@ -474,25 +476,6 @@ set_reply(SMFICTX *ctx,
}

/**********************************************************************
-*%FUNCTION: closefiles
-*%ARGUMENTS:
-* notthis - file descriptor that should not be closed
-*%RETURNS:
-* Nothing
-*%DESCRIPTION:
-* Hack -- closes a whole bunch of file descriptors. Use after a fork()
-* to conserve descriptors.
-***********************************************************************/
-static void
-closefiles(int notthis)
-{
- int i;
- for (i=0; i<CLOSEFDS; i++) {
- if (i != notthis) (void) close(i);
- }
-}
-
-/**********************************************************************
*%FUNCTION: mfconnect
*%ARGUMENTS:
* ctx -- Sendmail filter mail context
@@ -2287,6 +2270,7 @@ usage(void)
fprintf(stderr, " -t -- Do recipient checks before processing body\n");
fprintf(stderr, " -q -- Allow new connections to be queued by multiplexor\n");
fprintf(stderr, " -P file -- Write process-ID of daemon to specified file\n");
+ fprintf(stderr, " -o file -- Use specified file as a lock file\n");
fprintf(stderr, " -T -- Log filter times to syslog\n");
fprintf(stderr, " -b n -- Set listen() backlog to n\n");
fprintf(stderr, " -C -- Try very hard to conserve file descriptors\n");
@@ -2306,7 +2290,8 @@ usage(void)
exit(EXIT_FAILURE);
}

-#define REPORT_FAILURE(msg, len) do { if (kidpipe[1] >= 0) { write(kidpipe[1], "E" msg, len+1); } else { fprintf(stderr, "%s\n", msg); } } while(0)
+#define REPORT_FAILURE(msg) do { if (kidpipe[1] >= 0) { write(kidpipe[1], "E" msg, strlen(msg)+1); } else { fprintf(stderr, "%s\n", msg); } } while(0)
+
/**********************************************************************
* %FUNCTION: main
* %ARGUMENTS:
@@ -2322,7 +2307,6 @@ main(int argc, char **argv)
int c;
int mx_alive;
pid_t i;
- char *pidfile = NULL;
struct passwd *pw = NULL;
FILE *fp;
int facility = LOG_MAIL;
@@ -2331,7 +2315,10 @@ main(int argc, char **argv)
int got_p_option = 0;
int kidpipe[2];
char kidmsg[256];
-
+ int pidfile_fd = -1;
+ int lockfile_fd = -1;
+ int rc;
+ int j;
mode_t socket_umask = 077;
mode_t file_umask = 077;

@@ -2388,7 +2375,7 @@ main(int argc, char **argv)
}

/* Process command line options */
- while ((c = getopt(argc, argv, "GNCDHL:MP:R:S:TU:Xa:b:cdhkm:p:qrstvx:z:y")) != -1) {
+ while ((c = getopt(argc, argv, "GNCDHL:MP:o:R:S:TU:Xa:b:cdhkm:p:qrstvx:z:y")) != -1) {
switch (c) {
case 'y':
setsymlist_ok = 1;
@@ -2521,6 +2508,16 @@ main(int argc, char **argv)
exit(EXIT_FAILURE);
}
break;
+ case 'o':
+ /* Use this as our lock file */
+ if (lockfile != NULL) free(lockfile);
+
+ lockfile = strdup(optarg);
+ if (!lockfile) {
+ fprintf(stderr, "%s: Out of memory\n", argv[0]);
+ exit(EXIT_FAILURE);
+ }
+ break;
case 'P':
/* Write our pid to this file */
if (pidfile != NULL) free(pidfile);
@@ -2611,6 +2608,17 @@ main(int argc, char **argv)
exit(EXIT_FAILURE);
}

+ /* Open the pidfile as root. We'll write the pid later on in the grandchild */
+ if (pidfile) {
+ pidfile_fd = open(pidfile, O_RDWR|O_CREAT, 0666);
+ if (pidfile_fd < 0) {
+ syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+ exit(EXIT_FAILURE);
+ }
+ /* It needs to be world-readable */
+ fchmod(pidfile_fd, 0644);
+ }
+
/* Look up user */
if (user) {
pw = getpwnam(user);
@@ -2655,6 +2663,8 @@ main(int argc, char **argv)
exit(EXIT_FAILURE);
}

+ (void) closelog();
+
/* Daemonize */
if (!nodaemon) {
/* Set up a pipe so child can report back when it's happy */
@@ -2676,7 +2686,7 @@ main(int argc, char **argv)
if (i < 0) {
fprintf(stderr, "Error reading message from child: %s\n",
strerror(errno));
- exit(1);
+ exit(EXIT_FAILURE);
}
/* Zero-terminate the string */
kidmsg[i] = 0;
@@ -2700,9 +2710,7 @@ main(int argc, char **argv)
signal(SIGHUP, SIG_IGN);
i = fork();
if (i < 0) {
- fprintf(stderr, "%s: fork() failed\n", argv[0]);
- /* Signal the waiting parent */
- write(kidpipe[1], "Efork() failed", 14);
+ REPORT_FAILURE("fork() failed");
exit(EXIT_FAILURE);
} else if (i != 0) {
exit(EXIT_SUCCESS);
@@ -2713,23 +2721,32 @@ main(int argc, char **argv)
kidpipe[1] = -1;
}

- /* Write pid */
- if (pidfile) {
- if (write_and_lock_pidfile(pidfile) < 0) {
- /* Signal the waiting parent */
- REPORT_FAILURE("Cannot lock pidfile: Is another copy running?", 45);
- exit(1);
+ /* In the actual daemon */
+ for (j=0; j<CLOSEFDS; j++) {
+ /* If we are not a daemon, leave stdin/stdout/stderr open */
+ if (nodaemon && j < 3) {
+ continue;
+ }
+ if (j != pidfile_fd && j != kidpipe[1]) {
+ close(j);
}
- free(pidfile);
}

- (void) closelog();
- closefiles(kidpipe[1]);
+ /* Do the locking */
+ if (pidfile || lockfile) {
+ if ( (lockfile_fd = write_and_lock_pidfile(pidfile, lockfile, pidfile_fd)) < 0) {
+ /* Signal the waiting parent */
+ REPORT_FAILURE("Cannot lock lockfile: Is another copy running?");
+ exit(EXIT_FAILURE);
+ }
+ }

/* Direct stdin/stdout/stderr to /dev/null */
- open("/dev/null", O_RDWR);
- open("/dev/null", O_RDWR);
- open("/dev/null", O_RDWR);
+ if (!nodaemon) {
+ open("/dev/null", O_RDWR);
+ open("/dev/null", O_RDWR);
+ open("/dev/null", O_RDWR);
+ }

openlog("mimedefang", LOG_PID, facility);

@@ -2770,7 +2787,9 @@ main(int argc, char **argv)
syslog(LOG_INFO, "Multiplexor alive - entering main loop");
} else {
/* Signal the waiting parent */
- REPORT_FAILURE("Multiplexor socket did not appear. Exiting.", 44);
+ REPORT_FAILURE("Multiplexor socket did not appear. Exiting.");
+ if (pidfile) unlink(pidfile);
+ if (lockfile) unlink(lockfile);
exit(EXIT_FAILURE);
}

@@ -2779,7 +2798,14 @@ main(int argc, char **argv)
write(kidpipe[1], "X", 1);
close(kidpipe[1]);
}
- return smfi_main();
+ rc = (int) smfi_main();
+ if (pidfile) {
+ unlink(pidfile);
+ }
+ if (lockfile) {
+ unlink(lockfile);
+ }
+ return rc;
}

/**********************************************************************
diff --git a/mimedefang.h b/mimedefang.h
index 381316d..59cf247 100644
--- a/mimedefang.h
+++ b/mimedefang.h
@@ -66,7 +66,7 @@ extern int make_listening_socket(char const *str, int backlog, int must_be_unix)
extern void do_delay(char const *sleepstr);
extern int is_localhost(struct sockaddr *);
extern int remove_local_socket(char const *str);
-extern int write_and_lock_pidfile(char const *pidfile);
+extern int write_and_lock_pidfile(char const *pidfile, char *lockfile, int fd);
#ifdef EMBED_PERL
extern int make_embedded_interpreter(char const *progPath,
char const *subFilter,
diff --git a/redhat/mimedefang-init.in b/redhat/mimedefang-init.in
index d3a514f..8f1aff0 100644
--- a/redhat/mimedefang-init.in
+++ b/redhat/mimedefang-init.in
@@ -31,7 +31,7 @@
# scans on incoming mail messages.
# processname: mimedefang
# config: @CONFDIR_EVAL@/mimedefang-filter
-# pidfile: @SPOOLDIR@/mimedefang.pid
+# pidfile: /var/run/mimedefang.pid

### BEGIN INIT INFO
# Provides: mimedefang
@@ -122,36 +122,37 @@ start() {
echo -n "Starting $prog-multiplexor: "
[ -e $MX_SOCKET ] && rm -f $MX_SOCKET
# Tricky stuff below... "echo -E" won't work, hence the two-step.
- daemon $PROGDIR/$prog-multiplexor -p @SPOOLDIR@/$prog-multiplexor.pid \
- $([ -n "$FILTER" ] && echo "-f $FILTER") \
- $([ -n "$SYSLOG_FACILITY" ] && echo "-S $SYSLOG_FACILITY") \
- $([ -n "$SUBFILTER" ] && echo "-F $SUBFILTER") \
- $([ -n "$MX_MINIMUM" ] && echo "-m $MX_MINIMUM") \
- $([ -n "$MX_MAXIMUM" ] && echo "-x $MX_MAXIMUM") \
- $([ -n "$MX_RECIPOK_PERDOMAIN_LIMIT" ] && echo "-y $MX_RECIPOK_PERDOMAIN_LIMIT") \
- $([ -n "$MX_USER" ] && echo "-U $MX_USER") \
- $([ -n "$MX_IDLE" ] && echo "-i $MX_IDLE") \
- $([ -n "$MX_BUSY" ] && echo "-b $MX_BUSY") \
- $([ -n "$MX_QUEUE_SIZE" ] && echo "-q $MX_QUEUE_SIZE") \
- $([ -n "$MX_QUEUE_TIMEOUT" ] && echo "-Q $MX_QUEUE_TIMEOUT") \
- $([ -n "$MX_REQUESTS" ] && echo "-r $MX_REQUESTS") \
- $([ -n "$MX_MAP_SOCKET" ] && echo "-N $MX_MAP_SOCKET") \
- $([ -n "$MX_SLAVE_DELAY" ] && echo "-w $MX_SLAVE_DELAY") \
- $([ -n "$MX_MIN_SLAVE_DELAY" ] && echo "-W $MX_MIN_SLAVE_DELAY") \
- $([ -n "$MX_LOG_SLAVE_STATUS_INTERVAL" ] && echo "-L $MX_LOG_SLAVE_STATUS_INTERVAL") \
- $([ -n "$MX_MAX_RSS" ] && echo "-R $MX_MAX_RSS") \
- $([ -n "$MX_MAX_AS" ] && echo "-M $MX_MAX_AS") \
- $([ "$MX_EMBED_PERL" = "yes" ] && (echo -n "-"; echo "E")) \
- $([ "$MX_LOG" = "yes" ] && echo "-l") \
- $([ "$MX_STATS" = "yes" ] && echo "-t /var/log/mimedefang/stats") \
- $([ "$MX_STATUS_UPDATES" = "yes" ] && echo "-Z") \
- $([ "$MX_STATS" = "yes" -a "$MX_FLUSH_STATS" = "yes" ] && echo "-u") \
- $([ -n "$MX_TICK_REQUEST" ] && echo "-X $MX_TICK_REQUEST") \
- $([ -n "$MX_TICK_PARALLEL" ] && echo "-P $MX_TICK_PARALLEL") \
- $([ "$MX_STATS_SYSLOG" = "yes" ] && echo "-T") \
- $([ "$MD_ALLOW_GROUP_ACCESS" = "yes" ] && echo "-G") \
- $([ -n "$MX_NOTIFIER" ] && echo "-O $MX_NOTIFIER") \
- -s $MX_SOCKET
+ daemon $PROGDIR/$prog-multiplexor -p /var/run/$prog-multiplexor.pid \
+ -o @SPOOLDIR@/$prog-multiplexor.lock \
+ $([ -n "$FILTER" ] && echo "-f $FILTER") \
+ $([ -n "$SYSLOG_FACILITY" ] && echo "-S $SYSLOG_FACILITY") \
+ $([ -n "$SUBFILTER" ] && echo "-F $SUBFILTER") \
+ $([ -n "$MX_MINIMUM" ] && echo "-m $MX_MINIMUM") \
+ $([ -n "$MX_MAXIMUM" ] && echo "-x $MX_MAXIMUM") \
+ $([ -n "$MX_RECIPOK_PERDOMAIN_LIMIT" ] && echo "-y $MX_RECIPOK_PERDOMAIN_LIMIT") \
+ $([ -n "$MX_USER" ] && echo "-U $MX_USER") \
+ $([ -n "$MX_IDLE" ] && echo "-i $MX_IDLE") \
+ $([ -n "$MX_BUSY" ] && echo "-b $MX_BUSY") \
+ $([ -n "$MX_QUEUE_SIZE" ] && echo "-q $MX_QUEUE_SIZE") \
+ $([ -n "$MX_QUEUE_TIMEOUT" ] && echo "-Q $MX_QUEUE_TIMEOUT") \
+ $([ -n "$MX_REQUESTS" ] && echo "-r $MX_REQUESTS") \
+ $([ -n "$MX_MAP_SOCKET" ] && echo "-N $MX_MAP_SOCKET") \
+ $([ -n "$MX_SLAVE_DELAY" ] && echo "-w $MX_SLAVE_DELAY") \
+ $([ -n "$MX_MIN_SLAVE_DELAY" ] && echo "-W $MX_MIN_SLAVE_DELAY") \
+ $([ -n "$MX_LOG_SLAVE_STATUS_INTERVAL" ] && echo "-L $MX_LOG_SLAVE_STATUS_INTERVAL") \
+ $([ -n "$MX_MAX_RSS" ] && echo "-R $MX_MAX_RSS") \
+ $([ -n "$MX_MAX_AS" ] && echo "-M $MX_MAX_AS") \
+ $([ "$MX_EMBED_PERL" = "yes" ] && (echo -n "-"; echo "E")) \
+ $([ "$MX_LOG" = "yes" ] && echo "-l") \
+ $([ "$MX_STATS" = "yes" ] && echo "-t /var/log/mimedefang/stats") \
+ $([ "$MX_STATUS_UPDATES" = "yes" ] && echo "-Z") \
+ $([ "$MX_STATS" = "yes" -a "$MX_FLUSH_STATS" = "yes" ] && echo "-u") \
+ $([ -n "$MX_TICK_REQUEST" ] && echo "-X $MX_TICK_REQUEST") \
+ $([ -n "$MX_TICK_PARALLEL" ] && echo "-P $MX_TICK_PARALLEL") \
+ $([ "$MX_STATS_SYSLOG" = "yes" ] && echo "-T") \
+ $([ "$MD_ALLOW_GROUP_ACCESS" = "yes" ] && echo "-G") \
+ $([ -n "$MX_NOTIFIER" ] && echo "-O $MX_NOTIFIER") \
+ -s $MX_SOCKET
echo

# Start daemon
@@ -162,31 +163,27 @@ start() {
# thread-creation will fail on a very busy server.
ulimit -s 2048

- daemon $PROGDIR/$prog -P @SPOOLDIR@/$prog.pid \
- -m $MX_SOCKET \
- $([ -n "$LOOPBACK_RESERVED_CONNECTIONS" ] && echo "-R $LOOPBACK_RESERVED_CONNECTIONS") \
- $([ -n "$MX_USER" ] && echo "-U $MX_USER") \
- $([ -n "$SYSLOG_FACILITY" ] && echo "-S $SYSLOG_FACILITY") \
- $([ "$LOG_FILTER_TIME" = "yes" ] && echo "-T") \
- $([ "$MX_RELAY_CHECK" = "yes" ] && echo "-r") \
- $([ "$MX_HELO_CHECK" = "yes" ] && echo "-H") \
- $([ "$MX_SENDER_CHECK" = "yes" ] && echo "-s") \
- $([ "$MX_RECIPIENT_CHECK" = "yes" ] && echo "-t") \
- $([ "$KEEP_FAILED_DIRECTORIES" = "yes" ] && echo "-k") \
- $([ "$MD_ALLOW_GROUP_ACCESS" = "yes" ] && echo "-G") \
- $([ -n "$MD_EXTRA" ] && echo "$MD_EXTRA") \
- $([ "$ALLOW_NEW_CONNECTIONS_TO_QUEUE" = "yes" ] && echo "-q") \
- -p $SOCKET
+ daemon $PROGDIR/$prog -P /var/run/$prog.pid \
+ -o @SPOOLDIR@/$prog.lock \
+ -m $MX_SOCKET \
+ $([ -n "$LOOPBACK_RESERVED_CONNECTIONS" ] && echo "-R $LOOPBACK_RESERVED_CONNECTIONS") \
+ $([ -n "$MX_USER" ] && echo "-U $MX_USER") \
+ $([ -n "$SYSLOG_FACILITY" ] && echo "-S $SYSLOG_FACILITY") \
+ $([ "$LOG_FILTER_TIME" = "yes" ] && echo "-T") \
+ $([ "$MX_RELAY_CHECK" = "yes" ] && echo "-r") \
+ $([ "$MX_HELO_CHECK" = "yes" ] && echo "-H") \
+ $([ "$MX_SENDER_CHECK" = "yes" ] && echo "-s") \
+ $([ "$MX_RECIPIENT_CHECK" = "yes" ] && echo "-t") \
+ $([ "$KEEP_FAILED_DIRECTORIES" = "yes" ] && echo "-k") \
+ $([ "$MD_ALLOW_GROUP_ACCESS" = "yes" ] && echo "-G") \
+ $([ -n "$MD_EXTRA" ] && echo "$MD_EXTRA") \
+ $([ "$ALLOW_NEW_CONNECTIONS_TO_QUEUE" = "yes" ] && echo "-q") \
+ -p $SOCKET
RETVAL=$?
echo

[ $RETVAL -eq 0 ] && touch /var/lock/subsys/$prog

- # Red Hat gets upset if pid files are not under /var/run, so let's
- # keep Red Hat happy...
- sleep 1
- [ -f @SPOOLDIR@/$prog.pid ] && cp -f @SPOOLDIR@/$prog.pid /var/run
- [ -f @SPOOLDIR@/$prog-multiplexor.pid ] && cp -f @SPOOLDIR@/$prog-multiplexor.pid /var/run
return $RETVAL
}

@@ -219,7 +216,7 @@ stop() {
echo

[ -e $SOCKET ] && rm -f $SOCKET
- [ -f @SPOOLDIR@/$prog.pid ] && rm -f @SPOOLDIR@/$prog.pid
+ [ -f /var/run/$prog.pid ] && rm -f /var/run/$prog.pid
[ -f /var/run/$prog.pid ] && rm -f /var/run/$prog.pid

# Stop daemon
@@ -232,8 +229,8 @@ stop() {
if [ "$1" = "wait" ] ; then
printf "Waiting for daemons to exit"
WAITPID=""
- test -f @SPOOLDIR@/$prog.pid && WAITPID=`cat @SPOOLDIR@/$prog.pid`
- test -f @SPOOLDIR@/prog-multiplexor.pid && WAITPID="$WAITPID `cat @SPOOLDIR@/prog-multiplexor.pid`"
+ test -f /var/run/$prog.pid && WAITPID=`cat /var/run/$prog.pid`
+ test -f /var/run//prog-multiplexor.pid && WAITPID="$WAITPID `cat /var/run//prog-multiplexor.pid`"
n=0
while [ -n "$WAITPID" ] ; do
W2=""
@@ -252,7 +249,7 @@ stop() {
echo ""
fi

- [ -f @SPOOLDIR@/$prog-multiplexor.pid ] && rm -f @SPOOLDIR@/$prog-multiplexor.pid
+ [ -f /var/run/$prog-multiplexor.pid ] && rm -f /var/run/$prog-multiplexor.pid
[ -f /var/run/$prog-multiplexor.pid ] && rm -f /var/run/$prog-multiplexor.pid

[ $RETVAL -eq 0 ] && rm -f /var/lock/subsys/$prog
@@ -306,8 +303,8 @@ case "$1" in
echo "Could not communicate with $prog-multiplexor"
fi
else
- if [ -r @SPOOLDIR@/$prog-multiplexor.pid ] ; then
- kill -INT `cat @SPOOLDIR@/$prog-multiplexor.pid`
+ if [ -r /var/run/$prog-multiplexor.pid ] ; then
+ kill -INT `cat /var/run/$prog-multiplexor.pid`
RETVAL=$?
if [ $RETVAL = 0 ] ; then
echo "Told $prog-multiplexor to force reread of filter rules."
diff --git a/utils.c b/utils.c
index 7d4f9c1..4e84059 100644
--- a/utils.c
+++ b/utils.c
@@ -1300,30 +1300,53 @@ free_debug(void *ctx, void *x, char const *fname, int line)
#endif

int
-write_and_lock_pidfile(char const *pidfile)
+write_and_lock_pidfile(char const *pidfile, char *lockfile, int pidfile_fd)
{
- int fd;
struct flock fl;
char buf[64];
+ int lockfile_fd;
+ size_t len;
+
+ if (!lockfile) {
+ if (!pidfile) {
+ return -1;
+ }
+ len = strlen(pidfile) + 6;
+ /* If no lockfile was supplied, construct one based on pidfile */
+ lockfile = malloc(len);
+ if (!lockfile) {
+ return -1;
+ }
+
+ snprintf(lockfile, len, "%s.lock", pidfile);
+ }
+
+ lockfile_fd = open(lockfile, O_RDWR|O_CREAT, 0666);
+ if (lockfile_fd < 0) {
+ return -1;
+ }

fl.l_type = F_WRLCK;
fl.l_whence = SEEK_SET;
fl.l_start = 0;
fl.l_len = 0;

- fd = open(pidfile, O_RDWR|O_CREAT, 0666);
- if (fd < 0) {
- syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+ if (fcntl(lockfile_fd, F_SETLK, &fl) < 0) {
+ syslog(LOG_ERR, "Could not lock lockfile file %s: %m. Is another copy running?", lockfile);
return -1;
}
- if (fcntl(fd, F_SETLK, &fl) < 0) {
- syslog(LOG_ERR, "Could not lock PID file %s: %m. Is another copy running?", pidfile);
- return -1;
+ if (pidfile_fd >= 0) {
+ ftruncate(pidfile_fd, 0);
+ snprintf(buf, sizeof(buf), "%lu\n", (unsigned long) getpid());
+ write(pidfile_fd, buf, strlen(buf));
+
+ /* Close the pidfile fd; no longer needed */
+ if (close(pidfile_fd) < 0) {
+ return -1;
+ }
}
- ftruncate(fd, 0);
- snprintf(buf, sizeof(buf), "%lu\n", (unsigned long) getpid());
- write(fd, buf, strlen(buf));
- /* Do NOT close fd... it will close and lock will be released
+
+ /* Do NOT close lockfile_fd... it will close and lock will be released
when we exit */
- return 0;
+ return lockfile_fd;
}
Michael Orlitzky
2017-08-31 22:09:17 UTC
Permalink
Post by Dianne Skoll
Hi,
This is a much more extensive patch, but I believe it does finally
close the hole if you keep your PID files in a root-owned directory.
Please test this; I plan on releasing 2.81 tomorrow.
I applied the patch and updated the Gentoo init script with the new -p
and -o changes, and now everything looks good. The two PID files are
located directly in /run and owned by root:root, while the two lock
files live in the spool directory and are owned by defang:defang.

The daemon starts/stops without issue.

Thanks once more for your help with this. I'll ask for a CVE assignment
in a moment, and then wait until the new version is released before
making an announcement for the distros.
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list ***@lists.roaringpenguin.com
http://lists.roaringpenguin.com/mailm

Loading...