Re: [libvirt] [PATCH v2 14/14] 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. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_block.c | 64 ++++++++++++++++++- src/qemu/qemu_command.c | 73 +++++++++++++++++++--- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_domain.c | 4 ++ src/qemu/qemu_hotplug.c | 49 ++++++++++++++- ...xml2argv-disk-drive-network-iscsi-auth-AES.args | 41 ++++++++++++ ...uxml2argv-disk-drive-network-iscsi-auth-AES.xml | 43 +++++++++++++ ...ml2argv-hostdev-scsi-virtio-iscsi-auth-AES.args | 45 +++++++++++++ ...xml2argv-hostdev-scsi-virtio-iscsi-auth-AES.xml | 48 ++++++++++++++ tests/qemuxml2argvtest.c | 10 +++ 10 files changed, 366 insertions(+), 14 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-AES.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-AES.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-AES.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-AES.xml diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 7fb12ea5a..057fb8233 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -482,6 +482,64 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src) } +static virJSONValuePtr +qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) +{ + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); + char *target = NULL; + char *lunStr = NULL; + char *username = NULL; + char *objalias = NULL; + unsigned int lun = 0; + virJSONValuePtr ret = NULL; + qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(src); + + /* { driver:"iscsi", + * transport:"tcp" ("iser" also possible) + * portal:"example.com", + * target:"iqn.2017-04.com.example:iscsi-disks", + * lun:1, + * [user:"username", + * password-secret:"secret-alias",]

+ * } + */ + + if (VIR_STRDUP(target, src->path) < 0) + goto cleanup; + + /* Separate the target and lun */ + if ((lunStr = strchr(target, '/'))) { + *(lunStr++) = '\0'; + if (virStrToLong_ui(lunStr, NULL, 10, &lun) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse target for lunStr '%s'"), + target); + goto cleanup; + } + } + + if (src->auth) { + username = src->auth->username; + objalias = diskSrcPriv->secinfo->s.aes.alias; + } + + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", protocol, + "s:portal", src->hosts[0].name, + "s:target", target, + "u:lun", lun, + "s:transport", "tcp", + "S:user", username, + "S:password-secret", objalias, + NULL)); + goto cleanup; + + cleanup: + VIR_FREE(target); + return ret; +}

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c851823e7..f9edf623c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c

@@ -4846,10 +4854,13 @@ qemuBuildSCSIHostHostdevDrvStr(virDomainHostdevDefPtr dev) } static char * -qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) +qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps) { + char *netsource = NULL; char *source = NULL; - virStorageSource src; + virStorageSource src = { 0 }; + virJSONValuePtr srcprops = NULL; qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(dev); memset(&src, 0, sizeof(src)); @@ -4857,14 +4868,51 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; + src.type = VIR_STORAGE_TYPE_NETWORK; src.protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; src.path = iscsisrc->path; src.hosts = iscsisrc->hosts; src.nhosts = iscsisrc->nhosts; /* Rather than pull what we think we want - use the network disk code */ - source = qemuBuildNetworkDriveStr(&src, hostdevPriv->secinfo); + if (qemuDiskSourceNeedsProps(&src, qemuCaps)) { + /* The next pile of code hunts and gathers as if @src were a disk. + * In particular, using private data... So a bit more chicanery is + * going to be required */ + qemuDomainDiskSrcPrivatePtr diskSrcPriv; + + if (!iscsisrc->auth->username) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing username for iSCSI auth")); + goto cleanup; + } + src.auth = iscsisrc->auth; + + if (VIR_ALLOC(src.privateData) < 0) + goto cleanup;

+ diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(&src);

+ diskSrcPriv->secinfo = hostdevPriv->secinfo;

+ srcprops = qemuBlockStorageSourceGetBackendProps(&src); + VIR_FREE(src.privateData); + if (!srcprops) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to build the backend props")); + goto cleanup; + } + if (!(netsource = virQEMUBuildDriveCommandlineFromJSON(srcprops))) + goto cleanup; + if (virAsprintf(&source, "%s,if=none,format=raw", netsource) < 0) + goto cleanup; + } else { + if (!(netsource = qemuBuildNetworkDriveStr(&src, hostdevPriv->secinfo))) + goto cleanup; + if (virAsprintf(&source, "file=%s,if=none,format=raw", netsource) < 0) + goto cleanup; + } + + cleanup: + VIR_FREE(netsource); return source; } @@ -4907,7 +4955,8 @@ qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def, } char * -qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) +qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; char *source = NULL; @@ -4915,9 +4964,9 @@ 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; - virBufferAsprintf(&buf, "file=%s,if=none,format=raw", source); + virBufferAsprintf(&buf, "%s", source); } else { if (!(source = qemuBuildSCSIHostHostdevDrvStr(dev))) goto error; @@ -5414,10 +5463,14 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, /* SCSI */ if (virHostdevIsSCSIDevice(hostdev)) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { + qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev); char *drvstr; + if (qemuBuildDiskSecinfoCommandLine(cmd, hostdevPriv->secinfo) < 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_hotplug.c b/src/qemu/qemu_hotplug.c index a544cecb9..17228d1b4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c

@@ -3879,8 +3909,22 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, if (!(drivealias = qemuAliasFromHostdev(hostdev))) goto cleanup; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_ISCSI_PASSWORD_SECRET)) { + if (!(objAlias = + qemuDomainGetSecretAESAlias(hostdev->info->alias, false))) { + return -1; + }

+ } + 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)); + qemuDomainObjDiskSecretObjectAliasEntryRemove(priv, objAlias); + } + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; }