[PATCH 00/12] ceph: support idmapped mounts

[PATCH 00/12] ceph: support idmapped mounts

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Hey everyone,

This patch series enables cephfs to support idmapped mounts, i.e. the
ability to alter ownership information on a per-mount basis.

Container managers such as LXD support sharaing data via cephfs between
the host and unprivileged containers and between unprivileged containers.
They may all use different idmappings. Idmapped mounts can be used to
create mounts with the idmapping used for the container (or a different
one specific to the use-case).

There are in fact more use-cases such as remapping ownership for
mountpoints on the host itself to grant or restrict access to different
users or to make it possible to enforce that programs running as root
will write with a non-zero {g,u}id to disk.

The patch series is simple overall and few changes are needed to cephfs.
There is one cephfs specific issue that I would like to discuss and
solve which I explain in detail in:

[PATCH 02/12] ceph: handle idmapped mounts in create_request_message()

It has to do with how to handle mds serves which have id-based access
restrictions configured. I would ask you to please take a look at the
explanation in the aforementioned patch.

The patch series passes the vfs and idmapped mount testsuite as part of
xfstests. To run it you will need a config like:

[ceph]
export FSTYP=ceph
export TEST_DIR=/mnt/test
export TEST_DEV=10.103.182.10:6789:/
export TEST_FS_MOUNT_OPTS="-o name=admin,secret=$password

and then simply call

sudo ./check -g idmapped

The patch series is on top of my patches scheduled for v5.17. The
easiest way is to either fetch the branch (fs.idmapped.ceph.v1) or the
tag (tag.fs.idmapped.ceph.v1):

git fetch git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git fs.idmapped.ceph.v1
git fetch git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git tag.fs.idmapped.ceph.v1

Thanks!
Christian

Christian Brauner (12):
  ceph: stash idmapping in mdsc request
  ceph: handle idmapped mounts in create_request_message()
  ceph: allow idmapped mknod inode op
  ceph: allow idmapped symlink inode op
  ceph: allow idmapped mkdir inode op
  ceph: allow idmapped rename inode op
  ceph: allow idmapped getattr inode op
  ceph: allow idmapped permission inode op
  ceph: allow idmapped setattr inode op
  ceph/acl: allow idmapped set_acl inode op
  ceph/file: allow idmapped atomic_open inode op
  ceph: allow idmapped mounts

 fs/ceph/acl.c        |  2 +-
 fs/ceph/dir.c        |  4 ++++
 fs/ceph/file.c       | 16 ++++++++++++----
 fs/ceph/inode.c      | 15 +++++++++++----
 fs/ceph/mds_client.c | 29 +++++++++++++++++++++++++----
 fs/ceph/mds_client.h |  1 +
 fs/ceph/super.c      |  2 +-
 7 files changed, 55 insertions(+), 14 deletions(-)

-- 
2.32.0

[PATCH 01/12] ceph: stash idmapping in mdsc request

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

When sending a mds request cephfs will send relevant data for the
requested operation. For creation requests the caller's fs{g,u}id is
used to set the ownership of the newly created filesystem object. For
setattr requests the caller can pass in arbitrary {g,u}id values to
which the relevant filesystem object is supposed to be changed.

If the caller is performing the relevant operation via an idmapped mount
cephfs simply needs to take the idmapping into account when it sends the
relevant mds request.

In order to support idmapped mounts for cephfs we stash the idmapping
whenever they are relevant for the operation for the duration of the
request. Since mds requests can be queued and performed asynchronously
we make sure to keep the idmapping around and release it once the
request has finished.

In follow-up patches we will use this to send correct ownership
information over the wire. This patch just adds the basic infrastructure
to keep the idmapping around. The actual conversion patches are all
fairly minimal.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/mds_client.c | 7 +++++++
 fs/ceph/mds_client.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index c30eefc0ac19..ae2cc4ce1d48 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -12,6 +12,7 @@
 #include <linux/bits.h>
 #include <linux/ktime.h>
 #include <linux/bitmap.h>
+#include <linux/mnt_idmapping.h>
 
 #include "super.h"
 #include "mds_client.h"
@@ -862,6 +863,8 @@ void ceph_mdsc_release_request(struct kref *kref)
 	kfree(req->r_path1);
 	kfree(req->r_path2);
 	put_cred(req->r_cred);
+	if (!initial_idmapping(req->mnt_userns))
+		put_user_ns(req->mnt_userns);
 	if (req->r_pagelist)
 		ceph_pagelist_release(req->r_pagelist);
 	put_request_session(req);
@@ -918,6 +921,10 @@ static void __register_request(struct ceph_mds_client *mdsc,
 	insert_request(&mdsc->request_tree, req);
 
 	req->r_cred = get_current_cred();
+	if (!req->mnt_userns)
+		req->mnt_userns = &init_user_ns;
+	else
+		get_user_ns(req->mnt_userns);
 
 	if (mdsc->oldest_tid == 0 && req->r_op != CEPH_MDS_OP_SETFILELOCK)
 		mdsc->oldest_tid = req->r_tid;
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 97c7f7bfa55f..1b648645e340 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -275,6 +275,7 @@ struct ceph_mds_request {
 	union ceph_mds_request_args r_args;
 	int r_fmode;        /* file mode, if expecting cap */
 	const struct cred *r_cred;
+	struct user_namespace *mnt_userns;
 	int r_request_release_offset;
 	struct timespec64 r_stamp;
 
-- 
2.32.0

[PATCH 02/12] ceph: handle idmapped mounts in create_request_message()

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Inode operations that create a new filesystem object such as ->mknod,
->create, ->mkdir() and others don't take a {g,u}id argument explicitly.
Instead the caller's fs{g,u}id is used for the {g,u}id of the new
filesystem object.

Cephfs mds creation request argument structures mirror this filesystem
behavior. They don't encode a {g,u}id explicitly. Instead the caller's
fs{g,u}id that is always sent as part of any mds request is used by the
servers to set the {g,u}id of the new filesystem object.

In order to ensure that the correct {g,u}id is used map the caller's
fs{g,u}id for creation requests. This doesn't require complex changes.
It suffices to pass in the relevant idmapping recorded in the request
message. If this request message was triggered from an inode operation
that creates filesystem objects it will have passed down the relevant
idmaping. If this is a request message that was triggered from an inode
operation that doens't need to take idmappings into account the initial
idmapping is passed down which is an identity mapping and thus is
guaranteed to leave the caller's fs{g,u}id unchanged.,u}id is sent.

The last few weeks before Christmas 2021 I have spent time not just
reading and poking the cephfs kernel code but also took a look at the
ceph mds server userspace to ensure I didn't miss some subtlety.

This made me aware of one complication to solve. All requests send the
caller's fs{g,u}id over the wire. The caller's fs{g,u}id matters for the
server in exactly two cases:

1. to set the ownership for creation requests
2. to determine whether this client is allowed access on this server

Case 1. we already covered and explained. Case 2. is only relevant for
servers where an explicit uid access restriction has been set. That is
to say the mds server restricts access to requests coming from a
specific uid. Servers without uid restrictions will grant access to
requests from any uid by setting MDS_AUTH_UID_ANY.

Case 2. introduces the complication because the caller's fs{g,u}id is
not just used to record ownership but also serves as the {g,u}id used
when checking access to the server.

Consider a user mounting a cephfs client and creating an idmapped mount
from it that maps files owned by uid 1000 to be owned uid 0:

mount -t cephfs -o [...] /unmapped
mount-idmapped --map-mount 1000:0:1 /idmapped

That is to say if the mounted cephfs filesystem contains a file "file1"
which is owned by uid 1000:

- looking at it via /unmapped/file1 will report it as owned by uid 1000
  (One can think of this as the on-disk value.)
- looking at it via /idmapped/file1 will report it as owned by uid 0

Now, consider creating new files via the idmapped mount at /idmapped.
When a caller with fs{g,u}id 1000 creates a file "file2" by going
through the idmapped mount mounted at /idmapped it will create a file
that is owned by uid 1000 on-disk, i.e.:

- looking at it via /unmapped/file2 will report it as owned by uid 1000
- looking at it via /idmapped/file2 will report it as owned by uid 0

Now consider an mds server that has a uid access restriction set and
only grants access to requests from uid 0.

If the client sends a creation request for a file e.g. /idmapped/file2
it will send the caller's fs{g,u}id idmapped according to the idmapped
mount. So if the caller has fs{g,u}id 1000 it will be mapped to {g,u}id
0 in the idmapped mount and will be sent over the wire allowing the
caller access to the mds server.

However, if the caller is not issuing a creation request the caller's
fs{g,u}id will be send without the mount's idmapping applied. So if the
caller that just successfully created a new file on the restricted mds
server sends a request as fs{g,u}id 1000 access will be refused. This
however is inconsistent.

From my perspective the root of the problem lies in the fact that
creation requests implicitly infer the ownership from the {g,u}id that
gets sent along with every mds request.

I have thought of multiple ways of addressing this problem but the one I
prefer is to give all mds requests that create a filesystem object a
proper, separate {g,u}id field entry in the argument struct. This is,
for example how ->setattr mds requests work.

This way the caller's fs{g,u}id can be used consistenly for server
access checks and is separated from the ownership for new filesystem
objects.

Servers could then be updated to refuse creation requests whenever the
{g,u}id used for access checking doesn't match the {g,u}id used for
creating the filesystem object just as is done for setattr requests on a
uid restricted server. But I am, of course, open to other suggestions.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/mds_client.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index ae2cc4ce1d48..1fb43a8fd64c 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2459,6 +2459,8 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
 	void *p, *end;
 	int ret;
 	bool legacy = !(session->s_con.peer_features & CEPH_FEATURE_FS_BTIME);
+	kuid_t caller_fsuid;
+	kgid_t caller_fsgid;
 
 	ret = set_request_path_attr(req->r_inode, req->r_dentry,
 			      req->r_parent, req->r_path1, req->r_ino1.ino,
@@ -2524,10 +2526,22 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
 
 	head->mdsmap_epoch = cpu_to_le32(mdsc->mdsmap->m_epoch);
 	head->op = cpu_to_le32(req->r_op);
-	head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns,
-						 req->r_cred->fsuid));
-	head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns,
-						 req->r_cred->fsgid));
+	/*
+	 * Inode operations that create filesystem objects based on the
+	 * caller's fs{g,u}id like ->mknod(), ->create(), ->mkdir() etc. don't
+	 * have separate {g,u}id fields in their respective structs in the
+	 * ceph_mds_request_args union. Instead the caller_{g,u}id field is
+	 * used to set ownership of the newly created inode by the mds server.
+	 * For these inode operations we need to send the mapped fs{g,u}id over
+	 * the wire. For other cases we simple set req->mnt_userns to the
+	 * initial idmapping meaning the unmapped fs{g,u}id is sent.
+	 */
+	caller_fsuid = mapped_kuid_user(req->mnt_userns, &init_user_ns,
+					req->r_cred->fsuid);
+	caller_fsgid = mapped_kgid_user(req->mnt_userns, &init_user_ns,
+					req->r_cred->fsgid);
+	head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns, caller_fsuid));
+	head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns, caller_fsgid));
 	head->ino = cpu_to_le64(req->r_deleg_ino);
 	head->args = req->r_args;
 
