qemu: Use secret objects to pass iSCSI passwords

https://bugzilla.redhat.com/show_bug.cgi?id=1425757

The blockdev-add code provides a mechanism to sanely provide user
and password-secret arguments for iscsi without placing them on the
command line to be viewable by a 'ps -ef' type command or needing
to create separate -iscsi devices for each disk/volume found.

So modify the iSCSI command line building to check for the presence
of the capability in order properly setup and use the domain master
secret object to encrypt the password in a secret object and alter
the parameters for the command line to utilize.

Modify the xml2argvtest to exhibit the syntax for both disk and
hostdev configurations.

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d4ceef831cfba52d66e5d10fb2259ad0ce4dfbf0..0eb5912535c074bfaf3d3b518ce4e3e160a35788 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1465,7 +1465,8 @@ qemuDiskBusNeedsDeviceArg(int bus)
  * the legacy representation.
  */
 static bool
-qemuDiskSourceNeedsProps(virStorageSourcePtr src)
+qemuDiskSourceNeedsProps(virStorageSourcePtr src,
+                         virQEMUCapsPtr qemuCaps)
 {
     int actualType = virStorageSourceGetActualType(src);
 
@@ -1478,6 +1479,11 @@ qemuDiskSourceNeedsProps(virStorageSourcePtr src)
         src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS)
         return true;
 
+    if (actualType == VIR_STORAGE_TYPE_NETWORK &&
+        src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
+        virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_PASSWORD_SECRET))
+        return true;
+
     return false;
 }
 
@@ -1509,6 +1515,7 @@ qemuDiskSourceGetProps(virStorageSourcePtr src)
 
 static int
 qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
+                        virQEMUCapsPtr qemuCaps,
                         virBufferPtr buf)
 {
     int actualType = virStorageSourceGetActualType(disk->src);
@@ -1524,7 +1531,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
         encinfo = srcpriv->encinfo;
     }
 
-    if (qemuDiskSourceNeedsProps(disk->src) &&
+    if (qemuDiskSourceNeedsProps(disk->src, qemuCaps) &&
         !(srcprops = qemuDiskSourceGetProps(disk->src)))
         goto cleanup;
 
@@ -1722,7 +1729,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
 {
     virBuffer opt = VIR_BUFFER_INITIALIZER;
 
-    if (qemuBuildDriveSourceStr(disk, &opt) < 0)
+    if (qemuBuildDriveSourceStr(disk, qemuCaps, &opt) < 0)
         goto error;
 
     if (qemuDiskBusNeedsDeviceArg(disk->bus)) {
@@ -4956,25 +4963,36 @@ qemuBuildSCSIHostHostdevDrvStr(virDomainHostdevDefPtr dev)
 }
 
 static char *
-qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
+qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
+                                virQEMUCapsPtr qemuCaps)
 {
     char *source = NULL;
     char *netsource = NULL;
+    virJSONValuePtr srcprops = NULL;
     virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
     virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
     qemuDomainStorageSourcePrivatePtr srcPriv =
         QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
 
-    /* Rather than pull what we think we want - use the network disk code */
-    netsource = qemuBuildNetworkDriveStr(iscsisrc->src, srcPriv ?
-                                         srcPriv->secinfo : NULL);
-    if (!netsource)
-        goto cleanup;
-    if (virAsprintf(&source, "file=%s,if=none,format=raw", netsource) < 0)
-        goto cleanup;
+    if (qemuDiskSourceNeedsProps(iscsisrc->src, qemuCaps)) {
+        if (!(srcprops = qemuDiskSourceGetProps(iscsisrc->src)))
+            goto cleanup;
+        if (!(netsource = virQEMUBuildDriveCommandlineFromJSON(srcprops)))
+            goto cleanup;
+        if (virAsprintf(&source, "%s,if=none,format=raw", netsource) < 0)
+            goto cleanup;
+    } else {
+        /* Rather than pull what we think we want - use the network disk code */
+        if (!(netsource = qemuBuildNetworkDriveStr(iscsisrc->src, srcPriv ?
+                                                   srcPriv->secinfo : NULL)))
+            goto cleanup;
+        if (virAsprintf(&source, "file=%s,if=none,format=raw", netsource) < 0)
+            goto cleanup;
+    }
 
  cleanup:
     VIR_FREE(netsource);
+    virJSONValueFree(srcprops);
     return source;
 }
 
@@ -5017,7 +5035,8 @@ qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def,
 }
 
 char *
-qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
+qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
+                           virQEMUCapsPtr qemuCaps)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     char *source = NULL;
@@ -5025,7 +5044,7 @@ qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
     virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
 
     if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
