rpms / selinux-policy

Clone
Source Code
GIT

#15 Add check for config file consistency
Merged 4 years ago by lvrabec. Opened 4 years ago by vmojzis.
rpms/ vmojzis/selinux-policy config  into  master

file modified
+62 
@@ -337,6 +337,47 @@ 
 

  #install -m 644 -p %{SOURCE101} %{buildroot}/%{_unitdir}/ \
 

  #ln -s ../selinux-factory-reset@.service %{buildroot}/%{_unitdir}/basic.target.wants/selinux-factory-reset@%1.service
 

  

+ # Make sure the config is consistent with what packages are installed in the system
 

+ # this covers cases when system is installed with selinux-policy-{mls,minimal}
 

+ # or selinux-policy-{targeted,mls,minimal} where switched but the machine has not
 

+ # been rebooted yet.
 

+ # The macro should be called at the beginning of "post" (to make sure load_policy does not fail)
 

+ # and in "posttrans" (to make sure that the store is consistent when all package transitions are done)
 

+ # Parameter determines the policy type to be set in case of miss-configuration (if backup value is not usable)
 

+ # Steps:
 

+ # * load values from config and its backup
 

+ # * check whether SELINUXTYPE from backup is usable and make sure that it's set in the config if so
 

+ # * use "targeted" if it's being installed and BACKUP_SELINUXTYPE cannot be used
 

+ # * check whether SELINUXTYPE in the config is usable and change it to newly installed policy if it isn't
 

+ 

+ %define checkConfigConsistency() \
 

+ . %{_sysconfdir}/selinux/config; \
 

+ if [ -f %{_sysconfdir}/selinux/.config_backup ]; then \
 

+      . %{_sysconfdir}/selinux/.config_backup; \
 

+ else \
 

+      BACKUP_SELINUXTYPE=targeted; \
 

+ fi; \
 

+ if ls %{_sysconfdir}/selinux/$BACKUP_SELINUXTYPE/policy/policy.* &>/dev/null; then \
 

+      if [ "$BACKUP_SELINUXTYPE" != "$SELINUXTYPE" ]; then \
 

+           sed -i 's/^SELINUXTYPE=.*/SELINUXTYPE=$BACKUP_SELINUXTYPE/g' %{_sysconfdir}/selinux/config; \
 

+      fi; \
 

+ elif [ "%1" = "targeted" ]; then \
 

+      sed -i 's/^SELINUXTYPE=.*/SELINUXTYPE=%1/g' %{_sysconfdir}/selinux/config; \
 

+ elif ! ls  %{_sysconfdir}/selinux/$SELINUXTYPE/policy/policy.* &>/dev/null; then \
 

+      sed -i 's/^SELINUXTYPE=.*/SELINUXTYPE=%1/g' %{_sysconfdir}/selinux/config; \
 

+ fi;
 

+ 

+ 

+ # Create hidden backup of /etc/selinux/config and prepend BACKUP_ to names
 

+ # of variables inside so that they are easy to use later
 

+ # Should only be used in "pretrans" -- config content can change during RPM operations
 

+ %define backupConfig() \
 

+ %{__rm} -f %{_sysconfdir}/selinux/.config_backup \
 

+ if [ -f %{_sysconfdir}/selinux/config ]; then \
 

+      cp %{_sysconfdir}/selinux/config %{_sysconfdir}/selinux/.config_backup; \
 

+      sed -i 's/^SELINUX/BACKUP_SELINUX/g' %{_sysconfdir}/selinux/.config_backup; \
 

+ fi;
 

+ 

  %build
 

  

  %prep 
@@ -493,13 +534,20 @@ 
 

  %description targeted
 

  SELinux Reference policy targeted base module.
 

  

+ %pretrans targeted
 

+ %backupConfig
 

+ 

  %pre targeted
 

  %preInstall targeted
 

  

  %post targeted
 

+ %checkConfigConsistency targeted
 

  %postInstall $1 targeted
 

  exit 0
 

  

