Support idmapped mount

[youzhongyang]

Signed-off-by: Youzhong Yang [email protected]

This pull request adds support for idmapped mount. Although I have tried other approach, changing function signatures to pass 'struct user_namespace' pointer around seems to be the way to go.

Motivation and Context

Idmapped mount is a very useful functionality for some use cases. For example, a zfs dataset is owned by some role account, if a developer clones the dataset to create a sandbox and works on it, the ownership of the files/directories has to be changed to the developer, these changes can be very time-consuming, use memory and disk space too in case of a huge directory tree.

Description

How Has This Been Tested?

Manual testing is done. Automation requires the use of source code such as the one on https://github.com/brauner/mount-idmapped, which may lead to some sort of license issue.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[ghost]

It's great to see this missing functionality implemented!

Rather than using void * to pass the namespace around, it would be safer to make some zuserns_t or such typedef that can be struct user_namespace on Linux and void on FreeBSD.

[ghost]

Fixes #12923

[behlendorf]

This is great to see implemented! I think the approach you went with is pretty reasonable, even though it does mean modifying a lot of interfaces. One possible alternative would have been to attach the mount namespace to the credential, but I suspect in the end that would have been just as disruptive and probably more confusing.

On the testing front it looks like xfstests has incorporated idmapped mount test coverage for filesystems which support it. If we can manually verify those new tests pass as expected then this looks good to me.

[lukts30]

The interaction of the feature with #12263 should be checked carefully.

I think ZFS would end and up being the first filesystem that supports both FS_USERNS_MOUNT and FS_ALLOW_IDMAP. Upsteam Linux file system currently only support either one of these features.

There were no restrictions when the IDMAP feature was initially added to the kernel 5.12 but they were added post merge in 5.13 and are now present in
[5.12;5.16] [1].

	/* Don't yet support filesystem mountable in user namespaces. */
		if (m->mnt_sb->s_user_ns != &init_user_ns)
	 return -EINVAL;
	 
		/* We're not controlling the superblock. */
		if (!capable(CAP_SYS_ADMIN))
	 return -EPERM;

Theses added restrictions where then again removed in 5.17 [2].

	// struct user_namespace *fs_userns = m->mnt_sb->s_user_ns;
	/* We're not controlling the superblock. */
		if (!ns_capable(fs_userns, CAP_SYS_ADMIN))
	 return -EPERM;

[youzhongyang]

All the following tests passed:

generic/633
generic/644
generic/645
generic/656
generic/689

root@ubuntu-dev:~/xfstests-dev# src/vfs/vfstest --test-core --device /dev/loop10 --mount /vmgr/idmapped --fstype zfs ; echo $?
0
root@ubuntu-dev:~/xfstests-dev# src/vfs/vfstest --test-fscaps-regression --device /dev/loop10 --mount /vmgr/idmapped --fstype zfs ; echo $?
0
root@ubuntu-dev:~/xfstests-dev# src/vfs/vfstest --test-nested-userns --device /dev/loop10 --mount /vmgr/idmapped --fstype zfs ; echo $?
0
root@ubuntu-dev:~/xfstests-dev# src/vfs/vfstest --test-setattr-fix-968219708108 --device /dev/loop10 --mount /vmgr/idmapped --fstype zfs ; echo $?
0
root@ubuntu-dev:~/xfstests-dev# src/vfs/vfstest --test-setxattr-fix-705191b03d50 --device /dev/loop10 --mount /vmgr/idmapped --fstype zfs ; echo $?
0

[youzhongyang]

The interaction of the feature with #12263 should be checked carefully.

I think ZFS would end and up being the first filesystem that supports both FS_USERNS_MOUNT and FS_ALLOW_IDMAP. Upsteam Linux file system currently only support either one of these features.

There were no restrictions when the IDMAP feature was initially added to the kernel 5.12 but they were added post merge in 5.13 and are now present in [5.12;5.16] [1].

	/* Don't yet support filesystem mountable in user namespaces. */
		if (m->mnt_sb->s_user_ns != &init_user_ns)
	 return -EINVAL;
	 
		/* We're not controlling the superblock. */
		if (!capable(CAP_SYS_ADMIN))
	 return -EPERM;

