hi there!
just over a year ago, i submitted a bunch of patches for the comsatd
in mailutils-1.0. a couple were accepted but the majority fell by the
wayside, even though some initial interest was expressed in some of
them. i've been using my patched version of mu-1.0 since then.
i've just finally updated these patches to apply against
mailutils-1.2. i'm attaching all the patches; each one contains
explanations and justifications within the patch file.
unlike last time, i've implemented two of the patches by turning some
variables into globals. i realise this isn't beautiful, in some ways,
but it seems to be the cleanest way to implement what i'm trying to
do.
some of the patches only apply in a certain order. this order is
known to work:
mailutils-1.2-comsatd-new-escapes-f+o-3.patch
mailutils-1.2-comsatd-exec-without-tty-2.patch
mailutils-1.2-comsatd-reset-atime-1.patch
mailutils-1.2-comsatd-sigchld-fix-1.patch
mailutils-1.2-comsatd-secure-change-user-1.patch
hope some of this helps!
one other thing - in my original patches i included a fix for a bug
reading .biffrc files: if a blank line was encountered, processing
would stop and no further lines after the blank would be run. in
other words, this works in .biffrc:
echo line 1
echo line 2
but if you change it to the following only the first line gets printed:
echo line 1
echo line 2
my original fix was `sort of' incorporated but was much changed; i see
that this particular bug ended up not being fixed due to the changes.
i just noticed it and haven't fixed it again in the attached patches
but perhaps someone could look into fixing this? i can also send
another patch if desired.
cheers
-damon
Submitted by: Damon Harper <[EMAIL PROTECTED]>
Date: 2006-08-26, 2007-09-21
Summary: Add some new escapes to comsatd / .biffrc
This patch introduces two new escapes for use in comsatd's .biffrc:
$f: The full pathname of the mailbox file where the new mail is found.
$o: The offset into $f where the new mail starts.
This allows things like echoing the mailbox pathname when mail might
be in one of a number of inboxes, or passing the pathname and offset
to another program. For instance, I call a perl script that
rebroadcasts the comsat message "[EMAIL PROTECTED]:$f" to my workstation).
The patch is implemented by making path and offset global variables
within comsatd.
diff -urN mailutils-1.2.orig/comsat/action.c mailutils-1.2/comsat/action.c
--- mailutils-1.2.orig/comsat/action.c 2007-06-27 05:07:16.000000000 -0700
+++ mailutils-1.2/comsat/action.c 2007-09-21 02:02:48.000000000 -0700
@@ -18,6 +18,7 @@
MA 02110-1301 USA */
#include "comsat.h"
+#include <mu_umaxtostr.h>
#define obstack_chunk_alloc malloc
#define obstack_chunk_free free
#include <obstack.h>
@@ -96,6 +97,7 @@
mu_stream_t stream;
int rc = 1;
size_t size = 0, lncount = 0;
+ char *offset_str;
switch (*++p) /* skip past $ */
{
@@ -113,6 +115,21 @@
rc = 0;
break;
+ case 'f':
+ len = strlen (path);
+ obstack_grow (stk, path, len);
+ *pp = p;
+ rc = 0;
+ break;
+
+ case 'o':
+ offset_str = mu_umaxtostr(0, offset);
+ len = strlen (offset_str);
+ obstack_grow (stk, offset_str, len);
+ *pp = p;
+ rc = 0;
+ break;
+
case 'H':
/* Header field */
if (*++p != '{')
diff -urN mailutils-1.2.orig/comsat/comsat.c mailutils-1.2/comsat/comsat.c
--- mailutils-1.2.orig/comsat/comsat.c 2007-06-27 05:07:16.000000000 -0700
+++ mailutils-1.2/comsat/comsat.c 2007-09-21 02:02:38.000000000 -0700
@@ -104,7 +104,7 @@
static void comsat_daemon_init (void);
static void comsat_daemon (int _port);
static int comsat_main (int fd);
-static void notify_user (const char *user, const char *device, const char
*path, off_t offset);
+static void notify_user (const char *user, const char *device);
static int find_user (const char *name, char *tty);
static char *mailbox_path (const char *user);
static void change_user (const char *user);
@@ -226,6 +226,8 @@
time_t overflow_control_interval = 10; /* Overflow control interval */
time_t overflow_delay_time = 5;
+char *path = NULL; /* Will contain the mailbox filename */
+size_t offset; /* Will contain the offset of message within mailbox */
void
comsat_daemon (int port)
@@ -325,8 +327,6 @@
pid_t pid;
char tty[MAX_TTY_SIZE];
char *p, *endp;
- size_t offset;
- char *path = NULL;
len = sizeof sin_from;
rdlen = recvfrom (fd, buffer, sizeof buffer, 0,
@@ -400,7 +400,7 @@
}
/* Child: do actual I/O */
- notify_user (buffer, tty, path, offset);
+ notify_user (buffer, tty);
exit (0);
}
@@ -423,7 +423,7 @@
/* NOTE: Do not bother to free allocated memory, as the program exits
immediately after executing this */
static void
-notify_user (const char *user, const char *device, const char *path, off_t
offset)
+notify_user (const char *user, const char *device)
{
FILE *fp;
const char *cr;
diff -urN mailutils-1.2.orig/comsat/comsat.h mailutils-1.2/comsat/comsat.h
--- mailutils-1.2.orig/comsat/comsat.h 2007-06-27 05:07:16.000000000 -0700
+++ mailutils-1.2/comsat/comsat.h 2007-09-21 02:02:38.000000000 -0700
@@ -76,6 +76,8 @@
extern const char *username;
extern char hostname[];
extern struct daemon_param daemon_param;
+extern char *path;
+extern size_t offset;
extern void read_config (const char *config_file);
int acl_match (struct sockaddr_in *sa_in);
diff -urN mailutils-1.2.orig/doc/texinfo/mailutils.info-1
mailutils-1.2/doc/texinfo/mailutils.info-1
--- mailutils-1.2.orig/doc/texinfo/mailutils.info-1 2007-06-29
05:01:17.000000000 -0700
+++ mailutils-1.2/doc/texinfo/mailutils.info-1 2007-09-21 02:02:38.000000000
-0700
@@ -4043,6 +4043,12 @@
$h
Expands to hostname
+$f
+ Expands to the name of the folder containing the new message.
+
+$o
+ Expands to the offset in $f where the new message starts.
+
$H{name}
Expands to value of message header `name'.
diff -urN mailutils-1.2.orig/doc/texinfo/programs.texi
mailutils-1.2/doc/texinfo/programs.texi
--- mailutils-1.2.orig/doc/texinfo/programs.texi 2007-02-25
10:08:35.000000000 -0800
+++ mailutils-1.2/doc/texinfo/programs.texi 2007-09-21 02:02:38.000000000
-0700
@@ -4121,6 +4121,10 @@
Expands to username
@item $h
Expands to hostname
[EMAIL PROTECTED] $f
+Expands to the name of the folder containing the new message.
[EMAIL PROTECTED] $o
+Expands to the offset in $f where the new message starts.
@item [EMAIL PROTECTED]@}
Expands to value of message header @samp{name}.
@item $B(@var{c},@var{l})
Submitted by: Damon Harper <[EMAIL PROTECTED]>
Date: 2006-08-28, 2007-09-21
Summary: Run comsatd exec actions even if user is not logged in
Currently, comsatd's child process simply exits if it cannot find a
tty where the user to be notified is logged in. This is fine for the
default action, or echo and beep actions; however, this also prevents
exec actions from running unless the user is actively logged in at the
time of notification.
This should not be the desired behaviour. A user might use exec to
perform an action that should fire off even if that user is not logged
in to a tty. In the comsatd documentation, an example is given of an
exec command that will run xmessage to show a message in x-windows.
With the current behaviour, that example will only work if the user is
also logged in to a tty (perhaps via xterm) at the time of the
message.
This patch changes tty to a global variable (ttyfp). This global is
set to a tty file pointer if a suitable tty is found, but otherwise is
set to NULL. run_user_action, open_rc, and action_* functions are now
reworked to assume that global ttyfp could be NULL. action_beep and
action_echo both return immediately if this is the case, but
action_exec simply continues with all output directed to /dev/null.
This patch also fixes all functions to use the cr string (also made
global) instead of hard-coded \r\n when writing to the tty.
NOTE: This patch applies on top of:
mailutils-1.2-comsatd-new-escapes-f+o-3.patch
It will NOT apply cleanly to stock mailutils-1.2.
diff -urN mailutils-1.2.orig-new-escapes/comsat/action.c
mailutils-1.2/comsat/action.c
--- mailutils-1.2.orig-new-escapes/comsat/action.c 2007-09-21
02:47:36.000000000 -0700
+++ mailutils-1.2/comsat/action.c 2007-09-21 02:45:34.000000000 -0700
@@ -253,35 +253,41 @@
#define LB(c) ((c)&0x7f)
static void
-action_beep (FILE *tty)
+action_beep ()
{
- fprintf (tty, "\a\a");
+ if (ttyfp == NULL)
+ return;
+ fprintf (ttyfp, "\a\a");
}
static void
-echo_string (FILE *tty, const char *cr, char *str)
+echo_string (char *str)
{
if (!str)
return;
+ if (ttyfp == NULL)
+ return;
for (; *str; str++)
{
if (*str == '\n')
- fprintf (tty, "%s", cr);
+ fprintf (ttyfp, "%s", cr);
else
{
char c = LB (*str);
- putc (c, tty);
+ putc (c, ttyfp);
}
}
- fflush (tty);
+ fflush (ttyfp);
}
static void
-action_echo (FILE *tty, const char *cr, int omit_newline,
- int argc, char **argv)
+action_echo (int omit_newline, int argc, char **argv)
{
int i;
+ if (ttyfp == NULL)
+ return;
+
if (omit_newline)
{
argc--;
@@ -290,20 +296,20 @@
for (i = 0;;)
{
- echo_string (tty, cr, argv[i]);
+ echo_string (argv[i]);
if (++i < argc)
- echo_string (tty, cr, " ");
+ echo_string (" ");
else
{
if (!omit_newline)
- echo_string (tty, cr, "\n");
+ echo_string ("\n");
break;
}
}
}
static void
-action_exec (FILE *tty, int line, int argc, char **argv)
+action_exec (int line, int argc, char **argv)
{
pid_t pid;
struct stat stb;
@@ -338,11 +344,14 @@
pid = fork ();
if (pid == 0)
{
- close (1);
- close (2);
- dup2 (fileno (tty), 1);
- dup2 (fileno (tty), 2);
- fclose (tty);
+ if (ttyfp != NULL)
+ {
+ close (1);
+ close (2);
+ dup2 (fileno (ttyfp), 1);
+ dup2 (fileno (ttyfp), 2);
+ fclose (ttyfp);
+ }
execv (argv[0], argv);
syslog (LOG_ERR, _("Cannot execute %s: %s"), argv[0], strerror (errno));
exit (0);
@@ -350,7 +359,7 @@
}
static FILE *
-open_rc (const char *filename, FILE *tty)
+open_rc (const char *filename)
{
struct stat stb;
struct passwd *pw = getpwnam (username);
@@ -368,8 +377,9 @@
}
if ((stb.st_mode & 0777) != 0600)
{
- fprintf (tty, "%s\r\n",
- _("Warning: your .biffrc has wrong permissions"));
+ if (ttyfp != NULL)
+ fprintf (ttyfp, "%s%s",
+ _("Warning: your .biffrc has wrong permissions"), cr);
syslog (LOG_NOTICE, _("%s's %s has wrong permissions"),
username, filename);
return NULL;
@@ -379,14 +389,14 @@
}
void
-run_user_action (FILE *tty, const char *cr, mu_message_t msg)
+run_user_action (mu_message_t msg)
{
FILE *fp;
int nact = 0;
char *stmt = NULL;
size_t size = 0;
- fp = open_rc (BIFF_RC, tty);
+ fp = open_rc (BIFF_RC);
if (fp)
{
unsigned line = 1, n;
@@ -403,7 +413,7 @@
if (strcmp (argv[0], "beep") == 0)
{
/* FIXME: excess arguments are ignored */
- action_beep (tty);
+ action_beep ();
nact++;
}
else
@@ -423,18 +433,21 @@
if (strcmp (argv[0], "echo") == 0)
{
- action_echo (tty, cr, n_option, argc - 1, argv + 1);
+ action_echo (n_option, argc - 1, argv + 1);
nact++;
}
else if (strcmp (argv[0], "exec") == 0)
{
- action_exec (tty, line, argc - 1, argv + 1);
+ action_exec (line, argc - 1, argv + 1);
nact++;
}
else
{
- fprintf (tty, _(".biffrc:%d: unknown keyword"), line);
- fprintf (tty, "\r\n");
+ if (ttyfp != NULL)
+ {
+ fprintf (ttyfp, _(".biffrc:%d: unknown keyword"),
line);
+ fprintf (ttyfp, "%s", cr);
+ }
syslog (LOG_ERR, _("%s:.biffrc:%d: unknown keyword %s"),
username, line, argv[0]);
break;
@@ -448,5 +461,5 @@
}
if (nact == 0)
- echo_string (tty, cr, expand_line (default_action, msg));
+ echo_string (expand_line (default_action, msg));
}
diff -urN mailutils-1.2.orig-new-escapes/comsat/comsat.c
mailutils-1.2/comsat/comsat.c
--- mailutils-1.2.orig-new-escapes/comsat/comsat.c 2007-09-21
02:47:36.000000000 -0700
+++ mailutils-1.2/comsat/comsat.c 2007-09-21 02:45:40.000000000 -0700
@@ -228,6 +228,8 @@
char *path = NULL; /* Will contain the mailbox filename */
size_t offset; /* Will contain the offset of message within mailbox */
+FILE *ttyfp = NULL; /* Will hold chosen TTY's file pointer if one is found */
+const char *cr; /* Will hold the newline string */
void
comsat_daemon (int port)
@@ -326,6 +328,7 @@
char buffer[216]; /*FIXME: Arbitrary size */
pid_t pid;
char tty[MAX_TTY_SIZE];
+ int use_tty;
char *p, *endp;
len = sizeof sin_from;
@@ -375,8 +378,7 @@
syslog (LOG_ERR, _("Malformed input: [EMAIL PROTECTED] (near %s)"),
buffer, p, endp);
}
- if (find_user (buffer, tty) != SUCCESS)
- return 0;
+ use_tty = find_user (buffer, tty) == SUCCESS;
/* All I/O is done by child process. This is to avoid various blocking
problems. */
@@ -400,7 +402,7 @@
}
/* Child: do actual I/O */
- notify_user (buffer, tty);
+ notify_user (buffer, use_tty ? tty : NULL);
exit (0);
}
@@ -425,8 +427,6 @@
static void
notify_user (const char *user, const char *device)
{
- FILE *fp;
- const char *cr;
char *blurb;
mu_mailbox_t mbox = NULL, tmp = NULL;
mu_message_t msg;
@@ -436,13 +436,14 @@
size_t count, n;
change_user (user);
- if ((fp = fopen (device, "w")) == NULL)
+ if (device == NULL)
+ ttyfp = NULL;
+ else if ((ttyfp = fopen (device, "w")) == NULL)
{
- syslog (LOG_ERR, _("Cannot open device %s: %m"), device);
- exit (0);
+ syslog (LOG_WARNING, _("Cannot open device %s: %m; continuing with no
TTY"), device);
}
- cr = get_newline_str (fp);
+ cr = ttyfp == NULL ? "\r\n" : get_newline_str (ttyfp);
if (!path)
{
@@ -502,8 +501,9 @@
mu_mailbox_messages_count (tmp, &count);
mu_mailbox_get_message (tmp, 1, &msg);
- run_user_action (fp, cr, msg);
- fclose (fp);
+ run_user_action (msg);
+ if (ttyfp != NULL)
+ fclose (ttyfp);
}
/* Search utmp for the local user */
diff -urN mailutils-1.2.orig-new-escapes/comsat/comsat.h
mailutils-1.2/comsat/comsat.h
--- mailutils-1.2.orig-new-escapes/comsat/comsat.h 2007-09-21
02:47:36.000000000 -0700
+++ mailutils-1.2/comsat/comsat.h 2007-09-21 02:45:46.000000000 -0700
@@ -78,7 +78,9 @@
extern struct daemon_param daemon_param;
extern char *path;
extern size_t offset;
+extern FILE *ttyfp;
+extern const char *cr;
extern void read_config (const char *config_file);
int acl_match (struct sockaddr_in *sa_in);
-void run_user_action (FILE *tty, const char *cr, mu_message_t msg);
+void run_user_action (mu_message_t msg);
Submitted by: Damon Harper <[EMAIL PROTECTED]>
Date: 2006-08-26, 2007-09-21
Summary: Reset access/mod times after comsatd accesses a mailbox
This patch makes comsatd reset the access and modification times on a
mailbox after scanning it for new mail. MUAs like mutt depend on the
access and modification times to let the user know if there is new
mail in a mailbox. comsatd currently causes the access time to be
updated, which thwarts this mechanism and makes it look like there is
never new mail.
There may be a better way to implement this; for example I imagine it
would be better to add an option to the various functions used to scan
mail, which causes them to reset the access time while the mailbox is
still locked. With this patch, there is a race condition that means a
legitimate access/modification time not caused by comsat might be
clobbered. However for practical purposes this quick hack seems to be
just fine.
NOTE: This patch applies on top of:
mailutils-1.2-comsatd-exec-without-tty-2.patch
It will NOT apply cleanly to stock mailutils-1.2.
diff -urN mailutils-1.2.orig/comsat/comsat.c mailutils-1.2/comsat/comsat.c
--- mailutils-1.2.orig/comsat/comsat.c 2007-06-27 05:07:16.000000000 -0700
+++ mailutils-1.2/comsat/comsat.c 2007-09-21 01:36:22.000000000 -0700
@@ -434,6 +434,9 @@
int status;
off_t size;
size_t count, n;
+ int reset_times = 0;
+ struct stat sb;
+ struct utimbuf ub;
change_user (user);
if ((fp = fopen (device, "w")) == NULL)
@@ -451,6 +454,10 @@
return;
}
+ /* Store mailbox access and modification times. */
+ if (stat (path, &sb) != -1)
+ reset_times = 1;
+
if ((status = mu_mailbox_create (&mbox, path)) != 0
|| (status = mu_mailbox_open (mbox, MU_STREAM_READ)) != 0)
{
@@ -504,6 +511,14 @@
run_user_action (msg);
if (ttyfp != NULL)
fclose (ttyfp);
+
+ /* Now if possible reset the access and modification times, so programs
+ like mutt will still see new mail in the folder. */
+ if (reset_times) {
+ ub.modtime = sb.st_mtime;
+ ub.actime = sb.st_atime;
+ utime (path, &ub); /* Ignore return value - if it fails, too bad. */
+ }
}
/* Search utmp for the local user */
diff -urN mailutils-1.2.orig/comsat/comsat.h mailutils-1.2/comsat/comsat.h
--- mailutils-1.2.orig/comsat/comsat.h 2007-06-27 05:07:16.000000000 -0700
+++ mailutils-1.2/comsat/comsat.h 2007-09-21 01:35:43.000000000 -0700
@@ -37,6 +37,7 @@
#include <string.h>
#include <pwd.h>
#include <ctype.h>
+#include <utime.h>
#ifdef HAVE_PATHS_H
# include <paths.h>
Submitted by: Damon Harper <[EMAIL PROTECTED]>
Date: 2006-08-26, 2007-09-21
Summary: Fix SIGCHLD handling in comsatd
This patch fixes a problem with comsatd where child processes spawned
by .biffrc commands complain because SIGCHLD has been ignored. For
example, when calling a perl script with SIGCHLD ignored, perl
generates the following error on stderr, which then gets echoed to the
tty:
Can't ignore signal CHLD, forcing to default.
With this patch, comsatd is more careful about child processes, and
also will exit more quickly if its first child exits before the
select() timeout. For this reason I've also increased the timeout
before killing a child to 1 second; unless the child is really hung,
this should never be reached.
NOTE: This patch applies on top of:
mailutils-1.2-comsatd-exec-without-tty-2.patch
It will NOT apply cleanly to stock mailutils-1.2.
diff -urN mailutils-1.2.orig/comsat/comsat.c mailutils-1.2/comsat/comsat.c
--- mailutils-1.2.orig/comsat/comsat.c 2007-06-27 05:07:16.000000000 -0700
+++ mailutils-1.2/comsat/comsat.c 2007-09-21 01:39:30.000000000 -0700
@@ -192,6 +192,20 @@
}
void
+comsat_parent_sig_chld (int sig)
+{
+ if (waitpid(-1, NULL, WNOHANG) > 0)
+ exit (0); /* child is finished, we can exit immediately */
+}
+
+void
+comsat_child_sig_chld (int sig)
+{
+ while (waitpid(-1, NULL, WNOHANG) > 0)
+ ; /* reap but ignore */
+}
+
+void
comsat_init ()
{
mu_registrar_record (mu_path_record);
@@ -200,7 +214,7 @@
/* Set signal handlers */
signal (SIGTTOU, SIG_IGN);
- signal (SIGCHLD, SIG_IGN);
+ signal (SIGCHLD, comsat_parent_sig_chld);
signal (SIGHUP, SIG_IGN); /* Ignore SIGHUP. */
}
@@ -392,14 +406,15 @@
if (pid > 0)
{
struct timeval tv;
- tv.tv_sec = 0;
- tv.tv_usec = 100000;
+ tv.tv_sec = 1;
+ tv.tv_usec = 0;
select (0, NULL, NULL, NULL, &tv);
- kill (pid, SIGKILL); /* Just in case the child is hung */
+ kill (pid, SIGKILL); /* Child has not exited; must be hung */
return 0;
}
/* Child: do actual I/O */
+ signal (SIGCHLD, comsat_child_sig_chld);
notify_user (buffer, use_tty ? tty : NULL);
exit (0);
}
Submitted by: Damon Harper <[EMAIL PROTECTED]>
Date: 2006-08-28, 2007-09-21
Summary: Check for errors in comsatd change_user and exit if anything fails
In the change_user function of comsatd, no checks were performed on
the exit values of setgid, setuid and chdir. This could theoretically
lead to privilege escalation if setuid or setgid fails. With this
patch, comsatd will exit if one of these commands fails.
diff -urN mailutils-1.2.orig/comsat/comsat.c mailutils-1.2/comsat/comsat.c
--- mailutils-1.2.orig/comsat/comsat.c 2007-06-27 05:07:16.000000000 -0700
+++ mailutils-1.2/comsat/comsat.c 2007-09-21 01:38:05.000000000 -0700
@@ -584,9 +584,21 @@
exit (1);
}
- setgid (pw->pw_gid);
- setuid (pw->pw_uid);
- chdir (pw->pw_dir);
+ if (setgid (pw->pw_gid))
+ {
+ syslog (LOG_CRIT, _("Cannot set GID %d for user %s: %m"), pw->pw_gid,
user);
+ exit (1);
+ }
+ if (setuid (pw->pw_uid))
+ {
+ syslog (LOG_CRIT, _("Cannot set UID %d for user %s: %m"), pw->pw_uid,
user);
+ exit (1);
+ }
+ if(chdir (pw->pw_dir))
+ {
+ syslog (LOG_CRIT, _("Cannot chdir to %s for user %s: %m"), pw->pw_dir,
user);
+ exit (1);
+ }
username = user;
}
_______________________________________________
Bug-mailutils mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-mailutils
