[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: drop support for using qcow[2] encryptio
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] block: drop support for using qcow[2] encryption with system emulators |
Date: |
Sat, 11 Jun 2016 14:33:47 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 06/10/2016 09:32 AM, Daniel P. Berrange wrote:
> Back in the 2.3.0 release we declared qcow[2] encryption as
> deprecated, warning people that it would be removed in a future
> release.
>
> commit a1f688f4152e65260b94f37543521ceff8bfebe4
> Author: Markus Armbruster <address@hidden>
> Date: Fri Mar 13 21:09:40 2015 +0100
>
> block: Deprecate QCOW/QCOW2 encryption
>
> The code still exists today, but by a (happy?) accident we entirely
> broke the ability to use qcow[2] encryption in the system emulators
> in the 2.4.0 release due to
>
> commit 8336aafae1451d54c81dd2b187b45f7c45d2428e
> Author: Daniel P. Berrange <address@hidden>
> Date: Tue May 12 17:09:18 2015 +0100
>
> qcow2/qcow: protect against uninitialized encryption key
>
> This commit was designed to prevent future coding bugs which
> might cause QEMU to read/write data on an encrypted block
> device in plain text mode before a decryption key is set.
>
> It turns out this preventative measure was a little too good,
> because we already had a long standing bug where QEMU read
> encrypted data in plain text mode during system emulator
> startup, in order to guess disk geometry:
Interesting analysis.
> So rather than fix the crash, and backport it to stable
> releases, just go ahead with what we have warned users about
> and disable any use of qcow2 encryption in the system
> emulators. qemu-img/qemu-io/qemu-nbd are still able to access
> qcow2 encrypted images for the sake of data conversion.
>
> In the future, qcow2 will gain support for the alternative
> luks format, but when this happens it'll be using the
> '-object secret' infrastructure for gettings keys, which
> avoids this problematic scenario entirely.
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> block/qcow.c | 11 +++++++----
> block/qcow2.c | 11 +++++++----
> tests/qemu-iotests/087.out | 12 ++----------
> 3 files changed, 16 insertions(+), 18 deletions(-)
> +++ b/block/qcow.c
> @@ -162,10 +162,13 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
> if (s->crypt_method_header) {
> if (bdrv_uses_whitelist() &&
> s->crypt_method_header == QCOW_CRYPT_AES) {
> - error_report("qcow built-in AES encryption is deprecated");
> - error_printf("Support for it will be removed in a future
> release.\n"
> - "You can use 'qemu-img convert' to switch to an\n"
> - "unencrypted qcow image, or a LUKS raw image.\n");
> + error_setg(errp,
> + "Use of AES-CBC encrypted qcow images is no longer "
> + "supported in system emulators. You can use "
> + "'qemu-img convert' to convert your image to use "
> + "the LUKS format instead.");
error_setg() should not end in '.'. Better would be:
error_setg(errp, "Use of AES-CBC encrypted qcow images is not supported");
error_append_hint(errp, "You can use 'qemu-img convert'... instead.\n");
> +++ b/block/qcow2.c
> @@ -968,10 +968,13 @@ static int qcow2_open(BlockDriverState *bs, QDict
> *options, int flags,
> if (s->crypt_method_header) {
> if (bdrv_uses_whitelist() &&
> s->crypt_method_header == QCOW_CRYPT_AES) {
> - error_report("qcow2 built-in AES encryption is deprecated");
> - error_printf("Support for it will be removed in a future
> release.\n"
> - "You can use 'qemu-img convert' to switch to an\n"
> - "unencrypted qcow2 image, or a LUKS raw image.\n");
> + error_setg(errp,
> + "Use of AES-CBC encrypted qcow2 images is no longer "
> + "supported in system emulators. You can use "
> + "'qemu-img convert' to convert your image to use "
> + "the LUKS format instead.");
and again.
> + ret = -ENOSYS;
> + goto fail;
> }
>
> bs->encrypted = 1;
> diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
> index 055c553..99853c5 100644
> --- a/tests/qemu-iotests/087.out
> +++ b/tests/qemu-iotests/087.out
> @@ -42,22 +42,14 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
> encryption=on
> Testing: -S
> QMP_VERSION
> {"return": {}}
> -IMGFMT built-in AES encryption is deprecated
> -Support for it will be removed in a future release.
> -You can use 'qemu-img convert' to switch to an
> -unencrypted IMGFMT image, or a LUKS raw image.
> -{"error": {"class": "GenericError", "desc": "blockdev-add doesn't support
> encrypted devices"}}
> +{"error": {"class": "GenericError", "desc": "Use of AES-CBC encrypted qcow2
> images is no longer supported in system emulators. You can use 'qemu-img
> convert' to convert your image to use the LUKS format instead."}}
And this will need tweaking to match.
I'm in favor of the idea behind the patch, but the error_setg() usage
needs to be fixed for v2.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature