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

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

[Stefan Hajnoczi]

The LUKS payload has a significant size (>1 MB).  It must be included in the
qemu-img measure calculation so we arrive at the correct minimum image size.

Stefan Hajnoczi (2):
  qcow2: include LUKS payload overhead in qemu-img measure
  iotests: add LUKS payload overhead to 178 qemu-img measure test

 block/qcow2.c                    | 51 +++++++++++++++++++++++++++++++-
 tests/qemu-iotests/178           |  8 +++++
 tests/qemu-iotests/178.out.qcow2 | 24 +++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)

-- 
2.20.1

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

[Stefan Hajnoczi]

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 <stefanha@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
@@ -4231,6 +4231,24 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
     return ret;
 }
 
+static ssize_t qcow2_measure_crypto_hdr_init_func(QCryptoBlock *block,
+        size_t headerlen, void *opaque, Error **errp)
+{
+    size_t *headerlenp = opaque;
+
+    /* Stash away the payload size */
+    *headerlenp = headerlen;
+    return 0;
+}
+
+static ssize_t qcow2_measure_crypto_hdr_write_func(QCryptoBlock *block,
+        size_t offset, const uint8_t *buf, size_t buflen,
+        void *opaque, Error **errp)
+{
+    /* Discard the bytes, we're not actually writing to an image */
+    return buflen;
+}
+
 static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
                                        Error **errp)
 {
@@ -4240,11 +4258,13 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
     uint64_t virtual_size; /* disk size as seen by guest */
     uint64_t refcount_bits;
     uint64_t l2_tables;
+    uint64_t luks_payload_size = 0;
     size_t cluster_size;
     int version;
     char *optstr;
     PreallocMode prealloc;
     bool has_backing_file;
+    bool has_luks;
 
     /* Parse image creation options */
     cluster_size = qcow2_opt_get_cluster_size_del(opts, &local_err);
@@ -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;
+
+        crypto = qcrypto_block_create(&cryptoopts, "encrypt.",
+                                      qcow2_measure_crypto_hdr_init_func,
+                                      qcow2_measure_crypto_hdr_write_func,
+                                      &headerlen, &local_err);
+
+        g_free(optstr);
+        if (!crypto) {
+            goto err;
+        }
+        qcrypto_block_free(crypto);
+
+        luks_payload_size = ROUND_UP(headerlen, cluster_size);
+    }
+
     virtual_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
     virtual_size = ROUND_UP(virtual_size, cluster_size);
 
