-
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
libbeat/processors/add_process_metada: add capabilities to process me… #38252
Conversation
…tadata Extends process metadata with effective and permitted capabilities.
Pinging @elastic/sec-linux-platform (Team:Security-Linux Platform) |
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
while here zap the extra newline
The Windows errors seem to be unrelated, apparently the CI can't find the go binary and whatnot |
Pinging @elastic/elastic-agent (Team:Elastic-Agent) |
libbeat/processors/add_process_metadata/add_process_metadata_test.go
Outdated
Show resolved
Hide resolved
BuildKite should not prevent the PR from be merged yet. Once you get the approvals and Jenkins is green you should be able to merge it. |
…est.go Co-authored-by: Tiago Queiroz <tiago.queiroz@elastic.co>
Awesome, thanks :) |
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
Merging is blocked as waiting a review from @fearful-symmetry , is this because we need a reviewer from each team? |
💚 Build Succeeded
History
cc @haesbaert |
💚 Build Succeeded
History
cc @haesbaert |
💚 Build Succeeded
History
cc @haesbaert |
💚 Build Succeeded
History
cc @haesbaert |
💚 Build Succeeded
History
cc @haesbaert |
💚 Build Succeeded
History
cc @haesbaert |
💚 Build Succeeded
History
cc @haesbaert |
💚 Build Succeeded
History
cc @haesbaert |
💚 Build Succeeded
History
cc @haesbaert |
💚 Build Succeeded
History
cc @haesbaert |
@@ -52,17 +53,22 @@ func (p gosysinfoProvider) GetProcessMetadata(pid int) (result *processMetadata, | |||
} | |||
} | |||
|
|||
capPermitted, _ := capabilities.FromPid(capabilities.Permitted, pid) | |||
capEffective, _ := capabilities.FromPid(capabilities.Effective, pid) |
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.
Is there a reason why we're not checking the errors here?
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.
If it fails, we don't include the capabilities, same outcome if the capabilities were empty. There are valid/normal reasons for it to fail, like the process being gone by the time we try to fetch it. Also on Windows/OS X i fails with ErrUnsupported.
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.
Can we add a quick comment to that effect?
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.
/test |
Proposed commit message
Extends process metadata with effective and permitted capabilities.
Errors from
capabilities.FromPid()
are ignored since it returns a nil slice, which results in len() == 0 which supresses any output. A possible common error is getting ESRCH as the process might have already exited.Checklist
My code follows the style guidelines of this projectI have made corresponding changes to the documentationI have made corresponding change to the default configuration filesCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally
Related issues
Part of https://github.com/elastic/security-team/issues/4375
Related to #37453
Screenshots
output.txt