Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audit update #1199

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
41 changes: 40 additions & 1 deletion lib/audit_help.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#include "attr.h"
#include "prototypes.h"
#include "shadowlog.h"
#include "string/sprintf/snprintf.h"

int audit_fd;

void audit_help_open (void)
Expand All @@ -48,7 +50,7 @@ void audit_help_open (void)
* This function will log a message to the audit system using a predefined
* message format. Parameter usage is as follows:
*
* type - type of message: AUDIT_USER_CHAUTHTOK for changing any account
* type - type of message: AUDIT_USER_MGMT for changing any account
* attributes.
* pgname - program's name
* op - operation. "adding user", "changing finger info", "deleting group"
Expand All @@ -68,6 +70,43 @@ void audit_logger (int type, MAYBE_UNUSED const char *pgname, const char *op,
}
}

/*
* This function will log a message to the audit system using a predefined
* message format. Parameter usage is as follows:
*
* type - type of message: AUDIT_USER_MGMT for changing any account
* attributes.
* pgname - program's name
* op - operation. "adding user", "changing finger info", "deleting group"
* name - user's account or group name. If not available use NULL.
* id - uid or gid that the operation is being performed on. This is used
* only when user is NULL.
* grp - group name associated with event
*/
void audit_logger_with_group (int type, MAYBE_UNUSED const char *pgname,
const char *op, const char *name, unsigned int id,
const char *grp, shadow_audit_result result)
{
int len;
char enc_group[GROUP_NAME_MAX_LENGTH * 2 + 1];
char buf[NITEMS(enc_group) + 100];

if (audit_fd < 0) {
return;
}

len = strnlen(grp, sizeof(enc_group)/2);
if (audit_value_needs_encoding(grp, len)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, we don't allow names that would need encoding. This reminds me that we have #1158.

I will add " to the list of strictly disallowed characters, and then we don't need to encode anything here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. If it's fine for you we leave it like this for now, and if I see that #1158 mergse before this PR I will update the code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's fine. Thanks!

SNPRINTF(buf, "%s grp=%s", op,
audit_encode_value(enc_group, grp, len));
} else {
SNPRINTF(buf, "%s grp=\"%s\"", op, grp);
}

audit_log_acct_message (audit_fd, type, NULL, buf, name, id,
NULL, NULL, NULL, (int) result);
}

