Hi!

On Wed, 2019-02-06 at 19:58:52 +0100, Andreas Metzler wrote:
> Package: dpkg
> Version: 1.19.3
> Severity: important

> With 1.19.3 the following command stopped working:
> /sbin/start-stop-daemon --stop --retry 5 --quiet --pidfile /run/exim4/exim.pid
> /sbin/start-stop-daemon: matching only on non-root pidfile 
> /run/exim4/exim.pid is insecure
> 
> Afaict this broke exim #921326, amavisd-new #921016 and mldonkey-server
> #920466.

Ouch. :/

> dpkg's changelog.Debian says:
>  * start-stop-daemon: Check whether standalone --pidfile use is secure.
>     Prompted by Michael Orlitzky <mich...@orlitzky.com>.
> 
> the regular changelog is more verbose:
> ------------------------
> commit bc9736f6feae7625cc5ec063ea1b27d51a5f9317
> Author: Guillem Jover <guil...@debian.org>
> Date:   Sat Sep 22 12:12:05 2018 +0200
> 
>     s-s-d: Check whether standalone --pidfile use is secure
> 
>     If we are only matching on the pidfile, which is owned by a non-root
>     user, and we are running as a root user then this is a security risk,
>     and the contents cannot be trusted, because the daemon might have been
>     compromised which would allow modifying the pid within.
> 
>     If we are then calling start-stop-daemon as a privileged user, that
>     would allow acting on any PID in the system.
> 
>     Prompted-by: Michael Orlitzky <mich...@orlitzky.com>
>     Ref: https://redmine.kannel.org/issues/771
> ------------------------
> 
> However the manpage was not updated. Could you please describe which
> restrictions were added, what behavior I can rely on to work?

The man page had been updated in a previous release (1.19.1):

,---
commit 7afd25e72f447a4a4b130f38bba28ff6661cbb6c
Author: Guillem Jover <guil...@debian.org>
Date:   Fri Sep 14 21:35:16 2018 +0200

    man: Add a warning in s-s-d(8) about using --pidfile alone with non-root 
files

    Prompted-by: Michael Orlitzky <mich...@orlitzky.com>
    Ref: https://redmine.kannel.org/issues/771

diff --git a/debian/changelog b/debian/changelog
index ef297822c..2d2cf974c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -204,6 +204,8 @@ dpkg (1.19.1) UNRELEASED; urgency=medium
     - Fix man page markup. Closes: #900033, #900035, #900040
       Thanks to Bjarni Ingi Gislason <bjarn...@rhi.hi.is>.
     - Fix Doxygen comment for libdpkg dpkg_arch_find() function.
+    - Document the dangers of using start-stop-daemon(8) only with --pidfile
+      as matching option with the pid file owned by a non-privileged user.
   * Code internals:
     - Do not use stringy eval to define different sub implementations,
       just assign an anonymous sub to the typeglob.
diff --git a/man/start-stop-daemon.man b/man/start-stop-daemon.man
index cb01cff19..2391d5e9f 100644
--- a/man/start-stop-daemon.man
+++ b/man/start-stop-daemon.man
@@ -116,9 +116,17 @@ Check for a process with the specified parent pid 
\fIppid\fP
 The \fIppid\fP must be a number greater than 0.
 .TP
 .BR \-p ", " \-\-pidfile " \fIpid-file\fP"
-Check whether a process has created the file \fIpid-file\fP. Note: using this
-matching option alone might cause unintended processes to be acted on, if the
-old process terminated without being able to remove the \fIpid-file\fP.
+Check whether a process has created the file \fIpid-file\fP.
+.IP
+Note: using this matching option alone might cause unintended processes to
+be acted on, if the old process terminated without being able to remove the
+\fIpid-file\fP.
+.IP
+\fBWarning:\fP Using this match option alone with a daemon that writes the
+pidfile as an unprivileged user is a security risk, because if the daemon
+gets compromised the contents of the pidfile cannot be trusted, and then
+a privileged runner (such as an init script executed as root) would end up
+acting on any system process.
 .TP
 .BR \-x ", " \-\-exec " \fIexecutable\fP"
 Check for processes that are instances of this \fIexecutable\fP. The
`---

Is that not enough?

> For further entertainment exim does not use start-stop-daemon directly
> but uses lsb, which seems to translate
> killproc -p /run/exim4/exim.pid /usr/sbin/exim4
> to
> /sbin/start-stop-daemon --stop --retry 5 --quiet --pidfile /run/exim4/exim.pid
> 
> dropping the daemon name somewhere.

Hrrm, given that lsb is pretty much abandoned, and the maintainer has
stated in the past little interest in touching the sysvinit script
parts, it might be quickest perhaps to switch to use s-s-d directly?

> I would appreciate if you could agree to keep this dpkg update put of
> testing a little bit to be able to solve this.

I was planning a new upload during this week (the version in sid, will
not migrate as is due to a regression in dgit's autopkgtests), but can
postpone it a few days until you've uploaded exim.

Thanks,
Guillem

Reply via email to