-
Notifications
You must be signed in to change notification settings - Fork 380
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
Tetragon based file integrity monitoring (FIM) #2409
Comments
Hi 👋 , @kkourt! |
Once it is ready, please also add the CfP to the repo https://github.com/cilium/design-cfps |
Thanks @anfedotoff! Here are some first thoughts: Considering your proposal:
In the BPF code, what we do is: For the tetragon/bpf/process/types/basic.h Lines 2591 to 2598 in 82c4b13
We then filter: tetragon/bpf/process/types/basic.h Lines 1815 to 1820 in 82c4b13
And finally do the action: tetragon/bpf/process/types/basic.h Line 2358 in 82c4b13
So by the time we reach the action, we only have the string and we cannot get the hash. Hence, I believe we need to get the hash at the first step. So I was thinking something like:
I'm still not sure about the syntax, but the basic idea would be to push the computation of the hash early, when we extract the arguments. |
LGTM! We still able to filter by file path, before collecting a hash in your approach, right? In other words I mean not to call ima bpf-helpers if filtering is not passed. As far as I concerned, IMA bpf-helpers just retrieve the hash from IMA-measurement list. Difference between
Here you mean to call appropriate bpf-helper according to kernel version? Or user specifies the helper it prefers? I think, the first way is better. |
I think it should be possible to collect the hash after the filtering, but it's more tricky. In that case, collecting the hash in the action makes more sense to me, but we will need to maintain the necessary arguments to call the helpers.
I would do the simple thing first, allowing users to specify exactly what they want. We can add a detection function to reject the policy if the helper does not exist. |
Ah, I understood. Before args filtering, we need to retrieve all arguments. I think we can try to implement your approach. To get hash using an action, we need to store arguments for bpf-helpers somewhere (suppose in separate bpf-map). So, for now, using actions looks more complicated for me:)).
It makes sense. I'll take time to learn more about how to validate tracing policy for correctness. |
Here's an example of checking whether the "multi kprobe" feature is supported: Line 45 in e7c9ec3
What we can do then is check to see whether a specific feature is supported iff it's used by a tracing policy. See for example: tetragon/pkg/sensors/tracing/enforcer.go Line 283 in e7c9ec3
|
We already have #2566 merged, so I can start implementing IMA FIM 🚀! I came to the conclusion that Action for IMA Hash is better at the end and it is not so hard to implement as I think before LSM sensor PR. Maybe I became more familiar with Tetragon, who knows). So, let's consider the following tracingPolicy and imagine that we want to get IMA hash for file being opened: apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
name: "lsm-file-open"
spec:
lsmhooks:
- hook: "file_open"
args:
- index: 0
type: "file"
selectors:
- matchArgs:
- index: 0
operator: "Equal"
values:
- "/etc/passwd" File_open hook is triggered very often. So if we get hash at the time the tetragon/bpf/process/generic_calls.h Lines 225 to 234 in 2b07de6
These arguments are pointers that we need to pass to the ima_helpers. All other information that we need is also available at the Action phase. Another question is where to store an ima_hash? I think |
I think we can use a separate map BPF_MAP_TYPE_HASH for passing hashes to user space. The key can be u64 value (pid+4bytes of hash). So, this implementation Action + map will be look like stacktrace Action implementation. I think this will be a good design decision, because this new action will have week coupling with other tetragon code base. |
The usual way of passing arguments to userspace is to store them in
Adding a map seems like a premature optimization to me. Do we really need it? What is the use-case we are trying to optimize? |
For me it's OK to use |
That's a good question! I don't see why not, but it's not something we have done before I believe. |
Yes, we can use |
I started to develop IMA hashes collection for LSM events: (#2818) and met some problems:
Good news is that we can use bpf-to-bpf to lsm.s from lsm programs. In #2818 I managed to get IMA hash and print it to tracing_pipe. At least this is possible. I used such tracingpolicy: apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
name: "lsm"
spec:
lsmhooks:
- hook: "file_open"
args:
- index: 0
type: "file"
selectors:
- matchBinaries:
- operator: "Postfix"
values:
- "cat"
matchArgs:
- index: 0
operator: "Equal"
values:
- "/etc/passwd"
matchActions:
- action: NoPost Filtration worked I managed to get only hash for cc: @kkourt, @kevsecurity |
As I promised, I put my thoughts about overcoming our problems. First of all, I think we need to make as less changes in bpf code part as possible. Adding new code is better than changing generic concepts. Below I put a picture of generic_lsm sensor bpf part. We can support ima_hash collection for this hooks (I think with them we can handle most of the cases):
Depending on tracing policy, we will load an appropriate bpf program for hash calculation. Basically, proposed approach is look like stacktrace collection we already have in tetragon. @kkourt , @kevsecurity looking forward for your comments:) |
Btw, we can use |
Implemented in #2818 |
Is there an existing issue for this?
Is your feature request related to a problem?
No response
Describe the feature you would like
We could use Tetragon for file integrity monitoring: collect hashes of executed binaries and opened files and put this information in events. Hashes are calculated using IMA-measurement Linux integrity subsystem.
Describe your proposed solution
We already talked about FIM. I found some technical issues during my research, so I decided to provide a CFP before PR.
Code of Conduct
The text was updated successfully, but these errors were encountered: