RE: nfs setgid inheritance test

Hi, Christian,

Thank you so much for your explanation.

quoted

I tested on kernel 5.14.0-162.6.1.el9_1.x86_64, and it failed with

"no_root_squash" set.

quoted

But after I apply commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b

onto

quoted

kernel 5.14.0-162.6.1.el9_1.x86_64, the case will pass.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com
mit/?h=1639a49ccdce58ea248841ed9b23babcce6dbb0b

Ah, good. That's crucial information as no_root_squash did work before and it
would've been a regression if it suddenly would leave the setgid bit set.

If I don't apply this patch, the setgid bit will not be stripped.
So is this behavior an NFS bug? 
Does NFS need to strip SGID bit when the "no_root_squash" set or "root_squash" set?

Thanks,

★☆★☆★☆★☆FNSTオンラインへようこそ★☆★☆★☆★☆
   FNST最新情報盛りたくさん！
   http://online.fnst.cn.fujitsu.com/fnst-news
★☆★☆★☆★☆★☆★☆★☆★☆★☆★☆★☆★☆★☆

-----Original Message-----
From: Christian Brauner <brauner@kernel.org>
Sent: Tuesday, January 3, 2023 6:35 PM
To: Cui, Yue/崔 悦 <redacted>
Cc: Christian Brauner <christian@brauner.io>; linux-nfs@vger.kernel.org
Subject: Re: nfs setgid inheritance test

On Tue, Jan 03, 2023 at 02:58:29AM +0000, cuiyue-fnst@fujitsu.com wrote:

quoted

Hello Christian,

quoted

quoted

Thank you for your response.

quoted

Afaict, nothing has changed and the test should still be skipped.
I'm not sure I ever send a patch to skip this test specifically
for nfs though. I might just not have gotten around to that.

Can you please also send the exact steps for reproducing this issue?

The reproducing steps is as follows:

Client & Server:
1. Install xfstests
2. # yum install libcap-devel

Server:
1. Set exports file.
# echo "/nfstest

*(rw,insecure,no_subtree_check,no_root_squash,fsid=1)

quoted

/nfsscratch

*(rw,insecure,no_subtree_check,no_root_squash,fsid=2)" >/etc/exports

quoted

2. Restart services.
# systemctl restart rpcbind.service # systemctl restart
nfs-server.service # systemctl restart rpc-statd.service

Client:
1. Create mount point
# mkdir -p /mnt/test
# mkdir -p /mnt/scratch
2. Cofigure NFS parameters.
# echo "FSTYP=nfs
TEST_DEV=server_IP:/nfstest
TEST_DIR=/mnt/test
SCRATCH_DEV=server_IP:/nfsscratch
SCRATCH_MNT=/mnt/scratch
export KEEP_DMESG=yes
NFS_MOUNT_OPTIONS=\"-o vers=3\"">/var/lib/xfstests/local.config
3. Test
# ./check -d generic/633

The tests should pass with "no_root_squash" set. The root cause of
the original issue was that files created by root are squashed to
65534 which breaks setgid inheritance rules for S_ISGID directories.

But without root squashing the tests should succeed. If I reproduce
this exactly with your instructions on a v6.2-rc1 kernel I get a success as

expected.

quoted

quoted

I don't think you've told me What kernel you are testing this on?

Sorry, I didn't make it clearly before.

No worries.

quoted

I tested on kernel 5.14.0-162.6.1.el9_1.x86_64, and it failed with

"no_root_squash" set.

quoted

But after I apply commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b

onto

quoted

kernel 5.14.0-162.6.1.el9_1.x86_64, the case will pass.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com
mit/?h=1639a49ccdce58ea248841ed9b23babcce6dbb0b

Ah, good. That's crucial information as no_root_squash did work before and it
would've been a regression if it suddenly would leave the setgid bit set.

quoted

This patch moves S_ISGID stripping into the vfs, so NFS can solve the setgid

inheritance problem.

quoted

But although the test can succeed, when the root is squashed to nobody, is it

still suitable to use generic/633 to test?

No, when root squashing is enabled the test shouldn't run. I've mentioned this in
my earlier mail.

Just one example, when you create a new file in a setgid directory then the new
file will inherit the gid of the directory it has been created in. But with root
squashing that's no longer the case for the root user since root squashing
changes the {g,u}id that a file is created as. It essentially idmaps {g,u}id 0 to
655345. That means reasoning about setgid inheritance rules as the root user
doesn't work in the tests anymore. If that is a desirable thing then xfstests
should gain a new nfs specific test for this case.