Re: [dm-devel] [PATCH 1/1] block: Add support for setting inline encryption key per block device

[Milan Broz <gmazyland@gmail.com>]

On 21/07/2022 14:54, Christoph Hellwig wrote:
> On Wed, Jul 20, 2022 at 11:49:07PM -0700, Eric Biggers wrote:
>> On the other hand, I'd personally be fine with saying that this isn't actually
>> needed, i.e. that allowing arbitrary overriding of the default key is fine,
>> since userspace should just set up the keys properly.  For example, Android
>> doesn't need this at all, as it sets up all its keys properly.  But I want to
>> make sure that everyone is generally okay with this now, as I personally don't
>> see a fundamental difference between this and what the dm-crypt developers had
>> rejected *very* forcefully before.  Perhaps it's acceptable now simply because
>> it wouldn't be part of dm-crypt; it's a new thing, so it can have new semantics.
> 
> I agree with both the dm-crypt maintainer and you on this.  dm-crypt is
> a full disk encryption solution and has to provide guarantees, so it
> can't let upper layers degrade the cypher.  The block layer support on
> the other hand is just a building block an can as long as it is carefully
> documented.

Yes, that is what I meant when complaining about the last dm-crypt patch.

I think inline encryption for block devices is a good idea; my complaint was
integration with dm-crypt - as it can dilute expectations and create a new
threat model here.

But I also think that implementing this directly in the block layer makes sense.
Perhaps it should be generic enough.
(I can imagine dynamic inline encryption integrated into LVM ... :-)

>> I'm wondering if anyone any thoughts about how to allow data_unit_size >
>> logical_block_size with this patch's approach.  I suppose that the ioctl could
>> just allow setting it, and the block layer could fail any I/O that isn't
>> properly aligned to the data_unit_size.
> 
> We could do that, but we'd need to comunicate the limit to the upper
> layers both in the kernel an user space.  Effectively this changes the
> logical block size for all practical purposes.

I think you should support setting logical encryption size from the beginning,
at least prepare API for that. It has a big impact on performance.
The dm-crypt also requires aligned IO here. We propagate new logical size
to upper layers (and also to loop device below, if used).
(Also this revealed hacks in USB enclosures mapping unaligned devices...)

Milan