Bug 2223276

Summary: swtpm should not have a hard dependency on selinux
Product: [Fedora] Fedora Reporter: Daan De Meyer <daan.j.demeyer>
Component: swtpmAssignee: Stefan Berger <stefanb>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: awilliam, davide, kevin, marcandre.lureau, stefanb
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Linux   
Whiteboard:
Fixed In Version: swtpm-0.8.0-5.fc38 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2023-07-28 02:41:21 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Daan De Meyer 2023-07-17 08:26:23 UTC
Currently, the swtpm spec has an unconditional requires on selinux. This should not be required. swtpm should install its selinux policy module if selinux is installed on the system but not pull in selinux itself since swtpm can operate perfectly fine without having selinux installed on the system.

Reproducible: Always

Comment 1 Stefan Berger 2023-07-17 18:22:40 UTC
What is the proper solution for this? Are there supposed to be tests whether SELinux is enabled? Do you have an example of another package to see how the solved it? 

When I try to remove selinux-policy-targeted on my system then I see the following dependencies will be removed:

Removing:
 selinux-policy-targeted                            noarch             37.22-1.fc37                                @updates              18 M
Removing dependent packages:
 container-selinux                                  noarch             2:2.219.0-1.fc37                            @updates              67 k
 flatpak-selinux                                    noarch             1.14.4-1.fc37                               @updates              12 k
 gnome-boxes                                        x86_64             43.5-1.fc37                                 @updates             6.1 M
 libvirt                                            x86_64             8.6.0-5.fc37                                @updates               0
 libvirt-daemon-kvm                                 x86_64             8.6.0-5.fc37                                @updates               0
 moby-engine                                        x86_64             20.10.23-1.fc37                             @updates             116 M
 openwsman-selinux                                  noarch             2.7.1-7.fc37                                @fedora               14 k
 passt-selinux                                      noarch             0^20230625.g32660ce-1.fc37                  @updates             188 k
 selinux-policy                                     noarch             37.22-1.fc37                                @updates              25 k
 selinux-policy-devel                               noarch             37.22-1.fc37                                @updates              14 M
 smartmontools-selinux                              noarch             1:7.3-3.fc37                                @fedora               13 k
 swtpm                                              x86_64             0.7.3-2.20220427gitf2268ee.fc37             @fedora              217 k
 swtpm-tools                                        x86_64             0.7.3-2.20220427gitf2268ee.fc37             @fedora              278 k
 tigervnc-selinux                                   noarch             1.13.1-3.fc37                               @updates              14 k
 tpm2-abrmd-selinux                                 noarch             2.3.1-6.fc37                                @fedora               12 k
