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.

Sign up for GitHub

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

Fix audit records #1188

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix audit records #1188

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
038ff23
Fix audit records
stevegrubb Jan 18, 2025
a82a01a
Updates based on feedback
stevegrubb Jan 21, 2025
88e145a
Another round of updates
stevegrubb Jan 22, 2025
0d86d7c
Invert streq value
stevegrubb Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions lib/audit_help.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
#include <libaudit.h>
#include <errno.h>
#include <stdio.h>
#include <alloca.h>

#include "attr.h"
#include "prototypes.h"
#include "shadowlog.h"
#include "string/strcmp/streq.h"
int audit_fd;

void audit_help_open (void)
Expand All @@ -44,6 +46,19 @@ void audit_help_open (void)
}
}

/*
* This takes a string and replaces the old character with the new.
*/
static inline const char *
strtr(char *str, char old, char new)
{
for (char *p = str; !streq(p, ""); p++) {
if (*p == old)
*p = new;
}
return str;
}

/*
* This function will log a message to the audit system using a predefined
* message format. Parameter usage is as follows:
Expand All @@ -63,8 +78,15 @@ void audit_logger (int type, MAYBE_UNUSED const char *pgname, const char *op,
if (audit_fd < 0) {
return;
} else {
audit_log_acct_message (audit_fd, type, NULL, op, name, id,
NULL, NULL, NULL, result);
/*
* The audit system needs white space in the op field to
* be replaced with dashes so that parsers get the whole
* field.
*/
const char *fixed_op = strtr(strdupa(op), ' ', '-');
audit_log_acct_message(audit_fd, type, NULL,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've been thinking...

If this is a string literal that we absolutely control, and we only use it for this, why not just change the literals accordingly?

If the string literal is exactly the same as what is logged, the users will be able to grep the source code for the string that they see in the logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This reason it was done this way is because there are about 200 places to change. This patch is an attempt to fix the issue with the least amount of code churn. Long term that is the fix. But I don't have the time to do the bigger fix any time soon.

Copy link
Collaborator

@alejandro-colomar alejandro-colomar Jan 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You could probably start with this:

$ find -type f \
| grep '\.c$' \
| xargs pcre2grep -hM '(?s)\baudit_logger *\([^{};]*\);' \
| sed 's/^\s*//' \
| tr '\n' ' ' \
| sed 's/; /;\n/g' \
| cut -d, -f3 \
| sort \
| uniq;
 ""
 "adding SELinux user mapping"
 "adding group to /etc/group"
 "adding group to /etc/gshadow"
 "adding group"
 "adding home directory"
 "adding shadow password"
 "adding user to /etc/passwd"
 "adding user to /etc/shadow"
 "adding user to group"
 "adding user to shadow group"
 "adding user"
 "change age"
 "change all aging information"
 "change inactive days"
 "change last change date"
 "change max age"
 "change min age"
 "change passwd expiration"
 "change passwd warning"
 "changing admin name in shadow group"
 "changing comment"
 "changing expiration date"
 "changing group member"
 "changing home directory owner"
 "changing home directory"
 "changing inactive days"
 "changing mail file name"
 "changing mail file owner"
 "changing member in shadow group"
 "changing name"
 "changing password"
 "changing primary group"
 "changing uid"
 "changing user shell"
 "changing useradd defaults"
 "changing"
 "clearing-lastlog"
 "deleting group"
 "deleting home directory"
 "deleting mail file"
 "deleting shadow group"
 "deleting user entries"
 "deleting user from group"
 "deleting user from shadow group"
 "deleting user logged in"
 "deleting user not found"
 "deleting user"
 "display aging info"
 "locking group file"
 "locking password file"
 "locking shadow group file"
 "locking shadow password file"
 "locking subordinate group file"
 "locking subordinate user file"
 "modifying User mapping "
 "modifying account"
 "modifying group"
 "moving home directory"
 "opening group file"
 "opening password file"
 "opening shadow group file"
 "opening shadow password file"
 "opening subordinate group file"
 "opening subordinate user file"
 "refreshing-lastlog"
 "removing SELinux user mapping"
 "removing group from /etc/group"
 "removing group from /etc/gshadow"
 "removing group member"
 "removing user from shadow group"
 "unlocking group file"
 "unlocking gshadow file"
 "unlocking passwd file"
 "unlocking shadow file"
 "unlocking subordinate group file"
 "unlocking subordinate user file"
 "unlocking-group-file"
 "unlocking-gshadow-file"
 "updating passwd"
 "updating password"
 audit_buf
 buf
 info->audit_msg
 info_group.audit_msg
 info_gshadow.audit_msg
 info_passwd.audit_msg

And then try a script to replace them (please make sure to paste the script verbatim in the commit message if you do.

On the other hand, this shows that a few cases are not string literals. We should check that and see what they are.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Steve I'm thinking that we could open a PR to include the downstream patch in shadow. I don't think it would take a great deal of time to do it since I already had to update it for shadow 4.17.0 and I don't think the code has changed much since that was released.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ikerexxe feel free to do that. I can close this if a better patch is accepted.

fixed_op, name, id,
NULL, NULL, NULL, result);
}
}

Expand Down
Loading