-- 
2.32.0

[PATCH 03/12] ceph: allow idmapped mknod inode op

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Enable ceph_mknod() to handle idmapped mounts. This is just a matter of
passing down the mount's idmapping.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/dir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 133dbd9338e7..7278863fbd4a 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -871,6 +871,7 @@ static int ceph_mknod(struct user_namespace *mnt_userns, struct inode *dir,
 	req->r_parent = dir;
 	ihold(dir);
 	set_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags);
+	req->mnt_userns = mnt_userns;
 	req->r_args.mknod.mode = cpu_to_le32(mode);
 	req->r_args.mknod.rdev = cpu_to_le32(rdev);
 	req->r_dentry_drop = CEPH_CAP_FILE_SHARED | CEPH_CAP_AUTH_EXCL;
-- 
2.32.0

[PATCH 04/12] ceph: allow idmapped symlink inode op

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Enable ceph_symlink() to handle idmapped mounts. This is just a matter
of passing down the mount's idmapping.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/dir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 7278863fbd4a..67ce448a9ff5 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -939,6 +939,7 @@ static int ceph_symlink(struct user_namespace *mnt_userns, struct inode *dir,
 	req->r_num_caps = 2;
 	req->r_dentry_drop = CEPH_CAP_FILE_SHARED | CEPH_CAP_AUTH_EXCL;
 	req->r_dentry_unless = CEPH_CAP_FILE_EXCL;
+	req->mnt_userns = mnt_userns;
 	if (as_ctx.pagelist) {
 		req->r_pagelist = as_ctx.pagelist;
 		as_ctx.pagelist = NULL;
-- 
2.32.0

[PATCH 05/12] ceph: allow idmapped mkdir inode op

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Enable ceph_mkdir() to handle idmapped mounts. This is just a matter of
passing down the mount's idmapping.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/dir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 67ce448a9ff5..210257afb346 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1002,6 +1002,7 @@ static int ceph_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
 	ihold(dir);
 	set_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags);
 	req->r_args.mkdir.mode = cpu_to_le32(mode);