+ %posttrans targeted
 

+ %checkConfigConsistency targeted
 

+ 

  %postun targeted
 

  if [ $1 = 0 ]; then
 

      source /etc/selinux/config
 
@@ -565,6 +613,9 @@ 
 

  %description minimum
 

  SELinux Reference policy minimum base module.
 

  

+ %pretrans minimum
 

+ %backupConfig
 

+ 

  %pre minimum
 

  %preInstall minimum
 

  if [ $1 -ne 1 ]; then
 
@@ -572,6 +623,7 @@ 
 

  fi
 

  

  %post minimum
 

+ %checkConfigConsistency minimum
 

  contribpackages=`cat /usr/share/selinux/minimum/modules-contrib.lst`
 

  basepackages=`cat /usr/share/selinux/minimum/modules-base.lst`
 

  if [ ! -d /var/lib/selinux/minimum/active/modules/disabled ]; then
 
@@ -603,6 +655,9 @@ 
 

  fi
 

  exit 0
 

  

+ %posttrans minimum
 

+ %checkConfigConsistency minimum
 

+ 

  %postun minimum
 

  if [ $1 = 0 ]; then
 

      source /etc/selinux/config
 
@@ -660,13 +715,20 @@ 
 

  %description mls 

  SELinux Reference policy mls base module.
 

  

+ %pretrans mls
 

+ %backupConfig
 

+ 

  %pre mls 

  %preInstall mls
 

  

  %post mls 

+ %checkConfigConsistency mls
 

  %postInstall $1 mls
 

  exit 0
 

  

+ %posttrans mls
 

+ %checkConfigConsistency mls
 

+ 

  %postun mls
 

  if [ $1 = 0 ]; then
 

      source /etc/selinux/config

Test weather SELINUXTYPE listed in /etc/selinux/config is consistent
with installed packages and fix it if not.

Resolves: rhbz#1641631

Signed-off-by: Vit Mojzis vmojzis@redhat.com

I would add here rpm macro for /etc -> %{_sysconfdir}

Also here: /etc -> %{_sysconfdir}

Otherwise it looks good to me. 

# dnf install selinux-policy-3.14.3-21.fc30.noarch.rpm selinux-policy-minimum-3.14.3-21.fc30.noarch.rpm selinux-policy-mls-3.14.3-21.fc30.noarch.rpm selinux-policy-targeted-3.14.3-21.fc30.noarch.rpm
...

# grep -v '#' /etc/selinux/config

SELINUX=enforcing
SELINUXTYPE=minimum

Is it expected?

If I read that correctly, this would be better done in %posttrans instead of %post: at least in the case where a policy is replaced by another in the same transaction, at the time %post runs the old policy will be in place still and the system looks consistent so the script wont do anything, yet at the end of the transaction it'll be in inconsistent state. At %posttrans all erasures have been performed so this would catch and correct the inconsistency.

dnf install selinux-policy-3.14.3-21.fc30.noarch.rpm selinux-policy-minimum-3.14.3-21.fc30.noarch.rpm selinux-policy-mls-3.14.3-21.fc30.noarch.rpm selinux-policy-targeted-3.14.3-21.fc30.noarch.rpm

...

grep -v '#' /etc/selinux/config

SELINUX=enforcing
SELINUXTYPE=minimum

Is it expected?

No, but It shouldn't be a problem. When someone tries to install multiple policies we have no way to know which one he/she wants to be active... It's up to them to set the correct SELINUXTYPE. (Just as it was up until this patch... the only difference is that now at least the installation will work)

It would be possible to fix this by saving a backup of the confix file in %pretrans and improving the logic of checkConfigConsistency to take the backup into account.
Edited 4 years ago by vmojzis

If I read that correctly, this would be better done in %posttrans instead of %post: at least in the case where a policy is replaced by another in the same transaction, at the time %post runs the old policy will be in place still and the system looks consistent so the script wont do anything, yet at the end of the transaction it'll be in inconsistent state. At %posttrans all erasures have been performed so this would catch and correct the inconsistency.

