Project

General

Profile

Feature #47161

mds: add dedicated field to inode for fscrypt context

Added by Jeff Layton 5 months ago. Updated 5 months ago.

Status:
Rejected
Priority:
High
Assignee:
Category:
-
Target version:
% Done:

0%

Source:
Development
Tags:
Backport:
Reviewed:
Affected Versions:
Component(FS):
MDS, libcephfs
Labels (FS):
Pull request ID:

Description

fscrypt requires that each encrypted inode contain an encryption context:

https://www.kernel.org/doc/html/latest/filesystems/fscrypt.html#encryption-context

Usually 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:

  1. this info will (almost) never change once the inode is created
  2. the clients will always need this if it's present, so we probably don't want this under xattr caps
  3. 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.

History

#1 Updated by Patrick Donnelly 5 months 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

#2 Updated by Zheng Yan 5 months 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

#3 Updated by Jeff Layton 5 months 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?

#4 Updated by Zheng Yan 5 months 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

#5 Updated by Jeff Layton 5 months 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.

Also available in: Atom PDF