tracing: add flags to reject policies with signal and 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 <bentekkie@google.com>

docs/content/en/docs/concepts/enforcement/_index.md

@@ -20,6 +20,9 @@ security check functions allow to change their return value in this manner. Deta
 can configure tracing policies to override the return value can be found in the [Override
 action]({{< ref "/docs/concepts/tracing-policy/selectors#override-action" >}}) documentation.
 
+This behaviour can be disabled at the daemon level by setting the `--disable-override-actions` flag
+which may be desirable to prevent privilage escalation via the gRPC API.
+
 ## Signals
 
 Another type of enforcement is signals. For example, users can write a TracingPolicy (details can be
@@ -33,3 +36,6 @@ However, it does ensure that the process is terminated synchronously (and any th
 stopped). In some cases it may be sufficient to ensure the process is stopped and the process does
 not handle the return of the call. To ensure the operation is not completed, though, the `Signal`
 action should be combined with the `Override` action.
+
+This behaviour can be disabled at the daemon level by setting the `--disable-signal-actions` flag
+which may be desirable to prevent privilage escalation via the gRPC API.

pkg/option/config.go

@@ -32,6 +32,9 @@ type config struct {
 
 	DisableKprobeMulti bool
 
+	DisableOverrideActions bool
+	DisableSignalActions   bool
+
 	GopsAddr string
 
 	// On start used to store bpf prefix for --bpf-dir option,

pkg/option/flags.go

@@ -73,6 +73,9 @@ const (
 	KeyDisableKprobeMulti = "disable-kprobe-multi"
 	KeyDisableUprobeMulti = "disable-uprobe-multi"
 
+	KeyDisableSignalActions   = "disable-signal-actions"
+	KeyDisableOverrideActions = "disable-override-actions"
+
 	KeyRBSize      = "rb-size"
 	KeyRBSizeTotal = "rb-size-total"
 	KeyRBQueueSize = "rb-queue-size"
@@ -146,6 +149,9 @@ func ReadAndSetFlags() error {
 
 	Config.DisableKprobeMulti = viper.GetBool(KeyDisableKprobeMulti)
 
+	Config.DisableSignalActions = viper.GetBool(KeyDisableSignalActions)
+	Config.DisableOverrideActions = viper.GetBool(KeyDisableOverrideActions)
+
 	var err error
 
 	if Config.RBSize, err = strutils.ParseSize(viper.GetString(KeyRBSize)); err != nil {
@@ -353,6 +359,10 @@ func AddFlags(flags *pflag.FlagSet) {
 	// Allow to disable kprobe multi interface
 	flags.Bool(KeyDisableKprobeMulti, false, "Allow to disable kprobe multi interface")
 
+	// Allow to disable override and signall selectors
+	flags.Bool(KeyDisableOverrideActions, false, "Allow to disable override action selectors")
+	flags.Bool(KeyDisableSignalActions, false, "Allow to disable sigkill and signal selectors")
+
 	// Allow to specify perf ring buffer size
 	flags.String(KeyRBSizeTotal, "0", "Set perf ring buffer size in total for all cpus (default 65k per cpu, allows K/M/G suffix)")
 	flags.String(KeyRBSize, "0", "Set perf ring buffer size for single cpu (default 65k, allows K/M/G suffix)")

pkg/sensors/tracing/generickprobe.go

@@ -450,6 +450,10 @@ func preValidateKprobes(name string, kprobes []v1alpha1.KProbeSpec, lists []v1al
 	for i := range kprobes {
 		f := &kprobes[i]
 
+		if err := validateActionSelectors(f.Selectors); err != nil {
+			return fmt.Errorf("validation failed: %w", err)
+		}
+
 		var list *v1alpha1.ListSpec
 
 		// the f.Call is either defined as list:NAME

pkg/sensors/tracing/genericlsm.go

@@ -146,6 +146,9 @@ func handleGenericLsm(r *bytes.Reader) ([]observer.Event, error) {
 }
 
 func isValidLsmSelectors(selectors []v1alpha1.KProbeSelector) error {
+	if err := validateActionSelectors(selectors); err != nil {
+		return fmt.Errorf("validation failed: %w", err)
+	}
 	for _, s := range selectors {
 		if len(s.MatchReturnArgs) > 0 {
 			return fmt.Errorf("MatchReturnArgs selector is not supported")

pkg/sensors/tracing/generictracepoint.go

@@ -371,6 +371,9 @@ func createGenericTracepointSensor(
 
 	tracepoints := make([]*genericTracepoint, 0, len(confs))
 	for i := range confs {
+		if err := validateActionSelectors(confs[i].Selectors); err != nil {
+			return nil, fmt.Errorf("validation failed: %w", err)
+		}
 		tp, err := createGenericTracepoint(name, &confs[i], policyID, policyName, customHandler)
 		if err != nil {
 			return nil, err

pkg/sensors/tracing/genericuprobe.go

@@ -495,6 +495,12 @@ func (k *observerUprobeSensor) PolicyHandler(
 		return nil, fmt.Errorf("uprobe sensor does not implement policy filtering")
 	}
 
+	for _, probe := range spec.UProbes {
+		if err := validateActionSelectors(probe.Selectors); err != nil {
+			return nil, fmt.Errorf("validation failed: %w", err)
+		}
+	}
+
 	name := fmt.Sprintf("gup-sensor-%d", atomic.AddUint64(&sensorCounter, 1))
 	policyName := p.TpName()
 	return createGenericUprobeSensor(name, spec, policyName)

pkg/sensors/tracing/kprobe_validation_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/cilium/tetragon/pkg/kernels"
+	"github.com/cilium/tetragon/pkg/option"
 	"github.com/cilium/tetragon/pkg/sensors"
 	"github.com/cilium/tetragon/pkg/tracingpolicy"
 	"github.com/stretchr/testify/assert"
@@ -205,6 +206,147 @@ spec:
 	assert.Error(t, err)
 }
 
+func TestKprobeValidationOverrideDisabled(t *testing.T) {
+
+	// override on when override actions are disabled
+
+	crd := `
+apiVersion: cilium.io/v1alpha1
+kind: TracingPolicy
+metadata:
+  name: "list-syscalls"
+spec:
+  kprobes:
+  - call: "sys_symlinkat"
+    selectors:
+    - matchActions:
+      - action: Override
+        argError: -1
+`
+
+	oldDisableOverrideActions := option.Config.DisableOverrideActions
+	option.Config.DisableOverrideActions = true
+	t.Cleanup(func() {
+		option.Config.DisableOverrideActions = oldDisableOverrideActions
+	})
+	err := checkCrd(t, crd)
+	assert.Error(t, err)
+}
+
+func TestKprobeValidationSignalDisabled(t *testing.T) {
+
+	// signal when signal actions are disabled
+
+	crd := `
+apiVersion: cilium.io/v1alpha1
+kind: TracingPolicy
+metadata:
+  name: "list-syscalls"
+spec:
+  kprobes:
+  - call: "sys_symlinkat"
+    selectors:
+    - matchActions:
+      - action: Signal
+        argSig: 9
+`
+
+	oldDisableSignalActions := option.Config.DisableSignalActions
+	option.Config.DisableSignalActions = true
+	t.Cleanup(func() {
+		option.Config.DisableSignalActions = oldDisableSignalActions
+	})
+	err := checkCrd(t, crd)
+	assert.Error(t, err)
+}
+
+func TestKprobeValidationSigkillDisabled(t *testing.T) {
+
+	// sigkill when signal actions are disabled
+
+	crd := `
+apiVersion: cilium.io/v1alpha1
+kind: TracingPolicy
+metadata:
+  name: "list-syscalls"
+spec:
+  kprobes:
+  - call: "sys_symlinkat"
+    selectors:
+    - matchActions:
+      - action: Sigkill
+        argSig: 9
+`
+
+	oldDisableSignalActions := option.Config.DisableSignalActions
+	option.Config.DisableSignalActions = true
+	t.Cleanup(func() {
+		option.Config.DisableSignalActions = oldDisableSignalActions
+	})
+	err := checkCrd(t, crd)
+	assert.Error(t, err)
+}
+
+func TestKprobeValidationNotifyEnforcerSignalDisabled(t *testing.T) {
+
+	// signal via notify enforcer when signal actions are disabled
+
+	crd := `
+apiVersion: cilium.io/v1alpha1
+kind: TracingPolicy
+metadata:
+  name: "list-syscalls"
+spec:
+  enforcers:
+  - calls:
+    - "sys_symlinkat"
+  kprobes:
+  - call: "sys_symlinkat"
+    selectors:
+    - matchActions:
+      - action: NotifyEnforcer
+        argSig: 9
+`
+
+	oldDisableSignalActions := option.Config.DisableSignalActions
+	option.Config.DisableSignalActions = true
+	t.Cleanup(func() {
+		option.Config.DisableSignalActions = oldDisableSignalActions
+	})
+	err := checkCrd(t, crd)
+	assert.Error(t, err)
+}
+
+func TestKprobeValidationNotifyEnforcerOverrideDisabled(t *testing.T) {
+
+	// override via notify enforcer when overide actions are disabled
+
+	crd := `
+apiVersion: cilium.io/v1alpha1
+kind: TracingPolicy
+metadata:
+  name: "list-syscalls"
+spec:
+  enforcers:
+  - calls:
+    - "sys_symlinkat"
+  kprobes:
+  - call: "sys_symlinkat"
+    selectors:
+    - matchActions:
+      - action: NotifyEnforcer
+        argError: -1
+`
+
+	oldDisableOverrideActions := option.Config.DisableOverrideActions
+	option.Config.DisableOverrideActions = true
+	t.Cleanup(func() {
+		option.Config.DisableOverrideActions = oldDisableOverrideActions
+	})
+	err := checkCrd(t, crd)
+	assert.Error(t, err)
+}
+
 func TestKprobeValidationNonSyscallOverride(t *testing.T) {
 
 	// override on non syscall (non override-able) function