[Pkg-shadow-devel] audit support

[Steve Grubb]

On Tuesday 02 September 2008 18:11:01 Nicolas François wrote:
> IIRC, AUDIT_ADD_GROUP/AUDIT_ADD_USER were not defined in some previous
> versions of libaudit.

True. But these defines go way back. All the way to audit-1.0.5. That was Oct 
2005.


> Do you think it might be useful to redefine it to AUDIT_USER_CHAUTHTOK if
> configure detects such case?

The problem is that AUDIT_USER_CHAUTHTOK means that the password or 
credentials was updated. I suppose you could allow that. But its clearly 
wrong.


> I've also added some log in case of *_unlock() failures (this means the
> system is left in an inconsistent way).

What the audit system is intended to record is the willful change of user 
information. If a failure is due to something the user did, we want to record 
it. If there is a resource problem that the user has no influence over like 
malloc failing, we do not want to cause the user to be singled out in the 
audit logs. If however, they are doing something they are not allowed to do, 
we want it recorded. If we have started modifying files and the admin will 
have to "undo" or correct something, we want that recorded.


> Note that "adding SELinux user mapping" in useradd.c is not part of
> upstream shadow. It might be easier for your later maintenance to change
> the audit_logger in the patch that introduce this.

Ok. Any chance of merging that patch too? :)  SE Linux introduced the notion 
of roles around 2 years ago. When a user is created, he needs to be mapped to 
a default role befpre they ever try to log in. Maybe this can be conditional 
in configure as --with-seinux? We get beat up sometimes for selinux patches 
being hard to find and other distros not knowing everything they needed to 
apply. It helps everybody if its all out in the open.


> I have some questions regarding the type to use in some cases:
> (AUDIT_ADD_GROUP, AUDIT_ADD_USER, AUDIT_USER_CHAUTHTOK?)
>  * "changing user defaults" message in useradd.c?

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.


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


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


> Is there a need to report to audit if an error is detected before any
> changes are committed to the user/group databases (and before the change
> is reported to audit)?

Only permission problems or things that are the user's fault. If malloc fails 
or something beyond the user's control, we don't need to blame them for it 
until files are starting to be modified. At that point, we need to have a 
success/fail event since things have been changed and the admin may need to 
review the state of affairs.


> Here are some examples:
>  + *_lock() failures
>    Those failures should be caused by another account management program
>    still running.

Not really if no file has been changed and the lock failure is not due to 
permission problems.


>  + useradd <invalid user name>
>  + useradd <already existing user>

Not really. No change to any file was made. If we have an audit message 
already leave it. If we don't have one, no need to add it since this has been 
through CAPP and LSPP evals.

One thing I will mention is that long term, i think shadow-utils needs some 
work consolidating logging and audit. Many are identical. When we went 
through the CAPP eval for RHEL4, the patch added 326 audit events. That is 
more events than anything else including the kernel. :)

Cheers,
-Steve