| Message ID | 20220302121323.240432-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ceph: encrypt the snapshot directories | expand |
Hi Xiubo, xiubli@redhat.com writes: > From: Xiubo Li <xiubli@redhat.com> > > This patch series is base on the 'wip-fscrypt' branch in ceph-client. I gave this patchset a try but it looks broken. For example, if 'mydir' is an encrypted *and* locked directory doing: # ls -l mydir/.snap will result in: fscrypt (ceph, inode 1099511627782): Error -105 getting encryption context My RFC patch had an issue that I haven't fully analyzed (and that I "fixed" using the d_drop()). But why is the much simpler approach I used not acceptable? (I.e simply use fscryt_auth from parent in ceph_get_snapdir()). > V3: > - Add more detail comments in the commit comments and code comments. > - Fix some bugs. > - Improved the patches. > - Remove the already merged patch. > > V2: > - Fix several bugs, such as for the long snap name encrypt/dencrypt > - Skip double dencypting dentry names for readdir > > ====== > > NOTE: This patch series won't fix the long snap shot issue as Luis > is working on that. Yeah, I'm getting back to it right now. Let's see if I can untangle this soon ;-) Cheers,
Luís Henriques <lhenriques@suse.de> writes: > Hi Xiubo, > > xiubli@redhat.com writes: > >> From: Xiubo Li <xiubli@redhat.com> >> >> This patch series is base on the 'wip-fscrypt' branch in ceph-client. > > I gave this patchset a try but it looks broken. For example, if 'mydir' > is an encrypted *and* locked directory doing: > > # ls -l mydir/.snap > > will result in: > > fscrypt (ceph, inode 1099511627782): Error -105 getting encryption context > > My RFC patch had an issue that I haven't fully analyzed (and that I > "fixed" using the d_drop()). But why is the much simpler approach I used > not acceptable? (I.e simply use fscryt_auth from parent in > ceph_get_snapdir()). > >> V3: >> - Add more detail comments in the commit comments and code comments. >> - Fix some bugs. >> - Improved the patches. >> - Remove the already merged patch. >> >> V2: >> - Fix several bugs, such as for the long snap name encrypt/dencrypt >> - Skip double dencypting dentry names for readdir >> >> ====== >> >> NOTE: This patch series won't fix the long snap shot issue as Luis >> is working on that. > > Yeah, I'm getting back to it right now. Let's see if I can untangle this > soon ;-) OK, I've an initial attempt with PR#45224. I don't think I have the right permissions to explicitly request reviews, so I thought I'd better let you know about the PR by email. Cheers,
On 3/2/22 11:40 PM, Luís Henriques wrote: > Hi Xiubo, > > xiubli@redhat.com writes: > >> From: Xiubo Li <xiubli@redhat.com> >> >> This patch series is base on the 'wip-fscrypt' branch in ceph-client. > I gave this patchset a try but it looks broken. For example, if 'mydir' > is an encrypted *and* locked directory doing: > > # ls -l mydir/.snap > > will result in: > > fscrypt (ceph, inode 1099511627782): Error -105 getting encryption context Sorry, I forgot to mention you need the following ceph PRs: https://github.com/ceph/ceph/pull/45208 https://github.com/ceph/ceph/pull/45192 > My RFC patch had an issue that I haven't fully analyzed (and that I > "fixed" using the d_drop()). But why is the much simpler approach I used > not acceptable? (I.e simply use fscryt_auth from parent in > ceph_get_snapdir()). Sorry, I missed reading your patch. I will check more carefully about that. This patch series is mainly supporting other features, that is the long snap names inheirt from parent snaprealms. I will drop the related patch here and cherry-pick to use yours then and do the test. - Xiubo > >> V3: >> - Add more detail comments in the commit comments and code comments. >> - Fix some bugs. >> - Improved the patches. >> - Remove the already merged patch. >> >> V2: >> - Fix several bugs, such as for the long snap name encrypt/dencrypt >> - Skip double dencypting dentry names for readdir >> >> ====== >> >> NOTE: This patch series won't fix the long snap shot issue as Luis >> is working on that. > Yeah, I'm getting back to it right now. Let's see if I can untangle this > soon ;-) > > Cheers,
On 3/2/22 11:40 PM, Luís Henriques wrote: > Hi Xiubo, > > xiubli@redhat.com writes: > >> From: Xiubo Li <xiubli@redhat.com> >> >> This patch series is base on the 'wip-fscrypt' branch in ceph-client. > I gave this patchset a try but it looks broken. For example, if 'mydir' > is an encrypted *and* locked directory doing: > > # ls -l mydir/.snap > > will result in: > > fscrypt (ceph, inode 1099511627782): Error -105 getting encryption context > > My RFC patch had an issue that I haven't fully analyzed (and that I > "fixed" using the d_drop()). But why is the much simpler approach I used > not acceptable? (I.e simply use fscryt_auth from parent in > ceph_get_snapdir()). Hi Luis, I will drop this patch series except the first 2: ceph: fail the request when failing to decode dentry names ceph: do not dencrypt the dentry name twice for readdir Please go on with your RFC one. I will fix the long snap issue after that or you can fix it in your next version. Thanks. BRs - Xiubo > >> V3: >> - Add more detail comments in the commit comments and code comments. >> - Fix some bugs. >> - Improved the patches. >> - Remove the already merged patch. >> >> V2: >> - Fix several bugs, such as for the long snap name encrypt/dencrypt >> - Skip double dencypting dentry names for readdir >> >> ====== >> >> NOTE: This patch series won't fix the long snap shot issue as Luis >> is working on that. > Yeah, I'm getting back to it right now. Let's see if I can untangle this > soon ;-) > > Cheers,
Xiubo Li <xiubli@redhat.com> writes: > On 3/2/22 11:40 PM, Luís Henriques wrote: >> Hi Xiubo, >> >> xiubli@redhat.com writes: >> >>> From: Xiubo Li <xiubli@redhat.com> >>> >>> This patch series is base on the 'wip-fscrypt' branch in ceph-client. >> I gave this patchset a try but it looks broken. For example, if 'mydir' >> is an encrypted *and* locked directory doing: >> >> # ls -l mydir/.snap >> >> will result in: >> >> fscrypt (ceph, inode 1099511627782): Error -105 getting encryption context > > Sorry, I forgot to mention you need the following ceph PRs: > > https://github.com/ceph/ceph/pull/45208 > > https://github.com/ceph/ceph/pull/45192 Oh, wow! I completely missed those PRs. Yeah, that would probably explain why it was not working for me. Cheers,
From: Xiubo Li <xiubli@redhat.com> This patch series is base on the 'wip-fscrypt' branch in ceph-client. V3: - Add more detail comments in the commit comments and code comments. - Fix some bugs. - Improved the patches. - Remove the already merged patch. V2: - Fix several bugs, such as for the long snap name encrypt/dencrypt - Skip double dencypting dentry names for readdir ====== NOTE: This patch series won't fix the long snap shot issue as Luis is working on that. Xiubo Li (6): ceph: fail the request when failing to decode dentry names ceph: do not dencrypt the dentry name twice for readdir ceph: add ceph_get_snap_parent_inode() support ceph: use the parent inode of '.snap' to dencrypt the names for readdir ceph: use the parent inode of '.snap' to encrypt name to build path ceph: try to encrypt/decrypt long snap name fs/ceph/crypto.c | 95 ++++++++++++++++++++++++++++++++--- fs/ceph/crypto.h | 10 +++- fs/ceph/dir.c | 98 ++++++++++++++++++++++-------------- fs/ceph/inode.c | 115 ++++++++++++++++++++++++++++++++++++++----- fs/ceph/mds_client.c | 57 +++++++++++++-------- fs/ceph/mds_client.h | 3 ++ fs/ceph/snap.c | 24 +++++++++ fs/ceph/super.h | 2 + 8 files changed, 327 insertions(+), 77 deletions(-)