Debian Bug report logs - #24326
sysklogd: There should be a way to run sysklogd without root privileges (please revert this NMU)

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Topi Miettinen <tom@medialab.sonera.net>

To: submit@bugs.debian.org

Subject: sysklogd: syslogd shouldn't run as root

Date: Wed, 8 Jul 1998 00:18:29 +0300

Package: sysklogd
Version: 1.3-26
Severity: wishlist

Syslogd does not need superuser privileges except for startup. Klogd needs 
privileges to open /proc/kmsg, but in current kernels (2.0.34, 2.1.107)
non-root reading from the file descriptor fails. That should eventually
be fixed.

This patch implements a new flag, -u user which causes syslogd and klogd
to call setuid(user).

diff -ru ./klogd.c.orig ./klogd.c
--- ./klogd.c.orig	Sun Jun 28 23:22:48 1998
+++ ./klogd.c	Sun Jun 28 23:26:49 1998
@@ -216,6 +216,7 @@
 #include <stdarg.h>
 #include <paths.h>
 #include <stdlib.h>
+#include <pwd.h>
 #include "klogd.h"
 #include "ksyms.h"
 #include "pidfile.h"
@@ -240,7 +241,7 @@
 static char	*PidFile = "/etc/klogd.pid";
 #endif
 
-static int	kmsg,
+static int	kmsg = -1,
 		change_state = 0,
 		terminate = 0,
 		caught_TSTP = 0,
@@ -490,7 +491,7 @@
 		return(kernel);
 	}
 	
-	if ( (kmsg = open(_PATH_KLOG, O_RDONLY)) < 0 )
+	if ( kmsg == -1 && (kmsg = open(_PATH_KLOG, O_RDONLY)) < 0 )
 	{
 		fprintf(stderr, "klogd: Cannot open proc file system, " \
 			"%d - %s.\n", errno, strerror(errno));
@@ -878,10 +879,13 @@
 
 	auto char	*log_level = (char *) 0,
 			*output = (char *) 0;
+	uid_t		uid = 0;
+	gid_t		gid;
+	struct passwd	*pw;
 
 	chdir ("/");
 	/* Parse the command-line. */
-	while ((ch = getopt(argc, argv, "c:df:iIk:nopsvx")) != EOF)
+	while ((ch = getopt(argc, argv, "c:df:iIk:nopsu:vx")) != EOF)
 		switch((char)ch)
 		{
 		    case 'c':		/* Set console message level. */
@@ -915,6 +919,15 @@
 		    case 's':		/* Use syscall interface. */
 			use_syscall = 1;
 			break;
+		    case 'u':
+			pw = getpwnam(optarg);
+			if (!pw) {
+                                printf("Bad user name %s\n", optarg);
+                                break;
+                        }
+                        uid = pw->pw_uid;
+                        gid = pw->pw_gid;
+			break;
 		    case 'v':
 			printf("klogd %s-%s\n", VERSION, PATCHLEVEL);
 			exit (1);
@@ -1044,6 +1057,11 @@
 	if (symbol_lookup) {
 		InitKsyms(symfile);
 		InitMsyms();
+	}
+
+	if (uid > 0) {
+		setgid(gid);
+		setuid(uid);
 	}
 
         /* The main loop. */
diff -ru ./syslogd.c.orig ./syslogd.c
--- ./syslogd.c.orig	Sun Jun 28 23:22:49 1998
+++ ./syslogd.c	Tue Jun 23 15:07:01 1998
@@ -400,6 +400,7 @@
 #include <arpa/nameser.h>
 #include <arpa/inet.h>
 #include <resolv.h>
+#include <pwd.h>
 #include "pidfile.h"
 #include "version.h"
 
@@ -700,9 +701,12 @@
 	char line[MAXLINE +1];
 	extern int optind;
 	extern char *optarg;
+	uid_t uid = 0;
+	gid_t gid;
+	struct passwd *pw;
 
 	chdir ("/");
-	while ((ch = getopt(argc, argv, "dhf:l:m:np:rs:v")) != EOF)
+	while ((ch = getopt(argc, argv, "dhf:l:m:np:rs:u:v")) != EOF)
 		switch((char)ch) {
 		case 'd':		/* debug */
 			Debug = 1;
@@ -741,6 +745,15 @@
 			}
 			StripDomains = crunch_list(optarg);
 			break;
+		case 'u':		/* user */
+			pw = getpwnam(optarg);
+			if (!pw) {
+				printf("Bad user name %s\n", optarg);
+				break;
+			}
+			uid = pw->pw_uid;
+			gid = pw->pw_gid;
+			break;
 		case 'v':
 			printf("syslogd %s-%s\n", VERSION, PATCHLEVEL);
 			exit (0);
@@ -865,6 +878,11 @@
 	{
 		dprintf("Debugging disabled, SIGUSR1 to turn on debugging.\n");
 		debugging_on = 0;
+	}
+
+	if (uid > 0) {
+		setgid(gid);
+		setuid(uid);
 	}
 
 	/* Main loop begins here. */


--- Begin /etc/init.d/sysklogd (modified conffile)
test -f /sbin/klogd || exit 0
test -f /sbin/syslogd || exit 0
SYSLOGD="-u syslogd"
KLOGD="-u syslogd"
case "$1" in
  start)
    echo -n "Starting system log daemon: syslogd"
    start-stop-daemon --start --quiet --exec /sbin/syslogd -- $SYSLOGD
    echo -n " klogd"
    start-stop-daemon --start --quiet --exec /sbin/klogd -- $KLOGD
    echo "."
    ;;
  stop)
    echo -n "Stopping system log daemon: klogd"
    start-stop-daemon --stop --quiet --pidfile /var/run/klogd.pid
    echo -n " syslogd"
    start-stop-daemon --stop --quiet --pidfile /var/run/syslogd.pid
    echo "."
    ;;
  reload|force-reload)
    start-stop-daemon --stop --quiet --signal 1 --pidfile /var/run/syslogd.pid
    ;;
  restart)
    echo -n "Stopping system log daemon: klogd"
    start-stop-daemon --stop --quiet --pidfile /var/run/klogd.pid
    echo " syslogd"
    start-stop-daemon --stop --quiet --pidfile /var/run/syslogd.pid
    sleep 1
    echo -n "Starting system log daemon: syslogd"
    start-stop-daemon --start --quiet --exec /sbin/syslogd -- $SYSLOGD
    echo -n " klogd"
    start-stop-daemon --start --quiet --exec /sbin/klogd -- $KLOGD
    echo "."
    ;;
  *)
    echo "Usage: /etc/init.d/sysklogd {start|stop|reload|restart|force-reload}"
    exit 1
