Re: ceph-mds infrastructure for fscrypt

[Jeff Layton <jlayton@redhat.com>]

On Fri, 2021-04-30 at 07:20 -0700, Patrick Donnelly wrote:
> On Fri, Apr 30, 2021 at 6:45 AM Jeff Layton <jlayton@redhat.com> wrote:
> > > > The client can stuff that into the xattr blob when creating a new inode,
> > > > and the MDS can scrape it out of that and move the data into the correct
> > > > field in the inode. A setxattr on this field would update the new field
> > > > too. It's an ugly interface, but shouldn't be too bad to handle and we
> > > > have some precedent for this sort of thing.
> > > > 
> > > > The rules for handling the new field in the client would be a bit weird
> > > > though. We'll need to allow it to reading the fscrypt_ctx part without
> > > > any caps (since that should be static once it's set), but the size
> > > > handling needs to be under the same caps as the traditional size field
> > > > (Is that Fsx? The rules for this are never quite clear to me.)
> > > > 
> > > > Would it be better to have two different fields here -- fscrypt_auth and
> > > > fscrypt_file? Or maybe, fscrypt_static/_dynamic? We don't necessarily
> > > > need to keep all of this info together, but it seemed neater that way.
> > > 
> > > I'm not seeing a reason to split the struct.
> > > 
> > 
> > What caps should this live under? We have different requirements for
> > different parts of the struct.
> > 
> > 1) fscrypt context: needs to be always available, especially when an
> > inode is initially instantiated, though it should almost always be
> > static once it's set. The exception is that an empty directory can grow
> > a new context when it's first encrypted, and we'll want other clients to
> > pick up on this change when it occurs.
> 
> Do clients need to see this when not reading/writing to the file?
> 

Generally, yes. It's used for regular files when reading/writing,
directories for accessing their contents, and for encrypting/decrypting
symlink contents.


> > 2) "real" size: needs to be under Fwx, I think (though I need to look
> > more closely at the truncation path to be sure).
> 
> Frs would need the size as well.
>

Correct, I was speaking more about what you'd need to cache changes to
it. Reads would indeed need Fr or Fs.

> > ...and that's not even considering what rules we might want in the
> > future for other info we stuff into here. Given that the MDS needs to
> > treat this as opaque, what locks/caps should cover this new field?
> 
> I think because the encryption context is used for reads/writes, it
> can fall under the same lock domain as the file size. I don't see a
> need (yet) for accessing e.g. the encrypted version/blocksize outside
> of the Fsx cap. It's good to think about though and I wonder if anyone
> else has thoughts on it.
> 

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.

-- 
Jeff Layton <jlayton@redhat.com>