+	req->mnt_userns = mnt_userns;
 	req->r_dentry_drop = CEPH_CAP_FILE_SHARED | CEPH_CAP_AUTH_EXCL;
 	req->r_dentry_unless = CEPH_CAP_FILE_EXCL;
 	if (as_ctx.pagelist) {
-- 
2.32.0

[PATCH 06/12] ceph: allow idmapped rename inode op

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Enable ceph_rename() to handle idmapped mounts. This is just a matter of
passing down the mount's idmapping.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/dir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 210257afb346..9463c960f03b 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1248,6 +1248,7 @@ static int ceph_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
 	req->r_old_dentry_unless = CEPH_CAP_FILE_EXCL;
 	req->r_dentry_drop = CEPH_CAP_FILE_SHARED;
 	req->r_dentry_unless = CEPH_CAP_FILE_EXCL;
+	req->mnt_userns = mnt_userns;
 	/* release LINK_RDCACHE on source inode (mds will lock it) */
 	req->r_old_inode_drop = CEPH_CAP_LINK_SHARED | CEPH_CAP_LINK_EXCL;
 	if (d_really_is_positive(new_dentry)) {
-- 
2.32.0

[PATCH 07/12] ceph: allow idmapped getattr inode op

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Enable ceph_getattr() to handle idmapped mounts. This is just a matter
of passing down the mount's idmapping.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index e3322fcb2e8d..f648aecc5827 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2364,7 +2364,7 @@ int ceph_getattr(struct user_namespace *mnt_userns, const struct path *path,
 			return err;
 	}
 
-	generic_fillattr(&init_user_ns, inode, stat);
+	generic_fillattr(mnt_userns, inode, stat);
 	stat->ino = ceph_present_inode(inode);
 
 	/*
-- 
2.32.0

[PATCH 08/12] ceph: allow idmapped permission inode op

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Enable ceph_permission() to handle idmapped mounts. This is just a
matter of passing down the mount's idmapping.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index f648aecc5827..d5cecc3519fa 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2307,7 +2307,7 @@ int ceph_permission(struct user_namespace *mnt_userns, struct inode *inode,
 	err = ceph_do_getattr(inode, CEPH_CAP_AUTH_SHARED, false);
 
 	if (!err)
-		err = generic_permission(&init_user_ns, inode, mask);
+		err = generic_permission(mnt_userns, inode, mask);
 	return err;
 }
 
-- 
2.32.0

[PATCH 09/12] ceph: allow idmapped setattr inode op

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Enable __ceph_setattr() to handle idmapped mounts. This is just a matter
of passing down the mount's idmapping.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/inode.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index d5cecc3519fa..658b620efd50 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2032,6 +2032,13 @@ int __ceph_setattr(struct inode *inode, struct iattr *attr)
 
 	dout("setattr %p issued %s\n", inode, ceph_cap_string(issued));
 
+	/*
+	 * The attr->ia_{g,u}id members contain the target {g,u}id we're
+	 * sending over the wire. The mount idmapping only matters when we
+	 * create new filesystem objects based on the caller's mapped
+	 * fs{g,u}id.
+	 */
+	req->mnt_userns = &init_user_ns;
 	if (ia_valid & ATTR_UID) {
 		dout("setattr %p uid %d -> %d\n", inode,
 		     from_kuid(&init_user_ns, inode->i_uid),
@@ -2222,7 +2229,7 @@ int ceph_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
 	if (ceph_inode_is_shutdown(inode))
 		return -ESTALE;
 
-	err = setattr_prepare(&init_user_ns, dentry, attr);
+	err = setattr_prepare(mnt_userns, dentry, attr);
 	if (err != 0)
 		return err;
 
@@ -2237,7 +2244,7 @@ int ceph_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
 	err = __ceph_setattr(inode, attr);
 
 	if (err >= 0 && (attr->ia_valid & ATTR_MODE))
-		err = posix_acl_chmod(&init_user_ns, inode, attr->ia_mode);
+		err = posix_acl_chmod(mnt_userns, inode, attr->ia_mode);
 
 	return err;
 }
-- 
2.32.0

[PATCH 10/12] ceph/acl: allow idmapped set_acl inode op

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Enable ceph_set_acl() to handle idmapped mounts. This is just a matter
of passing down the mount's idmapping.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
index f4fc8e0b847c..7957d44bb27c 100644
--- a/fs/ceph/acl.c
+++ b/fs/ceph/acl.c
@@ -104,7 +104,7 @@ int ceph_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
 	case ACL_TYPE_ACCESS:
 		name = XATTR_NAME_POSIX_ACL_ACCESS;
 		if (acl) {
-			ret = posix_acl_update_mode(&init_user_ns, inode,
+			ret = posix_acl_update_mode(mnt_userns, inode,
 						    &new_mode, &acl);
 			if (ret)
 				goto out;
-- 
2.32.0

[PATCH 11/12] ceph/file: allow idmapped atomic_open inode op

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Enable ceph_atomic_open() to handle idmapped mounts. This is just a
matter of passing down the mount's idmapping.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/file.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index c138e8126286..7fecb41796c7 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -608,7 +608,10 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry,
 	in.truncate_seq = cpu_to_le32(1);
 	in.truncate_size = cpu_to_le64(-1ULL);
 	in.xattr_version = cpu_to_le64(1);
-	in.uid = cpu_to_le32(from_kuid(&init_user_ns, current_fsuid()));
+	in.uid = cpu_to_le32(from_kuid(&init_user_ns,
+				       mapped_kuid_user(req->mnt_userns,
+							&init_user_ns,
+							current_fsuid())));
 	if (dir->i_mode & S_ISGID) {
 		in.gid = cpu_to_le32(from_kgid(&init_user_ns, dir->i_gid));
 
@@ -616,11 +619,14 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry,
 		if (S_ISDIR(mode))
 			mode |= S_ISGID;
 		else if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP) &&
-			 !in_group_p(dir->i_gid) &&
-			 !capable_wrt_inode_uidgid(&init_user_ns, dir, CAP_FSETID))
+			 !in_group_p(i_gid_into_mnt(req->mnt_userns, dir)) &&
+			 !capable_wrt_inode_uidgid(req->mnt_userns, dir, CAP_FSETID))
 			mode &= ~S_ISGID;
 	} else {
-		in.gid = cpu_to_le32(from_kgid(&init_user_ns, current_fsgid()));
+		in.gid = cpu_to_le32(from_kgid(&init_user_ns,
+				     mapped_kgid_user(req->mnt_userns,
+						      &init_user_ns,
+						      current_fsgid())));
 	}
 	in.mode = cpu_to_le32((u32)mode);
 
@@ -677,6 +683,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 		     struct file *file, unsigned flags, umode_t mode)
 {
 	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
+	struct user_namespace *mnt_userns = file_mnt_user_ns(file);
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_mds_request *req;
 	struct dentry *dn;
@@ -719,6 +726,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 		mask |= CEPH_CAP_XATTR_SHARED;
 	req->r_args.open.mask = cpu_to_le32(mask);
 	req->r_parent = dir;
+	req->mnt_userns = mnt_userns;
 	ihold(dir);
 
 	if (flags & O_CREAT) {
-- 
2.32.0

[PATCH 12/12] ceph: allow idmapped mounts

[Christian Brauner]

From: Christian Brauner <christian.brauner@ubuntu.com>

Now that we converted cephfs internally to account for idmapped mounts
allow the creation of idmapped mounts on by setting the FS_ALLOW_IDMAP
flag.

Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/ceph/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index bab61232dc5a..eda4a26fcb0c 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1242,7 +1242,7 @@ static struct file_system_type ceph_fs_type = {
 	.name		= "ceph",
 	.init_fs_context = ceph_init_fs_context,
 	.kill_sb	= ceph_kill_sb,
-	.fs_flags	= FS_RENAME_DOES_D_MOVE,
+	.fs_flags	= FS_RENAME_DOES_D_MOVE | FS_ALLOW_IDMAP,
 };
 MODULE_ALIAS_FS("ceph");
 
-- 
2.32.0

Re: [PATCH 01/12] ceph: stash idmapping in mdsc request

[Jeff Layton]

On Tue, 2022-01-04 at 15:04 +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
> 
> When sending a mds request cephfs will send relevant data for the
> requested operation. For creation requests the caller's fs{g,u}id is
> used to set the ownership of the newly created filesystem object. For
> setattr requests the caller can pass in arbitrary {g,u}id values to
> which the relevant filesystem object is supposed to be changed.
> 
> If the caller is performing the relevant operation via an idmapped mount
> cephfs simply needs to take the idmapping into account when it sends the
> relevant mds request.
> 
> In order to support idmapped mounts for cephfs we stash the idmapping
> whenever they are relevant for the operation for the duration of the
> request. Since mds requests can be queued and performed asynchronously
> we make sure to keep the idmapping around and release it once the
> request has finished.
> 
> In follow-up patches we will use this to send correct ownership
> information over the wire. This patch just adds the basic infrastructure
> to keep the idmapping around. The actual conversion patches are all
> fairly minimal.
> 
> Cc: Jeff Layton <jlayton@kernel.org>
> Cc: Ilya Dryomov <idryomov@gmail.com>
> Cc: ceph-devel@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
>  fs/ceph/mds_client.c | 7 +++++++
>  fs/ceph/mds_client.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index c30eefc0ac19..ae2cc4ce1d48 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -12,6 +12,7 @@
>  #include <linux/bits.h>
>  #include <linux/ktime.h>
>  #include <linux/bitmap.h>
> +#include <linux/mnt_idmapping.h>
>  
>  #include "super.h"
>  #include "mds_client.h"
> @@ -862,6 +863,8 @@ void ceph_mdsc_release_request(struct kref *kref)
>  	kfree(req->r_path1);
>  	kfree(req->r_path2);
>  	put_cred(req->r_cred);
> +	if (!initial_idmapping(req->mnt_userns))
> +		put_user_ns(req->mnt_userns);

I assume initial_idmapping returns true if it's init_user_ns ?

>  	if (req->r_pagelist)
>  		ceph_pagelist_release(req->r_pagelist);
>  	put_request_session(req);
> @@ -918,6 +921,10 @@ static void __register_request(struct ceph_mds_client *mdsc,
>  	insert_request(&mdsc->request_tree, req);
>  
>  	req->r_cred = get_current_cred();
> +	if (!req->mnt_userns)
> +		req->mnt_userns = &init_user_ns;
> +	else
> +		get_user_ns(req->mnt_userns);
>  
>  	if (mdsc->oldest_tid == 0 && req->r_op != CEPH_MDS_OP_SETFILELOCK)
>  		mdsc->oldest_tid = req->r_tid;
> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> index 97c7f7bfa55f..1b648645e340 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -275,6 +275,7 @@ struct ceph_mds_request {
>  	union ceph_mds_request_args r_args;
>  	int r_fmode;        /* file mode, if expecting cap */
>  	const struct cred *r_cred;
> +	struct user_namespace *mnt_userns;

Can we call this r_mnt_userns to match the other fields ?

>  	int r_request_release_offset;
>  	struct timespec64 r_stamp;
>  

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 02/12] ceph: handle idmapped mounts in create_request_message()

[Jeff Layton]

On Tue, 2022-01-04 at 15:04 +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
> 
> Inode operations that create a new filesystem object such as ->mknod,
> ->create, ->mkdir() and others don't take a {g,u}id argument explicitly.
> Instead the caller's fs{g,u}id is used for the {g,u}id of the new
> filesystem object.
> 
> Cephfs mds creation request argument structures mirror this filesystem
> behavior. They don't encode a {g,u}id explicitly. Instead the caller's
> fs{g,u}id that is always sent as part of any mds request is used by the
> servers to set the {g,u}id of the new filesystem object.
> 
> In order to ensure that the correct {g,u}id is used map the caller's
> fs{g,u}id for creation requests. This doesn't require complex changes.
> It suffices to pass in the relevant idmapping recorded in the request
> message. If this request message was triggered from an inode operation
> that creates filesystem objects it will have passed down the relevant
> idmaping. If this is a request message that was triggered from an inode
> operation that doens't need to take idmappings into account the initial
> idmapping is passed down which is an identity mapping and thus is
> guaranteed to leave the caller's fs{g,u}id unchanged.,u}id is sent.
> 
> The last few weeks before Christmas 2021 I have spent time not just
> reading and poking the cephfs kernel code but also took a look at the
> ceph mds server userspace to ensure I didn't miss some subtlety.
> 
> This made me aware of one complication to solve. All requests send the
> caller's fs{g,u}id over the wire. The caller's fs{g,u}id matters for the
> server in exactly two cases:
> 
> 1. to set the ownership for creation requests
> 2. to determine whether this client is allowed access on this server
> 
> Case 1. we already covered and explained. Case 2. is only relevant for
> servers where an explicit uid access restriction has been set. That is
> to say the mds server restricts access to requests coming from a
> specific uid. Servers without uid restrictions will grant access to
> requests from any uid by setting MDS_AUTH_UID_ANY.
> 
> Case 2. introduces the complication because the caller's fs{g,u}id is
> not just used to record ownership but also serves as the {g,u}id used
> when checking access to the server.
> 
> Consider a user mounting a cephfs client and creating an idmapped mount
> from it that maps files owned by uid 1000 to be owned uid 0:
> 
> mount -t cephfs -o [...] /unmapped
> mount-idmapped --map-mount 1000:0:1 /idmapped
> 
> That is to say if the mounted cephfs filesystem contains a file "file1"
> which is owned by uid 1000:
> 
> - looking at it via /unmapped/file1 will report it as owned by uid 1000
>   (One can think of this as the on-disk value.)
> - looking at it via /idmapped/file1 will report it as owned by uid 0
> 
> Now, consider creating new files via the idmapped mount at /idmapped.
> When a caller with fs{g,u}id 1000 creates a file "file2" by going
> through the idmapped mount mounted at /idmapped it will create a file
> that is owned by uid 1000 on-disk, i.e.:
> 
> - looking at it via /unmapped/file2 will report it as owned by uid 1000
> - looking at it via /idmapped/file2 will report it as owned by uid 0
> 
> Now consider an mds server that has a uid access restriction set and
> only grants access to requests from uid 0.
> 
> If the client sends a creation request for a file e.g. /idmapped/file2
> it will send the caller's fs{g,u}id idmapped according to the idmapped
> mount. So if the caller has fs{g,u}id 1000 it will be mapped to {g,u}id
> 0 in the idmapped mount and will be sent over the wire allowing the
> caller access to the mds server.
> 
> However, if the caller is not issuing a creation request the caller's
> fs{g,u}id will be send without the mount's idmapping applied. So if the
> caller that just successfully created a new file on the restricted mds
> server sends a request as fs{g,u}id 1000 access will be refused. This
> however is inconsistent.
> 

IDGI, why would you send the fs{g,u}id without the mount's idmapping
applied in this case? ISTM that idmapping is wholly a client-side
feature, and that you should always map id's regardless of whether
you're creating or not.

> From my perspective the root of the problem lies in the fact that
> creation requests implicitly infer the ownership from the {g,u}id that
> gets sent along with every mds request.
> 
> I have thought of multiple ways of addressing this problem but the one I
> prefer is to give all mds requests that create a filesystem object a
> proper, separate {g,u}id field entry in the argument struct. This is,
> for example how ->setattr mds requests work.
> 
> This way the caller's fs{g,u}id can be used consistenly for server
> access checks and is separated from the ownership for new filesystem
> objects.
> 
> Servers could then be updated to refuse creation requests whenever the
> {g,u}id used for access checking doesn't match the {g,u}id used for
> creating the filesystem object just as is done for setattr requests on a
> uid restricted server. But I am, of course, open to other suggestions.
> 
> Cc: Jeff Layton <jlayton@kernel.org>
> Cc: Ilya Dryomov <idryomov@gmail.com>
> Cc: ceph-devel@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
>  fs/ceph/mds_client.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index ae2cc4ce1d48..1fb43a8fd64c 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -2459,6 +2459,8 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
>  	void *p, *end;
>  	int ret;
>  	bool legacy = !(session->s_con.peer_features & CEPH_FEATURE_FS_BTIME);
> +	kuid_t caller_fsuid;
> +	kgid_t caller_fsgid;
>  
>  	ret = set_request_path_attr(req->r_inode, req->r_dentry,
>  			      req->r_parent, req->r_path1, req->r_ino1.ino,
> @@ -2524,10 +2526,22 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
>  
>  	head->mdsmap_epoch = cpu_to_le32(mdsc->mdsmap->m_epoch);
>  	head->op = cpu_to_le32(req->r_op);
> -	head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns,
> -						 req->r_cred->fsuid));
> -	head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns,
> -						 req->r_cred->fsgid));
> +	/*
> +	 * Inode operations that create filesystem objects based on the
> +	 * caller's fs{g,u}id like ->mknod(), ->create(), ->mkdir() etc. don't
> +	 * have separate {g,u}id fields in their respective structs in the
> +	 * ceph_mds_request_args union. Instead the caller_{g,u}id field is
> +	 * used to set ownership of the newly created inode by the mds server.
> +	 * For these inode operations we need to send the mapped fs{g,u}id over
> +	 * the wire. For other cases we simple set req->mnt_userns to the
> +	 * initial idmapping meaning the unmapped fs{g,u}id is sent.
> +	 */
> +	caller_fsuid = mapped_kuid_user(req->mnt_userns, &init_user_ns,
> +					req->r_cred->fsuid);
> +	caller_fsgid = mapped_kgid_user(req->mnt_userns, &init_user_ns,
> +					req->r_cred->fsgid);
> +	head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns, caller_fsuid));
> +	head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns, caller_fsgid));
>  	head->ino = cpu_to_le64(req->r_deleg_ino);
>  	head->args = req->r_args;
>  

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 02/12] ceph: handle idmapped mounts in create_request_message()

