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

Sweet Tea Dorminy
2022-09-15T18:58:19Z

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

Definitely.

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

I agree that this has risks of inadvertent misuse in that fashion.

The usecase I'm oriented towards is: someone builds an unencrypted
subvolume with a container base filesystem, takes several snapshots of
the subvolume, starts a container on each subvolume, and has each
container encrypt its designated subvolume going forward with a
different key. This usecase needs some way to mark a subvolume/directory
already containing files as encrypted going forward; I've had a hard
time coming up with a way to both protect users against such accidental
misuse, but also allow this container usecase.

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

Could it be sufficient to check either empty or read-only, something
like (is_empty_dir(inode) || (FS_CFLG_PARTIAL && !inode_permission(...,
inode, MAY_WRITE)))? Then the user is unable to accidentally write
unencrypted data, since they've taken an action to make the directory
read-only, until they've set a policy and key and turned the directory
read-write again.

Thanks!