Add MCS label support

[ibuildthecloud]

This is a wholly untested PR atm. I'm just submitting it early to ensure that I'm not going about this completely wrong. @Random-Liu I'd love your feedback if this should be done in a different way. I very much want to get #1195 resolved.

[k8s-ci-robot]

Hi @ibuildthecloud. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

[ibuildthecloud]

@randomvariable let me know if you have any feedback, or if your actively working on this issue.

[mikebrow]

The build error (first error) is due to missing DCO.. commits have to be signed. git commit -s --amend.... then push backup :) Thx for the PR Darren.

[ibuildthecloud]

This PR should be fairly complete. I'm still testing it end to end.

The basic approach is that the process label is stored on the container.Metadata and sandbox.Metadata. When a sandbox is created or a container is create label.Store.Reserve is called to increment a usage counter. When a sandbox or container is removed from their respective stores label.Store.Release is call decrementing a counter. If the counter hits 0 the MCS label is released.

Also the behavior of how the labels are generated was changed slightly as it seemed to be more inline with cri-o. First, a container will inherit the sandbox process label if no selinux options are set on the container. If the user sets any selinux option, that option is just used to override the system option. Previously the was no concept of a system level option so the user had to supply all options to set a label. go-selinux will read the /etc/selinux files to determine what the default options are for a container so users settings just overlay on top. This means when running in SELinux mode all containers get a label unless they are set to Privileged. Also if the level option is not supplied a random one is chosen.

[ibuildthecloud]

So this appears to be working in my tests but I don't actually have this running with a SELinux enforcing, just permissive. The issue is coming up with the full selinux policy configuration to actually end to end test this. Which is a complete PITA.

[ibuildthecloud]

I believe volumes being used by container are not being relabeled. The volumes that are of immediate concern are secrets and config map volumes. I'll gladly fix but I don't actually know how the volumes like that work. Are they fully managed by the kubelet? If so, does the kubelet know to set the relabel flag on the volume? In containerd-cri are we actually relabeling volumes?

[mikebrow]

/ok-to-test
let's pull in the Kubernetes tests

[ibuildthecloud]

Does anyone know if the kubernetes e2e run in a context that has SELinux available? Right now the travis CI does not so no tests are actually being ran during travis CI. It would be great if the k8s CI ensured SELinux worked.

[mikebrow]

/test pull-cri-containerd-verify

[randomvariable]

@ibuildthecloud I don't think we have e2e tests in Kubernetes that fully exercise SELinux, partly why support's fallen through the cracks. There's a thread here with most of the details kubernetes/kubeadm#279

I'm OK with the approach, and haven't been actively working on it.

Sorry for the slow response, have been on PTO.

[cduchesne]

@ibuildthecloud i am looking to test this for functionality - any chance you can resolve conflicts?

[AkihiroSuda]

@ibuildthecloud What's current status?

[ibuildthecloud]

@AkihiroSuda I recently have renewed user interest in this. We will be taking up development on this PR again and getting it to work end to end in k3s.

[cjellick]

@ibuildthecloud did you tell me that someone ran/tested this PR and ran into issues? do you have a record of those issues or know who it was that ran into the problems?

[ibuildthecloud]

This PR needs containerd/containerd#4051 to fully work.

[crosbymichael]

If you all are too busy, I'm available to carry this PR if you need the help. Just let me know!

[mikebrow]

If you all are too busy, I'm available to carry this PR if you need the help. Just let me know!

Thx @crosbymichael would be a great help if you could carry this one.

[cjellick]

@crosbymichael - was that for the the reviewers or for the authors (Darren and Erik). @erikwilson is actively working on it.
Any interest in having a zoom call on this so that we can bring you up to speed with where we are?

[crosbymichael]

Ok, is there a time tomorrow that you all would like to sync? I’m on EDT time one.

[cjellick]

@crosbymichael thanks for the response! Sorry I couldnt get back to you yesterday. How about 3-4pm EDT tomorrow? I'm going to ping you on slack to help with the coordination.

[k8s-ci-robot]

@ibuildthecloud: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-cri-containerd-verify	307605f	link	/test pull-cri-containerd-verify
pull-cri-containerd-node-e2e	307605f	link	/test pull-cri-containerd-node-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[cjellick]

an issue that needs considered: k3s-io/k3s#1788
The user may be wrong in the label

[cjellick]

We synced with @crosbymichael and he's going to carry this PR forward to get it across the finish line and merged. Two open items to consider:

1. Consideration for error conditions as part of reference counting
2. Check whether the user the component of the labels is getting set and propagated properly (we think it could be causing SELinux blocking despite no mounts and proper ownership k3s-io/k3s#1788)

[crosbymichael]

One thing that is missing for an upstream change that we will have to do is change the policy:

---
 container.fc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/container.fc b/container.fc
index d646b9e..52b36a3 100644
--- a/container.fc
+++ b/container.fc
@@ -30,6 +30,7 @@
 /usr/sbin/ocid.*		--	gen_context(system_u:object_r:container_runtime_exec_t,s0)
 /usr/lib/docker/docker-novolume-plugin	--	gen_context(system_u:object_r:container_auth_exec_t,s0)
 /usr/lib/docker/[^/]*plugin	--	gen_context(system_u:object_r:container_runtime_exec_t,s0)
+/usr/local/bin/containerd(.*) --      gen_context(system_u:object_r:container_runtime_exec_t,s0)
+/usr/bin/containerd(.*) --      gen_context(system_u:object_r:container_runtime_exec_t,s0)
 
 /usr/lib/systemd/system/docker.*		--	gen_context(system_u:object_r:container_unit_file_t,s0)
 /usr/lib/systemd/system/lxd.*		--	gen_context(system_u:object_r:container_unit_file_t,s0)
@@ -38,6 +39,7 @@
 /etc/docker(/.*)?		gen_context(system_u:object_r:container_config_t,s0)
 /etc/docker-latest(/.*)?		gen_context(system_u:object_r:container_config_t,s0)
 /etc/crio(/.*)?		gen_context(system_u:object_r:container_config_t,s0)
+/etc/containerd(/.*)?          gen_context(system_u:object_r:container_config_t,s0)
 /exports(/.*)?		gen_context(system_u:object_r:container_var_lib_t,s0)
 
 /var/lib/registry(/.*)?	gen_context(system_u:object_r:container_var_lib_t,s0)
@@ -82,6 +84,10 @@
 /var/lib/docker-latest/overlay(/.*)?	gen_context(system_u:object_r:container_share_t,s0)
 /var/lib/docker-latest/overlay2(/.*)?	gen_context(system_u:object_r:container_share_t,s0)
 
+
+/var/lib/containerd(/.*)?	gen_context(system_u:object_r:container_var_lib_t,s0)
+/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots(/.*)?	 gen_context(system_u:object_r:container_share_t,s0)
+
 /var/run/containers(/.*)?		gen_context(system_u:object_r:container_var_run_t,s0)
 /var/run/crio(/.*)?		gen_context(system_u:object_r:container_var_run_t,s0)
 /var/run/docker(/.*)?		gen_context(system_u:object_r:container_var_run_t,s0)
-- 
2.26.0

I used these change to test on fedora 32

[erikwilson]

Also need to resolve this thread re merging pod's SELinux options (esp level) #1246 (comment)

[crosbymichael]

I opened #1487 to carry these changes to merge based on our discussions.

[AkihiroSuda]

Thanks Michael, can we close this one?

[crosbymichael]

Yes, we can close this one.