[Gregory Farnum]

On Tue, Jan 4, 2022 at 9:41 AM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Tue, 2022-01-04 at 15:04 +0100, Christian Brauner wrote:
> > From: Christian Brauner <christian.brauner@ubuntu.com>
> >
> > Inode operations that create a new filesystem object such as ->mknod,
> > ->create, ->mkdir() and others don't take a {g,u}id argument explicitly.
> > Instead the caller's fs{g,u}id is used for the {g,u}id of the new
> > filesystem object.
> >
> > Cephfs mds creation request argument structures mirror this filesystem
> > behavior. They don't encode a {g,u}id explicitly. Instead the caller's
> > fs{g,u}id that is always sent as part of any mds request is used by the
> > servers to set the {g,u}id of the new filesystem object.
> >
> > In order to ensure that the correct {g,u}id is used map the caller's
> > fs{g,u}id for creation requests. This doesn't require complex changes.
> > It suffices to pass in the relevant idmapping recorded in the request
> > message. If this request message was triggered from an inode operation
> > that creates filesystem objects it will have passed down the relevant
> > idmaping. If this is a request message that was triggered from an inode
> > operation that doens't need to take idmappings into account the initial
> > idmapping is passed down which is an identity mapping and thus is
> > guaranteed to leave the caller's fs{g,u}id unchanged.,u}id is sent.
> >
> > The last few weeks before Christmas 2021 I have spent time not just
> > reading and poking the cephfs kernel code but also took a look at the
> > ceph mds server userspace to ensure I didn't miss some subtlety.
> >
> > This made me aware of one complication to solve. All requests send the
> > caller's fs{g,u}id over the wire. The caller's fs{g,u}id matters for the
> > server in exactly two cases:
> >
> > 1. to set the ownership for creation requests
> > 2. to determine whether this client is allowed access on this server
> >
> > Case 1. we already covered and explained. Case 2. is only relevant for
> > servers where an explicit uid access restriction has been set. That is
> > to say the mds server restricts access to requests coming from a
> > specific uid. Servers without uid restrictions will grant access to
> > requests from any uid by setting MDS_AUTH_UID_ANY.
> >
> > Case 2. introduces the complication because the caller's fs{g,u}id is
> > not just used to record ownership but also serves as the {g,u}id used
> > when checking access to the server.
> >
> > Consider a user mounting a cephfs client and creating an idmapped mount
> > from it that maps files owned by uid 1000 to be owned uid 0:
> >
> > mount -t cephfs -o [...] /unmapped
> > mount-idmapped --map-mount 1000:0:1 /idmapped
> >
> > That is to say if the mounted cephfs filesystem contains a file "file1"
> > which is owned by uid 1000:
> >
> > - looking at it via /unmapped/file1 will report it as owned by uid 1000
> >   (One can think of this as the on-disk value.)
> > - looking at it via /idmapped/file1 will report it as owned by uid 0
> >
> > Now, consider creating new files via the idmapped mount at /idmapped.
> > When a caller with fs{g,u}id 1000 creates a file "file2" by going
> > through the idmapped mount mounted at /idmapped it will create a file
> > that is owned by uid 1000 on-disk, i.e.:
> >
> > - looking at it via /unmapped/file2 will report it as owned by uid 1000
> > - looking at it via /idmapped/file2 will report it as owned by uid 0
> >
> > Now consider an mds server that has a uid access restriction set and
> > only grants access to requests from uid 0.
> >
> > If the client sends a creation request for a file e.g. /idmapped/file2
> > it will send the caller's fs{g,u}id idmapped according to the idmapped
> > mount. So if the caller has fs{g,u}id 1000 it will be mapped to {g,u}id
> > 0 in the idmapped mount and will be sent over the wire allowing the
> > caller access to the mds server.
> >
> > However, if the caller is not issuing a creation request the caller's
> > fs{g,u}id will be send without the mount's idmapping applied. So if the
> > caller that just successfully created a new file on the restricted mds
> > server sends a request as fs{g,u}id 1000 access will be refused. This
> > however is inconsistent.
> >
>
> IDGI, why would you send the fs{g,u}id without the mount's idmapping
> applied in this case? ISTM that idmapping is wholly a client-side
> feature, and that you should always map id's regardless of whether
> you're creating or not.

Yeah, I'm confused. We want the fs {g,u}id to be consistent throughout
the request pipeline and to reflect the actual Ceph user all the way
through the server-side pipeline. What if client.greg is only
authorized to work as uid 12345 and access /users/greg/; why would you
send in a bunch of requests as root just because I mounted that way
inside my own space?

