Due to a recent update on Javascript code a full page refresh on your browser might be needed.
Bug 1723940 - selinux-policy-targeted needs stronger requires on selinux-policy (and way too much magic in scriplets)
Summary: selinux-policy-targeted needs stronger requires on selinux-policy (and way to...
Alias: None
Product: Fedora
Classification: Fedora
Component: selinux-policy
Version: 30
Hardware: Unspecified
OS: Unspecified
Target Milestone: ---
Assignee: Zdenek Pytela
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2019-06-25 19:36 UTC by Terje Røsten
Modified: 2020-01-23 16:24 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Last Closed:
Type: Bug

Attachments (Terms of Use)
Protect sourcing of /etc/selinux/config (5.26 KB, patch)
2019-07-04 18:43 UTC, Terje Røsten
no flags Details | Diff

Description Terje Røsten 2019-06-25 19:36:54 UTC
Description of problem:

via macro expand in spec file postinstall scriptlet (using /bin/sh):

ends up like:

postinstall scriptlet (using /bin/sh):

. /etc/selinux/config; 
if [ -e /etc/selinux/targeted/.rebuild ]; then 
   rm /etc/selinux/targeted/.rebuild; 
   /usr/sbin/semodule -B -n -s targeted; 
[ "${SELINUXTYPE}" == "targeted" ] && selinuxenabled && load_policy; 
if [ $1 -eq 1 ]; then 
   /sbin/restorecon -R /root /var/log /run /etc/passwd* /etc/group* /etc/*shadow* 2> /dev/null; 

. /etc/selinux/config; 
if [ $? = 0  -a "${SELINUXTYPE}" = targeted -a -f ${FILE_CONTEXT}.pre ]; then 
     /sbin/fixfiles -C ${FILE_CONTEXT}.pre restore &> /dev/null > /dev/null; 
     rm -f ${FILE_CONTEXT}.pre; 
if /sbin/restorecon -e /run/media -R /root /var/log /var/run /etc/passwd* /etc/group* /etc/*shadow* 2> /dev/null;then 
exit 0

This seems harmless and good, however /etc/selinux/config is shipped in
selinux-policy, and selinux-policy-targeted has only:

Requires: selinux-policy = %{version}-%{release}

this must be changed to:

Requires(post): selinux-policy = %{version}-%{release}

such that selinux-policy is installed before  selinux-policy-targeted (that's might be the case with a plain Requires).

I tracked down similar problem in flatpak-selinux which used the 
%selinux_modules_install  macro in post scripts (includes . /etc/selinux/config too), however had only Requires: selinux-policy. 

See: https://bugzilla.redhat.com/show_bug.cgi?id=1723118

Comment 1 Terje Røsten 2019-06-26 18:01:39 UTC
Wow! The problem is deeper. 

For some horrible reason the most important config file in the area in SELinux: /etc/selinux/config
it's not shipped as a regular file in the package, it's created by the %post script of selinux-policy.

This is not a sane design.

For starters it's not possible to have strict order of %post scripts in RPM, you can't say:
 run %post script for package A before %post script for package B in a package install transaction.

Current design is in other words racy and can't be fixed by whatever RPM Requires/Requires(foo) statements.

One solution is to add wrap all "source /etc/selinux/config" statements like this:

if [ -e /etc/selinux/config ]; then
  source /etc/selinux/config

ugly, however current design have no warranty /etc/selinux/config exists, hence all scripts
must work without its presence.

Comment 2 Terje Røsten 2019-07-02 06:11:11 UTC
See also:

Comment 3 Terje Røsten 2019-07-04 18:43:48 UTC
Created attachment 1587456 [details]
Protect sourcing of /etc/selinux/config

    /etc/selinux/config might be missing[1]: protect all scripts for this
    Protection is required as a failure in scriptlet will cause havoc in a
    (possibly) large package set install or upgrade transactions.
    [1]: /etc/selinux/config is not shipped as a normal file, it's created
    by selinux-policy %post scriptlet. There no way for other packages to
    make sure their post scriptlets is run before selinux-policy post
    scriptlets is done executing. If selinux-policy is large and slow,
    creation of required file will be after other packages scripts needs
    it. With current design it's not possible to handle race condition
    without protection added here.
    "Solution" here is a just stop gap to reduce impact.

Comment 4 Terje Røsten 2019-08-22 17:18:01 UTC

I still hit this bug when installing Fedora 30 via kickstart. 

Can you at least merge the attached patch to reduce impact of the problem?

Comment 5 Terje Røsten 2019-09-16 16:17:58 UTC

Comment 7 Terje Røsten 2019-10-28 19:32:00 UTC
Ok, so history repeats itself. Now I can't install Fedora 31 due to this problem, install fails

  DNF error: Error in POSTIN scriplet in rpm package flatpak-selinux


Is selinux-policy maintained in Fedora these days or should users avoid SELinux completely?

Comment 8 Terje Røsten 2019-10-28 19:41:13 UTC
In packaging.log in the failed install I see:

 20:21:35,246 ERR dnf.rpm: Error in POSTIN scriptlet in rpm package flatpak-selinux
 20:21:35,674 ERR dnf.rpm: Error in POSTIN scriptlet in rpm package selinux-policy-targeted

Comment 9 Ben Cotton 2019-10-31 18:43:41 UTC
This message is a reminder that Fedora 29 is nearing its end of life.
Fedora will stop maintaining and issuing updates for Fedora 29 on 2019-11-26.
It is Fedora's policy to close all bug reports from releases that are no longer
maintained. At that time this bug will be closed as EOL if it remains open with a
Fedora 'version' of '29'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 29 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 10 Terje Røsten 2019-10-31 18:58:05 UTC
Sorry for the rant. Updated PR against master:

Tested on Fedora 31, works fine here.

Comment 11 Lukas Vrabec 2019-11-01 12:02:28 UTC
Could you please create PR here: 
https://github.com/fedora-selinux/selinux-policy-macros ? 

We're trying to keep it in separate repository. 

Thank you.

Comment 12 Orion Poplawski 2019-11-02 15:06:39 UTC
So, to correct something:

- the selinux-policy-base providers (-targeted, -mls, and -minimum) have:

Requires(pre): selinux-policy = %{version}-%{release}
         NOT post

flatpak-selinux incorrectly has (PR in progess https://src.fedoraproject.org/rpms/flatpak/pull-request/5):

Requries(post): selinux-policy

instead of

Requires(post): selinux-policy-base

 I'm still not entirely convinced that that creates a contract that selinux-policy's %post script will be run before the flatpak-selinux's %post script, but hopefully in practice it won't matter.

Comment 13 Lukas Vrabec 2019-11-03 14:04:35 UTC
Hi Orion, 

PR merged, Thank you for flatpak issue please let's continue in different ticket. 


Comment 14 Panu Matilainen 2019-11-04 09:35:29 UTC
Um, guys. Looking at the package dependencies:
[root@lumikko-w ~]# rpm -qv --requires selinux-policy-targeted|grep selinux-policy
config: config(selinux-policy-targeted) = 3.14.4-39.fc31
manual: selinux-policy = 3.14.4-39.fc31
pre: selinux-policy = 3.14.4-39.fc31

It's all there already.

If selinux-policy-targeted is getting installed before selinux-policy despite this, there's a dependency loop tangle that needs unravelling, fiddling with pre/post does not make any difference.

Comment 15 Ian Donaldson 2019-12-05 06:04:43 UTC
Somebody should update version to 31.. (can't see how)

Comment 16 Fedora Admin XMLRPC Client 2020-01-23 16:24:14 UTC
This package has changed maintainer in the Fedora.
Reassigning to the new maintainer of this component.

Note You need to log in before you can comment on or make changes to this bug.