Bug 2174170

Upstream patch: https://github.com/virtio-win/kvm-guest-drivers-windows/pull/885

How to test:

1. Add "-o UID:GID" command line argument or set registry subkey "Owner" to "UID:GID" with desired host ID, or 0:0 for virtiofsd user ID.
2. Run VirtIO-FS.
3. If virtiofsd is privileged, new files and folders are created as specified ID. In unprivileged case, ID works if included in virtiofsd mapping and 0:0 works as virtiofsd user ID.

(In reply to Viktor Prutyanov from comment #3)
> How to test:
> 
> 1. Add "-o UID:GID" command line argument or set registry subkey "Owner" to
> "UID:GID" with desired host ID, or 0:0 for virtiofsd user ID.
> 2. Run VirtIO-FS.
> 3. If virtiofsd is privileged, new files and folders are created as
> specified ID. In unprivileged case, ID works if included in virtiofsd
> mapping and 0:0 works as virtiofsd user ID.

Hi(In reply to Viktor Prutyanov from comment #3)
> How to test:
> 
> 1. Add "-o UID:GID" command line argument or set registry subkey "Owner" to
> "UID:GID" with desired host ID, or 0:0 for virtiofsd user ID.
> 2. Run VirtIO-FS.
> 3. If virtiofsd is privileged, new files and folders are created as
> specified ID. In unprivileged case, ID works if included in virtiofsd
> mapping and 0:0 works as virtiofsd user ID.

Hi Yan, actually I'm not sure what the patch does. From comment0, I know that if creating a file in the shared dir, the new file's gid/uid would be set to shared directory's owner which is unexpected, what we hope is to set the new file's uid/gid to the users(virtiofsd process's owner), am I right?

The other thing is I'm trying to test with the previous steps for the privileged situation, but there is some virtiofsd error with '-o 1001:1001', here are detailed steps:
1. useradd u1 on host
# id u1
uid=1001(u1) gid=1001(u1) groups=1001(u1)
2. mk a dir under /home/u1
# ll
drwxr-xr-x. 2 u1   u1     62 Mar 27 02:58 test
3. switch u1 user and start virtiofsd
[u1@dell-per440-06 ~]$ /usr/libexec/virtiofsd -o source=test --socket-path=sock1
[2023-03-27T06:40:07Z WARN  virtiofsd] Use of deprecated option format '-o': Please specify options without it (e.g., '--cache auto' instead of '-o cache=auto')
[2023-03-27T06:40:07Z WARN  virtiofsd::limits] Failure when trying to set the limit to 1000000, the hard limit (8192) of open file descriptors is used instead.

4. start ws2022 guest and start virtiofs.exe with '-o 0:0'
success
5. try to create a new file in the shared Z:
success
-rw-rw-r--. 1 u1 u1 6 Mar 27 02:46 t3.txt

6. stop virtiofs.exe and restart with '-o 1001:1001'
success in guest and can create a new file.
-rw-rw-r--. 1 u1 u1 6 Mar 27 02:58 t4.txt

But there are error return from virtiofsd side.
[2023-03-27T06:58:53Z ERROR virtiofsd::passthrough] failed to change credentials back to root: Invalid argument (os error 22)
[2023-03-27T06:58:53Z ERROR virtiofsd::passthrough] failed to change credentials back to root: Invalid argument (os error 22)


virtiofsd version: virtiofsd-1.5.0-1.el9.x86_64
virtio-win prewhql version:0.1-235

Hi

These errors are OK. They just show that virtiofsd can't set uid=0, gid=0 (but this is not required). The file is still created.

(In reply to Viktor Prutyanov from comment #8)
> Hi
> 
> These errors are OK. They just show that virtiofsd can't set uid=0, gid=0
> (but this is not required). The file is still created.

Thanks Viktor, and in unprivileged case, could you speak more? I don't know how to test this scenario.

(In reply to Viktor Prutyanov from comment #3)
> How to test:
> 
> 1. Add "-o UID:GID" command line argument or set registry subkey "Owner" to
> "UID:GID" with desired host ID, or 0:0 for virtiofsd user ID.
> 2. Run VirtIO-FS.
> 3. If virtiofsd is privileged, new files and folders are created as
> specified ID. In unprivileged case, ID works if included in virtiofsd
> mapping and 0:0 works as virtiofsd user ID.

This is how to test

(In reply to Viktor Prutyanov from comment #10)
> (In reply to Viktor Prutyanov from comment #3)
> > How to test:
> > 
> > 1. Add "-o UID:GID" command line argument or set registry subkey "Owner" to
> > "UID:GID" with desired host ID, or 0:0 for virtiofsd user ID.
> > 2. Run VirtIO-FS.
> > 3. If virtiofsd is privileged, new files and folders are created as
> > specified ID. In unprivileged case, ID works if included in virtiofsd
> > mapping and 0:0 works as virtiofsd user ID.
> 
> This is how to test

I had seen this comment, but for the "In unprivileged case, ID works if included in virtiofsd mapping and 0:0 works as virtiofsd user ID." situation, does it need to work together with the following patch?
https://gitlab.com/virtio-fs/virtiofsd/-/merge_requests/137 

Or just the test steps in comment 7 is enough?

(In reply to xiagao from comment #11)
> (In reply to Viktor Prutyanov from comment #10)
> > (In reply to Viktor Prutyanov from comment #3)
> > > How to test:
> > > 
> > > 1. Add "-o UID:GID" command line argument or set registry subkey "Owner" to
> > > "UID:GID" with desired host ID, or 0:0 for virtiofsd user ID.
> > > 2. Run VirtIO-FS.
> > > 3. If virtiofsd is privileged, new files and folders are created as
> > > specified ID. In unprivileged case, ID works if included in virtiofsd
> > > mapping and 0:0 works as virtiofsd user ID.
> > 
> > This is how to test
> 
> I had seen this comment, but for the "In unprivileged case, ID works if
> included in virtiofsd mapping and 0:0 works as virtiofsd user ID."
> situation, does it need to work together with the following patch?
> https://gitlab.com/virtio-fs/virtiofsd/-/merge_requests/137 

Owner UID:GID feature was added to virtio-win before this virtiofsd PR were merged, so we need some time to determine how they are operate together.
> 
> Or just the test steps in comment 7 is enough?

At the moment, to test at least cases I described in comment #3 would be nice.

After https://gitlab.com/virtio-fs/virtiofsd/-/merge_requests/137 privileged case and unprivileged case without --uid-map and --gid-map should work just like before.

If --uid-map or --gid-map is specified, UID/GID specified through -o or Owner should be considered as UID/GID in virtiofsd user namespace.

(In reply to Viktor Prutyanov from comment #8)
> Hi
> 
> These errors are OK. They just show that virtiofsd can't set uid=0, gid=0
> (but this is not required). The file is still created.

Hi, Viktor
Can we remove this kind of error?
And in fact, this error occurs when setting '-o 1001:1001' but not '-o 0:0', so this is not trying to set uid=0,gid=0.

Thanks.

(In reply to xiagao from comment #14)
> (In reply to Viktor Prutyanov from comment #8)
> > Hi
> > 
> > These errors are OK. They just show that virtiofsd can't set uid=0, gid=0
> > (but this is not required). The file is still created.
> 
> Hi, Viktor
> Can we remove this kind of error?

This is virtiofsd side, but we can try to look at it.

> And in fact, this error occurs when setting '-o 1001:1001' but not '-o 0:0',
> so this is not trying to set uid=0,gid=0.

The virtiofsd daemon sets 1001:1001, then performs operation, then tries to restore previous UID:GID.
0:0 considered as credentials of virtiofsd process, so it tries to restore 0:0, fails and sets virtiofsd credentials as UID:GID which is completely OK for us.

> 
> Thanks.

I'm trying to verify this bug with unprivileged virtiofsd process, hit the following issue.

1. make a dir which owner and group is test_dir
drwxr-xr-x.  2 test   test            76 Apr 10 02:00 test_dir

# id test
uid=1000(test) gid=1000(test) groups=1000(test)

2. give group write permission to test_dir
# chmod g+w test_dir
drwxrwxr-x.  2 test   test            76 Apr 10 02:00 test_dir

3. create a user u1, and change it's supplement group to test.
# usermod -G 1000 u1
# id u1
uid=1001(u1) gid=1001(u1) groups=1001(u1),1000(test)

4. su u1 and start virtiofsd
[u1@dell-per440-06 ~]$ /usr/libexec/virtiofsd --shared-dir /home/test_dir --socket-path sock 

5. start vm

6. in guest, start virtiofs.exe with '-o 1000:1000'

7. in the shared dir of guest, try to write

actual result: Don't have permission to write to the shared directory.

Hi Viktor, I'm thinking the test user is the shared dir's owner, why can't it access the shared dir, is it expected or an issue? Btw, I think the basic function is ok, if it's another issue, I will open a new bug to track it.

Hi,

Supplementary groups are not taken into account by virtiofsd when it switches identity. So, this is an expected behavior.

Thanks for your kind reply.

Then I try to test the GID under unprivileged mode.
1. start a virtiofs under u1 user
[u1@dell-per440-06 ~]$ /usr/libexec/virtiofsd --socket-path ./sock --shared-dir /home/u1/test_u1 --sandbox none

Here is the shared directory, its group has written permission
# chmod g+w test_u1
drwxrwxr-x. 2 u1   u1     20 Apr 10 22:28 test_u1

Create a user u2, and change its group to u1
# usermod -g u1 u2
# id u2
uid=1002(u2) gid=1001(u1) groups=1001(u1)

2. in guest, start virtiofs.exe with '-o 1002:1001'

3. try to write to the shared dir

The result is failure to create a new file on it.

So, what's your opinion on this scenario?

You are running virtiofsd under u1, it can't switch to u2, so everything is right.

Thanks.
So, I think the test steps are clear now.
Here is the test case link, could you help to review it?
https://polarion.engineering.redhat.com/polarion/#/project/RHELVIRT/workitem?id=VIRT-297190

Change the bugs status to verify, as the basic functionality works.

For privileged mode:
start virtiofsd with root user and shared directory is test_dir
# /usr/libexec/virtiofsd --socket-path ./sock1 --shared-dir /home/test_dir

0)If not specify the -o parameter, the file's uid/gid is the same with shared dir.
#ll test_dir/file1
-rw-rw-r--. 1 test test 7 Apr 10 03:46 file1.txt
The ower/group are the same with the directory

1) setting uid/gid to 0:0, the file's uid/gid is the same with the process
VALUE=0:0
#ll test_dir/file2
-rw-rw-r--. 1 root root 7 Apr 10 03:46 file2.txt

2) Setting uid/gid to the known common user on host
VALUE=1000:1000 //'test' user id on host
#ll test_dir/file3
-rw-rw-r--. 1 test test 7 Apr 10 03:46 file3.txt

3) Setting uid/gidto the unknown user on host
VALUE=1111:1111 //unknown user
#ll test_dir/file4
-rw-rw-r--. 1 1111 1111 7 Apr 10 03:46 file4.txt

For unprivileged mode:
switch u1 user and start virtiofsd with /home/test_dir as the shared dir
[u1@dell-per440-06 ~]$ /usr/libexec/virtiofsd --shared-dir /home/test_dir --socket-path ./sock 
shared dir info:
drwxrwxr-x.  2 test   test            76 Apr 10 02:00 test_dir

0) without -o x:x parameter, the file's owner/group are the same with the process
-rw-rw-r--. 1 u1   u1   6 Apr 10 02:23 b1.txt

1) setting the uid/gid to 0:0
VALUE=0:0   
-rw-rw-r--. 1 u1   u1   5 Apr 10 02:24 b2.txt

2) setting the ower/group the same with the virtiofsd process's
VALUE=1001:1001 //'u1' user id on host
-rw-rw-r--. 1 u1   u1   5 Apr 10 02:26 b3.txt

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (virtio-win bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2023:6388