-
Notifications
You must be signed in to change notification settings - Fork 54
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
Some audit improvements #46
Conversation
1. Combined some time-change lines to make processing more efficient 2. Changed the search for privileged executables so that limits the find to ext2/3/4 and xfs mounts (in case, like my systems, there are NFS or GPFS filesystems mounted when the script is run [outside of kickstart])
Added a rule to exclude all audit entries with auid<1000 or auid=4294967295 (-1 unsigned) as a separate rule instead of adding to all the rules at the bottom of the list. Moved any rules that didn't have it above, and removed the conditions from the rules below.
A couple more non-code typos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good - they were separate due to scanning tools not catching them.
Do you want me to separate them back out again for the scanning tools before you merge them in? What can I do to have you merge? |
-a always,exit -F path=/usr/sbin/restorecon -F perm=x -F auid>=1000 -F auid!=4294967295 -F key=privileged-priv_change | ||
-a always,exit -F path=/usr/bin/userhelper -F perm=x -F auid>=1000 -F auid!=4294967295 -F key=privileged | ||
-a always,exit -F path=/usr/bin/sudoedit -F perm=x -F auid>=1000 -F auid!=4294967295 -F key=privileged | ||
-a always,exit -F path=/usr/libexec/pt_chown -F perm=x -F auid>=1000 -F auid!=4294967295 -F key=privileged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eternal debate on what level of auditing system daemons (UID < 1000) should have. Dropping the auid>=1000
and auid!=......
would go beyond most US Gov requirements. That is, of course, fine... but may impact system performance by auditing everything system daemons do.
-a always,exit -F arch=b32 -S init_module -S delete_module -k modules | ||
-a always,exit -F arch=b64 -S init_module -S delete_module -k modules | ||
|
||
# Ignore all the anonymous events. Often tagged on every rule, but ignoring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shawndwells I didn't remove the auid <1000 and =-1 constraints, I just moved them to this single rule above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the PR, it shows the old line was removed:
-a always,exit -F path=/usr/sbin/semanage -F perm=x -F auid>=1000 -F auid!=4294967295 -F key=privileged-priv_change
And replaced with:
+-a always,exit -F path=/usr/sbin/semanage -F perm=x -F key=privileged-priv_change
(without the auid tags)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Derp. Looked closer and saw what you were referring to. Clever. Never saw anyone setup the rules like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't take credit, I copied it from someone else. I'm leaving my clarification comment in place for future reviewers, even though our comments "crossed", since I didn't anchor my original comment on the correct line to make it clear what I was talking about.
# up front should improve processing time | ||
-a exit,never -F auid=4294967295 | ||
# Ignore system services | ||
-a exit,never -F auid<1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shawndwells Yes, I agree they were removed from the individual rules. Lines 307 and 309 were added that ignore everything where auid<1000 or auid=-1
Since 307/309 are above the rules where the constraint was removed, those rules will never be reached for system daemons.
I committed the two sets of audit changes separately so you can (I hope?) accept or reject them individually. The typo commit should be a no-brainer.