cilium
tetragon: extract struct dentry member using CO:RE
Thanks for contributing! Please ensure your pull request adheres to the following guidelines:
- [ ] All commits contain a well written commit message and are signed-off (see Submitting a pull request).
- [ ] All code is covered by unit and/or end-to-end tests where feasible.
- [ ] All generated files are updated if needed (see Making changes).
- [ ] Provide a title or release-note blurb suitable for the release notes (see guidelines).
- [ ] Update documentation and write an upgrade note if needed (see guidelines).
- [ ] Are you a user of Tetragon? Please add yourself to the Users doc in the Cilium repository.
Fixes: #2573
<!-- Enter the release note text here if needed or remove this section! -->
Deploy Preview for tetragon ready!
| Name | Link |
|---|---|
| Latest commit | 6e29ad2b1e115ed112ff8bfe2d87daf7b1c626e5 |
| Latest deploy log | https://app.netlify.com/sites/tetragon/deploys/66992639e725e50008a84182 |
| Deploy Preview | https://deploy-preview-2574--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.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hi! I'm encountering the following error when attempting to build these changes, and am not quite sure why... 🤔
Following the same procedures as 9889f27, I'm getting this error:
shell time="2024-06-17T12:16:50-04:00" level=fatal msg="Failed to start tetragon" error="failed loading tracing policy file \"/home/dave/src/github.com/dwindsor/tetragon/examples/tracingpolicy/security_inode_follow_link.yaml\": validation failed: \"follow-symlink\": validation failure list:\nspec.kprobes[0].args[0].type in body should be one of [auto int int8 uint8 int16 uint16 uint32 int32 uint64 int64 char_buf char_iovec size_t skb sock string fd file filename path nop bpf_attr perf_event bpf_map user_namespace capability kiocb iov_iter cred load_info module syscall64 kernel_cap_t cap_inheritable cap_permitted cap_effective linux_binprm data_loc net_device]"
Using the following Tracing Policy:
apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
name: "follow-symlink"
spec:
kprobes:
- call: "security_inode_follow_link"
syscall: false
args:
- index: 0
type: "dentry"
returnArg:
index: 0
type: "int"
selectors:
- matchArgs:
- index: 0
operator: "Equal"
values:
- "/tmp/softlink"
It appears the procedure has changed at least slightly since 9889f27, as 577a2ac introduced some changes.
i've tried following the same procedure as 9268fd0, but haven't had luck so far. I'll keep trying but wanted to see if anyone had any pointers?
cc: @kevsecurity @mtardy @jrfastab
I forgot to run make crds; not sure if that target was around last time?
Either way, above errors are fixed =)
I forgot to run
make crds; not sure if that target was around last time?
It was renamed from make generate to make crds recently: https://github.com/cilium/tetragon/commit/c26e483f7a3901eb3c5bc86671ba46f49c4b2481
Either way, above errors are fixed =)
Thanks! I plan to review this tomorrow.
Could you also fix the static check failures? Running make check locally should perform the same tests. This is not required for the review, but it's something that needs to be fixed before merging.
Thanks for the review @kkourt! I've responded to your comments and fixed the formatting issues, but I'm having trouble getting the test to work.
Running the test program on my system, I see the relevant event being produced in tetra:
{
"process_kprobe": {
"process": {
"exec_id": "bGF0ZXJhbHVzOjI0ODQ0MzUwNjU1MzM4OjMyNjQ2MQ==",
"pid": 326461,
"uid": 1000,
"cwd": "/home/dave/src/github.com/dwindsor/tetragon/contrib/tester-progs",
"binary": "/home/dave/src/github.com/dwindsor/tetragon/contrib/tester-progs/symlink-tester",
"flags": "execve clone",
"start_time": "2024-06-27T21:39:16.896790982Z",
"auid": 1000,
"parent_exec_id": "bGF0ZXJhbHVzOjU2NTkyMTAwMDAwMDA6ODUyNjE=",
"refcnt": 1,
"tid": 326461
},
"parent": {
"exec_id": "bGF0ZXJhbHVzOjU2NTkyMTAwMDAwMDA6ODUyNjE=",
"pid": 85261,
"uid": 1000,
"cwd": "/tmp",
"binary": "/usr/bin/bash",
"flags": "procFS auid",
"start_time": "2024-06-27T16:19:31.756135483Z",
"auid": 1000,
"parent_exec_id": "bGF0ZXJhbHVzOjU2NTkyMDAwMDAwMDA6ODUyNjA=",
"tid": 85261
},
"function_name": "security_inode_follow_link",
"args": [
{
"dentry_arg": {
"name": "/tmp/id"
}
}
],
"action": "KPROBE_ACTION_POST",
"policy_name": "path-traversal-block",
"return_action": "KPROBE_ACTION_POST"
},
"node_name": "lateralus",
"time": "2024-06-27T21:39:16.897100809Z"
}
Hi,
Thanks for the review @kkourt! I've responded to your comments and fixed the formatting issues, but I'm having trouble getting the test to work.
Thanks.
I think the issue might be that you are killing the test program before it has a chance to do the operation. I've tried the test locally, and this seems to work for me:
diff --git a/pkg/sensors/tracing/kprobe_test.go b/pkg/sensors/tracing/kprobe_test.go
index db8a53f0b..55fcbbaa6 100644
--- a/pkg/sensors/tracing/kprobe_test.go
+++ b/pkg/sensors/tracing/kprobe_test.go
@@ -6049,7 +6049,6 @@ func TestDentryExtractPath(t *testing.T) {
ops := func() {
err = command.Start()
assert.NoError(t, err)
- defer command.Process.Kill()
}
events := perfring.RunTestEvents(t, ctx, ops)
$ sudo $(which go) test ./pkg/sensors/tracing -bpf-lib $(pwd)/bpf/objs -test.run TestDentryExtractPath
ok github.com/cilium/tetragon/pkg/sensors/tracing 1.325s
Could you also rebase to latest main? I believe it should address some of the e2e test failures.
could you please split the change into separate logical parts? like at least bpf bits. tests, schema changes.. anything that makes sense to split, thnx
Hi @kkourt, thanks for the pointers! Your suggestion worked.
@olsajiri I've split the PR into logical patches. Some CI is failing now but for unrelated reasons.
Thanks!
Hi @kkourt, thanks for the pointers! Your suggestion worked.
@olsajiri I've split the PR into logical patches. Some CI is failing now but for unrelated reasons.
Thanks, can you also rebase? There seem to be conflicts. The conflicts seem to be in the auto-generated files, so it should be easy to resolve.
Hi @kkourt, thanks for the pointers! Your suggestion worked. @olsajiri I've split the PR into logical patches. Some CI is failing now but for unrelated reasons.
Thanks, can you also rebase? There seem to be conflicts. The conflicts seem to be in the auto-generated files, so it should be easy to resolve.
Thanks! I've tried running make crds, but it shows no differences to push
Hi @kkourt, thanks for the pointers! Your suggestion worked. @olsajiri I've split the PR into logical patches. Some CI is failing now but for unrelated reasons.
Thanks, can you also rebase? There seem to be conflicts. The conflicts seem to be in the auto-generated files, so it should be easy to resolve.
Thanks! I've tried running make crds, but it shows no differences to push
Have you tried running make protogen as well? The conflicts seem to be related to protobuf definitions.
Any progress on this?
I believe what this PR is trying to do can be achieved via https://github.com/cilium/tetragon/pull/3143, which is a more generic approach.
Rebasing and running make generate and make codegen should be all that is needed, I made an updated pr here https://github.com/cilium/tetragon/pull/3353
Any progress on this?
I believe what this PR is trying to do can be achieved via #3143, which is a more generic approach.
Oh nice, will that go in soon?
Any progress on this?
I believe what this PR is trying to do can be achieved via #3143, which is a more generic approach.
use resolve: "d_name.name" to get dentry? but how to get the full path? like /tmp/id,use d_name.name, only get id.
use
resolve: "d_name.name"to get dentry? but how to get the full path? like/tmp/id,used_name.name, only getid.
hum, could you please share your spec? at least which hook are you attached to? thanks
use
resolve: "d_name.name"to get dentry? but how to get the full path? like/tmp/id,used_name.name, only getid.hum, could you please share your spec? at least which hook are you attached to? thanks
kind: TracingPolicy
metadata:
name: "file-tamper-mknod"
spec:
kprobes:
- call: "security_path_mknod"
syscall: false
return: true
args:
- index: 0
type: "path"
- index: 1
type: "string"
resolve: "d_name.name"
returnArg:
index: 0
type: "int"
my TracingPolicy like this, use d_name.name or d_iname only retrieve the current part of the path, not the full path.
The result is as shown in the picture：
use
resolve: "d_name.name"to get dentry? but how to get the full path? like/tmp/id,used_name.name, only getid.hum, could you please share your spec? at least which hook are you attached to? thanks
kind: TracingPolicy metadata: name: "file-tamper-mknod" spec: kprobes: - call: "security_path_mknod" syscall: false return: true args: - index: 0 type: "path" - index: 1 type: "string" resolve: "d_name.name" returnArg: index: 0 type: "int"my TracingPolicy like this, use
d_name.nameord_inameonly retrieve the current part of the path, not the full path.
ok, for that we'd need the 'dentry' type ... you have path and dentry arguments that give the full path, but if you want to get full path from the dentry argument alone I think we need the dentry type .. we can revive this PR
use
resolve: "d_name.name"to get dentry? but how to get the full path? like/tmp/id,used_name.name, only getid.hum, could you please share your spec? at least which hook are you attached to? thanks
kind: TracingPolicy metadata: name: "file-tamper-mknod" spec: kprobes: - call: "security_path_mknod" syscall: false return: true args: - index: 0 type: "path" - index: 1 type: "string" resolve: "d_name.name" returnArg: index: 0 type: "int"my TracingPolicy like this, use
d_name.nameord_inameonly retrieve the current part of the path, not the full path.ok, for that we'd need the 'dentry' type ... you have
pathanddentryarguments that give the full path, but if you want to get full path from thedentryargument alone I think we need thedentrytype .. we can revive this PR
Any progress on this? Is there anything I can help with? I'd be happy to.
ok, for that we'd need the 'dentry' type ... you have
pathanddentryarguments that give the full path, but if you want to get full path from thedentryargument alone I think we need thedentrytype .. we can revive this PRAny progress on this? Is there anything I can help with? I'd be happy to.
I refreshed that and have draft version in here https://github.com/cilium/tetragon/pull/3423, it does not support walking through the different mounts ATM, need to figure that out, hopfuly this week