tetragon: Add support to use security_ functions in killer #2002

olsajiri · 2024-01-19T12:57:41Z

Adding support to attach killer to security_ functions and use NotifyKiller from kprobe. It's used to override specific security_* function in case we want to ensure we bypass syscall processing before sigkill is delivered.

netlify · 2024-01-20T15:27:40Z

✅ Deploy Preview for tetragon ready!

Name	Link
🔨 Latest commit	`42fc929`
🔍 Latest deploy log	https://app.netlify.com/sites/tetragon/deploys/65b23f84709a260008b6506e
😎 Deploy Preview	https://deploy-preview-2002--tetragon.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

kkourt

Thanks :)

Had a quick look and added some first comments.

kkourt · 2024-01-22T14:31:14Z

pkg/sensors/tracing/generickprobe.go

@@ -355,6 +355,9 @@ func createMultiKprobeSensor(sensorPath string, multiIDs []idtable.EntryID) ([]*
 		maps = append(maps, socktrack)
 	}

+	killerDataMap := program.MapBuilderPin("killer_data", "killer_data", load)


Let's use

tetragon/pkg/sensors/tracing/killer.go

Line 24 in 506677d

killerDataMapName = "killer_data"

?

nice, did not see that, thanks

pkg/sensors/tracing/killer.go

dwindsor · 2024-01-22T15:36:54Z

killer

Just curious, how is this different from killing a process inline by overriding the return value of one of the LSM calls to be non-zero?

olsajiri · 2024-01-22T17:09:22Z

killer

Just curious, how is this different from killing a process inline by overriding the return value of one of the LSM calls to be non-zero?

not too much different, this way we get invoked earlier directly from security_* function call, lsm hook would get invoked later on, but that would not really matter, it'd work the same I think.. another thing is we don't need to depend on lsm being enabled

jrfastab · 2024-01-24T06:08:31Z

Also its non-obvious that all syscalls map to a security hook. Many will but not sure its 1:1 without auditing.

kkourt

Thanks!

kkourt · 2024-01-25T08:32:15Z

pkg/sensors/tracing/killer.go

+		if hasSyscall && !bpf.HasModifyReturnSyscall() {
+			return OverrideMethodInvalid, fmt.Errorf("option fmod_ret on syscall set, but it is not supported")
+		}
+		if hasNormal && !bpf.HasModifyReturn() {
 			return OverrideMethodInvalid, fmt.Errorf("option fmod_ret set, but it is not supported")


I don't think the hasNormal check is needed.

AFIACT, if HadModifyReturnSyscall() is true , HasModifyReturn() is (always) true.
So I think the following check:

if !bpf.HasModifyReturn() || (hasSyscall && !bpf.HasModifyReturnSyscall()) { return err }

catches all the cases we care about, without the extra argument.

right, I guess I wanted it to be more explicit.. I'll change that

tixxdz · 2024-01-25T09:04:59Z

I didn't follow it closely but I have one question: would it be possible to just fmod_ret the security_ hook without killing the process? is the old behavior still supported? cause there is valid software there that is happy with -EPERM and continues to work as expected, delivering sigkill for such cases is too much and could break.

Also extra note didn't check code but as John noted is this planning a 1:1 mapping? the security hooks don't really map to syscalls, there are syscalls->operations (different context)-> lead to same security hook, and other cases where one syscall is handled through multiple security hooks according to context. The security hooks are more about the context of the process, actually constructing the context if we check that in details, then acting when necessary, where in syscall you may act in advance without context at all... The mapping could work for some read/write fs operation but for other cases where example the kernel acts on behalf is different story

To get error value for wrong pattern. Signed-off-by: Jiri Olsa <[email protected]>

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 <[email protected]>

We are about to allow security_* functions in killer, so let's rename killer's syscalls field to calls. Signed-off-by: Jiri Olsa <[email protected]>

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 <[email protected]>

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 <[email protected]>

olsajiri · 2024-01-25T09:51:23Z

I didn't follow it closely but I have one question: would it be possible to just fmod_ret the security_ hook without killing the process? is the old behavior still supported? cause there is valid software there that is happy with -EPERM and continues to work as expected, delivering sigkill for such cases is too much and could break.

yes, this change just adds functionality, does not remove any, it adds the possibility to attach killer to security_* functions

Also extra note didn't check code but as John noted is this planning a 1:1 mapping? the security hooks don't really map to syscalls, there are syscalls->operations (different context)-> lead to same security hook, and other cases where one syscall is handled through multiple security hooks according to context. The security hooks are more about the context of the process, actually constructing the context if we check that in details, then acting when necessary, where in syscall you may act in advance without context at all... The mapping could work for some read/write fs operation but for other cases where example the kernel acts on behalf is different story

the idea behind this change is to be able to kill process that's about to execute a syscall and make sure the syscall is not executed by attaching extra killer probe to security function (because there could be kernel configs where you can't override syscall directly) ... it's up to the user to pick up which security function to hook for which syscall, there's nothing hardcoded

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

