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 <arm...@redhat.com> > 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 <berra...@redhat.com> > 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 <berra...@redhat.com>
> ---
> 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
