Could we reflect this change also in semanage.conf?


Some numbers posted by pmoore.

  # grep '^expand-check' /etc/selinux/semanage.conf 
  expand-check=0
  # time semanage dontaudit off
  real    0m5.270s
  user    0m4.447s
  sys     0m0.799s
  # time semanage dontaudit on
  real    0m5.385s
  user    0m4.578s
  sys     0m0.786s

  # grep '^expand-check' /etc/selinux/semanage.conf 
  expand-check=1
  # time semanage dontaudit off
  real    0m5.555s
  user    0m4.745s
  sys     0m0.780s
  # time semanage dontaudit on
  real    0m5.861s
  user    0m5.037s
  sys     0m0.804s

Pushed to rawhide libsemanage-2.5-3.fc25.

Please test it and if there's only 7% penalty then I'll make an update for F24 as well.

I have seen this crash when using typebounds commands.

(In reply to Petr Lautrbach from comment #2)
> Pushed to rawhide libsemanage-2.5-3.fc25.

For reasons I don't understand, I'm still not seeing this update in Rawhide, suggestions?

  # dnf clean metadata
  Cleaning repos: pcmoore-kernel-secnext plautrba-selinux rawhide-debuginfo
                : rawhide
  17 metadata files removed
  11 dbcache files removed
  # dnf update libsemanage
  Fedora - Rawhide - Debug                        3.4 MB/s |  18 MB     00:05    
  Fedora - Rawhide - Developmental packages for t 1.8 MB/s |  47 MB     00:25    
  Copr repo for selinux owned by plautrba          75 kB/s |  40 kB     00:00    
  Copr repo for kernel-secnext owned by pcmoore   1.6 MB/s | 5.9 MB     00:03    
  Last metadata expiration check: 0:00:04 ago on Tue Mar 22 10:31:23 2016.
  Dependencies resolved.
  Nothing to do.
  Complete!
  # rpm -q libsemanage
  libsemanage-2.5-2.fc25.x86_64

I must have missed to run 'fedpkg build', sorry. It's being built right now - http://koji.fedoraproject.org/koji/taskinfo?taskID=13425830

Something's wrong in koji:

13425830 build (rawhide, //libsemanage:2041bbce9964058fdbac5eb5ee5da0a1d383dc86): open (buildhw-10.phx2.fedoraproject.org) -> FAILED: BuildError: Error running GIT command "git clone -n git://pkgs.fedoraproject.org//libsemanage /var/lib/mock/f25-build-5239985-592735/root/tmp/scmroot/libsemanage", see checkout.log for details

I've had koji/brew blow up with internal errors like that before, most of the time submitting the build again works.

Built by mgrepl, thanks!

http://koji.fedoraproject.org/koji/buildinfo?buildID=747934

I'm still not seeing libsemanage-2.5-3.fc25 on my Rawhide systems for some reason.

It might be some delay on mirrors. In the mean time, you can try the build from comment 8 or change the value in your semanage.conf  - expand-check=1

Update will warn you about changed config file and you will have semanage.conf.rpmnew file in /etc/selinux with the package default.

btw works for me:

# dnf update libsemanage
...
 libsemanage              x86_64      2.5-3.fc25       rawhide      148 k
 libsemanage-python       x86_64      2.5-3.fc25       rawhide      108 k
 libsemanage-python3      x86_64      2.5-3.fc25       rawhide      111 k

The update finally appeared on my Rawhide system and some quick tests revealed numbers similar to what I reported previously (see comment #1).

This will break if the user has any locally installed policy modules that violate a neverallow, which is not unlikely since they have never been enforced up until now and an audit2allow-generated policy could easily run afoul of a particular neverallow.  The end result would be that all semodule and semanage operations (including those triggered by a policy update) would start failing because the neverallow checks would fail, and the only way to fix would be to switch back to expand-check=0 temporarily, remove the offending module, and then re-enable expand-check=1.  The user would then need to fix the offending module's source (which may not be available to them any longer; again, if they generated it via audit2allow at some point in the past, installed the .pp file and then didn't backup or otherwise preserve the .te file) in order to recompile and re-install it.
So while I applaud re-enabling make validate on policy builds and encourage use of expand-check=1 by developers, I'm not sure you can inflict this on end users yet.

Also, as Dan pointed out, there seems to be a bug in the CIL type bounds error reporting logic that causes a seg fault in libsepol 2.5 for his docker example.

That's a good point that I hadn't thought of earlier, although at what point do we want to enable the expanded checks?  I think we need to work towards enabling this, and I fear it will be a flag-day event regardless.

Probably need more granular control over it, e.g. ability to separately disable/enable neverallow vs type hierarchy checking (easy), ability to exempt specific modules from specific checks (more difficult).

We are however getting neverallow and type hierarchy checking on the Fedora-shipped policy as a result of re-enabling make validate without overriding SEMOD_EXP in the Fedora selinux-policy spec file.  So we are only lacking the checks on third party and user policy modules.

We could turn the check on for only Fresh installs, and leave the check off on updates.  But this could still break third parties.

(In reply to Daniel Walsh from comment #16)
> We could turn the check on for only Fresh installs, and leave the check off
> on updates.  But this could still break third parties.

Is there precedence for doing something like that?  I worry we would get flooded by confused people trying to figure out why they can load policy module on system A (older, updated system), but it fails on system B (newer, fresh install).

These kind of changes usually happen in Rawhide where they are somehow expected. We  need to document it in release notes with the expand-check=0 workaround. As for F24, it's probably late to propose it as a Fedora change, but since we are in pre-Beta phase, it should be still feasible to have this change in Release notes as well. I can create a FESCO ticket for that to have officially approved.

I would say this should only be in Rawhide, to find out what kind of damage it will cause.

This could break people doing 

audit2allow -M mypol

But then again maybe it should.

Thanks Petr.  I guess we'll go with this and see what happens, maybe we break the whole world, maybe not; we'll never know how bad it is until we try.

(In reply to Daniel Walsh from comment #19)
> I would say this should only be in Rawhide, to find out what kind of damage
> it will cause.
> 
> This could break people doing 
> 
> audit2allow -M mypol
> 
> But then again maybe it should.

I agree with Rawhide for now. But definitely something what we need to have.

Apparently  it breaks docker - https://github.com/fedora-cloud/docker-selinux/issues/16 so lets postpone it to Rawhide/f25 to let docker time to fix it.

IMHO, this should be reverted (i.e. expand-check = 0 again in default semanage.conf). Rationale: Users may have previously installed local policy modules generated by audit2allow that violate neverallows.  In this situation, on upgrade, semodule will fail from %post in selinux-policy-targeted due to neverallow failure.  However, rpm will think the package has been successfully installed since this occurs from %post, even though libsemanage will have rolled back to the previous policy, so rpm -q will lie and the user may not even know that they do not have the correct policy installed.  The user will be unable to make any policy updates or changes until they disable expand-check or disable/remove the offending module(s), and even then, they'll have to manually do a reinstall selinux-policy-targeted to just get the current policy installed.

I think it suffices to perform the validation at build time (via make validate), and policy developers can manually enable expand-check=1 for their own development/testing, but we can't make this the default in Fedora now given that it hasn't been imposed from the beginning and users may have built up quite a history of local policy modules, some of which they may not even still have sources.

I don't agree here. I don't think people will hit this issue so often
and if so they will report a new bug or ask. We don't have a lot
neverallows checks and there is a reason for them and it makes SELinux
Policy more secure.

You can see

neverallow check failed at line 250 of
/var/lib/selinux/targeted/tmp/modules/100/authlogin/cil
  (neverallow authlogin_typeattr_1 shadow_t (file (read)))
    <root>
    allow at line 5 of /var/lib/selinux/targeted/tmp/modules/400/mypol/cil
      (allow httpd_t shadow_t (file (getattr open read)))


I wrote the blog

https://mgrepl.wordpress.com/2015/09/09/selinux-insides-part2-neverallow-assertions/

about that.

Also it is a rawhide system which is a correct place for changes like this. We should probably do a public announcement.

So I might agree if we had sane failure semantics, but that isn't the current situation.  Policy module install really needs to happen as part of the rpm transaction, not from %post, which was the purpose of the rpm selinux plugin work, but no one ever took that and used it AFAIK.

Again, a user on upgrade could end up running with the old policy due to neverallow failure while rpm -q tells them they are running the new policy.  And they might not even see the error message unless they happen to run the update interactively or go look at the logs.  And it will never self-correct on subsequent policy updates; it can only be fixed by the user manually removing the offending module or modifying semanage.conf to disable expand-check, then running dnf reinstall selinux-policy-targeted to force a re-install of the policy.

Your call, but consider yourself warned...

Thanks Stephen. Your concerns are definitely valuable. 

I'm reopening this bug as we need to address the upgrade problem and provide the documentation or rather create an official Fedora Change page for Fedora 25 to allow broader discussion. Incompatible changes sometimes happen in Fedora but they need to be advertised and preferably approved by FESCo.

As for the upgrade problem, it could be feasible to solve it by a rpm trigger script which would check if it's possible to rebuild original policy and wouldn't allow to update if not. But it's just an idea which needs to be investigated.

Given that we are talking about Rawhide before F25, I'd let expand-check enabled for now and reverted it before f25 branch if we can't address described issues.

Yes, I did not want to pretend there is no issue. Petr described it correctly.

Thank you guys for this discussion.

This bug appears to have been reported against 'rawhide' during the Fedora 25 development cycle.
Changing version to '25'.

FYI: since we don't have a solution for the concerns raised above, I've reverted the default value of expand-check to 0 in libsemanage-2.5-7.fc25.

We will continue with the effort to have expand-check=1 in Rawhide/Fedora 26.

This bug appears to have been reported against 'rawhide' during the Fedora 26 development cycle.
Changing version to '26'.

checkpolicy-2.7-7.fc28 libselinux-2.7-13.fc28 libsemanage-2.7-12.fc28 libsepol-2.7-6.fc28 policycoreutils-2.7-17.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-08531132c6

checkpolicy-2.7-7.fc28 libselinux-2.7-13.fc28 libsemanage-2.7-12.fc28 libsepol-2.7-6.fc28 policycoreutils-2.7-17.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-08531132c6

checkpolicy-2.7-7.fc28, libselinux-2.7-13.fc28, libsemanage-2.7-12.fc28, libsepol-2.7-6.fc28, policycoreutils-2.7-17.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-08531132c6

checkpolicy-2.7-7.fc28, libselinux-2.7-13.fc28, libsemanage-2.7-12.fc28, libsepol-2.7-6.fc28, policycoreutils-2.7-17.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.