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

Clayton Shotwell clayton.shotwell at rockwellcollins.com
Fri Jun 19 15:56:19 UTC 2015


Thomas,

Thanks for merging this in!

On Wed, Jun 17, 2015 at 4:45 PM, Thomas Petazzoni
<thomas.petazzoni at free-electrons.com> wrote:
> 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.

They have been submitted but I have not heard anything back yet. I'll
followup here in the next couple of weeks.

>> 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.

That works for me.

>> +     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.

I'll try to get that done today.

>> @@ -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

I think that is reasonable. I'll change and test.

> 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"

Yes I can.

>> +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?

Yep. Much cleaner.

>
>> +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.

That all looks good to me.

Thanks,
Clayton


More information about the buildroot mailing list