On Tue, Aug 23, 2016 at 07:25:56PM -0400, Daniel P. Berrange wrote:
On Tue, Aug 23, 2016 at 06:17:44PM -0400, Martin Kletzander wrote:On Tue, Aug 23, 2016 at 05:54:29PM -0400, Daniel P. Berrange wrote: > On Tue, Aug 23, 2016 at 05:06:20PM -0400, Martin Kletzander wrote: > > Hi everyone, > > > > so there was an idea about limiting the relabelling of images that > > libvirt does. And I'm taking the liberty of pitching my idea how to > > approach this. I feel like it's pretty simple thing and there's not > > much to talk about, but a) I could've missed something and b) you might > > hate the way I approach it. > > > > The idea is to extend the seclabel XML, for example: > > > > <seclabel type='dynamic' model='dac' relabel='whitelist'> > > <path>/var/lib/libvirt/images</path> > > <path>/data/virt-stuff</path> > > </seclabel> > > > > Either we allow 'relabel' to be set to 'whitelist' or add a new > > attribute with a name like 'mode' or something, which will control how > > we relabel the files (actually relabel='no' can mean 'whitelist' and > > relabel='yes' can mean blacklist without adding anything there). After > > that you can specify what paths are (dis)allowed to be labelled. > > > > Actually thinking about it I like the following the most: > > > > <seclabel type='dynamic' model='dac' relabel='no'> > > <whitelist path='/data'/> > > <blacklist path='/data/private/non-virt/stuff'/> > > </seclabel> > > > > which I believe is pretty explanatory. Feel free to ask if it's not. > > And let me know what you think. > > I don't think we need to get involved in the <seclabel> configuration. > I forgot to mention that I like that because you would be able to specify this per-disk as well as for the whole VM.Of course using info in <disk> directly achieves the same thing> We've already got the ability in the <disk> config to provide the > full backing chain explicitly. If a management app provides a full > backing chain in the XML, we could validate the app provided chain > against the chain we probe and report error if they mis-match. (Maybe > we in fact already report this?) > Not yet: "It is currently ignored on input and only used for output to describe the detected backing chains of running domains." It would help with this, but I don't feel like it's that usable. Also I feel like everyone will overuse that while misunderstanding what it actually does. Also if you do some block-merge or whatever, you need to update the backing chain and everyone will just re-probe it because no management layer likes keeping more information than needed.If you provide the <seclabel> whitelist though, you're essentially going to want to provide the same info that you would provide with the <backing> data, as the whitelist you're providing there is essentially trying to whitelist only the expected backing file. I don't feel like we should be inventing new seclabel elements to duplicate info we could already provide via existing backing data elements.
It just introduces more complexity that might result in more bugs, but yes, it is information duplication.
> Thinking bout this in the context of a recent OpenStack Nova CVE, > where Nova mistakenly set format=qcow2, instead of format=raw, allowing > a malicious guest to write a qcow2 header with backing file. If Nova > had provided the full backing chain it expected (no backing chain at > all), then libvirt would have seen the maliciously created backing > chain, and caught the problem despite Nova giving the wrong format. > > > Separately from this, in the context of storage pools, when giving > a <disk type=pool> in the domain XML, we could do validation to > ensure the backing file of the volume always pointed to a volume > that was also inside a storage pool. This would allow us to have > backing files pointing to volumes in different storage pools, but > would prevent them pointing to arbitrary files that were not in > storage pools at all (eg /etc/password, or /dev/root, etc). > That is good idea, but it won't cover all the cases, for example if you're not using storage pools.Yep, at least from OpenStack POV our goal is to switch 100% to using storage pools. For non-storage pool deployments though, I think it is common that the mgmt app will have a specific place where it will always put disks. eg on OpenStack file based disks will always live under /var/lib/nova/instances. So if there was a qemu.conf setting to whitelist allowed disk image locations, we could lock down where we permit relabelling for the openstack host as a whole, and not need to change anything on a per guest basis.
I like the per-guest approach better, but qemu.conf is satisfactory enough, I guess.
It could be nice to get feedback from upper mgmt layers, any idea where else to post this questions?ovirt might have feedback i guess
Cc'd ovirt-devel, even though I don't think they use it (or at least not yet). If you feel like it, don't hesitate to Cc appropriate OpenStack list as well (if that's not much cross-posting). Martin
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 :|
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list