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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87bky4j36l.fsf@brahms.olymp>
Date:   Thu, 17 Mar 2022 10:14:58 +0000
From:   Luís Henriques <lhenriques@...e.de>
To:     Xiubo Li <xiubli@...hat.com>
Cc:     Jeff Layton <jlayton@...nel.org>,
        Ilya Dryomov <idryomov@...il.com>,
        Ceph Development <ceph-devel@...r.kernel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v2 0/3] ceph: add support for snapshot names encryption

Xiubo Li <xiubli@...hat.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(-)
>>
>