Date: Mon, 12 Oct 2015 14:21:37 +0200
From: Natanael Copa <ncopa@alpinelinux.org>
To: Valery Kartel <valery.kartel@gmail.com>
Cc: alpine-aports@lists.alpinelinux.org
Subject: Re: [alpine-aports] [PATCH] main/net_snmp: init scripts cleanup and
 modify configs to run snmpd & snmptrapd out from the box
Message-ID: <20151012142137.42142be7@ncopa-desktop.alpinelinux.org>
In-Reply-To: <1443621912-29586-1-git-send-email-valery.kartel@gmail.com>
References: <1443621912-29586-1-git-send-email-valery.kartel@gmail.com>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Wed, 30 Sep 2015 17:05:12 +0300
Valery Kartel <valery.kartel@gmail.com> wrote:

> ---
>  main/net-snmp/APKBUILD        | 33 +++++++++++++++------------------
>  main/net-snmp/initd           | 19 +++++++++++++++++++
>  main/net-snmp/snmpd.confd     |  6 +++---
>  main/net-snmp/snmpd.initd     | 37 -------------------------------------
>  main/net-snmp/snmptrapd.confd |  6 +++---
>  main/net-snmp/snmptrapd.initd | 23 -----------------------
>  6 files changed, 40 insertions(+), 84 deletions(-)
>  create mode 100644 main/net-snmp/initd
>  delete mode 100644 main/net-snmp/snmpd.initd
>  delete mode 100644 main/net-snmp/snmptrapd.initd
> 
> diff --git a/main/net-snmp/APKBUILD b/main/net-snmp/APKBUILD
> index 3c0c455..f7ccf81 100644
> --- a/main/net-snmp/APKBUILD
> +++ b/main/net-snmp/APKBUILD
> @@ -2,7 +2,7 @@
>  # Maintainer: Carlo Landmeter <clandmeter@gmail.com>
>  pkgname=net-snmp
>  pkgver=5.7.3
> -pkgrel=3
> +pkgrel=4
>  pkgdesc="Simple Network Management Protocol"
>  url="http://www.net-snmp.org/"
>  arch="all"
> @@ -19,9 +19,8 @@ source="http://downloads.sourceforge.net/$pkgname/$pkgname-$pkgver.tar.gz
>  	fix-includes.patch
>  	CVE-2015-5621.patch
>  
> -	snmpd.initd
> +	initd

