Feature #47161
closedmds: add dedicated field to inode for fscrypt context
0%
Description
fscrypt requires that each encrypted inode contain an encryption context:
https://www.kernel.org/doc/html/latest/filesystems/fscrypt.html#encryption-contextUsually this is stored in an xattr (and that is what the prototype code so far does), but I think that's probably not ideal for cephfs:
- this info will (almost) never change once the inode is created
- the clients will always need this if it's present, so we probably don't want this under xattr caps
- we're going to want the MDS to block legacy clients from accessing fscrypted directories so the MDS will need to know whether this attr is present for some things, and that's simpler if we don't have to deal with xattrs
What I think we need at this point is a new field in the inode that is set to the crypto context. While these don't change often, we do need to allow them to be changed for some of the ioctls (particularly when adding a new crypto context when encrypting a new directory). So, we'd need the ability to change this with a SETATTR call as well. It seems like this might appropriately be under the aegis of auth caps.
With modern code, the contexts should be ~16 bytes, but eventually they could be expanded, so we should assume that they could be larger in the future.
Updated by Patrick Donnelly over 3 years ago
- Subject changed from add dedicated field to inode for fscrypt context to mds: add dedicated field to inode for fscrypt context
- Assignee set to Xiubo Li
- Priority changed from Normal to High
- Target version set to v16.0.0
- Source set to Development
Updated by Zheng Yan over 3 years ago
- Assignee changed from Xiubo Li to Jeff Layton
xattr is the most suitable place, because client can create file and set xattr at the same time. This is similar to security context. just search all 'ceph_security_xattr_wanted()' checks in kcephfs code and add fscrypt checks
Updated by Jeff Layton over 3 years ago
The prototype implementation uses an xattr, so I'm aware how that works, but there is more to this than just setting it at creation time:
1) this will almost always need to be fetched by the client, so that will effectively make the client require Xs caps for every lookup. This would be better under AUTH caps. (fwiw, ACLs would have been better under there too).
2) ideally, we want to prevent legacy clients that don't support fscrypt from ever traversing into trees where a crypto context is set, so the MDS will always need to know if one is present. Do you really want the MDS to have to grovel inside the xattr blob for every lookup?
Updated by Zheng Yan over 3 years ago
Jeff Layton wrote:
The prototype implementation uses an xattr, so I'm aware how that works, but there is more to this than just setting it at creation time:
1) this will almost always need to be fetched by the client, so that will effectively make the client require Xs caps for every lookup. This would be better under AUTH caps. (fwiw, ACLs would have been better under there too).
kclient with selinux enabled already does this
2) ideally, we want to prevent legacy clients that don't support fscrypt from ever traversing into trees where a crypto context is set, so the MDS will always need to know if one is present. Do you really want the MDS to have to grovel inside the xattr blob for every lookup?
mds can check xattr at file creation, and set a flag
Updated by Jeff Layton over 3 years ago
- Status changed from New to Rejected
Fair enough then. I'll keep working with this as an xattr for now. Let's go ahead and close this out then, and I'll reopen if it turns out to be untenable.