[libvirt PATCH 2/2] storage: add support for qcow2 LUKS encryption

Han Han hhan at redhat.com
Thu Sep 17 09:21:50 UTC 2020


On Thu, Sep 17, 2020 at 1:06 AM Daniel P. Berrangé <berrange at redhat.com>
wrote:

> The storage driver was wired up to support creating raw volumes in LUKS
> format, but was never adapted to support LUKS-in-qcow2. This is trivial
> as it merely requires the encryption properties to be prefixed with
> the "encrypt." prefix, and "encrypt.format=luks" when creating the
> volume.
>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/storage/storage_util.c | 70 +++++++++++++++++++++++++++++---------
>  src/util/virqemu.c         | 23 +++++++++----
>  src/util/virqemu.h         |  1 +
>  3 files changed, 72 insertions(+), 22 deletions(-)
>
> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
> index cf82ea0a87..e5e4fe428f 100644
> --- a/src/storage/storage_util.c
> +++ b/src/storage/storage_util.c
> @@ -707,7 +707,7 @@
> storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr encinfo,
>
>  virStorageFileFormatTypeToString(info->backingFormat));
>
>      if (encinfo)
> -        virQEMUBuildQemuImgKeySecretOpts(&buf, encinfo,
> info->secretAlias);
> +        virQEMUBuildQemuImgKeySecretOpts(&buf, info->format, encinfo,
> info->secretAlias);
>
>      if (info->preallocate) {
>          if (info->size_arg > info->allocation)
> @@ -761,7 +761,8 @@ storageBackendCreateQemuImgCheckEncryption(int format,
>  {
>      virStorageEncryptionPtr enc = vol->target.encryption;
>
> -    if (format == VIR_STORAGE_FILE_RAW) {
> +    if (format == VIR_STORAGE_FILE_RAW ||
> +        format == VIR_STORAGE_FILE_QCOW2) {
>          if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                             _("unsupported volume encryption format %d"),
> @@ -927,21 +928,34 @@
> storageBackendCreateQemuImgSecretObject(virCommandPtr cmd,
>  }
>
>
> -/* Add a --image-opts to the qemu-img resize command line:
> +/* Add a --image-opts to the qemu-img resize command line for use
> + * with encryption:
>   *    --image-opts
> driver=luks,file.filename=$volpath,key-secret=$secretAlias
> + * or
> + *    --image-opts
> driver=qcow2,file.filename=$volpath,encrypt.key-secret=$secretAlias
>   *
> - *    NB: format=raw is assumed
>   */
>  static int
>  storageBackendResizeQemuImgImageOpts(virCommandPtr cmd,
> +                                     int format,
>                                       const char *path,
>                                       const char *secretAlias)
>  {
>      g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>      g_autofree char *commandStr = NULL;
> +    const char *encprefix;
> +    const char *driver;
>
> -    virBufferAsprintf(&buf, "driver=luks,key-secret=%s,file.filename=",
> -                      secretAlias);
> +    if (format == VIR_STORAGE_FILE_QCOW2) {
> +        driver = "qcow2";
> +        encprefix = "encrypt.";
> +    } else {
> +        driver = "luks";
> +        encprefix = "";
> +    }
> +
> +    virBufferAsprintf(&buf, "driver=%s,%skey-secret=%s,file.filename=",
> +                      driver, encprefix, secretAlias);
>      virQEMUBuildBufferEscapeComma(&buf, path);
>
>      commandStr = virBufferContentAndReset(&buf);
> @@ -1006,6 +1020,16 @@
> virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool,
>              return -1;
>          }
>      }
>
storagevolxml2argvtest gets segment fault at this function:
➜  ~ abs_top_srcdir='/root/libvirt-6.7.0' LC_ALL='C'
abs_top_builddir='/root/libvirt-6.7.0/build'
abs_srcdir='/root/libvirt-6.7.0/tests'
abs_builddir='/root/libvirt-6.7.0/build/tests' VIR_TEST_EXPENSIVE='0'
LIBVIRT_AUTOSTART='0'
/root/libvirt-6.7.0/build/tests/storagevolxml2argvtest




TEST: storagevolxml2argvtest
      ...................![1]    916320 segmentation fault (core dumped)
 abs_top_srcdir='/root/libvirt-6.7.0' LC_ALL='C' abs_top_builddir=
abs_srcdir=


Backtrace:
(gdb) bt
#0  0x0000558b997ea149 in virStorageBackendCreateQemuImgSetInfo
(info=<synthetic pointer>, convertStep=VIR_STORAGE_VOL_ENCRYPT_CREATE,
inputvol=0x558b9b32e810, vol=0x558b9b32f840, pool=0x558b9b323b30)

    at ../src/storage/storage_util.c:1025
#1  0x0000558b997ea149 in virStorageBackendCreateQemuImgCmdFromVol
    (pool=0x558b9b323b30, vol=0x558b9b32f840, inputvol=0x558b9b32e810,
flags=0, create_tool=0x558b997f6a00 <create_tool> "qemu-img",
secretPath=0x558b997f6537 "/path/to/secretFile",
inputSecretPath=0x558b997f654b "/path/to/inputSecretFile",
convertStep=VIR_STORAGE_VOL_ENCRYPT_CREATE) at
../src/storage/storage_util.c:1103
#2  0x0000558b997e4b57 in testCompareXMLToArgvFiles
    (parse_flags=<optimized out>, flags=0, cmdline=0x558b9b325030
"/root/libvirt-6.7.0/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv",
inputvolxml=0x558b9b32f7f0
"/root/libvirt-6.7.0/tests/storagevolxml2xmlin/vol-encrypt2.xml",
inputpoolxml=0x558b9b331730
"/root/libvirt-6.7.0/tests/storagepoolxml2xmlin/pool-dir.xml",
volxml=0x558b9b3309f0
"/root/libvirt-6.7.0/tests/storagevolxml2xmlin/vol-file.xml",
poolxml=0x558b9b32a530
"/root/libvirt-6.7.0/tests/storagepoolxml2xmlin/pool-dir.xml",
shouldFail=false) at ../tests/storagevolxml2argvtest.c:92
#3  0x0000558b997e4b57 in testCompareXMLToArgvHelper (data=<optimized out>)
at ../tests/storagevolxml2argvtest.c:174
#4  0x0000558b997e561a in virTestRun (title=0x558b997f6928 "Storage Vol
XML-2-argv luks-convert-encrypt2fileraw", body=0x558b997e4930
<testCompareXMLToArgvHelper>, data=0x7ffea16a5870) at
../tests/testutils.c:142
#5  0x0000558b997e489a in mymain () at ../tests/storagevolxml2argvtest.c:271
#6  0x0000558b997e6512 in virTestMain (argc=1, argv=0x7ffea16a5aa8,
func=0x558b997e40d0 <mymain>) at ../tests/testutils.c:841


#7  0x00007f2597f5b7b3 in __libc_start_main (main=0x558b997e3fd0 <main>,
argc=1, argv=0x7ffea16a5aa8, init=<optimized out>, fini=<optimized out>,
rtld_fini=<optimized out>, stack_end=0x7ffea16a5a98) at
../csu/libc-start.c:308
#8  0x0000558b997e400e in _start () at
../tests/storagevolxml2argvtest.c:282

> +    if (inputvol && inputvol->target.format == VIR_STORAGE_FILE_RAW &&
> +        inputvol->target.encryption) {
> +        if (vol->target.encryption->format ==
> VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
> +            info->type = "luks";
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Only luks encryption is supported for raw
> files"));
> +            return -1;
> +        }
> +    }
>
>      if (inputvol &&
>          storageBackendCreateQemuImgSetInput(inputvol, convertStep, info)
> < 0)
> @@ -1056,6 +1080,8 @@
> virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
>      virStorageEncryptionPtr inputenc = inputvol ?
> inputvol->target.encryption : NULL;
>      virStorageEncryptionInfoDefPtr encinfo = NULL;
>      g_autofree char *inputSecretAlias = NULL;
> +    const char *encprefix;
> +    const char *inputencprefix;
>
>      virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
>
> @@ -1134,24 +1160,34 @@
> virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
>              virCommandAddArgFormat(cmd, "%lluK", info.size_arg);
>      } else {
>          /* source */
> -        if (inputenc)
> +        if (inputenc) {
> +            if (inputvol->target.format == VIR_STORAGE_FILE_QCOW2)
> +                inputencprefix = "encrypt.";
> +            else
> +                inputencprefix = "";
>              virCommandAddArgFormat(cmd,
> -
>  "driver=luks,file.filename=%s,key-secret=%s",
> -                                   info.inputPath, inputSecretAlias);
> -        else
> +
>  "driver=%s,file.filename=%s,%skey-secret=%s",
> +                                   info.inputType, info.inputPath,
> inputencprefix, inputSecretAlias);
> +        } else {
>              virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s",
>                                     info.inputType ? info.inputType :
> "raw",
>                                     info.inputPath);
> +        }
>
>          /* dest */
> -        if (enc)
> +        if (enc) {
> +            if (vol->target.format == VIR_STORAGE_FILE_QCOW2)
> +                encprefix = "encrypt.";
> +            else
> +                encprefix = "";
> +
>              virCommandAddArgFormat(cmd,
> -
>  "driver=%s,file.filename=%s,key-secret=%s",
> -                                   info.type, info.path,
> info.secretAlias);
> -        else
> +
>  "driver=%s,file.filename=%s,%skey-secret=%s",
> +                                   info.type, info.path, encprefix,
> info.secretAlias);
> +        } else {
>              virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s",
>                                     info.type, info.path);
> -
> +        }
>      }
>      VIR_FREE(info.secretAlias);
>
> @@ -2276,7 +2312,9 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr
> pool,
>                                                      secretAlias) < 0)
>              goto cleanup;
>
> -        if (storageBackendResizeQemuImgImageOpts(cmd, vol->target.path,
> +        if (storageBackendResizeQemuImgImageOpts(cmd,
> +                                                 vol->target.format,
> +                                                 vol->target.path,
>                                                   secretAlias) < 0)
>              goto cleanup;
>      }
> diff --git a/src/util/virqemu.c b/src/util/virqemu.c
> index 25d6fd35c5..bbb38eed75 100644
> --- a/src/util/virqemu.c
> +++ b/src/util/virqemu.c
> @@ -28,6 +28,7 @@
>  #include "virqemu.h"
>  #include "virstring.h"
>  #include "viralloc.h"
> +#include "virstoragefile.h"
>
>  #define VIR_FROM_THIS VIR_FROM_NONE
>
> @@ -407,36 +408,46 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf,
> const char *str)
>   */
>  void
>  virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
> +                                 int format,
>                                   virStorageEncryptionInfoDefPtr encinfo,
>                                   const char *alias)
>  {
> -    virBufferAsprintf(buf, "key-secret=%s,", alias);
> +    const char *encprefix;
> +
> +    if (format == VIR_STORAGE_FILE_QCOW2) {
> +        virBufferAsprintf(buf, "encrypt.format=luks,");
>
syntax check is failed here:
--- command ---
09:13:21 /usr/bin/make -C /root/libvirt-6.7.0/build/build-aux
sc_prohibit_virBufferAsprintf_with_string_literal
--- stdout ---
make: Entering directory '/root/libvirt-6.7.0/build/build-aux'
prohibit_virBufferAsprintf_with_string_literal
/root/libvirt-6.7.0/src/util/virqemu.c:418:        virBufferAsprintf(buf,
"encrypt.format=luks,");


> +        encprefix = "encrypt.";
> +    } else {
> +        encprefix = "";
> +    }
> +
> +    virBufferAsprintf(buf, "%skey-secret=%s,", encprefix, alias);
>
>      if (!encinfo->cipher_name)
>          return;
>
> -    virBufferAddLit(buf, "cipher-alg=");
> +    virBufferAsprintf(buf, "%scipher-alg=", encprefix);
>      virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_name);
>      virBufferAsprintf(buf, "-%u,", encinfo->cipher_size);
>      if (encinfo->cipher_mode) {
> -        virBufferAddLit(buf, "cipher-mode=");
> +        virBufferAsprintf(buf, "%scipher-mode=", encprefix);
>          virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_mode);
>          virBufferAddLit(buf, ",");
>      }
>      if (encinfo->cipher_hash) {
> -        virBufferAddLit(buf, "hash-alg=");
> +        virBufferAsprintf(buf, "%shash-alg=", encprefix);
>          virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_hash);
>          virBufferAddLit(buf, ",");
>      }
>      if (!encinfo->ivgen_name)
>          return;
>
> -    virBufferAddLit(buf, "ivgen-alg=");
> +    virBufferAsprintf(buf, "%sivgen-alg=", encprefix);
>      virQEMUBuildBufferEscapeComma(buf, encinfo->ivgen_name);
>      virBufferAddLit(buf, ",");
>
>      if (encinfo->ivgen_hash) {
> -        virBufferAddLit(buf, "ivgen-hash-alg=");
> +        virBufferAsprintf(buf, "%sivgen-hash-alg=", encprefix);
>          virQEMUBuildBufferEscapeComma(buf, encinfo->ivgen_hash);
>          virBufferAddLit(buf, ",");
>      }
> diff --git a/src/util/virqemu.h b/src/util/virqemu.h
> index b1296cb657..be14c04d51 100644
> --- a/src/util/virqemu.h
> +++ b/src/util/virqemu.h
> @@ -60,6 +60,7 @@ char
> *virQEMUBuildDriveCommandlineFromJSON(virJSONValuePtr src);
>
>  void virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str);
>  void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
> +                                      int format,
>                                        virStorageEncryptionInfoDefPtr enc,
>                                        const char *alias)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> --
> 2.26.2
>
>

-- 
Reviewed-by: Han Han <hhan at redhat.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200917/95b9e866/attachment-0001.htm>


More information about the libvir-list mailing list