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

[Max Reitz]

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

signature.asc
Description: OpenPGP digital signature