Re: [Qemu-devel] [PATCH 1/2] qcow2: include LUKS payload overhead in qemu-img measure

[Max Reitz]

On 21.01.19 14:15, Max Reitz wrote:
> On 21.01.19 14:08, Max Reitz wrote:
>> On 15.01.19 12:10, Stefan Hajnoczi wrote:
>>> LUKS encryption reserves clusters for its own payload data.  The size of
>>> this area must be included in the qemu-img measure calculation so that
>>> we arrive at the correct minimum required image size.
>>>
>>> (Ab)use the qcrypto_block_create() API to determine the payload
>>> overhead.  We discard the payload data that qcrypto thinks will be
>>> written to the image.
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
>>> ---
>>>  block/qcow2.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 50 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/block/qcow2.c b/block/qcow2.c
>>> index 4897abae5e..7ab93a5d2f 100644
>>> --- a/block/qcow2.c
>>> +++ b/block/qcow2.c
>>
>> [...]
>>
>>> @@ -4274,6 +4294,35 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts 
>>> *opts, BlockDriverState *in_bs,
>>>      has_backing_file = !!optstr;
>>>      g_free(optstr);
>>>  
>>> +    optstr = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT);
>>> +    has_luks = optstr && strcmp(optstr, "luks") == 0;
>>> +    g_free(optstr);
>>> +
>>> +    if (has_luks) {
>>> +        QCryptoBlockCreateOptions cryptoopts = {
>>> +            .format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
>>> +        };
>>> +        QCryptoBlock *crypto;
>>> +        size_t headerlen;
>>> +
>>> +        optstr = qemu_opt_get_del(opts, "encrypt.key-secret");
>>> +        cryptoopts.u.luks.has_key_secret = !!optstr;
>>> +        cryptoopts.u.luks.key_secret = optstr;
>>
>> I wonder if you couldn't just make some secret up here (if the user
>> doesn't specify anything).  Its content shouldn't matter, right?
> 
> And now I wonder whether other options may not have actual influence on
> the crypto header size.  I suppose in theory the cipher algorithm may
> cause a difference, although it's probably not enough to warrant another
> cluster...

I stand corrected:

With aes-128:

$ qemu-img create -f qcow2 --object secret,id=sec0,data=foo \
    -o
encrypt.format=luks,encrypt.key-secret=sec0,encrypt.cipher-alg=aes-128 \
    foo.qcow2 64M
Formatting 'foo.qcow2', fmt=qcow2 size=67108864 encrypt.format=luks
encrypt.key-secret=sec0 encrypt.cipher-alg=aes-128 cluster_size=65536
lazy_refcounts=off refcount_bits=16
$ qemu-img info foo.qcow2
[...]
disk size: 448K
[...]


With aes-256:

$ qemu-img create -f qcow2 --object secret,id=sec0,data=foo \
    -o
encrypt.format=luks,encrypt.key-secret=sec0,encrypt.cipher-alg=aes-256 \
    foo.qcow2 64MFormatting 'foo.qcow2', fmt=qcow2 size=67108864
Formatting 'foo.qcow2', fmt=qcow2 size=67108864 encrypt.format=luks
encrypt.key-secret=sec0 encrypt.cipher-alg=aes-256 cluster_size=65536
lazy_refcounts=off refcount_bits=16
$ qemu-img info foo.qcow2
[...]
disk size: 540K
[...]


So it does make a difference and those options should be parsed as well.

Max

signature.asc
Description: OpenPGP digital signature