Hi,

well this looks okay. If you would want to clean this up a little bit more and
add some comments, I would accept it as a patch.

Here, it is not an option to call killproc without the PID file since it would
kill all running instances of snort at once. We usually always do this anyway
though.

-Michael

On Mon, 2017-04-24 at 13:57 +0200, Matthias Fischer wrote:
> Hi,
> 
> On 24.04.2017 12:24, Michael Tremer wrote:
> > Hi,
> > 
> > yes indeed.
> > 
> > But is not 100% necessary to pass the -p parameter with the PID file. I just
> > removed that when ever it got difficult to use and did not even add that for
> > new
> > scripts.
> 
> I noticed that. 'killproc' is used in two different ways.
> 
> 'killproc -p' is only used in four init-scripts (dhcp, dhcrelay, rngd
> and snort). All other scripts use 'killproc [PROGRAMNAME]', sometimes
> 'killproc [PATH][PROGRAMNAME].
> 
> As I'm not really sure about this: which solution should we prefer?
> 
> > But using $DEVICE is definitely wrong in the snort script.
> 
> Ok - but as far as I can see, this is working?
> 
> Improving this would lead to changing the complete 'start'- and
> 'stop'-section:
> 
> Example:
> ...
>  stop)
>   DEVICES=""
>   if [ -r /var/run/snort_$BLUE_DEV.pid ]; then
>    DEVICES+="$BLUE_DEV "
>   fi
>   if [ -r /var/run/snort_$GREEN_DEV.pid ]; then
>    DEVICES+="$GREEN_DEV "
>   fi
> 
>   if [ -r /var/run/snort_$ORANGE_DEV.pid ]; then
>    DEVICES+="$ORANGE_DEV "
>   fi
> 
>   RED=`cat /var/ipfire/red/iface 2>/dev/null`
>   if [ -r /var/run/snort_$RED.pid ]; then
>    DEVICES+=`cat /var/ipfire/red/iface 2>/dev/null`
>   fi
> 
>    for DEVICE in $DEVICES; do
>     boot_mesg "Stopping Intrusion Detection System on $DEVICE..."
>     killproc -p /var/run/snort_$DEVICE.pid /var/run
>    done
> ...
> 
> The whole thing began because I wanted a 'reload' section for 'snort'
> for use after automatic rule updates, which seems to work:
> 
> ...
>  reload)
>   DEVICES=""
>   if [ -r /var/run/snort_$BLUE_DEV.pid ]; then
>    DEVICES+="$BLUE_DEV "
>   fi
> 
>   if [ -r /var/run/snort_$GREEN_DEV.pid ]; then
>    DEVICES+="$GREEN_DEV "
>   fi
> 
>   if [ -r /var/run/snort_$ORANGE_DEV.pid ]; then
>    DEVICES+="$ORANGE_DEV "
>   fi
> 
>   RED=`cat /var/ipfire/red/iface 2>/dev/null`
>    if [ -r /var/run/snort_$RED.pid ]; then
>     DEVICES+=`cat /var/ipfire/red/iface 2>/dev/null`
>    fi
> 
>   for DEVICE in $DEVICES; do
>    boot_mesg "Reloading Intrusion Detection System on $DEVICE..."
>    /bin/kill -SIGHUP `cat /var/run/snort_$DEVICE.pid`
>    evaluate_retval
>   done
>   ;;
> ...
> 
> Any better solution is welcome... ;-))
> 
> Best,
> 
> Matthias
> 
> > -Michael
> > 
> > On Sun, 2017-04-23 at 19:20 +0200, Matthias Fischer wrote:
> > > Hi,
> > > 
> > > while searching for something else I found this:
> > > 
> > > Usage for the 'killproc'-function in '/etc/init.d/'-files should be
> > > (cited):
> > > 
> > > "# Function - killproc  [-p pidfile] pathname [signal]
> > > #
> > > # Purpose:
> > > #
> > > # Inputs: -p pidfile, uses the specified pidfile
> > > #         pathname, pathname to the specified program
> > > ..."
> > > 
> > > But in the 'init'-files for 'dhcp', 'dhcrelay' and 'rndg' there is the
> > > PROGRAMname and in 'snort'-file '/var/run' is given. The latter leads to
> > > an error (FAIL) if both 'green0' and 'red0' are enabled and should be
> > > both stopped:
> > > 
> > > ...
> > > killproc -p /var/run/dhcpd.pid /usr/sbin/dhcpd
> > >                                         ^^^^^^
> > > ...
> > > killproc -p /var/run/dhcrelay.pid /usr/sbin/dhcrelay
> > >                                            ^^^^^^^^^
> > > ...
> > > killproc -p /var/run/rngd.pid /usr/sbin/rngd
> > >                                        ^^^^^
> > > ...
> > > killproc -p /var/run/snort_$DEVICE.pid /var/run
> > >                                        ^^^^^^^^
> > > 
> > > IMHO, all these should be changed to "pathname to the specified program"
> > > as cited above ('/usr/sbin').
> > > 
> > > Can anyone please confirm?
> > > 
> > > Best,
> > > Matthias
> 
>