Bug 1652078
Summary: | RFE: the security drivers must remember original permissions/labels and restore them after | ||
---|---|---|---|
Product: | Red Hat Enterprise Linux Advanced Virtualization | Reporter: | Jaroslav Suchanek <jsuchane> |
Component: | libvirt | Assignee: | Michal Privoznik <mprivozn> |
Status: | CLOSED ERRATA | QA Contact: | Luyao Huang <lhuang> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | --- | CC: | adrien+bugzillaredhat, berrange, cwei, cww, dyuan, fjin, fj-lsoft-ofuku, gsun, hbrock, hhan, jdenemar, jiyan, jiyin, jsuchane, kanderso, karlcz, knoel, laine, mkalinin, mprivozn, mschuppe, mtessun, mzhan, obockows, pablo.iranzo, rbalakri, rjones, spetreolle, wchadwic, xen-maint, xuzhang, yafu, yisun |
Target Milestone: | rc | Keywords: | Automation, FutureFeature, Reopened |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | libvirt-5.6.0-1.el8 | Doc Type: | Enhancement |
Doc Text: | Story Points: | --- | |
Clone Of: | 547546 | Environment: | |
Last Closed: | 2019-11-06 07:12:03 UTC | Type: | Feature Request |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 1740024, 1740506, 1741140, 1741456, 1771500, 1771501, 1780154 | ||
Bug Blocks: | 756082, 822052, 829181, 1126678, 1203710, 1205796, 1288337, 1420851, 1477664, 1524792, 1623566 |
Description
Jaroslav Suchanek
2018-11-21 14:24:22 UTC
Per Dec 6th blocker meeting, approved exception+ Moving to post accordingly to bug 1652078. (In reply to Jaroslav Suchanek from comment #6) > Moving to post accordingly to bug 1652078. Should be BZ 547546, I guess. :) This feature was disabled upstream due to some non-trivial bugs by commit fc3990c7e64be1da1631952d3ec384ebef50e125 Refs: v5.0.0-rc2-4-gfc3990c7e6 Author: Michal Privoznik <mprivozn> AuthorDate: Mon Jan 14 17:53:43 2019 +0100 Commit: Michal Privoznik <mprivozn> CommitDate: Tue Jan 15 09:45:22 2019 +0100 qemu: Temporary disable owner remembering Turns out, that there are few bugs that are not that trivial to fix (e.g. around block jobs). Instead of rushing in not thoroughly tested fixes disable the feature temporarily for the release. Signed-off-by: Michal Privoznik <mprivozn> ACKed-by: Peter Krempa <pkrempa> v4: https://www.redhat.com/archives/libvir-list/2019-April/msg01351.html Previous versions were targeting bug 547546. Hi Michal I found the following test result for guestfwd channel chardev. Could you please check whether it is normal? Thank you. Description: Xattr for “guestfwd channel” device will not be cleaned after VM was destroyed or device was detached; thus; Other VM can not use it. Version: libvirt-5.6.0-4.module+el8.1.0+4160+b50057dc.x86_64 qemu-kvm-4.1.0-9.module+el8.1.0+4210+23b2046a.x86_64 kernel-4.18.0-141.el8.x86_64 Steps: 1: Prepare 2 guests as follows: # virsh list --all Id Name State --------------------------------- - avocado-vt-vm1 shut off - test running 2. Start 1 guest with the following guestfwd channel and check file label # virsh dumpxml avocado-vt-vm1 --inactive |grep "<channel" -A5 <channel type='unix'> <source mode='bind' path='/mnt/guestfwd'/> <target type='guestfwd' address='10.0.2.1' port='4600'/> <alias name='ua-d830c2c4-93ac-4eb7-b714-593483f10044'/> </channel> # virsh start avocado-vt-vm1 Domain avocado-vt-vm1 started # getfattr -m trusted.libvirt.security -d /mnt/guestfwd getfattr: Removing leading '/' from absolute path names # file: tmp/guestfwd trusted.libvirt.security.dac="+0:+0" trusted.libvirt.security.ref_dac="1" trusted.libvirt.security.timestamp_dac="1568151376" 3. Detach channel device from the gust and check file label again; attach the device to the other guest # virsh detach-device-alias avocado-vt-vm1 ua-d830c2c4-93ac-4eb7-b714-593483f10044 Device detach request sent successfully # virsh dumpxml avocado-vt-vm1|grep "<channel" -A5 No output # getfattr -m trusted.libvirt.security -d /mnt/guestfwd getfattr: Removing leading '/' from absolute path names # file: tmp/guestfwd trusted.libvirt.security.dac="+0:+0" trusted.libvirt.security.ref_dac="1" trusted.libvirt.security.timestamp_dac="1568151376" # cat unix_channel_2.xml <channel type='unix'> <source mode='bind' path='/mnt/guestfwd'/> <target type='guestfwd' address='10.0.2.1' port='4600'/> <alias name='ua-d830c2c4-93ac-4eb7-b714-593483f10044'/> </channel> # virsh attach-device test unix_channel_2.xml error: Failed to attach device from unix_channel_2.xml error: internal error: unable to execute QEMU command 'chardev-add': Failed to unlink socket /mnt/guestfwd: Permission denied 4. Destroy the first guest and attach the device to the other one again # virsh destroy avocado-vt-vm1 Domain avocado-vt-vm1 destroyed # getfattr -m trusted.libvirt.security -d /tmp/guestfwd getfattr: Removing leading '/' from absolute path names # file: tmp/guestfwd trusted.libvirt.security.dac="+0:+0" trusted.libvirt.security.ref_dac="2" trusted.libvirt.security.timestamp_dac="1568151376" # virsh attach-device test unix_channel_2.xml error: Failed to attach device from unix_channel_2.xml error: internal error: unable to execute QEMU command 'chardev-add': Failed to unlink socket /mnt/guestfwd: Permission denied (In reply to jiyan from comment #12) > Hi Michal > I found the following test result for guestfwd channel chardev. > Could you please check whether it is normal? Thank you. > > Description: > Xattr for “guestfwd channel” device will not be cleaned after VM was > destroyed > or device was detached; thus; Other VM can not use it. > > Version: > libvirt-5.6.0-4.module+el8.1.0+4160+b50057dc.x86_64 > qemu-kvm-4.1.0-9.module+el8.1.0+4210+23b2046a.x86_64 > kernel-4.18.0-141.el8.x86_64 > > Steps: > 1: Prepare 2 guests as follows: > # virsh list --all > Id Name State > --------------------------------- > - avocado-vt-vm1 shut off > - test running > > 2. Start 1 guest with the following guestfwd channel and check file label > # virsh dumpxml avocado-vt-vm1 --inactive |grep "<channel" -A5 > <channel type='unix'> > <source mode='bind' path='/mnt/guestfwd'/> > <target type='guestfwd' address='10.0.2.1' port='4600'/> > <alias name='ua-d830c2c4-93ac-4eb7-b714-593483f10044'/> > </channel> > > # virsh start avocado-vt-vm1 > Domain avocado-vt-vm1 started > > # getfattr -m trusted.libvirt.security -d /mnt/guestfwd > getfattr: Removing leading '/' from absolute path names > # file: tmp/guestfwd > trusted.libvirt.security.dac="+0:+0" > trusted.libvirt.security.ref_dac="1" > trusted.libvirt.security.timestamp_dac="1568151376" > > 3. Detach channel device from the gust and check file label again; attach > the device to the other guest > # virsh detach-device-alias avocado-vt-vm1 > ua-d830c2c4-93ac-4eb7-b714-593483f10044 > Device detach request sent successfully > > # virsh dumpxml avocado-vt-vm1|grep "<channel" -A5 > No output > > # getfattr -m trusted.libvirt.security -d /mnt/guestfwd > getfattr: Removing leading '/' from absolute path names > # file: tmp/guestfwd > trusted.libvirt.security.dac="+0:+0" > trusted.libvirt.security.ref_dac="1" > trusted.libvirt.security.timestamp_dac="1568151376" Why is the file still there? When I try to reproduce the file is unlinked as soon as qemu closes the socket. At this point I get 'no such file or directory' error. Do you have something connected to the socket? > > # cat unix_channel_2.xml > <channel type='unix'> > <source mode='bind' path='/mnt/guestfwd'/> > <target type='guestfwd' address='10.0.2.1' port='4600'/> > <alias name='ua-d830c2c4-93ac-4eb7-b714-593483f10044'/> > </channel> > > # virsh attach-device test unix_channel_2.xml > error: Failed to attach device from unix_channel_2.xml > error: internal error: unable to execute QEMU command 'chardev-add': Failed > to unlink socket /mnt/guestfwd: Permission denied > > 4. Destroy the first guest and attach the device to the other one again > # virsh destroy avocado-vt-vm1 > Domain avocado-vt-vm1 destroyed > > # getfattr -m trusted.libvirt.security -d /tmp/guestfwd > getfattr: Removing leading '/' from absolute path names > # file: tmp/guestfwd > trusted.libvirt.security.dac="+0:+0" > trusted.libvirt.security.ref_dac="2" > trusted.libvirt.security.timestamp_dac="1568151376" I don't understand how is /tmp/guestfwd related to all of this - the XMLs show /mnt/... and not /tmp/... > > # virsh attach-device test unix_channel_2.xml > error: Failed to attach device from unix_channel_2.xml > error: internal error: unable to execute QEMU command 'chardev-add': Failed > to unlink socket /mnt/guestfwd: Permission denied Anyway, from code inspection I think there might be a problem because our set and restore code doesn't look exactly the same. So in a few cases more we might set the label but then not restore it. This is the patch: diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9e71513f14..1600abaf54 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1565,9 +1565,11 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr, break; case VIR_DOMAIN_CHR_TYPE_UNIX: - if (!dev_source->data.nix.listen && - virSecurityDACRestoreFileLabel(mgr, dev_source->data.nix.path) < 0) { - goto done; + if (!dev_source->data.nix.listen || + (dev_source->data.nix.path && + virFileExists(dev_source->data.nix.path))) { + if (virSecurityDACRestoreFileLabel(mgr, dev_source->data.nix.path) < 0) + goto done; } ret = 0; break; Can you test it please (or I can create a scratch build for you with that patch if you want me to)? Hi it is better to create a scratch build; I will test once I get it. :) Hi Michal Sry for testing the scratch build a little late. The following test steps shows; after detaching the channel device; the xattr info will be cleared; however, attaching the channel device to another guest failed as well. Steps: # rpm -qa libvirt libvirt-5.6.0-5.module+el8.1.0+4158+21722aafchannel.x86_64 # virsh list --all Id Name State ------------------------- - test_1 shut off - test_2 shut off # virsh dumpxml test_1 --inactive |grep "<channel" -A4 <channel type='unix'> <source mode='bind' path='/mnt/guestfwd'/> <target type='guestfwd' address='10.0.2.1' port='4600'/> <alias name='ua-d830c2c4-93ac-4eb7-b714-593483f10044'/> </channel> # virsh dumpxml test_2 --inactive |grep "<channel" -A4 No output # virsh start test_1 Domain test_1 started # virsh start test_2 Domain test_2 started # getfattr -m trusted.libvirt.security -d /mnt/guestfwd getfattr: Removing leading '/' from absolute path names # file: mnt/guestfwd trusted.libvirt.security.dac="+0:+0" trusted.libvirt.security.ref_dac="1" trusted.libvirt.security.timestamp_dac="1568706200" # virsh detach-device-alias test_1 ua-d830c2c4-93ac-4eb7-b714-593483f10044 Device detach request sent successfully # getfattr -m trusted.libvirt.security -d /mnt/guestfwd No output # virsh dumpxml test_1 |grep "<channel" -A4 No output # virsh attach-device test_2 channel.xml error: Failed to attach device from channel.xml error: internal error: unable to execute QEMU command 'chardev-add': Failed to unlink socket /mnt/guestfwd: Permission denied # getfattr -m trusted.libvirt.security -d /mnt/guestfwd # virsh attach-device test_2 channel.xml error: Failed to attach device from channel.xml error: internal error: unable to execute QEMU command 'chardev-add': Failed to unlink socket /mnt/guestfwd: Permission denied Since the bug: Bug 1591636 - Fails to hotplug/unplug guestfwd character device has been verified on libvirt-5.3.0-1.el8. Could you check whether the code changes have a influence on this feature? Thank you. :) The issue in comment 12 - comment 16 will be tracked in this bug https://bugzilla.redhat.com/show_bug.cgi?id=1754392, thank you. :) Verify this bug with libvirt-5.6.0-6.module+el8.1.0+4244+9aa4e6bb.x86_64: S1 Test file permission selinux/dac label remembering: 1. prepare a guest which disk use local file # virsh dumpxml vm1 ... <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2'/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </disk> ... 2. check guest image label # ll -Z /var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 -rw-r--r--. 1 root root system_u:object_r:virt_image_t:s0 722403328 Sep 24 22:48 /var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 3. check guest image XATTR param # getfattr -m trusted.libvirt.security -d /var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 4. start guest # virsh start vm1 Domain vm1 started 5. check image label # ll -Z /var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 -rw-r--r--. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c748,c836 722403328 Sep 24 22:49 /var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 6. check guest image XATTR param # getfattr -m trusted.libvirt.security -d /var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 getfattr: Removing leading '/' from absolute path names # file: var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 trusted.libvirt.security.dac="+0:+0" trusted.libvirt.security.ref_dac="1" trusted.libvirt.security.ref_selinux="1" trusted.libvirt.security.selinux="system_u:object_r:virt_image_t:s0" trusted.libvirt.security.timestamp_dac="1569204150" trusted.libvirt.security.timestamp_selinux="1569204150" 7. destroy guest # virsh destroy vm1 Domain vm1 destroyed 8. verify libvirt restore the label # ll -Z /var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 -rw-r--r--. 1 root root system_u:object_r:virt_image_t:s0 722403328 Sep 24 22:50 /var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 9. verify libvirt clear XATTR # getfattr -m trusted.libvirt.security -d /var/lib/libvirt/images/RHEL-8.1-x86_64-latest.qcow2 And test with different operation: hotplug, unplug, start, destroy, migrate and with different device: bios, disk, interface, channel, console, input, memory S2 Test ref count change # getfattr -m trusted.libvirt.security -d /tmp/nvdimm getfattr: Removing leading '/' from absolute path names # file: tmp/nvdimm trusted.libvirt.security.dac="+0:+0" trusted.libvirt.security.ref_dac="1" trusted.libvirt.security.ref_selinux="1" trusted.libvirt.security.selinux="unconfined_u:object_r:user_tmp_t:s0" trusted.libvirt.security.timestamp_dac="1569204150" trusted.libvirt.security.timestamp_selinux="1569204150" # virsh attach-device vm1 nvdimm.xml Device attached successfully # getfattr -m trusted.libvirt.security -d /tmp/nvdimm getfattr: Removing leading '/' from absolute path names # file: tmp/nvdimm trusted.libvirt.security.dac="+0:+0" trusted.libvirt.security.ref_dac="2" trusted.libvirt.security.ref_selinux="2" trusted.libvirt.security.selinux="unconfined_u:object_r:user_tmp_t:s0" trusted.libvirt.security.timestamp_dac="1569204150" trusted.libvirt.security.timestamp_selinux="1569204150" S3 Test remember_owner in qemu.conf 1. do the same test with S1 when disable remember_owner and verify there is no regression and there is no XATTR 3. start a guest with enable remember_owner in qemu.conf then disable remember_owner and restart libvirtd then do hotplug/hotunplug/destroy to test XATTR set/clear 4. start a guest with disable remember_owner in qemu.conf then enable remember_owner and restart libvirtd then do hotplug/hotunplug/destroy and check there is no XATTR change Clearing out sale needinfo. 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, 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-2019:3723 |