From 4e547c4b79750d7aab939e953ca918f59b5efad7 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 19 Jan 2024 21:25:52 +0000 Subject: [PATCH 1/8] tetragon: Return error in case we failed to compile pattern To get error value for wrong pattern. Signed-off-by: Jiri Olsa --- pkg/ftrace/ftrace.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/ftrace/ftrace.go b/pkg/ftrace/ftrace.go index a72ab2330e6..a855de0ad07 100644 --- a/pkg/ftrace/ftrace.go +++ b/pkg/ftrace/ftrace.go @@ -48,7 +48,10 @@ func ReadAvailFuncs(pattern string) ([]string, error) { var r *regexp.Regexp if pattern != "" { - r, _ = regexp.Compile(pattern) + r, err = regexp.Compile(pattern) + if err != nil { + return nil, err + } } final := []string{} From 8cbd36a9b4aa67dfe41834d182ef0d014e217b47 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 19 Jan 2024 21:27:08 +0000 Subject: [PATCH 2/8] tetragon: Log program attach string when loading Adding extra Attach field to the loading log message, from: logcapture.go:25: time="2024-01-22T08:53:45Z" level=info msg="Loading BPF program" Program=/home/jolsa/tetragon/bpf/objs/bpf_exit.o Type=kprobe logcapture.go:25: time="2024-01-22T08:53:45Z" level=info msg="Loading BPF program" Program=/home/jolsa/tetragon/bpf/objs/bpf_fork.o Type=kprobe time="2024-01-22T08:55:09Z" level=info msg="tetragon, map loaded." map=killer_data path=/sys/fs/bpf/tetragon/killer_data sensor=__killer__ time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:55:09Z" level=info msg="Loading registered BPF probe" Program=bpf/objs/bpf_fmodret_killer.o Type=killer to following: logcapture.go:25: time="2024-01-22T08:51:37Z" level=info msg="Loading BPF program" Attach=acct_process Program=/home/jolsa/tetragon/bpf/objs/bpf_exit.o Type=kprobe logcapture.go:25: time="2024-01-22T08:51:37Z" level=info msg="Loading BPF program" Attach=wake_up_new_task Program=/home/jolsa/tetragon/bpf/objs/bpf_fork.o Type=kprobe time="2024-01-22T08:57:12Z" level=info msg="tetragon, map loaded." map=killer_data path=/sys/fs/bpf/tetragon/killer_data sensor=__killer__ time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_audit_rule_free Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_audit_rule_init Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_audit_rule_known Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_audit_rule_match Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_binder_set_context_mgr Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_binder_transaction Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_binder_transfer_binder Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_binder_transfer_file Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_bounded_transition Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_bpf Program=bpf/objs/bpf_fmodret_killer.o Type=killer time="2024-01-22T08:57:12Z" level=info msg="Loading registered BPF probe" Attach=security_bpf_map Program=bpf/objs/bpf_fmodret_killer.o Type=killer Signed-off-by: Jiri Olsa --- pkg/sensors/load.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/sensors/load.go b/pkg/sensors/load.go index 722fed1e945..f7e73c4c863 100644 --- a/pkg/sensors/load.go +++ b/pkg/sensors/load.go @@ -322,9 +322,15 @@ func loadInstance(bpfDir, mapDir string, load *program.Program, version, verbose version = kernels.FixKernelVersion(version) probe, ok := registeredProbeLoad[load.Type] if ok { - logger.GetLogger().WithField("Program", load.Name).WithField("Type", load.Type).Info("Loading registered BPF probe") + logger.GetLogger().WithField("Program", load.Name). + WithField("Type", load.Type). + WithField("Attach", load.Attach). + Info("Loading registered BPF probe") } else { - logger.GetLogger().WithField("Program", load.Name).WithField("Type", load.Type).Info("Loading BPF program") + logger.GetLogger().WithField("Program", load.Name). + WithField("Type", load.Type). + WithField("Attach", load.Attach). + Info("Loading BPF program") } switch load.Type { From a06e6ed24510dab00a145c2d69167a89b582c89a Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Tue, 23 Jan 2024 08:41:12 +0000 Subject: [PATCH 3/8] tetragon: Rename killer's syscalls to calls We are about to allow security_* functions in killer, so let's rename killer's syscalls field to calls. Signed-off-by: Jiri Olsa --- docs/content/en/docs/concepts/tracing-policy/selectors.md | 4 ++-- examples/tracingpolicy/killer.yaml | 2 +- .../client/crds/v1alpha1/cilium.io_tracingpolicies.yaml | 6 +++--- .../crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml | 6 +++--- pkg/k8s/apis/cilium.io/v1alpha1/types.go | 4 ++-- pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go | 4 ++-- pkg/sensors/tracing/killer.go | 4 ++-- pkg/sensors/tracing/killer_builder.go | 2 +- .../client/crds/v1alpha1/cilium.io_tracingpolicies.yaml | 6 +++--- .../crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml | 6 +++--- .../tetragon/pkg/k8s/apis/cilium.io/v1alpha1/types.go | 4 ++-- .../k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go | 4 ++-- 12 files changed, 26 insertions(+), 26 deletions(-) diff --git a/docs/content/en/docs/concepts/tracing-policy/selectors.md b/docs/content/en/docs/concepts/tracing-policy/selectors.md index d906fe35ca9..0985a446d17 100644 --- a/docs/content/en/docs/concepts/tracing-policy/selectors.md +++ b/docs/content/en/docs/concepts/tracing-policy/selectors.md @@ -1015,7 +1015,7 @@ the `killer` program and attach it to specified syscalls. ```yaml spec: killers: - - syscalls: + - calls: - "list:dups" ``` @@ -1047,7 +1047,7 @@ spec: - "sys_dup" - "sys_dup2" killers: - - syscalls: + - calls: - "list:dups" tracepoints: - subsystem: "raw_syscalls" diff --git a/examples/tracingpolicy/killer.yaml b/examples/tracingpolicy/killer.yaml index 0c9f8ff2011..ea708abcdbb 100644 --- a/examples/tracingpolicy/killer.yaml +++ b/examples/tracingpolicy/killer.yaml @@ -11,7 +11,7 @@ spec: - "sys_dup2" - "__ia32_sys_dup" killers: - - syscalls: + - calls: - "list:dups" tracepoints: - subsystem: "raw_syscalls" diff --git a/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml b/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml index 482d36963bf..baa02776c27 100644 --- a/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml +++ b/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml @@ -38,13 +38,13 @@ spec: description: A killer spec. items: properties: - syscalls: - description: syscalls where killer is executed in + calls: + description: Calls where killer is executed in items: type: string type: array required: - - syscalls + - calls type: object type: array kprobes: diff --git a/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml b/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml index 80ad8cb6203..ea6631b1cf6 100644 --- a/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml +++ b/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml @@ -38,13 +38,13 @@ spec: description: A killer spec. items: properties: - syscalls: - description: syscalls where killer is executed in + calls: + description: Calls where killer is executed in items: type: string type: array required: - - syscalls + - calls type: object type: array kprobes: diff --git a/pkg/k8s/apis/cilium.io/v1alpha1/types.go b/pkg/k8s/apis/cilium.io/v1alpha1/types.go index 1220c8be2e8..6e36e428606 100644 --- a/pkg/k8s/apis/cilium.io/v1alpha1/types.go +++ b/pkg/k8s/apis/cilium.io/v1alpha1/types.go @@ -355,6 +355,6 @@ type PodInfoList struct { } type KillerSpec struct { - // syscalls where killer is executed in - Syscalls []string `json:"syscalls"` + // Calls where killer is executed in + Calls []string `json:"calls"` } diff --git a/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go b/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go index b1a926118ef..ee75b704aae 100644 --- a/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go @@ -226,8 +226,8 @@ func (in *KProbeSpec) DeepCopy() *KProbeSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KillerSpec) DeepCopyInto(out *KillerSpec) { *out = *in - if in.Syscalls != nil { - in, out := &in.Syscalls, &out.Syscalls + if in.Calls != nil { + in, out := &in.Calls, &out.Calls *out = make([]string, len(*in)) copy(*out, *in) } diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 108dbd2894a..0272093b12a 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -159,8 +159,8 @@ func (kh *killerHandler) createKillerSensor( killer := killers[0] // get all the syscalls - for idx := range killer.Syscalls { - sym := killer.Syscalls[idx] + for idx := range killer.Calls { + sym := killer.Calls[idx] if strings.HasPrefix(sym, "list:") { listName := sym[len("list:"):] diff --git a/pkg/sensors/tracing/killer_builder.go b/pkg/sensors/tracing/killer_builder.go index ade59a370cf..d95b85db83c 100644 --- a/pkg/sensors/tracing/killer_builder.go +++ b/pkg/sensors/tracing/killer_builder.go @@ -124,7 +124,7 @@ func (ksb *KillerSpecBuilder) Build() (*v1alpha1.TracingPolicy, error) { Validated: false, }) killers = append(killers, v1alpha1.KillerSpec{ - Syscalls: []string{listName}, + Calls: []string{listName}, }) } diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml index 482d36963bf..baa02776c27 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml @@ -38,13 +38,13 @@ spec: description: A killer spec. items: properties: - syscalls: - description: syscalls where killer is executed in + calls: + description: Calls where killer is executed in items: type: string type: array required: - - syscalls + - calls type: object type: array kprobes: diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml index 80ad8cb6203..ea6631b1cf6 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml @@ -38,13 +38,13 @@ spec: description: A killer spec. items: properties: - syscalls: - description: syscalls where killer is executed in + calls: + description: Calls where killer is executed in items: type: string type: array required: - - syscalls + - calls type: object type: array kprobes: diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/types.go b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/types.go index 1220c8be2e8..6e36e428606 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/types.go +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/types.go @@ -355,6 +355,6 @@ type PodInfoList struct { } type KillerSpec struct { - // syscalls where killer is executed in - Syscalls []string `json:"syscalls"` + // Calls where killer is executed in + Calls []string `json:"calls"` } diff --git a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go index b1a926118ef..ee75b704aae 100644 --- a/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go +++ b/vendor/github.com/cilium/tetragon/pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go @@ -226,8 +226,8 @@ func (in *KProbeSpec) DeepCopy() *KProbeSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KillerSpec) DeepCopyInto(out *KillerSpec) { *out = *in - if in.Syscalls != nil { - in, out := &in.Syscalls, &out.Syscalls + if in.Calls != nil { + in, out := &in.Calls, &out.Calls *out = make([]string, len(*in)) copy(*out, *in) } From 50cbf44788b34b1e66207b7ffaed95c7592c132f Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 19 Jan 2024 22:00:18 +0000 Subject: [PATCH 4/8] tetragon: Add support to use NotifyKiller action from kprobes Adding support to use NotifyKiller action from kprobes, which means we need to add the do_action_notify_killer to the kprobe ebpf program and share the killer_data map with the kprobe sensor. Signed-off-by: Jiri Olsa --- bpf/process/types/basic.h | 2 +- pkg/sensors/tracing/generickprobe.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/bpf/process/types/basic.h b/bpf/process/types/basic.h index 89c66c75611..777bcc8bd23 100644 --- a/bpf/process/types/basic.h +++ b/bpf/process/types/basic.h @@ -2084,7 +2084,7 @@ struct { __uint(value_size, sizeof(__u64) * PERF_MAX_STACK_DEPTH); } stack_trace_map SEC(".maps"); -#ifdef GENERIC_TRACEPOINT +#if defined GENERIC_TRACEPOINT || defined GENERIC_KPROBE static inline __attribute__((always_inline)) void do_action_notify_killer(int error, int signal) { diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index ff1ac2add53..f6ef4be275b 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -311,6 +311,9 @@ func createMultiKprobeSensor(sensorPath string, multiIDs []idtable.EntryID) ([]* maps = append(maps, socktrack) } + killerDataMap := program.MapBuilderPin(killerDataMapName, killerDataMapName, load) + maps = append(maps, killerDataMap) + filterMap.SetMaxEntries(len(multiIDs)) configMap.SetMaxEntries(len(multiIDs)) @@ -858,6 +861,9 @@ func createKprobeSensorFromEntry(kprobeEntry *genericKprobe, sensorPath string, maps = append(maps, socktrack) } + killerDataMap := program.MapBuilderPin(killerDataMapName, killerDataMapName, load) + maps = append(maps, killerDataMap) + if kprobeEntry.loadArgs.retprobe { pinRetProg := sensors.PathJoin(pinPath, fmt.Sprintf("%s_ret_prog", kprobeEntry.funcName)) loadret := program.Builder( From 644ce9dc7f71a6a5b419e889d70f2aaff5553d12 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 19 Jan 2024 09:49:51 +0000 Subject: [PATCH 5/8] tetragon: Add support to use security_ functions in killer Adding support to use security_* functions in killer in addition to syscalls. It's possible now to use spec like: killers: - syscalls: - "security_file_permission" Signed-off-by: Jiri Olsa --- pkg/arch/arch.go | 9 +++++++++ pkg/sensors/tracing/killer.go | 28 +++++++++++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/pkg/arch/arch.go b/pkg/arch/arch.go index bf3cd92b7af..6f4c00e9798 100644 --- a/pkg/arch/arch.go +++ b/pkg/arch/arch.go @@ -71,3 +71,12 @@ func CutSyscallPrefix(symbol string) (string, bool) { } return symbol, false } + +func HasSyscallPrefix(symbol string) bool { + for _, prefix := range supportedArchPrefix { + if strings.HasPrefix(symbol, prefix) { + return true + } + } + return false +} diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 0272093b12a..40cf8df6149 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -169,18 +169,32 @@ func (kh *killerHandler) createKillerSensor( return nil, fmt.Errorf("Error list '%s' not found", listName) } - if !isSyscallListType(list.Type) { - return nil, fmt.Errorf("Error list '%s' is not syscall type", listName) - } kh.syscallsSyms = append(kh.syscallsSyms, list.Values...) continue } - pfxSym, err := arch.AddSyscallPrefix(sym) - if err != nil { - return nil, err + kh.syscallsSyms = append(kh.syscallsSyms, sym) + } + + var err error + + // fix syscalls + for idx, sym := range kh.syscallsSyms { + isPrefix := arch.HasSyscallPrefix(sym) + isSyscall := strings.HasPrefix(sym, "sys_") + isSecurity := strings.HasPrefix(sym, "security_") + + if !isSyscall && !isSecurity && !isPrefix { + return nil, fmt.Errorf("killer sensor requires either syscall or security_ functions") + } + + if isSyscall { + sym, err = arch.AddSyscallPrefix(sym) + if err != nil { + return nil, err + } + kh.syscallsSyms[idx] = sym } - kh.syscallsSyms = append(kh.syscallsSyms, pfxSym) } // register killer sensor From 27677aebb122e76d0b6c7ada15be2806cd3fd564 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 19 Jan 2024 09:56:37 +0000 Subject: [PATCH 6/8] tetragon: Check fmodret support based on the symbol type Checking fmodret support based on the symbol types used in killer, so the detected support can properly evaluate the need to fail. Signed-off-by: Jiri Olsa --- pkg/sensors/tracing/killer.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 40cf8df6149..2ce7ceacbb0 100644 --- a/pkg/sensors/tracing/killer.go +++ b/pkg/sensors/tracing/killer.go @@ -108,7 +108,7 @@ func (kh *killerHandler) LoadProbe(args sensors.LoadProbeArgs) error { } // select proper override method based on configuration and spec options -func selectOverrideMethod(specOpts *specOptions) (OverrideMethod, error) { +func selectOverrideMethod(specOpts *specOptions, hasSyscall bool) (OverrideMethod, error) { overrideMethod := specOpts.OverrideMethod switch overrideMethod { case OverrideMethodDefault: @@ -125,7 +125,7 @@ func selectOverrideMethod(specOpts *specOptions) (OverrideMethod, error) { return OverrideMethodInvalid, fmt.Errorf("option override return set, but it is not supported") } case OverrideMethodFmodRet: - if !bpf.HasModifyReturnSyscall() { + if !bpf.HasModifyReturn() || (hasSyscall && !bpf.HasModifyReturnSyscall()) { return OverrideMethodInvalid, fmt.Errorf("option fmod_ret set, but it is not supported") } } @@ -158,6 +158,8 @@ func (kh *killerHandler) createKillerSensor( killer := killers[0] + var hasSyscall bool + // get all the syscalls for idx := range killer.Calls { sym := killer.Calls[idx] @@ -195,6 +197,8 @@ func (kh *killerHandler) createKillerSensor( } kh.syscallsSyms[idx] = sym } + + hasSyscall = hasSyscall || isSyscall || isPrefix } // register killer sensor @@ -211,7 +215,7 @@ func (kh *killerHandler) createKillerSensor( } // select proper override method based on configuration and spec options - overrideMethod, err := selectOverrideMethod(specOpts) + overrideMethod, err := selectOverrideMethod(specOpts, hasSyscall) if err != nil { return nil, err } From e62c1885e020434b64162b049e9d5e6662bf206c Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 19 Jan 2024 12:26:01 +0000 Subject: [PATCH 7/8] tetragon: Add direct-write-tester tester prog Adding direct-write-tester tester prog to test the bypass of sigkill action in pwrite. It's originally written by Anastasios. Signed-off-by: Jiri Olsa --- contrib/tester-progs/Makefile | 3 +- contrib/tester-progs/direct-write-tester.c | 43 ++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 contrib/tester-progs/direct-write-tester.c diff --git a/contrib/tester-progs/Makefile b/contrib/tester-progs/Makefile index c906352314e..0103194e461 100644 --- a/contrib/tester-progs/Makefile +++ b/contrib/tester-progs/Makefile @@ -20,7 +20,8 @@ PROGS = sigkill-tester \ threads-exit \ killer-tester \ drop-privileges \ - getcpu + getcpu \ + direct-write-tester # For now killer-tester is compiled to 32-bit only on x86_64 as we want # to test 32-bit binaries and system calls compatibility layer. diff --git a/contrib/tester-progs/direct-write-tester.c b/contrib/tester-progs/direct-write-tester.c new file mode 100644 index 00000000000..5a2cf50d034 --- /dev/null +++ b/contrib/tester-progs/direct-write-tester.c @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +// Copyright Authors of Tetragon + +#include +#include +#include +#include +#include +#include +#include + +#define BLOCKSIZE 4096 +#define O_DIRECT 00040000 + +int main(int argc, char **argv) +{ + char *avd = "NEW_MESSAGE\n"; + void *buffer; + int fd; + + if (argc != 2) + exit(-1); + + fd = open(argv[1], O_WRONLY | O_DIRECT | O_CREAT, S_IRWXU | S_IRWXG); + if (fd == -1) { + perror("open"); + exit(-1); + } + + posix_memalign(&buffer, BLOCKSIZE, BLOCKSIZE); + memset(buffer, 0, BLOCKSIZE); + memcpy(buffer, avd, strlen(avd) + 1); + + ssize_t ret = pwrite(fd, buffer, BLOCKSIZE, 0); + if (ret == -1) { + perror("write"); + exit(-1); + } + + close(fd); + free(buffer); + return 0; +} From 42fc9290e33a32c70b5ea9a710e115a1d83fc497 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 19 Jan 2024 12:20:49 +0000 Subject: [PATCH 8/8] tetragon: Add killer test for security hook Testing the ability to kill the process before it executes the syscall, in this case direct pwrite syscall. Standard Sigkill action kills executed from sys_pwrite probe kills the process, but only after the pwrite syscall is executed. Now we can mitigate that by attaching killer to security_file_permission function and override its return value to prevent the pwrite syscall execution. Signed-off-by: Jiri Olsa --- pkg/sensors/tracing/killer_test.go | 229 +++++++++++++++++++++++++++++ 1 file changed, 229 insertions(+) diff --git a/pkg/sensors/tracing/killer_test.go b/pkg/sensors/tracing/killer_test.go index 920c14efbcc..eee033feac6 100644 --- a/pkg/sensors/tracing/killer_test.go +++ b/pkg/sensors/tracing/killer_test.go @@ -14,9 +14,12 @@ import ( "github.com/cilium/tetragon/api/v1/tetragon" "github.com/cilium/tetragon/api/v1/tetragon/codegen/eventchecker" ec "github.com/cilium/tetragon/api/v1/tetragon/codegen/eventchecker" + "github.com/cilium/tetragon/pkg/arch" "github.com/cilium/tetragon/pkg/bpf" "github.com/cilium/tetragon/pkg/jsonchecker" + "github.com/cilium/tetragon/pkg/kernels" lc "github.com/cilium/tetragon/pkg/matchers/listmatcher" + sm "github.com/cilium/tetragon/pkg/matchers/stringmatcher" "github.com/cilium/tetragon/pkg/observer/observertesthelper" "github.com/cilium/tetragon/pkg/testutils" tus "github.com/cilium/tetragon/pkg/testutils/sensors" @@ -183,3 +186,229 @@ func TestKillerMultiNotSupported(t *testing.T) { err := checkCrd(t, yaml) assert.Error(t, err) } + +func testSecurity(t *testing.T, tracingPolicy, tempFile string) { + var doneWG, readyWG sync.WaitGroup + defer doneWG.Wait() + + ctx, cancel := context.WithTimeout(context.Background(), tus.Conf().CmdWaitTime) + defer cancel() + + createCrdFile(t, tracingPolicy) + + obs, err := observertesthelper.GetDefaultObserverWithFile(t, ctx, testConfigFile, + tus.Conf().TetragonLib, observertesthelper.WithMyPid()) + if err != nil { + t.Fatalf("GetDefaultObserverWithFile error: %s", err) + } + observertesthelper.LoopEvents(ctx, t, &doneWG, &readyWG, obs) + readyWG.Wait() + + testBin := testutils.RepoRootPath("contrib/tester-progs/direct-write-tester") + + cmd := exec.Command(testBin, tempFile) + err = cmd.Run() + assert.Error(t, err) + + t.Logf("Running: %s %v\n", cmd.String(), err) + + kpCheckerPwrite := ec.NewProcessKprobeChecker(""). + WithProcess(ec.NewProcessChecker(). + WithBinary(sm.Suffix(testBin))). + WithFunctionName(sm.Full(arch.AddSyscallPrefixTestHelper(t, "sys_pwrite64"))). + WithAction(tetragon.KprobeAction_KPROBE_ACTION_NOTIFYKILLER). + WithArgs(ec.NewKprobeArgumentListMatcher(). + WithOperator(lc.Ordered). + WithValues( + ec.NewKprobeArgumentChecker().WithFileArg( + ec.NewKprobeFileChecker().WithPath(sm.Full(tempFile))), + )) + + checker := ec.NewUnorderedEventChecker(kpCheckerPwrite) + err = jsonchecker.JsonTestCheck(t, checker) + assert.NoError(t, err) + + // check the pwrite syscall did not write anything + fileInfo, err := os.Stat(tempFile) + if assert.NoError(t, err) { + assert.NotEqual(t, 0, fileInfo.Size()) + } +} + +// Testing the ability to kill the process before it executes the syscall, +// in this case direct pwrite syscall. +// Standard Sigkill action kills executed from sys_pwrite probe kills the +// process, but only after the pwrite syscall is executed. +// Now we can mitigate that by attaching killer to security_file_permission +// function and override its return value to prevent the pwrite syscall +// execution. +// +// The testing spec below: +// - attaches probe to pwrite +// - attaches killer to security_file_permission +// - executes SigKill action for attempted pwrite to specific file +// - executes NotifyKiller action to instruct killer to override the +// security_file_permission return value with -1 +// - tests that no data got written to the monitored file + +func TestKillerSecuritySigKill(t *testing.T) { + if !bpf.HasSignalHelper() { + t.Skip("skipping killer test, bpf_send_signal helper not available") + } + + if !bpf.HasModifyReturn() { + t.Skip("skipping killer test, fmod_ret is not available") + } + + if !kernels.EnableLargeProgs() { + t.Skip("Older kernels do not support matchArgs for more than one arguments") + } + + tempFile := t.TempDir() + "/test" + + tracingPolicy := ` +apiVersion: cilium.io/v1alpha1 +kind: TracingPolicy +metadata: + name: "syswritefollowfdpsswd" +spec: + options: + - name: "override-method" + value: "fmod-ret" + killers: + - calls: + - "security_file_permission" + kprobes: + - call: "fd_install" + syscall: false + args: + - index: 0 + type: int + - index: 1 + type: "file" + selectors: + - matchArgs: + - index: 1 + operator: "Equal" + values: + - "` + tempFile + `" + matchActions: + - action: FollowFD + argFd: 0 + argName: 1 + - call: "sys_close" + syscall: true + args: + - index: 0 + type: "int" + selectors: + - matchActions: + - action: UnfollowFD + argFd: 0 + argName: 0 + - call: "sys_pwrite64" + syscall: true + args: + - index: 0 + type: "fd" + selectors: + - matchArgs: + - index: 0 + operator: "Equal" + values: + - "` + tempFile + `" + matchActions: + - action: Sigkill + - action: "NotifyKiller" + argError: -1 +` + + testSecurity(t, tracingPolicy, tempFile) +} + +// Testing the ability to kill the process before it executes the syscall, +// in similar way as in TestKillerSecuritySigKill test. +// The only difference is we use the NotifyKiller to send the signal instead +// of using SigKill action. +// +// The testing spec below: +// - attaches probe to pwrite +// - attaches killer to security_file_permission +// - executes NotifyKiller to instruct killer to send sigkill to current process +// and override the security_file_permission return value with -1 +// - tests that no data got written to the monitored file + +func TestKillerSecurityNotifyKiller(t *testing.T) { + if !bpf.HasSignalHelper() { + t.Skip("skipping killer test, bpf_send_signal helper not available") + } + + if !bpf.HasModifyReturn() { + t.Skip("skipping killer test, fmod_ret is not available") + } + + if !kernels.EnableLargeProgs() { + t.Skip("Older kernels do not support matchArgs for more than one arguments") + } + + tempFile := t.TempDir() + "/test" + + tracingPolicy := ` +apiVersion: cilium.io/v1alpha1 +kind: TracingPolicy +metadata: + name: "syswritefollowfdpsswd" +spec: + options: + - name: "override-method" + value: "fmod-ret" + killers: + - calls: + - "security_file_permission" + kprobes: + - call: "fd_install" + syscall: false + args: + - index: 0 + type: int + - index: 1 + type: "file" + selectors: + - matchArgs: + - index: 1 + operator: "Equal" + values: + - "` + tempFile + `" + matchActions: + - action: FollowFD + argFd: 0 + argName: 1 + - call: "sys_close" + syscall: true + args: + - index: 0 + type: "int" + selectors: + - matchActions: + - action: UnfollowFD + argFd: 0 + argName: 0 + - call: "sys_pwrite64" + syscall: true + args: + - index: 0 + type: "fd" + selectors: + - matchArgs: + - index: 0 + operator: "Equal" + values: + - "` + tempFile + `" + matchActions: + - action: "NotifyKiller" + argError: -1 + argSig: 9 +` + + testSecurity(t, tracingPolicy, tempFile) +}