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.

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

feat: Change hooked syscall event #3544

Closed

Conversation

OriGlassman
Copy link
Collaborator

@OriGlassman OriGlassman commented Oct 3, 2023

1. Explain what the PR does

change hooked_syscall event

2. Explain how to test it

tracee -e=hooked_syscall

3. Other comments

@CLAassistant
Copy link

CLAassistant commented Oct 3, 2023

CLA assistant check
All committers have signed the CLA.

@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Oct 14, 2023

Some preliminary comments (you said this was ready on your side but its DRAFT so I didn't know).

I'm getting:

panic: interface conversion: interface {} is string, not map[string]interface {}

goroutine 108 [running]:
plugin/unnamed-a7e1df0e51bf802afb26eccc7dd59f329a2028bc.(*SyscallTableHooking).OnEvent(0x7f094def4fa0, {{{{0x224ddb2, 0xe}, {0x222e00d, 0x4}, {0x223178d, 0x6}}, 0x0}, {0x2218940, 0xc004b96000}})
        /home/rafaeldtinoco/work/projects/tracee/signatures/golang/syscall_table_hooking.go:60 +0x447
github.com/aquasecurity/tracee/pkg/signatures/engine.signatureStart({0x7f094dd1c480, 0x7f094def4fa0}, 0x70614d7865646e49?, 0x672e6c61636f6c00?)
        /home/rafaeldtinoco/work/projects/tracee/pkg/signatures/engine/engine.go:80 +0x162
created by github.com/aquasecurity/tracee/pkg/signatures/engine.(*Engine).Start in goroutine 66
        /home/rafaeldtinoco/work/projects/tracee/pkg/signatures/engine/engine.go:118 +0xd8

Because of the signature metadata "Data" field wrong type conversion in some case.

@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Oct 14, 2023

When mocking the error, you're missing the following syscalls:

$ sudo ./dist/tracee --output json --events hooked_syscall | jq '.a
rgs[0].value'
"waitpid"
"oldfstat"
"oldstat"
"stime"
"stty"
"gtty"
"ftime"
"signal"
"oldolduname"
"sigaction"
"sigsuspend"
"sigpending"
"oldlstat"
"readdir"
"vm86old"
"ipc"
"sigreturn"
"sigprocmask"
"bdflush"
"afs_syscall"
"llseek"
"old_select"
"vm86"
"old_getrlimit"
"mmap2"
"truncate64"
"geteuid16"
"getegid16"
"setreuid16"
"setregid16"
"getgroups16"

From the string slice in core_amd64.go, so the initial bpf map population isn't complete.

Also, I think a map indexed by syscall id would be better there.

@rafaeldtinoco
Copy link
Contributor

I have tested hooking a syscall with:

https://github.com/rafaeldtinoco/hijack/blob/main/hijack.c

and, despite being able to:

[22372.068909] uname() intercepted!

I could not get a detection. Is the command line:

sudo ./dist/tracee --output json --events hooked_syscall

enough ?

@OriGlassman
Copy link
Collaborator Author

I have tested hooking a syscall with:

https://github.com/rafaeldtinoco/hijack/blob/main/hijack.c

and, despite being able to:

[22372.068909] uname() intercepted!

I could not get a detection. Is the command line:

sudo ./dist/tracee --output json --events hooked_syscall

enough ?

The event is periodic. If you don't want to wait a few minutes, try running it AFTER the table is hooked, as it runs immediately when tracee starts up.

@OriGlassman
Copy link
Collaborator Author

waitpid

Do we support 32bit? these are only 32 bit system syscalls

@OriGlassman OriGlassman force-pushed the change_hooked_syscall_event branch 2 times, most recently from 0d645ae to cee8e51 Compare October 16, 2023 10:03
@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Oct 16, 2023

waitpid

Do we support 32bit? these are only 32 bit system syscalls

Yep, but maybe because of internal system call version routing logic (32-bit userland vs 64-bit userland) the 64-bit symbols need to exist as well (I suppose, not sure, just a guess). I do have them enabled:

$ sudo cat /proc/kallsyms | grep -E "sys_waitpid"
ffffffff870a3d80 T __x64_sys_waitpid
ffffffff870a3da0 T __ia32_sys_waitpid

sudo cat /proc/kallsyms | grep -E "_[e|s]text"
ffffffff87000000 T _stext
ffffffff88001f02 T _etext

Either way, I get "fake" hook detections for those syscalls. If you decide to consider, blacklist, etc, up to you.

@rafaeldtinoco
Copy link
Contributor

Sorry, I missed the timer logic for the detection earlier on. Now:

$ sudo ./dist/tracee --output json --events hooked_syscall
{"timestamp":1697762297084454645,"threadStartTime":1697762293432704767,"processorId":2,"processId":306503,"cgroupId":4846,"threadId":306515,"parentProcessId":306502,"hostProcessId":306503,"hostThreadId":306515,"hostParentProcessId":306502,"userId":0,"mountNamespace":4026531841,"pidNamespace":4026531836,"processName":"tracee","executable":{"path":""},"hostName":"rugged","containerId":"","container":{},"kubernetes":{},"eventId":"2017","eventName":"hooked_syscall","matchedPolicies":[""],"argsNum":4,"returnValue":0,"syscall":"","stackAddresses":[0],"contextFlags":{"containerStarted":false,"isCompat":false},"threadEntityId":2644688824,"processEntityId":1587116564,"parentEntityId":4262210630,"args":[{"name":"syscall_name","type":"const char*","value":"uname"},{"name":"hooked.address","type":"const char*","value":"ffffffffc1758000"},{"name":"hooked.function_name","type":"const char*","value":"hooked_uname"},{"name":"hooked.owner","type":"const char*","value":"hijack"}]}

looks good for amd64 (haven't tested yet arm64, just the tracee execution).

I need to check why the check-code logic isn't passing (its passing locally so it has probably something to do with the runner picked).

@rafaeldtinoco
Copy link
Contributor

Now sure why tests are broken here (checking format), but after a rebase they pass (#3592). Also, there is an eBPF fix for kernels 6.5+ so this PR would need a rebase either way. I'll close this PR and merge that one if tests pass there (based on an offline msg of you saying you were done with this PR).

@rafaeldtinoco
Copy link
Contributor

Being merged at #3592 because of rebase needs and small test fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants