9830 praudit should be able to map users and groups correctly

Review Request #1204 - Created Sept. 11, 2018 and updated

Information
Peter Tribble
illumos-gate
9830
Reviewers
general

It's common to aggregate audit logs on a central system. Currently, running praudit then resolves uids and gids back to names on the system where praudit runs, which may be completely wrong.

This fix allows the user to copy the group and passwd files to the aggregated system and point praudit at those files. It does this by preloading the uid and gid caches introduced in 9106.

Run praudit with the -p and -g flags, verified that it resolves uids and gids correctly. Verified that without the flags being used, we get the same (wrong) results as before.

Issues

  • 8
  • 0
  • 0
  • 8
Description From Last Updated
We should be checking for and handling failures from fgetpwent() here, so that the program doesn't silently accept an error ... Joshua Clulow Joshua Clulow
Check for failures of fgetgrent(), too. Joshua Clulow Joshua Clulow
It looks like these definitions should go in praudit.h, rather than directly here. Joshua Clulow Joshua Clulow
pf is a pointer, so we should compare against NULL; i.e., if (pf != NULL) { Joshua Clulow Joshua Clulow
We should check the return of fclose() here, even if just via VERIFY0(fclose(pf)). Same with fclose(gf) a couple of lines ... Joshua Clulow Joshua Clulow
if (gf != NULL) { Joshua Clulow Joshua Clulow
Is there a period missing on the front of the line here? Joshua Clulow Joshua Clulow
Is this behaviour desirable? If the audit file is from a foreign system, would it not be more correct to ... Joshua Clulow Joshua Clulow
Andy Fiddaman
Ship It!
Joshua Clulow

   
usr/src/cmd/praudit/format.c (Diff revision 1)
 
 

We should be checking for and handling failures from fgetpwent() here, so that the program doesn't silently accept an error and move on.

usr/src/cmd/praudit/format.c (Diff revision 1)
 
 

Check for failures of fgetgrent(), too.

usr/src/cmd/praudit/main.c (Diff revision 1)
 
 
 

It looks like these definitions should go in praudit.h, rather than directly here.

usr/src/cmd/praudit/main.c (Diff revision 1)
 
 

pf is a pointer, so we should compare against NULL; i.e., if (pf != NULL) {

usr/src/cmd/praudit/main.c (Diff revision 1)
 
 

We should check the return of fclose() here, even if just via VERIFY0(fclose(pf)). Same with fclose(gf) a couple of lines down.

usr/src/cmd/praudit/main.c (Diff revision 1)
 
 

if (gf != NULL) {

usr/src/man/man1m/praudit.1m (Diff revision 1)
 
 

Is there a period missing on the front of the line here?

usr/src/man/man1m/praudit.1m (Diff revision 1)
 
 

Is this behaviour desirable? If the audit file is from a foreign system, would it not be more correct to use only the provided file and not fall back to IDs from the local system?

Loading...