Description of problem: I've spent hours chasing down a cockpit failure in RHEL 8.6 gating. Cockpit ships its own SELinux policy now, and the %post package script (autogenerated by SELinux RPM macros) does this at its core: if /usr/sbin/selinuxenabled 2>/dev/null; then /usr/sbin/semodule -n -s ${_policytype} -X 200 -i /usr/share/selinux/packages/targeted/cockpit.pp.bz2 /usr/sbin/selinuxenabled && /usr/sbin/load_policy || : fi The RHEL gating machinery uses virt-customize to upgrade/install packages, so these RPM operations don't run in a "real" environment, but the guestfs one. In that, the above %post script does not run. The root cause is that virt-customize/guestfs does not seem to realize that SELinux is enabled in the guest. Version-Release number of selected component (if applicable): guestfs-tools-1.47.2-2.fc35.x86_64 How reproducible: Always Steps to Reproduce: 1. Grab current CentOS 8 cloud guest image: curl -o c8.qcow2 https://cloud.centos.org/centos/8-stream/x86_64/images/CentOS-Stream-GenericCloud-8-20210603.0.x86_64.qcow2 2. Check if running it with virt-customize detects SELinux: virt-customize -a c8.qcow2 -v --run-command /usr/sbin/selinuxenabled Actual results: virt-customize: error: /usr/sbin/selinuxenabled: command exited with an error With -v verbose mode it also prints dozens of SELinux enabled state cached to: disabled Expected results: SELinux is recognized properly, `selinuxenabled` succeeds, and thus %post scripts run properly. Additional info: That %post script actually *could* run. I tested --run-command 'dnf install -y cockpit-ws; semodule -n -s targeted -X 200 -i /usr/share/selinux/packages/targeted/cockpit.pp.bz2' and that succeeds produces a working SELinux policy.
Adding Miroslav FYI, as this affects/breaks gating. Several packages have now adopted their own SELinux policy, these will break in a similar manner.
It (mis)behaves exactly the same with a Fedora 35 cloud image, so this does not seem (very) sensitive to the particular guest.
(In reply to Martin Pitt from comment #0) > > The RHEL gating machinery uses virt-customize to upgrade/install packages, > so these RPM operations don't run in a "real" environment, but the guestfs > one. In that, the above %post script does not run. Is this the first-time using virt-customize (and thus guestfs) to run these rpmbuild tasks? I'm trying to identify if we never really supported selinuxenabled in guestfs, or if this might be a regression. Do we have data points for this activity from previous versions? Also cc'ing Eric since Laszlo and Rich are out until Monday.
(In reply to Klaus Heinrich Kiwi from comment #4) > (In reply to Martin Pitt from comment #0) > > > > > The RHEL gating machinery uses virt-customize to upgrade/install packages, > > so these RPM operations don't run in a "real" environment, but the guestfs > > one. In that, the above %post script does not run. > > Is this the first-time using virt-customize (and thus guestfs) to run these > rpmbuild tasks? I'm trying to identify if we never really supported > selinuxenabled in guestfs, or if this might be a regression. Do we have data > points for this activity from previous versions? > > Also cc'ing Eric since Laszlo and Rich are out until Monday. Some more data: https://libguestfs.org/guestfs.3.html#selinux and https://libguestfs.org/guestfs.3.html#guestfs_get_selinux seems to indicate that selinux support on guestfs is "complicated", and getting it's status is apparently deprecated (and never worked right) https://libguestfs.org/guestfs.3.html#guestfs_set_selinux seems to indicate that the cached value can be changed if we pass 'selinux=1' to the guestfs, however, it will always be in permissive mode.. So two issues: 1) not sure if we should be explicitly setting this at the appliance level, after all, there might be cases where the image is not selinux enabled? 2) not sure if forcing the scripts to run (with selinux=1) will work, since selinux itself would be in permissive mode, and actually with no policy support at all (in case the script wants to run semanage, relabel etc)
https://libguestfs.org/guestfs.3.html#selinux states that libguestfs recognizes a guest that uses SELinux, but cannot directly enforce that policy in the appliance kernel (logically, this makes sense: the guest's labels are independent from the host, and therefore, the host cannot enforce the guest's labels, which in turn implies that the best we can do is run the guest with enforcing disabled and then touch up labels). [Hmm - that section mentions guestfs_setfiles, which does not exist, but also mentions that gestfs_setcon is deprecated, and _that_ function mentions guestfs_selinux_relabel. Bug in the docs?] (In reply to Martin Pitt from comment #0) > Description of problem: I've spent hours chasing down a cockpit failure in > RHEL 8.6 gating. Cockpit ships its own SELinux policy now, and the %post > package script (autogenerated by SELinux RPM macros) does this at its core: > > if /usr/sbin/selinuxenabled 2>/dev/null; then > /usr/sbin/semodule -n -s ${_policytype} -X 200 -i /usr/share/selinux/packages/targeted/cockpit.pp.bz2 > /usr/sbin/selinuxenabled && /usr/sbin/load_policy || : > fi Why is this %post checking /usr/sbin/selinuxenabled twice? It seems like the bug is in the %post... > Additional info: That %post script actually *could* run. I tested > > --run-command 'dnf install -y cockpit-ws; semodule -n -s targeted -X 200 > -i /usr/share/selinux/packages/targeted/cockpit.pp.bz2' > > and that succeeds produces a working SELinux policy. In other words, the step of running semodule does NOT care whether selinuxenabled is true - it is something you want to do merely if the selinuxenabled binary exists. The use of /usr/sbin/load_policy, on the other hand, is something that should only be done if you are in a real environment using SELinux. Shouldn't the %post be something more like: if [ -x /usr/sbin/selinuxenabled ]; then /usr/sbin/semodule ... /usr/sbin/selinuxenabled && /usr/sbin/load_policy || : fi that is, check merely whether the system is set up for SELinux, and not whether it is actually enabled, when it comes to installing the semodule?
> Is this the first-time using virt-customize (and thus guestfs) to run these rpmbuild tasks? No, I think the RHEL 8 gating tests have used this approach for a long time already. It just started to break now because cockpit 257 introduces its own SELinux policy in RHEL 8. I asked the SELinux team about this first whether this was kosher, but we all agreed that it was a good idea. If guestfs SELinux support is not really a thing, then perhaps RHEL 8 gating ought to stop using virt-customize, and install packages in an actually running VM instead? > Why is this %post checking /usr/sbin/selinuxenabled twice? It seems like the bug is in the %post... Looks odd for sure, but that doesn't seem to be the thing that actually breaks here. Cockpit's own %post does %if 0%{?with_selinux} if %{_sbindir}/selinuxenabled 2>/dev/null; then %selinux_modules_install -s %{selinuxtype} %{_datadir}/selinux/packages/%{selinuxtype}/%{name}.pp.bz2 %selinux_relabel_post -s %{selinuxtype} fi %endif I.e. the "inner" selinuxenabled call comes from the expansion of %selinux_relabel_post. > the step of running semodule does NOT care whether selinuxenabled is true - it is something you want to do merely if the selinuxenabled binary exists I am not of course happy to change this. Vit, Zdenek, does that make sense to you? That would certainly be the least intrusive way out. > selinux support on guestfs is "complicated", Yeah, I had a feeling that this would be the case -- conceptually, virt-customize is much closer to a chroot than an actually running VM, right? Thanks for your investigations!
> I am not of course happy to change this Sheesh, sorry for the typo. Of course I *am* happy to change this, and this in fact makes sense -- as Eric said above already, the `semodule` call can apparently work with SELinux disabled (it's "just" building a binary policy file from a source one, right?). I double-check this in https://src.osci.redhat.com/rpms/cockpit/pull-request/86
Works! I sent https://github.com/cockpit-project/cockpit/pull/16628 to fix the spec, and reassign this bug to cockpit. Thanks Eric and Klaus for your help!
Just clarifying a few things since I was on holiday last week: - libguestfs doesn't enable SELinux in the appliance, and cannot do because as Eric said above the host kernel policy is not compatible with whatever is happening the guest (which might be a completely different distro) - We can however relabel filesystems. This does not happen automatically, you must use the virt-customize --selinux-relabel option. (See also https://bugzilla.redhat.com/show_bug.cgi?id=1554735) - If adding --selinux-relabel doesn't fix things in this case then it could be a bug. - I don't understand what the proposed change to Cockpit is meant to do.
Sorry for the late reply. The %selinux_modules_install macro should not need to be enclosed in any check. Do we know why there was one (if /usr/sbin/selinuxenabled 2>/dev/null; then) -- i.e. was there any situation when it was deemed necessary (it's not in our guide, so not sure where it came from https://fedoraproject.org/wiki/SELinux/IndependentPolicy#Example_spec_file_changes_to_incorporate_-selinux_subpackage)?
FEDORA-2021-0f278d8cbd has been submitted as an update to Fedora 35. https://bodhi.fedoraproject.org/updates/FEDORA-2021-0f278d8cbd
Thanks Vit. No, we didn't have a situation where the extra check was necessary, it just crept in like this from some copy pasta. So our current fix was right, thanks for confirming!
FEDORA-2021-0f278d8cbd has been pushed to the Fedora 35 stable repository. If problem still persists, please make note of it in this bug report.