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 · 7 comments
Open
2 tasks done

Tetragon based file integrity monitoring (FIM) #2409

anfedotoff opened this issue May 3, 2024 · 7 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()
.

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