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