-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Auditbeat] normalized event.type/category/outcome fields for auditd module #11432
[Auditbeat] normalized event.type/category/outcome fields for auditd module #11432
Conversation
Pinging @elastic/secops |
@@ -473,6 +473,7 @@ func buildMetricbeatEvent(msgs []*auparse.AuditMessage, config Config) mb.Event | |||
"event": common.MapStr{ | |||
"category": auditEvent.Category.String(), | |||
"action": auditEvent.Summary.Action, | |||
"outcome": auditEvent.Result, |
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.
Does this need normalized too? We want to use failure
or success
.
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.
Done. Replaced fail
with failure
.
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.
For consistency I think it would make sense to substitute failure
for fail
all of the time. WDYT? It makes the breaking change a little bit bigger but I think it's a win for users.
action common.MapStr | ||
}{ | ||
{ | ||
fields: common.MapStr{ |
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.
Would these maps be continuously allocated for each event it processes?
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.
Yes, I improved it, thanks
jenkins, test this |
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.
LGTM
4d71035
to
6f3add3
Compare
I've realised the audit messages I was using, I'm testing if USER_AUTH works in all cases. |
After a quick investigation this is what I observe:
So the issue here is, if we also want to catch elevations from the console using su/sudo then it's necessary to also set |
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 comment is from yesterday. Just realized I had not submitted it yet. Still need to review the user_auth / user_login table.
@@ -473,6 +473,7 @@ func buildMetricbeatEvent(msgs []*auparse.AuditMessage, config Config) mb.Event | |||
"event": common.MapStr{ | |||
"category": auditEvent.Category.String(), | |||
"action": auditEvent.Summary.Action, | |||
"outcome": auditEvent.Result, |
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.
For consistency I think it would make sense to substitute failure
for fail
all of the time. WDYT? It makes the breaking change a little bit bigger but I think it's a win for users.
@adriansr That table is really helpful. Thank you for doing the research. I don't see an alternative, so I think we have to categorize both user_auth and user_login. @FrankHassanabad WDYT? I guess this could cause some of the authentication counts to be less accurate. |
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.
LGTM. Let's wait to hear from @FrankHassanabad regarding the earlier ^ question.
if !ok1 || !ok2 || !ok3 { | ||
return | ||
} | ||
if category == "user-login" && |
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 love the simplicity.
840638f
to
4a39983
Compare
Duplicates without knowing why they're duplicates would be really bad. Users of the system would def. ask questions and trust the system less if they started noticing all their users logging in were showing up twice as success. Alerts could be fired twice on a failure as well. I would imagine as a user they would want to query/show login attempts separate from account escalations so it would also be weird/bad if we had a system locked down to where root is absolutely not allowed to login but then we start to see what looks like root logging in from the outside when in fact it's that security trick where you have to login as your regular user account first and then do a We can use I would imagine threat hunters would first be looking for password attempts from outside -> in, and then the second would be legitimate creds are already on the system but are trying to gain escalation or account changes. If I were writing rules, I would know that bad password attempts are going to be a constant noisy thing (such as morning when users are all trying to login) and not setup an alert for that. But for more than say 2 privilege escalation failures within a day I would setup an alert on that as that should be something rare occurring. So keeping the two separate is a good thing and just doing logins as authenticated is a good starting point IMHO. |
Thanks for your response @FrankHassanabad , it makes complete sense. I've updated the PR to stop mapping USER_AUTH to authentication category, pending a further discussion on how to track privilege gains. |
The auditd module is updated to set normalized values for event.category and event.type. It also sets event.outcome from auditd.result. Currently it sets the following values for audit messages of USER_LOGIN and USER_AUTH type: | event.category | event.outcome | event.type | |----------------|---------------|------------------------| | authentication | success | authentication_success | | authentication | failure | authentication_failure | Closes elastic#11428
923728a
to
65f1f57
Compare
The auditd module is updated to set normalized values for event.category
and event.type. It also sets event.outcome from auditd.result.
Currently it supports the following values:
Closes #11428