lkml.org 
[lkml]   [2022]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH RFC 4/4] fscrypt: Add new encryption policy for btrfs.
From


On 7/25/22 19:32, Eric Biggers wrote:
> On Sat, Jul 23, 2022 at 08:52:28PM -0400, Sweet Tea Dorminy wrote:
>> Certain filesystems may want to use IVs generated and stored outside of
>> fscrypt's inode-based IV generation policies. In particular, btrfs can
>> have multiple inodes referencing a single block of data, and moves
>> logical data blocks to different physical locations on disk; these two
>> features mean inode or physical-location-based IV generation policies
>> will not work for btrfs. For these or similar reasons, such filesystems
>> may want to implement their own IV generation and storage for data
>> blocks.
>>
>> Plumbing each such filesystem's internals into fscrypt for IV generation
>> would be ungainly and fragile. Thus, this change adds a new policy,
>> IV_FROM_FS, and a new operation function pointer, get_fs_derived_iv. If
>> this policy is selected, the filesystem is required to provide the
>> function pointer, which populates the IV for a particular data block.
>> The IV buffer passed to get_fs_derived_iv() is pre-populated with the
>> inode contexts' nonce, in case the filesystem would like to use this
>> information; for btrfs, this is used for filename encryption. Any
>> filesystem using this policy is expected to appropriately generate and
>> store a persistent random IV for each block of data.
>
> This is changed from the original proposal to store just a random "starting IV"
> per extent, right?

This is intended to be a generic interface that doesn't require any
particular IV scheme from the filesystem. In practice, the btrfs side of
the code is using a per-extent starting IV, as originally proposed. I
don't see a way for the interface to require IVs per extent, but maybe
there is a better way than this. Or, is there more detail I can add to
the change description to clarify that the filesystem doesn't
necessarily have to store an IV for each individual data block?

> Given that this new proposal uses per-block metadata, has
> support for authenticated encryption been considered? Has space been reserved
> in the per-block metadata for authentication tags so that authenticated
> encryption support could be added later even if it's not in the initial version?

I don't know sufficiently much about authenticated encryption to have
considered it. As currently drafted, btrfs encrypts before checksumming
if checksums are enabled, and checks against checksums before
decrypting. Although at present we haven't discussed authentication
tags, btrfs could store them in a separate itemtype which could be added
at any time, much as we currently store fsverity data. We do have
sufficient room saved for adding other encryption types, if necessary;
we could use some of that to indicate the existence of authentication
tags for the extents' data.

>
> Also, could the new IV generation method just be defined as RANDOM_IV instead of
> IV_FROM_FS? Why do individual filesystems have to generate the IVs? Shouldn't
> IV generation happen in common code, with filesystems just storing and
> retrieving the IVs?
I think you're imagining an interface similar to get/set_context, where
the first time a block is written the filesystem's set_IV method is
called, and subsequent encryption/decryption calls get_IV, which is
definitely elegant in its symmetry. But I'm not sure how to have a
per-block set_IV and also only store an IV per extent, and it would be a
significant cost to store an IV per block.

I would be happy to add a fscrypt_get_random_iv() method, instead of
having the filesystem call get_random_bytes() itself, if you'd like.

Thank you!

Sweet Tea

\
 
 \ /
  Last update: 2022-07-26 04:17    [W:0.181 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site