Changeset
docs/formatstorageencryption.html.in               |  10 ++
docs/news.xml                                      |  12 ++
src/storage/storage_util.c                         | 159 +++++++++++++++------
src/storage/storage_util.h                         |  10 +-
src/util/virqemu.c                                 |  10 +-
src/util/virqemu.h                                 |   3 +-
tests/storagevolxml2argvdata/qcow2-1.1.argv        |   4 +-
tests/storagevolxml2argvdata/qcow2-compat.argv     |   4 +-
.../qcow2-from-logical-compat.argv                 |  12 +-
tests/storagevolxml2argvdata/qcow2-lazy.argv       |   6 +-
.../qcow2-nobacking-convert-prealloc-compat.argv   |  13 +-
.../qcow2-nobacking-prealloc-compat.argv           |   4 +-
.../qcow2-nocapacity-convert-prealloc.argv         |  14 +-
tests/storagevolxml2argvdata/qcow2-nocapacity.argv |   4 +-
.../storagevolxml2argvdata/qcow2-nocow-compat.argv |   6 +-
tests/storagevolxml2argvdata/qcow2-nocow.argv      |   3 +-
tests/storagevolxml2argvtest.c                     |  61 ++++++--
17 files changed, 255 insertions(+), 80 deletions(-)
Git apply log
Switched to a new branch '20180516122931.30854-1-jferlan@redhat.com'
Applying: storage_util: Alter qemu storage encryption arguments
Applying: storage_util: Fix qemu qcow[2] encryption convert processing
Applying: storage_util: Don't assume "luks" encryption for resize
Applying: docs: Update news.xml to describe encrypted image issues
To https://github.com/patchew-project/libvirt
 * [new tag]         patchew/20180516122931.30854-1-jferlan@redhat.com -> patchew/20180516122931.30854-1-jferlan@redhat.com
Test passed: syntax-check

loading

[libvirt] [PATCH v2 0/4] Fix qemu-img qcow[2] encrypted image handling
Posted by John Ferlan, 6 days ago
v1: https://www.redhat.com/archives/libvir-list/2018-April/msg01578.html

Changes since v1... Rather than disallow, alter the code in order to
handle the creation, conversion, and resize of qcow[2] encrypted images.
See conversion processing description from review:

https://www.redhat.com/archives/libvir-list/2018-April/msg02037.html

John Ferlan (4):
  storage_util: Alter qemu storage encryption arguments
  storage_util: Fix qemu qcow[2] encryption convert processing
  storage_util: Don't assume "luks" encryption for resize
  docs: Update news.xml to describe encrypted image issues

 docs/formatstorageencryption.html.in               |  10 ++
 docs/news.xml                                      |  12 ++
 src/storage/storage_util.c                         | 159 +++++++++++++++------
 src/storage/storage_util.h                         |  10 +-
 src/util/virqemu.c                                 |  10 +-
 src/util/virqemu.h                                 |   3 +-
 tests/storagevolxml2argvdata/qcow2-1.1.argv        |   4 +-
 tests/storagevolxml2argvdata/qcow2-compat.argv     |   4 +-
 .../qcow2-from-logical-compat.argv                 |  12 +-
 tests/storagevolxml2argvdata/qcow2-lazy.argv       |   6 +-
 .../qcow2-nobacking-convert-prealloc-compat.argv   |  13 +-
 .../qcow2-nobacking-prealloc-compat.argv           |   4 +-
 .../qcow2-nocapacity-convert-prealloc.argv         |  14 +-
 tests/storagevolxml2argvdata/qcow2-nocapacity.argv |   4 +-
 .../storagevolxml2argvdata/qcow2-nocow-compat.argv |   6 +-
 tests/storagevolxml2argvdata/qcow2-nocow.argv      |   3 +-
 tests/storagevolxml2argvtest.c                     |  61 ++++++--
 17 files changed, 255 insertions(+), 80 deletions(-)

-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] storage_util: Alter qemu storage encryption arguments
Posted by John Ferlan, 6 days ago
https://bugzilla.redhat.com/show_bug.cgi?id=1526382

As of QEMU 2.9, qemu-img has enforced using the "encrypt.key-secret"
in order to create a qcow[2] encrypted volume. Thus, the existing code
to create an encrypted volume using qcow[2] encryption techniques will
fail, such as :

  $ qemu-img create -f qcow2 -b /dev/null \
        -o backing_fmt=raw,encryption=on \
        demo.tmp 5242880K
  Formatting 'demo.tmp', fmt=qcow2 size=5368709120 backing_file=/dev/null
  backing_fmt=raw encryption=on cluster_size=65536 lazy_refcounts=off
  refcount_bits=16
  qemu-img: demo.tmp: Parameter 'encrypt.key-secret' is required for cipher
  $

