Add container module by 0xC0ncord · Pull Request #434 · SELinuxProject/refpolicy

0xC0ncord · 2021-11-16T17:23:06Z

This adds a container module to refpolicy, bringing minimum viable support for container engines.

The svirt_lxc_net_t type was recycled into container_t in container-selinux. This PR will do the same. This PR tries to maintain best semantics and nomenclature compatibility with container-selinux where possible, with the biggest deviation being that container_runtime_t (container-selinux) is called container_engine_t in this policy.

Types were added for podman and docker as well as for rootless podman having its own type for users. The same goes for spc_t and spc_user_t.

I'm hoping that by the time this is merged it will encourage more contributions to the module in order to support a wider array of container engines, i.e. Docker and Kubernetes. So far I have been building this policy while using podman on a production server.

Some things to consider:

container_engine_t conflicts with a type of the same name in container-selinux (https://github.com/containers/container-selinux/blob/db7dcc5b8377405ec8d47a940ef78e19199bebea/container.te#L1252)
Rename container_engine_t to container_manager_t or something else (also helps with the above)
~~Split out some access from container_engine_domainintocontainer_engine_t, since most of the testing was done with only podmananddocker`~~ Done
~~Create separate types for podman and docker as there are some accesses shared by both that are not used by both~~ Done
~~Split out some access from container_t or container_domain to container_net_domain like container-selinux~~ Done
Also move svirt_lxc_t out of the virt module (probably won't do this since svirt_lxc_t is tied heavily to libvirt)

Fixes: #397
Relates-To: flatcar-linux/Flatcar/issues/479

pebenito

Many thanks for working on this! I didn't look too deeply at all of the interfaces, but the overall direction looks good.

policy/modules/kernel/domain.te

policy/modules/apps/gpg.if

0xC0ncord · 2021-12-03T21:01:41Z

@pebenito I think the policy is very close to being complete, but I want your opinion on this before moving forward: How do you feel about splitting the current implementation of the container_engine_t type?

In its current state, container_engine_t is a monolithic type which covers both podman and docker (like container-selinux). However after some discussion, I think it would be best to split this type into other private types, i.e. docker_t and podman_t as well as podman_rootless_t (names not final). This would allow us to ensure that different container engines only have the accesses they need to function, but also allows us to more strictly confine rootless podman.

I think overall this has a significant advantage over the current implementation, albeit at the cost of a little extra complexity.

pebenito · 2021-12-03T22:08:42Z

I'm fine with that. We could have a common set of rules between them, and then final docker_t, podman_t, etc.

0xC0ncord · 2021-12-09T16:57:12Z

I think after a long while this is finally ready. Please go ahead with the review @pebenito !

policy/modules/kernel/devices.if

policy/modules/kernel/filesystem.if

pebenito · 2021-12-14T22:01:16Z

BTW I'm reviewing and commenting in order of the diff github shows, since this will take some time.

policy/modules/kernel/filesystem.if

policy/modules/kernel/kernel.if

ghost

Just a random comment and heads-up

Are you differentiating between user containers and/or docker instances and system containers and/or docker instances?

If true then you may want to rethink, here is why:

Podman (and I guess Docker as well). by default labels "containers" as per what is specified in lxc_contexts.

For example the "process =" entry tells Podman (and I guess Docker) what label it should manually associate with container processes. (similar for "file", "init_process" etc)

This means in a nutshell that you can only have one default container label set. If you decide to implement more than one container label set (for example by differentiating between user and system containers) then eventually you will end up with issues.

Most of the defaults can be manually overriding on the command line with Podman (and I guess Docker) but not everything.

For example "Pod infra containers" are labeled with (I believe the context specified with "process =" in lxc_contexts and there is (I believe) no way to manually override that context on the Podman (and I guess Docker) command line.

So if you specify for example "proccess = system_u:system_r:system_container_t:s0" then "user pods" will (I believe)) and up with that context and there will be no way to override it (AFAIK)

One take away I have from looking into this matter is that:

"Walsh went too far trying to simplify it" and now "I have to go too far trying get stuff to work"

Granted that back when Docker and Podman were initially conceived there were no usable user name spaces and so it was probably never foreseen that there would eventually not only be a "system instances" but also "user instance(s)".

Now were essentially forced to treat them the same,, with all of the nasties associated with that.

policy/modules/kernel/storage.if

Signed-off-by: Kenton Groombridge <me@concord.sh>

systemd running inside containers needs to be able to manage cgroups. Add this feature behind a tunable. Signed-off-by: Kenton Groombridge <me@concord.sh>

Signed-off-by: Kenton Groombridge <me@concord.sh>

Make conmon run in a separate domain and allow podman types to transition to it. Signed-off-by: Kenton Groombridge <me@concord.sh>

Signed-off-by: Kenton Groombridge <me@concord.sh>

Add a type and allow execute access to executable files that may be freely managed by users in their home directories. Although users may normally execute anything labeled user_home_t, this type is intended to be executed by user services such as the user's systemd --user instance. Signed-off-by: Kenton Groombridge <me@concord.sh>

Signed-off-by: Kenton Groombridge <me@concord.sh>

Add an interface to allow systemd user daemons to use systemd notify and an interface to write to the systemd user runtime named socket. Signed-off-by: Kenton Groombridge <me@concord.sh>

Rootlesskit is required by rootless docker Signed-off-by: Kenton Groombridge <me@concord.sh>

Rootless docker runs as root in a user namespace. Because of this, rootless docker containers will run as spc_user_t as docker cannot be SELinux-aware in its own container. Signed-off-by: Kenton Groombridge <me@concord.sh>

Signed-off-by: Kenton Groombridge <me@concord.sh>

Found to be required by a jellyfin container when testing. Signed-off-by: Kenton Groombridge <me@concord.sh>

pebenito · 2022-01-24T19:02:02Z

Again, I really appreciate your effort on this. This is a massive contribution. Thanks!

0xC0ncord · 2022-01-24T19:02:57Z

Again, I really appreciate your effort on this. This is a massive contribution. Thanks!

Thank you for your support!

pebenito reviewed Nov 18, 2021

View reviewed changes

policy/modules/kernel/domain.te Outdated Show resolved Hide resolved

policy/modules/apps/gpg.if Outdated Show resolved Hide resolved

policy/modules/apps/gpg.if Outdated Show resolved Hide resolved

0xC0ncord force-pushed the containers branch from be6e84b to ee70787 Compare November 20, 2021 00:01

0xC0ncord force-pushed the containers branch 7 times, most recently from 961e125 to d17b732 Compare December 2, 2021 21:04

0xC0ncord force-pushed the containers branch 7 times, most recently from 42ab547 to 8ab0483 Compare December 9, 2021 16:48

0xC0ncord marked this pull request as ready for review December 9, 2021 16:57

0xC0ncord changed the title ~~[DRAFT] Add container module~~ Add container module Dec 9, 2021