[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 07/14] block/crypto: implement the encryption key manageme
From: |
Maxim Levitsky |
Subject: |
Re: [PATCH v8 07/14] block/crypto: implement the encryption key management |
Date: |
Tue, 16 Jun 2020 16:57:45 +0300 |
User-agent: |
Evolution 3.34.4 (3.34.4-1.fc31) |
On Mon, 2020-06-08 at 14:14 +0200, Max Reitz wrote:
> On 08.06.20 11: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 <mlevitsk@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > ---
> > block/crypto.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++--
> > block/crypto.h | 34 +++++++++++++
> > 2 files changed, 161 insertions(+), 3 deletions(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 1960b47ceb..b9c40e6922 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
>
> [...]
>
> > +static void
> > +block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c,
> > + const BdrvChildRole role,
>
> Well, it isn’t wrong to have this be a const, nor is it against any
> coding guidelines. While I do believe this was an accident, I also
> think that in fact, maybe being strict about const-ness is what we
> should’ve done everywhere from the start.
>
> So this is not a complaint, quite the contrary.
>
> (I felt it was interesting enough to warrant this mail. *shrug*)
Yep, that was 100% accident I confess.
Best regards,
Maxim Levitsky
>
> > + BlockReopenQueue *reopen_queue,
> > + uint64_t perm, uint64_t shared,
> > + uint64_t *nperm, uint64_t *nshared)
> > +{
> > +
> > + BlockCrypto *crypto = bs->opaque;
> > +
> > + bdrv_default_perms(bs, c, role, reopen_queue, perm, shared, nperm,
> > nshared);
> > +
> > + /*
> > + * For backward compatibility, manually share the write
> > + * and resize permission
> > + */
> > + *nshared |= (BLK_PERM_WRITE | BLK_PERM_RESIZE);
> > + /*
> > + * Since we are not fully a format driver, don't always request
> > + * the read/resize permission but only when explicitly
> > + * requested
> > + */
> > + *nperm &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
> > + *nperm |= perm & (BLK_PERM_WRITE | BLK_PERM_RESIZE);
>
> Looks good, thanks!
>
> Max
>
- [PATCH v8 01/14] qcrypto/core: add generic infrastructure for crypto options amendment, (continued)
- [PATCH v8 01/14] qcrypto/core: add generic infrastructure for crypto options amendment, Maxim Levitsky, 2020/06/08
- [PATCH v8 02/14] qcrypto/luks: implement encryption key management, Maxim Levitsky, 2020/06/08
- [PATCH v8 03/14] block/amend: add 'force' option, Maxim Levitsky, 2020/06/08
- [PATCH v8 04/14] block/amend: separate amend and create options for qemu-img, Maxim Levitsky, 2020/06/08
- [PATCH v8 05/14] block/amend: refactor qcow2 amend options, Maxim Levitsky, 2020/06/08
- [PATCH v8 06/14] block/crypto: rename two functions, Maxim Levitsky, 2020/06/08
- [PATCH v8 08/14] block/qcow2: extend qemu-img amend interface with crypto options, Maxim Levitsky, 2020/06/08
- [PATCH v8 07/14] block/crypto: implement the encryption key management, Maxim Levitsky, 2020/06/08
- [PATCH v8 09/14] iotests: filter few more luks specific create options, Maxim Levitsky, 2020/06/08
- [PATCH v8 10/14] iotests: qemu-img tests for luks key management, Maxim Levitsky, 2020/06/08
- [PATCH v8 11/14] block/core: add generic infrastructure for x-blockdev-amend qmp command, Maxim Levitsky, 2020/06/08
- [PATCH v8 12/14] block/crypto: implement blockdev-amend, Maxim Levitsky, 2020/06/08
- [PATCH v8 13/14] block/qcow2: implement blockdev-amend, Maxim Levitsky, 2020/06/08
- [PATCH v8 14/14] iotests: add tests for blockdev-amend, Maxim Levitsky, 2020/06/08
- Re: [PATCH v8 00/14] LUKS: encryption slot management using amend interface, no-reply, 2020/06/08
- Re: [PATCH v8 00/14] LUKS: encryption slot management using amend interface, Max Reitz, 2020/06/08