[libvirt] [PATCH v4 10/10] 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. Add tests for sample output Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- This *replaces* the v3 of the series, the differences are mostly from code review, but with one addition change to remove an unnecessary change: 1. Delete unnecessary comment 2. Use exit_monitor label instead of failaddencsecret 3. Followed secobjProps w/r/t clearing encProps (including error path) 4. Followed similar processing in qemuDomainAttachSCSIDisk 5. Remove the encinfo cleanup in qemuDomainDetachDiskDevice (missed that during my merge/changes). src/qemu/qemu_command.c | 9 +++ src/qemu/qemu_domain.c | 25 +++++++- src/qemu/qemu_hotplug.c | 74 +++++++++++++++++++++- .../qemuxml2argv-luks-disk-cipher.args | 36 +++++++++++ .../qemuxml2argvdata/qemuxml2argv-luks-disks.args | 36 +++++++++++ tests/qemuxml2argvtest.c | 11 +++- 6 files changed, 185 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71e9e63..038a60c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1098,6 +1098,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) { @@ -1237,6 +1238,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,", @@ -1939,6 +1944,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 && @@ -1967,6 +1973,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 d51e82b..5405365 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -945,7 +945,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_PASSPHRASE)) { if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias, secretUsageType, username, seclookupdef, isLuks) < 0) @@ -1005,11 +1006,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; @@ -1025,6 +1029,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_PASSPHRASE, 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 da3e32a..ffbd8d9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -311,8 +311,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); virJSONValuePtr secobjProps = NULL; + virJSONValuePtr encProps = NULL; qemuDomainDiskPrivatePtr diskPriv; qemuDomainSecretInfoPtr secinfo; + qemuDomainSecretInfoPtr encinfo; if (!disk->info.type) { if (qemuDomainMachineIsS390CCW(vm->def) && @@ -352,6 +354,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, goto error; } + encinfo = diskPriv->encinfo; + if (encinfo && qemuBuildSecretInfoProps(encinfo, &encProps) < 0) + goto error; + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; @@ -371,6 +377,11 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, secobjProps) < 0) goto exit_monitor; + if (encProps && qemuMonitorAddObject(priv->mon, "secret", + encinfo->s.aes.alias, + encProps) < 0) + goto failaddencsecret; + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto failadddrive; @@ -386,6 +397,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, * for successful exit from monitor to clear; otherwise, error * paths wouldn't clean up properly */ secobjProps = NULL; + encProps = NULL; virDomainAuditDisk(vm, NULL, disk->src, "attach", true); @@ -394,6 +406,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, cleanup: virJSONValueFree(secobjProps); + virJSONValueFree(encProps); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); @@ -409,6 +422,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } failadddrive: + if (encProps) { + if (!orig_err) + orig_err = virSaveLastError(); + ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); + } + + failaddencsecret: if (secobjProps) { if (!orig_err) orig_err = virSaveLastError(); @@ -423,7 +443,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, exit_monitor: if (qemuDomainObjExitMonitor(driver, vm) < 0) releaseaddr = false; - secobjProps = NULL; /* qemuMonitorAddObject consumes props on failure too */ + /* qemuMonitorAddObject consumes props on failure too */ + secobjProps = NULL; + encProps = NULL; failexitmonitor: virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -574,10 +596,14 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, virDomainDiskDefPtr disk) { qemuDomainObjPrivatePtr priv = vm->privateData; + virErrorPtr orig_err = NULL; char *drivestr = NULL; char *devstr = NULL; int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virJSONValuePtr encProps = NULL; + qemuDomainDiskPrivatePtr diskPriv; + qemuDomainSecretInfoPtr encinfo; if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup; @@ -596,6 +622,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, &encProps) < 0) + goto error; + if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; @@ -605,9 +636,13 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) goto error; - /* Attach the device - 2 step process */ qemuDomainObjEnterMonitor(driver, vm); + if (encProps && qemuMonitorAddObject(priv->mon, "secret", + encinfo->s.aes.alias, + encProps) < 0) + goto exit_monitor; + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto failadddrive; @@ -617,6 +652,11 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (qemuDomainObjExitMonitor(driver, vm) < 0) goto failexitmonitor; + /* qemuMonitorAddObject consumes the props, but we need to wait + * for successful exit from monitor to clear; otherwise, error + * paths wouldn't clean up properly */ + encProps = NULL; + virDomainAuditDisk(vm, NULL, disk->src, "attach", true); virDomainDiskInsertPreAlloced(vm->def, disk); @@ -634,7 +674,17 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr); failadddrive: + orig_err = virSaveLastError(); + if (encProps) + ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } + + exit_monitor: ignore_value(qemuDomainObjExitMonitor(driver, vm)); + encProps = NULL; /* qemuMonitorAddObject consumes props on failure too */ failexitmonitor: virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -2821,6 +2871,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); @@ -2843,6 +2894,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 */ @@ -2850,6 +2915,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-disk-cipher.args b/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.args new file mode 100644 index 0000000..efb5cb0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.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/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 ca52ab0..aba346d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -62,10 +62,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, @@ -1342,6 +1349,8 @@ mymain(void) DO_TEST("encrypted-disk", NONE); DO_TEST("encrypted-disk-usage", NONE); + DO_TEST("luks-disks", QEMU_CAPS_OBJECT_SECRET); + DO_TEST("luks-disk-cipher", QEMU_CAPS_OBJECT_SECRET); DO_TEST("memtune", NONE); DO_TEST("memtune-unlimited", NONE); -- 2.5.5