-
Notifications
You must be signed in to change notification settings - Fork 382
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
Add dynamic extraction of a parameter attribute #3143
base: main
Are you sure you want to change the base?
Changes from all commits
7e6600a
291d411
083e578
a4713ec
b66a26d
4c447f3
8c3b2c5
48549b3
fa9ea58
9ec44b4
135ade1
e80e73d
bd2b422
f817507
391c5a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -142,6 +142,19 @@ struct selector_arg_filters { | |
__u32 argoff[5]; | ||
} __attribute__((packed)); | ||
|
||
struct config_btf_arg { | ||
__u32 offset; | ||
__u16 is_pointer; | ||
__u16 is_initialized; | ||
} __attribute__((packed)); | ||
|
||
struct extract_arg_data { | ||
struct config_btf_arg *btf_config; | ||
unsigned long *arg; | ||
}; | ||
|
||
#define MAX_BTF_ARG_DEPTH 10 | ||
|
||
struct event_config { | ||
__u32 func_id; | ||
__s32 arg0; | ||
|
@@ -172,6 +185,11 @@ struct event_config { | |
*/ | ||
__u32 policy_id; | ||
__u32 flags; | ||
struct config_btf_arg btf_arg0[MAX_BTF_ARG_DEPTH]; | ||
struct config_btf_arg btf_arg1[MAX_BTF_ARG_DEPTH]; | ||
struct config_btf_arg btf_arg2[MAX_BTF_ARG_DEPTH]; | ||
struct config_btf_arg btf_arg3[MAX_BTF_ARG_DEPTH]; | ||
struct config_btf_arg btf_arg4[MAX_BTF_ARG_DEPTH]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not follow the user space part and have just |
||
} __attribute__((packed)); | ||
|
||
#define MAX_ARGS_SIZE 80 | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -400,6 +400,61 @@ The `maxData` flag does not work with `returnCopy` flag at the moment, so it's | |||||||||||||||||||||||||||||||||||||||||||
usable only for syscalls/functions that do not require return probe to read the | ||||||||||||||||||||||||||||||||||||||||||||
data. | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
### Advanced usage | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would maybe be more specific on this one, like "extracting structure fields/attribute", whatever you prefer that describes best this feature. |
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
For specific use cases, you may want to extract a specific attribute from the argument. | ||||||||||||||||||||||||||||||||||||||||||||
For instance you have `struct linux_binprm` as first argument and want to filter parent | ||||||||||||||||||||||||||||||||||||||||||||
process name, you can do it as following. | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+405
to
+407
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
```yaml | ||||||||||||||||||||||||||||||||||||||||||||
apiVersion: cilium.io/v1alpha1 | ||||||||||||||||||||||||||||||||||||||||||||
kind: TracingPolicy | ||||||||||||||||||||||||||||||||||||||||||||
metadata: | ||||||||||||||||||||||||||||||||||||||||||||
name: "lsm" | ||||||||||||||||||||||||||||||||||||||||||||
spec: | ||||||||||||||||||||||||||||||||||||||||||||
lsmhooks: | ||||||||||||||||||||||||||||||||||||||||||||
- hook: "bprm_check_security" | ||||||||||||||||||||||||||||||||||||||||||||
args: | ||||||||||||||||||||||||||||||||||||||||||||
- index: 0 | ||||||||||||||||||||||||||||||||||||||||||||
type: "linux_binprm" | ||||||||||||||||||||||||||||||||||||||||||||
extractParam: "mm.owner.real_parent.comm" | ||||||||||||||||||||||||||||||||||||||||||||
overwriteType: "string" | ||||||||||||||||||||||||||||||||||||||||||||
selectors: | ||||||||||||||||||||||||||||||||||||||||||||
- matchActions: | ||||||||||||||||||||||||||||||||||||||||||||
- action: Post | ||||||||||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
The above policy will display the parent process name every time the hook is called. | ||||||||||||||||||||||||||||||||||||||||||||
The `extractParam` field is used to reach a specific data into the `struct | ||||||||||||||||||||||||||||||||||||||||||||
linux_binprm`. It is important to set `overwriteType` as well to make sure the | ||||||||||||||||||||||||||||||||||||||||||||
reached data is read correctly (as a string in this case). | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+427
to
+430
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I would say |
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
{{< caution >}} | ||||||||||||||||||||||||||||||||||||||||||||
- This feature requires you to know exactly what you are looking for in the attributes | ||||||||||||||||||||||||||||||||||||||||||||
of the hook parameters. For instance, if you want to have a look on what is | ||||||||||||||||||||||||||||||||||||||||||||
available inside `struct linux_binprm`, take a look at the | ||||||||||||||||||||||||||||||||||||||||||||
[Bootlin website](https://elixir.bootlin.com/linux/v6.12.5/source/include/linux/binfmts.h#L18) | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+433
to
+436
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
- Some structures are dynamic. This means that they may change at runtime. So you need to | ||||||||||||||||||||||||||||||||||||||||||||
be aware of what you are looking for. | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+438
to
+439
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
{{< /caution >}} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
Tetragon can also handle some structures such as `struct file` or `struct | ||||||||||||||||||||||||||||||||||||||||||||
path` and few others. This means you can also extract the whole struct, if it is | ||||||||||||||||||||||||||||||||||||||||||||
available in the attributes of the parameter, and set the type with the correct type | ||||||||||||||||||||||||||||||||||||||||||||
like this : | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+442
to
+445
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
```yaml | ||||||||||||||||||||||||||||||||||||||||||||
- index: 0 | ||||||||||||||||||||||||||||||||||||||||||||
type: "linux_binprm" | ||||||||||||||||||||||||||||||||||||||||||||
extractParam: "file" | ||||||||||||||||||||||||||||||||||||||||||||
overwriteType: "file" | ||||||||||||||||||||||||||||||||||||||||||||
# Or | ||||||||||||||||||||||||||||||||||||||||||||
# extractParam: "file.f_path" | ||||||||||||||||||||||||||||||||||||||||||||
# overwriteType: "path" | ||||||||||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+447
to
+456
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Mmh not sure my suggestion is correct but I would split it in two. |
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
## Return values | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
A `TracingPolicy` spec can specify that the return value should be reported in | ||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
apiVersion: cilium.io/v1alpha1 | ||
kind: TracingPolicy | ||
metadata: | ||
name: "lsm" | ||
spec: | ||
lsmhooks: | ||
- hook: "bprm_check_security" | ||
args: | ||
- index: 0 | ||
type: "linux_binprm" | ||
extractParam: "mm.owner.real_parent.real_parent.comm" | ||
overwriteType: "string" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need overwriteType? could we maybe just do something like:
the overwrite stuff might cause headaches later also maybe rename cc @kkourt There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, but fyi it seems it's motivated by this https://github.com/cilium/tetragon/pull/3143/files#diff-3c0b10317f585c0016d0208630c3abbb0e8d39571e91254af88874710903a260R442-R455. |
||
selectors: | ||
- matchActions: | ||
- action: Post |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not follow the user space and have just
btf_arg[5][10]
I don't see
btf_arg[1-4]
being used anywhere