Re: [PATCH v2 05/20] fscrypt: add extent-based encryption

Omar Sandoval
2022-09-07T20:00:13Z

On Mon, Sep 05, 2022 at 08:35:20PM -0400, Sweet Tea Dorminy wrote:
> Some filesystems need to encrypt data based on extents, rather than on
> inodes, due to features incompatible with inode-based encryption. For
> instance, btrfs can have multiple inodes referencing a single block of
> data, and moves logical data blocks to different physical locations on
> disk in the background; these two features mean inode or
> physical-location-based policies will not work for btrfs.

I really like how this abstracts away the encryption details from the
filesystem.

> This change introduces fscrypt_extent_context objects, in analogy to
> existing context objects based on inodes. For a filesystem which uses
> extents,

This makes it sounds like all filesystems that store allocations as
extents should define these, but ext4 (for example) uses extents but is
fine with inode-based encryption policies. Perhaps this can say
something like "A filesytem can opt into the extent-based encryption
policy by defining new hooks that manage a new fscrypt_extent_context."

> a new hook provides a new fscrypt_extent_context. During file
> content encryption/decryption, the existing fscrypt_context object
> provides key information, while the new fscrypt_extent_context provides
> IV information. For filename encryption, the existing IV generation
> methods are still used, since filenames are not stored in extents.
>
> As individually keyed inodes prevent sharing of extents, such policies
> are forbidden for filesystems with extent-based encryption.

This ends up forcing Btrfs to use Adiantum. However, I imagine that most
users would prefer to use AES if their CPU has AES instructions. From
what I understand, it should still be possible to use the AES encryption
modes with extent contexts, correct? We just need to decide how to make
that work with the encryption policy flags. I see a couple of options:

1. We add a specific FSCRYPT_POLICY_FLAG_EXTENT_BASED or something like
that which the user must specify for filesystems requiring
extent-based encryption.
2. The "default" mode (i.e., none of DIRECT_KEY, IV_INO_LBLK_64, nor
IV_INO_LBLK_32 are specified) automatically opts into extent-based
encryption for filesystems requiring it.

Either way, we should probably still disallow IV_INO_LBLK_64 and
IV_INO_LBLK_32 since neither of those make sense with per-extent IVs.

I'd love to hear what Eric would prefer here.

Thanks,
Omar