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

[Xiubo Li <xiubli@redhat.com>]

On 3/17/22 6:14 PM, Luís Henriques wrote:
> 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").

Check this only in ceph_crypt_empty_dir() seems not enough, at least not 
graceful.

Please see 
https://github.com/google/fscrypt/blob/master/cmd/fscrypt/commands.go#L305.

The google/fscrypt will read that directory to check where it's empty or 
not. So eventually we may need to fix it there. But for a workaround you 
may could fix it in ceph_crypt_empty_dir() and ceph_ioctl() when setting 
the key or policy ?

-- Xiubo


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