k3s-io / k3s-selinux

SELinux policy for k3s
Apache License 2.0
69 stars 20 forks source link

Issues with selinux, unlabeled container fs files. #34

Open Timdawson264 opened 2 years ago

Timdawson264 commented 2 years ago

Issues:

On my system I seem to have the below from containers, however k3s-selinux seems to have <> for the snapshot files. This results in pods not starting, I manually added a rule to label the files as container_ro_file_t, and the issue was immediately resolved.

OS: Rocky Linux release 8.6 (using rhel8 rpm)

system containerd rules:
/var/lib/containerd(/.*)?                          all files          system_u:object_r:container_var_lib_t:s0 
/var/lib/containerd/[^/]*/sandboxes(/.*)?          all files          system_u:object_r:container_ro_file_t:s0 
/var/lib/containerd/[^/]*/snapshots(/.*)?          all files          system_u:object_r:container_ro_file_t:s0 

k3s-selinux rules
/var/lib/rancher/k3s/agent/containerd/[^/]*/sandboxes(/.*)? all files          system_u:object_r:container_ro_file_t:s0 
/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots directory          system_u:object_r:container_ro_file_t:s0 
/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots/[^/]* directory          system_u:object_r:container_ro_file_t:s0 
/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots/[^/]*/.* all files          <<None>>

Manual Rule:
/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots(/.*)? all files          system_u:object_r:container_ro_file_t:s0 

Ask: Label this rule. /var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots/[^/]*/.* all files <<None>>

vands56 commented 1 year ago

Hi,

want to bump this thread, because we had this issue on our k3s instances while upgrading selinux policy from 1.2 to 1.3 The weird thing is that only some nodes were affected.

OS: RHEL 8.8 Sympthoms: such log messages in audit log type=AVC msg=audit(1686836924.829:903): avc: denied { read } for pid=18301 comm="java" path="/lib/x86_64-linux-gnu/libz.so.1.2.11" dev="dm-0" ino=202766785 scontext=system_u:system_r:container_t:s0:c547,c785 tcontext=system_u:object_r:var_lib_t:s0 tclass=file permissive=0

Cause: after upgrade of policy for some reason wrong labels were placed on k3s files. It is var_lib_t instead of container_file_t

drwxr-xr-x. 7 root root system_u:object_r:var_lib_t:s0 84 Aug 23 2022 /var/lib/rancher/k3s/agent/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/114/fs/lib/

Temporary quick fix: chcon -Rv system_u:object_r:container_file_t:s0 /var/lib/rancher/k3s/agent/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/*

Long term fix: as tim mentioned above it would be nice to have default label for that, so simple auto-relabel will fix an issue

Klaas- commented 1 year ago

I've seen the behaviour @vands56 describes in one of my updates as well, but I couldn't reproduce it. It happened during the rhel 8.8 updates so it may be because of the issues observed in #47

I did upgrade other clusters, they seemed to work fine. But I do not not understand what the selinux context should be of that directory. In the rule the files should have "none" context, but when running k3s the files do have contexts set.

The set contexts seem to be a mixture of system_u:object_r:container_ro_file_t:s0, system_u:object_r:container_file_t:s0 and system_u:object_r:container_var_lib_t:s0

So I am guessing my main question is "if the selinux contexts get messed up, how do you properly get this aligned again?" restorecon would be the obvious answer but that would mean the files in there get a fixed selinux context. Any insights on this @rancher-max or @galal-hussein

galal-hussein commented 1 year ago

Thanks for opening the issue, as far as I can see, this line:

/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots/[^/]*/.* <<none>>

which means that the policy doesnt relabel the file system files within the container, however this line has not been changed in the newer update for the policy and been there since the initial commit, the new policy simply remove the file transition pattern to not conflict with recent changes of container-selinux.

@Timdawson264 if you are still seeing the same issue, can you provide more info on the error you seeing

@vands56 if you have an older system with the older policy can you post the labels you saw on /var/lib/rancher/k3s/agent/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/114/fs/lib/ before the upgrade.

galal-hussein commented 1 year ago

@Klaas- as for your question, yes the relabeling of the files should get the labels aligned again with the file contexts configured by the policy.

vands56 commented 1 year ago

@galal-hussein it was system_u:object_r:container_ro_file_t:s0before upgrade

Klaas- commented 1 year ago

