[Pkg-shadow-devel] audit support

[Nicolas François]

Hi,

I prepared a new version of groupadd to test a new logging strategy
(attached).

It looks much better to me (for syslog logging, and it will also save me
from removing all the xstrdup/xmalloc which can currently cause missing
logs).


I added a table of functions that will be called on exit (atexit(3)).

When I'm able to confirm the intent to change an account, I register a
function that will be able to log the failure on exit (it's still not
running on signal reception, but should be).
When the intended action is successfully done, I remove the function from
the table.

For groupadd, this means:
 * validate user inputs (i.e. do not log failure in case of typo)
 * add_cleanup (cleanup_report_add_group);
 * add_cleanup (cleanup_report_add_group_group);
(* add_cleanup (cleanup_report_add_group_gshadow);)
 * gr_close (); => this change the group file on the file system
 * log /etc/group changes
 * del_cleanup (cleanup_report_add_group_group);
(* sgr_close ();)
(* log /etc/gshadow changes)
(* del_cleanup (cleanup_report_add_group_gshadow);)
 * del_cleanup (cleanup_report_add_group);
 * exit (E_SUCCESS)

I moved the gr_lock() and sgr_lock() to "validate user inputs" (no log of
failures).

I used audit_log_user_message(type=AUDIT_USYS_CONFIG) when files cannot be
unlocked (this is not really an account manipulation operation, and
providing a groupname was strange for this error).
Is this OK?

On Thu, Sep 04, 2008 at 02:49:43PM -0400, sgrubb at redhat.com wrote:
> On Wednesday 03 September 2008 09:08:17 Nicolas François wrote:
> > On Tue, Sep 02, 2008 at 08:14:59PM -0400, sgrubb at redhat.com wrote:
> > > I suppose there is AUDIT_USYS_CONFIG. But this is usually associated with
> > > hw clock changes. We could change it to that if you want. I was primarily
> > > noticing that rpm was adding users & groups to systems and it was not
> > > being recorded as such.
> >
> > This one is just to notify that /etc/default/useradd was changed.
> > (no changes in the passwd, shadow, group or gshadow databases)
> > So AUDIT_USYS_CONFIG looks better to me.
> 
> OK. Sound's fine to me.

Is audit_log_acct_message() valid in that case.
I've currently changed it to audit_log_user_message()

> You really want to log at the end. We should open the audit connection first 
> and abort if the audit_open fails. There is errno ECONNREFUSED for people 
> that do not compile in the audit subsystem to the kernel. So, we should make 
> an exception for that. The write to the audit logs would be at the end when 
> the results are known.

libmisc/audit_help.c uses:
if (errno == EINVAL || errno == EPROTONOSUPPORT || errno == EAFNOSUPPORT)
	return;

Is that wrong?
(pam is using the same)

> > > Many are identical.
> >
> > You mean syslog and libaudit receive the same log?
> 
> Actually, I was thinking that there is much duplicated text. I guess there is 
> a lot of error checking and a lot of similar but different text. The audit 
> system's text should be much simpler than syslogs text since it needs to be 
> machine parsable. I think that could be consolidated much easier than 
> syslog's text. For example, using AUDIT_ADD_USER means that we can probably 
> change "adding user" to "".

I used "" for the main group addition, and:
"adding group to /etc/group"
"adding group to /etc/gshadow"

Best Regards,
-- 
Nekral
-------------- next part --------------
A non-text attachment was scrubbed...
Name: groupadd.c
Type: text/x-csrc
Size: 18731 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/pkg-shadow-devel/attachments/20080906/b433f802/attachment.c