Re: [RFC PATCH v2 0/3] ceph: add support for snapshot names encryption

["Luís Henriques" <lhenriques@suse.de>]

Xiubo Li <xiubli@redhat.com> writes:

> Hi Luis,
>
> There has another issue you need to handle at the same time.
>
> Currently only the empty directory could be enabled the file encryption, such as
> for the following command:
>
> $ fscrypt encrypt mydir/
>
> But should we also make sure that the mydir/.snap/ is empty ?
>
> Here the 'empty' is not totally empty, which allows it should allow long snap
> names exist.
>
> Make sense ?

Right, actually I had came across that question in the past but completely
forgot about it.

Right now we simply check the dir stats to ensure a directory is empty.
We could add an extra check in ceph_crypt_empty_dir() to ensure that there
are no snapshots _above_ that directory (i.e. that there are no
"mydir/.snap/_name_xxxxx").

Unfortunately, I don't know enough of snapshots implementation details to
understand if it's a problem to consider a directory as being empty (in
the fscrypt context) when there are these '_name_xxx' directories.  My
feeling is that this is not a problem but I really don't know.

Do you (or anyone) have any ideas/suggestions?

Cheers,
-- 
Luís

>
> - Xiubo
>
>
> On 3/16/22 12:19 AM, Luís Henriques wrote:
>> Hi!
>>
>> A couple of changes since v1:
>>
>> - Dropped the dentry->d_flags change in ceph_mkdir().  Thanks to Xiubo
>>    suggestion, patch 0001 now skips calling ceph_fscrypt_prepare_context()
>>    if we're handling a snapshot.
>>
>> - Added error handling to ceph_get_snapdir() in patch 0001 (Jeff had
>>    already pointed that out but I forgot to include that change in previous
>>    revision).
>>
>> - Rebased patch 0002 to the latest wip-fscrypt branch.
>>
>> - Added some documentation regarding snapshots naming restrictions.
>>
>> As before, in order to test this code the following PRs are required:
>>
>>    mds: add protection from clients without fscrypt support #45073
>>    mds: use the whole string as the snapshot long name #45192
>>    mds: support alternate names for snapshots #45224
>>    mds: limit the snapshot names to 240 characters #45312
>>
>> Luís Henriques (3):
>>    ceph: add support for encrypted snapshot names
>>    ceph: add support for handling encrypted snapshot names
>>    ceph: update documentation regarding snapshot naming limitations
>>
>>   Documentation/filesystems/ceph.rst |  10 ++
>>   fs/ceph/crypto.c                   | 158 +++++++++++++++++++++++++----
>>   fs/ceph/crypto.h                   |  11 +-
>>   fs/ceph/inode.c                    |  31 +++++-
>>   4 files changed, 182 insertions(+), 28 deletions(-)
>>
>