[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 00/11] RFC crypto/luks: encryption key managment using ame
From: |
Maxim Levitsky |
Subject: |
Re: [PATCH v2 00/11] RFC crypto/luks: encryption key managment using amend interface |
Date: |
Tue, 12 Nov 2019 14:07:15 +0200 |
On Tue, 2019-11-12 at 12:58 +0100, Max Reitz wrote:
> On 08.11.19 16:07, Maxim Levitsky wrote:
> > On Fri, 2019-10-04 at 21:10 +0200, Max Reitz wrote:
> > > On 13.09.19 00:30, Maxim Levitsky wrote:
> > > > This patch series is continuation of my work to add encryption
> > > > key managment to luks/qcow2 with luks.
> > > >
> > > > This is second version of this patch set.
> > > > The changes are mostly addressing the review feedback,
> > > > plus I tested (and fixed sadly) the somewhat ugly code
> > > > that allows to still write share a raw luks device,
> > > > while preveting the key managment from happening in this case,
> > > > as it is unsafe.
> > > > I added a new iotest dedicated to that as well.
> > > >
> > > > Best regards,
> > > > Maxim Levitsky
> > >
> > > At least for an RFC looks good from my perspective. I didn’t look at
> > > the crypto things very closely (assuming Dan would do so), and I didn’t
> > > check the iotests in detail. (But it definitely doesn’t look like they
> > > lack in breadth. Maybe I’d like to see a test that you cannot have
> > > other useful nodes attached to the LUKS or qcow2 node while the
> > > amendment process is ongoing (because CONSISTENT_READ is unshared). But
> > > that’s the only thing I can think of.)
> >
> > Could you elaborate on this?
> >
> > Inside the same process several users can access that luks node at the same
> > time while one of them changes encryption keys, since this doesn't affect
> > IO of the data.
> >
> > Two users in same process I was *I think* told that can't do the amend in
> > the same time
> > since qmp is protected with a lock. However since I use a block job (to be
> > consistent with blockdev-create)
> > I wonder if several qmp amend commands couldn't race one with another.
> > These jobs is running
> > on the block device AIO context (I changed this recently after a review),
> > but stil I am not sure
> > there can't be a race.
> >
> > And when there is access to the same image from multiple processes, I do
> > have a test that
> > checks that as long as more that one process has the image open, noone can
> > change the encryption keys
> > (this is only relevant for raw luks format, since for qcow2 this is
> > forbidden anyway).
>
> Yes, sorry, I don’t remember/know where I got the qcow2 part from. (I
> probably just forgot during after reviewing that only LUKS’s permissions
> are changed by this series.)
>
> But for LUKS, those changed permissions do apply. If you can’t do
> something between two different qemu instances, you can’t do it in a
> single one: The file locks are equivalent to the internal permission mask.
>
> So if you can’t change the encryption keys while another process has the
> image open, you can’t change the encryption keys while another node uses
> the file node in the same process. For example, you can’t attach two
> LUKS nodes to a single file node and then change the keys on one of the
> nodes.
>
> Max
>
Ah, I understand now. I'll add a test for that!
Best regards,
Maxim Levitsky