containers / container-selinux

SELinux policy files for Container Runtimes
GNU General Public License v2.0
257 stars 91 forks source link

Concern with use of dac_override in home_container.cil #262

Closed PeterWhittaker closed 1 year ago

PeterWhittaker commented 1 year ago

udica-templates/home_container.cil grants the container process the dac_override capability. Given how dac_override is considered a potential security hole to be avoided, this is a concern.

Why is this is a concern? As @wrabcak notes in Why do you see DAC_OVERRIDE SELinux denials?, In most cases [the use of dac_override] is a bug in the application package, and as @rhatdan notes in SELinux team works to remove DAC_OVERRIDE Permissions, In most cases the requirement for DAC_OVERRIDE is a simple programmer error in the way he sets up his application and can be fixed by adjusting the permissions/ownership on file system objects. Loosening the SELinux constraints should be the last resort and When I look at containers, we allow DAC_OVERRIDE by default, because so many containers are badly written, but I think it would be great for us to be able to remove this permission by default.

I think we should consider at least three alternatives, in this order:

  1. Remove the dac_override capability altogether;
  2. Make dac_override an optional policy controlled via boolean, default on, and document the use of the appropriate boolean in container documentation, with appropriate warnings; or
  3. Justify the use continued use of dac_override in repository documentation - personally, I don't think so many containers are badly written is a strong justification, least of all five years later.
rhatdan commented 1 year ago

We don't currently enforce SELinux controls in containers over Capabilities, the reason for this is that container engines have their own ability to add and remove capabilities from the container. SELinux policy management for this would be too cumbersome, so we allow other parts of container security to manage it.

We treat this similarly to network access.

PeterWhittaker commented 1 year ago

@rhatdan I'm concerned about that answer for several reasons, all of which revolve around the use of this repository by the community, whether directly via udica, e.g., or indirectly via browsing this repo to learn best - or at least accepted - practices:

  1. The default SELinux policies generated by udica include dac_override, which I think we all agree is to be avoided unless absolutely necessary - people attempting to do the right thing for their containers are inadvertently introducing a potential security exposure;
  2. Making dac_override an optional policy to be enabled via boolean is not onerous, and forcing container developers/maintainers to set that boolean so at least they acknowledge the potential security hole is not (IMHO, at least) cumbersome; and
  3. If container developers/containers choose to not use udica but to develop their own policies, they may well (and I would argue are likely) to visit this repository to learn what the pros are doing - seeing dac_override baked in leaves a poor impression (one that could be interpreted in two divergent ways: "if the pros say so, it must be good" and "oh, what else did they get wrong?").

You've made the decision to close the issue, essentially as a "won't fix", and I'll respect that decision and leave this closed, I just think we are doing the wrong thing by leaving things as they are.

rhatdan commented 1 year ago

udica can do what it wants. The point being that if I do

podman run --cap-add dac-overide ...

Then there is no simple way to change the SELinux type to allow dac_override.

If a user runs docker run --cap-drop=dac_override then you will get the same thing you want, which is the kernel blocking access to dac_override.

With container policy, we are looking to limit the containers access to file systems on a MAC basis.

DAC protections in containers are provided via the capability handling along with User Namespaces.
If SELinux blocked the access then we would need to have different types for each capabilty or combination of capabilities. Which would end up with 32! different types, just for caps. Similarly we would need controlls for network stack.

Since other parts of the kernel support controlling these in a far more flexible manner then SELinux, we rely on those mechanisms to contol.

Writing general purpose policy to be used to control containers forces us to make Goldilocks compromises. Udica should be confined by default, But container-selinux needs to allow for the customization at the container engine.