esac
exit 0

--- End /etc/init.d/sysklogd

--- Begin /etc/cron.daily/sysklogd (modified conffile)
cd /var/log
for LOG in `syslogd-listfiles`
do
   if [ -f $LOG ]; then
      savelog -g adm -m 640 -u syslogd -c 7 $LOG >/dev/null
   fi
done
for LOG in `syslogd-listfiles --auth`
do
   if [ -f $LOG ]; then
      chown syslogd.adm $LOG
      chmod o-rwx $LOG
   fi
done
/etc/init.d/sysklogd reload

--- End /etc/cron.daily/sysklogd

--- Begin /etc/cron.weekly/sysklogd (modified conffile)
cd /var/log
for LOG in `syslogd-listfiles --weekly`
do
   if [ -f $LOG ]; then
      savelog -g adm -m 640 -u syslogd -c 4 $LOG >/dev/null
   fi
done
for LOG in `syslogd-listfiles --auth`
do
   if [ -f $LOG ]; then
      chown syslogd.adm $LOG
      chmod o-rwx $LOG
   fi
done
/etc/init.d/sysklogd reload

--- End /etc/cron.weekly/sysklogd

Message #10 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Richard Kettlewell <rjk@greenend.org.uk>

To: 24326@bugs.debian.org

Subject: removing privilege from sysklogd

Date: Tue, 27 Oct 98 21:16:34 +0000 (GMT)

Just a quick observation about that patch - you must call initgroups()
before you call setuid(uid).

Otherwise sysklogd will still have all of root's additional groups in
its personality, which could give it privileged access that it ought
not to have.

This would represent a security hole (at least compared to the
intended behaviour - it would still be more secure than running as
root all the time!)

Additionally I notice that you've neglected to do any error checking
on the calls to setgid() and setuid().

ttfn/rjk

Message #15 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Topi Miettinen <Topi.Miettinen@medialab.sonera.net>

To: 24326@bugs.debian.org

Subject: klogd -u patch update

Date: Sun, 22 Nov 1998 23:47:25 +0200

Good spotting. This new patch (for klogd only) calls setgroups, setgid and
setuid to change ids, checking return values.

BTW, at least in 2.1.129 /proc/kmsg should work as required.

syslogd needs more thought. The current patch does not work, as there are
several problems:
-restart (signal): reopening sockets will fail due to insufficient
privileges (may be circumvented with checking if the sockets are already
open, unless a config change requires a new privileged inet socket)
-cleanup at exit: removing unix sockets
-log file ownership

-Topi

diff -ru ./klogd.c.orig ./klogd.c
--- ./klogd.c.orig	Sun Nov 22 22:53:24 1998
+++ ./klogd.c	Sun Nov 22 22:55:48 1998
@@ -223,6 +223,8 @@
 #include <stdarg.h>
 #include <paths.h>
 #include <stdlib.h>
+#include <pwd.h>
+#include <grp.h>
 #include "klogd.h"
 #include "ksyms.h"
 #ifndef TESTING
@@ -251,7 +253,7 @@
 #endif
 #endif
 
-static int	kmsg,
+static int	kmsg = -1,
 		change_state = 0,
 		terminate = 0,
 		caught_TSTP = 0,
@@ -508,7 +510,7 @@
 	}
 
 #ifndef TESTING
