[Buildroot] [PATCH v7 04/22] audit: new package

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

Dear Clayton Shotwell,

On Tue,  2 Jun 2015 08:28:20 -0500, Clayton Shotwell wrote:

>  package/audit/0001-Enable-cross-compiling.patch    | 773 +++++++++++++++++++++
>  .../0002-Make-zos-remote-plugin-optional.patch     |  56 ++
>  ...03-Default-ADDR_NO_RANDOMIZE-if-not-found.patch |  44 ++
>  ...o-not-call-posix_fallocate-if-unavailable.patch |  47 ++
>  ...Fix-header-detection-when-cross-compiling.patch |  46 ++

I know we've already discussed this, but again, please make sure to
submit all these patches upstream.

> diff --git a/package/audit/Config.in b/package/audit/Config.in
> new file mode 100644
> index 0000000..66fceec
> --- /dev/null
> +++ b/package/audit/Config.in
> @@ -0,0 +1,18 @@
> +config BR2_PACKAGE_AUDIT
> +	bool "audit"
> +	# needs memory fences for internal libev
> +	depends on !BR2_bfin

Actually, I believe this package is only available on a much smaller
selection of architectures: x86, x86-64, PowerPC, ARM and AArch64. It
does not have the system call tables for other architectures (or
architectures not supported in Buildroot, such as Alpha or S390).

So, I've added a BR2_PACKAGE_AUDIT_ARCH_SUPPORTS instead to express
this architecture dependency.

> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	help
> +	  The audit package contains the user space utilities for
> +	  storing and searching the audit records generated by
> +	  the audit subsystem in the Linux 2.6 kernel
> +
> +	  Note: The z/OS remote plugin is disabled in this package
> +
> +	  http://people.redhat.com/sgrubb/audit/
> +
> +comment "audit needs a toolchain w/ threads"
> +	depends on !BR2_TOOLCHAIN_HAS_THREADS
> +	depends on !BR2_bfin
> +


> diff --git a/package/audit/S01auditd b/package/audit/S01auditd
> new file mode 100644
> index 0000000..27de572
> --- /dev/null
> +++ b/package/audit/S01auditd

I am still not entirely happy with this init script. However, since I
wanted the topic to make progress, I've applied the package, just after
removing the init script for now (and with some other changes I'll
detail below). Please resubmit a patch re-adding the init script with a
few fixes.

> @@ -0,0 +1,99 @@
> +#!/bin/sh
> +#
> +# auditd       This starts and stops auditd
> +#
> +# description: This starts the Linux Auditing System Daemon,
> +#              which collects security related events in a dedicated
> +#              audit log. If this daemon is turned off, audit events
> +#              will be sent to syslog.
> +#
> +
> +NAME=auditd
> +DAEMON=/usr/sbin/${NAME}
> +CONFIG=/etc/audit/auditd.conf
> +LOCK=/var/run/${NAME}.pid

Please name this variable PIDFILE. It's not really a lock, and we call
it PIDFILE in most other Buildroot packages.

