[libvirt] [PATCH v1 16/21] qemu: Enter the namespace on relabelling

[Daniel P. Berrange]

On Thu, Nov 24, 2016 at 03:48:05PM +0100, Michal Privoznik wrote:
> Instead of trying to fix our security drivers, we can use a
> simple trick to relabel paths in both namespace and the host.
> I mean, if we enter the namespace some paths are still shared
> with the host so any change done to them is visible from the host
> too.
> Therefore, we can just enter the namespace and call
> SetAllLabel()/RestoreAllLabel() from there. Yes, it has slight
> overhead because we have to fork in order to enter the namespace.
> But on the other hand, no complexity is added to our code.

I'm a little concerned that this may be storing problems for us
at a later date. If the security manager classes have any state
they update such changes are invisible to the main libvirt
process now. Also stuff running between fork+exec is restricted
to async signal safe functions only. I think it is hard for us
to be entirely confident about that safety when we're running
the entire security driver labelling code in that region.

Ultimately the only bit of the security drivers that needs to
run in the namespace is the setfilecon_raw() or chown() system
calls.

So I wonder if we should make it possible to provide a namespace
helper callback to the security drivers that they'd use only
for the setfilecon_raw/chown calls.

eg, we could do something like

  
  typedef (*virSecurityManagerNamespaceHelperFunc)(void *opaque)
  typedef (*virSecurityManagerNamespaceHelper)(virSecurityManagerNamespaceHelperFunc func, void *opaque)

  virSecurityManagerSetNamespacehelper(mgr, runInNamespaceHelper)

   ....do the labelling...

  virSecurityManagerSetNamespacehelper(mgr, NULL)

the namespace helper would have to be stored in a thread local
since we need to cope with some VMs not having separate namespaces

The security manager code would need todo

  struct SELinuxNamespaceData {
      const char *path;
      security_context_t ctx;
  }

Now instead of


   if (setfilecon_raw(path, tcon) < 0)
       ...

it would do

   static int SELinuxNamespaceHelperFunc(void *opaque) {
       struct SELinuxNamespaceData *data = opaque

       return setfilecon_raw(data->path, data->tcon)
   }

   struct SELinuxNamespaceData data = { path, tcon}

   if (namespacehelper(SELinuxNamespaceHelperFunc, &data) < 0)
      ...



Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|