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.
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.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 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