Re: ceph-mds infrastructure for fscrypt

[Patrick Donnelly <pdonnell@redhat.com>]

On Fri, May 7, 2021 at 6:07 AM Jeff Layton <jlayton@redhat.com> wrote:
>
> On Fri, 2021-04-30 at 18:03 -0700, Patrick Donnelly wrote:
> > On Fri, Apr 30, 2021 at 8:04 AM Jeff Layton <jlayton@redhat.com> wrote:
> > >
> > > On Fri, 2021-04-30 at 07:45 -0700, Patrick Donnelly wrote:
> > > > On Fri, Apr 30, 2021 at 7:33 AM Jeff Layton <jlayton@redhat.com> wrote:
> > > > > We specifically need this for directories and symlinks during pathwalks
> > > > > too. Eventually we may also want to encrypt certain data for other inode
> > > > > types as well (e.g. block/char devices). That's less critical though.
> > > > >
> > > > > The problem with fetching it after the inode is first instantiated is
> > > > > that we can end up recursing into a separate request while encoding a
> > > > > path. For instance, see this stack trace that Luis reported:
> > > > > https://lore.kernel.org/ceph-devel/53d5bebb28c1e0cd354a336a56bf103d5e3a6344.camel@kernel.org/T/#m0f7bbed6280623d761b8b4e70671ed568535d7fa
> > > > >
> > > > > While that implementation stored the context in an xattr, the problem
> > > > > isstill the same if you have to fetch the context in the middle of
> > > > > building a path. The best solution is just to always ensure it's
> > > > > available.
> > > >
> > > > Got it. Splitting the struct makes sense then. The pin cap would be
> > > > suitable for the immutable encryption context (if truly
> > > > immutable?).Otherwise maybe the Axs cap?
> > > >
> > >
> > > Ok. In that case, then we probably need to put the context blob under
> > > AUTH caps so we can ensure that it's consulted during the permission
> > > checks for pathwalks. The size will need to live under FILE.
> > >
> > > Now for the hard part...what do we name these fields?
> > >
> > >     fscrypt_context
> > >     fscrypt_size
> > >
> > > ...or maybe...
> > >
> > >     fscrypt_auth
> > >     fscrypt_file
> > >
> > > Since they'll be vector blobs, we can version these too so that we can
> > > add other fields later if the need arises (even for non-fscrypt stuff).
> > > Maybe we could consider:
> > >
> > >     client_opaque_auth
> > >     client_opaque_file
> >
> > An opaque blob makes sense but you'd want a sentinel indicating it's
> > an fscrypt blob. Don't think we'd be able to have two competing
> > use-cases but it'd be nice to have it generic enough for future
> > encryption libraries maybe.
> >
>
> I'm going with fscrypt_auth and fscrypt_file for now. We can rename them
> later though if we want. What I'll probably do is just declare a
> versioned format for these blobs. The MDS won't care about it, but the
> clients can follow that convention.
>
> I've made a bit of progress on this this week (fixing up the encoding
> and decoding was a bit of a hassle, fwiw). These fields are associated
> with the core inodes. The clients will use SETATTR calls to set them,
> though they will also be updated with cap flushes, etc.
>
> I need to be able to validate this feature in userland though and I
> don't really want to roll dedicated functions for them. What I may do is
> add new vxattrs (ceph.fscrypt_auth and ceph.fscrypt_file) and have those
> expose these fields. Doing a setxattr on them will do a SETATTR under
> the hood. The alternative is to declare new libcephfs routines for
> fetching and setting these.

A client-side vxattr sounds good to me.

-- 
Patrick Donnelly, Ph.D.
He / Him / His
Principal Software Engineer
Red Hat Sunnyvale, CA
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D