On Fri, May 7, 2021 at 6:07 AM Jeff Layton <jlayton@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx/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 _______________________________________________ Dev mailing list -- dev@xxxxxxx To unsubscribe send an email to dev-leave@xxxxxxx
