[Pkg-shadow-devel] audit support

[Steve Grubb]

On Wednesday 03 September 2008 09:08:17 Nicolas François wrote:
> I found some issues in userdel.c (see attached patch).
>
> In a future version, I will change these 1 and 0 results to
> SHADOW_AUDIT_SUCCESS and SHADOW_AUDIT_FAILURE.

OK.


> On Tue, Sep 02, 2008 at 08:14:59PM -0400, sgrubb at redhat.com wrote:
> Note that AUDIT_USER_CHAUTHTOK is still used wrongly in groupmod and
> usermod (at least).
>
> For these two, I think I can find the right audit type in some cases, but
> in other cases, when there is a change but not addition or removal of an
> account, there might be nothing to log (e.g. the current "changing
> comment" in usermod.c)

AUDIT_USER_CHAUTHTOK can be used when changing anything in /etc/passwd, 
shadow, group, or gshadow if new gid/uids are not being created.


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


> > >    AUDIT_USER_CHAUTHTOK or no audit log
> > >  * "adding user to group" in useradd
> > >    You changed it to AUDIT_ADD_USER, but it is a modification in the
> > > group file.
> >
> > I was thinking that was a failure in the act of adding a user. If its
> > adding a user to an existing group, then its not adding a group. If you
> > see a place where it was adding a new group, then we should record that
> > as adding a group. Adding a user to a group is still adding a user - but
> > modifying a group.
>
> OK. I will check. It might be that it will cancel the addition of the
> user, and then AUDIT_ADD_USER is the right type.

OK


> > >  * gr_unlock() failure in useradd
> > >    (i.e. the user was added, and the group file was possibly changed,
> > > and there were a later failure when the group file was unlocked)
> >
> > This is sufficient to call adding a user if a group was not added. If a
> > group was added, and it failed we need to record that.
>
> I read the second sentence as "If a group was intended to be added, and it
> failed..." right?

Sure.


> > One thing I will mention is that long term, i think shadow-utils needs
> > some work consolidating logging and audit.
>
> I fully agree with that.
> Would it make sense to change the libaudit logging to:
>  * After options processing
>    => log the intended actions

No, we need the results of the action. You don't know success or fail until 
the end. What't I'd recommend is having a string table and a enumeration that 
describes the failure. Then on exit the results are looked up and inserts 
applied. This is the way that pam does things if I remember. But the point is 
that there is 1 function that gets called and it figures out what to say or 
do (exit). We want 1 event for each action. If adding a user implies adding a 
group, we need 2 add events, one for user and one for group. If the user is 
assigned to an existing group, we have 1 record adding the user, 1 changing 
authtokens. What we have now probably does not follow this, because all the 
possible error paths are not exercised in CAPP?LSPP testing and would be 
overlooked.



>  * On success
>    => nothing (the intent was logged, an no failures)
>       (or should I explicitly log "user added")
>  * Log failures that (may) require admin action
>    e.g. the user was successfully removed, but we could not remove the
>    home directory.

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.


> For syslog logs, I would also like to define a logging policy
>  * Log changes to system files
>  * Log failures that result in or indicate inconsistent system files
>    (e.g. failure to unlock /etc/passwd, failure to open /etc/passwd)
> (* Log some attempts to change system files)

Sure. But similarly, I'd wait until the end when you know the results.


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

-Steve