mtardy

thanks looks good! :)

kkourt

Thanks!

olsajiri added the release-note/minor This PR introduces a minor user-visible change label Jan 19, 2024

olsajiri force-pushed the pr/olsajiri/killer_security branch 4 times, most recently from 87558af to 0452e39 Compare January 20, 2024 15:27

olsajiri force-pushed the pr/olsajiri/killer_security branch 3 times, most recently from ed095f2 to 44502b5 Compare January 22, 2024 09:01

olsajiri changed the title ~~Pr/olsajiri/killer security~~ tetragon: Add support to use security_ functions in killer Jan 22, 2024

olsajiri force-pushed the pr/olsajiri/killer_security branch from 44502b5 to a53fa6c Compare January 22, 2024 09:05

kkourt reviewed Jan 22, 2024

View reviewed changes

olsajiri force-pushed the pr/olsajiri/killer_security branch 3 times, most recently from ee0866a to 2fdef39 Compare January 23, 2024 10:48

olsajiri marked this pull request as ready for review January 23, 2024 11:52

olsajiri requested review from mtardy and a team as code owners January 23, 2024 11:52

jrfastab approved these changes Jan 24, 2024

View reviewed changes

kkourt reviewed Jan 25, 2024

View reviewed changes

olsajiri added 5 commits January 25, 2024 09:29

tetragon: Return error in case we failed to compile pattern

4e547c4

To get error value for wrong pattern. Signed-off-by: Jiri Olsa <[email protected]>

tetragon: Rename killer's syscalls to calls

a06e6ed

We are about to allow security_* functions in killer, so let's rename killer's syscalls field to calls. Signed-off-by: Jiri Olsa <[email protected]>

tetragon: Add support to use security_ functions in killer

644ce9d

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 <[email protected]>

olsajiri added 3 commits January 25, 2024 11:00

tetragon: Check fmodret support based on the symbol type

27677ae

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 <[email protected]>

tetragon: Add direct-write-tester tester prog

e62c188

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 <[email protected]>

olsajiri force-pushed the pr/olsajiri/killer_security branch from 2fdef39 to 42fc929 Compare January 25, 2024 11:01

mtardy approved these changes Jan 29, 2024

View reviewed changes

kkourt approved these changes Jan 29, 2024

View reviewed changes

olsajiri merged commit fac3a5e into main Jan 29, 2024
42 checks passed

olsajiri deleted the pr/olsajiri/killer_security branch January 29, 2024 12:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tetragon: Add support to use security_ functions in killer #2002

tetragon: Add support to use security_ functions in killer #2002

olsajiri commented Jan 19, 2024 •

edited

Loading

netlify bot commented Jan 20, 2024 •

edited

Loading

kkourt left a comment

kkourt Jan 22, 2024

olsajiri Jan 22, 2024

dwindsor commented Jan 22, 2024

olsajiri commented Jan 22, 2024

jrfastab commented Jan 24, 2024

kkourt left a comment

kkourt Jan 25, 2024

olsajiri Jan 25, 2024

tixxdz commented Jan 25, 2024

olsajiri commented Jan 25, 2024

mtardy left a comment

kkourt left a comment

tetragon: Add support to use security_ functions in killer #2002

tetragon: Add support to use security_ functions in killer #2002

Conversation

olsajiri commented Jan 19, 2024 • edited Loading

netlify bot commented Jan 20, 2024 • edited Loading

✅ Deploy Preview for tetragon ready!

kkourt left a comment

Choose a reason for hiding this comment

kkourt Jan 22, 2024

Choose a reason for hiding this comment

olsajiri Jan 22, 2024

Choose a reason for hiding this comment

dwindsor commented Jan 22, 2024

olsajiri commented Jan 22, 2024

jrfastab commented Jan 24, 2024

kkourt left a comment

Choose a reason for hiding this comment

kkourt Jan 25, 2024

Choose a reason for hiding this comment

olsajiri Jan 25, 2024

Choose a reason for hiding this comment

tixxdz commented Jan 25, 2024

olsajiri commented Jan 25, 2024

mtardy left a comment

Choose a reason for hiding this comment

kkourt left a comment

Choose a reason for hiding this comment

olsajiri commented Jan 19, 2024 •

edited

Loading

netlify bot commented Jan 20, 2024 •

edited

Loading