[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 13/18] qcow2: add support for LUKS encryption
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-block] [PATCH v6 13/18] qcow2: add support for LUKS encryption format |
Date: |
Tue, 9 May 2017 15:05:38 +0100 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Wed, Apr 26, 2017 at 12:46:18PM -0500, Eric Blake wrote:
> On 04/25/2017 10:38 AM, Daniel P. Berrange wrote:
> > This adds support for using LUKS as an encryption format
> > with the qcow2 file, using the new encrypt.format parameter
> > to request "luks" format. e.g.
> >
> > # qemu-img create --object secret,data=123456,id=sec0 \
> > -f qcow2 -o encrypt.-format=luks,encrypt.key-secret=sec0 \
>
> s/encrypt.-format/encrypt.format/
>
> > test.qcow2 10G
> >
>
> >
> > Aside from all the cryptographic differences implied by
> > use of the LUKS format, there is one further key difference
> > between the use of legacy AES and LUKS encryption in qcow2.
> > For LUKS, the initialiazation vectors are generated using
> > the host physical sector as the input, rather than the
> > guest virtual sector. This guarantees unique initialization
> > vectors for all sectors when qcow2 internal snapshots are
> > used, thus giving stronger protection against watermarking
> > attacks.
> >
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
>
> > @@ -165,6 +246,47 @@ static int qcow2_read_extensions(BlockDriverState *bs,
> > uint64_t start_offset,
> > }
> > break;
> >
> > + case QCOW2_EXT_MAGIC_CRYPTO_HEADER: {
> > + unsigned int cflags = 0;
> > + if (s->crypt_method_header != QCOW_CRYPT_LUKS) {
> > + error_setg(errp, "CRYPTO header extension only "
> > + "expected with LUKS encryption method");
> > + return -EINVAL;
> > + }
> > + if (ext.len != sizeof(Qcow2CryptoHeaderExtension)) {
> > + error_setg(errp, "CRYPTO header extension size %u, "
> > + "but expected size %zu", ext.len,
> > + sizeof(Qcow2CryptoHeaderExtension));
> > + return -EINVAL;
> > + }
> > +
> > + ret = bdrv_pread(bs->file, offset, &s->crypto_header, ext.len);
> > + if (ret < 0) {
> > + error_setg_errno(errp, -ret,
> > + "Unable to read CRYPTO header extension");
> > + return ret;
> > + }
> > + be64_to_cpus(&s->crypto_header.offset);
> > + be64_to_cpus(&s->crypto_header.length);
> > +
> > + if ((s->crypto_header.offset % s->cluster_size) != 0) {
> > + error_setg(errp, "Encryption header offset '%" PRIu64 "'
> > is "
> > + "not a multiple of cluster size '%u'",
> > + s->crypto_header.offset, s->cluster_size);
> > + return -EINVAL;
> > + }
>
> Do we need to sanity check that crypto_header.length is not bogus?
The only sanity check I can see would be to put an arbitrary size limit
on the length ? I'm a little loathe to do that since LUKSv2 is going to
make the header extensible, and/or future LUKSv1 changes might adjust
padding, both making the header longer than it would be today. I would
like qemu-img to be able to open such future files, even if it then
refuses support for the features.
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 :|
- Re: [Qemu-block] [PATCH v6 13/18] qcow2: add support for LUKS encryption format,
Daniel P. Berrange <=