This patch will resolve this by adding the correct parameters for
the creation. The new format of parameters roughly follows that of
LUKS encryption model with a few minor differences:

   1. Usage of "encrypt.key-secret=$alias" instead of just plain
      "key-secret=$alias" as the parameter.
   2. Usage of "encrypt.format=aes" instead of "encryption=on"

The result is the following command syntax for the same example:

  $ qemu-img create -f qcow2 -b /dev/null \
        --object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
        -o encrypt.format=aes,encrypt.key-secret=OtherDemo.img_encrypt0 \
        demo.tmp 5242880K
  Formatting 'test.img', fmt=qcow2 size=5368709120 backing_file=/dev/null
  backing_fmt=raw encrypt.format=aes encrypt.key-secret=sec0 cluster_size=65536
  lazy_refcounts=off refcount_bits=16
  $

Thus this patch removes the LUKS specific checks in a few places and
alters the algorithms as necessary in order to allow either form of
encryption.

The storagevolxml2argvtest.c test is adjusted to pass a dummy path to
the secret file and the outputs adjusted to illustrate the new format
for the various arguments.

This patch requires usage of the secrets object and model. There is no
plan for backwards compatibility for qcow[2] encryption. The desire is
to move towards usage of LUKS encryption anyway.

NB: Although the qemu-img convert examples change in the test output,
they are essentially still broken (they wouldn't work before this patch
either for the same reasons create fails). A follow-up patch will alter
the algorithm and syntax.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/storage/storage_util.c                         | 24 +++++++++++-----------
 src/util/virqemu.c                                 | 10 +++++++--
 src/util/virqemu.h                                 |  3 ++-
 tests/storagevolxml2argvdata/qcow2-1.1.argv        |  4 +++-
 tests/storagevolxml2argvdata/qcow2-compat.argv     |  4 +++-
 .../qcow2-from-logical-compat.argv                 |  3 ++-
 tests/storagevolxml2argvdata/qcow2-lazy.argv       |  6 ++++--
 .../qcow2-nobacking-convert-prealloc-compat.argv   |  4 +++-
 .../qcow2-nobacking-prealloc-compat.argv           |  4 +++-
 .../qcow2-nocapacity-convert-prealloc.argv         |  7 ++++---
 tests/storagevolxml2argvdata/qcow2-nocapacity.argv |  4 +++-
 .../storagevolxml2argvdata/qcow2-nocow-compat.argv |  6 ++++--
 tests/storagevolxml2argvdata/qcow2-nocow.argv      |  3 ++-
 tests/storagevolxml2argvtest.c                     |  2 +-
 14 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 554fc757ed..a8a6a3e401 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -827,11 +827,10 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
         virBufferAsprintf(&buf, "backing_fmt=%s,",
                           virStorageFileFormatTypeToString(info.backingFormat));
 
-    if (info.format == VIR_STORAGE_FILE_RAW && enc) {
-        virQEMUBuildQemuImgKeySecretOpts(&buf, enc, info.secretAlias);
-    } else {
-        if (info.encryption)
-            virBufferAddLit(&buf, "encryption=on,");
+    if (enc) {
+        bool qcow = (info.format == VIR_STORAGE_FILE_QCOW ||
+                     info.format == VIR_STORAGE_FILE_QCOW2);
+        virQEMUBuildQemuImgKeySecretOpts(&buf, enc, info.secretAlias, qcow);
     }
 
     if (info.preallocate) {
@@ -1231,8 +1230,12 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
     if (info.backingPath)
         virCommandAddArgList(cmd, "-b", info.backingPath, NULL);
 
-    if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption &&
-        vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
+    if (vol->target.encryption) {
+        if (!secretPath) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("path to secret data file is required"));
+            return NULL;
+        }
         if (virAsprintf(&info.secretAlias, "%s_encrypt0", vol->name) < 0)
             goto error;
         if (storageBackendCreateQemuImgSecretObject(cmd, info.secretPath,
@@ -1344,11 +1347,8 @@ storageBackendGenerateSecretData(virStoragePoolObjPtr pool,
             return -1;
     }
 
-    if (vol->target.format == VIR_STORAGE_FILE_RAW &&
-        enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
-        if (!(*secretPath = storageBackendCreateQemuImgSecretPath(pool, vol)))
-            return -1;
-    }
+    if (!(*secretPath = storageBackendCreateQemuImgSecretPath(pool, vol)))
+        return -1;
 
     return 0;
 }
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index 04cd71605e..b20d09d945 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -307,6 +307,7 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str)
  * @buf: buffer to build the string into
  * @enc: pointer to encryption info
  * @alias: alias to use
+ * @qcow: using qcow encryption
  *
  * Generate the string for id=$alias and any encryption options for
  * into the buffer.
@@ -315,7 +316,8 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str)
  * it's expected other arguments are appended after the id=$alias string.
  * So either turn something like:
  *