Theses added restrictions where then again removed in 5.17 [2].

	// struct user_namespace *fs_userns = m->mnt_sb->s_user_ns;
	/* We're not controlling the superblock. */
		if (!ns_capable(fs_userns, CAP_SYS_ADMIN))
	 return -EPERM;

@lukts30 - Thanks for the feedback. You are right, I verified that idmapped mount is not allowed in user namespace on Ubuntu 22.04 (which runs kernel 5.15.0), but there is nothing we can do about it, isn't it? In last few days I also got a chance to review the PR #12263, FS_USERNS_MOUNT and FS_ALLOW_IDMAP are totally two different features, FS_ALLOW_IDMAP is for idmapping on the dataset/mount point, while FS_USERNS_MOUNT is for the visibility and accessibility of the dataset/mount point, yes they are both related to the user namespace, but in a completely different way.

Does this address your concern?

[youzhongyang]

I've rebased this PR and CI results look good. It's ready for review. Please let me know if anything else needs to be done.

I understand the approach of modifying a lot of interfaces looks ugly, but so far it is the only reliable one. My attempt of messing with cred_t has been unsuccessful (it looks elegant, passes all manual testing but fails the zfs-tests suites.)

[lukts30]

Thanks for the feedback. You are right, I verified that idmapped mount is not allowed in user namespace on Ubuntu 22.04 (which runs kernel 5.15.0), but there is nothing we can do about it, isn't it?

My concerns are more applicable when the precautious kernel checks are missing (kernel 5.17 or newer).

In last few days I also got a chance to review the PR #12263, FS_USERNS_MOUNT and FS_ALLOW_IDMAP are totally two different features, FS_ALLOW_IDMAP is for idmapping on the dataset/mount point, while FS_USERNS_MOUNT is for the visibility and accessibility of the dataset/mount point, yes they are both related to the user namespace, but in a completely different way.

I am aware that FS_USERNS_MOUNT and FS_ALLOW_IDMAP serve different purposes but they can be used at the same time with kernel 5.17 or newer.

Therefore I want to be sure that a idmapped mount inside of userns with a delegated dataset works as expected.
It should only be possible to idmap already accessible uid/gid ranges with a idmapped mount. No unexpected uid/gid range elevation should be possible.

That might already be guaranteed by the kernel idmapping functions so my concerns might be unwarranted.

[youzhongyang]

@lukts30 - I've run the idmapped tests available in xfs-tests suite, on kernel 5.17 and 5.19 in both global zone and zone, here are the results:

Kernel	vfstest --test-core	vfstest --test-fscaps-regression	vfstest --test-nested-userns	vfstest --test-setattr-fix-968219708108	vfstest --test-setxattr-fix-705191b03d50	Notes
5.15.0-46-lowlatency	PASS	PASS	PASS	PASS	PASS	Not Applicable to zone (non-default user namespace)
5.17.0-1003-oem	PASS in global zone; FAIL in zone	PASS	PASS	PASS	FAIL
5.19.0-17-generic	PASS in global zone; FAIL in zone	PASS	PASS	PASS	PASS

Here 'global zone' refers to default user namespace; 'zone' refers to using non-default user namespace.

There seem to be changes in kernel > 5.15, I will figure out.

[youzhongyang]

I've investigated the vfstest --test-core failure when running in a 'zone' (non-default user namespace) on kernel 5.19.0.

Bottom Line on Top: it is expected and nothing we can do about it.

The script I used to run xfstests cases is as follows:

# cat run-idmap-tests
#!/bin/bash

ZPOOL=rpool
# make sure xfstests-dev has already been built
VFSTEST=/root/xfstests-dev/src/vfs/vfstest
# make sure /dev/loop0 is not in-use, run 'losetup' to find out;
# specifying --device is just to make vfstest program happy
DEVICE=/dev/loop0