@@ -4344,7 +4393,7 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
     info = g_new(BlockMeasureInfo, 1);
     info->fully_allocated =
         qcow2_calc_prealloc_size(virtual_size, cluster_size,
-                                 ctz32(refcount_bits));
+                                 ctz32(refcount_bits)) + luks_payload_size;
 
     /* Remove data clusters that are not required.  This overestimates the
      * required size because metadata needed for the fully allocated file is
-- 
2.20.1

[Qemu-devel] [PATCH 2/2] iotests: add LUKS payload overhead to 178 qemu-img measure test

[Stefan Hajnoczi]

The previous patch includes the LUKS payload overhead into the qemu-img
measure calculation for qcow2.  Update qemu-iotests 178 to exercise this
new code path.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/178           |  8 ++++++++
 tests/qemu-iotests/178.out.qcow2 | 24 ++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178
index 3f4b4a4564..23eb017ea1 100755
--- a/tests/qemu-iotests/178
+++ b/tests/qemu-iotests/178
@@ -142,6 +142,14 @@ for ofmt in human json; do
             # The backing file doesn't need to exist :)
             $QEMU_IMG measure --output=$ofmt -o backing_file=x \
                               -f "$fmt" -O "$IMGFMT" "$TEST_IMG"
+
+            echo
+            echo "== $fmt input image and LUKS encryption =="
+            echo
+            $QEMU_IMG measure --output=$ofmt \
+                              --object secret,id=sec0,data=base \
+                              -o encrypt.format=luks,encrypt.key-secret=sec0 \
+                              -f "$fmt" -O "$IMGFMT" "$TEST_IMG"
         fi
 
         echo
diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2
index d42d4a4597..55a8dc926f 100644
--- a/tests/qemu-iotests/178.out.qcow2
+++ b/tests/qemu-iotests/178.out.qcow2
@@ -68,6 +68,11 @@ converted image file size in bytes: 458752
 required size: 1074135040
 fully allocated size: 1074135040
 
+== qcow2 input image and LUKS encryption ==
+
+required size: 2686976
+fully allocated size: 1076232192
+
 == qcow2 input image and preallocation (human) ==
 
 required size: 1074135040
@@ -114,6 +119,11 @@ converted image file size in bytes: 524288
 required size: 1074135040
 fully allocated size: 1074135040
 
+== raw input image and LUKS encryption ==
+
+required size: 2686976
+fully allocated size: 1076232192
+
 == raw input image and preallocation (human) ==
 
 required size: 1074135040
@@ -205,6 +215,13 @@ converted image file size in bytes: 458752
     "fully-allocated": 1074135040
 }
 
+== qcow2 input image and LUKS encryption ==
+
+{
+    "required": 2686976,
+    "fully-allocated": 1076232192
+}
+
 == qcow2 input image and preallocation (json) ==
 
 {
@@ -263,6 +280,13 @@ converted image file size in bytes: 524288
     "fully-allocated": 1074135040
 }
 
+== raw input image and LUKS encryption ==
+
+{
+    "required": 2686976,
+    "fully-allocated": 1076232192
+}
+
 == raw input image and preallocation (json) ==
 
 {
-- 
2.20.1

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

[Philippe Mathieu-Daudé]

On 1/15/19 12:10 PM, Stefan Hajnoczi wrote:
> The LUKS payload has a significant size (>1 MB).  It must be included in the
> qemu-img measure calculation so we arrive at the correct minimum image size.
> 
> Stefan Hajnoczi (2):
>   qcow2: include LUKS payload overhead in qemu-img measure
>   iotests: add LUKS payload overhead to 178 qemu-img measure test
> 
>  block/qcow2.c                    | 51 +++++++++++++++++++++++++++++++-
>  tests/qemu-iotests/178           |  8 +++++
>  tests/qemu-iotests/178.out.qcow2 | 24 +++++++++++++++
>  3 files changed, 82 insertions(+), 1 deletion(-)
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190115111007.27159-1-stefanha@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14
=== TEST SCRIPT END ===

  CC      crypto/pbkdf-nettle.o
  CC      crypto/ivgen.o
/tmp/qemu-test/src/block/sheepdog.c: In function 'find_vdi_name':
/tmp/qemu-test/src/block/sheepdog.c:1239:5: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
     strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors


The full log is available at
http://patchew.org/logs/20190115111007.27159-1-stefanha@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

On Sun, Jan 20, 2019 at 05:51:06PM -0800, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20190115111007.27159-1-stefanha@redhat.com/
...
> /tmp/qemu-test/src/block/sheepdog.c: In function 'find_vdi_name':
> /tmp/qemu-test/src/block/sheepdog.c:1239:5: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
>      strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors

Wasn't me, I swear!

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

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

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 2275 bytes --]

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 <stefanha@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?

Max

> +
> +        crypto = qcrypto_block_create(&cryptoopts, "encrypt.",
> +                                      qcow2_measure_crypto_hdr_init_func,
> +                                      qcow2_measure_crypto_hdr_write_func,
> +                                      &headerlen, &local_err);
> +
> +        g_free(optstr);
> +        if (!crypto) {
> +            goto err;
> +        }
> +        qcrypto_block_free(crypto);
> +
> +        luks_payload_size = ROUND_UP(headerlen, cluster_size);
> +    }
> +
>      virtual_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
>      virtual_size = ROUND_UP(virtual_size, cluster_size);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 2/2] iotests: add LUKS payload overhead to 178 qemu-img measure test

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On 15.01.19 12:10, Stefan Hajnoczi wrote:
> The previous patch includes the LUKS payload overhead into the qemu-img
> measure calculation for qcow2.  Update qemu-iotests 178 to exercise this
> new code path.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/qemu-iotests/178           |  8 ++++++++
>  tests/qemu-iotests/178.out.qcow2 | 24 ++++++++++++++++++++++++
>  2 files changed, 32 insertions(+)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1933 bytes --]

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 <stefanha@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...

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 3006 bytes --]

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 <stefanha@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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Daniel P. Berrangé]

On Mon, Jan 21, 2019 at 02:15:03PM +0100, 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 <stefanha@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...

The LUKS header comprises three parts

 - A field size initial header with general config properties
 - A series of fixed size key slot headers, one per slot, fixed
   at 8 slots in the code
 - A series of regions of key material, one per slot.

The last one is a problem - the size of the key slot regions is
the number of anti-forensic stripes (4096) multipled by the size
of the master key. The latter depends on the size of the cipher
key. This is referred to as "splitkeylen" in the code creating
this. The overall space reserved for headers is then determined
by:

    luks->header.payload_offset =
        (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
        (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
                  (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
                   QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) *
         QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);

So for AES-128  key materials takes 0.5 MB, while for AES-256 key material
takes 1 MB, so adding together you get 1052672 byte for header in AES-128
and 2068480 bytes for AES-256, rounded up to qcow2 cluster size (whatever
that is configured to be)

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-devel] [PATCH 1/2] qcow2: include LUKS payload overhead in qemu-img measure

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3890 bytes --]

On Mon, Jan 21, 2019 at 01:45:44PM +0000, Daniel P. Berrangé wrote:
> On Mon, Jan 21, 2019 at 02:15:03PM +0100, 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 <stefanha@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...
> 
> The LUKS header comprises three parts
> 
>  - A field size initial header with general config properties
>  - A series of fixed size key slot headers, one per slot, fixed
>    at 8 slots in the code
>  - A series of regions of key material, one per slot.
> 
> The last one is a problem - the size of the key slot regions is
> the number of anti-forensic stripes (4096) multipled by the size
> of the master key. The latter depends on the size of the cipher
> key. This is referred to as "splitkeylen" in the code creating
> this. The overall space reserved for headers is then determined
> by:
> 
>     luks->header.payload_offset =
>         (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
>          QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
>         (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
>                   (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
>                    QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) *
>          QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> 
> So for AES-128  key materials takes 0.5 MB, while for AES-256 key material
> takes 1 MB, so adding together you get 1052672 byte for header in AES-128
> and 2068480 bytes for AES-256, rounded up to qcow2 cluster size (whatever
> that is configured to be)

Thanks.  I think qemu-img measure needs to take all of
QCryptoBlockCreateOptions and pass them in to qcrypto_block_create().
That way the result is accurate (although specific to the options that
were provided).

The current patch constructs a dummy QCryptoBlockCreateOptions instead
of populating it with options from the command-line.  I'll try to fix
this in v2.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]