bpf: read and copy proc exe at execve for matchBinaries #1926
Conversation
mtardy
added
area/bpf
mtardy
force-pushed
the
pr/mtardy/fix-execve-path
branch
4 times, most recently
from
0bfffab to
eeed60e
Compare
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
mtardy
force-pushed
the
pr/mtardy/fix-execve-path
branch
from
eeed60e to
a0cb89e
Compare
|
Unfortunately, it looks like it doesn't fix for 4.19, I'll have to put it out for small progs kernels :/ |
This is needed for matchBinaries in the next commit. Previously we were using the tracepoint argugement filename, which is potentially a relative path. This change has two main benefits: all paths checked against matchBinaries will be absolutes and all symbolic links will be resolved by the kernel. Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
Previously, filename from the args was copied into the execve_map, used later for matchBinaries. With this change, we copy the absolute path we read from the proc exe at the execve tracepoint stage to use it later. Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
mtardy
force-pushed
the
pr/mtardy/fix-execve-path
branch
from
a0cb89e to
4fcb858
Compare
| #define BINARY_PATH_MAX_LEN 256 | ||
|
|
||
| struct heap_exe { | ||
| // because of verifier limitations, this has to be 2 * 256 bytes while 256 |
There was a problem hiding this comment.
Is this true with large programs? We might be able to widen this some if we feel its an actual issue.
| struct heap_exe { | ||
| // because of verifier limitations, this has to be 2 * 256 bytes while 256 | ||
| // should be theoretically sufficient, and actually is, in unit tests. | ||
| char buf[BINARY_PATH_MAX_LEN * 2]; |
There was a problem hiding this comment.
If we were a bit smarter this can be improved 2x waste of space. Over 256B though I don't care to do the work.
|
OK lets try this. Merging I want to land this in a test cluster and play with it though so I'll do that shortly. |
So I wanted to suggest for security reasons and be bullet proof go inside https://github.com/cilium/tetragon/blob/main/bpf/process/bpf_execve_bprm_commit_creds.c#L47 using bprm->file As it is now on the tracepoint hook is somehow late the window race is so small and I don't think it can be triggered to be honest, but I prefer no surprise ;-) |
Fix for #1741
This PR changes how we read the binary path at
execvetime. We now readexefrom thetask_structinstead of relying upon the tracepoint argument that was just the path given toexecve(thus possibly relative). This was an issue sincematchBinariescould be avoided using relative links instead of absolute ones atexecve.For a future patch maybe:
In addition to this, we could also use this
exevalue for the binary path of the event. We also previously used the argument ofexecve, being possibly relative and corrected in userspace usingcwd, but also possibly a symlink. This would thus introduce a user-facing change: tetragon events will now contain the canonical binary path while previously it could have been a symlink.Without that user might try to "matchBinaries" a symlink file displayed in the Tetragon events and fail to make it work.
Preliminary work would be to make
prepend_namework with 4096 (orMAX_PATH) buffer size.