Re: audit

On Mon, 5 Sep 2005, Peter Vrabec wrote:

> Here we go.

Thank you Peter.

I have few questions about attached patch:

>--- shadow/configure.in.audit	2005-08-30 20:10:25.000000000 +0000
>+++ shadow/configure.in	2005-09-05 13:38:05.000000000 +0000
>@@ -260,6 +262,14 @@
> AC_CHECK_LIB(crypt, crypt, [LIBCRYPT=-lcrypt],
> 	[AC_MSG_ERROR([crypt() not found])])
> 
>+AC_SUBST(AUDIT)
>+if test "$with_audit" != "no"; then
>+       AC_CHECK_HEADER(libaudit.h, [ audit_header="yes" ], [ audit_header="no" ])
>+       if test "$audit_header" = "yes"; then
>+               AC_CHECK_LIB(audit, audit_send_message_if_enabled, AC_DEFINE(WITH_AUDIT,1,[Define if you want to enable Audit messages]) [LIBS="-Wl,-Bstatic -laudit -Wl,-dy $LIBS"])
>+       fi
>+fi
>+
> AC_SUBST(LIBCRACK)
> if test "$with_libcrack" = "yes"; then
> 	echo "checking cracklib flavour, don't be surprised by the results"

Is it realy neccessary static linking with libaudit ?
On current devel Fedora system:

$ rpm -q --whatrequires libaudit.so.0
passwd-0.70-1
util-linux-2.13-0.2.pre2
dbus-0.36.1-1
pam-0.80-7
nscd-2.3.90-11
shadow-utils-4.0.12-2

If not neccessary .. probably in FC package can be ommited build and 
package static libaudit.

>--- shadow/lib/defines.h.audit	2005-08-03 18:11:27.000000000 +0000
>+++ shadow/lib/defines.h	2005-09-05 13:38:05.000000000 +0000
>@@ -331,4 +331,16 @@
> #define SHADOW_PASSWD_STRING "x"
> #endif
> 
>+#ifdef WITH_AUDIT
>+  #ifdef __u8 		/* in case we use pam < 0.80 */
>+    #undef __u8
>+  #endif
>+  #ifdef __u32
>+    #undef __u32
>+  #endif
>+
>+  #include <libaudit.h>
>+#endif
>+
> #endif				/* _DEFINES_H_ */
>+

Probably better will be add checking installed PAM version on autoconf 
stage. Add this kinf check will probably easy but I want see your opinion
on this.

[..]
>+#ifdef WITH_AUDIT
>+	audit_logger(AUDIT_USER_CHAUTHTOK,Prog,"deleting group", group_name,group_id,1);
>+#endif
> 	SYSLOG ((LOG_INFO, "remove group `%s'\n", group_name));
> 	return;
> }
>@@ -231,6 +235,10 @@
> 	int retval;
> #endif
> 
>+#ifdef WITH_AUDIT
>+	audit_help_open();
>+#endif
>+

In you previouse patch (sended directly to me) audit_help_open() and 
audit_logger() was without #ifdef WITH_AUDIT .. #endif conditions.
This can be correctly optimized by GCC global optimimization to the same 
binary code as with conditions but probably have better form for 
reading source code (?).
(probably the same trick can be used on some parts in SELinux support).

[..]
>--- shadow/src/Makefile.am.audit	2005-07-07 08:40:27.000000000 +0000
>+++ shadow/src/Makefile.am	2005-09-05 13:38:05.000000000 +0000
>@@ -51,28 +51,28 @@
> AM_CPPFLAGS    = -DLOCALEDIR=\"$(datadir)/locale\"
> 
> chpasswd_LDADD = $(LDADD) $(LIBPAM)
>-chage_LDADD    = $(LDADD) $(LIBPAM)
>+chage_LDADD    = $(LDADD) $(LIBPAM) $(AUDIT)
> chfn_LDADD     = $(LDADD) $(LIBPAM)
> chsh_SOURCES   = \
> 	chsh.c \
> 	chsh_chkshell.c
> chsh_LDADD     = $(LDADD) $(LIBPAM)
>-groupadd_LDADD = $(LDADD) $(LIBPAM)
>-groupdel_LDADD = $(LDADD) $(LIBPAM)
>-groupmod_LDADD = $(LDADD) $(LIBPAM)
>+groupadd_LDADD = $(LDADD) $(LIBPAM) $(AUDIT)
>+groupdel_LDADD = $(LDADD) $(LIBPAM) $(AUDIT)
>+groupmod_LDADD = $(LDADD) $(LIBPAM) $(AUDIT)
> login_SOURCES  = \
> 	login.c \
> 	login_nopam.c
> login_LDADD    = $(LDADD) $(LIBPAM)
> newusers_LDADD = $(LDADD) $(LIBPAM)
>-passwd_LDADD   = $(LDADD) $(LIBPAM) $(LIBCRACK)
>+passwd_LDADD   = $(LDADD) $(LIBPAM) $(LIBCRACK) $(AUDIT)
> su_SOURCES     = \
> 	su.c \
> 	suauth.c
> su_LDADD       = $(LDADD) $(LIBPAM)
>-useradd_LDADD  = $(LDADD) $(LIBPAM)
>-userdel_LDADD  = $(LDADD) $(LIBPAM)
>-usermod_LDADD  = $(LDADD) $(LIBPAM)
>+useradd_LDADD  = $(LDADD) $(LIBPAM) $(AUDIT)
>+userdel_LDADD  = $(LDADD) $(LIBPAM) $(AUDIT)
>+usermod_LDADD  = $(LDADD) $(LIBPAM) $(AUDIT)
> 
> install-am: all-am
> 	$(MAKE) $(AM_MAKEFLAGS) install-exec-am install-data-am

This looks incosistent compare to configure.in fixes because in 
configure.in -laudit is added to LIBS (incorrect because not all binaries 
will be linked with libaudit and must added to AUDIT).

I want only see opinion about above (I can finish this at this point).

kloczek
-- 
-----------------------------------------------------------
*Ludzie nie majÂą problemĂłw, tylko sobie sami je stwarzajÂą*
-----------------------------------------------------------
Tomasz KÂłoczko, sys adm @zie.pg.gda.pl|*e-mail: [email protected]*