From a1901b36addfb7a064a5d451e1cfb47d5737c8eb Mon Sep 17 00:00:00 2001 Message-Id: From: John Ferlan Date: Mon, 25 Jul 2016 12:43:00 -0400 Subject: [PATCH] qemu: Add luks support for domain disk Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1301021 Generate the luks command line using the AES secret key to encrypt the luks secret. A luks secret object will be in addition to a an AES secret. For hotplug, check if the encinfo exists and if so, add the AES secret for the passphrase for the secret object used to decrypt the device. Modify/augment the fakeSecret* in qemuxml2argvtest in order to handle find a uuid or a volume usage with a specific path prefix in the XML (corresponds to the already generated XML tests). Add error message when the 'usageID' is not 'mycluster_myname'. Commit id '1d632c39' altered the error message generation to rely on the errors from the secret_driver (or it's faked replacement). Add the .args output for adding the LUKS disk to the domain Signed-off-by: John Ferlan (cherry picked from commit da86c6c22674ccc147224afa2740e33d8cbdbf22) NB: The .args output was modified from upstream since the downstream does not contain commit id 'e114b09157b7fcca12b218b531debfbc0c3a09d7' which adds ",sockets=1,cores=1,threads=1" to the "-smp 1" on the command line Signed-off-by: John Ferlan --- src/qemu/qemu_command.c | 9 +++ src/qemu/qemu_domain.c | 25 +++++++- src/qemu/qemu_hotplug.c | 68 ++++++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-luks-disks.args | 36 ++++++++++++ tests/qemuxml2argvtest.c | 24 +++++++- 5 files changed, 156 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0ee1d34..6c91e53 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1087,6 +1087,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, int actualType = virStorageSourceGetActualType(disk->src); qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus); if (idx < 0) { @@ -1226,6 +1227,10 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, secinfo->s.aes.alias); } + if (encinfo) + virQEMUBuildLuksOpts(&opt, &disk->src->encryption->encinfo, + encinfo->s.aes.alias); + if (disk->src->format > 0 && disk->src->type != VIR_STORAGE_TYPE_DIR) virBufferAsprintf(&opt, "format=%s,", @@ -1928,6 +1933,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, virDomainDiskDefPtr disk = def->disks[i]; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; /* PowerPC pseries based VMs do not support floppy device */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && @@ -1956,6 +1962,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0) return -1; + if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0) + return -1; + virCommandAddArg(cmd, "-drive"); optstr = qemuBuildDriveStr(disk, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 27158f4..26d0d12 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -946,7 +946,8 @@ qemuDomainSecretSetup(virConnectPtr conn, { if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && - secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH) { + (secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH || + secretUsageType == VIR_SECRET_USAGE_TYPE_VOLUME)) { if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias, secretUsageType, username, seclookupdef, isLuks) < 0) @@ -1006,11 +1007,14 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk) { virStorageSourcePtr src = disk->src; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = NULL; - if (conn && qemuDomainSecretDiskCapable(src)) { + if (!conn) + return 0; + + if (qemuDomainSecretDiskCapable(src)) { virSecretUsageType secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI; - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); if (VIR_ALLOC(secinfo) < 0) return -1; @@ -1026,6 +1030,21 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, diskPriv->secinfo = secinfo; } + if (!virStorageSourceIsEmpty(src) && src->encryption && + src->format == VIR_STORAGE_FILE_LUKS) { + + if (VIR_ALLOC(secinfo) < 0) + return -1; + + if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, + VIR_SECRET_USAGE_TYPE_VOLUME, NULL, + &src->encryption->secrets[0]->seclookupdef, + true) < 0) + goto error; + + diskPriv->encinfo = secinfo; + } + return 0; error: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9cb1d44..6509867 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -312,11 +312,14 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, bool releaseaddr = false; bool driveAdded = false; bool secobjAdded = false; + bool encobjAdded = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); virJSONValuePtr secobjProps = NULL; + virJSONValuePtr encobjProps = NULL; qemuDomainDiskPrivatePtr diskPriv; qemuDomainSecretInfoPtr secinfo; + qemuDomainSecretInfoPtr encinfo; if (!disk->info.type) { if (qemuDomainMachineIsS390CCW(vm->def) && @@ -356,6 +359,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, goto error; } + encinfo = diskPriv->encinfo; + if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) + goto error; + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; @@ -379,6 +386,15 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } secobjAdded = true; + if (encobjProps) { + rv = qemuMonitorAddObject(priv->mon, "secret", encinfo->s.aes.alias, + encobjProps); + encobjProps = NULL; /* qemuMonitorAddObject consumes */ + if (rv < 0) + goto exit_monitor; + } + encobjAdded = true; + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto exit_monitor; driveAdded = true; @@ -398,6 +414,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, cleanup: virJSONValueFree(secobjProps); + virJSONValueFree(encobjProps); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); @@ -413,6 +430,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } if (secobjAdded) ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias)); + if (encobjAdded) + ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); @@ -570,11 +589,17 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, { size_t i; qemuDomainObjPrivatePtr priv = vm->privateData; + virErrorPtr orig_err; char *drivestr = NULL; char *devstr = NULL; bool driveAdded = false; + bool encobjAdded = false; int ret = -1; + int rv; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virJSONValuePtr encobjProps = NULL; + qemuDomainDiskPrivatePtr diskPriv; + qemuDomainSecretInfoPtr encinfo; if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup; @@ -605,6 +630,11 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0) goto error; + diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + encinfo = diskPriv->encinfo; + if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) + goto error; + if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; @@ -616,6 +646,15 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, qemuDomainObjEnterMonitor(driver, vm); + if (encobjProps) { + rv = qemuMonitorAddObject(priv->mon, "secret", encinfo->s.aes.alias, + encobjProps); + encobjProps = NULL; /* qemuMonitorAddObject consumes */ + if (rv < 0) + goto exit_monitor; + } + encobjAdded = true; + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto exit_monitor; driveAdded = true; @@ -632,6 +671,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, ret = 0; cleanup: + virJSONValueFree(encobjProps); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); @@ -643,6 +683,14 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (driveAdded) VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr); + orig_err = virSaveLastError(); + if (encobjAdded) + ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } + ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -2856,6 +2904,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; char *drivestr; char *objAlias = NULL; + char *encAlias = NULL; VIR_DEBUG("Removing disk %s from domain %p %s", disk->info.alias, vm, vm->def->name); @@ -2881,6 +2930,20 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } } + /* Similarly, if this is possible a device using LUKS encryption, we + * can remove the luks object password too + */ + if (!virStorageSourceIsEmpty(disk->src) && disk->src->encryption && + disk->src->format == VIR_STORAGE_FILE_LUKS) { + + if (!(encAlias = + qemuDomainGetSecretAESAlias(disk->info.alias, true))) { + VIR_FREE(objAlias); + VIR_FREE(drivestr); + return -1; + } + } + qemuDomainObjEnterMonitor(driver, vm); /* If it fails, then so be it - it was a best shot */ @@ -2888,6 +2951,11 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, ignore_value(qemuMonitorDelObject(priv->mon, objAlias)); VIR_FREE(objAlias); + /* If it fails, then so be it - it was a best shot */ + if (encAlias) + ignore_value(qemuMonitorDelObject(priv->mon, encAlias)); + VIR_FREE(encAlias); + qemuMonitorDriveDel(priv->mon, drivestr); VIR_FREE(drivestr); if (qemuDomainObjExitMonitor(driver, vm) < 0) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args new file mode 100644 index 0000000..efb5cb0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name encryptdisk \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-encryptdisk/master-key.aes \ +-M pc-i440fx-2.1 \ +-m 1024 \ +-smp 1 \ +-uuid 496898a6-e6ff-f7c8-5dc2-3cf410945ee9 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-encryptdisk/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-object secret,id=virtio-disk0-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=/storage/guest_disks/encryptdisk,\ +key-secret=virtio-disk0-luks-secret0,format=luks,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=virtio-disk1-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=/storage/guest_disks/encryptdisk2,\ +key-secret=virtio-disk1-luks-secret0,format=luks,if=none,id=drive-virtio-disk1 \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d37d125..eddacf8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -49,12 +49,22 @@ fakeSecretGetValue(virSecretPtr obj ATTRIBUTE_UNUSED, static virSecretPtr fakeSecretLookupByUsage(virConnectPtr conn, - int usageType ATTRIBUTE_UNUSED, + int usageType, const char *usageID) { unsigned char uuid[VIR_UUID_BUFLEN]; - if (STRNEQ(usageID, "mycluster_myname")) + if (usageType == VIR_SECRET_USAGE_TYPE_VOLUME) { + if (!STRPREFIX(usageID, "/storage/guest_disks/")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "test provided invalid volume storage prefix '%s'", + usageID); + return NULL; + } + } else if (STRNEQ(usageID, "mycluster_myname")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "test provided incorrect usage '%s'", usageID); return NULL; + } if (virUUIDGenerate(uuid) < 0) return NULL; @@ -62,10 +72,17 @@ fakeSecretLookupByUsage(virConnectPtr conn, return virGetSecret(conn, uuid, usageType, usageID); } +static virSecretPtr +fakeSecretLookupByUUID(virConnectPtr conn, + const unsigned char *uuid) +{ + return virGetSecret(conn, uuid, 0, ""); +} + static virSecretDriver fakeSecretDriver = { .connectNumOfSecrets = NULL, .connectListSecrets = NULL, - .secretLookupByUUID = NULL, + .secretLookupByUUID = fakeSecretLookupByUUID, .secretLookupByUsage = fakeSecretLookupByUsage, .secretDefineXML = NULL, .secretGetXMLDesc = NULL, @@ -1362,6 +1379,7 @@ mymain(void) DO_TEST("encrypted-disk", NONE); DO_TEST("encrypted-disk-usage", NONE); + DO_TEST("luks-disks", QEMU_CAPS_OBJECT_SECRET); DO_TEST("memtune", NONE); DO_TEST("memtune-unlimited", NONE); -- 2.9.2