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 <jferlan@redhat.com>

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,",
@@ -1929,6 +1934,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 &&
@@ -1965,6 +1971,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
         if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0)
             return -1;
 
+        if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0)
+            return -1;
+
         virCommandAddArg(cmd, "-drive");
 
         if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps)))

src/qemu/qemu_domain.c

@@ -993,7 +993,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)
@@ -1053,11 +1054,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;
@@ -1073,6 +1077,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:

src/qemu/qemu_hotplug.c

@@ -311,11 +311,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) &&
@@ -355,6 +358,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;
 
@@ -378,6 +385,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;
@@ -397,6 +413,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 
  cleanup:
     virJSONValueFree(secobjProps);
+    virJSONValueFree(encobjProps);
     qemuDomainSecretDiskDestroy(disk);
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
@@ -412,6 +429,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);
@@ -569,11 +588,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;
@@ -604,6 +629,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;
 
@@ -615,6 +645,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;
@@ -631,6 +670,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
     ret = 0;
 
  cleanup:
+    virJSONValueFree(encobjProps);
     qemuDomainSecretDiskDestroy(disk);
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
@@ -642,6 +682,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)

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,sockets=1,cores=1,threads=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

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,
@@ -1348,6 +1365,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);