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 <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...
>
> 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
signature.asc
Description: PGP signature
