[libvirt] [PATCH] Forbid use of ':' in RBD pool names

[Daniel Veillard]

On Mon, May 13, 2013 at 02:00:31PM +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> The QEMU command line syntax for RBD disks is
> 
>    file=rbd:pool/image:opt1=val1:opt2=val2...
> 
> There is no way to escape the ':' if it appears in the
> pool or image name. Thus it must be explicitly forbidden
> if it occurs in the libvirt XML. People are known to
> be abusing the lack of escaping in current libvirt to
> pass arbitrary args to QEMU.

  Urgh, :-(

> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/qemu/qemu_command.c                            |  6 ++++
>  ...qemuxml2argv-disk-drive-network-rbd-invalid.xml | 37 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  5 +++
>  3 files changed, 48 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-invalid.xml
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index eddc263..c0cb250 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2383,6 +2383,12 @@ qemuBuildRBDString(virConnectPtr conn,
>      char *secret = NULL;
>      size_t secret_size;
>  
> +    if (strchr(disk->src, ':')) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("':' not allowed in RBD source volume name"));
> +        return -1;
> +    }
> +
>      virBufferEscape(opt, ',', ",", "rbd:%s", disk->src);
>      if (disk->auth.username) {
>          virBufferEscape(opt, '\\', ":", ":id=%s", disk->auth.username);
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-invalid.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-invalid.xml
> new file mode 100644
> index 0000000..e8d3280
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-invalid.xml
> @@ -0,0 +1,37 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <disk type='network' device='disk'>
> +      <driver name='qemu' type='raw'/>
> +      <source protocol='rbd' name='poolname/imagename:rbd_cache=1:rbd_cache_size=67108864:rbd_cache_max_dirty=0'>
> +        <host name='mon1.example.org' port='6321'/>
> +        <host name='mon2.example.org' port='6322'/>
> +        <host name='mon3.example.org' port='6322'/>
> +      </source>
> +      <target dev='vda' bus='virtio'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 98ceb83..579c016 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -160,6 +160,9 @@ static int testCompareXMLToArgvFiles(const char *xml,
>                                       VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) {
>          if (flags & FLAG_EXPECT_FAILURE) {
>              ret = 0;
> +            if (virTestGetDebug() > 1)
> +                fprintf(stderr, "Got expected error: %s\n",
> +                        virGetLastErrorMessage());
>              virResetLastError();
>          }
>          goto out;
> @@ -528,6 +531,8 @@ mymain(void)
>              QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
>      DO_TEST("disk-drive-network-rbd-ipv6",
>              QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
> +    DO_TEST_FAILURE("disk-drive-network-rbd-invalid",
> +                    QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
>      DO_TEST("disk-drive-no-boot",
>              QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_BOOTINDEX);
>      DO_TEST("disk-usb",  NONE);

  ACK,

daniel

-- 
Daniel Veillard      | Open Source and Standards, Red Hat
veillard at redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/