[libvirt] [PATCH 2/2] storage: Check qemu-img encryption type capability
John Ferlan
jferlan at redhat.com
Wed Apr 18 12:08:41 UTC 2018
On 04/18/2018 04:29 AM, Daniel P. Berrangé wrote:
> On Tue, Apr 17, 2018 at 03:23:33PM -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1526382
>>
>> As of QEMU 2.9, qemu-img has enforced using the "key-secret" for
>> creation of encrypted volumes. That is, LUKS encryption is now
>> required and the old (awful) qcow[2] encryption methodolgy is
>> no longer supported.
>
> Not quite right actually. The 'key-secret' approach can be used to
> create both LUKS and the old qcow[2] encryption.
>
> We only forbid qcow[2] encryption with the system emulators, still
> have full support in qemu-img for sake of interoperability. The only
> break there was the command line syntax
>
Oh, OK - well I didn't find that to be obvious... So there is a way
using secret objects to create a qcow[2] encrypted volume?
Still Jano has NACK'd using help scraping (and posted a separate series
removing it completely).
So then the question becomes does this change "convert" into a disallow
this type of creation going forward? Do we just cause failure in
storageBackendCreateQemuImgCheckEncryption when not using LUKS or do we
let the qemu-img just be the bad guy and do nothing in our code?
>>
>> In order to check for this, we scan the qemu-img -o help options
>> looking for "key-secret" and if set, we enforce during the create
>> volume processing that the about to be encrypted volume doesn't
>> attempt to use the old crufty encryption mechanism.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>> src/storage/storage_util.c | 32 ++++++++++++++++++++++++++++++--
>> 1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
>> index 7df52239c2..d2e02a57ca 100644
>> --- a/src/storage/storage_util.c
>> +++ b/src/storage/storage_util.c
>> @@ -797,6 +797,7 @@ storagePloopResize(virStorageVolDefPtr vol,
>> enum {
>> QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
>> QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
>> + QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET,
>> };
>>
>> static char *
>> @@ -824,6 +825,14 @@ virStorageBackendQemuImgSupportsCompat(const char *output)
>> return strstr(output, "\ncompat ");
>> }
>>
>> +
>> +static bool
>> +virStorageBackendQemuImgRequiresKeySecret(const char *output)
>> +{
>> + return strstr(output, "key-secret");
>> +}
>> +
>> +
>> static int
>> virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
>> {
>> @@ -843,6 +852,11 @@ virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
>> if (virStorageBackendQemuImgSupportsCompat(output))
>> ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
>>
>> + /* QEMU 2.9 enforced that qemu-img creation of an encrypted volume
>> + * uses LUKS encryption. */
>> + if (virStorageBackendQemuImgRequiresKeySecret(output))
>> + ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET;
>> +
>> cleanup:
>> VIR_FREE(output);
>> return ret;
>> @@ -934,6 +948,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
>>
>> /* storageBackendCreateQemuImgCheckEncryption:
>> * @format: format of file found
>> + * @imgformat: image format capability
>> * @conn: pointer to connection
>> * @vol: pointer to volume def
>> *
>> @@ -943,6 +958,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
>> */
>> static int
>> storageBackendCreateQemuImgCheckEncryption(int format,
>> + int imgformat,
>> const char *type,
>> virStorageVolDefPtr vol)
>> {
>> @@ -956,6 +972,12 @@ storageBackendCreateQemuImgCheckEncryption(int format,
>> vol->target.encryption->format);
>> return -1;
>> }
>> + if (imgformat >= QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("qemu-img no longer supports qcow encryption, "
>> + "use LUKS encryption instead"));
>> + return -1;
>> + }
>
> Why is imgformat being compared against QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET ?
>
> Aren't those two sides of the expression from completely different
> enum types.
>
Although perhaps not well named, @imgformat is fetched via
virStorageBackendQEMUImgBackingFormat which returns
QEMU_IMG_BACKING_FORMAT_OPTIONS* type enum's.
John
>> if (enc->nsecrets > 1) {
>> virReportError(VIR_ERR_XML_ERROR, "%s",
>> _("too many secrets for qcow encryption"));
>> @@ -1264,8 +1286,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
>> return NULL;
>>
>> if (info.encryption &&
>> - storageBackendCreateQemuImgCheckEncryption(info.format, type,
>> - vol) < 0)
>> + storageBackendCreateQemuImgCheckEncryption(info.format, imgformat,
>> + type, vol) < 0)
>> return NULL;
>>
>>
>> @@ -2359,6 +2381,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
>> {
>> int ret = -1;
>> char *img_tool = NULL;
>> + int imgformat;
>> virCommandPtr cmd = NULL;
>> const char *type;
>> char *secretPath = NULL;
>> @@ -2371,6 +2394,10 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
>> return -1;
>> }
>>
>> + imgformat = virStorageBackendQEMUImgBackingFormat("qemu-img");
>> + if (imgformat < 0)
>> + goto cleanup;
>> +
>> if (vol->target.encryption) {
>> if (vol->target.format == VIR_STORAGE_FILE_RAW)
>> type = "luks";
>> @@ -2380,6 +2407,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
>> storageBackendLoadDefaultSecrets(vol);
>>
>> if (storageBackendCreateQemuImgCheckEncryption(vol->target.format,
>> + imgformat,
>> type, vol) < 0)
>> goto cleanup;
>>
>> --
>> 2.13.6
>>
>> --
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
> Regards,
> Daniel
>
More information about the libvir-list
mailing list