[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#924312: stunnel4: Fails to stop with sysvinit: start-stop-daemon: matching only on non-root pidfile /var/lib/stunnel4///stunnel4.pid is insecure



On Mon, Mar 11, 2019 at 09:28:04PM +0100, Axel Beckert wrote:
> Control: reassign -1 lsb-base
> Control: forcemerge 921558 -1
> Control: affects 921558 + stunnel4
> 
> Hi Paul,
> 
> Paul Gevers wrote:
> > On Mon, 11 Mar 2019 14:15:25 +0100 Axel Beckert <abe@debian.org> wrote:
> > > Version: 3:5.50-3
> > 
> > ^^^^^^^^^^^^^^^^^^^
> > I don't think the bug is only in this version, is it?
> 
> No. But for a reason I did not expect:
> 
> In contrary to the other cases of this issue I run into recently
> (miredo and smokeping), stunnel4's init script doesn't use
> start-stop-daemon directly but via killproc() from
> /lib/lsb/init-functions of lsb-base. And the actual bug seems to be in
> that shell script library:
> 
>     125 # start-stop-daemon uses the same algorithm as "pidofproc" above.
>     126 killproc () {
>     127     local pidfile sig status base name_param is_term_sig OPTIND
>     128     pidfile=
>     129     name_param=
>     130     is_term_sig=
>     131 
>     132     OPTIND=1
>     133     while getopts p: opt ; do
>     134         case "$opt" in
>     135             p)  pidfile="$OPTARG";;
>     136         esac
>     137     done
>     138     shift $(($OPTIND - 1))
>     139 
>     140     base=${1##*/}
>     141     if [ ! $pidfile ]; then
>     142         name_param="--name $base --pidfile /var/run/$base.pid"
>     143     else
>     144         name_param="--pidfile $pidfile"
>     145     fi
>     […]     […]
>     152     status=0
>     153     if [ ! "$is_term_sig" ]; then
>     154         if [ -n "$sig" ]; then
>     155             /sbin/start-stop-daemon --stop --signal "$sig" \
>     156                 --quiet $name_param || status="$?"
>     157         else
>     158             /sbin/start-stop-daemon --stop \
>     159                 --retry 5 \
>     160                 --quiet $name_param || status="$?"
>     161         fi
>     162     else
>     163         /sbin/start-stop-daemon --stop --quiet \
>     164             --oknodo $name_param || status="$?"
>     165     fi
> 
> Since neither name_param nor any of the start-stop-daemon --stop calls
> contain --exec or -x nor does killproc allow to pass additional
> parameters, this looks like a bug in lsb-base. And indeed, it is and
> is also already reported: https://bugs.debian.org/921558
> 
> Reassigning accordingly.

Thanks for reporting this and for your analysis!  I came up with the
same result (didn't reply since I was still going through the Git log to
see in which version of stunnel the use of killproc was introduced) and
I have a patch for stunnel4 now that uses start-stop-daemon directly,
although this would be a divergence from the upstream source - the
upstream author switched to the LSB killproc several years ago after
seeing that I had made that change in Debian... so it would be a pity
to now say "well, yeah, but you see, it doesn't really work, so let's go
back to looking for the list of processes and sending the signals by
ourselves" :)

To be honest, I don't think I should upload stunnel with a patch that
makes the init script use the LSB start_daemon function to start
the process and then uses start-stop-daemon directly to stop it;
I might make another change that uses start-stop-daemon in both places,
but, honestly, I would prefer some kind of change in init-functions that
would let me continue to use the LSB shell functions.  Even if it means
passing an additional argument to killproc to make it use --exec, this
would still be an acceptable divergence from upstream for me.

> > This bug is currently blocking the migration of stunnel4 to testing
> > which is needed to have openssl migrate. I'd like to do that today
> > because tomorrow the full freeze starts.
> 
> Should be solved now. Thanks for poking me so that I had a closer
> look!

Thanks to both of you for raising this and noting its importance!

G'luck,
Peter

-- 
Peter Pentchev  roam@{ringlet.net,debian.org,FreeBSD.org} pp@storpool.com
PGP key:        http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint 2EE7 A7A5 17FC 124C F115  C354 651E EFB0 2527 DF13

Attachment: signature.asc
Description: PGP signature


Reply to: