tetragon icon indicating copy to clipboard operation
tetragon copied to clipboard

selectors: add matchParentBinaries selector

Open kobrineli opened this issue 1 month ago • 34 comments

Description

This change adds matchParentBinaries selector, which might be useful for proper and granular filtering of parent binaries, which is needed in some specific cases.

For instance, there is a python script, which invokes some system calls, which we want to intercept and report. If such script is executed by some system process, we want to filter it out. Otherwise, we report it. For this filtering we need a selector for parent binary, because we cannot filter out events only by current binary, which in case of python script execution is always python.

For more real example, consider we want to hook all calls of chmod system call to prevent creating new binaries on the system manually. apt-key binary, when it installs some packages (such cases we don't want to report), doesn't call chmod directly, but uses /usr/bin/chmod binary, which calls chmod system call inside. matchParentBinaries selector would help to create accurate exclusion for this case.

Example of policy with matchParentBinaries selector:

Consider we want to get events about all files, which were made executable, with chmod syscall, but don't want to get events about apt-key making files executable. Unfortunately, apt-key doesn't make files executable with syscall directly, but uses /usr/bin/chmod binary, which calls chmod function, so to filter such events we need to have selectors for both parent and current binary, so the resulting policy will look like this:

apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: chmod-x-bit
spec:
  kprobes:
    - call: "sys_chmod"
      syscall: true
      args:
        - index: 0
          type: "string"
          label: "pathname"
        - index: 1
          type: "int"
          label: "mode"
      selectors:
        - matchArgs:
            - index: 1
              operator: "Mask"
              values:
                - "73" # X bit for owner, group or all (001001001)
          matchBinaries:
            - operator: "NotIn"
              values:
                "/usr/bin/chmod"
        - matchArgs:
            - index: 1
              operator: "Mask"
              values:
                - "73" # X bit for owner, group or all (001001001)
          matchBinaries:
            - operator: "In"
              values:
              - "/usr/bin/chmod"
          matchParentBinaries:
            - operator: "NotIn"
              values:
              - "/usr/bin/apt-key"

If current binary is /usr/bin/chmod, we don't care about parent binary, but if current binary is /usr/bin/chmod, we don't want the parent binary to be /usr/bin/apt-key.

Changelog

kobrineli avatar Oct 27 '25 13:10 kobrineli

Deploy Preview for tetragon ready!

Name Link
Latest commit 9defede66357a1eaf9fc7136a12f18ea82c68115
Latest deploy log https://app.netlify.com/projects/tetragon/deploys/6942ae0ca9537400080e7ff4
Deploy Preview https://deploy-preview-4254--tetragon.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

netlify[bot] avatar Oct 27 '25 13:10 netlify[bot]

Probably I should add docs for this selector

kobrineli avatar Oct 27 '25 13:10 kobrineli

Another thing to note is that the functionality is similar to https://tetragon.io/docs/concepts/tracing-policy/selectors/#follow-children. So I wonder if something like:

- matchBinaries:
  - operator: "In"
    values:
    - "/usr/sbin/sshd"
    followChildren: true
  - operator: "NotIn"
    values:
    - "/usr/sbin/sshd"

Would achieve a similar result. The first thing to check would be if matchBinaries are ANDed as, for example, MatchAargs are. The second thing, is that this will not just match the parent, but every ancestor which might or might not be what we want.

kkourt avatar Oct 27 '25 14:10 kkourt

@kkourt Tetragon currently supports only one match binary operator by selector: https://github.com/cilium/tetragon/blob/main/pkg/selectors/kernel.go#L1426

So listing multiple operators in match binary selector will fail.

kobrineli avatar Oct 27 '25 14:10 kobrineli

One more thing to note is that this looks related to

@kkourt Tetragon currently supports only one match binary operator by selector: https://github.com/cilium/tetragon/blob/main/pkg/selectors/kernel.go#L1426

So listing multiple operators in match binary selector will fail.

Thanks!

I don't remember why this limitation exists, but my guess is that this is something that can be fixed. So, then, the question in my mind is whether the use-case you are describing is better served by matching on the immediate parent or all on ancestors (for which some support already exists).

PS: Might be worth updating the docs to reflect above.

kkourt avatar Oct 27 '25 14:10 kkourt

@kkourt Tetragon currently supports only one match binary operator by selector: https://github.com/cilium/tetragon/blob/main/pkg/selectors/kernel.go#L1426

So listing multiple operators in match binary selector will fail.

I wonder if supporting 2 binary selectors would be easier than supporting N, but would potentially achieve the result without supporting parent selectors altogether?

We could maybe have a limit to how many "generations" of children we follow. In this case, a limit of 1 generation would prevent reporting grandchildren. Haven't looked to see if this is easy or not however.

kevsecurity avatar Oct 27 '25 14:10 kevsecurity

I wonder if supporting 2 binary selectors would be easier than supporting N, but would potentially achieve the result without supporting parent selectors altogether?

Currently exactly 1 matchBinaries selector is mapped by selector idx. If it is not complicated to support mapping the array of matchBinaries selectors by selector idx, probably with additional field with index in tg_mb_paths map, then it would be easier, than supporting matchParents selector. But I'm afraid, that such changes will lead to hard refactoring in bpf code, while supporting separate selector, which can also be only one for current selectors set, seems pretty easy.

Moreover, maybe explicit parents filtering, working in the same way as current process binary filtering, is easier to understand and to read.

kobrineli avatar Oct 27 '25 15:10 kobrineli

So, then, the question in my mind is whether the use-case you are describing is better served by matching on the immediate parent or all on ancestors (for which some support already exists).

With current existing support we cannot exclude the binary itself because of the selector number limitation. Moreover, we may want to apply filter for concrete child of parent process, rather than to all process children (as with followChildren option), combining matchBinaries and matchParents selectors.

kobrineli avatar Oct 27 '25 15:10 kobrineli

So I wonder if something like:

- matchBinaries:
  - operator: "In"
    values:
    - "/usr/sbin/sshd"
    followChildren: true
  - operator: "NotIn"
    values:
    - "/usr/sbin/sshd"

Would achieve a similar result.

If we want to match concrete child process, seems like this wouldn't achieve the same result.

kobrineli avatar Oct 27 '25 15:10 kobrineli

@kkourt Hi! I've separated PR into several commits.

kobrineli avatar Oct 28 '25 08:10 kobrineli

@kkourt Hi! I've separated PR into several commits.

Thanks! Can you please add some context in the commit messages (see: https://tetragon.io/docs/contribution-guide/submitting-a-pull-request/)?

Before merging the PR, we would need tests and documentation updates. If you are looking for early feedback, can you add an example (maybe in the commit message) on how to use the new selector?

kkourt avatar Oct 28 '25 08:10 kkourt

I'm also seeing some CI failures:

--- FAIL: TestKprobeSigkillExecveMap1 (306.00s)
    kprobe_sigkill_test.go:65: child pid is 32313 and spec file is /tmp/sigkill-3506928072.yaml
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="BTF discovery: default kernel btf file found" btf-file=/sys/kernel/btf/vmlinux
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="Creating new EventCache" retries=15 delay=2s
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="Starting process manager" enableK8s=false enableProcessCred=true enableProcessNs=true
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="Starting JSON exporter"
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="Cgroup rate disabled (0/0s)"
    logcapture.go:24: time=2025-10-28T08:17:34.682Z level=INFO msg="BTF file: using metadata file" metadata=/sys/kernel/btf/vmlinux
    logcapture.go:24: time=2025-10-28T08:17:34.682Z level=INFO msg="Loading sensor" name=__base__
    logcapture.go:24: time=2025-10-28T08:17:34.682Z level=INFO msg="Loading kernel version 6.11.11"
    logcapture.go:24: time=2025-10-28T08:17:34.945Z level=INFO msg="Loaded sensor successfully" sensor=__base__
    logcapture.go:24: time=2025-10-28T08:17:34.984Z level=INFO msg="Read ProcFS /proc appended 222/284 entries"
    logcapture.go:24: time=2025-10-28T08:17:34.984Z level=WARN msg="failed to put value in execve_map" value="&{Process:{Pid:10 Pad:0 Ktime:540000000} Parent:{Pid:2 Pad:0 Ktime:540000000} Flags:0 Nspid:0 Namespaces:{UtsInum:4026531838 IpcInum:4026531839 MntInum:4026531841 PidInum:4026531836 PidChildInum:4026531836 NetInum:4026531840 TimeInum:4026531834 TimeChildInum:4026531834 CgroupInum:4026531835 UserInum:4026531837} Capabilities:{Permitted:2199023255551 Effective:2199023255551 Inheritable:0} Binary:{PathLength:27 Reversed:0 Path:[107 119 111 114 107 101 114 47 48 58 48 72 45 101 118 101 110 116 115 95 104 105 103 104 112 114 105 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] End:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] EndR:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Args:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] MBSet:0 MBGen:0}}" error="update: key too big for map: argument list too long"

kkourt avatar Oct 28 '25 08:10 kkourt

I'm also seeing some CI failures:

--- FAIL: TestKprobeSigkillExecveMap1 (306.00s)
    kprobe_sigkill_test.go:65: child pid is 32313 and spec file is /tmp/sigkill-3506928072.yaml
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="BTF discovery: default kernel btf file found" btf-file=/sys/kernel/btf/vmlinux
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="Creating new EventCache" retries=15 delay=2s
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="Starting process manager" enableK8s=false enableProcessCred=true enableProcessNs=true
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="Starting JSON exporter"
    logcapture.go:24: time=2025-10-28T08:17:34.681Z level=INFO msg="Cgroup rate disabled (0/0s)"
    logcapture.go:24: time=2025-10-28T08:17:34.682Z level=INFO msg="BTF file: using metadata file" metadata=/sys/kernel/btf/vmlinux
    logcapture.go:24: time=2025-10-28T08:17:34.682Z level=INFO msg="Loading sensor" name=__base__
    logcapture.go:24: time=2025-10-28T08:17:34.682Z level=INFO msg="Loading kernel version 6.11.11"
    logcapture.go:24: time=2025-10-28T08:17:34.945Z level=INFO msg="Loaded sensor successfully" sensor=__base__
    logcapture.go:24: time=2025-10-28T08:17:34.984Z level=INFO msg="Read ProcFS /proc appended 222/284 entries"
    logcapture.go:24: time=2025-10-28T08:17:34.984Z level=WARN msg="failed to put value in execve_map" value="&{Process:{Pid:10 Pad:0 Ktime:540000000} Parent:{Pid:2 Pad:0 Ktime:540000000} Flags:0 Nspid:0 Namespaces:{UtsInum:4026531838 IpcInum:4026531839 MntInum:4026531841 PidInum:4026531836 PidChildInum:4026531836 NetInum:4026531840 TimeInum:4026531834 TimeChildInum:4026531834 CgroupInum:4026531835 UserInum:4026531837} Capabilities:{Permitted:2199023255551 Effective:2199023255551 Inheritable:0} Binary:{PathLength:27 Reversed:0 Path:[107 119 111 114 107 101 114 47 48 58 48 72 45 101 118 101 110 116 115 95 104 105 103 104 112 114 105 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] End:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] EndR:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Args:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] MBSet:0 MBGen:0}}" error="update: key too big for map: argument list too long"

Is it an issue of this PR? There is error update: key too big for map: argument list too long while putting a value into execve_map, which was not changed in this PR.

kobrineli avatar Oct 28 '25 09:10 kobrineli

I see checkpatch is complaining and other CI checks, you can run make checkpatch locally and see more docs on how to create a nice PR here https://tetragon.io/docs/contribution-guide/submitting-a-pull-request/ if that can help :)

mtardy avatar Oct 28 '25 15:10 mtardy

@olsajiri Hi! Current implementation of matchBinaries selector allows to have only one matchBinaries per selector, so filtering both current binary and parent binary will not be possible.

matchBinaries selector refactoring might be complicated and not so easy as adding additional new selector for direct parent filtering.

Moreover, matchBinaries selector supports followChildren option, which works with mbsetid field of process in tetragon execve map, so combining 2 different matchBinaries selector, which both can change this field, will be a problem, and this place will need to be changed as well.

kobrineli avatar Oct 29 '25 09:10 kobrineli

@olsajiri Hi! Current implementation of matchBinaries selector allows to have only one matchBinaries per selector, so filtering both current binary and parent binary will not be possible.

would both be enabled at the same time? do we have use case for that?

matchBinaries selector refactoring might be complicated and not so easy as adding additional new selector for direct parent filtering.

sure, we can do hard things ;-)

Moreover, matchBinaries selector supports followChildren option, which works with mbsetid field of process in tetragon execve map, so combining 2 different matchBinaries selector, which both can change this field, will be a problem, and this place will need to be changed as well.

did not think of that yet, but perhaps we could just disable it of for parent filter?

olsajiri avatar Oct 29 '25 10:10 olsajiri

would both be enabled at the same time? do we have use case for that?

If we want granular exclusion for some specific case, we may want to exclude both parent and current binary at the same time.

sure, we can do hard things ;-)

Do we really need to do them, if adding simple selector seems much easier, and the syntax of separate selector for parent filtering may be easier to understand (imho).

kobrineli avatar Oct 29 '25 10:10 kobrineli

@kevsecurity @mtardy I've made all fixes. CI still fails on Go Test with error in TestKprobeSigkillExecveMap1 test, which is not related to this PR at all, so seems like the problem is not in changes from the PR. However, all tests for specific kernel versions passed, including added tests for new selector.

The other failing task is build-every-commit, which fails with this:

Committer identity unknown

*** Please tell me who you are.

Run

  git config --global user.email "[email protected]"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: empty ident name (for <runner@runnervmwhb2z.4giwmtbyxs3efnkvhxwlgixj1f.dx.internal.cloudapp.net>) not allowed

I don't know what is the cause of this errors, but seems also not related to PR.

upd. Will try to add followChildren option as we discussed with @olsajiri and add some tests for combining binary and parent selectors.

kobrineli avatar Oct 29 '25 12:10 kobrineli

@olsajiri @kkourt I removed code duplication, tried to make match binaries parsing more generic. followChildren works fine for matchParentBinaries.

CI fails, but failing jobs are e2e tests (failed to install helm), Go Test (fail on execve map test, which is not related to this pr, seems like more general problem wih this test) and build every commit check. Other jobs passed, tests on vm with different kernels passed as well.

kobrineli avatar Oct 29 '25 20:10 kobrineli

It's not obvious to me that the go test failure is unrelated. The main branch for this test seems to be mostly green https://isovalent.slack.com/archives/C09KTK2PB36/p1761583400602649 and I don't think I've seen this failure before.

Can you please squash the changes from the review feedback into the relevant original commits? (git rebase --interactive using the squash and fixup actions should help).

build every commit check

This should be fixed as well, to make the git tree bisectable.

kkourt avatar Oct 29 '25 20:10 kkourt

@kkourt Still didn't manage to learn what is the problem It's interesting, that in vmtests the same test passed successfully:

✅ pkg.sensors.tracing.TestKprobeSigkillExecveMap1 (total:1 failed:0 skipped:0) 3.349768698s 6m5.223952926s

I've opened draft PR to test bpf changes (because I cannot reproduce the same result locally), and indeed, without my changes in bpf code this tests passes successfully. But I still don't understand, how at the same time tests for different kernel versions pass.

kobrineli avatar Oct 30 '25 10:10 kobrineli

@kkourt Hi! Tests finally passed, all seems to be done.

You were right, the test failure was related to pr changes: in test with execvemap with 1 entry, we failed to find parent process and returned error, but in fact we shouldn't return error if there were no parent filters, event if parent process not found.

kobrineli avatar Oct 31 '25 13:10 kobrineli

@kkourt Hi! I've fixed all issues.

I've decided to implement the variant with max_selectors offset in maps, because it was easier to do the same for paths map. Separated commits as you asked and added more description in commit introducing matchParentBinaries selector.

Failing test for ARM seems to be expected: https://github.com/cilium/tetragon/pull/4291#issuecomment-3487459186

Also test TestSigkill for kernel 6.12 failed (and only for it), not sure whether it is related to PR changes. Could you restart it, please?

kobrineli avatar Nov 05 '25 13:11 kobrineli

Hello ! I'm joining the conversation because I think this PR is very interesting. I'm not very aware of how the execve_map works, but what happened if the real_parent is a kernel thread ? Is it skipped and we reach the grand parent ?

I'm wondering if it could be possible to not add a new matchParentBinaries selector but instead add a new type: "task_struct". This idea is to use the execve_map to retrieve the binary path and compare it with the user specific data. And if the task_struct does not exists in the map, we use d_path_local to build the path. It's just an idea, but what do you think ?

Like this we could do something like

data:
- type: "task_struct"
  source: "current_task"
  resolve: "real_parent"
selectors:
- matchData:
  - index: 0
    operator: "Equal"
    values:
    - "/usr/bin/apt-key"

This would also allowed to get real_parents and have more granular control on task_struct chaining.

tdaudi avatar Nov 06 '25 10:11 tdaudi

@olsajiri I've fixed your issue. I renamed fields and added generic prefix, but all mactchBinaries-like selectors are now stored in same maps.

All tests passed.

kobrineli avatar Nov 06 '25 11:11 kobrineli

Hello ! I'm joining the conversation because I think this PR is very interesting. I'm not very aware of how the execve_map works, but what happened if the real_parent is a kernel thread ? Is it skipped and we reach the grand parent?

Hi! I'm not quite sure how it works in this case, but if we cannot find entry in execve map for real parent, we will try to find it for the real parent of real parent, i.e. we will reach grand parent: https://github.com/cilium/tetragon/blob/main/bpf/lib/bpf_task.h#L114-L120

I'm wondering if it could be possible to not add a new matchParentBinaries selector but instead add a new type: "task_struct".

I think that adding matchParentBinaries is more user-friendly and I like that it will be matchBinaries-like.

Did I understand correctly, that your idea is to get binary path of the real parent even if it is not found in execve map? Maybe we can implement it with matchParentBinaries selector, but just change the logic in bpf code?

I think it's good idea, but I suggest to implement in separate PR after this one is merged, and discuss it in more details there.

kobrineli avatar Nov 06 '25 11:11 kobrineli

@olsajiri Hi! Are there any issues left that block merge of this PR?

kobrineli avatar Nov 07 '25 08:11 kobrineli

I don't know why golangci-lint failed on windows only with issues unrelated to the PR. Ran make check locally, everything is fine:

make check
docker run --rm -v `pwd`:/app:Z -w /app --env GOTOOLCHAIN=auto docker.io/golangci/golangci-lint:v2.6.0@sha256:cc8c1277eefdb5f88ba1381ee30a8bdf709e3615db9c843c9fcc04d9ac1d27a8 golangci-lint run
0 issues.

kobrineli avatar Nov 11 '25 13:11 kobrineli

CI failed on the same job here: https://github.com/cilium/tetragon/actions/runs/19268199834/job/55091515756?pr=4316

kobrineli avatar Nov 11 '25 14:11 kobrineli

@olsajiri @kkourt Hi! I have a solution, which fixes bug described in https://github.com/cilium/tetragon/pull/4254#discussion_r2513836256.

The bug is that when we match for parent binaries, we search for parent by current process real parent pid. But if execve system call was invoked in process without previous fork system call, i.e. new binary is executed in the same process, matchParentBinaries selector would lose info about real parent of current process (same pid, different binary), and filter by actual grand parent.

There are 2 possible solutions:

  1. save parent binary in execve_map_value, but it will have big memory overhead (value size will increase almost twice)
  2. create binary_heap_map and binaries_map, which we will use to dynamically store info about parent binary only in cases when cleanup process pid is equal to current process pid. The map would look like this:
struct {
    __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
    __uint(max_entries, 1);
    __type(key, __u32);
    __type(value, struct binary);
} binary_heap_map SEC(".maps");

struct {
    __uint(type, BPF_MAP_TYPE_HASH);
    __uint(max_entries, 1);
    __type(key, __u32);
    __type(value, struct binary);
} binaries_map SEC(".maps");

And we will work with them in bpf_execve_event like this:

        if (curr->key.pid == event->cleanup_key.pid) {
            __u32 zero = 0;
            struct binary *bin = map_lookup_elem(&binary_heap_map, &zero);
            if (bin) {
                memcpy(bin, &curr->bin, sizeof(curr->bin));
                map_update_elem(&binaries_map, &curr->key.pid, bin, BPF_ANY);
            }
        }

And then, when we match parent binaries, we check for the entry in binaries map and if it exists, use it instead of parent binary. I checked, that worked.

Separate map could give us not so big memory overhead as parent binary field in execve map value, because we could set its size to be N times less than execve map (because case when parent pid and current process pid are same is not common)

What do you think about it?

kobrineli avatar Nov 14 '25 13:11 kobrineli