- *     "key-secret=$alias,"
+ *     "key-secret=$alias," or
+ *     "encrypt.format=aes,encrypt.key-secret=$alias,"
  *
  * or
  *     "key-secret=$alias,cipher-alg=twofish-256,cipher-mode=cbc,
@@ -325,8 +327,12 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str)
 void
 virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
                                  virStorageEncryptionInfoDefPtr enc,
-                                 const char *alias)
+                                 const char *alias,
+                                 bool qcow)
 {
+    if (qcow)
+        virBufferAddLit(buf, "encrypt.format=aes,encrypt.");
+
     virBufferAsprintf(buf, "key-secret=%s,", alias);
 
     if (!enc->cipher_name)
diff --git a/src/util/virqemu.h b/src/util/virqemu.h
index 2599481753..9a01640c6e 100644
--- a/src/util/virqemu.h
+++ b/src/util/virqemu.h
@@ -52,7 +52,8 @@ char *virQEMUBuildDriveCommandlineFromJSON(virJSONValuePtr src);
 void virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str);
 void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
                                       virStorageEncryptionInfoDefPtr enc,
-                                      const char *alias)
+                                      const char *alias,
+                                      bool qcow)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 
 #endif /* __VIR_QEMU_H_ */
diff --git a/tests/storagevolxml2argvdata/qcow2-1.1.argv b/tests/storagevolxml2argvdata/qcow2-1.1.argv
index c4dcb1bc3c..ff3d62d0a1 100644
--- a/tests/storagevolxml2argvdata/qcow2-1.1.argv
+++ b/tests/storagevolxml2argvdata/qcow2-1.1.argv
@@ -1,3 +1,5 @@
 qemu-img create -f qcow2 -b /dev/null \
--o backing_fmt=raw,encryption=on,compat=1.1 \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o backing_fmt=raw,encrypt.format=aes,\
+encrypt.key-secret=OtherDemo.img_encrypt0,compat=1.1 \
 /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvdata/qcow2-compat.argv b/tests/storagevolxml2argvdata/qcow2-compat.argv
index 37ad2c078d..8aa8c7ce84 100644
--- a/tests/storagevolxml2argvdata/qcow2-compat.argv
+++ b/tests/storagevolxml2argvdata/qcow2-compat.argv
@@ -1,3 +1,5 @@
 qemu-img create -f qcow2 -b /dev/null \
--o backing_fmt=raw,encryption=on,compat=0.10 \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o backing_fmt=raw,encrypt.format=aes,\
+encrypt.key-secret=OtherDemo.img_encrypt0,compat=0.10 \
 /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv
index 5f365b1f84..849c5f0218 100644
--- a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv
+++ b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv
@@ -1,3 +1,4 @@
 qemu-img convert -f raw -O qcow2 \
--o encryption=on,compat=0.10 \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o encrypt.format=aes,encrypt.key-secret=OtherDemo.img_encrypt0,compat=0.10 \
 /dev/HostVG/Swap /var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-lazy.argv b/tests/storagevolxml2argvdata/qcow2-lazy.argv
index b7058b84cc..0c29a3fb33 100644
--- a/tests/storagevolxml2argvdata/qcow2-lazy.argv
+++ b/tests/storagevolxml2argvdata/qcow2-lazy.argv
@@ -1,3 +1,5 @@
 qemu-img create -f qcow2 -b /dev/null \
--o backing_fmt=raw,encryption=on,compat=1.1,lazy_refcounts \
-/var/lib/libvirt/images/OtherDemo.img 5242880K
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o backing_fmt=raw,encrypt.format=aes,\
+encrypt.key-secret=OtherDemo.img_encrypt0,compat=1.1,\
+lazy_refcounts /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv
index 3d93ec8480..a95749eafa 100644
--- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv
@@ -1,3 +1,5 @@
 qemu-img convert -f raw -O qcow2 \
--o encryption=on,preallocation=metadata,compat=0.10 \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o encrypt.format=aes,encrypt.key-secret=OtherDemo.img_encrypt0,\
+preallocation=metadata,compat=0.10 \
 /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv
index 903c94e33d..30b61442a4 100644
--- a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv
@@ -1,3 +1,5 @@
 qemu-img create -f qcow2 \
--o encryption=on,preallocation=metadata,compat=0.10 \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o encrypt.format=aes,encrypt.key-secret=OtherDemo.img_encrypt0,\
+preallocation=metadata,compat=0.10 \
 /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
index 73499178e7..51bdaaf684 100644
--- a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
@@ -1,4 +1,5 @@
 qemu-img convert -f raw -O qcow2 \
--o encryption=on,preallocation=falloc,compat=0.10 \
-/var/lib/libvirt/images/sparse.img \
-/var/lib/libvirt/images/OtherDemo.img
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o encrypt.format=aes,encrypt.key-secret=OtherDemo.img_encrypt0,\
+preallocation=falloc,compat=0.10 \
+/var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity.argv b/tests/storagevolxml2argvdata/qcow2-nocapacity.argv
index fd88055890..920cff8771 100644
--- a/tests/storagevolxml2argvdata/qcow2-nocapacity.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nocapacity.argv
@@ -1,5 +1,7 @@
 qemu-img create \
 -f qcow2 \
 -b /dev/null \
--o backing_fmt=raw,encryption=on,compat=0.10 \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o backing_fmt=raw,encrypt.format=aes,\
+encrypt.key-secret=OtherDemo.img_encrypt0,compat=0.10 \
 /var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv b/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv
index d5a7547011..1c9a1a4da4 100644
--- a/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv
@@ -1,3 +1,5 @@
 qemu-img create -f qcow2 -b /dev/null \
--o backing_fmt=raw,encryption=on,nocow=on,compat=0.10 \
-/var/lib/libvirt/images/OtherDemo.img 5242880K
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o backing_fmt=raw,encrypt.format=aes,\
+encrypt.key-secret=OtherDemo.img_encrypt0,nocow=on,\
+compat=0.10 /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvdata/qcow2-nocow.argv b/tests/storagevolxml2argvdata/qcow2-nocow.argv
index e54801c78a..68c16f8e20 100644
--- a/tests/storagevolxml2argvdata/qcow2-nocow.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nocow.argv
@@ -1,3 +1,4 @@
 qemu-img create -f qcow2 -b /dev/null \
--o backing_fmt=raw,encryption=on,nocow=on \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+-o encrypt.format=aes,encrypt.key-secret=OtherDemo.img_encrypt0,nocow=on \
 /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 0265a0ffe2..4286c50c6e 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -82,7 +82,7 @@ testCompareXMLToArgvFiles(bool shouldFail,
     cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol,
                                                    inputvol, flags,
                                                    create_tool,
-                                                   NULL);
+                                                   "/path/to/secretFile");
     if (!cmd) {
         if (shouldFail) {
             virResetLastError();
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/4] storage_util: Fix qemu qcow[2] encryption convert processing
Posted by John Ferlan, 6 days ago
As with qcow[2] encryption create processing, the convert processing
requires usage of the "encrypt.key-secret" option and secret objects
for converting an input volume to use qcow[2] encryption. Assuming
an input file sparse.img exists (e.g. qemu-img create -f raw sparse 500K):

$ qemu-img convert -f raw -O qcow2 -o encryption=on sparse.img demo.img
qemu-img: demo.img: error while converting qcow2: Parameter
'encrypt.key-secret' is required for cipher
$

Unlike create processing, the convert processing cannot be done in
one command option, such as:

$ qemu-img convert -f raw -O qcow2 \
    --object secret,id=demo.img_encrypt0,file=/path/to/secretFile \
    -o encrypt.format=aes,encrypt.key-secret=demo.img_encrypt0 \
    sparse.img demo.img
qemu-img: Could not open 'demo.img': Parameter 'encrypt.key-secret' is
required for cipher
$

What convert processing requires is a two step process which first creates
the object using the sizing parameters from the input source and then uses
the --image-opts, -n, and --target-image-opts options along with inline
driver options to describe the input and output files, thus resulting in:

$ qemu-img create -f qcow2 \
    --object secret,id=demo.img_encrypt0,file=/path/to/secretFile \
    -o encrypt.format=aes,encrypt.key-secret=demo.img_encrypt0 \
    demo.img 500K
Formatting 'demo.img', fmt=qcow2 size=512000 encrypt.format=aes
encrypt.key-secret=sec0 cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ qemu-img convert --image-opts -n --target-image-opts \
    --object secret,id=demo.img_encrypt0,file=/path/to/secretFile \
    driver=raw,file.filename=sparse.img \
    driver=qcow2,file.filename=demo.img,encrypt.key-secret=demo.img_encrypt0
$

Similar processing would be used for LUKS encryption, except the
"encrypt.format=aes" is not provided and the "encrypt.key-secret"
is only "key-secret", e.g.:

$ qemu-img create -f luks \
    --object secret,id=demo.img_encrypt0,file=/path/to/secretFile \
    -o key-secret=demo.img_encrypt0 \
    demo.img 500K
Formatting 'demo.img', fmt=luks size=512000 key-secret=demo.img_encrypt0
$ qemu-img convert --image-opts -n --target-image-opts \
    --object secret,id=demo.img_encrypt0,file=/path/to/secretFile \
    driver=raw,file.filename=sparse.img \
    driver=luks,file.filename=demo.img,key-secret=demo.img_encrypt0
$

This patch handles the convert processing by running the processing
in a do..while loop essentially reusing the existing create logic and
arguments to create the target vol from the inputvol and then converting
the inputvol using new arguments.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 docs/formatstorageencryption.html.in               |  10 ++
 src/storage/storage_util.c                         | 113 ++++++++++++++++-----
 src/storage/storage_util.h                         |  10 +-
 .../qcow2-from-logical-compat.argv                 |   9 +-
 .../qcow2-nobacking-convert-prealloc-compat.argv   |   9 +-
 .../qcow2-nocapacity-convert-prealloc.argv         |   9 +-
 tests/storagevolxml2argvtest.c                     |  61 ++++++++---
 7 files changed, 178 insertions(+), 43 deletions(-)

diff --git a/docs/formatstorageencryption.html.in b/docs/formatstorageencryption.html.in
index 23efbf932e..984c7d8b8b 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -38,6 +38,16 @@
       secret value at the time of volume creation, and store it using the
       specified <code>uuid</code>.
     </p>
+    <p>
+      <span class="since">Since 4.4.0,</span> the command line generated
+      by libvirt to create a <code>default</code> or <code>qcow</code>
+      encrypted volume has changed. This is a result of changes made
+      to qemu-img in QEMU 2.9 which requires different arguments to be
+      provided in order to create a qcow encrypted volume. This change
+      is not compatible with older qemu-img images and there is no plan
+      to provide backwards compatibility. It is <b>strongly recommended</b>
+      to use the "luks" encryption format.
+    </p>
     <h3><a id="StorageEncryptionDefault">"default" format</a></h3>
     <p>
       <code>&lt;encryption format="default"/&gt;</code> can be specified only
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index a8a6a3e401..29adf0cdbe 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -943,12 +943,15 @@ storageBackendCreateQemuImgCheckEncryption(int format,
 
 static int
 storageBackendCreateQemuImgSetInput(virStorageVolDefPtr inputvol,
+                                    virStorageVolEncryptConvertStep convertStep,
                                     struct _virStorageBackendQemuImgInfo *info)
 {
-    if (!(info->inputPath = inputvol->target.path)) {
-        virReportError(VIR_ERR_INVALID_ARG, "%s",
-                       _("missing input volume target path"));
-        return -1;
+    if (convertStep != VIR_STORAGE_VOL_ENCRYPT_CREATE) {
+        if (!(info->inputPath = inputvol->target.path)) {
+            virReportError(VIR_ERR_INVALID_ARG, "%s",
+                           _("missing input volume target path"));
+            return -1;
+        }
     }
 
     info->inputFormat = inputvol->target.format;
@@ -1119,6 +1122,7 @@ static int
 virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool,
                                       virStorageVolDefPtr vol,
                                       virStorageVolDefPtr inputvol,
+                                      virStorageVolEncryptConvertStep convertStep,
                                       struct _virStorageBackendQemuImgInfo *info)
 {
     /* Treat output block devices as 'raw' format */
@@ -1166,7 +1170,7 @@ virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool,
     }
 
     if (inputvol &&
-        storageBackendCreateQemuImgSetInput(inputvol, info) < 0)
+        storageBackendCreateQemuImgSetInput(inputvol, convertStep, info) < 0)
         return -1;
 
     if (virStorageSourceHasBacking(&vol->target) &&
@@ -1185,6 +1189,27 @@ virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool,
 }
 
 
+static void
+virStorageBackendCreateQemuImgCmdEncryptConvert(virCommandPtr cmd,
+                                                virStorageEncryptionPtr enc,
+                                                struct _virStorageBackendQemuImgInfo info)
+{
+    /* source */
+    virCommandAddArgFormat(cmd, "driver=raw,file.filename=%s", info.inputPath);
+
+    /* dest */
+    if (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
+        virCommandAddArgFormat(cmd,
+                               "driver=luks,file.filename=%s,key-secret=%s",
+                               info.path, info.secretAlias);
+    } else {
+        virCommandAddArgFormat(cmd,
+                               "driver=qcow2,file.filename=%s,encrypt.key-secret=%s",
+                               info.path, info.secretAlias);
+    }
+}
+
+
 /* Create a qemu-img virCommand from the supplied arguments */
 virCommandPtr
 virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
@@ -1192,7 +1217,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
                                          virStorageVolDefPtr inputvol,
                                          unsigned int flags,
                                          const char *create_tool,
-                                         const char *secretPath)
+                                         const char *secretPath,
+                                         virStorageVolEncryptConvertStep convertStep)
 {
     virCommandPtr cmd = NULL;
     struct _virStorageBackendQemuImgInfo info = {
@@ -1208,22 +1234,30 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
         .secretPath = secretPath,
         .secretAlias = NULL,
     };
-    virStorageEncryptionInfoDefPtr enc = NULL;
+    virStorageEncryptionPtr enc = NULL;
+    virStorageEncryptionInfoDefPtr encinfo = NULL;
 
     virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
 
-    if (virStorageBackendCreateQemuImgSetInfo(pool, vol, inputvol, &info) < 0)
+    if (virStorageBackendCreateQemuImgSetInfo(pool, vol, inputvol,
+                                              convertStep, &info) < 0)
         goto error;
 
     cmd = virCommandNew(create_tool);
 
-    /* ignore the backing volume when we're converting a volume */
-    if (info.inputPath)
+    /* ignore the backing volume when we're converting a volume
+     * including when we're doing a two step convert during create */
+    if (info.inputPath || convertStep == VIR_STORAGE_VOL_ENCRYPT_CREATE)
         info.backingPath = NULL;
 
-    if (info.inputPath)
+    /* Converting to use encryption is a two step process - step 1 is to
+     * create the image and step 2 is to convert it using special arguments */
+    if (info.inputPath && convertStep == VIR_STORAGE_VOL_ENCRYPT_NONE)
         virCommandAddArgList(cmd, "convert", "-f", info.inputFormatStr,
                              "-O", info.type, NULL);
+    else if (info.inputPath && convertStep == VIR_STORAGE_VOL_ENCRYPT_CONVERT)
+        virCommandAddArgList(cmd, "convert", "--image-opts", "-n",
+                             "--target-image-opts", NULL);
     else
         virCommandAddArgList(cmd, "create", "-f", info.type, NULL);
 
@@ -1241,19 +1275,23 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
         if (storageBackendCreateQemuImgSecretObject(cmd, info.secretPath,
                                                     info.secretAlias) < 0)
             goto error;
-        enc = &vol->target.encryption->encinfo;
+        enc = vol->target.encryption;
+        encinfo = &enc->encinfo;
     }
 
-    if (storageBackendCreateQemuImgSetOptions(cmd, enc, info) < 0)
-        goto error;
+    if (convertStep != VIR_STORAGE_VOL_ENCRYPT_CONVERT) {
+        if (storageBackendCreateQemuImgSetOptions(cmd, encinfo, info) < 0)
+            goto error;
+        if (info.inputPath)
+            virCommandAddArg(cmd, info.inputPath);
+        virCommandAddArg(cmd, info.path);
+        if (!info.inputPath && (info.size_arg || !info.backingPath))
+            virCommandAddArgFormat(cmd, "%lluK", info.size_arg);
+    } else {
+        virStorageBackendCreateQemuImgCmdEncryptConvert(cmd, enc, info);
+    }
     VIR_FREE(info.secretAlias);
 
-    if (info.inputPath)
-        virCommandAddArg(cmd, info.inputPath);
-    virCommandAddArg(cmd, info.path);
-    if (!info.inputPath && (info.size_arg || !info.backingPath))
-        virCommandAddArgFormat(cmd, "%lluK", info.size_arg);
-
     return cmd;
 
  error:
@@ -1360,14 +1398,15 @@ storageBackendDoCreateQemuImg(virStoragePoolObjPtr pool,
                               virStorageVolDefPtr inputvol,
                               unsigned int flags,
                               const char *create_tool,
-                              const char *secretPath)
+                              const char *secretPath,
+                              virStorageVolEncryptConvertStep convertStep)
 {
     int ret;
     virCommandPtr cmd;
 
     cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol,
                                                    flags, create_tool,
-                                                   secretPath);
+                                                   secretPath, convertStep);
     if (!cmd)
         return -1;
 
