[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 11/18] qcow2: convert QCow2 to use QCryptoBlo
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH v5 11/18] qcow2: convert QCow2 to use QCryptoBlock for encryption |
Date: |
Mon, 24 Apr 2017 17:50:37 +0100 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Tue, Feb 21, 2017 at 02:30:10PM +0100, Alberto Garcia wrote:
> On Tue 21 Feb 2017 12:55:05 PM CET, Daniel P. Berrange wrote:
> > + switch (s->crypt_method_header) {
> > + case QCOW_CRYPT_NONE:
> > + break;
> > +
> > + case QCOW_CRYPT_AES:
> > + r->crypto_opts = block_crypto_open_opts_init(
> > + Q_CRYPTO_BLOCK_FORMAT_QCOW, opts, "aes-", errp);
> > + break;
> > +
> > + default:
> > + error_setg(errp, "Unsupported encryption method %d",
> > + s->crypt_method_header);
> > + break;
> > + }
> > + if (s->crypt_method_header && !r->crypto_opts) {
> > + ret = -EINVAL;
> > + goto fail;
> > + }
>
> This last condition relies on the assumption that QCOW_CRYPT_NONE == 0.
>
> I think it's safe to assume that its value is never going to change and
> therefore this isn't too important, but I'm just pointing it out in case
> you want to make it explicit.
Yeah, I'll make it explicit to be kinder to future reviewers :-)
>
> > @@ -1122,6 +1145,24 @@ static int qcow2_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > goto fail;
> > }
> >
> > + if (s->crypt_method_header == QCOW_CRYPT_AES) {
> > + unsigned int cflags = 0;
> > + if (flags & BDRV_O_NO_IO) {
> > + cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
> > + }
> > + /* TODO how do we pass the same crypto opts down to the
> > + * backing file by default, so we don't have to manually
> > + * provide the same key-secret property against the full
> > + * backing chain
> > + */
> > + s->crypto = qcrypto_block_open(s->crypto_opts, NULL, NULL,
> > + cflags, errp);
> > + if (!s->crypto) {
> > + ret = -EINVAL;
> > + goto fail;
> > + }
> > + }
>
> Actually this has the same problem that I mentioned for patch 9: if
> qcow2_open() fails then s->crypto is leaked.
Yep, and the crypto_opts actually
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v5 11/18] qcow2: convert QCow2 to use QCryptoBlock for encryption,
Daniel P. Berrange <=