-	if ( (kmsg = open(_PATH_KLOG, O_RDONLY)) < 0 )
+	if (kmsg == -1 && (kmsg = open(_PATH_KLOG, O_RDONLY)) < 0)
 	{
 		fprintf(stderr, "klogd: Cannot open proc file system, " \
 			"%d - %s.\n", errno, strerror(errno));
@@ -902,12 +904,15 @@
 
 	auto char	*log_level = (char *) 0,
 			*output = (char *) 0;
-
+	uid_t           uid = 0;
+	gid_t           gid = 0;
+	struct passwd   *pw;
+  
 #ifndef TESTING
 	chdir ("/");
 #endif
 	/* Parse the command-line. */
-	while ((ch = getopt(argc, argv, "c:df:iIk:nopsvx")) != EOF)
+	while ((ch = getopt(argc, argv, "c:df:iIk:nopsu:vx")) != EOF)
 		switch((char)ch)
 		{
 		    case 'c':		/* Set console message level. */
@@ -941,6 +946,15 @@
 		    case 's':		/* Use syscall interface. */
 			use_syscall = 1;
 			break;
+		    case 'u':
+			pw = getpwnam(optarg);
+			if (!pw) {
+                                printf("Bad user name %s\n", optarg);
+                                break;
+                        }
+                        uid = pw->pw_uid;
+                        gid = pw->pw_gid;
+			break;
 		    case 'v':
 			printf("klogd %s-%s\n", VERSION, PATCHLEVEL);
 			exit (1);
@@ -1071,6 +1085,15 @@
 	if (symbol_lookup) {
 		InitKsyms(symfile);
 		InitMsyms();
+	}
+
+	if (uid > 0) {
+		if (setgroups(0, NULL) < 0 ||
+		    setgid(gid) < 0 ||
+		    setuid(uid) < 0) {
+			perror("error changing ids");
+			exit(1);
+		}
 	}
 
         /* The main loop. */

Message #20 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Topi Miettinen <Topi.Miettinen@medialab.sonera.fi>

To: 24326@bugs.debian.org

Subject: non-root syslogd

Date: Sun, 04 Apr 1999 23:53:36 +0300

It's better to start execution unprivileged rather than to switch uids later. 
This patch for syslogd allows changing pid file location, which was in the 
end the only problem with that approach. Restarting daemon and exit 
cleanup are no longer a problem.

This way requires that
- /dev/log is a symlink to a daemon-writable location (/var/run/syslogd/log)
- all current log files are writable by the daemon
- savelog scripts recreate log files with suitable ownership and 
permissions

Attached are the patch, /etc/cron.daily/sysklogd, and /etc/init.d/sysklogd.

Same approach could be attempted with klogd, using a new flag for specifying 
alternate /proc/kmsg file with '-' for stdin. Start with:
su - syslogd "klogd -P -" </proc/kmsg

-Topi

diff -ru ./syslogd.c.orig ./syslogd.c
--- ./syslogd.c.orig	Sat Jan 30 19:03:09 1999
+++ ./syslogd.c	Sun Apr  4 23:33:51 1999
@@ -797,7 +797,7 @@
 		funix[i]  = -1;
 	}
 
-	while ((ch = getopt(argc, argv, "a:dhf:l:m:np:rs:v")) != EOF)
+	while ((ch = getopt(argc, argv, "a:dhf:l:m:np:P:rs:v")) != EOF)
 		switch((char)ch) {
 		case 'a':
 			if (nfunix < MAXFUNIX)
@@ -830,6 +830,9 @@
 			break;
 		case 'p':		/* path to regular log socket */
 			funixn[0] = optarg;
+			break;
+		case 'P':		/* path to pid file*/
+			PidFile = optarg;
 			break;
 		case 'r':		/* accept remote messages */
 			AcceptRemote = 1;

#! /bin/sh

# sysklogd	Cron script to rotate system log files daily.
#
#		If you want to rotate other logfiles daily, edit
#		this script.  An easy way is to add them manually
#		or to add -a to syslogd-listfiles and add some grep
#		stuff
#
#		Written by Martin Schulze <joey@debian.org>.
#		$Id: cron.daily,v 1.2 1998/01/02 03:07:21 joey Exp $

cd /var/log
for LOG in `syslogd-listfiles`
do
   if [ -f $LOG ]; then
      savelog -g bin -m 640 -u bin -c 7 $LOG >/dev/null
   fi
done

for LOG in `syslogd-listfiles --all`
do
   if [ -f $LOG ]; then
      chown syslogd.syslogd $LOG
      chmod o-rwx $LOG
   fi
done

# Restart syslogd
#
/etc/init.d/sysklogd reload

#! /bin/sh
# /etc/init.d/sysklogd: start system and kernel log daemons.

test -f /sbin/klogd || exit 0
test -f /sbin/syslogd || exit 0

# Options for start/restart the daemons
#   For remote UDP logging use SYSLOGD="-r"
#
#SYSLOGD="-u syslogd"
SPID="/home/syslogd/syslogd.pid"
SYSLOGD="-p /home/syslogd/log -P $SPID"

#  Use KLOGD="-k /boot/System.map-$(uname -r)" to specify System.map
#
KLOGD="-u syslogd"
KPID="/var/run/klogd.pid"

case "$1" in
  start)
    echo -n "Starting system log daemon: syslogd"
    su - syslogd -c "/sbin/start-stop-daemon --start --quiet --exec /sbin/syslogd -- $SYSLOGD"
    echo -n " klogd"
    start-stop-daemon --start --quiet --exec /sbin/klogd -- $KLOGD
    echo "."
    ;;
  stop)
    echo -n "Stopping system log daemon: klogd"
    start-stop-daemon --stop --quiet --pidfile $KPID
    echo -n " syslogd"
    start-stop-daemon --stop --quiet --pidfile $SPID
    echo "."
    ;;
  reload|force-reload)
    start-stop-daemon --stop --quiet --signal 1 --pidfile $SPID
    ;;
  restart)
    echo -n "Stopping system log daemon: klogd"
    start-stop-daemon --stop --quiet --pidfile $KPID
    echo " syslogd"
    start-stop-daemon --stop --quiet --pidfile $SPID
    sleep 1
    echo -n "Starting system log daemon: syslogd"
    su - syslogd -c "/sbin/start-stop-daemon --start --quiet --exec /sbin/syslogd -- $SYSLOGD"
    echo -n " klogd"
    start-stop-daemon --start --quiet --exec /sbin/klogd -- $KLOGD
    echo "."
    ;;
  *)
    echo "Usage: /etc/init.d/sysklogd {start|stop|reload|restart|force-reload}"
    exit 1
