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

helm: mount /sys/kernel/security/lsm for generic_lsm sensor #3404

Closed
wants to merge 1 commit into from

Conversation

anfedotoff
Copy link
Contributor

We need to mount /sys/kernel/security/lsm from host to check if lsm bpf is enabled.

Fixes #3392

@anfedotoff anfedotoff requested a review from a team as a code owner February 15, 2025 12:40
@anfedotoff anfedotoff requested a review from mtardy February 15, 2025 12:40
This file is needed for generic_lsm sensor to check if lsm bpf is enabled.

Signed-off-by: Andrei Fedotov <[email protected]>
Copy link
Member

@mtardy mtardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for the PR.

Comment on lines +117 to +119
- name: security-lsm
hostPath:
path: /sys/kernel/security/lsm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering that using hostPath is a sensitive feature in k8s and that tetragon is a runtime security product we should maybe do a few things here:

Copy link
Contributor Author

@anfedotoff anfedotoff Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I ask some questions, just to make it clear, please?

  1. I think, type: "File might be OK, but if /sys/kernel/security/lsm is not exists (in other words LSM is not enabled), than Pod will not be loaded, right? I think it's not good.
  2. If we use an option, it might fix the problem above, but what will be if we have nodes with different configuration? Some of the nodes have LSM enabled, but some don't?
  3. Am I right that this config is the superset of this config? In other words, I can add some field to helm, but it is not necessary to add this field to tetragon.yaml?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For point 3, just worry about https://github.com/cilium/tetragon/blob/main/install/kubernetes/tetragon/values.yaml, the other one is just an example helm config, I don't exactly know what is its use.

For your point 1 and 2, I did some research, and in any case, if the file the volume points to do not exists, the pod creation will fail, even without a type.

Failed attempt 1

So ideally we would write something like this:

apiVersion: v1
kind: Pod
metadata:
  name: lsm-pod
spec:
  containers:
    - name: nginx
      image: nginx
      volumeMounts:
        - name: lsm-volume
          mountPath: /sys/kernel/security/lsm
          readOnly: true
  volumes:
    - name: lsm-volume
      hostPath:
        path: /sys/kernel/security/lsm
        type: File

But for some reason I don't understand, containerd OCI runtime creation failed to create antyhing under /sys/kernel/security/lsm with

Error: failed to create containerd task: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error mounting "/sys/kernel/security/lsm" to rootfs at "/sys/kernel/security/lsm": open /run/containerd/io.containerd.runtime.v2.task/k8s.io/nginx/rootfs/sys/kernel/security/lsm: no such file or directory: unknown

My hypothesis is that the fs under /sys is readonly in the container fs, and only /sys/kernel/security exists.

Failed attempt 2

So I tried something like

```yaml
apiVersion: v1
kind: Pod
metadata:
  name: lsm-pod
spec:
  containers:
    - name: nginx
      image: nginx
      volumeMounts:
        - name: lsm-volume
          mountPath: /sys/kernel/security/lsm
          subPath: lsm
          readOnly: true
  volumes:
    - name: lsm-volume
      hostPath:
        path: /sys/kernel/security
        type: Directory

But it's the same issue. Note that you can basically used the attempt 1 and attempt 2 if you want to mount the file anywhere else. It will work, the issue is with mounting under /sys/kernel/security/lsm.

Attempt 3

It seems the only way is to mount under this exact path is to mount the entire security directory.

apiVersion: v1
kind: Pod
metadata:
  name: lsm-pod
spec:
  containers:
    - name: nginx
      image: nginx
      volumeMounts:
        - name: lsm-volume
          mountPath: /sys/kernel/security
          readOnly: true
  volumes:
    - name: lsm-volume
      hostPath:
        path: /sys/kernel/security
        type: Directory

Conclusions

So, would the /sys/kernel/security folder at least exist (if empty) on most kernel, even those without most security modules enabled? That could be a first solution if exposing the entire directory is not a security issue.

If not, it's possible to only mount /sys/kernel/security/lsm but you will bump into situation where it doesn't exist (and thus the deployment will fail) and it seems you cannot mount it at the exact same path in the container fs, so you'll need to mount it elsewhere and teach tetragon where to look (flag/hardcoding/etc).

Kind config

For you to retry this on a linux machine, you'll need to mount an extra volume on the kind cluster, you can use this config:

apiVersion: kind.x-k8s.io/v1alpha4
kind: Cluster
nodes:
  - role: control-plane
    extraMounts:
      - hostPath: /sys/kernel/security
        containerPath: /sys/kernel/security

And then kind create cluster --config kind.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for such detailed explanation! I like you idea to mount the entire directory. Mounting file, we can come up to situation when pod will not start, because file doesn't exist. It's not good. The user might be confused.

And you are right, when we mount the entire directory we need to keep in mind that it is used to manage some LSM stuff (IMA, Apparmor, etc). From other side we will mount it in read-only mode and by default this is would be disabled. I need time to think on it a bit more).

Just a thing suddenly came up to my mind. Do we really need file check???
Please, have a look at the code below:

tetragon/pkg/bpf/detect.go

Lines 230 to 254 in bd433e6

func detectLSM() bool {
if features.HaveProgramType(ebpf.LSM) != nil {
return false
}
b, err := os.ReadFile("/sys/kernel/security/lsm")
if err != nil {
logger.GetLogger().WithError(err).Error("failed to read /sys/kernel/security/lsm")
return false
}
if strings.Contains(string(b), "bpf") {
prog, err := ebpf.NewProgram(&ebpf.ProgramSpec{
Name: "probe_lsm_file_open",
Type: ebpf.LSM,
Instructions: asm.Instructions{
asm.Mov.Imm(asm.R0, 0),
asm.Return(),
},
AttachTo: "file_open",
AttachType: ebpf.AttachLSMMac,
License: "Dual BSD/GPL",
})
if err != nil {
logger.GetLogger().WithError(err).Error("failed to load lsm probe")
return false
}

We have 3 type of checks there. The last one is the most important: we try to load a simple LSM program. Maybe it's an overkill to check the file?

Here we recommend to check if the LSM BPF is enabled in any case:

if !bpf.HasLSMPrograms() || !kernels.EnableLargeProgs() {
return nil, fmt.Errorf("Does you kernel support the bpf LSM? You can enable LSM BPF by modifying" +
"the GRUB configuration /etc/default/grub with GRUB_CMDLINE_LINUX=\"lsm=bpf\"")
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you added the file check ahah, what are we checking for? Can we just try to load the eBPF prog and give a helpful error message if it doesn't work? Maybe we can just use BPF progs to detect feature as we do in many places in cilium/ebpf

Copy link
Contributor Author

@anfedotoff anfedotoff Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahah, yeah you are right that was me)). This file check looks if LSM BPF is enabled on system or not. I though that it's a very cheep check. (At that time I couldn't imagine k8s problems)). Next check tries to load LSM program. This is the most reliable test. It covers the check above. And I think we can remove the file check at all. The last check would be enough.
I think we can add at the beginning of error message something like "Unable to load simple LSM BPF program." + current message. Because the most common case is that LSM BPF is not enabled.

This way can help us to avoid any security issues tight with mounting /sys/kernel/security.
I'll close this PR and open another one with appropriate fix for underlying problem. Thanks for helping).

@mtardy mtardy added the release-note/minor This PR introduces a minor user-visible change label Feb 17, 2025
@anfedotoff anfedotoff closed this Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem with applying LSM policies in k8s cluster
2 participants