Is it possible to remove eg. selinux-policy-targeted and install selinux-policy-minimal in 1 transaction?
If not then this needs to happen in %post since %postInstall contains "policy_load" command which will fail with inconsistent config. And policy_load cannot be moved to %posttrans since other packages may depend on the policy being loaded.

Is it possible to remove eg. selinux-policy-targeted and install selinux-policy-minimal in 1 transaction?

Sure it is, hence the comment about it.

If not then this needs to happen in %post since %postInstall contains "policy_load" command which will fail with inconsistent config. And policy_load cannot be moved to %posttrans since other packages may depend on the policy being loaded.

Note that this is somewhat dubious: the policy install/upgrade will happen in a largely arbitrary place in the transaction, so other packages cannot really rely on it (short of an explicit dependency on a policy which has its own issues). Loading the policy from %posttrans would make the behavior much more consistent and predictable for the rest of the system, BUT that's just one way to look at it and I'm positive it's not that simple.

Is it possible to remove eg. selinux-policy-targeted and install selinux-policy-minimal in 1 transaction?

Sure it is, hence the comment about it.

How?
Except for a package that would require selinix-policy-minimal and conflict selinux-policy-targeted. Because there shouldn't really be a reason for that.

If not then this needs to happen in %post since %postInstall contains "policy_load" command which will fail with inconsistent config. And policy_load cannot be moved to %posttrans since other packages may depend on the policy being loaded.

Note that this is somewhat dubious: the policy install/upgrade will happen in a largely arbitrary place in the transaction, so other packages cannot really rely on it (short of an explicit dependency on a policy which has its own issues). Loading the policy from %posttrans would make the behavior much more consistent and predictable for the rest of the system, BUT that's just one way to look at it and I'm positive it's not that simple.

Explicit dependencies on selinux-policy-* packages is how we deal with shipping custom policy modules. So it is getting more frequent.

How?
Except for a package that would require selinix-policy-minimal and conflict selinux-policy-targeted. Because there shouldn't really be a reason for that.

In general, simply by placing an install and a remove into the same transaction. Nothing special to it from rpm POV. dnf cli might not support it directly but "dnf shell" does, and so do a number of other package management clients (GUI-oriented in particular). It can also happen through obsoletion.

Explicit dependencies on selinux-policy-* packages is how we deal with shipping custom policy modules. So it is getting more frequent.

Right, such a scenario would be affected and bigger changes to how this is handled is way out of scope here anyway. I'd suggest doing an additional sanity check in %posttrans then to handle the case of removal within the same transaction.

How?
Except for a package that would require selinix-policy-minimal and conflict selinux-policy-targeted. Because there shouldn't really be a reason for that.

In general, simply by placing an install and a remove into the same transaction. Nothing special to it from rpm POV. dnf cli might not support it directly but "dnf shell" does, and so do a number of other package management clients (GUI-oriented in particular). It can also happen through obsoletion.

Explicit dependencies on selinux-policy-* packages is how we deal with shipping custom policy modules. So it is getting more frequent.

Right, such a scenario would be affected and bigger changes to how this is handled is way out of scope here anyway. I'd suggest doing an additional sanity check in %posttrans then to handle the case of removal within the same transaction.

Ok, thank you, I'll add another check in %posttrans

rebased onto 46c51e1
4 years ago

I added logic that allows us to choose targeted mode over others if possible (but still only if the config file is inconsistent with the installed packages).

LGTM.

Pull-Request has been merged by lvrabec
4 years ago

This was not valid and breaks initial system install (and thus Rawhide composes). See https://bugzilla.redhat.com/show_bug.cgi?id=1683365 . You cannot use shell script in %pretrans.

Oh, sorry for missing that!

Changes Summary 1
+62 -0
file changed
selinux-policy.spec
Powered by Pagure 5.13.3
Documentation • File an Issue • About this Instance • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.