passt SELinux labelling (was: Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start)

[Daniel P. Berrangé]

On Fri, Mar 03, 2023 at 07:23:41AM -0800, Andrea Bolognani wrote:
> On Fri, Mar 03, 2023 at 10:03:02AM -0500, Laine Stump wrote:
> > On 2/23/23 5:47 AM, Daniel P. Berrangé wrote:
> > > This really isn't difficult to do in the security manager IMHO. It is
> > > just a variation on the existing virSecurityManagerSetChildProcessLabel
> > > method, which instead of using the standard QEMU svirt_t labels, queries
> > > the label by calling getfilecon on the binary to be execd and appending
> > > the MCS label.
> >
> > It seems it's not as simple as this.
> >
> > I have an implementation of this, available at
> >
> > https://gitlab.com/lainestump/libvirt/-/commits/active-passt-6
> >
> > The problem with it is that it doesn't end up setting the label to passt_t.
> > Instead, it sets it to passt_exec_t. My understanding is that, similar to
> > many other binaries (e.g. dnsmasq), the context type of the binary file is
> > passt_exec_t, and the rules in the policies use that as an indicator to
> > transition the process to passt_t.
> >
> > Here's the results of a few different relabelling strategies:
> >
> > In the case of Stefano's patch where libvirt's relabelling is completely
> > removed (it just calls virCommandRun() directly), the passt process looks
> > like this:
> >
> >   unconfined_u:unconfined_r:passt_t:s0-s0:c0.c1023 97132
> >
> > When I allow libvirt to do its normal relabelling, it ends up like this:
> >
> >   unconfined_u:unconfined_r:svirt_t:s0:c239,c560 104213
> >
> > When I run it with my patch (which calls getfilecon, then adds the MCS), it
> > looks like this:
> >
> >   system_u:object_r:passt_exec_t:s0:c239,c560
> >
> > Am I correct to think that what everyone is suggesting is that the process
> > ends up like this? (it's what would happen with Stefano's hard-coded hack
> > that this message is replying to):
> >
> >   unconfined_u:unconfined_r:passt_t:s0:c239,c560 104213
> >
> > If so, how are we to derive "passt_t" from "passt_exec_t", since both those
> > names are arbritrary, and some other selinux policy for some other binary
> > might choose names that don't simply differ by having "_exec" added/removed.
> > Is it possible to get the label of the process after it has been forked, and
> > grab the context type from there?
> 
> I'm in no way a SELinux expert, but the idea of figuring out the
> runtime label for the process based on information found on the
> filesystem makes me uncomfortable. The idea of using some sort of
> text transformation to get from one to the other, even more so.

Using the label on the filesystem is precisely the right way to
do this with SELinux. It is what the kernel does every time a
binary is invokved, unless the caller has overriden the target
type.

> Since we know that we're launching passt and not some other random
> helper, why can't we simply use passt_t directly here? It feels like
> that would be less prone to issues caused by accidental (or
> intentional) misconfigurations.

That ties libvirt's code to a specific policy impl which is
not a desirable thing. Same reason we don't hardcode svirt_t
as a type for QEMU, but instead query it dynamically from
the installed policy.

With 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 :|