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]

Date:   Thu, 17 Mar 2022 18:52:49 +0800
From:   Xiubo Li <xiubli@...hat.com>
To:     Jeff Layton <jlayton@...nel.org>,
        Luís Henriques <lhenriques@...e.de>
Cc:     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


On 3/17/22 6:01 PM, Jeff Layton wrote:
> I'm not sure we want to worry about .snap directories here since they
> aren't "real". IIRC, snaps are inherited from parents too, so you could
> do something like
>
>      mkdir dir1
>      mkdir dir1/.snap/snap1
>      mkdir dir1/dir2
>      fscrypt encrypt dir1/dir2
>
> There should be nothing to prevent encrypting dir2, but I'm pretty sure
> dir2/.snap will not be empty at that point.

If we don't take care of this. Then we don't know which snapshots should 
do encrypt/dencrypt and which shouldn't when building the path in lookup 
and when reading the snapdir ?

-- Xiubo

>
> -- Jeff
>
> On Thu, 2022-03-17 at 13:27 +0800, Xiubo Li wrote:
>> 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 ?
>>
>> - 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(-)
>>>