Re: [PATCH v4 7/7] qemu: Add luks support for domain disk

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

 



On Mon, Jul 11, 2016 at 02:07:58PM -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.

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@xxxxxxxxxx>
---
src/qemu/qemu_command.c                            |  9 +++
src/qemu/qemu_domain.c                             | 25 +++++++-
src/qemu/qemu_hotplug.c                            | 74 +++++++++++++++++++++-
.../qemuxml2argvdata/qemuxml2argv-luks-disks.args  | 36 +++++++++++
tests/qemuxml2argvtest.c                           | 24 ++++++-
5 files changed, 160 insertions(+), 8 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 c97d0f6..c356450 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,",
@@ -1940,6 +1945,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 &&
@@ -1976,6 +1982,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)))
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d046633..ef7e3c3 100644
--- a/src/qemu/qemu_domain.c
+++ b/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:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 376e6aa..d8a9fee 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;

Naming the labels after what they do instead of where we came from
makes the main body easier to read. The downside is that you don't know
where you jumped from in the rollback section, but it should be simple
enough not to need it.

I suggest 'remove_secret' (and the next step would do
'remove_encryption_secret'), if we don't need the bool-based cleanup as
I suggested in 5/7.

+
    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;


Same comments as in 5/7 regarding the props stealing and *DiskDevice
object removal conditions.

ACK with the double frees on qemuDomainObjExitMonitor fixed.

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]