void audit_logger_message (const char *message, shadow_audit_result result)
{
if (audit_fd < 0) {
Expand Down
22 changes: 11 additions & 11 deletions lib/cleanup_group.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void cleanup_report_mod_group (void *cleanup_info)
gr_dbname (),
info->action));
#ifdef WITH_AUDIT
audit_logger (AUDIT_USER_ACCT, log_get_progname(),
audit_logger (AUDIT_GRP_MGMT, log_get_progname(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding

b2c8eff

You could move the filename in the subject to the prefix:

lib/cleanup_group.c: Update audit messages

Also, could you add some little explanation in the commit message of what this update is about?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding

b2c8eff

You could move the filename in the subject to the prefix:

lib/cleanup_group.c: Update audit messages

Or will you squash the commits? (I think it would make sense to squash them.)

info->audit_msg,
info->name, AUDIT_NO_ID,
SHADOW_AUDIT_FAILURE);
Expand All @@ -80,7 +80,7 @@ void cleanup_report_mod_gshadow (void *cleanup_info)
sgr_dbname (),
info->action));
#ifdef WITH_AUDIT
audit_logger (AUDIT_USER_ACCT, log_get_progname(),
audit_logger (AUDIT_GRP_MGMT, log_get_progname(),
info->audit_msg,
info->name, AUDIT_NO_ID,
SHADOW_AUDIT_FAILURE);
Expand All @@ -101,7 +101,7 @@ void cleanup_report_add_group_group (void *group_name)
SYSLOG ((LOG_ERR, "failed to add group %s to %s", name, gr_dbname ()));
#ifdef WITH_AUDIT
audit_logger (AUDIT_ADD_GROUP, log_get_progname(),
"adding group to /etc/group",
"adding-group",
name, AUDIT_NO_ID,
SHADOW_AUDIT_FAILURE);
#endif
Expand All @@ -120,8 +120,8 @@ void cleanup_report_add_group_gshadow (void *group_name)

SYSLOG ((LOG_ERR, "failed to add group %s to %s", name, sgr_dbname ()));
#ifdef WITH_AUDIT
audit_logger (AUDIT_ADD_GROUP, log_get_progname(),
"adding group to /etc/gshadow",
audit_logger (AUDIT_GRP_MGMT, log_get_progname(),
"adding-shadow-group",
name, AUDIT_NO_ID,
SHADOW_AUDIT_FAILURE);
#endif
Expand All @@ -143,8 +143,8 @@ void cleanup_report_del_group_group (void *group_name)
"failed to remove group %s from %s",
name, gr_dbname ()));
#ifdef WITH_AUDIT
audit_logger (AUDIT_ADD_GROUP, log_get_progname(),
"removing group from /etc/group",
audit_logger (AUDIT_DEL_GROUP, log_get_progname(),
"removing-group",
name, AUDIT_NO_ID,
SHADOW_AUDIT_FAILURE);
#endif
Expand All @@ -166,8 +166,8 @@ void cleanup_report_del_group_gshadow (void *group_name)
"failed to remove group %s from %s",
name, sgr_dbname ()));
#ifdef WITH_AUDIT
audit_logger (AUDIT_ADD_GROUP, log_get_progname(),
"removing group from /etc/gshadow",
audit_logger (AUDIT_GRP_MGMT, log_get_progname(),
"removing-shadow-group",
name, AUDIT_NO_ID,
SHADOW_AUDIT_FAILURE);
#endif
Expand All @@ -187,7 +187,7 @@ void cleanup_unlock_group (MAYBE_UNUSED void *arg)
log_get_progname(), gr_dbname ());
SYSLOG ((LOG_ERR, "failed to unlock %s", gr_dbname ()));
#ifdef WITH_AUDIT
audit_logger_message ("unlocking group file",
audit_logger_message ("unlocking-group",
SHADOW_AUDIT_FAILURE);
#endif
}
Expand All @@ -207,7 +207,7 @@ void cleanup_unlock_gshadow (MAYBE_UNUSED void *arg)
log_get_progname(), sgr_dbname ());
SYSLOG ((LOG_ERR, "failed to unlock %s", sgr_dbname ()));
#ifdef WITH_AUDIT
audit_logger_message ("unlocking gshadow file",
audit_logger_message ("unlocking-gshadow",
SHADOW_AUDIT_FAILURE);
#endif
}
Expand Down
12 changes: 6 additions & 6 deletions lib/cleanup_user.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void cleanup_report_mod_passwd (void *cleanup_info)
pw_dbname (),
info->action));
#ifdef WITH_AUDIT
audit_logger (AUDIT_USER_ACCT, log_get_progname(),
audit_logger (AUDIT_USER_MGMT, log_get_progname(),
info->audit_msg,
info->name, AUDIT_NO_ID,
SHADOW_AUDIT_FAILURE);
Expand All @@ -65,7 +65,7 @@ void cleanup_report_add_user_passwd (void *user_name)
SYSLOG ((LOG_ERR, "failed to add user %s to %s", name, pw_dbname ()));
#ifdef WITH_AUDIT
audit_logger (AUDIT_ADD_USER, log_get_progname(),
"adding user to /etc/passwd",
"adding-user",
name, AUDIT_NO_ID,
SHADOW_AUDIT_FAILURE);
#endif
Expand All @@ -84,8 +84,8 @@ void cleanup_report_add_user_shadow (void *user_name)

SYSLOG ((LOG_ERR, "failed to add user %s to %s", name, spw_dbname ()));
#ifdef WITH_AUDIT
audit_logger (AUDIT_ADD_USER, log_get_progname(),
"adding user to /etc/shadow",
audit_logger (AUDIT_USER_MGMT, log_get_progname(),
"adding-shadow-user",
name, AUDIT_NO_ID,
SHADOW_AUDIT_FAILURE);
#endif
Expand All @@ -104,7 +104,7 @@ void cleanup_unlock_passwd (MAYBE_UNUSED void *arg)
log_get_progname(), pw_dbname ());
SYSLOG ((LOG_ERR, "failed to unlock %s", pw_dbname ()));
#ifdef WITH_AUDIT
audit_logger_message ("unlocking passwd file",
audit_logger_message ("unlocking-passwd",
SHADOW_AUDIT_FAILURE);
#endif
}
Expand All @@ -123,7 +123,7 @@ void cleanup_unlock_shadow (MAYBE_UNUSED void *arg)
log_get_progname(), spw_dbname ());
SYSLOG ((LOG_ERR, "failed to unlock %s", spw_dbname ()));
#ifdef WITH_AUDIT
audit_logger_message ("unlocking shadow file",
audit_logger_message ("unlocking-shadow",
SHADOW_AUDIT_FAILURE);
#endif
}
Expand Down
9 changes: 9 additions & 0 deletions lib/prototypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,21 @@ extern int audit_fd;
extern void audit_help_open (void);
/* Use AUDIT_NO_ID when a name is provided to audit_logger instead of an ID */
#define AUDIT_NO_ID ((unsigned int) -1)
#ifndef AUDIT_GRP_MGMT
#define AUDIT_GRP_MGMT 1132 /* Group account was modified */
#endif
#ifndef AUDIT_GRP_CHAUTHTOK
#define AUDIT_GRP_CHAUTHTOK 1133 /* Group account password was changed */
#endif
Comment on lines +185 to +190
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for those specific values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They come from audit-records.h and probably they weren't available in all distributions when this patch was originally created (that happened years ago). I'm fine with removing this part of the patch as probably they will be available in all distributions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please try removing them, and if builds everywhere, let's forget about them (and if not, reintroduce them).

