Re: [PATCH v6 23/40] exec: handle idmapped mounts

5 messages, 3 authors, 2021-01-27 · open the first message on its own page

Re: [PATCH v6 23/40] exec: handle idmapped mounts

From: Eric W. Biederman <hidden>
Date: 2021-01-25 16:44:20 

Christian Brauner [off-list ref] writes:

When executing a setuid binary the kernel will verify in bprm_fill_uid()
that the inode has a mapping in the caller's user namespace before
setting the callers uid and gid. Let bprm_fill_uid() handle idmapped
mounts. If the inode is accessed through an idmapped mount it is mapped
according to the mount's user namespace. Afterwards the checks are
identical to non-idmapped mounts. If the initial user namespace is
passed nothing changes so non-idmapped mounts will see identical
behavior as before.
This does not handle the v3 capabilites xattr with embeds a uid.
So at least at that level you are missing some critical conversions.

Eric
quoted hunk
Link: https://lore.kernel.org/r/20210112220124.837960-32-christian.brauner@ubuntu.com
Cc: Christoph Hellwig <hch@lst.de>
Cc: David Howells <dhowells@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <redacted>
---
/* v2 */
unchanged

/* v3 */
unchanged

/* v4 */
- Serge Hallyn [off-list ref]:
  - Use "mnt_userns" to refer to a vfsmount's userns everywhere to make
    terminology consistent.

/* v5 */
unchanged
base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837

/* v6 */
base-commit: 19c329f6808995b142b3966301f217c831e7cf31

- Christoph Hellwig [off-list ref]:
  - Use new file_mnt_user_ns() helper.
