Bug 1747960

Do both machines (host and guest) have SELinux enabled?

This is same issue like rhbz#1748079.

*** Bug 1690291 has been marked as a duplicate of this bug. ***

*** Bug 1738845 has been marked as a duplicate of this bug. ***

(In reply to Milos Malik from comment #1)
> Do both machines (host and guest) have SELinux enabled?

No, It only enabled SELinux inside guest.

(In reply to Lukas Vrabec from comment #2)
> This is same issue like rhbz#1748079.

It actually doesn't. The difference is that in the bug you're referencing libvirt prevented itself from setting a different SELinux label on a disk image that's already in use. In this bug qemu guest agent is unable to perform fstrim. This needs to be fixed in selinux policy.

Hey folks,

The root directory on the sdb2 device has no label:

type=AVC msg=audit(09/02/2019 19:06:22.870:140) : avc:  denied  { read } for  pid=1978 comm=qemu-ga name=/ dev="sdb" ino=2 scontext=system_u:system_r:virt_qemu_ga_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=dir permissive=0

It may be similar for other files on the filesystem, but we do not have enough data to assert that. Anyway, this issue needs to be fixed on the device used.

Additionally, DAC permissions may also need to be adjusted so that root does not require CAP_DAC_OVERRIDE, but this is not completely clear as in the audit logs we can see a set of 2 possibly independent actions.

Switching the component to qemu-kvm as it seems to be the closest one in the dropdown to choose.
Should there be any question on interpreting the AVC denials, feel free to ask.

QEMU has been recently split into sub-components and as a one-time operation to avoid breakage of tools, we are setting the QEMU sub-component of this BZ to "General". Please review and change the sub-component if necessary the next time you review this BZ. Thanks

Hit a similar issue.

1. boot up guest with two disks and check selinux policy.
# getenforce
Enforcing

2. format the data disk and mount
# mkfs.ext4 /dev/sda1
# mount /dev/sda1 /media

3. issue fsfreeze cmd
{ "execute": "guest-fsfreeze-freeze"}
{"error": {"class": "GenericError", "desc": "failed to open /media: Permission denied"}}

4.# setenforce 0
[root@vm-74-77 home]# getenforce
Permissive

5. issue fsfreeze cmd
{ "execute": "guest-fsfreeze-freeze"}
{"return": 3}

pkg:
[root@vm-74-77 home]# rpm -qa |grep agent
qemu-guest-agent-2.12.0-98.module+el8.2.0+5698+10a84757.x86_64

Are there any SELinux denials?

# ausearch -m avc -m user_avc -m selinux_err -m user_selinux_err -i -ts today

Is the /dev/sda1 device labeled correctly?

# restorecon -Rvn /media

This is all happening in a *guest*, where qemu-ga is running and trying to do
its job, which selinux forbids in enforcing mode. This has nothing to do with
labeling of host resources done by libvirt.

Thus the solution needs to be negotiated between qemu-ga and selinux-policy.
IMHO requiring any specific labels on any file system for qemu-ga to be able
to perform fsfreeze would be strange. I can imagine a selinux bool which would
enable qemu-ga (virt_qemu_ga_t) do fo fsfreeze on any mount point so that the
guest admin can decide whether they want fsfreeze to work or not. But I guess
who know how fsfreeze is implemented in qemu-ga could talk to selinux guys and
come up with other options and choose the right one.

BTW, shouldn't there be a subcomponent for guest side stuff (qemu-ga)?

(In reply to Milos Malik from comment #10)
> Are there any SELinux denials?
> 
> # ausearch -m avc -m user_avc -m selinux_err -m user_selinux_err -i -ts today

yes.

----
type=USER_AVC msg=audit(02/26/2020 03:31:40.968:33) : pid=941 uid=dbus auid=unset ses=unset subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 msg='avc:  denied  { send_msg } for msgtype=method_call interface=org.freedesktop.DBus.Properties member=GetAll dest=:1.14 spid=1018 tpid=1023 scontext=system_u:system_r:accountsd_t:s0 tcontext=system_u:system_r:realmd_t:s0 tclass=dbus permissive=0  exe=/usr/bin/dbus-daemon sauid=dbus hostname=? addr=? terminal=?' 
----
type=USER_AVC msg=audit(02/26/2020 03:31:50.684:72) : pid=941 uid=dbus auid=unset ses=unset subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 msg='avc:  denied  { send_msg } for msgtype=method_call interface=org.freedesktop.DBus.Properties member=GetAll dest=:1.14 spid=1018 tpid=1023 scontext=system_u:system_r:accountsd_t:s0 tcontext=system_u:system_r:realmd_t:s0 tclass=dbus permissive=0  exe=/usr/bin/dbus-daemon sauid=dbus hostname=? addr=? terminal=?' 
----
type=USER_AVC msg=audit(02/26/2020 08:32:09.652:93) : pid=941 uid=dbus auid=unset ses=unset subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 msg='avc:  denied  { send_msg } for msgtype=method_call interface=org.freedesktop.DBus.Properties member=GetAll dest=:1.14 spid=1018 tpid=1023 scontext=system_u:system_r:accountsd_t:s0 tcontext=system_u:system_r:realmd_t:s0 tclass=dbus permissive=0  exe=/usr/bin/dbus-daemon sauid=dbus hostname=? addr=? terminal=?' 
----
type=USER_AVC msg=audit(02/26/2020 08:32:39.586:116) : pid=941 uid=dbus auid=unset ses=unset subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 msg='avc:  denied  { send_msg } for msgtype=method_call interface=org.freedesktop.DBus.Properties member=GetAll dest=:1.14 spid=1018 tpid=1023 scontext=system_u:system_r:accountsd_t:s0 tcontext=system_u:system_r:realmd_t:s0 tclass=dbus permissive=0  exe=/usr/bin/dbus-daemon sauid=dbus hostname=? addr=? terminal=?' 
----
type=AVC msg=audit(02/26/2020 08:34:46.101:119) : avc:  denied  { read } for  pid=938 comm=qemu-ga name=/ dev="sda1" ino=2 scontext=system_u:system_r:virt_qemu_ga_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=dir permissive=0 
----
type=AVC msg=audit(02/26/2020 08:34:46.101:120) : avc:  denied  { read } for  pid=938 comm=qemu-ga name=/ dev="sda1" ino=2 scontext=system_u:system_r:virt_qemu_ga_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=dir permissive=0 


> 
> Is the /dev/sda1 device labeled correctly?
> 
> # restorecon -Rvn /media

# restorecon -Rvn /media
Would relabel /media from system_u:object_r:unlabeled_t:s0 to system_u:object_r:mnt_t:s0
Would relabel /media/lost+found from system_u:object_r:unlabeled_t:s0 to system_u:object_r:mnt_t:s0

Why does /media lose mnt_t after mount ?

Can you make qemu-ga permission for fstrim more permissive, so it can trim all mount points?

In any case, I don't see what solution qemu-ga could have here.

thanks

*** Bug 1823636 has been marked as a duplicate of this bug. ***

Zdenek Pytela - a needinfo on Milos Malik has gone unanswered since Feb - perhaps you can help move this along?

John,

Without further context this answer may not be complete. I suppose /media is an empty directory first: it gets mnt_t type which ic correct. If you mount a filesystem to /media, all files and directories from the filesystem appear on this mountpoint, including their SELinux context. If the filesystem does not contain SELinux context or does not support extended attributes at all, unlabeled_t as the type appears.

(In reply to Zdenek Pytela from comment #19)
> John,
> 
> Without further context this answer may not be complete. I suppose /media is
> an empty directory first: it gets mnt_t type which ic correct. If you mount
> a filesystem to /media, all files and directories from the filesystem appear
> on this mountpoint, including their SELinux context. If the filesystem does
> not contain SELinux context or does not support extended attributes at all,
> unlabeled_t as the type appears.

What can qemu-ga do? restorecon before freeze? That sounds wrong.

Can SELinux rules be more permissive for qemu-ga to freeze any FS?

Marc-Andre,

There are permissions we can allow in the policy if it it required.

However, can you help me with understanding what is going on?
What is the "freeze" used for?
Why the mounted filesystems were not created with selinux enabled?
Is there an actual use case available?

(In reply to Zdenek Pytela from comment #21)
> However, can you help me with understanding what is going on?
> What is the "freeze" used for?

Afaik, the qemu guest agent can help migration load/time (or snapshot, or other cases) by triming and freezing all filesystems:

https://git.qemu.org/?p=qemu.git;a=blob;f=qga/qapi-schema.json;h=4be9aad48e26931022663801e0123f479792e899;hb=HEAD#l511
https://git.qemu.org/?p=qemu.git;a=blob;f=qga/qapi-schema.json;h=4be9aad48e26931022663801e0123f479792e899;hb=HEAD#l450

> Why the mounted filesystems were not created with selinux enabled?

That's out of qemu-ga control.

Thank you for the answers. I think we can switch the component to selinux-policy and resolve it 
- either for this particular request, i. e. for images with no selinux extended attributes
- or for all types of files/dirs allowing quite a generic access: in this case it would grant too many permissions to the virt_qemu_ga_t domain (qemu-ga is the command) and possibly pose a security risk. Would be okay then to only allow the access when a boolean (e. g. virt_qemu_ga_read_all_files) is turned on? With the boolean in place, virt_qemu_ga_t would not have this access until enabled by an administrator or a script.
Additionally, is read access sufficient to finish the operation?
Is there a test for this feature available? With selinux permissive, it would be good to gather all possible subsequent denials.


Also note the USER_AVCs from c#15 are allowed in the current selinux-policy package version/

(In reply to Zdenek Pytela from comment #23)
> Thank you for the answers. I think we can switch the component to
> selinux-policy and resolve it 
> - either for this particular request, i. e. for images with no selinux
> extended attributes
> - or for all types of files/dirs allowing quite a generic access: in this
> case it would grant too many permissions to the virt_qemu_ga_t domain
> (qemu-ga is the command) and possibly pose a security risk. Would be okay
> then to only allow the access when a boolean (e. g.
> virt_qemu_ga_read_all_files) is turned on? With the boolean in place,
> virt_qemu_ga_t would not have this access until enabled by an administrator
> or a script.

The second option seems good to me. Do we already have other selinux bool for qemu-ga ?

> Additionally, is read access sufficient to finish the operation?

Probably not, since it can do ioctl(fd, FITRIM) & ioctl(fd, FIFREEZE/FITHAW) on them

> Is there a test for this feature available? With selinux permissive, it
> would be good to gather all possible subsequent denials.

Unfortunately, most qemu-ga tests are manual, here following the steps in the comments. QA might have some extra test/avocado scripts.

> 
> Also note the USER_AVCs from c#15 are allowed in the current selinux-policy
> package version/

ok, thanks

(In reply to Marc-Andre Lureau from comment #24)
> (In reply to Zdenek Pytela from comment #23)
> > Thank you for the answers. I think we can switch the component to
> > selinux-policy and resolve it 
> > - either for this particular request, i. e. for images with no selinux
> > extended attributes
> > - or for all types of files/dirs allowing quite a generic access: in this
> > case it would grant too many permissions to the virt_qemu_ga_t domain
> > (qemu-ga is the command) and possibly pose a security risk. Would be okay
> > then to only allow the access when a boolean (e. g.
> > virt_qemu_ga_read_all_files) is turned on? With the boolean in place,
> > virt_qemu_ga_t would not have this access until enabled by an administrator
> > or a script.
> 
> The second option seems good to me. Do we already have other selinux bool
> for qemu-ga ?

  # semanage boolean -l|grep qemu_ga
virt_read_qemu_ga_data         (off  ,  off)  Allow qemu-ga to read qemu-ga date.
virt_rw_qemu_ga_data           (off  ,  off)  Allow qemu-ga to manage qemu-ga date.

This is, however, the other way round, allowing other domain deal with files labeled virt_qemu_ga_data_t.

> 
> > Additionally, is read access sufficient to finish the operation?
> 
> Probably not, since it can do ioctl(fd, FITRIM) & ioctl(fd, FIFREEZE/FITHAW)
> on them
> 
We usually allow the following set:

getattr read ioctl lock

and optionally open (supposedly required in this case) and map (supposedly not); particularly, as I understand, no write permission is necessary. The target set is files, directories, symlinks. Can we expect there are also device files, named pipes, socket files etc. in the image?

> > Is there a test for this feature available? With selinux permissive, it
> > would be good to gather all possible subsequent denials.
> 
> Unfortunately, most qemu-ga tests are manual, here following the steps in
> the comments. QA might have some extra test/avocado scripts.
That's really unfortunate, there is a risk the fix in selinux-policy may not be complete, and we will hardly be able to perform correct functional testing.

Dehan Meng: Will you be able to help with testing?


Switching the component given the latest development.

I've submitted a Fedora PR to address the issue:
https://github.com/fedora-selinux/selinux-policy-contrib/pull/317

Please review to assure no important piece is missing.
The proposed boolean name is virt_qemu_ga_read_nonsecurity_files.

This is the list of permissions to be added:
allow virt_qemu_ga_t non_security_file_type:dir { getattr search open read lock ioctl };
allow virt_qemu_ga_t non_security_file_type:file { open { getattr read ioctl lock } };
allow virt_qemu_ga_t non_security_file_type:lnk_file { getattr read };

Still Hit this issue again

Version-Release：
1.selinux-policy-3.14.3-51.el8.noarch
2. Four qga version :
  1) qemu-guest-agent-5.1.0-2.module+el8.3.0+7652+b30e6901.x86_64 / 
  2) qemu-guest-agent-5.1.0-0.scrmod+el8.3.0+7646+9f889f10.wrb200812.x86_64 / 
  3) qemu-guest-agent-4.2.0-31.module+el8.3.0+7437+4bb96e0d / 
  4) qemu-guest-agent-4.2.0-32.module+el8.3.0+7629+c86ce105.x86_64
above four version of qga in guest that I run  were hit this issue again 

describe steps:
1. boot up guest with two disk successfully and check selinux policy.
#getenforce
Enforcing

2.format the data disk and mount
# mkfs.ext4 /dev/sdb
# mount /dev/sda1 /mnt/sdb
3. issue ping,status,fsfreeze cmd
{"execute":"guest-ping"}	
{"return": {}}
{"execute":"guest-fsfreeze-status"}
{"return": "thawed"}
{ "execute": "guest-fsfreeze-freeze"}
{"error": {"class": "GenericError", "desc": "failed to open /mnt/sdb: Permission denied"}}

expected result:
{ "execute": "guest-fsfreeze-freeze"}
{"return": 3}

4.issue command `setenforce 0`
# getenforce
Permissive

5. issue fsfreeze cmd
{ "execute": "guest-fsfreeze-freeze"}
{"return": 3}

Dehan,

Just ensuring: has the virt_qemu_ga_read_nonsecurity_files boolean been turned on before testing?

If yes, please include AVC denials from the step 2.

(In reply to Zdenek Pytela from comment #32)
> Dehan,
> 
> Just ensuring: has the virt_qemu_ga_read_nonsecurity_files boolean been
> turned on before testing?
> 
> If yes, please include AVC denials from the step 2.

Zdenek 

I've checked virt_qemu_ga_read_nonsecurity_files boolean was off .
and then I tried to setsebool boolean virt_qemu_ga_read_nonsecurity_files to on,permission accessed. 
So I need to set selinux boolean value before testing every single time?

thanks,Zdenek

Dehan,

The boolean is turned off by default, see the previous discussion for the context. Turning it on using semanage boolean or setsebool -P commands, the change is permanent; otherwise is valid until reboot.

This is the default state:

  # semanage boolean -l | grep virt_qemu_ga_read_nonsecurity_files
virt_qemu_ga_read_nonsecurity_files (off  ,  off)  Allow qemu-ga read all non-security file types.

(In reply to Zdenek Pytela from comment #34)
> Dehan,
> 
> The boolean is turned off by default, see the previous discussion for the
> context. Turning it on using semanage boolean or setsebool -P commands, the
> change is permanent; otherwise is valid until reboot.
> 
> This is the default state:
> 
>   # semanage boolean -l | grep virt_qemu_ga_read_nonsecurity_files
> virt_qemu_ga_read_nonsecurity_files (off  ,  off)  Allow qemu-ga read all
> non-security file types.

thanks for you explanation ,I've done with your suggestions and know how to set selinux boolean value ,Added steps to case yet.

thanks
dehan

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 (selinux-policy 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-2020:4528