[Spice-devel] [PATCH] spec: call semanage in posttrans not in post

[Daniel P. Berrangé]

On Wed, Jan 30, 2019 at 04:05:27AM -0500, Frediano Ziglio wrote:
> > On Tue, Jan 29, 2019 at 06:40:32PM +0200, Uri Lublin wrote:
> > > It can happen that selinux-policy (targeted) is installed only after
> > > spice-streaming-agent (upon system installation). In that case
> > > running semanage in post scriptlet will fail.
> > > 
> > > In posttrans all packages are already installed, so it should be
> > > safe to call semanage at that point.
> > > 
> > > rhbz#1647789
> > > 
> > > Signed-off-by: Uri Lublin <uril at redhat.com>
> > > ---
> > > 
> > > In a first patch I wrote I also added a condition that
> > > checks if selinuxenabled. If people feel it's better
> > > I'll send a V2 with it.
> > > 
> > > ---
> > >  spice-streaming-agent.spec.in | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/spice-streaming-agent.spec.in b/spice-streaming-agent.spec.in
> > > index 5a06e89..6b5ac22 100644
> > > --- a/spice-streaming-agent.spec.in
> > > +++ b/spice-streaming-agent.spec.in
> > > @@ -13,7 +13,7 @@ BuildRequires:  catch-devel
> > >  BuildRequires:  pkgconfig(udev)
> > >  # we need /usr/sbin/semanage program which is available on different
> > >  # packages depending on distribution
> > > -Requires(post): /usr/sbin/semanage
> > > +Requires(posttrans): /usr/sbin/semanage
> > >  Requires(postun): /usr/sbin/semanage
> > >  
> > >  %description
> > > @@ -45,7 +45,9 @@ if test -d "%{buildroot}/%{_libdir}/%{name}/plugins";
> > > then
> > >      find %{buildroot}/%{_libdir}/%{name}/plugins -name '*.la' -delete
> > >  fi
> > >  
> > > -%post
> > > +# See rhbz#1647789 - call semanage in posttrans, not in post
> > > +# and https://fedoraproject.org/wiki/Packaging:Scriptlets
> > > +%posttrans
> > >  semanage fcontext -a -t xserver_exec_t %{_bindir}/spice-streaming-agent
> > >  2>/dev/null || :
> > >  restorecon %{_bindir}/spice-streaming-agent || :
> > 
> > I'm curious why these commands are present at all ? The normal way to deal
> > with this would be to file a bug against the SELinux policy to explicitly
> > add the spice-streaming-agent binary to the default policy, so that RPM
> > will set the correct context at install time.
> 
> I think the main reasons are historic. We were not sure about the context
> and file name so we end up with manually setting it in the spec.
> What the advantages on setting on the global policies?
> I see the disadvantage to add the policies in all systems, even if they
> won't have these files and the burden of opening all tickets.

Adding to the SELinux policy ensures that security policy additions get
reviewed by the SELinux maintainers. It also ensures that he policy has
the right rules regardless of how the user installs the binary. Not every
distro that uses SELinux uses RPMs, or the RPM spec bundled here. It
would also have avoided the bug you hit here with the race condition.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|