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
[release/1.5] seutil: Fix setting the "container_kvm_t" label #6381
[release/1.5] seutil: Fix setting the "container_kvm_t" label #6381
Conversation
The ability to handle KVM based runtimes with SELinux has been added as part of d715d00. However, that commit introduced some logic to check whether the "container_kvm_t" label would or not be present in the system, and while the intentions were good, there's two major issues with the approach: 1. Inspecting "/etc/selinux/targeted/contexts/customizable_types" is not the way to go, as it doesn't list the "container_kvm_t" at all. 2. There's no need to check for the label, as if the label is invalid an "Invalid Label" error will be returned and that's it. With those two in mind, let's simplify the logic behind setting the "container_kvm_t" label, removing all the unnecessary code. Here's an output of VMM process running, considering: * The state before this patch: ``` $ containerd --version containerd github.com/containerd/containerd v1.6.0-beta.3-88-g7fa44fc98 7fa44fc $ kubectl apply -f ~/simple-pod.yaml pod/nginx created $ ps -auxZ | grep cloud-hypervisor system_u:system_r:container_runtime_t:s0 root 609717 4.0 0.5 2987512 83588 ? Sl 08:32 0:00 /usr/bin/cloud-hypervisor --api-socket /run/vc/vm/be9d5cbabf440510d58d89fc8a8e77c27e96ddc99709ecaf5ab94c6b6b0d4c89/clh-api.sock ``` * The state after this patch: ``` $ containerd --version containerd github.com/containerd/containerd v1.6.0-beta.3-89-ga5f2113c9 a5f2113c9fc15b19b2c364caaedb99c22de4eb32 $ kubectl apply -f ~/simple-pod.yaml pod/nginx created $ ps -auxZ | grep cloud-hypervisor system_u:system_r:container_kvm_t:s0:c638,c999 root 614842 14.0 0.5 2987512 83228 ? Sl 08:40 0:00 /usr/bin/cloud-hypervisor --api-socket /run/vc/vm/f8ff838afdbe0a546f6995fe9b08e0956d0d0cdfe749705d7ce4618695baa68c/clh-api.sock ``` Note, the tests were performed using the following configuration snippet: ``` [plugins] [plugins.cri] enable_selinux = true [plugins.cri.containerd] [plugins.cri.containerd.runtimes] [plugins.cri.containerd.runtimes.kata] runtime_type = "io.containerd.kata.v2" privileged_without_host_devices = true ``` And using the following pod yaml: ``` apiVersion: v1 kind: Pod metadata: name: nginx spec: runtimeClassName: kata containers: - name: nginx image: nginx:1.14.2 ports: - containerPort: 80 ``` Fixes: containerd#6371 Backports: containerd#6372 Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com> (cherry picked from commit f1c7993)
Hi @fidencio. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
Build succeeded.
|
Thanks again @fidencio |
The ability to handle KVM based runtimes with SELinux has been added as
part of d715d00.
However, that commit introduced some logic to check whether the
"container_kvm_t" label would or not be present in the system, and while
the intentions were good, there's two major issues with the approach:
the way to go, as it doesn't list the "container_kvm_t" at all.
"Invalid Label" error will be returned and that's it.
With those two in mind, let's simplify the logic behind setting the
"container_kvm_t" label, removing all the unnecessary code.
Here's an output of VMM process running, considering:
The state before this patch:
The state after this patch:
Note, the tests were performed using the following configuration snippet:
And using the following pod yaml:
Fixes: #6371
Backports: #6372
Signed-off-by: Fabiano Fidêncio fabiano.fidencio@intel.com
(cherry picked from commit f1c7993)