Re: [PATCH 05/21] fscrypt: add new encryption policy for btrfs.

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Aug 18, 2022 at 08:22:53PM -0400, Sweet Tea Dorminy wrote:
> 
> 
> On 8/18/22 18:07, Eric Biggers wrote:
> > On Wed, Aug 17, 2022 at 11:54:56AM -0400, Sweet Tea Dorminy wrote:
> > > 
> > > 
> > > On 8/17/22 10:49, Sweet Tea Dorminy wrote:
> > > > Encryption for btrfs must be extent-based, rather than 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. Instead, btrfs can store an IV per extent, generated by
> > > > fscrypt and iterated per block within the extent, and provide that IV to
> > > > fscrypt for encryption/decryption.
> > > > 
> > > > Plumbing filesystem 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.  btrfs will
> > > > require this policy to be used, and implements get_fs_derived_iv by
> > > > getting the IV stored with the relevant file extent and iterating the IV
> > > > to the appropriate block number. Thus, each individual block has its own
> > > > IV, but all blocks within a file extent have iterated IVs according to
> > > > their block number, similarly to the IV_INO_LBLK* policy iterating IVs
> > > > for a given inode by lblk number.
> > > > 
> > > > The IV buffer passed to get_fs_derived_iv() is pre-populated with the
> > > > inode contexts' nonce, as not all data to be encrypted is associated
> > > > with a file extent: for btrfs, this is used for filename encryption.
> > > > 
> > > > Filesystems implementing this policy are expected to be incompatible
> > > > with existing IV generation policies, so if the function pointer is set,
> > > > only dummy or IV_FROM_FS policies are permitted. If there is a
> > > > filesystem which allows other policies as well as IV_FROM_FS, it may be
> > > > better to expose the policy to filesystems, so they can determine
> > > > whether any given policy is compatible with their operation.
> > > > 
> > > > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> > > > ---
> > > 
> > > I realized after sending that this doesn't have Documentation/ updates for
> > > the new policy, still; apologies, and it remains on my queue.
> > 
> > It looks like you also didn't address my feedback about IV_FROM_FS at
> > https://lore.kernel.org/linux-fscrypt/YuBAiRg9K8IrlCqV@gmail.com ?
> 
> Apologies; I must not have grasped what you were requesting fully. Should I
> change the name from IV_FROM_FS to IV_PER_EXTENT?

Yes, though IV_RAND_PER_EXTENT would probably be even better.  I am asking for a
proper name that expresses how the cryptography works in your scheme...  And for
a slightly different way of thinking about your scheme in general, where the way
the cryptography works is a first class citizen of what your new flag means,
rather than some abstract fs-specific thing that you are trying to hide from the
UAPI and the fs/crypto/ library.

- Eric