---
 fs/exec.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index d803227805f6..48d1e8b1610b 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1580,6 +1580,7 @@ static void check_unsafe_exec(struct linux_binprm *bprm)
 static void bprm_fill_uid(struct linux_binprm *bprm, struct file *file)
 {
 	/* Handle suid and sgid on files */
+	struct user_namespace *mnt_userns;
 	struct inode *inode;
 	unsigned int mode;
 	kuid_t uid;
@@ -1596,13 +1597,15 @@ static void bprm_fill_uid(struct linux_binprm *bprm, struct file *file)
 	if (!(mode & (S_ISUID|S_ISGID)))
 		return;
 
+	mnt_userns = file_mnt_user_ns(file);
+
 	/* Be careful if suid/sgid is set */
 	inode_lock(inode);
 
 	/* reload atomically mode/uid/gid now that lock held */
 	mode = inode->i_mode;
-	uid = inode->i_uid;
-	gid = inode->i_gid;
+	uid = i_uid_into_mnt(mnt_userns, inode);
+	gid = i_gid_into_mnt(mnt_userns, inode);
 	inode_unlock(inode);
 
 	/* We ignore suid/sgid if there are no mappings for them in the ns */

Re: [PATCH v6 23/40] exec: handle idmapped mounts

From: "Serge E. Hallyn" <serge@hallyn.com>
Date: 2021-01-26 02:34:53 

On Mon, Jan 25, 2021 at 05:44:04PM +0100, Christian Brauner wrote:
On Mon, Jan 25, 2021 at 10:39:01AM -0600, Eric W. Biederman wrote:
quoted
Christian Brauner [off-list ref] writes:
quoted
When executing a setuid binary the kernel will verify in bprm_fill_uid()
that the inode has a mapping in the caller's user namespace before
setting the callers uid and gid. Let bprm_fill_uid() handle idmapped
mounts. If the inode is accessed through an idmapped mount it is mapped
according to the mount's user namespace. Afterwards the checks are
identical to non-idmapped mounts. If the initial user namespace is
passed nothing changes so non-idmapped mounts will see identical
behavior as before.
This does not handle the v3 capabilites xattr with embeds a uid.
So at least at that level you are missing some critical conversions.
Thanks for looking. Vfs v3 caps are handled earlier in the series. I'm
not sure what you're referring to here. There are tests in xfstests that
verify vfs3 capability behavior.
*just* to make sure i'm not misunderstanding - s/vfs3/v3/ right?

Re: [PATCH v6 23/40] exec: handle idmapped mounts

From: Christian Brauner <hidden>
Date: 2021-01-26 20:10:37 

On Mon, Jan 25, 2021 at 11:03:16AM -0600, Serge Hallyn wrote:
On Mon, Jan 25, 2021 at 05:44:04PM +0100, Christian Brauner wrote:
quoted
On Mon, Jan 25, 2021 at 10:39:01AM -0600, Eric W. Biederman wrote:
quoted
Christian Brauner [off-list ref] writes:
quoted
When executing a setuid binary the kernel will verify in bprm_fill_uid()
that the inode has a mapping in the caller's user namespace before
setting the callers uid and gid. Let bprm_fill_uid() handle idmapped
mounts. If the inode is accessed through an idmapped mount it is mapped
according to the mount's user namespace. Afterwards the checks are
identical to non-idmapped mounts. If the initial user namespace is
passed nothing changes so non-idmapped mounts will see identical
behavior as before.
This does not handle the v3 capabilites xattr with embeds a uid.
So at least at that level you are missing some critical conversions.
Thanks for looking. Vfs v3 caps are handled earlier in the series. I'm
not sure what you're referring to here. There are tests in xfstests that
verify vfs3 capability behavior.
*just* to make sure i'm not misunderstanding - s/vfs3/v3/ right?
Yes, in my mind it's always as "vfs v3 caps -> vfs3 caps". Sorry for the
confusion.

Re: [PATCH v6 23/40] exec: handle idmapped mounts

From: Christian Brauner <hidden>
Date: 2021-01-26 20:10:43 

On Mon, Jan 25, 2021 at 10:39:01AM -0600, Eric W. Biederman wrote:
Christian Brauner [off-list ref] writes:
quoted
When executing a setuid binary the kernel will verify in bprm_fill_uid()
that the inode has a mapping in the caller's user namespace before
setting the callers uid and gid. Let bprm_fill_uid() handle idmapped
mounts. If the inode is accessed through an idmapped mount it is mapped
according to the mount's user namespace. Afterwards the checks are
identical to non-idmapped mounts. If the initial user namespace is
passed nothing changes so non-idmapped mounts will see identical
behavior as before.
This does not handle the v3 capabilites xattr with embeds a uid.
So at least at that level you are missing some critical conversions.
Thanks for looking. Vfs v3 caps are handled earlier in the series. I'm
not sure what you're referring to here. There are tests in xfstests that
verify vfs3 capability behavior.

Christian

Re: [PATCH v6 23/40] exec: handle idmapped mounts

From: "Serge E. Hallyn" <serge@hallyn.com>
Date: 2021-01-27 06:15:13 

On Mon, Jan 25, 2021 at 05:44:04PM +0100, Christian Brauner wrote:
On Mon, Jan 25, 2021 at 10:39:01AM -0600, Eric W. Biederman wrote:
quoted
Christian Brauner [off-list ref] writes:
quoted
When executing a setuid binary the kernel will verify in bprm_fill_uid()
that the inode has a mapping in the caller's user namespace before
setting the callers uid and gid. Let bprm_fill_uid() handle idmapped
mounts. If the inode is accessed through an idmapped mount it is mapped
according to the mount's user namespace. Afterwards the checks are
identical to non-idmapped mounts. If the initial user namespace is
passed nothing changes so non-idmapped mounts will see identical
behavior as before.
This does not handle the v3 capabilites xattr with embeds a uid.
So at least at that level you are missing some critical conversions.
Thanks for looking. Vfs v3 caps are handled earlier in the series. I'm
not sure what you're referring to here. There are tests in xfstests that
verify vfs3 capability behavior.

Christian
So fwiw I just tested it manually as well.  Scenario:

uid 1000 user ubuntu
uid 1001 user u2
sudo ./mount-idmapped --map-mount b:1000:1001:1 /home/ubuntu/ /mnt
su - u2
  cp /usr/bin/id /mnt/
  unshare -Urm
    setcap cap_setuid=pe /mnt/id
At this point, from init_user_ns,
ubuntu@fscaps:~/mount-idmapped$ /home/u2/rcap /mnt/id
v3, rootid is 1001
ubuntu@fscaps:~/mount-idmapped$ /home/u2/rcap //home/ubuntu/id
v3, rootid is 1000

-serge
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help