[libvirt] [PATCH 0/3] Allow inputvol when creating vol from inputvol to be encrypted
https://bugzilla.redhat.com/show_bug.cgi?id=1613737 Details in the patches (and even more in the bz). John Ferlan (3): storage: Remove secretPath from _virStorageBackendQemuImgInfo storage: Allow for inputvol to have any format for encryption storage: Allow inputvol to be encrypted src/storage/storage_util.c | 79 ++++++++++++++++--- src/storage/storage_util.h | 1 + .../luks-convert-encrypt.argv | 11 +++ .../luks-convert-encrypt2fileqcow2.argv | 7 ++ .../luks-convert-encrypt2fileraw.argv | 7 ++ .../luks-convert-qcow2.argv | 9 +++ tests/storagevolxml2argvtest.c | 19 ++++- tests/storagevolxml2xmlin/vol-encrypt1.xml | 21 +++++ tests/storagevolxml2xmlin/vol-encrypt2.xml | 21 +++++ tests/storagevolxml2xmlin/vol-file-qcow2.xml | 21 +++++ 10 files changed, 184 insertions(+), 12 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/storagevolxml2argvdata/luks-convert-qcow2.argv create mode 100644 tests/storagevolxml2xmlin/vol-encrypt1.xml create mode 100644 tests/storagevolxml2xmlin/vol-encrypt2.xml create mode 100644 tests/storagevolxml2xmlin/vol-file-qcow2.xml -- 2.17.1
There's really no need for it to be there since it's only ever used inside virStorageBackendCreateQemuImgCmdFromVol Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 42a9b6abf0..b32e3ccf7d 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -716,7 +716,6 @@ struct _virStorageBackendQemuImgInfo { int inputFormat; char *secretAlias; - const char *secretPath; }; @@ -1088,7 +1087,6 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, .compat = vol->target.compat, .features = vol->target.features, .nocow = vol->target.nocow, - .secretPath = secretPath, .secretAlias = NULL, }; virStorageEncryptionPtr enc = vol->target.encryption; @@ -1131,14 +1129,14 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, virCommandAddArgList(cmd, "-b", info.backingPath, NULL); if (enc) { - if (!info.secretPath) { + if (!secretPath) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("path to secret data file is required")); goto error; } if (virAsprintf(&info.secretAlias, "%s_encrypt0", vol->name) < 0) goto error; - if (storageBackendCreateQemuImgSecretObject(cmd, info.secretPath, + if (storageBackendCreateQemuImgSecretObject(cmd, secretPath, info.secretAlias) < 0) goto error; encinfo = &enc->encinfo; -- 2.17.1
Commit 39cef12a9 altered/fixed the inputvol processing to create a multistep process when using an inputvol to create an encrypted output volume; however, it unnecessarily assumed/restricted the inputvol to be of 'raw' format only. Modify the processing code to allow the inputvol format to be checked and used in order to create the encrypted volume. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 15 +++++++++++-- .../luks-convert-qcow2.argv | 9 ++++++++ tests/storagevolxml2argvtest.c | 4 ++++ tests/storagevolxml2xmlin/vol-file-qcow2.xml | 21 +++++++++++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tests/storagevolxml2argvdata/luks-convert-qcow2.argv create mode 100644 tests/storagevolxml2xmlin/vol-file-qcow2.xml diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index b32e3ccf7d..cc49a5b9f7 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -699,6 +699,7 @@ storagePloopResize(virStorageVolDefPtr vol, struct _virStorageBackendQemuImgInfo { int format; const char *type; + const char *inputType; const char *path; unsigned long long size_arg; unsigned long long allocation; @@ -1021,6 +1022,15 @@ virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool, return -1; } + if (inputvol && + !(info->inputType = + virStorageFileFormatTypeToString(inputvol->target.format))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown inputvol storage vol type %d"), + inputvol->target.format); + return -1; + } + if (info->preallocate && info->format != VIR_STORAGE_FILE_QCOW2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("metadata preallocation only available with qcow2")); @@ -1080,6 +1090,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, struct _virStorageBackendQemuImgInfo info = { .format = vol->target.format, .type = NULL, + .inputType = NULL, .path = vol->target.path, .allocation = vol->target.allocation, .encryption = !!vol->target.encryption, @@ -1152,8 +1163,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, virCommandAddArgFormat(cmd, "%lluK", info.size_arg); } else { /* source */ - virCommandAddArgFormat(cmd, "driver=raw,file.filename=%s", - info.inputPath); + virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s", + info.inputType, info.inputPath); /* dest */ virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s,key-secret=%s", diff --git a/tests/storagevolxml2argvdata/luks-convert-qcow2.argv b/tests/storagevolxml2argvdata/luks-convert-qcow2.argv new file mode 100644 index 0000000000..9124f5f27c --- /dev/null +++ b/tests/storagevolxml2argvdata/luks-convert-qcow2.argv @@ -0,0 +1,9 @@ +qemu-img create -f luks \ +--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \ +-o key-secret=OtherDemo.img_encrypt0 \ +/var/lib/libvirt/images/OtherDemo.img 5242880K +qemu-img convert --image-opts -n --target-image-opts \ +--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \ +driver=qcow2,file.filename=/var/lib/libvirt/images/sparse-qcow2.img \ +driver=luks,file.filename=/var/lib/libvirt/images/OtherDemo.img,\ +key-secret=OtherDemo.img_encrypt0 diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index b795f83aee..6a9a080dd1 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -284,6 +284,10 @@ mymain(void) "pool-dir", "vol-file", "luks-convert", 0); + DO_TEST("pool-dir", "vol-luks-convert", + "pool-dir", "vol-file-qcow2", + "luks-convert-qcow2", 0); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/storagevolxml2xmlin/vol-file-qcow2.xml b/tests/storagevolxml2xmlin/vol-file-qcow2.xml new file mode 100644 index 0000000000..025e7e0239 --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-file-qcow2.xml @@ -0,0 +1,21 @@ +<volume> + <name>sparse-qcow2.img</name> + <source/> + <capacity unit="TiB">1</capacity> + <allocation unit="bytes">0</allocation> + <target> + <path>/var/lib/libvirt/images/sparse-qcow2.img</path> + <format type="qcow2"/> + <permissions> + <mode>0</mode> + <owner>0744</owner> + <group>0</group> + <label>virt_image_t</label> + </permissions> + <timestamps> + <atime>1341933637.273190990</atime> + <mtime>1341930622.047245868</mtime> + <ctime>1341930622.047245868</ctime> + </timestamps> + </target> +</volume> -- 2.17.1
On 08/21/2018 06:23 PM, John Ferlan wrote:
Commit 39cef12a9 altered/fixed the inputvol processing to create a multistep process when using an inputvol to create an encrypted output volume; however, it unnecessarily assumed/restricted the inputvol to be of 'raw' format only.
Modify the processing code to allow the inputvol format to be checked and used in order to create the encrypted volume.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 15 +++++++++++-- .../luks-convert-qcow2.argv | 9 ++++++++ tests/storagevolxml2argvtest.c | 4 ++++ tests/storagevolxml2xmlin/vol-file-qcow2.xml | 21 +++++++++++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tests/storagevolxml2argvdata/luks-convert-qcow2.argv create mode 100644 tests/storagevolxml2xmlin/vol-file-qcow2.xml
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index b32e3ccf7d..cc49a5b9f7 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -699,6 +699,7 @@ storagePloopResize(virStorageVolDefPtr vol, struct _virStorageBackendQemuImgInfo { int format; const char *type; + const char *inputType; const char *path; unsigned long long size_arg; unsigned long long allocation; @@ -1021,6 +1022,15 @@ virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool, return -1; }
+ if (inputvol && + !(info->inputType = + virStorageFileFormatTypeToString(inputvol->target.format))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown inputvol storage vol type %d"), + inputvol->target.format); + return -1; + } + if (info->preallocate && info->format != VIR_STORAGE_FILE_QCOW2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("metadata preallocation only available with qcow2")); @@ -1080,6 +1090,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, struct _virStorageBackendQemuImgInfo info = { .format = vol->target.format, .type = NULL, + .inputType = NULL, .path = vol->target.path, .allocation = vol->target.allocation, .encryption = !!vol->target.encryption, @@ -1152,8 +1163,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, virCommandAddArgFormat(cmd, "%lluK", info.size_arg); } else { /* source */ - virCommandAddArgFormat(cmd, "driver=raw,file.filename=%s", - info.inputPath); + virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s", + info.inputType, info.inputPath);
so if inputvol == NULL in virStorageBackendCreateQemuImgSetInfo then info.inputType is also NULL. Don't we need something like: info.inputType ? info.inputType : "raw" (I wish we could use GNU extensions and write is as info.inputType ?: "raw" ) Michal
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@redhat.com> --- 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); + } VIR_FREE(info.secretAlias); + VIR_FREE(inputSecretAlias); return cmd; error: VIR_FREE(info.secretAlias); + VIR_FREE(inputSecretAlias); virCommandFree(cmd); return NULL; } @@ -1261,6 +1297,7 @@ storageBackendDoCreateQemuImg(virStoragePoolObjPtr pool, unsigned int flags, const char *create_tool, const char *secretPath, + const char *inputSecretPath, virStorageVolEncryptConvertStep convertStep) { int ret; @@ -1268,7 +1305,8 @@ storageBackendDoCreateQemuImg(virStoragePoolObjPtr pool, cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol, flags, create_tool, - secretPath, convertStep); + secretPath, inputSecretPath, + convertStep); if (!cmd) return -1; @@ -1289,6 +1327,7 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, int ret = -1; char *create_tool; char *secretPath = NULL; + char *inputSecretPath = NULL; virStorageVolEncryptConvertStep convertStep = VIR_STORAGE_VOL_ENCRYPT_NONE; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); @@ -1305,16 +1344,21 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, !(secretPath = storageBackendCreateQemuImgSecretPath(pool, vol))) goto cleanup; + if (inputvol && inputvol->target.encryption && + !(inputSecretPath = storageBackendCreateQemuImgSecretPath(pool, + inputvol))) + goto cleanup; + /* Using an input file for encryption requires a multi-step process * to create an image of the same size as the inputvol and then to * convert the inputvol afterwards. */ - if (secretPath && inputvol) + if ((secretPath || inputSecretPath) && inputvol) convertStep = VIR_STORAGE_VOL_ENCRYPT_CREATE; do { ret = storageBackendDoCreateQemuImg(pool, vol, inputvol, flags, create_tool, secretPath, - convertStep); + inputSecretPath, convertStep); /* Failure to convert, attempt to delete what we created */ if (ret < 0 && convertStep == VIR_STORAGE_VOL_ENCRYPT_CONVERT) @@ -1336,6 +1380,10 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, unlink(secretPath); VIR_FREE(secretPath); } + if (inputSecretPath) { + unlink(inputSecretPath); + VIR_FREE(inputSecretPath); + } VIR_FREE(create_tool); return ret; } diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index 6fc8e8972c..58b991c772 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -167,6 +167,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, unsigned int flags, const char *create_tool, const char *secretPath, + const char *inputSecretPath, virStorageVolEncryptConvertStep convertStep); int virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt.argv b/tests/storagevolxml2argvdata/luks-convert-encrypt.argv new file mode 100644 index 0000000000..b2ad16b7cb --- /dev/null +++ b/tests/storagevolxml2argvdata/luks-convert-encrypt.argv @@ -0,0 +1,11 @@ +qemu-img create -f luks \ +--object secret,id=encrypt2.img_encrypt0,file=/path/to/secretFile \ +-o key-secret=encrypt2.img_encrypt0 \ +/var/lib/libvirt/images/encrypt2.img 5242880K +qemu-img convert --image-opts -n --target-image-opts \ +--object secret,id=encrypt2.img_encrypt0,file=/path/to/secretFile \ +--object secret,id=encrypt1.img_encrypt0,file=/path/to/inputSecretFile \ +driver=luks,file.filename=/var/lib/libvirt/images/encrypt1.img,\ +key-secret=encrypt1.img_encrypt0 \ +driver=luks,file.filename=/var/lib/libvirt/images/encrypt2.img,\ +key-secret=encrypt2.img_encrypt0 diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv new file mode 100644 index 0000000000..82cb364b61 --- /dev/null +++ b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv @@ -0,0 +1,7 @@ +qemu-img create -f qcow2 \ +-o compat=0.10 /var/lib/libvirt/images/sparse-qcow2.img 1073741824K +qemu-img convert --image-opts -n --target-image-opts \ +--object secret,id=encrypt2.img_encrypt0,file=/path/to/inputSecretFile \ +driver=luks,file.filename=/var/lib/libvirt/images/encrypt2.img,\ +key-secret=encrypt2.img_encrypt0 \ +driver=qcow2,file.filename=/var/lib/libvirt/images/sparse-qcow2.img diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv new file mode 100644 index 0000000000..2661c345a8 --- /dev/null +++ b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv @@ -0,0 +1,7 @@ +qemu-img create -f raw \ +/var/lib/libvirt/images/sparse.img 1073741824K +qemu-img convert --image-opts -n --target-image-opts \ +--object secret,id=encrypt2.img_encrypt0,file=/path/to/inputSecretFile \ +driver=luks,file.filename=/var/lib/libvirt/images/encrypt2.img,\ +key-secret=encrypt2.img_encrypt0 \ +driver=raw,file.filename=/var/lib/libvirt/images/sparse.img diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 6a9a080dd1..105705f348 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -85,7 +85,7 @@ testCompareXMLToArgvFiles(bool shouldFail, * convert the inputvol afterwards. Since we only care about the * command line we have to copy code from storageBackendCreateQemuImg * and adjust it for the test needs. */ - if (inputvol && vol->target.encryption) + if (inputvol && (vol->target.encryption || inputvol->target.encryption)) convertStep = VIR_STORAGE_VOL_ENCRYPT_CREATE; do { @@ -93,6 +93,7 @@ testCompareXMLToArgvFiles(bool shouldFail, inputvol, flags, create_tool, "/path/to/secretFile", + "/path/to/inputSecretFile", convertStep); if (!cmd) { if (shouldFail) { @@ -288,6 +289,18 @@ mymain(void) "pool-dir", "vol-file-qcow2", "luks-convert-qcow2", 0); + DO_TEST("pool-dir", "vol-encrypt2", + "pool-dir", "vol-encrypt1", + "luks-convert-encrypt", 0); + + DO_TEST("pool-dir", "vol-file", + "pool-dir", "vol-encrypt2", + "luks-convert-encrypt2fileraw", 0); + + DO_TEST("pool-dir", "vol-file-qcow2", + "pool-dir", "vol-encrypt2", + "luks-convert-encrypt2fileqcow2", 0); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/storagevolxml2xmlin/vol-encrypt1.xml b/tests/storagevolxml2xmlin/vol-encrypt1.xml new file mode 100644 index 0000000000..681734dc7b --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-encrypt1.xml @@ -0,0 +1,21 @@ +<volume> + <name>encrypt1.img</name> + <key>/var/lib/libvirt/images/encrypt1.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/encrypt1.img</path> + <format type='raw'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='luks'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + </target> +</volume> diff --git a/tests/storagevolxml2xmlin/vol-encrypt2.xml b/tests/storagevolxml2xmlin/vol-encrypt2.xml new file mode 100644 index 0000000000..0507d3b9e6 --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-encrypt2.xml @@ -0,0 +1,21 @@ +<volume> + <name>encrypt2.img</name> + <key>/var/lib/libvirt/images/encrypt2.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/encrypt2.img</path> + <format type='raw'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='luks'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709af480'/> + </encryption> + </target> +</volume> -- 2.17.1
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@redhat.com> --- 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. Michal
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@redhat.com> --- 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. John
On 09/11/2018 04:15 PM, John Ferlan wrote:
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@redhat.com> --- 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.
Yep, it's only inputType that I had in mind.
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.
Okay, Michal
ping? Tks - John On 08/21/2018 12:23 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1613737
Details in the patches (and even more in the bz).
John Ferlan (3): storage: Remove secretPath from _virStorageBackendQemuImgInfo storage: Allow for inputvol to have any format for encryption storage: Allow inputvol to be encrypted
src/storage/storage_util.c | 79 ++++++++++++++++--- src/storage/storage_util.h | 1 + .../luks-convert-encrypt.argv | 11 +++ .../luks-convert-encrypt2fileqcow2.argv | 7 ++ .../luks-convert-encrypt2fileraw.argv | 7 ++ .../luks-convert-qcow2.argv | 9 +++ tests/storagevolxml2argvtest.c | 19 ++++- tests/storagevolxml2xmlin/vol-encrypt1.xml | 21 +++++ tests/storagevolxml2xmlin/vol-encrypt2.xml | 21 +++++ tests/storagevolxml2xmlin/vol-file-qcow2.xml | 21 +++++ 10 files changed, 184 insertions(+), 12 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/storagevolxml2argvdata/luks-convert-qcow2.argv create mode 100644 tests/storagevolxml2xmlin/vol-encrypt1.xml create mode 100644 tests/storagevolxml2xmlin/vol-encrypt2.xml create mode 100644 tests/storagevolxml2xmlin/vol-file-qcow2.xml
ping^2? Tks, John On 08/24/2018 10:27 AM, John Ferlan wrote:
ping?
Tks -
John
On 08/21/2018 12:23 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1613737
Details in the patches (and even more in the bz).
John Ferlan (3): storage: Remove secretPath from _virStorageBackendQemuImgInfo storage: Allow for inputvol to have any format for encryption storage: Allow inputvol to be encrypted
src/storage/storage_util.c | 79 ++++++++++++++++--- src/storage/storage_util.h | 1 + .../luks-convert-encrypt.argv | 11 +++ .../luks-convert-encrypt2fileqcow2.argv | 7 ++ .../luks-convert-encrypt2fileraw.argv | 7 ++ .../luks-convert-qcow2.argv | 9 +++ tests/storagevolxml2argvtest.c | 19 ++++- tests/storagevolxml2xmlin/vol-encrypt1.xml | 21 +++++ tests/storagevolxml2xmlin/vol-encrypt2.xml | 21 +++++ tests/storagevolxml2xmlin/vol-file-qcow2.xml | 21 +++++ 10 files changed, 184 insertions(+), 12 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/storagevolxml2argvdata/luks-convert-qcow2.argv create mode 100644 tests/storagevolxml2xmlin/vol-encrypt1.xml create mode 100644 tests/storagevolxml2xmlin/vol-encrypt2.xml create mode 100644 tests/storagevolxml2xmlin/vol-file-qcow2.xml
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 08/21/2018 06:23 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1613737
Details in the patches (and even more in the bz).
John Ferlan (3): storage: Remove secretPath from _virStorageBackendQemuImgInfo storage: Allow for inputvol to have any format for encryption storage: Allow inputvol to be encrypted
src/storage/storage_util.c | 79 ++++++++++++++++--- src/storage/storage_util.h | 1 + .../luks-convert-encrypt.argv | 11 +++ .../luks-convert-encrypt2fileqcow2.argv | 7 ++ .../luks-convert-encrypt2fileraw.argv | 7 ++ .../luks-convert-qcow2.argv | 9 +++ tests/storagevolxml2argvtest.c | 19 ++++- tests/storagevolxml2xmlin/vol-encrypt1.xml | 21 +++++ tests/storagevolxml2xmlin/vol-encrypt2.xml | 21 +++++ tests/storagevolxml2xmlin/vol-file-qcow2.xml | 21 +++++ 10 files changed, 184 insertions(+), 12 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/storagevolxml2argvdata/luks-convert-qcow2.argv create mode 100644 tests/storagevolxml2xmlin/vol-encrypt1.xml create mode 100644 tests/storagevolxml2xmlin/vol-encrypt2.xml create mode 100644 tests/storagevolxml2xmlin/vol-file-qcow2.xml
ACK bu see my comments before pushing. Michal
participants (2)
-
John Ferlan -
Michal Privoznik