typedef enum {
SHADOW_AUDIT_FAILURE = 0,
SHADOW_AUDIT_SUCCESS = 1} shadow_audit_result;
extern void audit_logger (int type, const char *pgname, const char *op,
const char *name, unsigned int id,
shadow_audit_result result);
void audit_logger_with_group (int type, MAYBE_UNUSED const char *pgname,
const char *op, const char *name, unsigned int id,
const char *grp, shadow_audit_result result);
void audit_logger_message (const char *message, shadow_audit_result result);
#endif

Expand Down
39 changes: 18 additions & 21 deletions src/chage.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ fail_exit (int code)

#ifdef WITH_AUDIT
if (E_SUCCESS != code) {
audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
"change age", user_name, user_uid, 0);
audit_logger (AUDIT_USER_MGMT, Prog,
"change-age", user_name, user_uid, SHADOW_AUDIT_FAILURE);
}
#endif

Expand Down Expand Up @@ -789,10 +789,7 @@ int main (int argc, char **argv)
fprintf (stderr, _("%s: Permission denied.\n"), Prog);
fail_exit (E_NOPERM);
}
#ifdef WITH_AUDIT
audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
"display aging info", user_name, user_uid, 1);
#endif
/* Displaying fields is not of interest to audit */
list_fields ();
fail_exit (E_SUCCESS);
}
Expand All @@ -811,39 +808,39 @@ int main (int argc, char **argv)
}
#ifdef WITH_AUDIT
else {
audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
"change all aging information",
user_name, user_uid, 1);
audit_logger (AUDIT_USER_MGMT, Prog,
"change-all-aging-information",
user_name, user_uid, SHADOW_AUDIT_SUCCESS);
}
#endif
} else {
#ifdef WITH_AUDIT
if (Mflg) {
audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
"change max age", user_name, user_uid, 1);
audit_logger (AUDIT_USER_MGMT, Prog,
"change-max-age", user_name, user_uid, SHADOW_AUDIT_SUCCESS);
}
if (mflg) {
audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
"change min age", user_name, user_uid, 1);
audit_logger (AUDIT_USER_MGMT, Prog,
"change-min-age", user_name, user_uid, 1);
}
if (dflg) {
audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
"change last change date",
audit_logger (AUDIT_USER_MGMT, Prog,
"change-last-change-date",
user_name, user_uid, 1);
}
if (Wflg) {
audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
"change passwd warning",
audit_logger (AUDIT_USER_MGMT, Prog,
"change-passwd-warning",
user_name, user_uid, 1);
}
if (Iflg) {
audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
"change inactive days",
audit_logger (AUDIT_USER_MGMT, Prog,
"change-inactive-days",
user_name, user_uid, 1);
}
if (Eflg) {
audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
"change passwd expiration",
audit_logger (AUDIT_USER_MGMT, Prog,
"change-passwd-expiration",
user_name, user_uid, 1);
}
#endif
Expand Down
Loading
Loading