On 10.05.20 15:40, Maxim Levitsky wrote: > This implements the encryption key management using the generic code in > qcrypto layer and exposes it to the user via qemu-img > > This code adds another 'write_func' because the initialization > write_func works directly on the underlying file, and amend > works on instance of luks device. > > This commit also adds a 'hack/workaround' I and Kevin Wolf (thanks) > made to make the driver both support write sharing (to avoid breaking the > users), > and be safe against concurrent metadata update (the keyslots) > > Eventually the write sharing for luks driver will be deprecated > and removed together with this hack. > > The hack is that we ask (as a format driver) for BLK_PERM_CONSISTENT_READ > and then when we want to update the keys, we unshare that permission. > So if someone else has the image open, even readonly, encryption > key update will fail gracefully. > > Also thanks to Daniel Berrange for the idea of > unsharing read, rather that write permission which allows > to avoid cases when the other user had opened the image read-only. > > Signed-off-by: Maxim Levitsky <[email protected]> > Reviewed-by: Daniel P. Berrangé <[email protected]> > --- > block/crypto.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++-- > block/crypto.h | 34 +++++++++++++ > 2 files changed, 158 insertions(+), 3 deletions(-) > > diff --git a/block/crypto.c b/block/crypto.c > index 2e16b62bdc..b14cb0ff06 100644 > --- a/block/crypto.c > +++ b/block/crypto.c
[...]
> +static void
> +block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c,
> + const BdrvChildRole *role,
> + BlockReopenQueue *reopen_queue,
> + uint64_t perm, uint64_t shared,
> + uint64_t *nperm, uint64_t *nshared)
> +{
> +
> + BlockCrypto *crypto = bs->opaque;
> +
> + bdrv_filter_default_perms(bs, c, role, reopen_queue,
> + perm, shared, nperm, nshared);
> + /*
> + * Ask for consistent read permission so that if
> + * someone else tries to open this image with this permission
> + * neither will be able to edit encryption keys, since
> + * we will unshare that permission while trying to
> + * update the encryption keys
> + */
> + if (!(bs->open_flags & BDRV_O_NO_IO)) {
> + *nperm |= BLK_PERM_CONSISTENT_READ;
> + }
I’m not sure this is important, because this really means we won’t do
I/O. Its only relevant use in this case is for qemu-img info. Do we
really care if someone edits the key slots while qemu-img info is
processing?
OTOH, I don’t think it’ll harm much. Well, apart from the fact that
BDRV_O_NO_IO won’t do much for LUKS images.
*shrug*
Reviewed-by: Max Reitz <[email protected]>
signature.asc
Description: OpenPGP digital signature