echo "Run tests in the global 'zone'"
zfs create -o zoned=off -o mountpoint=/userns -o acltype=posix ${ZPOOL}/userns

${VFSTEST} --test-core --device ${DEVICE} --mount /userns --fstype zfs
${VFSTEST} --test-fscaps-regression --device ${DEVICE} --mount /userns --fstype zfs
${VFSTEST} --test-nested-userns --device ${DEVICE} --mount /userns --fstype zfs
${VFSTEST} --test-setattr-fix-968219708108 --device ${DEVICE} --mount /userns --fstype zfs
${VFSTEST} --test-setxattr-fix-705191b03d50 --device ${DEVICE} --mount /userns --fstype zfs

zfs destroy ${ZPOOL}/userns

echo "Run tests in a 'zone'"
zfs create -o zoned=on -o mountpoint=/userns -o acltype=posix ${ZPOOL}/userns
unshare -Um --keep-caps sleep 2h &
SLEEPPID=$!
sleep 1
echo '0 0 4294967295' > /proc/${SLEEPPID}/uid_map
echo '0 0 4294967295' > /proc/${SLEEPPID}/gid_map
zfs zone /proc/${SLEEPPID}/ns/user ${ZPOOL}/userns
nsenter -t ${SLEEPPID} --all zfs mount ${ZPOOL}/userns

nsenter -t ${SLEEPPID} --all ${VFSTEST} --test-core --device ${DEVICE} --mount /userns --fstype zfs
nsenter -t ${SLEEPPID} --all ${VFSTEST} --test-fscaps-regression --device ${DEVICE} --mount /userns --fstype zfs
nsenter -t ${SLEEPPID} --all ${VFSTEST} --test-nested-userns --device ${DEVICE} --mount /userns --fstype zfs
nsenter -t ${SLEEPPID} --all ${VFSTEST} --test-setattr-fix-968219708108 --device ${DEVICE} --mount /userns --fstype zfs
nsenter -t ${SLEEPPID} --all ${VFSTEST} --test-setxattr-fix-705191b03d50 --device ${DEVICE} --mount /userns --fstype zfs

nsenter -t ${SLEEPPID} --all zfs unmount ${ZPOOL}/userns
zfs unzone /proc/${SLEEPPID}/ns/user ${ZPOOL}/userns
kill -TERM ${SLEEPPID}
zfs destroy ${ZPOOL}/userns

The output of running it:

root@ubuntu-dev:~# ./run-idmap-tests
Run tests in the global 'zone'
Run tests in a 'zone'
vfstest.c: 1569: setgid_create - Operation not permitted - failure: mknodat
vfstest.c: 1882: run_test - Success - failure: create operations in directories with setgid bit set

The vfstest --test-core fails here in xfstests-dev/src/vfs/vfstest.c:

/* create a character device via mknodat() vfs_mknod */
if (mknodat(info->t_dir1_fd, CHRDEV1, S_IFCHR | S_ISGID | S_IXGRP, makedev(5, 1)))
        die("failure: mknodat");

The call sequence of mknoddat() is as follows:

mknodat() -> do_mknodat() -> vfs_mknod() -> capable(CAP_MKNOD)

capable(CAP_MKNOD) returns false!

Why?

Again its call sequence is as follows:

capable(int cap) -> ns_capable(&init_user_ns, cap) ->
ns_capable_common(ns, cap, CAP_OPT_NONE) -> 
security_capable(current_cred(), ns, cap, opts) -> 
cap_capable(cred, ns, cap, opts)

I ran the following bpftrace script to find out why:

# cat capable.bt
#include <linux/user_namespace.h>
#include <linux/cred.h>

kretprobe:zpl_mknod,kretprobe:vfs_mknod,kretprobe:security_inode_mknod
/comm == "vfstest"/
{
   printf("%s %d %s ret %d\n", comm, pid, probe, retval);
}

kprobe:capable /comm == "vfstest"/
{
   printf("%s %d %s cap %d\n", comm, pid, probe, arg0);
   @[tid] = 1;
}

