On 06/21/2016 09:03 AM, Peter Krempa wrote:
On Mon, Jun 13, 2016 at 20:27:58 -0400, John Ferlan wrote:
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.
Add tests for sample output
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 12 ++++++-- src/qemu/qemu_domain.c | 19 ++++++++++-- .../qemuxml2argv-luks-disk-cipher.args | 36 ++++++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-luks-disks.args | 36 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++++- 5 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args
Note that this won't work with disk hotplug until you add code that will add the secret objects too.
Oh right - I had forgotten about the secret object... I had the same issue with the TLS code, so I know what has to be done...
Also I've noticed that RBD hotplug is now broken too due to the same reason. The code that selects whether to use plaintext password or pass it via the secret object does not check whether it's called on the hotplug path and thus uses the secret object.
Oh damn... I think that's a disconnect in qemuDomainSecretSetup (called from qemuDomainSecretDiskPrepare from hotplug) and the assumption of an AES secinfo for RBD when there's a secret object available.
The secret object is not added using the monitor though. You'll need to fix that first and if you opt for the correct approach this will then work automagically.
Looks okay but I can't ACK this with hotplug not working.
Also please note that on hot-unplug you need to remove the secrets as you'll possibly be adding a secret with the same name (Since alias will be reused)
yep...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 490260f..7062c17 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1103,6 +1103,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,10 +1238,13 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, qemuBufferEscapeComma(&opt, source); virBufferAddLit(&opt, ",");
- if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) virBufferAsprintf(&opt, "password-secret=%s,", secinfo->s.aes.alias); - } + + if (encinfo) + virQEMUBuildLuksOpts(&opt, disk->src->encryption, + encinfo->s.aes.alias);
This wrapper is not really useful here. It only adds "key-secret=" all the other options are necessary only if creating the volume.
I was thinking of the future if/when new things are added. I'm not clear yet what would have to be done for block-copy and snapshots.
if (disk->src->format > 0 && disk->src->type != VIR_STORAGE_TYPE_DIR) @@ -1920,6 +1924,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) && @@ -1949,6 +1954,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 c288fa0..fb3e91f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c
[...]
@@ -1026,7 +1028,6 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
virSecretUsageType secretUsageType; - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
if (VIR_ALLOC(secinfo) < 0) return -1; @@ -1044,6 +1045,20 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, diskPriv->secinfo = secinfo; }
+ if (conn && !virStorageSourceIsEmpty(src) &&
This part is the same with the block above. It may be worth extracting it ...
Oh right... I think I had it that way at one time, too... Thanks again - John
+ 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_KEY, NULL, + &src->encryption->secrets[0]->secdef) < 0) + goto error; + + diskPriv->encinfo = secinfo; + } + return 0;
error: