[Qemu-devel] [PATCH] block: Deprecate QCOW/QCOW2 encryption

[Qemu-devel] [PATCH] block: Deprecate QCOW/QCOW2 encryption

[Markus Armbruster]

We've steered users away from QCOW/QCOW2 encryption for a while,
because it's a flawed design (commit 136cd19 Describe flaws in
qcow/qcow2 encryption in the docs).

In addition to flawed crypto, we have comically bad usability, and
plain old bugs.  Let me show you.

= Example images =

I'm going to use a raw image as backing file, and two QCOW2 images,
one encrypted, and one not:

    $ qemu-img create -f raw backing.img 4m
    Formatting 'backing.img', fmt=raw size=4194304
    $ qemu-img create -f qcow2 -o encryption,backing_file=backing.img,backing_fmt=raw geheim.qcow2 4m
    Formatting 'geheim.qcow2', fmt=qcow2 size=4194304 backing_file='backing.img' backing_fmt='raw' encryption=on cluster_size=65536 lazy_refcounts=off
    $ qemu-img create -f qcow2 -o backing_file=backing.img,backing_fmt=raw normal.qcow2 4m
    Formatting 'normal.qcow2', fmt=qcow2 size=4194304 backing_file='backing.img' backing_fmt='raw' encryption=off cluster_size=65536 lazy_refcounts=off

= Usability issues =

== Confusing startup ==

When no image is encrypted, and you don't give -S, QEMU starts the
guest immediately:

    $ qemu-system-x86_64 -nodefaults -display none -monitor stdio normal.qcow2
    QEMU 2.2.50 monitor - type 'help' for more information
    (qemu) info status
    VM status: running

But as soon as there's an encrypted image in play, the guest is *not*
started, with no notification whatsoever:

    $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2
    QEMU 2.2.50 monitor - type 'help' for more information
    (qemu) info status
    VM status: paused (prelaunch)

If the user figured out that he needs to type "cont" to enter his
keys, the confusion enters the next level: "cont" asks for at most
*one* key.  If more are needed, it then silently does nothing.  The
user has to type "cont" once per encrypted image:

    $ qemu-system-x86_64 -nodefaults -display none -monitor stdio -drive if=none,file=geheim.qcow2 -drive if=none,file=geheim.qcow2
    QEMU 2.2.50 monitor - type 'help' for more information
    (qemu) info status
    VM status: paused (prelaunch)
    (qemu) c
    none0 (geheim.qcow2) is encrypted.
    Password: ******
    (qemu) info status
    VM status: paused (prelaunch)
    (qemu) c
    none1 (geheim.qcow2) is encrypted.
    Password: ******
    (qemu) info status
    VM status: running

== Incorrect passwords not caught ==

All existing encryption schemes give you the GIGO treatment: garbage
password in, garbage data out.  Guests usually refuse to mount
garbage, but other usage is prone to data loss.

== Need to stop the guest to add an encrypted image ==

    $ qemu-system-x86_64 -nodefaults -display none -monitor stdio
    QEMU 2.2.50 monitor - type 'help' for more information
    (qemu) info status
    VM status: running
    (qemu) drive_add "" if=none,file=geheim.qcow2
    Guest must be stopped for opening of encrypted image
    (qemu) stop
    (qemu) drive_add "" if=none,file=geheim.qcow2
    OK

Commit c3adb58 added this restriction.  Before, we could expose images
lacking an encryption key to guests, with potentially catastrophic
results.  See also "Use without key is not always caught".

= Bugs =

== Use without key is not always caught ==

Encrypted images can be in an intermediate state "opened, but no key".
The weird startup behavior and the need to stop the guest are there to
ensure the guest isn't exposed to that state.  But other things still
are!

* drive_backup

    $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2
    QEMU 2.2.50 monitor - type 'help' for more information
    (qemu) drive_backup -f ide0-hd0 out.img raw
    Formatting 'out.img', fmt=raw size=4194304

  I guess this writes encrypted data to raw image out.img.  Good luck
  with figuring out how to decrypt that again.

* commit

    $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2
    QEMU 2.2.50 monitor - type 'help' for more information
    (qemu) commit ide0-hd0

  I guess this writes encrypted data into the unencrypted raw backing
  image, effectively destroying it.

== QMP device_add of usb-storage fails when it shouldn't ==

When the image is encrypted, device_add creates the device, defers
actually attaching it to when the key becomes available, then fails.
This is wrong.  device_add must either create the device and succeed,
or do nothing and fail.

    $ qemu-system-x86_64 -nodefaults -display none -usb -qmp stdio -drive if=none,id=foo,file=geheim.qcow2
    {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 2}, "package": ""}, "capabilities": []}}
    { "execute": "qmp_capabilities" }
    {"return": {}}
    { "execute": "device_add", "arguments": { "driver": "usb-storage", "id": "bar", "drive": "foo" } }
    {"error": {"class": "DeviceEncrypted", "desc": "'foo' (geheim.qcow2) is encrypted"}}
    {"execute":"device_del","arguments": { "id": "bar" } }
    {"timestamp": {"seconds": 1426003440, "microseconds": 237181}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/bar/bar.0/legacy[0]"}}
    {"timestamp": {"seconds": 1426003440, "microseconds": 238231}, "event": "DEVICE_DELETED", "data": {"device": "bar", "path": "/machine/peripheral/bar"}}
    {"return": {}}

This stuff is worse than useless, it's a trap for users.

If people become sufficiently interested in encrypted images to
contribute a cryptographically sane implementation for QCOW2 (or
whatever other format), then rewriting the necessary support around it
from scratch will likely be easier and yield better results than
fixing up the existing mess.

Let's deprecate the mess now, drop it after a grace period, and move
on.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c                    |  7 +++++++
 qemu-doc.texi              | 11 ++++++-----
 tests/qemu-iotests/049.out |  6 ++++++
 tests/qemu-iotests/087.out | 18 ++++++++++++++++++
 4 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 191a847..10c32eb 100644
--- a/block.c
+++ b/block.c
@@ -1065,6 +1065,13 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
         goto free_and_fail;
     }
 
+    if (bs->encrypted) {
+        error_report("Encrypted images are deprecated");
+        error_printf("Support for them will be removed in a future release.\n"
+                     "You can use 'qemu-img convert' to convert your image"
+                     " to an unencrypted one.\n");
+    }
+
     ret = refresh_total_sectors(bs, bs->total_sectors);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not refresh total sector count");
diff --git a/qemu-doc.texi b/qemu-doc.texi
index f5b0dc4..8aa6dbf 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -539,8 +539,8 @@ storage.
 @item qcow2
 QEMU image format, the most versatile format. Use it to have smaller
 images (useful if your filesystem does not supports holes, for example
-on Windows), optional AES encryption, zlib based compression and
-support of multiple VM snapshots.
+on Windows), zlib based compression and support of multiple VM
+snapshots.
 
 Supported options:
 @table @code
@@ -574,9 +574,10 @@ original file must then be securely erased using a program like shred,
 though even this is ineffective with many modern storage technologies.
 @end itemize
 
-Use of qcow / qcow2 encryption is thus strongly discouraged. Users are
-recommended to use an alternative encryption technology such as the
-Linux dm-crypt / LUKS system.
+Use of qcow / qcow2 encryption with QEMU is deprecated, and support for
+it will go away in a future release.  Users are recommended to use an
+alternative encryption technology such as the Linux dm-crypt / LUKS
+system.
 
 @item cluster_size
 Changes the qcow2 cluster size (must be between 512 and 2M). Smaller cluster
diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index 765afdd..9f93666 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -188,6 +188,12 @@ qemu-img create -f qcow2 -o encryption=off TEST_DIR/t.qcow2 64M
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
 
 qemu-img create -f qcow2 -o encryption=on TEST_DIR/t.qcow2 64M
+qemu-img: Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
+qemu-img: Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=on cluster_size=65536 lazy_refcounts=off refcount_bits=16
 
 == Check lazy_refcounts option (only with v3) ==
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index 0ba2e43..c71bb3a 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -44,10 +44,19 @@ QMP_VERSION
 
 === Encrypted image ===
 
+qemu-img: Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
+qemu-img: Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
 Testing: -S
 QMP_VERSION
 {"return": {}}
+Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
 {"error": {"class": "GenericError", "desc": "blockdev-add doesn't support encrypted devices"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
@@ -57,6 +66,9 @@ QMP_VERSION
 Testing:
 QMP_VERSION
 {"return": {}}
+Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
 {"error": {"class": "GenericError", "desc": "Guest must be stopped for opening of encrypted image"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
@@ -66,6 +78,12 @@ QMP_VERSION
 
 === Missing driver ===
 
+qemu-img: Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
+qemu-img: Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
 Testing: -S
 QMP_VERSION
-- 
1.9.3

Re: [Qemu-devel] [PATCH] block: Deprecate QCOW/QCOW2 encryption

[Markus Armbruster]

I fat-fingered Dan's e-mail address.  I apologize for the inconvenience.

Re: [Qemu-devel] [PATCH] block: Deprecate QCOW/QCOW2 encryption

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]

On 03/13/2015 02:09 PM, Markus Armbruster wrote:
> We've steered users away from QCOW/QCOW2 encryption for a while,
> because it's a flawed design (commit 136cd19 Describe flaws in
> qcow/qcow2 encryption in the docs).
> 
> In addition to flawed crypto, we have comically bad usability, and
> plain old bugs.  Let me show you.
> 

> This stuff is worse than useless, it's a trap for users.
> 
> If people become sufficiently interested in encrypted images to
> contribute a cryptographically sane implementation for QCOW2 (or
> whatever other format), then rewriting the necessary support around it
> from scratch will likely be easier and yield better results than
> fixing up the existing mess.
> 
> Let's deprecate the mess now, drop it after a grace period, and move
> on.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block.c                    |  7 +++++++
>  qemu-doc.texi              | 11 ++++++-----
>  tests/qemu-iotests/049.out |  6 ++++++
>  tests/qemu-iotests/087.out | 18 ++++++++++++++++++
>  4 files changed, 37 insertions(+), 5 deletions(-)

Worth having in 2.3.

Reviewed-by: Eric Blake <eblake@redhat.com>

> +++ b/qemu-doc.texi
> @@ -539,8 +539,8 @@ storage.
>  @item qcow2
>  QEMU image format, the most versatile format. Use it to have smaller
>  images (useful if your filesystem does not supports holes, for example
> -on Windows), optional AES encryption, zlib based compression and
> -support of multiple VM snapshots.
> +on Windows), zlib based compression and support of multiple VM
> +snapshots.

[Side note - Windows NTFS supports holes (so the claim that Windows
doesn't support holes is false, although it is true for other typical
Windows filesystems such as FAT).  On the other hand, Windows hole
support is so bad that it typically causes worse performance (at one
point, Cygwin used NTFS holes wherever possible, but now defaults to no
holes unless you explicitly modify mount options to request Cygwin to
use them, because of the performance improvement).  Doesn't affect this
patch, though.]


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] block: Deprecate QCOW/QCOW2 encryption

[Daniel P. Berrange]

