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

tracing: add flags to reject policies with signal and override actions #2850

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bentekkie
Copy link

Fixes

Description

This patch allows disabling of the 2 action types that allow modification of syscall behaviour.
This is desirable in certain scenarios to prevent privilage escalation via loading policies via the gRPC API.

This patch implements this by adding 2 flags to the tetragon binary

  • --disable-signal-actions which causes tetragon to reject any policy
    that specifies Signal or Sigkill actions
  • --disable-override-actions which causes tetragon to reject any policy
    that specifies Override actions

Rejection was preferred over silent removal of the disabled actions from the policy as that will have less unexpected behaviour.

Changelog

Add tetragon flags `--disable-signal-actions` and `--disable-override-actions` to allow rejection of policies with `Signal` or `Override` actions.

This patch allows disabling of the 2 action types that allow
modification of syscall behaviour.
This is desirable in certain scenarios to prevent privilage escalation
via loading policies via the gRPC API.

This patch implements this by adding 2 flags to the tetragon binary
  --disable-signal-actions which causes tetragon to reject any policy
  that specifies `Signal` or `Sigkill` actions
  --disable-override-actions which causes tetragon to reject any policy
  that specifies `Override` actions

Rejection was preferred over silent removal of the disabled actions from
the policy as that will have less unexpected behaviour.

Signed-off-by: Ben Segall <[email protected]>
Copy link

netlify bot commented Aug 28, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 0e0f824
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/66cf4a784a22120008d307bb
😎 Deploy Preview https://deploy-preview-2850--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
Copy link
Member

tixxdz commented Aug 28, 2024

Note that the grpc can be restricted by --server-address unix:///var/run/tetragon/tetragon.sock owned by root.

@bentekkie
Copy link
Author

This is not sufficient if we want a client running as a user to be able to stream events from tetragon

@kkourt kkourt added the release-note/minor This PR introduces a minor user-visible change label Aug 29, 2024
Comment on lines +586 to +593
case "sigkill":
if option.Config.DisableSignalActions {
return fmt.Errorf("sigkill actions are disabled")
}
case "signal":
if option.Config.DisableSignalActions {
return fmt.Errorf("signal actions are disabled")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: combine these two cases into a single case since they seem to be checking the same thing.

Copy link
Member

Choose a reason for hiding this comment

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

I would also say, would we want to group these two options so that we don't end up with "too many flags"? Is it very likely that someone that would use one would like to use the other?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just meant that having something like:

case "sigkill", "signal":
...

@kkourt
Copy link
Contributor

kkourt commented Aug 29, 2024

Thanks for the PR. Having a switch for the use-case you describe makes sense to me. Can you please have a look at the GH action failures?

There seems to be a typo in one of the commit messages:

Warning: WARNING:TYPO_SPELLING: 'privilage' may be misspelled - perhaps 'privilege'?

And you need to run make generate-flags and commit the results for the error in https://github.com/cilium/tetragon/actions/runs/10600314787/job/29404443800?pr=2850

@tixxdz
Copy link
Member

tixxdz commented Aug 29, 2024

This is not sufficient if we want a client running as a user to be able to stream events from tetragon

Well I see two separate things here:

  • Ability to load bpf programs which is usually privileged root
  • Possibility for clients to run as users to receive events which could be achieved by separate socket configured by group ownership.

From the privilege escalation perspective: these actions are just the aggressive ones, with ebpf you can attach anywhere, could probably sniff anywhere and get an escalation without performing any action.

So I would say for time being this flag can make sense, what about following --enable-actions "post,nopost,..." ?

  • The default value is "all" meaning no restrictions, all actions are enabled.
  • List a comma separated actions that are enabled, anything not listed is disabled.

We will add new actions in the future and for this: 1. Users won't have to update their config if they care, and 2. We can't just base the fact these two actions are special where you are loading full bpf programs.

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.

4 participants