Re: [libvirt] [PATCH 3/3] storage: Allow inputvol to be encrypted

On 09/11/2018 07:16 AM, Michal Privoznik wrote: > On 08/21/2018 06:23 PM, John Ferlan wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=1613737 >> >> When processing the inputvol for encryption, we need to handle >> the case where the inputvol is encrypted. This then allows for >> the encrypted inputvol to be used either for an output encrypted >> volume or an output volume of some XML provided type. >> >> Add tests to show the various conversion options when either input >> or output is encrypted. This includes when both are encrypted. >> >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> src/storage/storage_util.c | 62 ++++++++++++++++--- >> src/storage/storage_util.h | 1 + >> .../luks-convert-encrypt.argv | 11 ++++ >> .../luks-convert-encrypt2fileqcow2.argv | 7 +++ >> .../luks-convert-encrypt2fileraw.argv | 7 +++ >> tests/storagevolxml2argvtest.c | 15 ++++- >> tests/storagevolxml2xmlin/vol-encrypt1.xml | 21 +++++++ >> tests/storagevolxml2xmlin/vol-encrypt2.xml | 21 +++++++ >> 8 files changed, 137 insertions(+), 8 deletions(-) >> create mode 100644 tests/storagevolxml2argvdata/luks-convert-encrypt.argv >> create mode 100644 tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv >> create mode 100644 tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv >> create mode 100644 tests/storagevolxml2xmlin/vol-encrypt1.xml >> create mode 100644 tests/storagevolxml2xmlin/vol-encrypt2.xml >> >> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c >> index cc49a5b9f7..3c1e875b27 100644 >> --- a/src/storage/storage_util.c >> +++ b/src/storage/storage_util.c >> @@ -1084,6 +1084,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, >> unsigned int flags, >> const char *create_tool, >> const char *secretPath, >> + const char *inputSecretPath, >> virStorageVolEncryptConvertStep convertStep) >> { >> virCommandPtr cmd = NULL; >> @@ -1101,6 +1102,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, >> .secretAlias = NULL, >> }; >> virStorageEncryptionPtr enc = vol->target.encryption; >> + char *inputSecretAlias = NULL; >> + virStorageEncryptionPtr inputenc = inputvol ? inputvol->target.encryption : NULL; >> virStorageEncryptionInfoDefPtr encinfo = NULL; >> >> virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); >> @@ -1114,6 +1117,12 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, >> goto error; >> } >> >> + if (inputenc && inputenc->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { >> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> + _("encryption format of inputvol must be LUKS")); >> + goto error; >> + } >> + >> if (virStorageBackendCreateQemuImgSetInfo(pool, vol, inputvol, >> convertStep, &info) < 0) >> goto error; >> @@ -1153,6 +1162,20 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, >> encinfo = &enc->encinfo; >> } >> >> + if (inputenc && convertStep == VIR_STORAGE_VOL_ENCRYPT_CONVERT) { >> + if (!inputSecretPath) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> + _("path to inputvol secret data file is required")); >> + goto error; >> + } >> + if (virAsprintf(&inputSecretAlias, "%s_encrypt0", >> + inputvol->name) < 0) >> + goto error; >> + if (storageBackendCreateQemuImgSecretObject(cmd, inputSecretPath, >> + inputSecretAlias) < 0) >> + goto error; >> + } >> + >> if (convertStep != VIR_STORAGE_VOL_ENCRYPT_CONVERT) { >> if (storageBackendCreateQemuImgSetOptions(cmd, encinfo, info) < 0) >> goto error; >> @@ -1163,19 +1186,32 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, >> virCommandAddArgFormat(cmd, "%lluK", info.size_arg); >> } else { >> /* source */ >> - virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s", >> - info.inputType, info.inputPath); >> + if (inputenc) >> + virCommandAddArgFormat(cmd, >> + "driver=luks,file.filename=%s,key-secret=%s", >> + info.inputPath, inputSecretAlias); >> + else >> + virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s", >> + info.inputType, info.inputPath); >> >> /* dest */ >> - virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s,key-secret=%s", >> - info.type, info.path, info.secretAlias); >> + if (enc) >> + virCommandAddArgFormat(cmd, >> + "driver=%s,file.filename=%s,key-secret=%s", >> + info.type, info.path, info.secretAlias); >> + else >> + virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s", >> + info.type, info.path); >> + > > Same comment here as in previous patch. > OK, yeah, right, but only for info.inputType which just means a merge conflict resolution for this patch, unless there was something else you were referencing that I'm missing.

For this particular patch, altering the dest to account for the case when the source was encrypted, but the destination is not - IOW: the removal of secretAlias in that case.