Re: [PATCH v4] ceph: mark directory as non-complete complete after loading key

[Xiubo Li]

On 30/11/2022 14:54, Gregory Farnum wrote:
> On Tue, Nov 29, 2022 at 7:21 AM Ilya Dryomov <idryomov@xxxxxxxxx> wrote:
> > On Tue, Nov 29, 2022 at 3:50 PM Xiubo Li <xiubli@xxxxxxxxxx> wrote:
> > > On 29/11/2022 22:32, Ilya Dryomov wrote:
> > > > On Tue, Nov 29, 2022 at 3:15 PM Xiubo Li <xiubli@xxxxxxxxxx> wrote:
> > > > > On 29/11/2022 18:39, Luís Henriques wrote:
> > > > > > When setting a directory's crypt context, ceph_dir_clear_complete() needs to
> > > > > > be called otherwise if it was complete before, any existing (old) dentry will
> > > > > > still be valid.
> > > > > >
> > > > > > This patch adds a wrapper around __fscrypt_prepare_readdir() which will
> > > > > > ensure a directory is marked as non-complete if key status changes.
> > > > > >
> > > > > > Signed-off-by: Luís Henriques <lhenriques@xxxxxxx>
> > > > > > ---
> > > > > > Hi Xiubo,
> > > > > >
> > > > > > Here's a rebase of this patch.  I did some testing but since this branch
> > > > > > doesn't really have full fscrypt support, I couldn't even reproduce the
> > > > > > bug.  So, my testing was limited.
> > > > > I'm planing not to update the wip-fscrypt branch any more, except the IO
> > > > > path related fixes, which may introduce potential bugs each time as before.
> > > > >
> > > > > Since the qa tests PR has finished and the tests have passed, so we are
> > > > > planing to merge the first none IO part, around 27 patches. And then
> > > > > pull the reset patches from wip-fscrypt branch.
> > > > I'm not sure if merging metadata and I/O path patches separately
> > > > makes sense.  What would a user do with just filename encryption?
> > > Hi Ilya,
> > >
> > > I think the IO ones should be followed soon.
> > >
> > > Currently the filename ones have been well testes. And the contents will
> > > be by passed for now.
> > >
> > > Since this is just for Dev Preview feature IMO it should be okay (?)
> > I don't think there is such a thing as a Dev Preview feature when it
> > comes to the mainline kernel, particularly in the area of filesystems
> > and storage.  It should be ready for users at least to some extent.  So
> > my question stands: what would a user do with just filename encryption?
> I think how this merges is up to you guys and the kernel practices.
> Merging only the filename encryption is definitely of *limited*
> utility, but I don't think it's totally pointless -- the data versus
> metadata paths are different and you are protecting against somewhat
> different vulnerabilities and threat models with them. For instance,
> MDS logs dump filenames, but OSD logs do not dump object data. There's
> some obvious utility there even if you basically trust your provider,
> or run your own cluster but want to be more secure about sending logs
> via ceph-post-file.

Hi Greg,

Sounds reasonable to me.

I will leave this to Ilya.

Thanks!

- Xiubo

> -Greg
>
> > Thanks,
> >
> >                  Ilya