@@ -1388,6 +1427,7 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
     int ret = -1;
     char *create_tool;
     char *secretPath = NULL;
+    virStorageVolEncryptConvertStep convertStep = VIR_STORAGE_VOL_ENCRYPT_NONE;
 
     virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1);
 
@@ -1402,8 +1442,33 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
     if (storageBackendGenerateSecretData(pool, vol, &secretPath) < 0)
         goto cleanup;
 
-    ret = storageBackendDoCreateQemuImg(pool, vol, inputvol, flags,
-                                        create_tool, secretPath);
+    /* 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)
+        convertStep = VIR_STORAGE_VOL_ENCRYPT_CREATE;
+
+    do {
+        ret = storageBackendDoCreateQemuImg(pool, vol, inputvol, flags,
+                                            create_tool, secretPath,
+                                            convertStep);
+
+        /* Failure to convert, attempt to delete what we created */
+        if (ret < 0 && convertStep == VIR_STORAGE_VOL_ENCRYPT_CONVERT)
+            ignore_value(virFileRemove(vol->target.path,
+                                       vol->target.perms->uid,
+                                       vol->target.perms->gid));
+
+        if (ret < 0 || convertStep == VIR_STORAGE_VOL_ENCRYPT_NONE)
+            goto cleanup;
+
+        if (convertStep == VIR_STORAGE_VOL_ENCRYPT_CREATE)
+            convertStep = VIR_STORAGE_VOL_ENCRYPT_CONVERT;
+        else if (convertStep == VIR_STORAGE_VOL_ENCRYPT_CONVERT)
+            convertStep = VIR_STORAGE_VOL_ENCRYPT_DONE;
+    } while (convertStep != VIR_STORAGE_VOL_ENCRYPT_DONE);
+
+
  cleanup:
     if (secretPath) {
         unlink(secretPath);
diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h
index 9307702754..6fc8e8972c 100644
--- a/src/storage/storage_util.h
+++ b/src/storage/storage_util.h
@@ -153,13 +153,21 @@ char *virStorageBackendStablePath(virStoragePoolObjPtr pool,
                                   const char *devpath,
                                   bool loop);
 
+typedef enum {
+    VIR_STORAGE_VOL_ENCRYPT_NONE = 0,
+    VIR_STORAGE_VOL_ENCRYPT_CREATE,
+    VIR_STORAGE_VOL_ENCRYPT_CONVERT,
+    VIR_STORAGE_VOL_ENCRYPT_DONE,
+} virStorageVolEncryptConvertStep;
+
 virCommandPtr
 virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
                                          virStorageVolDefPtr vol,
                                          virStorageVolDefPtr inputvol,
                                          unsigned int flags,
                                          const char *create_tool,
-                                         const char *secretPath);
+                                         const char *secretPath,
+                                         virStorageVolEncryptConvertStep convertStep);
 
 int virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool,
                                  uint32_t scanhost);
