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....
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>