[libvirt] [PATCH 14/14] qemu: command: Add support for HTTP cookies

Peter Krempa posted 14 patches 8 years, 9 months ago
[libvirt] [PATCH 14/14] qemu: command: Add support for HTTP cookies
Posted by Peter Krempa 8 years, 9 months ago
Format the string into the "curl" format so that it's accepted by qemu.

Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140164
---
 src/qemu/qemu_command.c                            | 27 +++++++++++-
 .../qemuxml2argv-disk-drive-network-http.args      | 32 +++++++++++++++
 .../qemuxml2argv-disk-drive-network-http.xml       | 48 ++++++++++++++++++++++
 tests/qemuxml2argvtest.c                           |  1 +
 4 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e9c3ea952..980559859 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1467,6 +1467,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
     qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo;
     virJSONValuePtr srcprops = NULL;
     char *source = NULL;
+    size_t i;
     int ret = -1;

     if (qemuGetDriveSourceProps(disk->src, &srcprops) < 0)
@@ -1535,8 +1536,6 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
         case VIR_STORAGE_NET_PROTOCOL_RBD:
         case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
         case VIR_STORAGE_NET_PROTOCOL_ISCSI:
-        case VIR_STORAGE_NET_PROTOCOL_HTTP:
-        case VIR_STORAGE_NET_PROTOCOL_HTTPS:
         case VIR_STORAGE_NET_PROTOCOL_FTP:
         case VIR_STORAGE_NET_PROTOCOL_FTPS:
         case VIR_STORAGE_NET_PROTOCOL_TFTP:
@@ -1544,6 +1543,30 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
         case VIR_STORAGE_NET_PROTOCOL_LAST:
             break;

+        case VIR_STORAGE_NET_PROTOCOL_HTTP:
+        case VIR_STORAGE_NET_PROTOCOL_HTTPS:
+            if (disk->src->ncookies > 0) {
+                if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCK_CURL_OPTIONS)) {
+                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                                   _("this qemu does not support http cookies"));
+                    goto cleanup;
+                }
+
+                virBufferAddLit(buf, "file.cookie=");
+                for (i = 0; i < disk->src->ncookies; i++) {
+                    if (i > 0)
+                        virBufferAddLit(buf, "; ");
+
+                    virBufferAsprintf(buf, "%s=%s",
+                                      disk->src->cookies[i]->name,
+                                      disk->src->cookies[i]->value);
+                }
+
+                virBufferAddLit(buf, ",");
+            }
+
+            break;
+
         case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
             if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL))
                 virBufferAsprintf(buf, "file.debug=%d,", cfg->glusterDebugLevel);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.args
new file mode 100644
index 000000000..9900866cc
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.args
@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=http://example.org:80/test.img,format=raw,if=none,\
+id=drive-virtio-disk0 \
+-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
+id=virtio-disk0 \
+-drive file=https://example.org:443/test2.img,format=raw,if=none,\
+id=drive-virtio-disk1 \
+-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\
+id=virtio-disk1 \
+-drive 'file=http://example.org:1234/test3.img,\
+file.cookie=test=testcookievalue; test2=blurb,format=raw,if=none,\
+id=drive-virtio-disk2' \
+-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,\
+id=virtio-disk2
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.xml
new file mode 100644
index 000000000..fc5ac6e5e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.xml
@@ -0,0 +1,48 @@
+<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-system-i686</emulator>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source protocol='http' name='test.img'>
+        <host name='example.org'/>
+      </source>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source protocol='https' name='test2.img'>
+        <host name='example.org'/>
+      </source>
+      <target dev='vdb' bus='virtio'/>
+    </disk>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source protocol='http' name='test3.img'>
+        <host name='example.org' port='1234'/>
+        <cookies>
+          <cookie name='test'>testcookievalue</cookie>
+          <cookie name='test2'>blurb</cookie>
+        </cookies>
+      </source>
+      <target dev='vdc' bus='virtio'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c1b014b7d..fb49caa92 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -886,6 +886,7 @@ mymain(void)
     DO_TEST("disk-drive-network-iscsi-lun",
             QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI,
             QEMU_CAPS_SCSI_BLOCK);
+    DO_TEST("disk-drive-network-http", QEMU_CAPS_BLOCK_CURL_OPTIONS);
     DO_TEST("disk-drive-network-gluster",
             QEMU_CAPS_GLUSTER_DEBUG_LEVEL);
     DO_TEST("disk-drive-network-rbd", NONE);
-- 
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/14] qemu: command: Add support for HTTP cookies
Posted by Daniel P. Berrange 8 years, 9 months ago
On Wed, Apr 26, 2017 at 07:52:44PM +0200, Peter Krempa wrote:
> Format the string into the "curl" format so that it's accepted by qemu.
> 
> Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140164

[snip]

> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.args
> new file mode 100644
> index 000000000..9900866cc
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-http.args
> @@ -0,0 +1,32 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-i686 \
> +-name QEMUGuest1 \
> +-S \
> +-M pc \
> +-m 214 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-nographic \
> +-nodefaults \
> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
> +-no-acpi \
> +-boot c \
> +-usb \
> +-drive file=http://example.org:80/test.img,format=raw,if=none,\
> +id=drive-virtio-disk0 \
> +-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
> +id=virtio-disk0 \
> +-drive file=https://example.org:443/test2.img,format=raw,if=none,\
> +id=drive-virtio-disk1 \
> +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\
> +id=virtio-disk1 \
> +-drive 'file=http://example.org:1234/test3.img,\
> +file.cookie=test=testcookievalue; test2=blurb,format=raw,if=none,\

Your example cookie is rather tame, but I wonder if we should
consider cookie values to be security sensitive data, and thus
use the secrets mechanism. If we did this would also entail fixes
to QEMU to let use its secrets mechanism too.

I'm just wary of re-introducing a bug like CVE-2015-5160 (rbd
password information leak), via sensitive cookie values.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/14] qemu: command: Add support for HTTP cookies
Posted by Peter Krempa 8 years, 9 months ago
On Thu, Apr 27, 2017 at 16:30:44 +0100, Daniel Berrange wrote:
> On Wed, Apr 26, 2017 at 07:52:44PM +0200, Peter Krempa wrote:
> > Format the string into the "curl" format so that it's accepted by qemu.
> > 
> > Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140164

 [snip]

> Your example cookie is rather tame, but I wonder if we should
> consider cookie values to be security sensitive data, and thus
> use the secrets mechanism. If we did this would also entail fixes
> to QEMU to let use its secrets mechanism too.

I thought briefly about the same before posting this, but I went through
anyways ...

> 
> I'm just wary of re-introducing a bug like CVE-2015-5160 (rbd
> password information leak), via sensitive cookie values.

We could allow generic cookies passed on the command line
and then perhaps add a <cookie name="ble" secure='yes'>value</cookie>
which will be passed via the secrets infrastructure.

In that case I should probably add a statement saying that the cookies
are passed in a insecure way.,

This way generic cookies can be passed even now and the provision for
secure cookies can be added once qemu adds that feature.

Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/14] qemu: command: Add support for HTTP cookies
Posted by Peter Krempa 8 years, 9 months ago
On Thu, Apr 27, 2017 at 17:46:16 +0200, Peter Krempa wrote:
> On Thu, Apr 27, 2017 at 16:30:44 +0100, Daniel Berrange wrote:
> > On Wed, Apr 26, 2017 at 07:52:44PM +0200, Peter Krempa wrote:
> > > Format the string into the "curl" format so that it's accepted by qemu.
> > > 
> > > Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140164
> 
>  [snip]
> 
> > Your example cookie is rather tame, but I wonder if we should
> > consider cookie values to be security sensitive data, and thus
> > use the secrets mechanism. If we did this would also entail fixes
> > to QEMU to let use its secrets mechanism too.
> 
> I thought briefly about the same before posting this, but I went through
> anyways ...
> 
> > 
> > I'm just wary of re-introducing a bug like CVE-2015-5160 (rbd
> > password information leak), via sensitive cookie values.
> 
> We could allow generic cookies passed on the command line
> and then perhaps add a <cookie name="ble" secure='yes'>value</cookie>
> which will be passed via the secrets infrastructure.

Or better, treat them as secure by default and make users add
secure='no' explicitly so that they are aware of the way we pass it.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/14] qemu: command: Add support for HTTP cookies
Posted by Daniel P. Berrange 8 years, 9 months ago
On Thu, Apr 27, 2017 at 05:46:16PM +0200, Peter Krempa wrote:
> On Thu, Apr 27, 2017 at 16:30:44 +0100, Daniel Berrange wrote:
> > On Wed, Apr 26, 2017 at 07:52:44PM +0200, Peter Krempa wrote:
> > > Format the string into the "curl" format so that it's accepted by qemu.
> > > 
> > > Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140164
> 
>  [snip]
> 
> > Your example cookie is rather tame, but I wonder if we should
> > consider cookie values to be security sensitive data, and thus
> > use the secrets mechanism. If we did this would also entail fixes
> > to QEMU to let use its secrets mechanism too.
> 
> I thought briefly about the same before posting this, but I went through
> anyways ...
> 
> > 
> > I'm just wary of re-introducing a bug like CVE-2015-5160 (rbd
> > password information leak), via sensitive cookie values.
> 
> We could allow generic cookies passed on the command line
> and then perhaps add a <cookie name="ble" secure='yes'>value</cookie>
> which will be passed via the secrets infrastructure.
> 
> In that case I should probably add a statement saying that the cookies
> are passed in a insecure way.,
> 
> This way generic cookies can be passed even now and the provision for
> secure cookies can be added once qemu adds that feature.

The thing is it feels like the compelling reason to use cookies in
context of QEMU is precisely as an authorization mechanism. Even
if we document them as "insecure" people will do it anyway, and
the security flaw that results will be a libvirt CVE because we
don't provide apps an alternative todo what they need.

In addition, if the connection is using https: protocol, then I
we think we should be doing encryption for all cookies, and not
expect apps to set a secure=yes|no flag in the XML.

Last time we accepted a temporary insecure solution we waited 5 years
for QEMU to get us a fix...

So I'm inclined to NACK this feature until QEMU provides us a way
to handle cookies securely.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list