kretprobe:capable /comm == "vfstest"/
{
   printf("%s %d %s ret %d\n", comm, pid, probe, retval);
   @[tid] = 0;
   delete(@[tid]);
}

kprobe:cap_capable /comm == "vfstest" && @[tid]/
{
   $cred = (struct cred *) arg0;
   $ns = (struct user_namespace *) arg1;
   printf("%s %d %s cap %d\n", comm, pid, probe, arg2);
   printf("targ_ns %p parent %p owner %d level %d\n", $ns, $ns->parent,
      $ns->owner.val, $ns->level);
   printf("cred euid %d user_ns %p parent %p level %d cap_effective %x %x\n",
      $cred->euid.val, $cred->user_ns, $cred->user_ns->parent,
      $cred->user_ns->level,
      $cred->cap_effective.cap[0], $cred->cap_effective.cap[1]);
}

kretprobe:cap_capable /comm == "vfstest" && @[tid]/
{
   printf("%s %d %s ret %d\n", comm, pid, probe, retval);
}

root@ubuntu-dev:~# nsenter -t ${SLEEPPID} --all ${VFSTEST} --test-core --device ${DEVICE} --mount /userns --fstype zfs
vfstest.c: 1569: setgid_create - Operation not permitted - failure: mknodat
vfstest.c: 1882: run_test - Success - failure: create operations in directories with setgid bit set

root@ubuntu-dev:~# bpftrace capable.bt
Attaching 7 probes...
vfstest 70281 kprobe:capable cap 14
vfstest 70281 kprobe:cap_capable cap 14
targ_ns 0xffffffff9d0812a0 parent (nil) owner 0 level 0
cred euid 0 user_ns 0xffff89e7d21b04e0 parent 0xffffffff9d0812a0 level 1 cap_effective ffffffff 1ff
vfstest 70281 kretprobe:cap_capable ret -1
vfstest 70281 kretprobe:capable ret 0
vfstest 70288 kprobe:capable cap 14
vfstest 70288 kprobe:cap_capable cap 14
targ_ns 0xffffffff9d0812a0 parent (nil) owner 0 level 0
cred euid 0 user_ns 0xffff89e7d21b04e0 parent 0xffffffff9d0812a0 level 1 cap_effective ffffffff 1ff
vfstest 70288 kretprobe:cap_capable ret -1
vfstest 70288 kretprobe:capable ret 0
vfstest 70300 kprobe:capable cap 4
vfstest 70300 kprobe:cap_capable cap 4
targ_ns 0xffffffff9d0812a0 parent (nil) owner 0 level 0
cred euid 0 user_ns 0xffff89e7d21b04e0 parent 0xffffffff9d0812a0 level 1 cap_effective ffffffff 1ff
vfstest 70300 kretprobe:cap_capable ret -1
vfstest 70300 kretprobe:capable ret 0
vfstest 70302 kprobe:capable cap 27
vfstest 70302 kprobe:cap_capable cap 27
targ_ns 0xffffffff9d0812a0 parent (nil) owner 0 level 0
cred euid 0 user_ns 0xffff89e7d21b04e0 parent 0xffffffff9d0812a0 level 1 cap_effective ffffffff 1ff
vfstest 70302 kretprobe:cap_capable ret -1
vfstest 70302 kretprobe:capable ret 0
vfstest 70302 kretprobe:vfs_mknod ret -1

By the way, CAP_MKNOD = 27.

Observing the code of cap_capable() and the above bpftrace output, it's not difficult to understand why cap_capable() returns -EPERM:

• targ_ns (which is &init_user_ns) is the parent of cred->user_ns!

[lukts30]

Creating device nodes is still a privileged operation and a known limitation of containers not running in the initial user namespace.

Looking at xfstest source code it looks like it only uses mknodat to create a /dev/console (char 5:1) like file. Additionally, normal file creation is done with mknodat which should be an unprivileged operation.

Therefore it might be possible to run unmodified xfstest inside a lxd container with security.syscalls.intercept.mknod true set.
https://linuxcontainers.org/lxd/docs/master/syscall-interception/

