Re: [PATCH v2 02/20] fscrypt: add flag allowing partially-encrypted directories

Eric Biggers
2022-09-12T01:42:42Z

On Mon, Sep 05, 2022 at 08:35:17PM -0400, Sweet Tea Dorminy wrote:
> From: Omar Sandoval <osandov@osandov.com>
>
> Creating several new subvolumes out of snapshots of another subvolume,
> each for a different VM's storage, is a important usecase for btrfs. We
> would like to give each VM a unique encryption key to use for new writes
> to its subvolume, so that secure deletion of the VM's data is as simple
> as securely deleting the key; to avoid needing multiple keys in each VM,
> we envision the initial subvolume being unencrypted. However, this means
> that the snapshot's directories would have a mix of encrypted and
> unencrypted files. During lookup with a key, both unencrypted and
> encrypted forms of the desired name must be queried.
>
> To allow this, add another FS_CFLG to allow filesystems to opt into
> partially encrypted directories.
>
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> fs/crypto/fname.c | 17 ++++++++++++++++-
> include/linux/fscrypt.h | 2 ++
> 2 files changed, 18 insertions(+), 1 deletion(-)

I'm still trying to wrap my head around what this part involves exactly. This
is a pretty big change in semantics.

Could this be moved to the end of the patchset, or is this a fundamental part of
the btrfs fscrypt support that the rest of your patchset depends on? I'd think
it would be a lot easier to review if this change was an add-on at the end.

One thing to keep in mind is that FS_IOC_SET_ENCRYPTION_POLICY failing on
nonempty directories can actually be very useful, since it makes it possible to
detect bugs where people create files in encrypted directories (expecting that
they are encrypted) before an encryption policy actually gets assigned. Since
FS_IOC_SET_ENCRYPTION_POLICY fails in that case, such bugs can be detected and
fixed.

If it succeeds, then everything would just silently work and some files wouldn't
be encrypted as intended. That's kind of scary.

It might be warranted to use an encryption policy flag to explicitly indicate
that mixing encrypted and unencrypted files is being allowed.

- Eric