I would like to call it snmpd.initd. I sometimes grep stuff */*.initd.

>  	snmpd.confd
> -	snmptrapd.initd
>  	snmptrapd.confd
>  	"
>  
> @@ -92,11 +91,12 @@ package() {
>  		|| return 1
>  	ln -s snmptrap "$pkgdir"/usr/bin/snmpinform || return 1
>  
> -	install -m755 -D "$srcdir"/snmpd.initd "$pkgdir"/etc/init.d/snmpd
> +	install -m755 -D "$srcdir"/initd "$pkgdir"/etc/init.d/snmpd
> +	install -m755 -D "$srcdir"/initd "$pkgdir"/etc/init.d/snmptrapd

Since snmpd and snmptrapd init script is identical, maybe we should
just symlink it?

>  	install -m644 -D "$srcdir"/snmpd.confd "$pkgdir"/etc/conf.d/snmpd
> -	install -m755 -D "$srcdir"/snmptrapd.initd "$pkgdir"/etc/init.d/snmptrapd
>  	install -m644 -D "$srcdir"/snmptrapd.confd "$pkgdir"/etc/conf.d/snmptrapd
> -	install -m644 -D EXAMPLE.conf "$pkgdir"/etc/snmp/snmpd.conf.example
> +	install -m644 -D EXAMPLE.conf "$pkgdir"/etc/snmp/snmpd.conf
> +	echo "authCommunity log,execute,net public" > "$pkgdir"/etc/snmp/snmptrapd.conf

Those example configs, are they secure by default? We want a default
install be secure and let user enable stuff he needs rather than the
opposite, that things works by default but user need to disable stuff
or harden it afterwards.

>  	mkdir -p "$pkgdir"/var/lib/net-snmp
>  	find "$pkgdir" -name perllocal.pod -delete
>  }

...

> diff --git a/main/net-snmp/initd b/main/net-snmp/initd
> new file mode 100644
> index 0000000..3790d77
> --- /dev/null
> +++ b/main/net-snmp/initd
> @@ -0,0 +1,19 @@
> +#!/sbin/openrc-run
> +
> +pidfile="/var/run/${SVCNAME}.pid"
> +command="/usr/sbin/${SVCNAME}"
> +command_args="-p ${pidfile} ${OPTS}"
> +required_files="/etc/snmp/${SVCNAME}.conf"
> +extra_started_commands="reload"
> +
> +depend() {
> +        use logger
> +        need net
> +        after firewall
> +}
> +
> +reload() {
> +        ebegin "Reloading ${SVCNAME}"
> +        start-stop-daemon --signal HUP --pidfile ${pidfile} --name ${SVCNAME}
> +        eend $?
> +}

I like this, that we use the default start/stop functions and that we
reuse same init.d script for both snmpd and snmptrapd. However, this
will also break existing configs, which I want to avoid if possible.

We could maybe do something like:

# for backward compat
case "$SVCNAME" in
snmpd) : ${OPTS:=$SNMPD_FLAGS} ;;
esac

That way will users who have their setting in SNMPD_FLAGS be able to
upgrade without any problems.

> diff --git a/main/net-snmp/snmpd.confd b/main/net-snmp/snmpd.confd
> index 7b178da..8495175 100644
> --- a/main/net-snmp/snmpd.confd
> +++ b/main/net-snmp/snmpd.confd
> @@ -2,13 +2,13 @@
>  OPTS=""
>  
>  # Enable connection logging.
> -#SNMPD_FLAGS="${OPTS} -a"
> +#OPTS="${OPTS} -a"
>  
>  # Enable syslog and disable file log.
> -SNMPD_FLAGS="${OPTS} -LSwd -Lf /dev/null"
> +OPTS="${OPTS} -LSwd -Lf /dev/null"
>  
>  # Enable agentx socket as /var/agentx/master
>  # *NOTE* Before uncommenting this, make sure
>  #        the /var/agentx directory exists.
> -#SNMPD_FLAGS="${OPTS} -x /var/agentx/master"
> +#OPTS="${OPTS} -x /var/agentx/master"
>  
> diff --git a/main/net-snmp/snmpd.initd b/main/net-snmp/snmpd.initd
> deleted file mode 100644
> index 65d0555..0000000
> --- a/main/net-snmp/snmpd.initd
> +++ /dev/null

...

> diff --git a/main/net-snmp/snmptrapd.confd b/main/net-snmp/snmptrapd.confd
> index d9cee61..7f10cfe 100644
> --- a/main/net-snmp/snmptrapd.confd
> +++ b/main/net-snmp/snmptrapd.confd
> @@ -2,11 +2,11 @@
>  OPTS=""
>  
>  # ignore authentication failure traps
> -#SNMPTRAPD_FLAGS="${OPTS} -a"
> +#OPTS="${OPTS} -a"
>  
>  # log messages to specified file
> -#SNMPTRAPD_FLAGS="${OPTS} -Lf /var/log/snmptrapd.log"
> +#OPTS="${OPTS} -Lf /var/log/snmptrapd.log"
>  
>  # log messages to syslog with the specified facility
>  # where facility is: 'd' = LOG_DAEMON, 'u' = LOG_USER, [0-7] = LOG_LOCAL[0-7]
> -#SNMPTRAPD_FLAGS="${OPTS} -Ls d"
> +#OPTS="${OPTS} -Ls d"

...

-nc


---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---