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

tetragon: store caps during fork #2275

Merged
merged 5 commits into from
Apr 3, 2024
Merged

Conversation

tixxdz
Copy link
Member

@tixxdz tixxdz commented Mar 30, 2024

No description provided.

This moves the definition of MsgGenericCred from tracinapi to
processapi. No logic change here.

This is preparation for next patches where we will use the
MsgGenericCred to send execve and clone event from bpf.

Then at the end we will remove the MsgGenericCredMinimal that was
used a temporary solution to improve things, and make Tetragon
send full creds during execve and clone.

Signed-off-by: Djalal Harouni <[email protected]>
@tixxdz tixxdz requested a review from a team as a code owner March 30, 2024 15:39
@tixxdz tixxdz requested a review from mtardy March 30, 2024 15:39
@tixxdz tixxdz added the release-note/minor This PR introduces a minor user-visible change label Mar 30, 2024
@tixxdz tixxdz marked this pull request as draft March 30, 2024 15:51
This change moves the bpf execve event from using msg_cred_minimal
to use the msg_cred to send the credentials which includes sending
capabilities part of msg_cred. As credentials are represented by
both caps, uids, and the user namespace owner.

This does not change the proto API event exported to users through
gRPC, but just the internal representation between bpf<->userspace.

Also remove the msg_cred_minimal now that was used as a temporary
solution.

Signed-off-by: Djalal Harouni <[email protected]>
Store the thread leader caps during fork so we can check later
if capabilities changed during the execve, just before the execve hook
point where we collect new capabilities and a new exec_id.

Right now during fork they are zeroed in the execve_map which make it
unreliable to detect if they changed between the fork and the final
execve.

Any hook just before the execve could report that they changed even if
they did not, this is the case of a privileged that is as an example
executing sudo or su.

An event will be generated that capabilities did change (raised) but in
reality they did not. As they were zero'ed the caps change comparison
will be performed against zero which will always succeed.

Fix this by storing caps during fork, and ensure that we only report
positive cases.

Signed-off-by: Djalal Harouni <[email protected]>
@tixxdz tixxdz marked this pull request as ready for review April 1, 2024 15:57
@tixxdz tixxdz changed the title wip tetragon: store caps during fork Apr 1, 2024
@kkourt kkourt self-requested a review April 1, 2024 16:08
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.

Cool, thanks!

I have on question. You mention:

so we can check later if capabilities changed during the execve,

Where is this check happening?

@tixxdz
Copy link
Member Author

tixxdz commented Apr 3, 2024

Cool, thanks!

I have on question. You mention:

so we can check later if capabilities changed during the execve,

Where is this check happening?

On the exec path on current, just after commit_cred() and before our exec hook, this is our window.

@tixxdz tixxdz merged commit 91ec63f into main Apr 3, 2024
40 checks passed
@tixxdz tixxdz deleted the pr/tixxdz/2024-03-clone-fixes branch April 3, 2024 10:10
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.

2 participants