Removing unused dependencies:
 containerd                                         x86_64             1.6.19-1.fc37                               @updates             135 M
 corosynclib                                        x86_64             3.1.7-1.fc37                                @updates             159 k
 genisoimage                                        x86_64             1.1.11-50.fc37                              @fedora              1.1 M
 glusterfs                                          x86_64             10.4-1.fc37                                 @updates             2.7 M
 glusterfs-cli                                      x86_64             10.4-1.fc37                                 @updates             482 k
 glusterfs-fuse                                     x86_64             10.4-1.fc37                                 @updates             556 k
 libglusterd0                                       x86_64             10.4-1.fc37                                 @updates             8.7 k
 libqb                                              x86_64             2.0.6-3.fc37                                @fedora              212 k
 libusal                                            x86_64             1.1.11-50.fc37                              @fedora              469 k
 libvirt-daemon-config-nwfilter                     x86_64             8.6.0-5.fc37                                @updates              20 k
 libvirt-daemon-driver-interface                    x86_64             8.6.0-5.fc37                                @updates             606 k
 libvirt-daemon-driver-libxl                        x86_64             8.6.0-5.fc37                                @updates             893 k
 libvirt-daemon-driver-lxc                          x86_64             8.6.0-5.fc37                                @updates             985 k
 libvirt-daemon-driver-nodedev                      x86_64             8.6.0-5.fc37                                @updates             671 k
 libvirt-daemon-driver-nwfilter                     x86_64             8.6.0-5.fc37                                @updates             701 k
 libvirt-daemon-driver-qemu                         x86_64             8.6.0-5.fc37                                @updates             2.7 M
 libvirt-daemon-driver-secret                       x86_64             8.6.0-5.fc37                                @updates             598 k
 libvirt-daemon-driver-storage                      x86_64             8.6.0-5.fc37                                @updates               0
 libvirt-daemon-driver-storage-disk                 x86_64             8.6.0-5.fc37                                @updates              32 k
 libvirt-daemon-driver-storage-gluster              x86_64             8.6.0-5.fc37                                @updates              40 k
 libvirt-daemon-driver-storage-iscsi                x86_64             8.6.0-5.fc37                                @updates              24 k
 libvirt-daemon-driver-storage-logical              x86_64             8.6.0-5.fc37                                @updates              32 k
 libvirt-daemon-driver-storage-mpath                x86_64             8.6.0-5.fc37                                @updates              16 k
 libvirt-daemon-driver-storage-rbd                  x86_64             8.6.0-5.fc37                                @updates              44 k
 libvirt-daemon-driver-storage-scsi                 x86_64             8.6.0-5.fc37                                @updates              24 k
 libvirt-daemon-driver-storage-sheepdog             x86_64             8.6.0-5.fc37                                @updates              20 k
 libvirt-daemon-driver-storage-zfs                  x86_64             8.6.0-5.fc37                                @updates              24 k
 libvirt-daemon-driver-vbox                         x86_64             8.6.0-5.fc37                                @updates             856 k
 libvirt-gconfig                                    x86_64             4.0.0-6.fc37                                @fedora              393 k
 libvirt-gobject                                    x86_64             4.0.0-6.fc37                                @fedora              236 k
 lzop                                               x86_64             1.04-9.fc37                                 @fedora              107 k
 policycoreutils-devel                              x86_64             3.5-1.fc37                                  @updates             331 k
 qemu-kvm                                           x86_64             2:7.0.0-15.fc37                             @updates               0
 rpm-plugin-selinux                                 x86_64             4.18.1-2.fc37                               @updates              16 k
 sheepdog                                           x86_64             1.0.1-18.fc37                               @fedora              736 k
 systemd-container                                  x86_64             251.14-2.fc37                               @updates             1.3 M
 trousers                                           x86_64             0.3.15-7.fc37                               @fedora              362 k
 zfs-fuse                                           x86_64             0.7.2.2-28.fc37     


So do we need a package swtpm-selinux?

Comment 2 Daan De Meyer 2023-07-18 10:44:29 UTC
Yes, the selinux module should be shipped in a package swtpm-selinux and swtpm should have a conditional requires on swtpm-selinux if selinux-policy is installed or something that vein. The idea is that swtpm-selinux only gets installed if the selinux-policy package is installed.