This might be more obvious in the userspace Client, which is already
set up to be friendlier to mapped users for Ganesha etc:
mknod (https://github.com/ceph/ceph/blob/master/src/client/Client.cc#L7297)
and similar calls receive a "UserPerm" from the caller specifying who
the call should be performed as, and they pass that in to the generic
make_requst() function
(https://github.com/ceph/ceph/blob/master/src/client/Client.cc#L1778)
which uses it to set the uid and gid fields you found in the message.
-Greg

> > From my perspective the root of the problem lies in the fact that
> > creation requests implicitly infer the ownership from the {g,u}id that
> > gets sent along with every mds request.
> >
> > I have thought of multiple ways of addressing this problem but the one I
> > prefer is to give all mds requests that create a filesystem object a
> > proper, separate {g,u}id field entry in the argument struct. This is,
> > for example how ->setattr mds requests work.
> >
> > This way the caller's fs{g,u}id can be used consistenly for server
> > access checks and is separated from the ownership for new filesystem
> > objects.
> >
> > Servers could then be updated to refuse creation requests whenever the
> > {g,u}id used for access checking doesn't match the {g,u}id used for
> > creating the filesystem object just as is done for setattr requests on a
> > uid restricted server. But I am, of course, open to other suggestions.
> >
> > Cc: Jeff Layton <jlayton@kernel.org>
> > Cc: Ilya Dryomov <idryomov@gmail.com>
> > Cc: ceph-devel@vger.kernel.org
> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > ---
> >  fs/ceph/mds_client.c | 22 ++++++++++++++++++----
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > index ae2cc4ce1d48..1fb43a8fd64c 100644
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -2459,6 +2459,8 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
> >       void *p, *end;
> >       int ret;
> >       bool legacy = !(session->s_con.peer_features & CEPH_FEATURE_FS_BTIME);
> > +     kuid_t caller_fsuid;
> > +     kgid_t caller_fsgid;
> >
> >       ret = set_request_path_attr(req->r_inode, req->r_dentry,
> >                             req->r_parent, req->r_path1, req->r_ino1.ino,
> > @@ -2524,10 +2526,22 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
> >
> >       head->mdsmap_epoch = cpu_to_le32(mdsc->mdsmap->m_epoch);
> >       head->op = cpu_to_le32(req->r_op);
> > -     head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns,
> > -                                              req->r_cred->fsuid));
> > -     head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns,
> > -                                              req->r_cred->fsgid));
> > +     /*
> > +      * Inode operations that create filesystem objects based on the
> > +      * caller's fs{g,u}id like ->mknod(), ->create(), ->mkdir() etc. don't
> > +      * have separate {g,u}id fields in their respective structs in the
> > +      * ceph_mds_request_args union. Instead the caller_{g,u}id field is
> > +      * used to set ownership of the newly created inode by the mds server.
> > +      * For these inode operations we need to send the mapped fs{g,u}id over
> > +      * the wire. For other cases we simple set req->mnt_userns to the
> > +      * initial idmapping meaning the unmapped fs{g,u}id is sent.
> > +      */
> > +     caller_fsuid = mapped_kuid_user(req->mnt_userns, &init_user_ns,
> > +                                     req->r_cred->fsuid);
> > +     caller_fsgid = mapped_kgid_user(req->mnt_userns, &init_user_ns,
> > +                                     req->r_cred->fsgid);
> > +     head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns, caller_fsuid));
> > +     head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns, caller_fsgid));
> >       head->ino = cpu_to_le64(req->r_deleg_ino);
> >       head->args = req->r_args;
> >
>
> --
> Jeff Layton <jlayton@kernel.org>
>

Re: [PATCH 02/12] ceph: handle idmapped mounts in create_request_message()

[Christian Brauner]

On Tue, Jan 04, 2022 at 12:40:51PM -0500, Jeff Layton wrote:
> On Tue, 2022-01-04 at 15:04 +0100, Christian Brauner wrote:
> > From: Christian Brauner <christian.brauner@ubuntu.com>
> > 
> > Inode operations that create a new filesystem object such as ->mknod,
> > ->create, ->mkdir() and others don't take a {g,u}id argument explicitly.
> > Instead the caller's fs{g,u}id is used for the {g,u}id of the new
> > filesystem object.
> > 
> > Cephfs mds creation request argument structures mirror this filesystem
> > behavior. They don't encode a {g,u}id explicitly. Instead the caller's
> > fs{g,u}id that is always sent as part of any mds request is used by the
> > servers to set the {g,u}id of the new filesystem object.
> > 
> > In order to ensure that the correct {g,u}id is used map the caller's
> > fs{g,u}id for creation requests. This doesn't require complex changes.
> > It suffices to pass in the relevant idmapping recorded in the request
> > message. If this request message was triggered from an inode operation
> > that creates filesystem objects it will have passed down the relevant
> > idmaping. If this is a request message that was triggered from an inode
> > operation that doens't need to take idmappings into account the initial
> > idmapping is passed down which is an identity mapping and thus is
> > guaranteed to leave the caller's fs{g,u}id unchanged.,u}id is sent.
> > 
> > The last few weeks before Christmas 2021 I have spent time not just
> > reading and poking the cephfs kernel code but also took a look at the
> > ceph mds server userspace to ensure I didn't miss some subtlety.
> > 
> > This made me aware of one complication to solve. All requests send the
> > caller's fs{g,u}id over the wire. The caller's fs{g,u}id matters for the
> > server in exactly two cases:
> > 
> > 1. to set the ownership for creation requests
> > 2. to determine whether this client is allowed access on this server
> > 
> > Case 1. we already covered and explained. Case 2. is only relevant for
> > servers where an explicit uid access restriction has been set. That is
> > to say the mds server restricts access to requests coming from a
> > specific uid. Servers without uid restrictions will grant access to
> > requests from any uid by setting MDS_AUTH_UID_ANY.
> > 
> > Case 2. introduces the complication because the caller's fs{g,u}id is
> > not just used to record ownership but also serves as the {g,u}id used
> > when checking access to the server.
> > 
> > Consider a user mounting a cephfs client and creating an idmapped mount
> > from it that maps files owned by uid 1000 to be owned uid 0:
> > 
> > mount -t cephfs -o [...] /unmapped
> > mount-idmapped --map-mount 1000:0:1 /idmapped
> > 
> > That is to say if the mounted cephfs filesystem contains a file "file1"
> > which is owned by uid 1000:
> > 
> > - looking at it via /unmapped/file1 will report it as owned by uid 1000
> >   (One can think of this as the on-disk value.)
> > - looking at it via /idmapped/file1 will report it as owned by uid 0
> > 
> > Now, consider creating new files via the idmapped mount at /idmapped.
> > When a caller with fs{g,u}id 1000 creates a file "file2" by going
> > through the idmapped mount mounted at /idmapped it will create a file
> > that is owned by uid 1000 on-disk, i.e.:
> > 
> > - looking at it via /unmapped/file2 will report it as owned by uid 1000
> > - looking at it via /idmapped/file2 will report it as owned by uid 0
> > 
> > Now consider an mds server that has a uid access restriction set and
> > only grants access to requests from uid 0.
> > 
> > If the client sends a creation request for a file e.g. /idmapped/file2
> > it will send the caller's fs{g,u}id idmapped according to the idmapped
> > mount. So if the caller has fs{g,u}id 1000 it will be mapped to {g,u}id
> > 0 in the idmapped mount and will be sent over the wire allowing the
> > caller access to the mds server.
> > 
> > However, if the caller is not issuing a creation request the caller's
> > fs{g,u}id will be send without the mount's idmapping applied. So if the
> > caller that just successfully created a new file on the restricted mds
> > server sends a request as fs{g,u}id 1000 access will be refused. This
> > however is inconsistent.
> > 
> 
> IDGI, why would you send the fs{g,u}id without the mount's idmapping
> applied in this case? ISTM that idmapping is wholly a client-side
> feature, and that you should always map id's regardless of whether
> you're creating or not.

Since the idmapping is a property of the mount and not a property of the
caller the caller's fs{g,u}id aren't mapped. What is mapped are the
inode's i{g,u}id when accessed from a particular mount.

The fs{g,u}id are only ever mapped when a new filesystem object is
created. So if I have an idmapped mount that makes it so that files
owned by 1000 on-disk appear to be owned by uid 0 then a user with uid 0
creating a new file will create files with uid 1000 on-disk when going
through that mount. For cephfs that'd be the uid we would be sending
with creation requests as I've currently written it.

So then when the user looks at the file it created it will see it as
being owned by uid 0 from that idmapped mount (whereas on-disk it's
1000). But the user's fs{g,u}id isn't per se changed when going through
that mount. So in my opinion I was thinking that the server with access
permissions set would want to always check permissions on the users
"raw" fs{g,u}id. That would mean I'd have to change the patch obviously.
My suggestion was to send the {g,u}id the file will be created with
separately. The alternative would be to not just pass the idmapping into
the creation iop's but into all iops so that we can always map it for
cephfs. But this would mean a lot of vfs changes for one filesystem. So
if we could first explore alternatives approaches I'd be grateful.

