[libvirt] [PATCH 4/7] security: Label parent directories of character devices

[Martin Kletzander]

On Fri, Aug 14, 2015 at 12:09:13PM +0100, Daniel P. Berrange wrote:
>On Fri, Aug 14, 2015 at 12:20:46PM +0200, Martin Kletzander wrote:
>> On Fri, Aug 14, 2015 at 11:10:05AM +0100, Daniel P. Berrange wrote:
>> >On Fri, Aug 14, 2015 at 11:58:54AM +0200, Martin Kletzander wrote:
>> >>On Thu, Aug 13, 2015 at 04:59:47PM +0100, Daniel P. Berrange wrote:
>> >>>On Thu, Aug 13, 2015 at 05:47:42PM +0200, Martin Kletzander wrote:
>> >>>>We are currently unable to label parent directories for some paths.
>> >>>>However, we will need to have per-domain directories that we would like
>> >>>>to have labelled, but we can't label all of them.  So let's add a
>> >>>>boolean variable that will determine whether parent directory for such
>> >>>>chardev should be labelled as well as that character device itself.
>> >>>>
>> >>>>Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> >>>>---
>> >>>> src/conf/domain_conf.h          |  1 +
>> >>>> src/security/security_dac.c     | 13 ++++++++++++-
>> >>>> src/security/security_selinux.c | 13 ++++++++++++-
>> >>>> 3 files changed, 25 insertions(+), 2 deletions(-)
>> >>>>
>> >>>>diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> >>>>index e1872bca002c..9d549a395e29 100644
>> >>>>--- a/src/conf/domain_conf.h
>> >>>>+++ b/src/conf/domain_conf.h
>> >>>>@@ -1191,6 +1191,7 @@ struct _virDomainChrSourceDef {
>> >>>>         } udp;
>> >>>>         struct {
>> >>>>             char *path;
>> >>>>+            bool autopath;
>> >>>>             bool listen;
>> >>>>         } nix;
>> >>>>         int spicevmc;
>> >>>
>> >>>I don't think we need this - it seems we can just pass a 'bool labelParent'
>> >>>parameter into  virSecurityManagerSetChardevLabel() when calling it for
>> >>>the monitor socket.
>> >>>
>> >>
>> >>It's not used only for the monitor socket, but mainly for virtio
>> >>channel's target's unix socket as well and maybe more in the future.
>> >>But I agree it could be named 'labelParent' as well.  Should I resend
>> >>it with that changed?
>> >
>> >In the non-monitor cases how will we decide whether it is appropriate
>> >to set labelParent or not ? Those paths are broadly user specified,
>> >so we can't assume the parent is per-VM
>> >
>>
>> We will label only those that we are sure that are per-VM, so only
>> those that are generated by the qemu driver itself.  That's exactly
>> what the parameter is used for -- labelling parent directories only
>> for those paths that are auto-generated by us, but leaving all
>> user-specified ones alone.
>
>IIUC, the paths generated by us will all end up in the same directory.
>So if we label that directory when setting up the monitor, then it
>will be correct for all the other paths too, so it doesn't seem like
>we need to store this parameter in virDomainDef. In fact, I tend to
>think we should perhaps add a virSecurityManagerSetDirLabel() that
>we just invoke once for the per-VM directory we created, and not try
>to associate it with any chardev
>

Monitor socket goes to $LIBDIR/libvirt/qemu/domain-$DOM/monitor.sock,
the other mentioned sockets go to
$LIBDIR/libvirt/qemu/channel/target/domain-$DOM/$NAME due to the fact
that it was pre-existing.  It's also good to have that separated so
users can specify targets with name 'monitor.sock'.

I thought about the virSecurityManagerSetDirLabel(), but I didn't want
to make this more complicated than it already is.

>
>Regards,
>Daniel
>--
>|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
>|: http://libvirt.org              -o-             http://virt-manager.org :|
>|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
>|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150814/f8230d56/attachment-0001.sig>