diff --git a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv
index 849c5f0218..46d54d01c6 100644
--- a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv
+++ b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv
@@ -1,4 +1,9 @@
-qemu-img convert -f raw -O qcow2 \
+qemu-img create -f qcow2 \
 --object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
 -o encrypt.format=aes,encrypt.key-secret=OtherDemo.img_encrypt0,compat=0.10 \
-/dev/HostVG/Swap /var/lib/libvirt/images/OtherDemo.img
+/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=raw,file.filename=/dev/HostVG/Swap \
+driver=qcow2,file.filename=/var/lib/libvirt/images/OtherDemo.img,\
+encrypt.key-secret=OtherDemo.img_encrypt0
diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv
index a95749eafa..b755c1e9c4 100644
--- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv
@@ -1,5 +1,10 @@
-qemu-img convert -f raw -O qcow2 \
+qemu-img create -f qcow2 \
 --object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
 -o encrypt.format=aes,encrypt.key-secret=OtherDemo.img_encrypt0,\
 preallocation=metadata,compat=0.10 \
-/var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img
+/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=raw,file.filename=/var/lib/libvirt/images/sparse.img \
+driver=qcow2,file.filename=/var/lib/libvirt/images/OtherDemo.img,\
+encrypt.key-secret=OtherDemo.img_encrypt0
diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
index 51bdaaf684..fca8cba49b 100644
--- a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
@@ -1,5 +1,10 @@
-qemu-img convert -f raw -O qcow2 \
+qemu-img create -f qcow2 \
 --object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
 -o encrypt.format=aes,encrypt.key-secret=OtherDemo.img_encrypt0,\
 preallocation=falloc,compat=0.10 \
