Re: [PATCH v2 00/11] RFC crypto/luks: encryption key managment using amend interface

[Maxim Levitsky]

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