Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

tetragon: resolve uid to username for exec events from /proc fs #2588

Merged
merged 4 commits into from
Jun 26, 2024
Merged

tetragon: resolve uid to username for exec events from /proc fs #2588

merged 4 commits into from
Jun 26, 2024

Conversation

tixxdz
Copy link
Member

@tixxdz tixxdz commented Jun 20, 2024 

tetragon: resolve uid to username for exec events from /proc fs

@tixxdz tixxdz added the release-note/minor This PR introduces a minor user-visible change label Jun 20, 2024
@tixxdz tixxdz requested a review from a team as a code owner June 20, 2024 15:54
@tixxdz tixxdz requested a review from kevsecurity June 20, 2024 15:54
@tixxdz tixxdz marked this pull request as draft June 20, 2024 15:55
@tixxdz tixxdz force-pushed the pr/tixxdz/username-from-procfs branch from 270499b to 65688a6 Compare June 20, 2024 16:37
@netlify Netlify
Copy link

netlify bot commented Jun 20, 2024 •
edited
Loading

✅ Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 28ce5ef
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/667983a27256170008b4119d
😎 Deploy Preview https://deploy-preview-2588--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tixxdz tixxdz marked this pull request as ready for review June 20, 2024 16:50
@tixxdz tixxdz requested a review from mtardy as a code owner June 20, 2024 16:50
@tixxdz tixxdz requested a review from kkourt June 20, 2024 17:04
kkourt
kkourt requested changes Jun 21, 2024
View reviewed changes
Copy link
Contributor

@kkourt kkourt left a comment

Choose a reason for hiding this comment

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

Thanks, please find some comments below.

tixxdz added 4 commits June 24, 2024 15:12
@tixxdz
tetragon: resolve uid->username for processes from /proc
0e8b7f1 
Right now we resolve uid->username only for processes that
start after tetragon. To also handle the ones that start before,
we need to resolve the username during /proc fs scanning.

This patch adds userinfo module part of sensors/exec and call it
for both bpf exec sensors and proc_reader procfs scanning.

We keep same semantics if a process is not in mount and user host
namespaces we do not resolve its username.

Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
@tixxdz
tests: add test for getAccountUnix()
4aefd84 
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
@tixxdz
process: trace log when uid to username fails
80fbda1 
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
@tixxdz
process:username: metric for errors
28ce5ef 
Add two metrics:

- process_metadata_username_ignored_not_in_host_namespaces:
  This is to note that we did not perform uid->username resolution
  and it was ignored due the target process not being in mount or
  user host namespaces.

- process_metadata_username_failed: that is to note that we did
  try to resolve target process uid->username but it failed for some
  reasons.

Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
@tixxdz tixxdz force-pushed the pr/tixxdz/username-from-procfs branch from 65688a6 to 28ce5ef Compare June 24, 2024 14:33
@tixxdz tixxdz requested a review from kkourt June 24, 2024 14:34
@tixxdz
Copy link
Member Author

tixxdz commented Jun 24, 2024

Thanks, please find some comments below.

Done, thank you, PTAL!

@tixxdz
Copy link
Member Author

tixxdz commented Jun 24, 2024 •
edited
Loading

backport is here: #2603 , when merged will amend

@kkourt kkourt merged commit a1cbacb into main Jun 26, 2024
46 checks passed
@kkourt kkourt deleted the pr/tixxdz/username-from-procfs branch June 26, 2024 14:52
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@kkourt kkourt kkourt approved these changes

@kevsecurity kevsecurity Awaiting requested review from kevsecurity kevsecurity is a code owner automatically assigned from cilium/tetragon

@mtardy mtardy Awaiting requested review from mtardy mtardy is a code owner

Assignees
No one assigned
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tixxdz @kkourt