@Klaas- as for your question, yes the relabeling of the files should get the labels aligned again with the file contexts configured by the policy.

You misunderstood my question :) The policy just says "no selinux file context" for files inside that dir. The current reality is that the files inside those dirs are a mixture of 3 contexts (at least in my systems): system_u:object_r:container_ro_file_t:s0, system_u:object_r:container_file_t:s0 and system_u:object_r:container_var_lib_t:s0

So it means k3s/containerd(/somthing else?) sets those contexts at the moment using some kind of logic on the fly. Where is that logic hiding? :) Are there objectively correct selinux contexts? So that we can put them into a selinux rule that we can put into k3s-selinux instead of the <<none>> line we have currently (which will then enable using restorecon/autorelabel to fix a broken state)

galal-hussein commented 1 year ago

@Klaas- yeah sorry about the misunderstanding, so k3s indeed adds logic to label these files, so the k3s policy has the following fc rules:

/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots           -d  gen_context(system_u:object_r:container_file_t,s0)
/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots/[^/]*     -d  gen_context(system_u:object_r:container_file_t,s0)
/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots/[^/]*/.*      <<none>>

The reason that you might end up with different contexts like container_ro_t is because of a recent update that happened to container-selinux https://github.com/containers/container-selinux/commit/8ca4b89b82846d126000601f6b8530a962f0ecd3#diff-d87cfd6c86daf9204f212619e52210d06d7575d9b4ad27b41966ab3e1b80e2a8R536 where they basically converted the container_ro_t to container_file_t, which caused a conflict with our policy, thats why we had to change the file contexts in some distros to align with container-selinux, to give you more contexts, the container_file_t is only needed when using buildkit, so it really doesnt cause a current problem with our policy.

The line:

/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots/[^/]*/.*      <<none>>

specifically has been there from the early start of the k3s-selinux policy, and I really don't know how its different now with the new policy, I am currently investigating this to see if that line is actually needed or not.

Klaas- commented 1 year ago

@galal-hussein okay I now understand where part of this problem comes from. Have you thought about a different approach for the containerd dirs? I think you could save yourself a few headaches if you just set equivalencies.

So as an example: Do an equivalency of /var/lib/containerd to /var/lib/rancher/k3s/agent/containerd and you no longer need to think about upstream changes, you'll just follow container-selinux "blindly"

Of cause this only works as long as you do not need to modify anything from upstream, but that seems to be the case here.

Timdawson264 commented 1 year ago

My fix was to add an new selinux rule to the system. I haven't had the issue since.

semanage fcontext -t container_ro_file_t -a '/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots(/.*)?'

Klaas- commented 1 year ago

My fix was to add an new selinux rule to the system. I haven't had the issue since.

semanage fcontext -t container_ro_file_t -a '/var/lib/rancher/k3s/agent/containerd/[^/]*/snapshots(/.*)?'

So I would say it should be container_file_t

https://github.com/containers/container-selinux/blob/e3d7d0b13381f00fb39181ea0bb3e329aeed4726/container.fc#L77

container_ro_file_t is for the sandboxes path:

https://github.com/containers/container-selinux/blob/e3d7d0b13381f00fb39181ea0bb3e329aeed4726/container.fc#L78

or this needs to be corrected upstream :) but as I said, I would guess for k3s the easiest thing is setting a equivalency /var/lib/rancher/k3s/agent/containerd to /var/lib/containerd and same for /var/run/k3s/containerd to /var/run/containerd

galal-hussein commented 1 year ago

@galal-hussein okay I now understand where part of this problem comes from. Have you thought about a different approach for the containerd dirs? I think you could save yourself a few headaches if you just set equivalencies.

So as an example: Do an equivalency of /var/lib/containerd to /var/lib/rancher/k3s/agent/containerd and you no longer need to think about upstream changes, you'll just follow container-selinux "blindly"

Of cause this only works as long as you do not need to modify anything from upstream, but that seems to be the case here.

This is interesting approach, I will have to test it but this can save us a lot of headache with recent changes to the containerd dirs, I will look into it, thanks!

Klaas- commented 1 year ago

In general I can just recommend to use equivalencies where possible, that way you do not need to create your own logic, this is especially useful if you just relocate something. this containerd directory behaves exactly like the standard containerd directory. This container data dir behaves exactly like the normal container data dir etc

Klaas- commented 2 months ago

@galal-hussein did you have time to think about this? :)