Skip to content

[Auditbeat] User metricset: Fetch groups by user#9732

Merged
cwurm merged 4 commits intoelastic:masterfrom
cwurm:groups_by_user
Jan 3, 2019
Merged

[Auditbeat] User metricset: Fetch groups by user#9732
cwurm merged 4 commits intoelastic:masterfrom
cwurm:groups_by_user

Conversation

@cwurm
Copy link
Copy Markdown
Contributor

@cwurm cwurm commented Dec 20, 2018

Currently, the user metricset reads all users, then reads all groups and their members and matches one to the other. This can be a problem when groups have a lot of members (see #9679).

This changes to looking up groups of individual users.

It also changes the types of the system.audit.user.uid and system.audit.user.gid fields from integer to keyword to accommodate Windows in the future (Go's User and Group structs use strings, so does ECS user.id/group.id).

Because the internal structure of the User struct changes, this invalidates previous beat.db files. I have not added any conversion logic this time since this metricset is not released yet - but we will have to do it in the future.

Fixes #9679.

@cwurm cwurm added review needs_backport PR is waiting to be backported to other branches. Auditbeat SecOps labels Dec 20, 2018
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 20, 2018

Pinging @elastic/secops

@cwurm cwurm requested a review from a team as a code owner December 20, 2018 19:56
webmat
webmat approved these changes Dec 21, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@webmat webmat left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The issue's paste of the error truncates the interesting bits, alas 😂

Looking at the failure in TEST-go-unit.out from the zip file (pasted below, non-truncated for posterity), it looks to me like there's actually two errors:

  • error while reading group file, which you're addressing here
  • failed to get users , from user:291

I'm not actually sure how to interpret the full stack trace. Is this two goroutines failing at roughly the same time? Or is one failure causing the other?

The tests are now passing, so this looks promising. But I think someone else should have a look as well, to get another opinion.

=== RUN   TestData
--- FAIL: TestData (0.05s)
    user_test.go:20: received error: numerical result out of range
        error while reading group file
        github.com/elastic/beats/x-pack/auditbeat/module/system/user.readGroupFile
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/x-pack/auditbeat/module/system/user/users_linux.go:153
        github.com/elastic/beats/x-pack/auditbeat/module/system/user.enrichWithGroups
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/x-pack/auditbeat/module/system/user/users_linux.go:95
        github.com/elastic/beats/x-pack/auditbeat/module/system/user.GetUsers
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/x-pack/auditbeat/module/system/user/users_linux.go:36
        github.com/elastic/beats/x-pack/auditbeat/module/system/user.(*MetricSet).reportState
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/x-pack/auditbeat/module/system/user/user.go:289
        github.com/elastic/beats/x-pack/auditbeat/module/system/user.(*MetricSet).Fetch
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/x-pack/auditbeat/module/system/user/user.go:270
        github.com/elastic/beats/metricbeat/mb/testing.ReportingFetchV2
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/metricbeat/mb/testing/modules.go:203
        github.com/elastic/beats/x-pack/auditbeat/module/system/user.TestData
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/x-pack/auditbeat/module/system/user/user_test.go:18
        testing.tRunner
        	/var/lib/jenkins/.gvm/versions/go1.11.3.linux.amd64/src/testing/testing.go:827
        runtime.goexit
        	/var/lib/jenkins/.gvm/versions/go1.11.3.linux.amd64/src/runtime/asm_amd64.s:1333
        failed to get users
        github.com/elastic/beats/x-pack/auditbeat/module/system/user.(*MetricSet).reportState
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/x-pack/auditbeat/module/system/user/user.go:291
        github.com/elastic/beats/x-pack/auditbeat/module/system/user.(*MetricSet).Fetch
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/x-pack/auditbeat/module/system/user/user.go:270
        github.com/elastic/beats/metricbeat/mb/testing.ReportingFetchV2
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/metricbeat/mb/testing/modules.go:203
        github.com/elastic/beats/x-pack/auditbeat/module/system/user.TestData
        	/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-linux/beat/x-pack/auditbeat/label/ubuntu/src/github.com/elastic/beats/x-pack/auditbeat/module/system/user/user_test.go:18
        testing.tRunner
        	/var/lib/jenkins/.gvm/versions/go1.11.3.linux.amd64/src/testing/testing.go:827
        runtime.goexit
        	/var/lib/jenkins/.gvm/versions/go1.11.3.linux.amd64/src/runtime/asm_amd64.s:1333
FAIL
FAIL	github.com/elastic/beats/x-pack/auditbeat/module/system/user	0.064s

@cwurm cwurm mentioned this pull request Dec 21, 2018
Closed
21 tasks
@webmat
Copy link
Copy Markdown
Contributor

webmat commented Dec 21, 2018

@cw Be careful about that x-pack/auditbeat/include/fields.go conflict. That file should have been deleted in #9724, so it's no longer being generated. So rebasing this will give you a merge conflict that make update won't overwrite.

Just delete that file and you're good. I just spent 1h+ figuring this out ;-)

@cwurm
Copy link
Copy Markdown
Contributor Author

cwurm commented Dec 21, 2018

Thanks @webmat - I've just rebased, hopefully successfully.

andrewkroh
andrewkroh approved these changes Jan 2, 2019
View reviewed changes
Copy link
Copy Markdown
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

@cwurm cwurm merged commit 42421e9 into elastic:master Jan 3, 2019
cwurm pushed a commit to cwurm/beats that referenced this pull request Jan 3, 2019
[Auditbeat] User metricset: Fetch groups by user (elastic#9732)
a3d069e 
Changes the user metricset to looking up groups by user instead of users by groups.

Also changes the types of the system.audit.user.uid and system.audit.user.gid fields from integer to keyword to accommodate Windows in the future.

Fixes elastic#9679.

(cherry picked from commit 42421e9)
@cwurm cwurm mentioned this pull request Jan 3, 2019
Merged
@cwurm cwurm added v6.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Jan 3, 2019
cwurm pushed a commit to cwurm/beats that referenced this pull request Jan 3, 2019
[Auditbeat] User metricset: Fetch groups by user (elastic#9732)
3202128 
Changes the user metricset to looking up groups by user instead of users by groups.

Also changes the types of the system.audit.user.uid and system.audit.user.gid fields from integer to keyword to accommodate Windows in the future.

Fixes elastic#9679.

(cherry picked from commit 42421e9)
@cwurm cwurm mentioned this pull request Jan 3, 2019
Merged
@cwurm cwurm added v6.6.0 and removed v6.7.0 labels Jan 3, 2019
cwurm pushed a commit that referenced this pull request Jan 4, 2019
[Auditbeat] Cherry-pick #9732 to 6.x: User metricset: Fetch groups by…
c7e9688 
… user (#9870)

Cherry-pick of PR #9732 to 6.x branch. Original message: 

Changes the user metricset to looking up groups by user instead of users by groups.

Also changes the types of the system.audit.user.uid and system.audit.user.gid fields from integer to keyword to accommodate Windows in the future.

Fixes #9679.
cwurm pushed a commit that referenced this pull request Jan 4, 2019
[Auditbeat] Cherry-pick #9732 to 6.6: User metricset: Fetch groups by…
4b6c147 
… user (#9872)

Cherry-pick of PR #9732 to 6.6 branch. Original message: 

Changes the user metricset to looking up groups by user instead of users by groups.

Also changes the types of the system.audit.user.uid and system.audit.user.gid fields from integer to keyword to accommodate Windows in the future.

Fixes #9679.
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
[Auditbeat] Cherry-pick elastic#9732 to 6.6: User metricset: Fetch gr…
2f4c411 
…oups by user (elastic#9872)

Cherry-pick of PR elastic#9732 to 6.6 branch. Original message: 

Changes the user metricset to looking up groups by user instead of users by groups.

Also changes the types of the system.audit.user.uid and system.audit.user.gid fields from integer to keyword to accommodate Windows in the future.

Fixes elastic#9679.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andrewkroh andrewkroh andrewkroh approved these changes

+1 more reviewer

@webmat webmat webmat approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Auditbeat review v6.6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cwurm @elasticmachine @webmat @andrewkroh