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

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