On 2/26/22 10:58 PM, Luís Henriques wrote:
Xiubo Li <xiubli@xxxxxxxxxx> writes:
On 2/25/22 5:48 PM, Luís Henriques wrote:
Xiubo Li <xiubli@xxxxxxxxxx> writes:
On 2/24/22 7:21 PM, Luís Henriques wrote:
Since filenames in encrypted directories are already encrypted and shown
as a base64-encoded string when the directory is locked, snapshot names
should show a similar behaviour.
Signed-off-by: Luís Henriques <lhenriques@xxxxxxx>
---
fs/ceph/dir.c | 15 +++++++++++++++
fs/ceph/inode.c | 10 +++++++++-
2 files changed, 24 insertions(+), 1 deletion(-)
Support on the MDS for names that'll be > MAX_NAME when base64 encoded is
still TBD. I thought it would be something easy to do, but snapshots
don't seem to make use of the CDir/CDentry (which is where alternate_name
is stored on the MDS). I'm still looking into this, but I may need some
help there :-(
Cheers,
--
Luís
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index a449f4a07c07..20ae600ee7cd 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1065,6 +1065,13 @@ static int ceph_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
op = CEPH_MDS_OP_MKSNAP;
dout("mksnap dir %p snap '%pd' dn %p\n", dir,
dentry, dentry);
+ /* XXX missing support for alternate_name in snapshots */
+ if (IS_ENCRYPTED(dir) && (dentry->d_name.len >= 189)) {
+ dout("encrypted snapshot name too long: %pd len: %d\n",
+ dentry, dentry->d_name.len);
+ err = -ENAMETOOLONG;
+ goto out;
+ }
} else if (ceph_snap(dir) == CEPH_NOSNAP) {
dout("mkdir dir %p dn %p mode 0%ho\n", dir, dentry, mode);
op = CEPH_MDS_OP_MKDIR;
@@ -1109,6 +1116,14 @@ static int ceph_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
!req->r_reply_info.head->is_target &&
!req->r_reply_info.head->is_dentry)
err = ceph_handle_notrace_create(dir, dentry);
+
+ /*
+ * If we have created a snapshot we need to clear the cache, otherwise
+ * snapshot will show encrypted filenames in readdir.
+ */
Do you mean dencrypted filenames ?
What I see without this d_drop() is that, if I run an 'ls' in a snapshot
directory immediately after creating it, the filenames in that snapshot
will be encrypted. Maybe there's a bug somewhere else and this d_drop()
isn't the right fix...?
BTW, how did you reproduce this ?
The snapshot name hasn't encrypted yet ? Did you add one patch to do this ?
I don't think I understand what you're referring to. I haven't looked
into you patch (probably won't be able to do in before Wednesday) but if
you remove the d_drop() in ceph_mkdir() in this patch, here's what I use
to reproduce the issue:
# mkdir mydir
# fscrypt encrypt mydir
# cd mydir
# create a few files
# mkdir .snap/snapshot-01
# ls -l .snap/snapshot-01
This is my test by using the branch 'wip-fscrypt' branch:
[root@lxbceph1 kcephfs]# cd mydir/
[root@lxbceph1 mydir]# mkdir a b cd
[root@lxbceph1 mydir]# ls
a b c
[root@lxbceph1 mydir]# ls .snap
[root@lxbceph1 mydir]# mkdir .snap/mydir_snap1
[root@lxbceph1 mydir]# ls .snap/mydir_snap1
a b c
[root@lxbceph1 mydir]# ls .snap/
mydir_snap1
[root@lxbceph1 mydir]# touch file1 file2 file3
[root@lxbceph1 mydir]# mkdir .snap/mydir_snap2
[root@lxbceph1 mydir]# ls .snap/mydir_snap2
a b c file1 file2 file3
[root@lxbceph1 mydir]# ls .snap/
mydir_snap1 mydir_snap2
[root@lxbceph1 mydir]# ls
a b c file1 file2 file3
[root@lxbceph1 mydir]# cd ..
[root@lxbceph1 kcephfs]# fscrypt lock mydir/
"mydir/" is now locked.
[root@lxbceph1 kcephfs]# cd mydir/
[root@lxbceph1 mydir]# ls
5D7Q8T0rdSRiJZnvbpCWRFQnLb3BxO4-zyVHsFCH98o
LWbsdS2rvuCALUXE0TrTqbuElw4zU6cZHg62-LY5GMA
u2nEDclQZAdtetYqQ7aCWNFwu8-1FDH9vI6pM4o6ZN8
-fKqUSM9DGlT1KNE-pkygEXAdjwf9fTDROA_6ZkDEio
m42jW1zJs75o2dx0bEHNmEWx9GmYXxHveSmBFagwPOw
ZLYwBnb7WT78Saz5RFEOdrKn3OLb6AfHk-IElAmEVps
[root@lxbceph1 mydir]# ls .snap/
mydir_snap1 mydir_snap2
[root@lxbceph1 mydir]# ls .snap/mydir_snap1
LWbsdS2rvuCALUXE0TrTqbuElw4zU6cZHg62-LY5GMA
m42jW1zJs75o2dx0bEHNmEWx9GmYXxHveSmBFagwPOw
ZLYwBnb7WT78Saz5RFEOdrKn3OLb6AfHk-IElAmEVps
[root@lxbceph1 mydir]# ls .snap/mydir_snap2
5D7Q8T0rdSRiJZnvbpCWRFQnLb3BxO4-zyVHsFCH98o
LWbsdS2rvuCALUXE0TrTqbuElw4zU6cZHg62-LY5GMA
u2nEDclQZAdtetYqQ7aCWNFwu8-1FDH9vI6pM4o6ZN8
-fKqUSM9DGlT1KNE-pkygEXAdjwf9fTDROA_6ZkDEio
m42jW1zJs75o2dx0bEHNmEWx9GmYXxHveSmBFagwPOw
ZLYwBnb7WT78Saz5RFEOdrKn3OLb6AfHk-IElAmEVps
[root@lxbceph1 mydir]# cd ..
[root@lxbceph1 kcephfs]# fscrypt unlock mydir/
Enter custom passphrase for protector "l":
"mydir/" is now unlocked and ready for use.
[root@lxbceph1 kcephfs]# cd mydir/
[root@lxbceph1 mydir]# ls
a b c file1 file2 file3
[root@lxbceph1 mydir]# ls .snap/mydir_snap1
a b c
[root@lxbceph1 mydir]# ls .snap/mydir_snap2
a b c file1 file2 file3
[root@lxbceph1 mydir]#
We can see that only the "mydir_snap1" and "mydir_snap2" snapshot names
are not encrypted when the 'mydir' is locked, my patch above is fixing
this issue. All the others work as expected.
And this would show the contents 'snapshot-01' but with the filenames
encrypted, even with 'mydir' isn't locked.
With this d_drop(), this behaviour will go away, i.e. you'll see the
correct (unencrypted) filenames.
The tests above without this changes in your patch.
Also, after locking 'mydir' (fscrypt lock mydir), an 'ls' in the snapshot
directory ('ls mydir/.snap') will show the _encrypted_ snapshot names and
an 'ls' in the snapshot itself ('ls mydir/.snap/<ENCRYPTED NAME>') will
show the encrypted filenames as in an 'ls mydir'.
Not sure whether I missed something here, so strange I couldn't
reproduce it.
BTW, which branch were you using to test this ?
I will post my patch to fix the issue I mentioned.
- Xiubo
Cheers,