-/var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img
+/var/lib/libvirt/images/OtherDemo.img 0K
+qemu-img convert --image-opts -n --target-image-opts \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/secretFile \
+driver=raw,file.filename=/var/lib/libvirt/images/sparse.img \
+driver=qcow2,file.filename=/var/lib/libvirt/images/OtherDemo.img,\
+encrypt.key-secret=OtherDemo.img_encrypt0
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 4286c50c6e..e72e08a7d2 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -43,6 +43,7 @@ testCompareXMLToArgvFiles(bool shouldFail,
                           unsigned long parse_flags)
 {
     char *actualCmdline = NULL;
+    virStorageVolEncryptConvertStep convertStep = VIR_STORAGE_VOL_ENCRYPT_NONE;
     int ret = -1;
 
     virCommandPtr cmd = NULL;
@@ -79,20 +80,56 @@ testCompareXMLToArgvFiles(bool shouldFail,
     testSetVolumeType(vol, def);
     testSetVolumeType(inputvol, inputpool);
 
-    cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol,
-                                                   inputvol, flags,
-                                                   create_tool,
-                                                   "/path/to/secretFile");
-    if (!cmd) {
-        if (shouldFail) {
-            virResetLastError();
-            ret = 0;
+    /* 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. 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)
+        convertStep = VIR_STORAGE_VOL_ENCRYPT_CREATE;
+
+    do {
+        cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol,
+                                                       inputvol, flags,
+                                                       create_tool,
+                                                       "/path/to/secretFile",
+                                                       convertStep);
+        if (!cmd) {
+            if (shouldFail) {
+                virResetLastError();
+                ret = 0;
+            }
+            goto cleanup;
         }
-        goto cleanup;
-    }
 
-    if (!(actualCmdline = virCommandToString(cmd)))
-        goto cleanup;
+        if (convertStep != VIR_STORAGE_VOL_ENCRYPT_CONVERT) {
+            if (!(actualCmdline = virCommandToString(cmd)))
+                goto cleanup;
+        } else {
+            char *createCmdline = actualCmdline;
+            char *cvtCmdline;
+            int rc;
+
+            if (!(cvtCmdline = virCommandToString(cmd)))
+                goto cleanup;
+
+            rc = virAsprintf(&actualCmdline, "%s\n%s",
+                             createCmdline, cvtCmdline);
+
+            VIR_FREE(createCmdline);
+            VIR_FREE(cvtCmdline);
+            if (rc < 0)
+                goto cleanup;
+        }
+
+        if (convertStep == VIR_STORAGE_VOL_ENCRYPT_NONE)
+            convertStep = VIR_STORAGE_VOL_ENCRYPT_DONE;
+        else if (convertStep == VIR_STORAGE_VOL_ENCRYPT_CREATE)
+            convertStep = VIR_STORAGE_VOL_ENCRYPT_CONVERT;
+        else if (convertStep == VIR_STORAGE_VOL_ENCRYPT_CONVERT)
+            convertStep = VIR_STORAGE_VOL_ENCRYPT_DONE;
+
+    } while (convertStep != VIR_STORAGE_VOL_ENCRYPT_DONE);
 
     if (virTestCompareToFile(actualCmdline, cmdline) < 0)
         goto cleanup;
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/4] storage_util: Don't assume "luks" encryption for resize
Posted by John Ferlan, 6 days ago
Similar to encrypted image creation/conversion resizing the
volume requires providing different parameters for luks and
qcow[2] encryption.

Alter storageBackendResizeQemuImgImageOpts to take the @type
parameter filled in during storageBackendResizeQemuImg to either
the current type or "luks" for a RAW image and use that to
generate the "driver=%s" value and to determine whether to use
"encrypt.key-secret" or "key-secret".

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/storage/storage_util.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 29adf0cdbe..b7b86d76cb 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1088,20 +1088,26 @@ storageBackendCreateQemuImgSecretObject(virCommandPtr cmd,
 
 
 /* Add a --image-opts to the qemu-img resize command line:
- *    --image-opts driver=luks,file.filename=$volpath,key-secret=$secretAlias
- *
- *    NB: format=raw is assumed
+ *    --image-opts driver=%s,\
+ *        [encrypt.]key-secret=$secretAlias,\
+ *        file.filename=$volpath
  */
 static int
 storageBackendResizeQemuImgImageOpts(virCommandPtr cmd,
+                                     const char *type,
                                      const char *path,
                                      const char *secretAlias)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     char *commandStr = NULL;
 
-    virBufferAsprintf(&buf, "driver=luks,key-secret=%s,file.filename=",
-                      secretAlias);
+    virBufferAsprintf(&buf, "driver=%s,", type);
+    if (STREQ(type, "luks"))
+        virBufferAsprintf(&buf, "key-secret=%s,", secretAlias);
+    else
+        virBufferAsprintf(&buf, "encrypt.key-secret=%s,", secretAlias);
+    virBufferAddLit(&buf, "file.filename=");
+
     virQEMUBuildBufferEscapeComma(&buf, path);
 
     if (virBufferCheckError(&buf) < 0) {
@@ -2403,7 +2409,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
     int ret = -1;
     char *img_tool = NULL;
     virCommandPtr cmd = NULL;
-    const char *type;
+    const char *type = virStorageFileFormatTypeToString(vol->target.format);
     char *secretPath = NULL;
     char *secretAlias = NULL;
 
@@ -2417,8 +2423,6 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
     if (vol->target.encryption) {
         if (vol->target.format == VIR_STORAGE_FILE_RAW)
             type = "luks";
-        else
-            type = virStorageFileFormatTypeToString(vol->target.format);
 
         storageBackendLoadDefaultSecrets(vol);
 
@@ -2448,7 +2452,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
                                                     secretAlias) < 0)
             goto cleanup;
 
-        if (storageBackendResizeQemuImgImageOpts(cmd, vol->target.path,
+        if (storageBackendResizeQemuImgImageOpts(cmd, type, vol->target.path,
                                                  secretAlias) < 0)
             goto cleanup;
     }
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/4] docs: Update news.xml to describe encrypted image issues
Posted by John Ferlan, 6 days ago
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 docs/news.xml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 7d40e85b9a..216e8b9754 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -67,6 +67,18 @@
       </change>
     </section>
     <section title="Bug fixes">
+      <change>
+        <summary>
+          Fix issues with encrypted image creation, conversion, and resize
+        </summary>
+        <description>
+          Changes in QEMU 2.9 to arguments for qemu-img to use secrets for
+          encrypted image creation, conversion, and resize are incompatible
+          with prior versions of qemu-img. Alter encrypted image handling
+          to follow the model currently used for LUKS images for QCOW[2]
+          encrypted images.
+        </description>
+      </change>
     </section>
   </release>
   <release version="v4.3.0" date="2018-05-02">
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list