Skip to content
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

Open
2 tasks done
anfedotoff opened this issue May 3, 2024 · 16 comments
Open
2 tasks done

Tetragon based file integrity monitoring (FIM) #2409

anfedotoff opened this issue May 3, 2024 · 16 comments
Labels
kind/enhancement This improves or streamlines existing functionality

Comments

@anfedotoff
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

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

  • I agree to follow this project's Code of Conduct
@anfedotoff anfedotoff added the kind/enhancement This improves or streamlines existing functionality label May 3, 2024
@anfedotoff
Copy link
Contributor Author

Hi 👋 , @kkourt!
If you have time, please, have a look. I'll be happy to have some discussion on implementation details.

@xmulligan
Copy link
Member

Once it is ready, please also add the CfP to the repo https://github.com/cilium/design-cfps

@kkourt
Copy link
Contributor

kkourt commented May 17, 2024

Thanks @anfedotoff!

Here are some first thoughts:

Considering your proposal:

spec:
  lsm:
  - call: "bprm_check_security"
    args:
    - index: 0
      type: "linux_binprm" # file type also is allowed
    selectors:
      - matchArgs:
          - index: 0
            operator: "Prefix"
            values:
              - "/usr/bin"
      - matchActions:
          - action: FileHash
            argHash 0

In the BPF code, what we do is:

For the linux_binprm type, we first copy the path:

case linux_binprm_type: {
struct linux_binprm *bprm = (struct linux_binprm *)arg;
struct file *file;
arg = (unsigned long)_(&bprm->file);
probe_read(&file, sizeof(file), (const void *)arg);
path_arg = _(&file->f_path);
goto do_copy_path;

We then filter:

case string_type:
case net_dev_ty:
case data_loc_type:
/* for strings, we just encode the length */
pass &= filter_char_buf(filter, args, 4);
break;

And finally do the action:

do_actions(void *ctx, struct msg_generic_kprobe *e, struct selector_action *actions,

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:

spec:
  lsm:
  - call: "bprm_check_security"
    args:
    - index: 0
      type: "linux_binprm" # file type also is allowed
    - index: 1 # argument 1 will be the result of applying operation ima_file_hash() to argument index 0
      type: "hash"
      sourceIndex: 0
      operator: "ima_file_hash"
    selectors:
      - matchArgs:
          - index: 0
            operator: "Prefix"
            values:
              - "/usr/bin"

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.

@anfedotoff
Copy link
Contributor Author

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 bpf_ima_inode_hash (5.15) and bpf_ima_file_hash (5.18): if there is no hash in IMA-measurement list bpf_ima_file_hash will calculate the hash, update IMA-measurement list and return it to the caller.

operator: "ima_file_hash"

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.

@kkourt
Copy link
Contributor

kkourt commented May 17, 2024

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.

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.

As far as I concerned, IMA bpf-helpers just retrieve the hash from IMA-measurement list. Difference between bpf_ima_inode_hash (5.15) and bpf_ima_file_hash (5.18): if there is no hash in IMA-measurement list bpf_ima_file_hash will calculate the hash, update IMA-measurement list and return it to the caller.

operator: "ima_file_hash"

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 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.

@anfedotoff
Copy link
Contributor Author

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.

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:)).

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.

It makes sense. I'll take time to learn more about how to validate tracing policy for correctness.

@kkourt
Copy link
Contributor

kkourt commented May 22, 2024

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:

func detectKprobeMulti() bool {

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:

useMulti := !specOpts.DisableKprobeMulti && !option.Config.DisableKprobeMulti && bpf.HasKprobeMulti()
.

@anfedotoff anfedotoff mentioned this issue Jul 1, 2024
@anfedotoff
Copy link
Contributor Author

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 args being resolved we will retrieve/calculate hash for every file! If we use action, we will have args filter and we will calculate hash only fo /etc/passwd. The problem is how to get parameters that are needed to call ima_helpers. IIUC, arguments are saved to msg_generic_kprobe here:

#ifdef GENERIC_LSM
struct bpf_raw_tracepoint_args *raw_args = (struct bpf_raw_tracepoint_args *)ctx;
e->a0 = BPF_CORE_READ(raw_args, args[0]);
e->a1 = BPF_CORE_READ(raw_args, args[1]);
e->a2 = BPF_CORE_READ(raw_args, args[2]);
e->a3 = BPF_CORE_READ(raw_args, args[3]);
e->a4 = BPF_CORE_READ(raw_args, args[4]);
generic_process_init(e, MSG_OP_GENERIC_LSM, config);
#endif

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 msg_generic_kprobe is good and easy choice for that. @kkourt what's your opinion on this?

@anfedotoff
Copy link
Contributor Author

Another question is where to store an ima_hash?

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.

@kkourt
Copy link
Contributor

kkourt commented Aug 12, 2024

The usual way of passing arguments to userspace is to store them in ->args of msg_generic_kprobe

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.

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?

@anfedotoff
Copy link
Contributor Author

anfedotoff commented Aug 12, 2024

The usual way of passing arguments to userspace is to store them in ->args of msg_generic_kprobe

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.

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 ->args to pass hashes to user space. I suppose it is possible to put hashes in ->args at Action phase? Maybe it is better to use ->args, as you suggest.

@kkourt
Copy link
Contributor

kkourt commented Aug 12, 2024

I suppose it is possible to put hashes in ->args at Action phase? Maybe it is better to use ->args, as you suggest.

That's a good question! I don't see why not, but it's not something we have done before I believe.

@anfedotoff
Copy link
Contributor Author

I suppose it is possible to put hashes in ->args at Action phase? Maybe it is better to use ->args, as you suggest.

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 ->args! I found a solution about how to reserve space for a hash and pass hashes to user space!

@anfedotoff
Copy link
Contributor Author

anfedotoff commented Aug 20, 2024

I started to develop IMA hashes collection for LSM events: (#2818) and met some problems:

  1. bpf_ima_inode_hash/bpf_ima_file_hash can be called only from BPF_F_SLEEPABLE lsm programs: lsm.s.
  2. lsm.s programs are strictly limited for maps usage. Only arrays, hashes and ringbuffer are allowed. We can't use BPF_MAP_TYPE_PROG_ARRAY and BPF_MAP_TYPE_PERF_EVENT_ARRAY. It means for us that we can't use bpf-to-bpf calls from lsm.s programs and send events from lsm.s programs with perfbuffer.
  3. verfier doesn't allow us to call ima helpers from generic programs even if we make them sleepable and use allowed maps: 38: (85) call bpf_ima_file_hash#193 R1 type=scalar expected=ptr_, trusted_ptr. R1 is always turns out as scalar value.

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 /etc/passwd according to policy. This success and Kevin's suggestions in the Tetragon Slack channel gave me an idea for solving the IMA problem. I'll describe my solution a little bit later. Solution is might not be so pretty as we have for generic bpf programs due to limitations of lsm.s programs. But we can discus it!

cc: @kkourt, @kevsecurity

@anfedotoff
Copy link
Contributor Author

anfedotoff commented Aug 21, 2024

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.

photo_2024-08-21_15-39-59

We can support ima_hash collection for this hooks (I think with them we can handle most of the cases):

  • security_bprm_check(struct linux_binprm *bprm)
  • security_file_open(struct file *file)
  • security_mmap_file(struct file *file, unsigned long prot, unsigned long flags)

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:)

@anfedotoff
Copy link
Contributor Author

anfedotoff commented Aug 21, 2024

Btw, we can use struct msg_execve_key current, as key for ima_hashes map. So we only need to set
e->common.flags at action phase, that we need to tail call to lsm.s after generic_output event. Also I think we can use BPF_MAP_LRU_HASH to store ima hashes. (I hope we able to use this type of maps).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement This improves or streamlines existing functionality
Projects
None yet
Development

No branches or pull requests

3 participants