Re: ceph-mds infrastructure for fscrypt

[Jeff Layton <jlayton@redhat.com>]

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.

I'm not terribly crazy about either, but I have a slight preference for
the vxattr since it's something we could replicate in the kernel for
debugging purposes.

Thoughts?
-- 
Jeff Layton <jlayton@redhat.com>