On Fri, Mar 13, 2015 at 09:09:40PM +0100, Markus Armbruster wrote:
> We've steered users away from QCOW/QCOW2 encryption for a while,
> because it's a flawed design (commit 136cd19 Describe flaws in
> qcow/qcow2 encryption in the docs).
> 
> In addition to flawed crypto, we have comically bad usability, and
> plain old bugs.  Let me show you.
> 
> = Example images =
> 
> I'm going to use a raw image as backing file, and two QCOW2 images,
> one encrypted, and one not:
> 
>     $ qemu-img create -f raw backing.img 4m
>     Formatting 'backing.img', fmt=raw size=4194304
>     $ qemu-img create -f qcow2 -o encryption,backing_file=backing.img,backing_fmt=raw geheim.qcow2 4m
>     Formatting 'geheim.qcow2', fmt=qcow2 size=4194304 backing_file='backing.img' backing_fmt='raw' encryption=on cluster_size=65536 lazy_refcounts=off
>     $ qemu-img create -f qcow2 -o backing_file=backing.img,backing_fmt=raw normal.qcow2 4m
>     Formatting 'normal.qcow2', fmt=qcow2 size=4194304 backing_file='backing.img' backing_fmt='raw' encryption=off cluster_size=65536 lazy_refcounts=off
> 
> = Usability issues =
> 
> == Confusing startup ==
> 
> When no image is encrypted, and you don't give -S, QEMU starts the
> guest immediately:
> 
>     $ qemu-system-x86_64 -nodefaults -display none -monitor stdio normal.qcow2
>     QEMU 2.2.50 monitor - type 'help' for more information
>     (qemu) info status
>     VM status: running
> 
> But as soon as there's an encrypted image in play, the guest is *not*
> started, with no notification whatsoever:
> 
>     $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2
>     QEMU 2.2.50 monitor - type 'help' for more information
>     (qemu) info status
>     VM status: paused (prelaunch)
> 
> If the user figured out that he needs to type "cont" to enter his
> keys, the confusion enters the next level: "cont" asks for at most
> *one* key.  If more are needed, it then silently does nothing.  The
> user has to type "cont" once per encrypted image:
> 
>     $ qemu-system-x86_64 -nodefaults -display none -monitor stdio -drive if=none,file=geheim.qcow2 -drive if=none,file=geheim.qcow2
>     QEMU 2.2.50 monitor - type 'help' for more information
>     (qemu) info status
>     VM status: paused (prelaunch)
>     (qemu) c
>     none0 (geheim.qcow2) is encrypted.
>     Password: ******
>     (qemu) info status
>     VM status: paused (prelaunch)
>     (qemu) c
>     none1 (geheim.qcow2) is encrypted.
>     Password: ******
>     (qemu) info status
>     VM status: running
> 
> == Incorrect passwords not caught ==
> 
> All existing encryption schemes give you the GIGO treatment: garbage
> password in, garbage data out.  Guests usually refuse to mount
> garbage, but other usage is prone to data loss.
> 
> == Need to stop the guest to add an encrypted image ==
> 
>     $ qemu-system-x86_64 -nodefaults -display none -monitor stdio
>     QEMU 2.2.50 monitor - type 'help' for more information
>     (qemu) info status
>     VM status: running
>     (qemu) drive_add "" if=none,file=geheim.qcow2
>     Guest must be stopped for opening of encrypted image
>     (qemu) stop
>     (qemu) drive_add "" if=none,file=geheim.qcow2
>     OK
> 
> Commit c3adb58 added this restriction.  Before, we could expose images
> lacking an encryption key to guests, with potentially catastrophic
> results.  See also "Use without key is not always caught".
> 
> = Bugs =
> 
> == Use without key is not always caught ==
> 
> Encrypted images can be in an intermediate state "opened, but no key".
> The weird startup behavior and the need to stop the guest are there to
> ensure the guest isn't exposed to that state.  But other things still
> are!
> 
> * drive_backup
> 
>     $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2
>     QEMU 2.2.50 monitor - type 'help' for more information
>     (qemu) drive_backup -f ide0-hd0 out.img raw
>     Formatting 'out.img', fmt=raw size=4194304
> 
>   I guess this writes encrypted data to raw image out.img.  Good luck
>   with figuring out how to decrypt that again.
> 
> * commit
> 
>     $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2
>     QEMU 2.2.50 monitor - type 'help' for more information
>     (qemu) commit ide0-hd0
> 
>   I guess this writes encrypted data into the unencrypted raw backing
>   image, effectively destroying it.
> 
> == QMP device_add of usb-storage fails when it shouldn't ==
> 
> When the image is encrypted, device_add creates the device, defers
> actually attaching it to when the key becomes available, then fails.
> This is wrong.  device_add must either create the device and succeed,
> or do nothing and fail.
> 
>     $ qemu-system-x86_64 -nodefaults -display none -usb -qmp stdio -drive if=none,id=foo,file=geheim.qcow2
>     {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 2}, "package": ""}, "capabilities": []}}
>     { "execute": "qmp_capabilities" }
>     {"return": {}}
>     { "execute": "device_add", "arguments": { "driver": "usb-storage", "id": "bar", "drive": "foo" } }
>     {"error": {"class": "DeviceEncrypted", "desc": "'foo' (geheim.qcow2) is encrypted"}}
>     {"execute":"device_del","arguments": { "id": "bar" } }
>     {"timestamp": {"seconds": 1426003440, "microseconds": 237181}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/bar/bar.0/legacy[0]"}}
>     {"timestamp": {"seconds": 1426003440, "microseconds": 238231}, "event": "DEVICE_DELETED", "data": {"device": "bar", "path": "/machine/peripheral/bar"}}
>     {"return": {}}
> 
> This stuff is worse than useless, it's a trap for users.
> 
> If people become sufficiently interested in encrypted images to
> contribute a cryptographically sane implementation for QCOW2 (or
> whatever other format), then rewriting the necessary support around it
> from scratch will likely be easier and yield better results than
> fixing up the existing mess.
> 
> Let's deprecate the mess now, drop it after a grace period, and move
> on.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


Looks good for this release, then we kill in next release.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] block: Deprecate QCOW/QCOW2 encryption

[Kevin Wolf]

Am 13.03.2015 um 21:09 hat Markus Armbruster geschrieben:
> We've steered users away from QCOW/QCOW2 encryption for a while,
> because it's a flawed design (commit 136cd19 Describe flaws in
> qcow/qcow2 encryption in the docs).
> 
> In addition to flawed crypto, we have comically bad usability, and
> plain old bugs.  Let me show you.
> [...] 
> Let's deprecate the mess now, drop it after a grace period, and move
> on.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Thanks, applied to the block branch.

Kevin