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.

Sign up for GitHub

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

Jump to bottom

Add suspicious_syscall_source event #3953

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add suspicious_syscall_source event #3953

wants to merge 2 commits into from

Conversation

oshaked1
Copy link
Contributor

@oshaked1 oshaked1 commented Apr 3, 2024 •
edited
Loading

1. Explain what the PR does

Add a new event, suspicious_syscall_source, which reports invocations of syscalls from unusual code locations (stack, heap and anonymous VMAs).
It is implemented using generic syscall kprobes, which trigger the analysis only for syscalls that were selected using event parameters, e.g. tracee -e suspicious_syscall_source.args.syscall=open,openat.
Closes #4002

2. Explain how to test it

The easiest way to test the event is using the included tester program, which executes a small shellcode that invokes the exit syscall from the stack/heap/anonymous VMA according to the command line argument. Note that it must be compiled with -z execstack to be able to run the shellcode from the stack.

Another way of testing the event (for arbitrary syscalls) is by packing a program:

# compile statically, so libc syscall wrappers are not loaded to separate (legitimate) code areas
gcc -o my_program -static my_program.c

# pack the program
upx -o my_program_upx my_program

# run it
./my_program_upx

@oshaked1
Copy link
Contributor Author

oshaked1 commented Apr 3, 2024 •
edited
Loading

The ebpf code for this event contains a call to the bpf_find_vma helper if it exists (an alternative logic is implemented for older kernels where the helper doesn't exist).

Because this helper uses a callback function or in verifier terms a "bpf-to-bpf call", this program cannot be combined with tail calls (which it uses) on some older kernels, even though the helper call would not be called on those kernels.

As a result, this event must be split into 2 separate programs that will be loaded conditionally based on kernel version. Currently no such mechanism exists, but is in the works. When such a mechanism will be implemented this event will be split into 2 programs so it can be merged.

EDIT

The logic for newer kernels which uses the bpf_find_vma call was removed, so the event will now only work on kernel version < 6.1. When conditional program loading will be implemented, the logic for newer kernels will be added as a separate program.

@oshaked1 oshaked1 mentioned this pull request Apr 3, 2024
Merged
@oshaked1 oshaked1 force-pushed the syscall_checker branch 3 times, most recently from c45b5fc to a6c4e35 Compare April 4, 2024 15:17
@oshaked1 oshaked1 marked this pull request as ready for review April 7, 2024 11:22
@oshaked1 oshaked1 force-pushed the syscall_checker branch 4 times, most recently from d90fe4e to 3a29a8c Compare May 12, 2024 18:53
@oshaked1 oshaked1 force-pushed the syscall_checker branch 5 times, most recently from daec435 to 03d7be9 Compare June 5, 2024 11:35
@oshaked1 oshaked1 force-pushed the syscall_checker branch 4 times, most recently from abd31e6 to eb312fe Compare June 25, 2024 15:56
@oshaked1 oshaked1 force-pushed the syscall_checker branch 9 times, most recently from 6fc9f61 to 4c70e9e Compare October 14, 2024 11:29
@oshaked1 oshaked1 force-pushed the syscall_checker branch 7 times, most recently from 82bc699 to 49499c2 Compare October 27, 2024 12:04
@oshaked1 oshaked1 marked this pull request as ready for review October 27, 2024 12:58
@oshaked1 oshaked1 force-pushed the syscall_checker branch 3 times, most recently from 0d4d80e to 6e8e468 Compare October 28, 2024 12:13
@oshaked1 oshaked1 changed the title Add check_syscall_source event Add suspicious_syscall_source event Nov 24, 2024
yanivagman
yanivagman requested changes Nov 24, 2024
View reviewed changes
pkg/ebpf/c/common/common.h Outdated Show resolved Hide resolved
.github/workflows/pr.yaml Outdated Show resolved Hide resolved
pkg/ebpf/c/common/memory.h Outdated Show resolved Hide resolved
pkg/ebpf/c/tracee.bpf.c Outdated Show resolved Hide resolved
pkg/ebpf/event_parameters.go Outdated Show resolved Hide resolved
pkg/ebpf/event_parameters.go
type eventParameterHandler func(t *Tracee, eventParams []map[string]filters.Filter[*filters.StringFilter]) error

var eventParameterHandlers = map[events.ID]eventParameterHandler{
events.SuspiciousSyscallSource: attachSuspiciousSyscallSourceProbes,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you created event parameters, we can move print_mem_dump in here as well (I'm not sure if there are other events that abuse data filters as parameters)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

print_mem_dump uses event filters to populate the required ksymbols. handleEventParameters is called at the end of initialization, because it needs to happen after initBPF, which is called after the kernel symbols are initialized.

pkg/ebpf/tracee.go Outdated
Comment on lines 128 to 129
// Probes created for suspicious_syscall_source event
suspiciousSyscallSourceProbes *probes.ProbeGroup
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it is a good idea to add probe groups into the tracee struct.
It makes the struct coupled to specific events, and we want to decouple things, not the other way around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where should it be added? The main probe group is also part of the tracee struct

Copy link
Collaborator

Choose a reason for hiding this comment

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

The main probe group is the only group that exists now, and it is not event specific like this one

Copy link
Contributor Author

@oshaked1 oshaked1 Nov 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

So where should I put it? I also plan to add additional "syscall checker" events that will all utilize the same probe group which will make it not specific to a single event but for a "class" of events

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yanivagman I added a map to the tracee struct that holds extra probe groups (indexed by a given name) that can be added dynamically.

@oshaked1
Create generic extra probe groups in tracee struct
e430940 
Instead of saving the probe group for suspicious_syscall_source directly to the tracee struct, it is added to a generic map that holds extra probe groups which can be added dynamically.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yanivagman yanivagman yanivagman requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@oshaked1 oshaked1

Labels
area/build area/ebpf area/events area/filtering area/testing kind/documentation milestone/v0.23.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add suspicious_syscall_source event
3 participants
@oshaked1 @geyslan @yanivagman