(I'll be traveling for the latter half of this week starting today at
CET afternoon so apologies but I'll probably take some time to respond.)

Re: [PATCH 02/12] ceph: handle idmapped mounts in create_request_message()

[Christian Brauner]

On Tue, Jan 04, 2022 at 11:33:33AM -0800, Gregory Farnum wrote:
> On Tue, Jan 4, 2022 at 9:41 AM Jeff Layton <jlayton@kernel.org> wrote:
> >
> > On Tue, 2022-01-04 at 15:04 +0100, Christian Brauner wrote:
> > > From: Christian Brauner <christian.brauner@ubuntu.com>
> > >
> > > Inode operations that create a new filesystem object such as ->mknod,
> > > ->create, ->mkdir() and others don't take a {g,u}id argument explicitly.
> > > Instead the caller's fs{g,u}id is used for the {g,u}id of the new
> > > filesystem object.
> > >
> > > Cephfs mds creation request argument structures mirror this filesystem
> > > behavior. They don't encode a {g,u}id explicitly. Instead the caller's
> > > fs{g,u}id that is always sent as part of any mds request is used by the
> > > servers to set the {g,u}id of the new filesystem object.
> > >
> > > In order to ensure that the correct {g,u}id is used map the caller's
> > > fs{g,u}id for creation requests. This doesn't require complex changes.
> > > It suffices to pass in the relevant idmapping recorded in the request
> > > message. If this request message was triggered from an inode operation
> > > that creates filesystem objects it will have passed down the relevant
> > > idmaping. If this is a request message that was triggered from an inode
> > > operation that doens't need to take idmappings into account the initial
> > > idmapping is passed down which is an identity mapping and thus is
> > > guaranteed to leave the caller's fs{g,u}id unchanged.,u}id is sent.
> > >
> > > The last few weeks before Christmas 2021 I have spent time not just
> > > reading and poking the cephfs kernel code but also took a look at the
> > > ceph mds server userspace to ensure I didn't miss some subtlety.
> > >
> > > This made me aware of one complication to solve. All requests send the
> > > caller's fs{g,u}id over the wire. The caller's fs{g,u}id matters for the
> > > server in exactly two cases:
> > >
> > > 1. to set the ownership for creation requests
> > > 2. to determine whether this client is allowed access on this server
> > >
> > > Case 1. we already covered and explained. Case 2. is only relevant for
> > > servers where an explicit uid access restriction has been set. That is
> > > to say the mds server restricts access to requests coming from a
> > > specific uid. Servers without uid restrictions will grant access to
> > > requests from any uid by setting MDS_AUTH_UID_ANY.
> > >
> > > Case 2. introduces the complication because the caller's fs{g,u}id is
> > > not just used to record ownership but also serves as the {g,u}id used
> > > when checking access to the server.
> > >
> > > Consider a user mounting a cephfs client and creating an idmapped mount
> > > from it that maps files owned by uid 1000 to be owned uid 0:
> > >
> > > mount -t cephfs -o [...] /unmapped
> > > mount-idmapped --map-mount 1000:0:1 /idmapped
> > >
> > > That is to say if the mounted cephfs filesystem contains a file "file1"
> > > which is owned by uid 1000:
> > >
> > > - looking at it via /unmapped/file1 will report it as owned by uid 1000
> > >   (One can think of this as the on-disk value.)
> > > - looking at it via /idmapped/file1 will report it as owned by uid 0
> > >
> > > Now, consider creating new files via the idmapped mount at /idmapped.
> > > When a caller with fs{g,u}id 1000 creates a file "file2" by going
> > > through the idmapped mount mounted at /idmapped it will create a file
> > > that is owned by uid 1000 on-disk, i.e.:
> > >
> > > - looking at it via /unmapped/file2 will report it as owned by uid 1000
> > > - looking at it via /idmapped/file2 will report it as owned by uid 0
> > >
> > > Now consider an mds server that has a uid access restriction set and
> > > only grants access to requests from uid 0.
> > >
> > > If the client sends a creation request for a file e.g. /idmapped/file2
> > > it will send the caller's fs{g,u}id idmapped according to the idmapped
> > > mount. So if the caller has fs{g,u}id 1000 it will be mapped to {g,u}id
> > > 0 in the idmapped mount and will be sent over the wire allowing the
> > > caller access to the mds server.
> > >
> > > However, if the caller is not issuing a creation request the caller's
> > > fs{g,u}id will be send without the mount's idmapping applied. So if the
> > > caller that just successfully created a new file on the restricted mds
> > > server sends a request as fs{g,u}id 1000 access will be refused. This
> > > however is inconsistent.
> > >
> >
> > IDGI, why would you send the fs{g,u}id without the mount's idmapping
> > applied in this case? ISTM that idmapping is wholly a client-side
> > feature, and that you should always map id's regardless of whether
> > you're creating or not.
> 
> Yeah, I'm confused. We want the fs {g,u}id to be consistent throughout
> the request pipeline and to reflect the actual Ceph user all the way
> through the server-side pipeline. What if client.greg is only
> authorized to work as uid 12345 and access /users/greg/; why would you
> send in a bunch of requests as root just because I mounted that way
> inside my own space?
> 
> This might be more obvious in the userspace Client, which is already
> set up to be friendlier to mapped users for Ganesha etc:
> mknod (https://github.com/ceph/ceph/blob/master/src/client/Client.cc#L7297)
> and similar calls receive a "UserPerm" from the caller specifying who
> the call should be performed as, and they pass that in to the generic
> make_requst() function
> (https://github.com/ceph/ceph/blob/master/src/client/Client.cc#L1778)
> which uses it to set the uid and gid fields you found in the message.

Thank you for those links. I think I read through this code before and
I'll give it another read.

Re: [PATCH 02/12] ceph: handle idmapped mounts in create_request_message()

[Jeff Layton]

On Wed, 2022-01-05 at 15:10 +0100, Christian Brauner wrote:
> On Tue, Jan 04, 2022 at 12:40:51PM -0500, Jeff Layton wrote:
> > On Tue, 2022-01-04 at 15:04 +0100, Christian Brauner wrote:
> > > From: Christian Brauner <christian.brauner@ubuntu.com>
> > > 
> > > Inode operations that create a new filesystem object such as ->mknod,
> > > ->create, ->mkdir() and others don't take a {g,u}id argument explicitly.
> > > Instead the caller's fs{g,u}id is used for the {g,u}id of the new
> > > filesystem object.
> > > 
> > > Cephfs mds creation request argument structures mirror this filesystem
> > > behavior. They don't encode a {g,u}id explicitly. Instead the caller's
> > > fs{g,u}id that is always sent as part of any mds request is used by the
> > > servers to set the {g,u}id of the new filesystem object.
> > > 
> > > In order to ensure that the correct {g,u}id is used map the caller's
> > > fs{g,u}id for creation requests. This doesn't require complex changes.
> > > It suffices to pass in the relevant idmapping recorded in the request
> > > message. If this request message was triggered from an inode operation
> > > that creates filesystem objects it will have passed down the relevant
> > > idmaping. If this is a request message that was triggered from an inode
> > > operation that doens't need to take idmappings into account the initial
> > > idmapping is passed down which is an identity mapping and thus is
> > > guaranteed to leave the caller's fs{g,u}id unchanged.,u}id is sent.
> > > 
> > > The last few weeks before Christmas 2021 I have spent time not just
> > > reading and poking the cephfs kernel code but also took a look at the
> > > ceph mds server userspace to ensure I didn't miss some subtlety.
> > > 
> > > This made me aware of one complication to solve. All requests send the
> > > caller's fs{g,u}id over the wire. The caller's fs{g,u}id matters for the
> > > server in exactly two cases:
> > > 
> > > 1. to set the ownership for creation requests
> > > 2. to determine whether this client is allowed access on this server
> > > 
> > > Case 1. we already covered and explained. Case 2. is only relevant for
> > > servers where an explicit uid access restriction has been set. That is
> > > to say the mds server restricts access to requests coming from a
> > > specific uid. Servers without uid restrictions will grant access to
> > > requests from any uid by setting MDS_AUTH_UID_ANY.
> > > 
> > > Case 2. introduces the complication because the caller's fs{g,u}id is
> > > not just used to record ownership but also serves as the {g,u}id used
> > > when checking access to the server.
> > > 
> > > Consider a user mounting a cephfs client and creating an idmapped mount
> > > from it that maps files owned by uid 1000 to be owned uid 0:
> > > 
> > > mount -t cephfs -o [...] /unmapped
> > > mount-idmapped --map-mount 1000:0:1 /idmapped
> > > 
> > > That is to say if the mounted cephfs filesystem contains a file "file1"
> > > which is owned by uid 1000:
> > > 
> > > - looking at it via /unmapped/file1 will report it as owned by uid 1000
> > >   (One can think of this as the on-disk value.)
> > > - looking at it via /idmapped/file1 will report it as owned by uid 0
> > > 
> > > Now, consider creating new files via the idmapped mount at /idmapped.
> > > When a caller with fs{g,u}id 1000 creates a file "file2" by going
> > > through the idmapped mount mounted at /idmapped it will create a file
> > > that is owned by uid 1000 on-disk, i.e.:
> > > 
> > > - looking at it via /unmapped/file2 will report it as owned by uid 1000
> > > - looking at it via /idmapped/file2 will report it as owned by uid 0
> > > 
> > > Now consider an mds server that has a uid access restriction set and
> > > only grants access to requests from uid 0.
> > > 
> > > If the client sends a creation request for a file e.g. /idmapped/file2
> > > it will send the caller's fs{g,u}id idmapped according to the idmapped
> > > mount. So if the caller has fs{g,u}id 1000 it will be mapped to {g,u}id
> > > 0 in the idmapped mount and will be sent over the wire allowing the
> > > caller access to the mds server.
> > > 
> > > However, if the caller is not issuing a creation request the caller's
> > > fs{g,u}id will be send without the mount's idmapping applied. So if the
> > > caller that just successfully created a new file on the restricted mds
> > > server sends a request as fs{g,u}id 1000 access will be refused. This
> > > however is inconsistent.
> > > 
> > 
> > IDGI, why would you send the fs{g,u}id without the mount's idmapping
> > applied in this case? ISTM that idmapping is wholly a client-side
> > feature, and that you should always map id's regardless of whether
> > you're creating or not.
> 
> Since the idmapping is a property of the mount and not a property of the
> caller the caller's fs{g,u}id aren't mapped. What is mapped are the
> inode's i{g,u}id when accessed from a particular mount.
> 
> The fs{g,u}id are only ever mapped when a new filesystem object is
> created. So if I have an idmapped mount that makes it so that files
> owned by 1000 on-disk appear to be owned by uid 0 then a user with uid 0
> creating a new file will create files with uid 1000 on-disk when going
> through that mount. For cephfs that'd be the uid we would be sending
> with creation requests as I've currently written it.
> 
> So then when the user looks at the file it created it will see it as
> being owned by uid 0 from that idmapped mount (whereas on-disk it's
> 1000). But the user's fs{g,u}id isn't per se changed when going through
> that mount. So in my opinion I was thinking that the server with access
> permissions set would want to always check permissions on the users
> "raw" fs{g,u}id. That would mean I'd have to change the patch obviously.
> My suggestion was to send the {g,u}id the file will be created with
> separately. The alternative would be to not just pass the idmapping into
> the creation iop's but into all iops so that we can always map it for
> cephfs. But this would mean a lot of vfs changes for one filesystem. So
> if we could first explore alternatives approaches I'd be grateful.
> 

You'll probably need to do this for NFS anyway, if you have plans in
that direction. Extending the protocol there will be much more
difficult. I think that approach sounds much cleaner overall.

> (I'll be traveling for the latter half of this week starting today at
> CET afternoon so apologies but I'll probably take some time to respond.)

Ok. I guess you can get away with this on a local fs because the backend
storage doesn't really care about uid/gids at all. The only permission
checking is done in the kernel and you (presumably) can just map the
inode's uid/gid prior to checking permissions.

I'm a little confused as to what you mean by "raw" id here. In your
earlier example with a mapping of 1000:0:1, which one is the raw id for
the actual user?
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 02/12] ceph: handle idmapped mounts in create_request_message()

[Christian Brauner]

On Wed, Jan 05, 2022 at 10:03:06AM -0500, Jeff Layton wrote:
> On Wed, 2022-01-05 at 15:10 +0100, Christian Brauner wrote:
> > On Tue, Jan 04, 2022 at 12:40:51PM -0500, Jeff Layton wrote:
> > > On Tue, 2022-01-04 at 15:04 +0100, Christian Brauner wrote:
> > > > From: Christian Brauner <christian.brauner@ubuntu.com>
> > > > 
> > > > Inode operations that create a new filesystem object such as ->mknod,
> > > > ->create, ->mkdir() and others don't take a {g,u}id argument explicitly.
> > > > Instead the caller's fs{g,u}id is used for the {g,u}id of the new
> > > > filesystem object.
> > > > 
> > > > Cephfs mds creation request argument structures mirror this filesystem
> > > > behavior. They don't encode a {g,u}id explicitly. Instead the caller's
> > > > fs{g,u}id that is always sent as part of any mds request is used by the
> > > > servers to set the {g,u}id of the new filesystem object.
> > > > 
> > > > In order to ensure that the correct {g,u}id is used map the caller's
> > > > fs{g,u}id for creation requests. This doesn't require complex changes.
> > > > It suffices to pass in the relevant idmapping recorded in the request
> > > > message. If this request message was triggered from an inode operation
> > > > that creates filesystem objects it will have passed down the relevant
> > > > idmaping. If this is a request message that was triggered from an inode
> > > > operation that doens't need to take idmappings into account the initial
> > > > idmapping is passed down which is an identity mapping and thus is
> > > > guaranteed to leave the caller's fs{g,u}id unchanged.,u}id is sent.
> > > > 
> > > > The last few weeks before Christmas 2021 I have spent time not just
> > > > reading and poking the cephfs kernel code but also took a look at the
> > > > ceph mds server userspace to ensure I didn't miss some subtlety.
> > > > 
> > > > This made me aware of one complication to solve. All requests send the
> > > > caller's fs{g,u}id over the wire. The caller's fs{g,u}id matters for the
> > > > server in exactly two cases:
> > > > 
> > > > 1. to set the ownership for creation requests
> > > > 2. to determine whether this client is allowed access on this server
> > > > 
> > > > Case 1. we already covered and explained. Case 2. is only relevant for
> > > > servers where an explicit uid access restriction has been set. That is
> > > > to say the mds server restricts access to requests coming from a
> > > > specific uid. Servers without uid restrictions will grant access to
> > > > requests from any uid by setting MDS_AUTH_UID_ANY.
> > > > 
> > > > Case 2. introduces the complication because the caller's fs{g,u}id is
> > > > not just used to record ownership but also serves as the {g,u}id used
> > > > when checking access to the server.
> > > > 
> > > > Consider a user mounting a cephfs client and creating an idmapped mount
> > > > from it that maps files owned by uid 1000 to be owned uid 0:
> > > > 
> > > > mount -t cephfs -o [...] /unmapped
> > > > mount-idmapped --map-mount 1000:0:1 /idmapped
> > > > 
> > > > That is to say if the mounted cephfs filesystem contains a file "file1"
> > > > which is owned by uid 1000:
> > > > 
> > > > - looking at it via /unmapped/file1 will report it as owned by uid 1000
> > > >   (One can think of this as the on-disk value.)
> > > > - looking at it via /idmapped/file1 will report it as owned by uid 0
> > > > 
> > > > Now, consider creating new files via the idmapped mount at /idmapped.
> > > > When a caller with fs{g,u}id 1000 creates a file "file2" by going
> > > > through the idmapped mount mounted at /idmapped it will create a file
> > > > that is owned by uid 1000 on-disk, i.e.:
> > > > 
> > > > - looking at it via /unmapped/file2 will report it as owned by uid 1000
> > > > - looking at it via /idmapped/file2 will report it as owned by uid 0
> > > > 
> > > > Now consider an mds server that has a uid access restriction set and
> > > > only grants access to requests from uid 0.
> > > > 
> > > > If the client sends a creation request for a file e.g. /idmapped/file2
> > > > it will send the caller's fs{g,u}id idmapped according to the idmapped
> > > > mount. So if the caller has fs{g,u}id 1000 it will be mapped to {g,u}id
> > > > 0 in the idmapped mount and will be sent over the wire allowing the
> > > > caller access to the mds server.
> > > > 
> > > > However, if the caller is not issuing a creation request the caller's
> > > > fs{g,u}id will be send without the mount's idmapping applied. So if the
> > > > caller that just successfully created a new file on the restricted mds
> > > > server sends a request as fs{g,u}id 1000 access will be refused. This
> > > > however is inconsistent.
> > > > 
> > > 
> > > IDGI, why would you send the fs{g,u}id without the mount's idmapping
> > > applied in this case? ISTM that idmapping is wholly a client-side
> > > feature, and that you should always map id's regardless of whether
> > > you're creating or not.
> > 
> > Since the idmapping is a property of the mount and not a property of the
> > caller the caller's fs{g,u}id aren't mapped. What is mapped are the
> > inode's i{g,u}id when accessed from a particular mount.
> > 
> > The fs{g,u}id are only ever mapped when a new filesystem object is
> > created. So if I have an idmapped mount that makes it so that files
> > owned by 1000 on-disk appear to be owned by uid 0 then a user with uid 0
> > creating a new file will create files with uid 1000 on-disk when going
> > through that mount. For cephfs that'd be the uid we would be sending
> > with creation requests as I've currently written it.
> > 
> > So then when the user looks at the file it created it will see it as
> > being owned by uid 0 from that idmapped mount (whereas on-disk it's
> > 1000). But the user's fs{g,u}id isn't per se changed when going through
> > that mount. So in my opinion I was thinking that the server with access
> > permissions set would want to always check permissions on the users
> > "raw" fs{g,u}id. That would mean I'd have to change the patch obviously.
> > My suggestion was to send the {g,u}id the file will be created with
> > separately. The alternative would be to not just pass the idmapping into
> > the creation iop's but into all iops so that we can always map it for
> > cephfs. But this would mean a lot of vfs changes for one filesystem. So
> > if we could first explore alternatives approaches I'd be grateful.
> > 
> 
> You'll probably need to do this for NFS anyway, if you have plans in
> that direction. Extending the protocol there will be much more
> difficult. I think that approach sounds much cleaner overall.

Ok. Is it ok if I take a little while to work on this?
I have some other work I need to be looking at first and then I have
Februrary "free".

> 
> > (I'll be traveling for the latter half of this week starting today at
> > CET afternoon so apologies but I'll probably take some time to respond.)
> 
> Ok. I guess you can get away with this on a local fs because the backend
> storage doesn't really care about uid/gids at all. The only permission
> checking is done in the kernel and you (presumably) can just map the
> inode's uid/gid prior to checking permissions.

Yes, we always map the inode as that's semantically cleaner and easier
to reason about in my opinion.

> 
> I'm a little confused as to what you mean by "raw" id here. In your
> earlier example with a mapping of 1000:0:1, which one is the raw id for
> the actual user?

Oh, sorry. In this context I really just meant the values gotten from
current_fs{g,u}id() as they are sent now.

Re: [PATCH 02/12] ceph: handle idmapped mounts in create_request_message()

[Aleksandr Mikhalitsyn]

Dear colleagues,

On Wed, 5 Jan 2022 15:10:23 +0100
Christian Brauner <christian.brauner@ubuntu.com> wrote:

> On Tue, Jan 04, 2022 at 12:40:51PM -0500, Jeff Layton wrote:
> > On Tue, 2022-01-04 at 15:04 +0100, Christian Brauner wrote:
> > > From: Christian Brauner <christian.brauner@ubuntu.com>
> > > 
> > > Inode operations that create a new filesystem object such as ->mknod,
> > > ->create, ->mkdir() and others don't take a {g,u}id argument explicitly.
> > > Instead the caller's fs{g,u}id is used for the {g,u}id of the new
> > > filesystem object.
> > > 
> > > Cephfs mds creation request argument structures mirror this filesystem
> > > behavior. They don't encode a {g,u}id explicitly. Instead the caller's
> > > fs{g,u}id that is always sent as part of any mds request is used by the
> > > servers to set the {g,u}id of the new filesystem object.
> > > 
> > > In order to ensure that the correct {g,u}id is used map the caller's
> > > fs{g,u}id for creation requests. This doesn't require complex changes.
> > > It suffices to pass in the relevant idmapping recorded in the request
> > > message. If this request message was triggered from an inode operation
> > > that creates filesystem objects it will have passed down the relevant
> > > idmaping. If this is a request message that was triggered from an inode
> > > operation that doens't need to take idmappings into account the initial
> > > idmapping is passed down which is an identity mapping and thus is
> > > guaranteed to leave the caller's fs{g,u}id unchanged.,u}id is sent.
> > > 
> > > The last few weeks before Christmas 2021 I have spent time not just
> > > reading and poking the cephfs kernel code but also took a look at the
> > > ceph mds server userspace to ensure I didn't miss some subtlety.
> > > 
> > > This made me aware of one complication to solve. All requests send the
> > > caller's fs{g,u}id over the wire. The caller's fs{g,u}id matters for the
> > > server in exactly two cases:
> > > 
> > > 1. to set the ownership for creation requests
> > > 2. to determine whether this client is allowed access on this server
> > > 
> > > Case 1. we already covered and explained. Case 2. is only relevant for
> > > servers where an explicit uid access restriction has been set. That is
> > > to say the mds server restricts access to requests coming from a
> > > specific uid. Servers without uid restrictions will grant access to
> > > requests from any uid by setting MDS_AUTH_UID_ANY.
> > > 
> > > Case 2. introduces the complication because the caller's fs{g,u}id is
> > > not just used to record ownership but also serves as the {g,u}id used
> > > when checking access to the server.
> > > 
> > > Consider a user mounting a cephfs client and creating an idmapped mount
> > > from it that maps files owned by uid 1000 to be owned uid 0:
> > > 
> > > mount -t cephfs -o [...] /unmapped
> > > mount-idmapped --map-mount 1000:0:1 /idmapped
> > > 
> > > That is to say if the mounted cephfs filesystem contains a file "file1"
> > > which is owned by uid 1000:
> > > 
> > > - looking at it via /unmapped/file1 will report it as owned by uid 1000
> > >   (One can think of this as the on-disk value.)
> > > - looking at it via /idmapped/file1 will report it as owned by uid 0
> > > 
> > > Now, consider creating new files via the idmapped mount at /idmapped.
> > > When a caller with fs{g,u}id 1000 creates a file "file2" by going
> > > through the idmapped mount mounted at /idmapped it will create a file
> > > that is owned by uid 1000 on-disk, i.e.:
> > > 
> > > - looking at it via /unmapped/file2 will report it as owned by uid 1000
> > > - looking at it via /idmapped/file2 will report it as owned by uid 0
> > > 
> > > Now consider an mds server that has a uid access restriction set and
> > > only grants access to requests from uid 0.
> > > 
> > > If the client sends a creation request for a file e.g. /idmapped/file2
> > > it will send the caller's fs{g,u}id idmapped according to the idmapped
> > > mount. So if the caller has fs{g,u}id 1000 it will be mapped to {g,u}id
> > > 0 in the idmapped mount and will be sent over the wire allowing the
> > > caller access to the mds server.
> > > 
> > > However, if the caller is not issuing a creation request the caller's
> > > fs{g,u}id will be send without the mount's idmapping applied. So if the
> > > caller that just successfully created a new file on the restricted mds
> > > server sends a request as fs{g,u}id 1000 access will be refused. This
> > > however is inconsistent.
> > > 
> > 
> > IDGI, why would you send the fs{g,u}id without the mount's idmapping
> > applied in this case? ISTM that idmapping is wholly a client-side
> > feature, and that you should always map id's regardless of whether
> > you're creating or not.
> 
> Since the idmapping is a property of the mount and not a property of the
> caller the caller's fs{g,u}id aren't mapped. What is mapped are the
> inode's i{g,u}id when accessed from a particular mount.
> 
> The fs{g,u}id are only ever mapped when a new filesystem object is
> created. So if I have an idmapped mount that makes it so that files
> owned by 1000 on-disk appear to be owned by uid 0 then a user with uid 0
> creating a new file will create files with uid 1000 on-disk when going
> through that mount. For cephfs that'd be the uid we would be sending
> with creation requests as I've currently written it.
> 
> So then when the user looks at the file it created it will see it as
> being owned by uid 0 from that idmapped mount (whereas on-disk it's
> 1000). But the user's fs{g,u}id isn't per se changed when going through
> that mount. So in my opinion I was thinking that the server with access
> permissions set would want to always check permissions on the users
> "raw" fs{g,u}id. That would mean I'd have to change the patch obviously.
> My suggestion was to send the {g,u}id the file will be created with
> separately. The alternative would be to not just pass the idmapping into
> the creation iop's but into all iops so that we can always map it for
> cephfs. But this would mean a lot of vfs changes for one filesystem. So
> if we could first explore alternatives approaches I'd be grateful.

I can't understand which kind of operations we are talking about here.
Right now almost all inode_operations are taking struct mnt_idmap as a parameter
(at the moment of this series was posted it was struct user_namespace, but that's not important).

The only iops those are not taking idmap is lookup, readlink, fiemap, update_time, atomic_open
and a few more. So, we want to pass struct mnt_idmap to them to always map current_fs{g,u}id
according to a mount idmapping? As Christian pointed above:

> Since the idmapping is a property of the mount and not a property of the
> caller the caller's fs{g,u}id aren't mapped. What is mapped are the
> inode's i{g,u}id when accessed from a particular mount.

If we want to go this way then we don't need to pass mnt_idmap to any additional inode ops
and the current approach works fine. Please, correct me if I'm wrong.

> 
> (I'll be traveling for the latter half of this week starting today at
> CET afternoon so apologies but I'll probably take some time to respond.)
>

P.S.

I'm trying to make a respin for this series, I've made a formal rebase on top of
the current Linux kernel tree and fixed it according to the Jeff's review comment:
https://lore.kernel.org/all/041afbfd171915d62ab9a93c7a35d9c9d5c5bf7b.camel@kernel.org/

This thing is really important for LXD/LXC project so I'll be happy to help with pushing
this forward.

Current tree:
https://github.com/mihalicyn/linux/commits/fs.idmapped.ceph.v2

Kind regards,
Alex