On Wed, Jan 18, 2017 at 07:13:19PM +0100, Max Reitz wrote: > On 03.01.2017 19:27, Daniel P. Berrange wrote: > > This converts the qcow2 driver to make use of the QCryptoBlock > > APIs for encrypting image content, using the legacyy QCow2 AES > > scheme. > > > > With this change it is now required to use the QCryptoSecret > > object for providing passwords, instead of the current block > > password APIs / interactive prompting. > > > > $QEMU \ > > -object secret,id=sec0,filename=/home/berrange/encrypted.pw \ > > -drive file=/home/berrange/encrypted.qcow2,aes-key-secret=sec0 > > > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > > --- > > block/qcow2-cluster.c | 47 +---------- > > block/qcow2.c | 190 > > +++++++++++++++++++++++++++++---------------- > > block/qcow2.h | 5 +- > > qapi/block-core.json | 7 +- > > tests/qemu-iotests/049 | 2 +- > > tests/qemu-iotests/049.out | 4 +- > > tests/qemu-iotests/082.out | 27 +++++++ > > tests/qemu-iotests/087 | 28 ++++++- > > tests/qemu-iotests/087.out | 6 +- > > tests/qemu-iotests/134 | 18 +++-- > > tests/qemu-iotests/134.out | 10 +-- > > tests/qemu-iotests/158 | 19 +++-- > > tests/qemu-iotests/158.out | 14 +--- > > 13 files changed, 219 insertions(+), 158 deletions(-) > > [...] > > > diff --git a/block/qcow2.c b/block/qcow2.c > > index 3c14c86..5c9e196 100644 > > --- a/block/qcow2.c > > +++ b/block/qcow2.c > > [...] > > > @@ -1122,6 +1144,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; > > + } > > + /* XXX how do we pass the same crypto opts down to the > > I think a TODO instead of an XXX would have been sufficient, but it's > your call.
Sure, I can put TODO. > > + * 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; > > + } > > [...] > > > @@ -2022,6 +2027,44 @@ static int > > qcow2_change_backing_file(BlockDriverState *bs, > > return qcow2_update_header(bs); > > } > > > > + > > +static int qcow2_change_encryption(BlockDriverState *bs, QemuOpts *opts, > > + Error **errp) > > I think this name is not quite appropriate, since this doesn't change > the format of the file if it is already encrypted (and it will not > encrypt any existing data). > > Maybe set_up instead of change? Yep, will change to that > > diff --git a/block/qcow2.h b/block/qcow2.h > > index 033d8c0..f4cb171 100644 > > --- a/block/qcow2.h > > +++ b/block/qcow2.h > > @@ -25,7 +25,7 @@ > > #ifndef BLOCK_QCOW2_H > > #define BLOCK_QCOW2_H > > > > -#include "crypto/cipher.h" > > +#include "crypto/block.h" > > #include "qemu/coroutine.h" > > > > //#define DEBUG_ALLOC > > @@ -256,7 +256,8 @@ typedef struct BDRVQcow2State { > > > > CoMutex lock; > > > > - QCryptoCipher *cipher; /* current cipher, NULL if no key yet */ > > + QCryptoBlockOpenOptions *crypto_opts; /* Disk encryption runtime > > options */ > > + QCryptoBlock *crypto; /* Disk encryption format driver */ > > uint32_t crypt_method_header; > > uint64_t snapshots_offset; > > int snapshots_size; > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index c2b70e8..2ca5674 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -1935,6 +1935,9 @@ > > # @cache-clean-interval: #optional clean unused entries in the L2 and > > refcount > > # caches. The interval is in seconds. The default > > value > > # is 0 and it disables this feature (since 2.5) > > +# @aes-key-secret: #optional the ID of a QCryptoSecret object > > providing > > +# the AES decryption key (since 2.9) Mandatory > > except > > Missing full stop after the closing parenthesis. > > Also, it's mandatory only for encrypted images. I know it's obvious but > that's not what it says here. True, I'll clarify > > > +# when doing a metadata-only probe of the image. > > # > > # Since: 1.7 > > ## > > @@ -1948,8 +1951,8 @@ > > '*cache-size': 'int', > > '*l2-cache-size': 'int', > > '*refcount-cache-size': 'int', > > - '*cache-clean-interval': 'int' } } > > - > > + '*cache-clean-interval': 'int', > > + '*aes-key-secret': 'str' } } > > > > ## > > # @BlockdevOptionsArchipelago: > > diff --git a/tests/qemu-iotests/049 b/tests/qemu-iotests/049 > > index fff0760..7da4ac8 100755 > > --- a/tests/qemu-iotests/049 > > +++ b/tests/qemu-iotests/049 > > @@ -106,7 +106,7 @@ test_qemu_img create -f $IMGFMT -o preallocation=1234 > > "$TEST_IMG" 64M > > echo "== Check encryption option ==" > > echo > > test_qemu_img create -f $IMGFMT -o encryption=off "$TEST_IMG" 64M > > -test_qemu_img create -f $IMGFMT -o encryption=on "$TEST_IMG" 64M > > +test_qemu_img create -f $IMGFMT --object secret,id=sec0,data=123456 -o > > encryption=on,qcow-key-secret=sec0 "$TEST_IMG" 64M > > s/qcow-key-secret/aes-key-secret/ Opps, that change accidentally got squashed into the next patch instead of this one. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|