Comment 3 Stefan Berger 2023-07-18 13:34:55 UTC
(In reply to Daan De Meyer from comment #2)
> Yes, the selinux module should be shipped in a package swtpm-selinux and
> swtpm should have a conditional requires on swtpm-selinux if selinux-policy
> is installed or something that vein. The idea is that swtpm-selinux only
> gets installed if the selinux-policy package is installed.

I am aware of static conditional requires (like the one below) but they won't help here since we would need something more 'dynamic' that depends on what is installed on the current system:

%if %{undefined rhel}	
BuildRequires:  trousers >= 0.3.9
%endif


Also, we don't have an explicit Requires on the selinux-policy and selinux-policy-base packages but they seem to get pulled in due to the usage of macros

%post
for pp in /usr/share/selinux/packages/swtpm.pp \	
          /usr/share/selinux/packages/swtpm_svirt.pp; do
  %selinux_modules_install -s %{selinuxtype} ${pp}
done


$ rpm -q -R swtpm
...
selinux-policy >= 38.6-1.fc38
selinux-policy-base >= 38.6-1.fc38
swtpm-libs = 0.8.0-3.fc38


So I guess the 1st step for a solution would be to make selinux-policy a suggested package:

@@ -50,6 +50,7 @@ Requires:       libtpms >= 0.6.0
 %if ! 0%{?flatpak}
 %{?selinux_requires}
 %endif
+Suggests:      selinux-policy

 %description
 TPM emulator built on libtpms providing TPM functionality for QEMU VMs


This does seem to work:

$ rpm -qp --suggests ~/rpmbuild/RPMS/x86_64/swtpm-0.8.0-3000.fc38.x86_64.rpm
selinux-policy


However, I am not sure whether the RPM SELinux macros we are using are prepared for a missing selinux-policy and its dependencies:

https://src.fedoraproject.org/rpms/swtpm/blob/rawhide/f/swtpm.spec

%post
for pp in /usr/share/selinux/packages/swtpm.pp \
          /usr/share/selinux/packages/swtpm_svirt.pp; do
  %selinux_modules_install -s %{selinuxtype} ${pp}
done	
restorecon %{_bindir}/swtpm


restorecon would likely not be installed and therefore fail.
The macro looks like this.

# %selinux_modules_install [-s <policytype>] [-p <modulepriority>] module [module]...
%selinux_modules_install("s:p:") \
if [ -e /etc/selinux/config ]; then \
  . /etc/selinux/config \
fi \
_policytype=%{-s*} \
if [ -z "${_policytype}" ]; then \
  _policytype="targeted" \
fi \
if [ "${SELINUXTYPE}" = "${_policytype}" ]; then \
  %{_sbindir}/semodule -n -s ${_policytype} -X %{!-p:200}%{-p*} -i %* || : \
  %{_sbindir}/selinuxenabled && %{_sbindir}/load_policy || : \
fi \
%{nil}

Where does it pull SELINUXTYPE from? Obviously we must not fail calling semodule.

So, I am not sure whether these macros are safe for this condition and whether this is the proper way to do this. Also, would these post installation scripts need to run when an SElinux policy was to be installed later on?

Comment 4 Daan De Meyer 2023-07-19 06:57:15 UTC
I found https://fedoraproject.org/wiki/SELinuxModularityPackagingGuidance which links to an example spec file showing how to do policy as a subpackage: https://src.fedoraproject.org/rpms/memcached//blob/rawhide/f/memcached.spec. From looking at that spec file, it seems the macros can handle if the selinux-policy package is not installed. Can you do something similar to what the memcached package is doing for selinux?

Comment 5 Stefan Berger 2023-07-19 12:13:23 UTC
(In reply to Daan De Meyer from comment #4)
> I found https://fedoraproject.org/wiki/SELinuxModularityPackagingGuidance
> which links to an example spec file showing how to do policy as a
> subpackage:
> https://src.fedoraproject.org/rpms/memcached//blob/rawhide/f/memcached.spec.
> From looking at that spec file, it seems the macros can handle if the
> selinux-policy package is not installed. Can you do something similar to
> what the memcached package is doing for selinux?

Can this potential swtpm-selinux package be dynamically installed when SELinux is found to be enabled and not installed when there's no SELinux? Or do all users that so far just installed swtpm have to manually install swtpm-selinux then to get the SELinux labels?

Comment 6 Stefan Berger 2023-07-19 12:25:11 UTC
(In reply to Stefan Berger from comment #5)
> (In reply to Daan De Meyer from comment #4)
> > I found https://fedoraproject.org/wiki/SELinuxModularityPackagingGuidance
> > which links to an example spec file showing how to do policy as a
> > subpackage:
> > https://src.fedoraproject.org/rpms/memcached//blob/rawhide/f/memcached.spec.
> > From looking at that spec file, it seems the macros can handle if the
> > selinux-policy package is not installed. Can you do something similar to
> > what the memcached package is doing for selinux?
> 
> Can this potential swtpm-selinux package be dynamically installed when
> SELinux is found to be enabled and not installed when there's no SELinux? Or
> do all users that so far just installed swtpm have to manually install
> swtpm-selinux then to get the SELinux labels?

Probably with this here ...

Requires: (%{name}-selinux if selinux-policy-targeted)

Comment 7 Stefan Berger 2023-07-19 13:48:21 UTC
I split off the SELinux policy into swtpm-selinux : https://src.fedoraproject.org/rpms/swtpm/c/f662e81fe35dfce1617330459b383c06fe03df6e?branch=rawhide

I built it for rawhide for now: https://koji.fedoraproject.org/koji/taskinfo?taskID=103557263

Comment 8 Kevin Fenzi 2023-07-20 15:11:28 UTC
So, this broke the rawhide compose today: 

https://pagure.io/releng/failed-composes/issue/5196

The problem is that it tried to install and run the scriptlets for swtpm-selinux, but the ordering was such that swtpm wasn't itself installed yet. 
You need a Requires(pre) or the like on swtpm.

I have untagged the existing build to get composes working again.

Comment 9 Stefan Berger 2023-07-21 21:13:37 UTC
(In reply to Kevin Fenzi from comment #8)
> So, this broke the rawhide compose today: 
> 
> https://pagure.io/releng/failed-composes/issue/5196
> 
> The problem is that it tried to install and run the scriptlets for
> swtpm-selinux, but the ordering was such that swtpm wasn't itself installed
> yet. 
> You need a Requires(pre) or the like on swtpm.
> 
> I have untagged the existing build to get composes working again.

I think a Require in swtpm-selinux on swtpm should solve the problem. I built a version like this now.

Comment 10 Adam Williamson 2023-07-22 16:40:22 UTC
Nope, that broke it again:

https://kojipkgs.fedoraproject.org//work/tasks/9881/103739881/livemedia-out.log

You can see in that log that we get an error running swtpm-selinux's %post script, and at that point, swtpm has not been installed.

If you just do Requires, libsolv doesn't treat that as a strict ordering requirement - if A just "Requires" B, B doesn't *have* to be installed before A. Depending on other ordering considerations, B might come after A.

In this case swtpm needs to be installed before swtpm-selinux's %post script runs, so we need swtpm-selinux to do:

Requires(post): swtpm

Comment 11 Adam Williamson 2023-07-22 16:48:33 UTC
I've fixed this myself, so we can try a new Rawhide compose - https://src.fedoraproject.org/rpms/swtpm/c/645aeb8117492e09b57da52d52c270375b909ff4?branch=rawhide

Comment 12 Fedora Update System 2023-07-23 13:33:50 UTC
FEDORA-2023-247b8594c4 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-247b8594c4

Comment 13 Adam Williamson 2023-07-23 16:46:22 UTC
You made a mistake in backporting this to F38: you left out the `%files selinux`, so the selinux subpackage is not actually generated - note there's no swtpm-selinux at https://koji.fedoraproject.org/koji/buildinfo?buildID=2259257 .

Comment 14 Adam Williamson 2023-07-23 22:40:17 UTC
I've run a build that should fix that, will edit it into the f38 update.

Comment 15 Stefan Berger 2023-07-24 00:04:49 UTC
(In reply to Adam Williamson from comment #14)
> I've run a build that should fix that, will edit it into the f38 update.

I had done a diff with the latest swtpm.spec from Rawhide and didn't see the missing '%files selinux'... Thanks for covering...

Comment 16 Fedora Update System 2023-07-24 01:37:48 UTC
FEDORA-2023-247b8594c4 has been pushed to the Fedora 38 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-247b8594c4`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-247b8594c4

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 17 Fedora Update System 2023-07-28 02:41:21 UTC
FEDORA-2023-247b8594c4 has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.