On Mon, Jul 25, 2022 at 10:16:07PM -0400, Sweet Tea Dorminy wrote: > > > 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. I don't think that's a good way to do it. The fscrypt settings are supposed to be very concrete, meaning that they specify a particular way of doing the encryption, which can be reviewed for its security and which can be tested for correctness of the on-disk format. There shouldn't be cryptographic differences between how different filesystems implement the same setting. The fscrypt settings also shouldn't specify internal implementation details of the code, as "IV_FROM_FS" does. From userspace's perspective, *all* fscrypt settings have IVs chosen by the filesystem; the division between the "filesystem" and fs/crypto/ is an internal kernel implementation detail. So I think you should go with something like RANDOM_IV or IV_PER_EXTENT. > In practice, the btrfs side of the code is using a per-extent starting IV, as > originally proposed. This is inconsistent with your commit message, which says that there is a random IV for each block of data. It's also inconsistent with your proposed change to fscrypt_limit_io_blocks(). So I don't know which to believe. Clearly this can't be properly reviewed on its own, so please send out the whole patch series and not just the fs/crypto/ parts. - Eric
|