[ovirt-devel] [libvirt] RFC: Limited dynamic ownership

Mon Aug 29 09:19:31 UTC 2016

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 :|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20160829/a7e7cd41/attachment-0001.sig>