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/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; +} 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/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/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{} 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/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 { 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( diff --git a/pkg/sensors/tracing/killer.go b/pkg/sensors/tracing/killer.go index 108dbd2894a..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,9 +158,11 @@ func (kh *killerHandler) createKillerSensor( killer := killers[0] + var hasSyscall bool + // 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:"):] @@ -169,18 +171,34 @@ 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) + + hasSyscall = hasSyscall || isSyscall || isPrefix } // register killer sensor @@ -197,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 } 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/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) +} 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) }