esac

exit 0

Message #29 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Stefano Zacchiroli <zack@debian.org>

To: 24326@bugs.debian.org, 35325@bugs.debian.org

Cc: debian-derivatives@lists.debian.org, Matt Zimmerman <mdz@debian.org>, Martin Pitt <mpitt@debian.org>

Subject: sysklogd: diff for NMU version 1.5-6.1, DELAYED/15

Date: Sun, 24 Apr 2011 10:08:40 +0200

[Message part 1 (text/plain, inline)]

tags 24326 + patch pending
tags 35325 + patch pending
thanks

Hi Martin,
  I've reviewed a patch to sysklogd prepared by Matt Zimmerman, based on
previous work by Martin Pitt, that adds supports for running sysklogd as
non root and enables such a behavior by default.

I believe it's a valuable change with potential security implications;
it's also a well tested one in other derivatives (Ubuntu and all its
descendants). Considering all this, I'm hereby declaring my intention to
sponsor the NMU prepared by Matt. For your convenience and in the hope
to help you out, I'll upload the NMU to DELAYED/15, which won't hit the
archive before 15 days from now. Please let me know if you are fine with
the changes, so that I can adapt the delay.

For all readers: I'm running the modified package without any troubles
on a couple of machines, but more testing is always welcome! You can
find patched packages at <http://people.debian.org/~zack/sysklogd/>.

Please try it out and report any issue.

Cheers.
-- 
Stefano Zacchiroli -o- PhD in Computer Science \ PostDoc @ Univ. Paris 7
zack@{upsilon.cc,pps.jussieu.fr,debian.org} -<>- http://upsilon.cc/zack/
Quando anche i santi ti voltano le spalle, |  .  |. I've fans everywhere
ti resta John Fante -- V. Capossela .......| ..: |.......... -- C. Adams

[sysklogd-1.5-6.1-nmu.diff (text/x-diff, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #34 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Guillem Jover <guillem@debian.org>

To: Stefano Zacchiroli <zack@debian.org>

Cc: 24326@bugs.debian.org, 35325@bugs.debian.org, debian-derivatives@lists.debian.org, Matt Zimmerman <mdz@debian.org>, Martin Pitt <mpitt@debian.org>

Subject: Re: sysklogd: diff for NMU version 1.5-6.1, DELAYED/15

Date: Thu, 5 May 2011 11:54:46 +0200

Hi!

On Sun, 2011-04-24 at 10:08:40 +0200, Stefano Zacchiroli wrote:
> I believe it's a valuable change with potential security implications;
> it's also a well tested one in other derivatives (Ubuntu and all its
> descendants). Considering all this, I'm hereby declaring my intention to
> sponsor the NMU prepared by Matt. For your convenience and in the hope
> to help you out, I'll upload the NMU to DELAYED/15, which won't hit the
> archive before 15 days from now. Please let me know if you are fine with
> the changes, so that I can adapt the delay.

Some nitpicking comments, mostly stuff that does not seem to match the
existing coding style, and then some implementation comments.


> diff -u sysklogd-1.5/syslogd.c sysklogd-1.5/syslogd.c
> --- sysklogd-1.5/syslogd.c
> +++ sysklogd-1.5/syslogd.c
> @@ -878,6 +885,11 @@
>  	extern char *optarg;
>  	int maxfds;
>  
> +        /* user and group id to drop to */
> +        uid_t uid = 0;
> +        gid_t gid = 0;
> +        const char* username = NULL;
> +

This is being indented with spaces, when the rest is using tabs.
The spacing around the asterisk seems inverted.

> @@ -886,7 +898,7 @@
>  		funix[i]  = -1;
>  	}
>  
> -	while ((ch = getopt(argc, argv, "a:dhf:l:m:np:rs:v")) != EOF)
> +	while ((ch = getopt(argc, argv, "a:dhf:l:m:np:rs:vu:")) != EOF)	

Added trailing tab.

