Re: [PATCH v3 10/10] qemu: Add luks support for domain disk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 24, 2016 at 04:53:39PM -0400, John Ferlan wrote:
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@xxxxxxxxxx>
---
src/qemu/qemu_command.c                            |  9 +++
src/qemu/qemu_domain.c                             | 25 ++++++-
src/qemu/qemu_hotplug.c                            | 82 +++++++++++++++++++---
.../qemuxml2argv-luks-disk-cipher.args             | 36 ++++++++++
.../qemuxml2argvdata/qemuxml2argv-luks-disks.args  | 36 ++++++++++
tests/qemuxml2argvtest.c                           | 11 ++-
6 files changed, 187 insertions(+), 12 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)) {

Since we have not allowed VIR_SECRET_USAGE_TYPE_PASSPHRASE before,
can we error out if we don't have both VIR_CRYPTO_CIPHER_AES256CBC and
QEMU_CAPS_OBJECT_SECRET to avoid putting plaintext on qemu command line?

        if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias,
                                     secretUsageType, username,
                                     seclookupdef, isLuks) < 0)
@@ -364,7 +370,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
    if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
        goto error;

-    /* Attach the device - possible 3 step process */
+    /* Attach the device - possible 4 step process */

If you delete this outdated comment in 8/10, you don't have to update it
here.

    qemuDomainObjEnterMonitor(driver, vm);

    if (secobjProps && qemuMonitorAddObject(priv->mon, "secret",
@@ -373,6 +379,12 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
        goto failaddobjsecret;
    secobjProps = NULL;

+    if (encProps && qemuMonitorAddObject(priv->mon, "secret",
+                                         encinfo->s.aes.alias,
+                                         encProps) < 0)
+        goto failaddencsecret;

s/failaddencsecret/delete_secinfo/

+    encProps = NULL;
+
    if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
        goto failadddrive;

@@ -391,6 +403,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,

 cleanup:
    virJSONValueFree(secobjProps);
+    virJSONValueFree(encProps);
    qemuDomainSecretDiskDestroy(disk);
    VIR_FREE(devstr);
    VIR_FREE(drivestr);
@@ -410,8 +423,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
    }

 failadddrive:
+    if (encProps)
+        ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));

This can also overwrite the error.

+
+ failaddencsecret:
    if (secobjProps)
        ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias));
+    encProps = NULL; /* qemuMonitorAddObject consumes props on failure too */

 failaddobjsecret:
    if (qemuDomainObjExitMonitor(driver, vm) < 0)
@@ -571,6 +589,9 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
    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;
@@ -589,6 +610,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;

@@ -598,9 +624,15 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
    if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
        goto error;

-    /* Attach the device - 2 step process */
+    /* Attach the device - possible 3 step process */
    qemuDomainObjEnterMonitor(driver, vm);

+    if (encProps && qemuMonitorAddObject(priv->mon, "secret",
+                                         encinfo->s.aes.alias,
+                                         encProps) < 0)

Don't we need to deal with secinfo like we do for virtio disks?

+        goto failaddencsecret;

s/failaddencsecret/exit_monitor/

+    encProps = NULL;
+
    if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
        goto failadddrive;


ACK only if we don't need to deal with diskPriv->secinfo;

Jan

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]