> +
> +start(){
> +	echo -n "Initializing ${NAME}: "

Should be "Starting ${NAME}: " to match other Buildroot packages.

> +
> +	# Create dir to store log files in if one doesn't exist. Create
> +	# the directory with SELinux permissions if possible
> +	command -v matchpathcon >/dev/null 2>&1 && \
> +		mkdir -p /var/log/audit -Z `matchpathcon -n /var/log/audit` || \
> +		mkdir -p /var/log/audit

This seems a bit hard to read, maybe (untested):

	if command -v matchpathcon >/dev/null 2>&1 ; then
		mkdir -p /var/log/audit -Z `matchpathcon -n /var/log/audit`
	else
		mkdir -p /var/log/audit
	fi

But it's a bit weird to do things "if possible". Either it is needed
and we always do it (which requires a dependency on libselinux, since
that's where matchpathcon is), or we never do it.

> +	# Run audit daemon executable
> +	start-stop-daemon -S -q -p ${LOCK} --exec ${DAEMON}
> +
> +	# Load the default rules
> +	test -f /etc/audit/rules.d/audit.rules && /usr/sbin/auditctl -R /etc/audit/rules.d/audit.rules >/dev/null
> +
> +	echo "OK"

Lacks some error handling, no? Look at other init scripts.

> +}
> +
> +stop(){
> +	echo -n "Uninitializing ${NAME}: "

"Stopping ${NAME}: "

> +
> +	start-stop-daemon -K -q -p ${LOCK}
> +
> +	echo "OK"
> +}
> +
> +reload(){
> +	echo -n "Reloading ${NAME} configuration: "
> +	if [ -e ${LOCK} ]; then
> +		kill -HUP `cat ${LOCK}`
> +		RETVAL=$?
> +		if [ ${RETVAL} ]; then
> +			echo "OK"
> +		else
> +			echo "FAILED"
> +		fi
> +	else
> +		echo "FAILED: ${NAME} not started"
> +		RETVAL=1
> +	fi
> +	return ${RETVAL}

Can you try just something like:

	echo -n "Reloading ${NAME} configuration: "
	start-stop-daemon --stop -s 1 -p ${PIDFILE}
	[ $? = 0 ] && echo "OK" || echo "FAIL"

> +rotate(){
> +	echo -n "Rotating ${NAME} logs: "
> +	if [ -e ${LOCK} ]; then
> +		kill -USR1 `cat ${LOCK}`
> +		RETVAL=$?
> +		if [ ${RETVAL} ]; then
> +			echo "OK"
> +		else
> +			echo "FAILED"
> +		fi
> +	else
> +		echo "FAILED: ${NAME} not started"
> +		RETVAL=1
> +	fi
> +	return ${RETVAL}

Same, but with -s 30 passed as start-stop-daemon argument?


> +exit $?

Not needed probably.

> diff --git a/package/audit/audit.mk b/package/audit/audit.mk
> new file mode 100644
> index 0000000..c3ee0bb
> --- /dev/null
> +++ b/package/audit/audit.mk
> @@ -0,0 +1,43 @@
> +################################################################################
> +#
> +# audit
> +#
> +################################################################################
> +
> +AUDIT_VERSION = 2.4.1
> +AUDIT_SITE = http://people.redhat.com/sgrubb/audit/
> +AUDIT_LICENSE = GPLv2
> +AUDIT_LICENSE_FILES = COPYING
> +
> +AUDIT_INSTALL_STAGING = YES
> +
> +# Patching configure.ac and Makefile.am
> +AUDIT_AUTORECONF = YES
> +
> +AUDIT_CONF_OPTS = --without-python --disable-zos-remote
> +
> +ifeq ($(BR2_PACKAGE_LIBCAP_NG),y)
> +AUDIT_DEPENDENCIES += libcap-ng
> +AUDIT_CONF_OPTS += --with-libcap-ng=yes
> +else
> +AUDIT_CONF_OPTS += --with-libcap-ng=no
> +endif
> +
> +ifeq ($(BR2_arm)$(BR2_armeb),y)
> +AUDIT_CONF_OPTS += --with-arm
> +endif
> +ifeq ($(BR2_aarch64),y)
> +AUDIT_CONF_OPTS += --with-aarch64
> +endif

I've added a comment above these lines to explain why we need special
handling for ARM and AArch64 and not for other architectures.

> +
> +define AUDIT_INSTALL_INIT_SYSV
> +	$(INSTALL) -m 755 package/audit/S01auditd $(TARGET_DIR)/etc/init.d/
> +endef

I've removed these lines, since I did not include the init script for
the moment.

> +
> +define AUDIT_INSTALL_CLEANUP
> +	$(RM) -rf $(TARGET_DIR)/etc/rc.d
> +	$(RM) -rf $(TARGET_DIR)/etc/sysconfig
> +endef
> +AUDIT_POST_INSTALL_TARGET_HOOKS += AUDIT_INSTALL_CLEANUP
> +
> +$(eval $(autotools-package))

So, patch applied, with the comments mentioned above: removal of init
script, addition of BR2_PACKAGE_AUDIT_ARCH_SUPPORTS, and comment in
the .mk file about the ARM/AArch64 options.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com