[libvirt] qemu command line quoting (was: Re: [Libguestfs] [PATCH 0/5] rbd improvements)
On Sun, May 12, 2013 at 02:42:36PM -0400, Mike Kelly wrote:
On Thu, May 9, 2013 at 12:21 PM, Richard W.M. Jones <rjones@redhat.com> wrote:
On Thu, May 09, 2013 at 11:23:55AM -0400, Mike Kelly wrote:
On Wed, May 8, 2013 at 6:53 AM, Richard W.M. Jones <rjones@redhat.com> wrote:
One worry I have is whether quoting is required for the server name(s), export name, username and secret.
Well. I think the main things we had to quote were ':' and ';', but none of those are valid in a hostname. Username also probably doesn't contain anything special, and secret is a base64-encoded string. I confirmed that even with the string ending in '==', it was parsed just fine by qemu, at least in my limited manual testing.
If you can suggest a way to be more robust this, though, then I can try to work that into a future patch series.
The quoting problem happens when someone writes a program which takes (eg) a hostname string from the user and passes it unmodified to the guestfs API. It's an issue if this string can cause unexpected [even malicious/exploitable] things to happen when passed unquoted on the qemu command line.
Well, I'm not sure if this way of setting things up is still encouraged, but at least this documentation suggests basically using the fact that libvirt won't quote the image name as a "feature":
http://ceph.com/w/index.php?title=QEMU-RBD#Caching
<disk type='network' device='disk'> <source protocol='rbd' name='poolname/imagename:rbd_cache=1:rbd_cache_size=67108864:rbd_cache_max_dirty=0'/>
Hmmm ... This is a bug in libvirt, but also a missing feature of libvirt since it cannot express these other configuration fields.
<driver name='qemu' type='rbd'/> <target dev='vda' bus='virtio'/> </disk>
The more official documentation for configuring caching doesn't seem to make any specific mention of this:
http://ceph.com/docs/master/rbd/qemu-rbd/#qemu-cache-options http://ceph.com/docs/master/rbd/libvirt/ http://ceph.com/docs/master/rbd/rbd-config-ref/
By the way, I fixed some qemu-img command line quoting issues yesterday (but not in ceph): https://github.com/libguestfs/libguestfs/commit/914d3e68ec272436f91080f47ddf... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
On Sun, May 12, 2013 at 09:15:33PM +0100, Richard W.M. Jones wrote:
On Sun, May 12, 2013 at 02:42:36PM -0400, Mike Kelly wrote:
On Thu, May 9, 2013 at 12:21 PM, Richard W.M. Jones <rjones@redhat.com> wrote:
On Thu, May 09, 2013 at 11:23:55AM -0400, Mike Kelly wrote:
On Wed, May 8, 2013 at 6:53 AM, Richard W.M. Jones <rjones@redhat.com> wrote:
One worry I have is whether quoting is required for the server name(s), export name, username and secret.
Well. I think the main things we had to quote were ':' and ';', but none of those are valid in a hostname. Username also probably doesn't contain anything special, and secret is a base64-encoded string. I confirmed that even with the string ending in '==', it was parsed just fine by qemu, at least in my limited manual testing.
If you can suggest a way to be more robust this, though, then I can try to work that into a future patch series.
The quoting problem happens when someone writes a program which takes (eg) a hostname string from the user and passes it unmodified to the guestfs API. It's an issue if this string can cause unexpected [even malicious/exploitable] things to happen when passed unquoted on the qemu command line.
Well, I'm not sure if this way of setting things up is still encouraged, but at least this documentation suggests basically using the fact that libvirt won't quote the image name as a "feature":
http://ceph.com/w/index.php?title=QEMU-RBD#Caching
<disk type='network' device='disk'> <source protocol='rbd' name='poolname/imagename:rbd_cache=1:rbd_cache_size=67108864:rbd_cache_max_dirty=0'/>
Hmmm ... This is a bug in libvirt, but also a missing feature of libvirt since it cannot express these other configuration fields.
Yes, that lack of escaping is a nasty bug. In fact it seems there is no way to escape ':' in a pool/image name, so I've sent a patch to just forbid it entirely in the XML, to avoid this kind of abuse. 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 :|
participants (2)
-
Daniel P. Berrange -
Richard W.M. Jones