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

policylibrary: add a catch all for setuid root and suid execution #1706

Merged
merged 3 commits into from
Nov 24, 2023

Conversation

tixxdz
Copy link
Member

@tixxdz tixxdz commented Nov 2, 2023

No description provided.

@tixxdz tixxdz requested a review from a team as a code owner November 2, 2023 23:34
@tixxdz tixxdz requested a review from olsajiri November 2, 2023 23:34
@tixxdz tixxdz changed the title policylibrary: add a catch all for setuid syscall interfaces policylibrary: add a catch all for setuid root Nov 2, 2023
@tixxdz tixxdz force-pushed the pr/tixxdz/policylibrary-setuid-family branch from 9d16f3b to c29634a Compare November 2, 2023 23:35
@tixxdz tixxdz added the release-note/minor This PR introduces a minor user-visible change label Nov 8, 2023
@tixxdz tixxdz force-pushed the pr/tixxdz/policylibrary-setuid-family branch from c29634a to 5686f7a Compare November 12, 2023 22:27
@tixxdz tixxdz requested a review from mtardy as a code owner November 12, 2023 22:27
@tixxdz tixxdz requested a review from kkourt November 12, 2023 22:27
Copy link

netlify bot commented Nov 12, 2023

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 10b9707
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/655b6fe94f2bec0008984df8
😎 Deploy Preview https://deploy-preview-1706--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 changed the title policylibrary: add a catch all for setuid root policylibrary: add a catch all for setuid root and suid execution Nov 13, 2023
@tixxdz tixxdz force-pushed the pr/tixxdz/policylibrary-setuid-family branch 2 times, most recently from a2fa14f to 963a113 Compare November 13, 2023 08:35
Comment on lines 175 to 176
Detecting [setuid()](https://www.man7.org/linux/man-pages/man2/setuid.2.html) and [setgid()](https://www.man7.org/linux/man-pages/man2/setgid.2.html) calls that set the user ID or group ID to root is a common
best-practice to identify when privileges are raised or still elevated.
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between raised or "still elevated"? Should we just keep raised?

Copy link
Member Author

Choose a reason for hiding this comment

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

I kept raised for simplicity and let users can parse the events...

We also report case when an already effective uid 0 do setuid(0). For now it is fine, as we don't have the filters based on uids, but this is planned next to reduce noise anyway.

# - __ia32_sys_setfsgid
# - __x64_sys_setfsgid16
# - __ia32_sys_setfsgid16
#
Copy link
Contributor

Choose a reason for hiding this comment

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

do we miss seteuid/setegid ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The seteuid/setegid are usually library that call syscall setreuid and setregid which is handled

Copy link
Contributor

Choose a reason for hiding this comment

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

hm, but it's still separate syscall right? should we cover any possible way? I might misunderstood the purpose of that spec though

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi Jiri so the seteuid/setegid are wrappers https://www.man7.org/linux/man-pages/man2/seteuid.2.html check c library, internally they are calling the real system calls setreuid/setregid (there is no seteuid syscall). In this tracing library we handle all the syscalls around that, if you check, the setreuid/setregid are covered ;-)

Thank you Jiri for the review ;-)

@tixxdz tixxdz force-pushed the pr/tixxdz/policylibrary-setuid-family branch from 963a113 to 10b9707 Compare November 20, 2023 14:40
@tixxdz tixxdz merged commit 2d9c11e into main Nov 24, 2023
33 of 34 checks passed
@tixxdz tixxdz deleted the pr/tixxdz/policylibrary-setuid-family branch November 24, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants