[Buildroot] [PATCH 2/2] package/suricata: new package

[Fabrice Fontaine]

Hello Thomas,

Le sam. 13 avr. 2019 à 22:53, Thomas Petazzoni
<thomas.petazzoni at bootlin.com> a écrit :
>
> Hello Fabrice,
>
> On Thu, 14 Mar 2019 22:26:00 +0100
> Fabrice Fontaine <fontaine.fabrice at gmail.com> wrote:
>
> > Suricata is a free and open source, mature, fast and robust
> > network threat detection engine.
> >
> > The Suricata engine is capable of real time intrusion
> > detection (IDS), inline intrusion prevention (IPS), network
> > security monitoring (NSM) and offline pcap processing.
> >
> > https://suricata-ids.org
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice at gmail.com>
>
> Overall looks good. I was about to commit, but I have some doubts about
> the systemd unit, and therefore will take advantage of those doubts to
> also make a few comments about other aspects.
>
> > diff --git a/package/suricata/S99suricata b/package/suricata/S99suricata
> > new file mode 100644
> > index 0000000000..35a034b179
> > --- /dev/null
> > +++ b/package/suricata/S99suricata
>
> In terms of init scripts, package/busybox/S02klogd is now the
> "reference". I recommend following this example.
OK, updated in v2
>
> > @@ -0,0 +1,39 @@
> > +#!/bin/sh
> > +
> > +NAME=suricata
> > +PIDFILE=/var/run/$NAME.pid
> > +DAEMON=/usr/bin/$NAME
> > +DAEMON_ARGS="-c /etc/suricata/suricata.yaml -i eth0"
>
> You clearly want to include a /etc/default/${DAEMON} file. DAEMON
> should be just the name of the program, see S02klogd.
>
> > +case "$1" in
> > +  start)
> > +     start
> > +     ;;
> > +  stop)
> > +     stop
> > +     ;;
> > +  restart|reload)
> > +     restart
> > +     ;;
> > +  *)
> > +     echo "Usage: $0 {start|stop|restart}"
> > +     exit 1
>
> Please follow the indentation style of S02klogd.
OK
>
>
> > +ifeq ($(BR2_PACKAGE_PYTHON),y)
> > +SURICATA_CONF_OPTS += --enable-python
> > +SURICATA_DEPENDENCIES += python
> > +else
> > +SURICATA_CONF_OPTS += --disable-python
> > +endif
>
> So only Python 2.x is supported ?
python3 is also supported, I updated v2.
>
> > +ifeq ($(BR2_TOOLCHAIN_HAS_SSP),y)
> > +SURICATA_CONF_OPTS += --enable-gccprotect
> > +else
> > +SURICATA_CONF_OPTS += --disable-gccprotect
> > +endif
>
> We should unconditionally use --disable-gccprotect and let our
> gcc/wrapper pass the appropriate SSP/hardening options.
OK
>
> > diff --git a/package/suricata/suricata.service b/package/suricata/suricata.service
> > new file mode 100644
> > index 0000000000..ca0be02dae
> > --- /dev/null
> > +++ b/package/suricata/suricata.service
> > @@ -0,0 +1,13 @@
> > +[Unit]
> > +Description=Suricata Intrusion Detection Service
> > +After=network.target
> > +
> > +[Service]
> > +ExecStartPre=/bin/rm -f /var/run/suricata.pid
> > +ExecStartPre=/usr/bin/mkdir -p /var/log/suricata
> > +ExecStart=/usr/bin/suricata -c /etc/suricata/suricata.yaml -i eth0 --pidfile /var/run/suricata.pid
> > +ExecReload=/bin/kill -USR2 $MAINPID
>
> I am a bit skeptical about the PID file handling. How is systemd going
> to know that the PID file is /var/run/suricata.pid ? Is this useful in
> the context of systemd ?
I followed the template given by upstream in
https://github.com/OISF/suricata/blob/ec77632e84a106ddbcd0baef4e4368b4fe5c5f9e/etc/suricata.service.in

I updated v2 to add an EnvironmentFile to /etc/default/suricata.
Concerning the PID, it works because $MAINPID is an internal systemd
variable that save the same value than the /var/run/suricata.pid file
because we didn't ask systemd to fork suricata.


>
> I'm by no means not a systemd expert, but this seems weird to me. If a
> systemd-person could give more details about this, it would be nice.
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com