[youzhongyang]

Creating device nodes is still a privileged operation and a known limitation of containers not running in the initial user namespace.

Looking at xfstest source code it looks like it only uses mknodat to create a /dev/console (char 5:1) like file. Additionally, normal file creation is done with mknodat which should be an unprivileged operation.

Therefore it might be possible to run unmodified xfstest inside a lxd container with security.syscalls.intercept.mknod true set. https://linuxcontainers.org/lxd/docs/master/syscall-interception/

@lukts30 - It does not work. Did I do anything wrong?

zfs create -o zoned=on -o mountpoint=/userns -o acltype=posix rpool/userns
lxc init images:ubuntu/jammy mylxc -c security.syscalls.intercept.mknod=true
lxc config device add mylxc zfs unix-char path=/dev/zfs source=/dev/zfs
lxc config set mylxc raw.idmap 'both 0-999998 0-999998'
lxc start mylxc
CONTAINERS_PID=$(ps -efww | grep 'containers mylxc' | grep -v grep | awk '{print $2}')
LXCINIT_PID=$(ps -efww | awk -v ppid=$CONTAINERS_PID '$3 == ppid' | awk '{print $2}')
zfs zone /proc/$LXCINIT_PID/ns/user rpool/userns
lxc file push -r -p /root/zfs-idmap mylxc/root
lxc exec mylxc -- bash
cd /root/zfs-idmap/
dpkg -i libnvpair3_*.deb libuutil3_*.deb libzfs5_*.deb libzpool5_*.deb zfs_*.deb
zfs mount rpool/userns

#
# build xfstests-dev in container
# ...
#

ZPOOL=rpool
VFSTEST=/root/xfstests-dev/src/vfs/vfstest
DEVICE=/dev/loop0

${VFSTEST} --test-core --device ${DEVICE} --mount /userns --fstype zfs

	vfstest.c: 1569: setgid_create - Operation not permitted - failure: mknodat
	vfstest.c: 1882: run_test - Success - failure: create operations in directories with setgid bit set

[youzhongyang]

Commenting out mknodat() for character device in setgid_create(), now it fails at linkat():

                /* create tmpfile via filesystem tmpfile api */
                if (supported) {
                        tmpfile_fd = openat(info->t_dir1_fd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID);
                        if (tmpfile_fd < 0)
                                die("failure: create");
                        /* link the temporary file into the filesystem, making it permanent */
                        if (linkat(tmpfile_fd, "", info->t_dir1_fd, FILE3, AT_EMPTY_PATH))
                                die("failure: linkat");

which is due to capable(CAP_DAC_READ_SEARCH) returning false, very much similar to the above capable(CAP_MKNOD) case.

	 * To use null names we require CAP_DAC_READ_SEARCH
	 * This ensures that not everyone will be able to create
	 * handlink using the passed filedescriptor.
	 */
	if (flags & AT_EMPTY_PATH && !capable(CAP_DAC_READ_SEARCH)) {
		error = -ENOENT;
		goto out_putnames;
	}

Conclusion: I don't think xfstests idmapped test cases are designed to run in a non-default user namespace.

[youzhongyang]

Updates:

• Created idmap_util.c which can check if idmapped mount is supported or not, remount a folder with the given idmap information
• A few test cases created
• Rebased against master 5 days ago
• Ran xfstests cases, all good

--test-core
--test-fscaps-regression
--test-nested-userns
--test-setattr-fix-968219708108
--test-setxattr-fix-705191b03d50
--test-setgid-create-umask
--test-setgid-create-acl

It's ready for review. Please take a look. Without any feedback, I have no idea what to do next.

[behlendorf]

This looks to be is good shape and it's great to see this implemented. Thanks for sticking with it, and adding the new test cases, and doing the manual testing. This is ready to merge.

[behlendorf]

Merged. While we aren't aware of any overlooked corner cases, and everything passes xfstests, any additional manual testing would be welcome. This should be a bit easier now that it is merged. Thanks!