>  		switch((char)ch) {
>  		case 'a':
>  			if (nfunix < MAXFUNIX)
> @@ -934,6 +946,21 @@
>  		case 'v':
>  			printf("syslogd %s.%s\n", VERSION, PATCHLEVEL);
>  			exit (0);
> +		case 'u':
> +                        if (optarg) {
> +                                username = strdup (optarg);
> +                                struct passwd *pw = getpwnam (username);

Mixed code and variable declarations is C99, I don't think the code
base has that assumption.

> +                                if (!pw) {
> +                                        fprintf (stderr, "User %s does not exist, aborting.\n", username);
> +                                        exit (1);
> +                                }
> +                                uid = pw->pw_uid;
> +                                gid = pw->pw_gid;
> +                        } else {
> +                                fputs ("Internal error: -u optarg == NULL!\n", stderr);
> +                                exit (1);

Hmm not sure this is really needed? The other cases just assume getopt
works fine.

> +                        }
> +			break;

Spaces → tabs for indentation for this whole block.

>  		case '?':
>  		default:
>  			usage();
> @@ -1087,6 +1114,19 @@
>  		kill (ppid, SIGTERM);
>  #endif
>  
> +        /*
> +         * Drop privileges if -u was specified
> +         */
> +	if (username) {
> +                if (initgroups (username, gid) || 

Trailing space.

> +                    setgid (gid) || setuid (uid)) {
> +                        perror ("Could not drop to specified user privileges");
> +                        exit (1);
> +                }
> +                free (username);
> +                username = NULL;
> +        }
> +

Spaces → tabs for indentation.

> @@ -1603,10 +1643,10 @@
>  	int msglen;
>  	char *timestamp;
>  #ifdef __gnu_linux__
> -	sigset_t mask;
> +        sigset_t mask;
>  #else
>  #ifndef SYSV
> -	sigset_t omask;
> +        sigset_t omask;
>  #endif
>  #endif
>  
> @@ -1618,9 +1658,9 @@
>  	sigaddset(&mask, SIGALRM);
>  	sigprocmask(SIG_BLOCK, &mask, NULL);
>  #else
> -#ifndef SYSV
> +#  ifndef SYSV
>  	omask = sigblock(sigmask(SIGHUP)|sigmask(SIGALRM));
> -#endif
> +#  endif
>  #endif
>  
>  	/*
> @@ -1660,11 +1700,11 @@
>  			f->f_file = -1;
>  		}
>  #ifdef __gnu_linux__
> -		sigprocmask(SIG_UNBLOCK, &mask, NULL);
> +	sigprocmask(SIG_UNBLOCK, &mask, NULL);
>  #else
> -#ifndef SYSV
> +#  ifndef SYSV
>  		(void) sigsetmask(omask);
> -#endif
> +#  endif
>  #endif
>  		return;
>  	}
> @@ -1731,9 +1771,9 @@
>  #ifdef __gnu_linux__
>  	sigprocmask(SIG_UNBLOCK, &mask, NULL);
>  #else
> -#ifndef SYSV
> +#  ifndef SYSV
>  	(void) sigsetmask(omask);
> -#endif
> +#  endif
>  #endif
>  }
>  #if FALSE

All this previous hunks seem to have suffered space damage.

> diff -u sysklogd-1.5/debian/rc.klogd sysklogd-1.5/debian/rc.klogd
> --- sysklogd-1.5/debian/rc.klogd
> +++ sysklogd-1.5/debian/rc.klogd
> @@ -12,37 +12,55 @@
>  
>  PATH=/bin:/usr/bin:/sbin:/usr/sbin
>  
> -pidfile=/var/run/klogd.pid
> +pidfile=/var/run/klogd/klogd.pid
> +kmsgpipe=/var/run/klogd/kmsg
> +kmsgpidfile=/var/run/klogd/kmsgpipe.pid
>  binpath=/sbin/klogd
>  
>  test -f $binpath || exit 0
>  
> -test ! -r /etc/default/klogd || . /etc/default/klogd
> -
>  . /lib/lsb/init-functions
>  
> +#  Use KLOGD="-k /boot/System.map-$(uname -r)" to specify System.map
> +#
> +KLOGD="-P $kmsgpipe"
> +
> +test ! -r /etc/default/klogd || . /etc/default/klogd
> +
>  case "$1" in
>    start)
>      log_begin_msg "Starting kernel log daemon..."
> -    start-stop-daemon --start --quiet --pidfile $pidfile --name klogd --startas $binpath -- $KLOGD
> +    # create klog-writeable pid and fifo directory
> +    mkdir -p /var/run/klogd
> +    chown klog:klog /var/run/klogd
> +    mkfifo -m 700 $kmsgpipe
> +    chown klog:klog $kmsgpipe
> + 
> +    # shovel /proc/kmsg to pipe readable by klogd user
> +    start-stop-daemon --start --pidfile $kmsgpidfile --exec /bin/dd -b -m -- bs=1 if=/proc/kmsg of=$kmsgpipe
> + 
> +    # start klogd as non-root with reading from kmsgpipe
> +    start-stop-daemon --start --quiet --chuid klog --exec $binpath -- $KLOGD
>      log_end_msg $?
> -    test -d /lib/init/rw/sendsigs.omit.d || mkdir -p /lib/init/rw/sendsigs.omit.d
> -    test ! -f /lib/init/rw/sendsigs.omit.d/klogd || rm -f /lib/init/rw/sendsigs.omit.d/klogd
> -    ln -s $pidfile /lib/init/rw/sendsigs.omit.d/klogd
>      ;;

Hmmm, this seems pretty ugly to me, why not just change the owner of
/proc/kmsg instead, which avoids that daemonized dd?

> diff -u sysklogd-1.5/debian/rc sysklogd-1.5/debian/rc
> --- sysklogd-1.5/debian/rc
> +++ sysklogd-1.5/debian/rc
> @@ -19,8 +19,31 @@
>  
>  test -x $binpath || exit 0
>  
> +# syslogd options should be set in /etc/default/syslogd
> +SYSLOGD=""
> +
> +# user to run syslogd as - this can overriden in /etc/default/syslogd
> +USER="syslog"
> +
>  test ! -r /etc/default/syslogd || . /etc/default/syslogd
>  
> +# Figure out under which user syslogd should be running as
> +if echo ${SYSLOGD} | grep -q '^.*-u[[:space:]]*\([[:alnum:]]*\)[[:space:]]*.*$'
> +then
> +	# A specific user has been set on the command line, try to extract it.
> +	USER=$(echo ${SYSLOGD} | sed -e 's/^.*-u[[:space:]]*\([[:alnum:]]*\)[[:space:]]*.*$/\1/')
> +else
> +	# By default, run syslogd under the syslog user
> +	SYSLOGD="${SYSLOGD} -u ${USER}"
> +fi

This seems a bit ugly, why is this needed instead of just relying on
USER being properly set? The -u option is new anyway so it's not
possible it was previously set in SYSLOGD.

> +# Unable to get the user under which syslogd should be running, stop.
> +if [ -z "${USER}" ]
> +then
> +	log_failure_msg "Unable to get syslog user"
> +	exit 1
> +fi
> +
>  . /lib/lsb/init-functions
>  
>  create_xconsole()
> @@ -36,8 +59,18 @@
[...]
> +fix_log_ownership()
> +{
> +	for l in `syslogd-listfiles -a`
> +	do
> +		chown ${USER}:adm $l
> +	done
>  }

Why is this needed?

> --- sysklogd-1.5.orig/klogd.c
> +++ sysklogd-1.5/klogd.c
> @@ -313,6 +322,8 @@
>  
>  static FILE *output_file = (FILE *) 0;
>  
> +static char     *kmsg_file = NULL; /* NULL means default /proc/kmsg */
> +

I guess either trimming spaces or using tabs?

> @@ -543,6 +554,22 @@
>  		       "console output.");
>  	}
>  
> +        /* Do we read kernel messages from a pipe? */
> +        if ( kmsg_file ) {
> +                if ( !strcmp(kmsg_file, "-") )

I always find clearer to use strcmp() == 0, instead of !strcmp().

> +                        kmsg = fileno(stdin);
> +                else {
> +                        if ( (kmsg = open(kmsg_file, O_RDONLY)) < 0 )
> +                        {
> +                                fprintf(stderr, "klogd: Cannot open kmsg file, " \
> +                                        "%d - %s.\n", errno, strerror(errno));
> +                                ksyslog(7, NULL, 0);
> +                                exit(1);
> +                        }
> +                }
> +                return proc;
> +        }
> +

