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

Add events 4720 4722 4723 4724 4725 4726 4738 4740 4767 4781 - LogonID correlation #13530

Merged
merged 13 commits into from
Oct 15, 2019
Merged

Conversation

janniten
Copy link
Contributor

@janniten janniten commented Sep 6, 2019

Added User management events

Event Description
4720 A user account was created
4722 A user account was enabled
4723 An attempt was made to change an account's password
4724 An attempt was made to reset an account's password
4725 An user account was disabled
4726 An user account was deleted
4738 An user account was changed
4740 An user account was locked out
4767 An account was unlocked
4781 The name of an account was changed

The function addLogonIds in order to populate winlog.logon.id was remove and replaced by two functions copySubjectUserLogonId and copyTargettUserLogonId so we can put only the value that has sense/meaning in the context of the event and use that value to correlate.
(https://discuss.elastic.co/t/winlogbeat-new-ecs-fields-and-security-module-questions/190479/13)
I have also added a function to add the event.action for this events
image50%

@janniten janniten requested a review from a team as a code owner September 6, 2019 14:39
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

1 similar comment
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@janniten
Copy link
Contributor Author

janniten commented Sep 10, 2019

Travis CI failed.
It is not clear for me if the problem is in my code or if i did something wrong

make[1]: *** [system-tests-environment] Error 2
make[1]: Leaving directory /home/travis/gopath/src/github.com/elastic/beats/filebeat' make: *** [testsuite] Error 2 make: Leaving directory /home/travis/gopath/src/github.com/elastic/beats/filebeat'
The command "make $TARGETS" exited with 2.

Any help will be appreciated

@andrewkroh andrewkroh self-requested a review September 11, 2019 01:27
@andrewkroh
Copy link
Member

jenkins, test it

You're PR probably didn't cause the Travis failure since it doesn't test Windows. I'll trigger the Jenkins testing which will test Winlogbeat on Windows.

@andrewkroh
Copy link
Member

jenkins, test this

1 similar comment
@adriansr
Copy link
Contributor

adriansr commented Oct 1, 2019

jenkins, test this

@andrewkroh andrewkroh added module review Winlogbeat x-pack Issues and pull requests for X-Pack features. Team:SIEM labels Oct 1, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

I'll update the golden files and push them into this PR.

@@ -1206,6 +1271,36 @@ var security = (function () {
// 4672 - Special privileges assigned to new logon.
4672: event4672.Run,

// 4720 - A user account was created
4720: userMgmtEvts.Run,
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a spacing issue due to tabs. Let's replace all the tabs with spaces.

@janniten janniten requested review from a team as code owners October 1, 2019 15:51
@ycombinator ycombinator removed the request for review from a team October 1, 2019 15:54
@andrewkroh
Copy link
Member

@janniten I rebased the PR branch on the latest version of the master branch. You'll probably need to reset your local branch to be able to pull down the update.

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

There are a few changes needed, but overall LGTM. Thanks! After you make the requested updates I can go ahead and run the cd x-pack\winlogbeat\module\security and go test ./... -update for you to regenerate the golden files for us to review.

Once Elastic Common Schema (ECS) defines a set of event.category values we'll come back to these modules and update them appropriately.

{from: "winlog.event_data.TargetLogonId", to: "winlog.logon.id"},
],
ignore_missing: true,
mode: "rename"
Copy link
Member

Choose a reason for hiding this comment

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

You can remove the mode: "rename" so that the Convert processor will actually copy the value as suggested by copyTargetUserLogonId. Same thing with copySubjectUserLogonId.

@@ -1076,17 +1101,7 @@ var security = (function () {

// Add logon IDs to winlog.logon.id to make it easy to find all activity
Copy link
Member

Choose a reason for hiding this comment

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

This comment should be removed with the function it accompanied.

@janniten
Copy link
Contributor Author

janniten commented Oct 2, 2019

There are a few changes needed, but overall LGTM. Thanks! After you make the requested updates I can go ahead and run the cd x-pack\winlogbeat\module\security and go test ./... -update for you to regenerate the golden files for us to review.

Once Elastic Common Schema (ECS) defines a set of event.category values we'll come back to these modules and update them appropriately.

I have made the requested updates. Please let me know If I should do something more.

@andrewkroh
Copy link
Member

jenkins, test this

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM. I update the docs with the new event IDs.

@andrewkroh andrewkroh requested a review from webmat October 9, 2019 12:00
@@ -19,6 +19,22 @@ var security = (function () {
"11": "CachedInteractive",
};

var eventActionTypes = {
Copy link
Member

@andrewkroh andrewkroh Oct 9, 2019

Choose a reason for hiding this comment

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

@webmat This is adding these event.action values based on the event ID. Do you want these action values or would you like to hold off until the action values are more well defined for ECS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewkroh @webmat I was thinking about that point this morning. Personally i think is is better to define the event.action as defined in for auditbeat (some of them can be maybe reused). For example: event.action -> deleted-group-account-from
what do you thing? I can modify the event.action in order to match or follow the same criteria as auditbeat

Copy link
Contributor Author

@janniten janniten Oct 14, 2019

Choose a reason for hiding this comment

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

I took the events actions from auditbeat'a logs and I think these event.action are more suitable.

Event event.action
4624 logged-in
4625 logon-failed
4634 logged-out
4672 logged-in-special
4720 added-user-account
4722 enabled-user-account
4723 changed-password
4724 reseted-password
4725 disabled-user-account
4726 deleted-user-account
4738 modified-user-account
4740 locked-out-user-account
4767 unlocked-user-account
4781 renamed-user-account

@andrewkroh @webmat What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

The field event.action is not reserved, and can therefore be defined each source. So no limitations coming from ECS in that regard :-)

These values for event.action look fine to me 👍

Copy link
Contributor Author

@janniten janniten Oct 14, 2019

Choose a reason for hiding this comment

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

@andrewkroh do I change the event.actions to the ones in the table above or should I keep the previous ones?
I have already write the code to support the events for group management (**) and some enhancements in order to map the UAC values to a human readable form for events 4720 y 4738. If you prefer we can continue with this PR and once closed, I can create a new one with the new event.actions, new events support and the UAC enhancements.
What do you think?

Copy link
Member

@andrewkroh andrewkroh Oct 14, 2019

Choose a reason for hiding this comment

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

I like the proposed ones that match up with auditbeat. Can you please change them in this PR? Then I'll push an update with the golden files.

The UAC fields can be done in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

janniten and others added 2 commits October 14, 2019 17:24
Update golden files.
Update asciidocs for module.
Fixed past tense of "reset" too.
@andrewkroh
Copy link
Member

jenkins, test this

@andrewkroh
Copy link
Member

jenkins, test this

@andrewkroh
Copy link
Member

jenkins, test this

@andrewkroh andrewkroh merged commit 62dfeb2 into elastic:master Oct 15, 2019
@janniten janniten deleted the usr_mgm_evt branch October 15, 2019 14:30
@urso urso added the v7.5.0 label Oct 22, 2019
andrewkroh added a commit that referenced this pull request Feb 5, 2020
…module (#15217)

Added Audit and Log Management related events, Computer Object Management Events, Distribution Groups Events. Changed user.name field for user management events and related.user mapping.

New Events

Due to that Windows events are the source of information for Winlogbeat the events 1100, 1102, 1104, 1105, 1108 and 4719 has been added in order to monitor changes in the audit policy configuration, log deletion and other failures in the log subsystem.

For event 4719, a human readable description was added in order to know which setting was modified (winlog.event_data.SubCategory) and to which value (winlog.event_data.AuditPolicyChangesDescription).

Distribution Groups (Security-Disabled) Management Events were added. Those events are processed in the same way and with the same function that Security Groups (#14299). In order to add information about the nature of the group being managed the type (Security-Disabled/Security-Enabled) and scope (Local,Global,Universal) where added as winlog.group.type and winlog.group.scope.

ComputerObject Management events were also added.

Changes to ECS mappings

In elastic/ecs#678 and elastic/ecs#589 we have been discussing how n-ary relationship between users in an event should be named and mapping into ECS. In #13530 winlog.event_data.TargetUserName has been mapped to user.name but from the reasons exposed in elastic/ecs#678 and elastic/ecs#589 the mapping winlog.event_data.SubjectUserName -> user.name is more appropriate. This mapping was changed.

Also, with the adding of related fields in ECS 1.3 and specifically the related.user field (elastic/ecs#694) all the user names appearing in one event were mapped to the related user events. Every time a SubjectUserName or TargetUserName is copied also is added to the related.user field, as well as other users appearing in the event.

Event test data were added for all events with the exception of event 1108 which I was not able to reproduce.

Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Andrew Kroh <[email protected]>
andrewkroh pushed a commit to andrewkroh/beats that referenced this pull request Mar 18, 2020
…module (elastic#15217)

Added Audit and Log Management related events, Computer Object Management Events, Distribution Groups Events. Changed user.name field for user management events and related.user mapping.

New Events

Due to that Windows events are the source of information for Winlogbeat the events 1100, 1102, 1104, 1105, 1108 and 4719 has been added in order to monitor changes in the audit policy configuration, log deletion and other failures in the log subsystem.

For event 4719, a human readable description was added in order to know which setting was modified (winlog.event_data.SubCategory) and to which value (winlog.event_data.AuditPolicyChangesDescription).

Distribution Groups (Security-Disabled) Management Events were added. Those events are processed in the same way and with the same function that Security Groups (elastic#14299). In order to add information about the nature of the group being managed the type (Security-Disabled/Security-Enabled) and scope (Local,Global,Universal) where added as winlog.group.type and winlog.group.scope.

ComputerObject Management events were also added.

Changes to ECS mappings

In elastic/ecs#678 and elastic/ecs#589 we have been discussing how n-ary relationship between users in an event should be named and mapping into ECS. In elastic#13530 winlog.event_data.TargetUserName has been mapped to user.name but from the reasons exposed in elastic/ecs#678 and elastic/ecs#589 the mapping winlog.event_data.SubjectUserName -> user.name is more appropriate. This mapping was changed.

Also, with the adding of related fields in ECS 1.3 and specifically the related.user field (elastic/ecs#694) all the user names appearing in one event were mapped to the related user events. Every time a SubjectUserName or TargetUserName is copied also is added to the related.user field, as well as other users appearing in the event.

Event test data were added for all events with the exception of event 1108 which I was not able to reproduce.

Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Andrew Kroh <[email protected]>
(cherry picked from commit e624aef)
andrewkroh added a commit that referenced this pull request Mar 18, 2020
…ent Events - ECS related.user field mapping (#17090)

Added Audit and Log Management related events, Computer Object Management Events, Distribution Groups Events. Changed user.name field for user management events and related.user mapping.

New Events

Due to that Windows events are the source of information for Winlogbeat the events 1100, 1102, 1104, 1105, 1108 and 4719 has been added in order to monitor changes in the audit policy configuration, log deletion and other failures in the log subsystem.

For event 4719, a human readable description was added in order to know which setting was modified (winlog.event_data.SubCategory) and to which value (winlog.event_data.AuditPolicyChangesDescription).

Distribution Groups (Security-Disabled) Management Events were added. Those events are processed in the same way and with the same function that Security Groups (#14299). In order to add information about the nature of the group being managed the type (Security-Disabled/Security-Enabled) and scope (Local,Global,Universal) where added as winlog.group.type and winlog.group.scope.

ComputerObject Management events were also added.

Changes to ECS mappings

In elastic/ecs#678 and elastic/ecs#589 we have been discussing how n-ary relationship between users in an event should be named and mapping into ECS. In #13530 winlog.event_data.TargetUserName has been mapped to user.name but from the reasons exposed in elastic/ecs#678 and elastic/ecs#589 the mapping winlog.event_data.SubjectUserName -> user.name is more appropriate. This mapping was changed.

Also, with the adding of related fields in ECS 1.3 and specifically the related.user field (elastic/ecs#694) all the user names appearing in one event were mapped to the related user events. Every time a SubjectUserName or TargetUserName is copied also is added to the related.user field, as well as other users appearing in the event.

Event test data were added for all events with the exception of event 1108 which I was not able to reproduce.

Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Andrew Kroh <[email protected]>
Co-authored-by: Anabella Cristaldi <[email protected]>

(cherry picked from commit e624aef)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module review v7.5.0 Winlogbeat x-pack Issues and pull requests for X-Pack features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants