From 40d8e2ba37163a86d072452774b098b12c918c7e Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Wed, 6 Apr 2016 15:00:59 -0400 Subject: [PATCH] qemu: Introduce qemuDomainSecretPrepare and Destroy Rather than needing to pass the conn parameter to various command line building API's, add qemuDomainSecretPrepare just prior to the qemuProcessLaunch which calls qemuBuilCommandLine. The function must be called after qemuProcessPrepareHost since it's expected to eventually need the domain masterKey generated during the prepare host call. Additionally, future patches may require device aliases (assigned during the prepare domain call) in order to associate the secret objects. The qemuDomainSecretDestroy is called after the qemuProcessLaunch finishes in order to clear and free memory used by the secrets that were recently prepared, so they are not kept around in memory too long. Placing the setup here is beneficial for future patches which will need the domain masterKey in order to generate an encrypted secret along with an initialization vector to be saved and passed (since the masterKey shouldn't be passed around). Finally, since the secret is not added during command line build, the hotplug code will need to get the secret into the private disk data. Signed-off-by: John Ferlan --- src/qemu/qemu_command.c | 45 +++--------- src/qemu/qemu_command.h | 5 +- src/qemu/qemu_domain.c | 150 ++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 15 +++- src/qemu/qemu_driver.c | 10 +-- src/qemu/qemu_hotplug.c | 27 +++++--- src/qemu/qemu_hotplug.h | 1 - src/qemu/qemu_process.c | 8 +++ 8 files changed, 203 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3b6d2726a7..aa8f5c4273 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -832,7 +832,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, int qemuGetDriveSourceString(virStorageSourcePtr src, - virConnectPtr conn, + qemuDomainSecretInfoPtr secinfo, char **source) { int actualType = virStorageSourceGetActualType(src); @@ -846,31 +846,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src, if (virStorageSourceIsEmpty(src)) return 1; - if (conn) { - if (actualType == VIR_STORAGE_TYPE_NETWORK && - src->auth && - (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI || - src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { - bool encode = false; - int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; - const char *protocol = virStorageNetProtocolTypeToString(src->protocol); - username = src->auth->username; - - if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { - /* qemu requires the secret to be encoded for RBD */ - encode = true; - secretType = VIR_SECRET_USAGE_TYPE_CEPH; - } - - if (!(secret = virSecretGetSecretString(conn, - protocol, - encode, - src->auth, - secretType))) - goto cleanup; - } - } - switch ((virStorageType) actualType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: @@ -881,6 +856,11 @@ qemuGetDriveSourceString(virStorageSourcePtr src, break; case VIR_STORAGE_TYPE_NETWORK: + if (secinfo) { + username = secinfo->s.plain.username; + secret = secinfo->s.plain.secret; + } + if (!(*source = qemuBuildNetworkDriveURI(src, username, secret))) goto cleanup; break; @@ -894,7 +874,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src, ret = 0; cleanup: - VIR_FREE(secret); return ret; } @@ -1033,8 +1012,7 @@ qemuCheckFips(void) char * -qemuBuildDriveStr(virConnectPtr conn, - virDomainDiskDefPtr disk, +qemuBuildDriveStr(virDomainDiskDefPtr disk, bool bootable, virQEMUCapsPtr qemuCaps) { @@ -1046,6 +1024,7 @@ qemuBuildDriveStr(virConnectPtr conn, int busid = -1, unitid = -1; char *source = NULL; int actualType = virStorageSourceGetActualType(disk->src); + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); if (idx < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1127,7 +1106,7 @@ qemuBuildDriveStr(virConnectPtr conn, break; } - if (qemuGetDriveSourceString(disk->src, conn, &source) < 0) + if (qemuGetDriveSourceString(disk->src, diskPriv->secinfo, &source) < 0) goto error; if (source && @@ -1816,7 +1795,6 @@ qemuBuildDriveDevStr(const virDomainDef *def, static int qemuBuildDiskDriveCommandLine(virCommandPtr cmd, - virConnectPtr conn, const virDomainDef *def, virQEMUCapsPtr qemuCaps, bool emitBootindex) @@ -1910,7 +1888,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, deviceFlagMasked = true; } } - optstr = qemuBuildDriveStr(conn, disk, + optstr = qemuBuildDriveStr(disk, emitBootindex ? false : !!bootindex, qemuCaps); if (deviceFlagMasked) @@ -9367,8 +9345,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildDiskDriveCommandLine(cmd, conn, def, qemuCaps, - emitBootindex) < 0) + if (qemuBuildDiskDriveCommandLine(cmd, def, qemuCaps, emitBootindex) < 0) goto error; if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 1e9ce66906..a7e7b74b5b 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -101,8 +101,7 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps); /* Both legacy & current support */ -char *qemuBuildDriveStr(virConnectPtr conn, - virDomainDiskDefPtr disk, +char *qemuBuildDriveStr(virDomainDiskDefPtr disk, bool bootable, virQEMUCapsPtr qemuCaps); @@ -179,7 +178,7 @@ char *qemuBuildRedirdevDevStr(const virDomainDef *def, int qemuNetworkPrepareDevices(virDomainDefPtr def); int qemuGetDriveSourceString(virStorageSourcePtr src, - virConnectPtr conn, + qemuDomainSecretInfoPtr secinfo, char **source); int qemuCheckDiskConfig(virDomainDiskDefPtr disk); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b14b2ddfc6..b25fdc3962 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -46,6 +46,7 @@ #include "viratomic.h" #include "virprocess.h" #include "virrandom.h" +#include "secret_util.h" #include "base64.h" #ifdef WITH_GNUTLS # include @@ -791,6 +792,146 @@ qemuDomainDiskPrivateDispose(void *obj) } +/* qemuDomainSecretPlainSetup: + * @conn: Pointer to connection + * @secinfo: Pointer to secret info + * @protocol: Protocol for secret + * @authdef: Pointer to auth data + * + * Taking a secinfo, fill in the plaintext information + * + * Returns 0 on success, -1 on failure with error message + */ +static int +qemuDomainSecretPlainSetup(virConnectPtr conn, + qemuDomainSecretInfoPtr secinfo, + virStorageNetProtocol protocol, + virStorageAuthDefPtr authdef) +{ + bool encode = false; + int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; + const char *protocolstr = virStorageNetProtocolTypeToString(protocol); + + secinfo->type = VIR_DOMAIN_SECRET_INFO_PLAIN; + if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0) + return -1; + + if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { + /* qemu requires the secret to be encoded for RBD */ + encode = true; + secretType = VIR_SECRET_USAGE_TYPE_CEPH; + } + + if (!(secinfo->s.plain.secret = + virSecretGetSecretString(conn, protocolstr, encode, + authdef, secretType))) + return -1; + + return 0; +} + + +/* qemuDomainSecretDiskDestroy: + * @disk: Pointer to a disk definition + * + * Clear and destroy memory associated with the secret + */ +void +qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) +{ + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (!diskPriv->secinfo) + return; + + qemuDomainSecretInfoFree(&diskPriv->secinfo); +} + + +/* qemuDomainSecretDiskPrepare: + * @conn: Pointer to connection + * @disk: Pointer to a disk definition + * + * For the right disk, generate the qemuDomainSecretInfo structure. + * + * Returns 0 on success, -1 on failure + */ +int +qemuDomainSecretDiskPrepare(virConnectPtr conn, + virDomainDiskDefPtr disk) +{ + virStorageSourcePtr src = disk->src; + qemuDomainSecretInfoPtr secinfo = NULL; + + if (conn && !virStorageSourceIsEmpty(src) && + virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK && + src->auth && + (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI || + src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { + + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (VIR_ALLOC(secinfo) < 0) + return -1; + + if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol, + src->auth) < 0) + goto error; + + diskPriv->secinfo = secinfo; + } + + return 0; + + error: + qemuDomainSecretInfoFree(&secinfo); + return -1; +} + + +/* qemuDomainSecretDestroy: + * @vm: Domain object + * + * Once completed with the generation of the command line it is + * expect to remove the secrets + */ +void +qemuDomainSecretDestroy(virDomainObjPtr vm) +{ + size_t i; + + for (i = 0; i < vm->def->ndisks; i++) + qemuDomainSecretDiskDestroy(vm->def->disks[i]); +} + + +/* qemuDomainSecretPrepare: + * @conn: Pointer to connection + * @vm: Domain object + * + * For any objects that may require an auth/secret setup, create a + * qemuDomainSecretInfo and save it in the approriate place within + * the private structures. This will be used by command line build + * code in order to pass the secret along to qemu in order to provide + * the necessary authentication data. + * + * Returns 0 on success, -1 on failure with error message set + */ +int +qemuDomainSecretPrepare(virConnectPtr conn, + virDomainObjPtr vm) +{ + size_t i; + + for (i = 0; i < vm->def->ndisks; i++) { + if (qemuDomainSecretDiskPrepare(conn, vm->def->disks[i]) < 0) + return -1; + } + + return 0; +} + + /* This is the old way of setting up per-domain directories */ static int qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver, @@ -3799,8 +3940,7 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver, bool -qemuDomainDiskSourceDiffers(virConnectPtr conn, - virDomainDiskDefPtr disk, +qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk, virDomainDiskDefPtr origDisk) { char *diskSrc = NULL, *origDiskSrc = NULL; @@ -3816,8 +3956,10 @@ qemuDomainDiskSourceDiffers(virConnectPtr conn, if (diskEmpty ^ origDiskEmpty) return true; - if (qemuGetDriveSourceString(disk->src, conn, &diskSrc) < 0 || - qemuGetDriveSourceString(origDisk->src, conn, &origDiskSrc) < 0) + /* This won't be a network storage, so no need to get the diskPriv + * in order to fetch the secret, thus NULL for param2 */ + if (qemuGetDriveSourceString(disk->src, NULL, &diskSrc) < 0 || + qemuGetDriveSourceString(origDisk->src, NULL, &origDiskSrc) < 0) goto cleanup; /* So far in qemu disk sources are considered different diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9cfe3e4be8..bde71a4eb4 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -497,8 +497,7 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, bool force_probe, bool report_broken); -bool qemuDomainDiskSourceDiffers(virConnectPtr conn, - virDomainDiskDefPtr disk, +bool qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk, virDomainDiskDefPtr origDisk); bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, @@ -616,4 +615,16 @@ int qemuDomainMasterKeyCreate(virQEMUDriverPtr driver, void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv); +void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) + ATTRIBUTE_NONNULL(1); + +int qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +void qemuDomainSecretDestroy(virDomainObjPtr vm) + ATTRIBUTE_NONNULL(1); + +int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a31986a897..0538c159ea 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7712,14 +7712,16 @@ qemuDomainChangeDiskLive(virConnectPtr conn, orig_disk->startupPolicy = dev->data.disk->startupPolicy; orig_disk->snapshot = dev->data.disk->snapshot; - if (qemuDomainDiskSourceDiffers(conn, disk, orig_disk)) { + if (qemuDomainDiskSourceDiffers(disk, orig_disk)) { /* Add the new disk src into shared disk hash table */ if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) goto cleanup; - if (qemuDomainChangeEjectableMedia(driver, conn, vm, - orig_disk, dev->data.disk->src, force) < 0) { - ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, vm->def->name)); + if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, + dev->data.disk->src, + force) < 0) { + ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, + vm->def->name)); goto rollback; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ac1f1fe7d2..77a65a75c0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -148,7 +148,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, /** * qemuDomainChangeEjectableMedia: * @driver: qemu driver structure - * @conn: connection structure * @vm: domain definition * @disk: disk definition to change the source of * @newsrc: new disk source to change to @@ -163,7 +162,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, */ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, - virConnectPtr conn, virDomainObjPtr vm, virDomainDiskDefPtr disk, virStorageSourcePtr newsrc, @@ -232,7 +230,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } while (rc < 0); if (!virStorageSourceIsEmpty(newsrc)) { - if (qemuGetDriveSourceString(newsrc, conn, &sourcestr) < 0) + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (qemuGetDriveSourceString(newsrc, diskPriv->secinfo, &sourcestr) < 0) goto error; if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR) { @@ -369,7 +369,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; - if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps))) + if (qemuDomainSecretDiskPrepare(conn, disk) < 0) + goto error; + + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps))) @@ -411,6 +414,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virDomainDiskInsertPreAlloced(vm->def, disk); cleanup: + qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); VIR_FREE(drivealias); @@ -574,10 +578,13 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; + if (qemuDomainSecretDiskPrepare(conn, disk) < 0) + goto error; + if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; - if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps))) + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) @@ -608,6 +615,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, virDomainDiskInsertPreAlloced(vm->def, disk); cleanup: + qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); virObjectUnref(cfg); @@ -620,8 +628,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, static int -qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn, - virQEMUDriverPtr driver, +qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk) { @@ -644,7 +651,7 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn, if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; - if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps))) + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; @@ -732,7 +739,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, goto cleanup; } - if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, + if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, disk->src, false) < 0) goto cleanup; @@ -754,7 +761,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, _("disk device='lun' is not supported for usb bus")); break; } - ret = qemuDomainAttachUSBMassStorageDevice(conn, driver, vm, disk); + ret = qemuDomainAttachUSBMassStorageDevice(driver, vm, disk); break; case VIR_DOMAIN_DISK_BUS_VIRTIO: diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 87d146e720..bcce9e83af 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -29,7 +29,6 @@ # include "domain_conf.h" int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, - virConnectPtr conn, virDomainObjPtr vm, virDomainDiskDefPtr disk, virStorageSourcePtr newsrc, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6e1d505fc2..d966c21ff2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5665,6 +5665,9 @@ qemuProcessStart(virConnectPtr conn, if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0) goto stop; + if (qemuDomainSecretPrepare(conn, vm) < 0) + goto cleanup; + if ((rv = qemuProcessLaunch(conn, driver, vm, asyncJob, incoming, snapshot, vmop, flags)) < 0) { if (rv == -2) @@ -5673,6 +5676,8 @@ qemuProcessStart(virConnectPtr conn, } relabel = true; + qemuDomainSecretDestroy(vm); + if (incoming && incoming->deferredURI && qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0) @@ -5734,6 +5739,9 @@ qemuProcessCreatePretendCmd(virConnectPtr conn, if (qemuProcessPrepareDomain(conn, driver, vm, flags) < 0) goto cleanup; + if (qemuDomainSecretPrepare(conn, vm) < 0) + goto cleanup; + VIR_DEBUG("Building emulator command line"); cmd = qemuBuildCommandLine(conn, driver, -- GitLab