Spaces → tab indentation. This does not send the klogd started
message to syslog. It might make sense to refactor the open() logic
at the end of the function, which is pretty similar.

>  	/*
>  	 * First do a stat to determine whether or not the proc based
>  	 * file system is available to get kernel messages from.
> @@ -1024,6 +1051,9 @@
>  		    case 'p':
>  			SetParanoiaLevel(1);	/* Load symbols on oops. */
>  			break;	
> +                    case 'P':           /* Alternative kmsg file path */
> +                        kmsg_file = strdup(optarg);
> +                        break;

Spaces → tab indentation.

>  		    case 's':		/* Use syscall interface. */
>  			use_syscall = 1;
>  			break;
> @@ -1035,7 +1065,6 @@
>  			break;
>  		}
>  
> -
>  	/* Set console logging level. */
>  	if ( log_level != (char *) 0 )
>  	{

Unneeded line removal.

regards,
guillem

Message #39 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Stefano Zacchiroli <zack@debian.org>

To: Guillem Jover <guillem@debian.org>, 24326@bugs.debian.org, 35325@bugs.debian.org, Matt Zimmerman <mdz@debian.org>, Martin Pitt <mpitt@debian.org>

Cc: debian-derivatives@lists.debian.org

Subject: Re: sysklogd: diff for NMU version 1.5-6.1, DELAYED/15

Date: Fri, 6 May 2011 14:16:00 +0200

[Message part 1 (text/plain, inline)]

On Thu, May 05, 2011 at 11:54:46AM +0200, Guillem Jover wrote:
> Some nitpicking comments, mostly stuff that does not seem to match the
> existing coding style, and then some implementation comments.

Thanks a lot for the review, Guillem. I've looked into it and I see no
blockers for letting the DELAYED NMU go ahead. Nonetheless, if Martin or
Matt want to update the patch to take into account some of your
comments, I'll be happy to sponsor another upload with the updated
patch.

Let me know,
Cheers.
-- 
Stefano Zacchiroli -o- PhD in Computer Science \ PostDoc @ Univ. Paris 7
zack@{upsilon.cc,pps.jussieu.fr,debian.org} -<>- http://upsilon.cc/zack/
Quando anche i santi ti voltano le spalle, |  .  |. I've fans everywhere
ti resta John Fante -- V. Capossela .......| ..: |.......... -- C. Adams

[signature.asc (application/pgp-signature, inline)]

Message #44 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Martin Pitt <mpitt@debian.org>

To: Stefano Zacchiroli <zack@debian.org>

Cc: Guillem Jover <guillem@debian.org>, 24326@bugs.debian.org, 35325@bugs.debian.org, Matt Zimmerman <mdz@debian.org>, debian-derivatives@lists.debian.org

Subject: Re: sysklogd: diff for NMU version 1.5-6.1, DELAYED/15

Date: Tue, 10 May 2011 10:46:18 +0200

[Message part 1 (text/plain, inline)]

Hello all,

sorry for the late answer, I didn't have much time for Debian
recently.

Stefano Zacchiroli [2011-05-06 14:16 +0200]:
> Thanks a lot for the review, Guillem. I've looked into it and I see no
> blockers for letting the DELAYED NMU go ahead. Nonetheless, if Martin or
> Matt want to update the patch to take into account some of your
> comments, I'll be happy to sponsor another upload with the updated
> patch.

TBH it's pretty obsolete. rsyslog is the standard logging daemon now,
and since then the kernel also has been fixed to not require root
privileges for reading (not opening) from /proc/kmsg any more (which
was the reason for the dd process). So rsyslog never had the nasty dd
hack, since at that point we rather got the kernel fixed.

The same approach could now be done for klogd, i. e. open /proc/kmsg
as root, then drop to the system user afterwards, and keep running.
So I honestly think we should just bury this patch and also revert
what's left of it in the Ubuntu package.

Thanks,

Martin

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

[signature.asc (application/pgp-signature, inline)]

Message #49 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Matt Zimmerman <mdz@debian.org>

To: Martin Pitt <mpitt@debian.org>

Cc: Stefano Zacchiroli <zack@debian.org>, Guillem Jover <guillem@debian.org>, 24326@bugs.debian.org, 35325@bugs.debian.org, debian-derivatives@lists.debian.org

Subject: Re: sysklogd: diff for NMU version 1.5-6.1, DELAYED/15

Date: Tue, 10 May 2011 11:36:09 +0200

On Tue, May 10, 2011 at 10:46:18AM +0200, Martin Pitt wrote:
> Hello all,
> 
> sorry for the late answer, I didn't have much time for Debian
> recently.

Unfortunate timing, as the patch has now landed in Debian. :-/

> 
> Stefano Zacchiroli [2011-05-06 14:16 +0200]:
> > Thanks a lot for the review, Guillem. I've looked into it and I see no
> > blockers for letting the DELAYED NMU go ahead. Nonetheless, if Martin or
> > Matt want to update the patch to take into account some of your
> > comments, I'll be happy to sponsor another upload with the updated
> > patch.
> 
> TBH it's pretty obsolete. rsyslog is the standard logging daemon now,
> and since then the kernel also has been fixed to not require root
> privileges for reading (not opening) from /proc/kmsg any more (which
> was the reason for the dd process). So rsyslog never had the nasty dd
> hack, since at that point we rather got the kernel fixed.

The kernel has been fixed in Ubuntu, in Debian, or in upstream?

-- 
 - mdz

Message #54 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Martin Pitt <mpitt@debian.org>

To: Matt Zimmerman <mdz@debian.org>, Stefano Zacchiroli <zack@debian.org>, Guillem Jover <guillem@debian.org>, 24326@bugs.debian.org, 35325@bugs.debian.org, debian-derivatives@lists.debian.org

Subject: Re: sysklogd: diff for NMU version 1.5-6.1, DELAYED/15

Date: Tue, 10 May 2011 12:09:23 +0200

[Message part 1 (text/plain, inline)]

Matt Zimmerman [2011-05-10 11:36 +0200]:
> The kernel has been fixed in Ubuntu, in Debian, or in upstream?

All of them by now. The fix got landed upstream in February 2010:

  http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=002345925e6c45861f60db6f4fc6236713fd8847

I think it landed in 2.6.35.

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

[signature.asc (application/pgp-signature, inline)]

Message #67 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Roger Leigh <rleigh@codelibre.net>

To: Martin Pitt <mpitt@debian.org>, 35325@bugs.debian.org

Cc: Matt Zimmerman <mdz@debian.org>, Stefano Zacchiroli <zack@debian.org>, Guillem Jover <guillem@debian.org>, 24326@bugs.debian.org, debian-derivatives@lists.debian.org

Subject: Re: Bug#35325: sysklogd: diff for NMU version 1.5-6.1, DELAYED/15

Date: Tue, 13 Dec 2011 19:05:09 +0000

[Message part 1 (text/plain, inline)]

On Tue, May 10, 2011 at 12:09:23PM +0200, Martin Pitt wrote:
> Matt Zimmerman [2011-05-10 11:36 +0200]:
> > The kernel has been fixed in Ubuntu, in Debian, or in upstream?
> 
> All of them by now. The fix got landed upstream in February 2010:
> 
>   http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=002345925e6c45861f60db6f4fc6236713fd8847
> 
> I think it landed in 2.6.35.

As a result of this, the following patch is proposed to correct this
change by reverting the previous NMU.  This also cleans up by
deleting the users and changing the ownership of the files back to
root:root.  Comments?

It might need additional logic to remove /var/run/klog after
restarting the daemon.


Regards,
Roger

-- 
  .''`.  Roger Leigh
 : :' :  Debian GNU/Linux             http://people.debian.org/~rleigh/
 `. `'   Printing on GNU/Linux?       http://gutenprint.sourceforge.net/
   `-    GPG Public Key: 0x25BFB848   Please GPG sign your mail.

[sysklogd-revert-nmu.patch (text/x-diff, attachment)]

Message #72 received at 24326@bugs.debian.org (full text, mbox, reply):

From: Stefano Zacchiroli <zack@debian.org>

To: Roger Leigh <rleigh@codelibre.net>

Cc: Martin Pitt <mpitt@debian.org>, 35325@bugs.debian.org, Matt Zimmerman <mdz@debian.org>, Guillem Jover <guillem@debian.org>, 24326@bugs.debian.org, debian-derivatives@lists.debian.org

Subject: Re: Bug#35325: sysklogd: diff for NMU version 1.5-6.1, DELAYED/15

Date: Tue, 13 Dec 2011 21:45:54 +0100

On Tue, Dec 13, 2011 at 07:05:09PM +0000, Roger Leigh wrote:
> As a result of this, the following patch is proposed to correct this
> change by reverting the previous NMU.  This also cleans up by
> deleting the users and changing the ownership of the files back to
> root:root.  Comments?

No objection on my part, of course. And apologies for acting on this
with the previous NMU well after it was actually useful to do so. I
doubt you need my help to proceed, but in case you do (e.g. for
testing), let me know

> It might need additional logic to remove /var/run/klog after
> restarting the daemon.

Let me point out that all this is about sysklogd, which is no longer the
default syslog on Debian since quite a while. (Which explains the steep
drop of popularity of the package in popularity contest.) That is no
reason to not clean up what has been introduced by previous changes.
Nonetheless if we really want to clean up, it'd probably make sense to
add some warning/hint that suggests to migrate to rsyslog (even only if
in NEWS.Debian), our default syslog since quite a while.

Cheers.
-- 
Stefano Zacchiroli     zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
Maître de conférences   ......   http://upsilon.cc/zack   ......   . . o
Debian Project Leader    .......   @zack on identi.ca   .......    o o o
« the first rule of tautology club is the first rule of tautology club »

Message #77 received at 24326-done@bugs.debian.org (full text, mbox, reply):

From: Debian FTP Masters <ftpmaster@ftp-master.debian.org>

To: 24326-done@bugs.debian.org,35325-done@bugs.debian.org,38781-done@bugs.debian.org,44523-done@bugs.debian.org,48326-done@bugs.debian.org,59662-done@bugs.debian.org,70429-done@bugs.debian.org,89677-done@bugs.debian.org,91804-done@bugs.debian.org,94285-done@bugs.debian.org,100043-done@bugs.debian.org,104278-done@bugs.debian.org,108653-done@bugs.debian.org,144790-done@bugs.debian.org,147471-done@bugs.debian.org,156232-done@bugs.debian.org,174038-done@bugs.debian.org,199989-done@bugs.debian.org,205239-done@bugs.debian.org,206947-done@bugs.debian.org,222120-done@bugs.debian.org,222245-done@bugs.debian.org,223554-done@bugs.debian.org,226036-done@bugs.debian.org,241657-done@bugs.debian.org,247680-done@bugs.debian.org,253213-done@bugs.debian.org,273269-done@bugs.debian.org,278892-done@bugs.debian.org,279120-done@bugs.debian.org,281448-done@bugs.debian.org,285087-done@bugs.debian.org,308580-done@bugs.debian.org,334295-done@bugs.debian.org,337389-done@bugs.debian.org,353744-done@bugs.debian.org,365265-done@bugs.debian.org,365339-done@bugs.debian.org,370339-done@bugs.debian.org,385292-done@bugs.debian.org,386122-done@bugs.debian.org,419496-done@bugs.debian.org,427415-done@bugs.debian.org,439833-done@bugs.debian.org,449581-done@bugs.debian.org,452559-done@bugs.debian.org,472948-done@bugs.debian.org,487173-done@bugs.debian.org,498777-done@bugs.debian.org,500776-done@bugs.debian.org,513217-done@bugs.debian.org,513218-done@bugs.debian.org,514051-done@bugs.debian.org,515696-done@bugs.debian.org,525959-done@bugs.debian.org,527874-done@bugs.debian.org,532300-done@bugs.debian.org,535498-done@bugs.debian.org,547968-done@bugs.debian.org,557119-done@bugs.debian.org,626244-done@bugs.debian.org,626247-done@bugs.debian.org,627404-done@bugs.debian.org,639872-done@bugs.debian.org,647023-done@bugs.debian.org,648166-done@bugs.debian.org,650428-done@bugs.debian.org,652086-done@bugs.debian.org,680099-done@bugs.debian.org,680170-done@bugs.debian.org,689578-done@bugs.debian.org,689686-done@bugs.debian.org,691700-done@bugs.debian.org,

Cc: sysklogd@packages.debian.org, sysklogd@packages.qa.debian.org

Subject: Bug#707655: Removed package(s) from unstable

Date: Sat, 11 May 2013 10:05:52 +0000

Version: 1.5-6.2+rm

Dear submitter,

as the package sysklogd has just been removed from the Debian archive
unstable we hereby close the associated bug reports.  We are sorry
that we couldn't deal with your issue properly.

For details on the removal, please see http://bugs.debian.org/707655

The version of this package that was in Debian prior to this removal
can still be found using http://snapshot.debian.org/.

This message was generated automatically; if you believe that there is
a problem with it please contact the archive administrators by mailing
ftpmaster@debian.org.

Debian distribution maintenance software
pp.
Ansgar Burchardt (the ftpmaster behind the curtain)

Send a report that this bug log contains spam.