On Thu, Apr 8, 2021 at 2:17 PM Kai Lüke <kai@xxxxxxxxxx> wrote: > > Hello, > > to detect if SELinux is disabled, in version 2.4 there was a check for > /proc/PID/attr/current to have something else than "kernel" as value. > This allowed to distinguish between "Disabled" and "Permissive" when > the filesystem is mounted and the /enforce file has the value 0. > > That check got removed in later versions and a check was added based > on whether /etc/selinux/config exists. These were commits 685f4aeeadc0b60f3770404d4f149610d656e3c8 and c08c4eacab8d55598b9e5caaef8a871a7a476cab respectively; the commit descriptions provide the rationale for the changes and the associated bugs. > This leads to two problems. The first one is that older versions which > have SELinux disabled in the config file still have the filesystem > mounted (unless selinux=0 is passed as kernel param) which causes the > newer behavior to think SELinux is "Permissive" instead of disabled. That shouldn't be the case; selinuxfs is unmounted by selinux_init_load_policy() if SELinux is disabled in the config file too. > The second problem is that the existence of the config file is only > loosely related to whether SELinux is disabled or not. On one hand a > recent change of the config file (creation/removal) is not valid now > but only after it got applied, e.g., by a reboot, on the second hand > the check does not work from containers.¹ It is true that the existence of the config file is a weak indicator; that check was only added to address a particular bug where systems had no /etc/selinux/config and expected SELinux to be treated as disabled. With respect to containers, the Fedora / Red Hat approach in general has always been to treat SELinux as disabled within the container so that userspace within the container doesn't try to load policy or set labels. The entire container is run with a single security context. > I suggest to drop the config file check and replace it by a > /proc/PID/attr/current check which is more reliable because it tells > something about the current state and works from containers. > What do you think? This would essentially be a revert of the prior two commits which would seem to re-introduce the original issue that was being fixed.