-        if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(dev)))
+        if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(dev, qemuCaps)))
             goto error;
         virBufferAdd(&buf, source, -1);
     } else {
@@ -5524,10 +5543,24 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
         /* SCSI */
         if (virHostdevIsSCSIDevice(hostdev)) {
             if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
+                virDomainHostdevSubsysSCSIPtr scsisrc =
+                    &hostdev->source.subsys.u.scsi;
                 char *drvstr;
 
+                if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
+                    virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc =
+                        &scsisrc->u.iscsi;
+                    qemuDomainStorageSourcePrivatePtr srcPriv =
+                        QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
+
+                    if (qemuBuildDiskSecinfoCommandLine(cmd, srcPriv ?
+                                                        srcPriv->secinfo :
+                                                        NULL) < 0)
+                        return -1;
+                }
+
                 virCommandAddArg(cmd, "-drive");
-                if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev)))
+                if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps)))
                     return -1;
                 virCommandAddArg(cmd, drvstr);
                 VIR_FREE(drvstr);

diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 18e0894581dbb7ebee9b1e40a8ccf995304bddff..bdde6f918109c83f77467446f3bf12b523ffe6e5 100644 (file)
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -150,7 +150,8 @@ char *qemuBuildUSBHostdevDevStr(const virDomainDef *def,
                                 virDomainHostdevDefPtr dev,
                                 virQEMUCapsPtr qemuCaps);
 
-char *qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev);
+char *qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
+                                 virQEMUCapsPtr qemuCaps);
 
 char *qemuBuildSCSIHostdevDevStr(const virDomainDef *def,
                                  virDomainHostdevDefPtr dev,

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9036deb9503aa5025756321dfdb23b4db7ce5b9b..9dab6b24b15ce4d5eb520685f738c229f80618d8 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1185,9 +1185,13 @@ qemuDomainSecretSetup(virConnectPtr conn,
                       virSecretLookupTypeDefPtr seclookupdef,
                       bool isLuks)
 {
+    bool iscsiHasPS = virQEMUCapsGet(priv->qemuCaps,
+                                     QEMU_CAPS_ISCSI_PASSWORD_SECRET);
+
     if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) &&
         virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) &&
         (usageType == VIR_SECRET_USAGE_TYPE_CEPH ||
+         (usageType == VIR_SECRET_USAGE_TYPE_ISCSI && iscsiHasPS) ||
          usageType == VIR_SECRET_USAGE_TYPE_VOLUME ||
          usageType == VIR_SECRET_USAGE_TYPE_TLS)) {
         if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias,

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 030b177f2a3bb3d39c53989f97de31422cb5f152..cbc7af59b74eff117571000d1f0def65bea47815 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2263,6 +2263,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
                                virDomainHostdevDefPtr hostdev)
 {
     size_t i;
+    int rv;
     int ret = -1;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virErrorPtr orig_err;
@@ -2273,6 +2274,12 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
     bool teardownlabel = false;
     bool teardowndevice = false;
     bool driveAdded = false;
+    bool secobjAdded = false;
+    virJSONValuePtr secobjProps = NULL;
+    virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+    virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
+    qemuDomainStorageSourcePrivatePtr srcPriv;
+    qemuDomainSecretInfoPtr secinfo;
 
     if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -2313,7 +2320,14 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
     if (qemuDomainSecretHostdevPrepare(conn, priv, hostdev) < 0)
         goto cleanup;
 
-    if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev)))
+    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
+    secinfo = srcPriv->secinfo;
+    if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
+        if (qemuBuildSecretInfoProps(secinfo, &secobjProps) < 0)
+            goto cleanup;
+    }
+
+    if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv->qemuCaps)))
         goto cleanup;
 
     if (!(drivealias = qemuAliasFromHostdev(hostdev)))
@@ -2327,6 +2341,15 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
 
     qemuDomainObjEnterMonitor(driver, vm);
 
+    if (secobjProps) {
+        rv = qemuMonitorAddObject(priv->mon, "secret", secinfo->s.aes.alias,
+                                  secobjProps);
+        secobjProps = NULL; /* qemuMonitorAddObject consumes */
+        if (rv < 0)
+            goto exit_monitor;
+        secobjAdded = true;
+    }
+
     if (qemuMonitorAddDrive(priv->mon, drvstr) < 0)
         goto exit_monitor;
     driveAdded = true;
@@ -2344,7 +2367,6 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
     ret = 0;
 
  cleanup:
-    qemuDomainSecretHostdevDestroy(hostdev);
     if (ret < 0) {
         qemuHostdevReAttachSCSIDevices(driver, vm->def->name, &hostdev, 1);
         if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
@@ -2356,6 +2378,8 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
             qemuDomainNamespaceTeardownHostdev(driver, vm, hostdev) < 0)
             VIR_WARN("Unable to remove host device from /dev");
     }
+    qemuDomainSecretHostdevDestroy(hostdev);
+    virJSONValueFree(secobjProps);
     VIR_FREE(drivealias);
     VIR_FREE(drvstr);
     VIR_FREE(devstr);
@@ -2368,6 +2392,8 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
                  "qemuMonitorAddDevice",
                  drvstr, devstr);
     }
+    if (secobjAdded)
+        ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias));
     ignore_value(qemuDomainObjExitMonitor(driver, vm));
     virErrorRestore(&orig_err);
 
@@ -3851,6 +3877,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
     int ret = -1;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     char *drivealias = NULL;
+    char *objAlias = NULL;
     bool is_vfio = false;
 
     VIR_DEBUG("Removing host device %s from domain %p %s",
@@ -3862,11 +3889,29 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
     }
 
     if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
+        virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+        virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
+
         if (!(drivealias = qemuAliasFromHostdev(hostdev)))
             goto cleanup;
 
+        /* Look for the markers that the iSCSI hostdev was added with a
+         * secret object to manage the username/password. If present, let's
+         * attempt to remove the object as well. */
+        if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI &&
+            virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_ISCSI_PASSWORD_SECRET) &&
+            qemuDomainSecretDiskCapable(iscsisrc->src)) {
+            if (!(objAlias = qemuDomainGetSecretAESAlias(hostdev->info->alias, false)))
+                goto cleanup;
+        }
+
         qemuDomainObjEnterMonitor(driver, vm);
         qemuMonitorDriveDel(priv->mon, drivealias);
+
+        /* If it fails, then so be it - it was a best shot */
+        if (objAlias)
+            ignore_value(qemuMonitorDelObject(priv->mon, objAlias));
+
         if (qemuDomainObjExitMonitor(driver, vm) < 0)
             goto cleanup;
     }
@@ -3938,6 +3983,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
 
  cleanup:
     VIR_FREE(drivealias);
+    VIR_FREE(objAlias);
     virObjectUnref(cfg);
     return ret;
 }

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-virtio-iscsi-auth-AES.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-virtio-iscsi-auth-AES.args
new file mode 100644 (file)
index 0000000..8683467
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-virtio-iscsi-auth-AES.args
@@ -0,0 +1,43 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest2 \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest2/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
+-usb \
+-object secret,id=virtio-disk0-secret0,\
+data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
+keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
+-drive file.driver=iscsi,file.portal=example.org:6000,\
+file.target=iqn.1992-01.com.example:storage,file.lun=1,file.transport=tcp,\
+file.user=myname,file.password-secret=virtio-disk0-secret0,format=raw,if=none,\
+id=drive-virtio-disk0 \
+-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
+id=virtio-disk0 \
+-object secret,id=hostdev0-secret0,\
+data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
+keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
+-drive file.driver=iscsi,file.portal=example.org:6000,\
+file.target=iqn.1992-01.com.example:storage,file.lun=2,file.transport=tcp,\
+file.user=myname,file.password-secret=hostdev0-secret0,if=none,format=raw,\
+id=drive-hostdev0 \
+-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
+drive=drive-hostdev0,id=hostdev0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-virtio-iscsi-auth-AES.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-virtio-iscsi-auth-AES.xml
new file mode 100644 (file)
index 0000000..21a0b11
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-virtio-iscsi-auth-AES.xml
@@ -0,0 +1,43 @@
+<domain type='qemu'>
+  <name>QEMUGuest2</name>
+  <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</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'/>
+      <auth username='myname'>
+        <secret type='iscsi' usage='mycluster_myname'/>
+      </auth>
+      <source protocol='iscsi' name='iqn.1992-01.com.example:storage/1'>
+        <host name='example.org' port='6000'/>
+      </source>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <controller type='scsi' index='0' model='virtio-scsi'/>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <hostdev mode='subsystem' type='scsi' managed='yes'>
+      <source protocol='iscsi' name='iqn.1992-01.com.example:storage/2'>
+        <host name='example.org' port='6000'/>
+        <auth username='myname'>
+          <secret type='iscsi' usage='mycluster_myname'/>
+        </auth>
+      </source>
+    </hostdev>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 3fb3183212cf622fe9c4702258c81c2140144d57..29df253b889a22a03ade0c1376b1059c8da3834d 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2364,6 +2364,12 @@ mymain(void)
     DO_TEST("hostdev-scsi-virtio-iscsi-auth",
             QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI,
             QEMU_CAPS_DEVICE_SCSI_GENERIC);
+# ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
+    DO_TEST("disk-hostdev-scsi-virtio-iscsi-auth-AES",
+            QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI,
+            QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_OBJECT_SECRET,
+            QEMU_CAPS_ISCSI_PASSWORD_SECRET);
+# endif
     DO_TEST("hostdev-scsi-vhost-scsi-ccw",
             QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI,
             QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_VIRTIO_CCW);