Re: [libvirt] [PATCH 0/5] security_stack: Perform rollback if one of stacked drivers fails
On 9/16/19 5:12 AM, Michal Privoznik wrote: See 5/5 for explanation. Michal Prívozník (5): security: Pass @migrated to virSecurityManagerSetAllLabel security: Rename virSecurityManagerGetDriver() to virSecurityManagerGetVirtDriver() security: Introduce virSecurityManagerGetDriver() security_stack: Turn list of nested drivers into a doubly linked list security_stack: Perform rollback if one of stacked drivers fails src/lxc/lxc_process.c| 2 +- src/qemu/qemu_process.c | 3 +- src/qemu/qemu_security.c | 6 +- src/qemu/qemu_security.h | 3 +- src/security/security_apparmor.c | 3 +- src/security/security_dac.c | 3 +- src/security/security_driver.h | 3 +- src/security/security_manager.c | 17 ++- src/security/security_manager.h | 4 +- src/security/security_nop.c | 3 +- src/security/security_selinux.c | 9 +- src/security/security_stack.c| 220 +-- tests/qemusecuritytest.c | 2 +- tests/securityselinuxlabeltest.c | 2 +- 14 files changed, 222 insertions(+), 58 deletions(-) For the series: Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] security_stack: Perform rollback if one of stacked drivers fails
On Thu, Oct 10, 2019 at 11:29:17AM +0100, Richard W.M. Jones wrote: > On Wed, Oct 09, 2019 at 07:49:29PM -0400, Cole Robinson wrote: > > In that bug, I see that rjones (cc'd) said that libvirt not > > remembering labels/uid causes issues for libguestfs that requires > > workarounds. Rich, do you have links to threads or bug reports where > > this is described in more detail? > > I think there are two problems (which I often confuse) and they are > possibly related. This one where libvirt doesn't restore permissions > afterwards, and the other one where qemu:///session cannot be used as > root which implies that when you run libguestfs as root it doesn't > have access to things that root would normally have access to (bug 890291 > / 1045069). > > In answer to your question this is the only one I could find which is > definitely related to this bug: > > https://www.redhat.com/archives/libguestfs/2013-May/msg00115.html Anything related to device nodes & permissions/ownership shouldn't be an issue any more. We switched to create a private mount namespace for each QEMU and setup a custom /dev populated with only the devices QEMU is allowed. Thus we should no longer be touching permisisons/owners in the real /dev > Here's another one, but I think this is related to the other bug: > > https://bugs.launchpad.net/nova/+bug/1241659/comments/6 > > I suspect there are cases where openstack sets LIBGUESTFS_BACKEND=direct > to workaround one of these two bugs. > > Is fixing the qemu:///session as root problem going to also solve this? If we had a real qemu:///session mode running QEMU itself as root, then we would never change permissions/ownership. We would still need to be changing SELinux labels & so the label restore logic is needd there. We should be able to use qemu:///system & the DAC driver to run QEMU as root though. There was previously a problem wrt monitor sockets that you hit when trying this with libguestfs, but I believe that should now be fixed: https://bugzilla.redhat.com/show_bug.cgi?id=890291#c30 If using the DAC driver to request running as root, the only remaining difference in terms of permissions is that we clear CAP_DAC_OVERRIDE, so the root user will only be able to access files which explicitly grant root access. We could fix this limitation in the DAC driver I believe to allow capabilities to be retained. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] security_stack: Perform rollback if one of stacked drivers fails
On Wed, Oct 09, 2019 at 07:49:29PM -0400, Cole Robinson wrote: > In that bug, I see that rjones (cc'd) said that libvirt not > remembering labels/uid causes issues for libguestfs that requires > workarounds. Rich, do you have links to threads or bug reports where > this is described in more detail? I think there are two problems (which I often confuse) and they are possibly related. This one where libvirt doesn't restore permissions afterwards, and the other one where qemu:///session cannot be used as root which implies that when you run libguestfs as root it doesn't have access to things that root would normally have access to (bug 890291 / 1045069). In answer to your question this is the only one I could find which is definitely related to this bug: https://www.redhat.com/archives/libguestfs/2013-May/msg00115.html Here's another one, but I think this is related to the other bug: https://bugs.launchpad.net/nova/+bug/1241659/comments/6 I suspect there are cases where openstack sets LIBGUESTFS_BACKEND=direct to workaround one of these two bugs. Is fixing the qemu:///session as root problem going to also solve this? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] security_stack: Perform rollback if one of stacked drivers fails
On 9/16/19 5:12 AM, Michal Privoznik wrote: See 5/5 for explanation. Michal Prívozník (5): security: Pass @migrated to virSecurityManagerSetAllLabel security: Rename virSecurityManagerGetDriver() to virSecurityManagerGetVirtDriver() security: Introduce virSecurityManagerGetDriver() security_stack: Turn list of nested drivers into a doubly linked list security_stack: Perform rollback if one of stacked drivers fails src/lxc/lxc_process.c| 2 +- src/qemu/qemu_process.c | 3 +- src/qemu/qemu_security.c | 6 +- src/qemu/qemu_security.h | 3 +- src/security/security_apparmor.c | 3 +- src/security/security_dac.c | 3 +- src/security/security_driver.h | 3 +- src/security/security_manager.c | 17 ++- src/security/security_manager.h | 4 +- src/security/security_nop.c | 3 +- src/security/security_selinux.c | 9 +- src/security/security_stack.c| 220 +-- tests/qemusecuritytest.c | 2 +- tests/securityselinuxlabeltest.c | 2 +- 14 files changed, 222 insertions(+), 58 deletions(-) I gotta admit I'm seriously wondering if supporting this label remembering stuff is worth it. I know you've put a heroic amount of work into it over a long period of time, but I think it's worth taking another look at this whole thing end to end to decide whether it's worth the complexity for what we are actually getting The old RHEL bug that was tracking this is here: https://bugzilla.redhat.com/show_bug.cgi?id=547546 It's closed because it was against RHEL7 and these patches aren't going to hit RHEL7. Is there still a major product or project issue that this is solving? In that bug, I see that rjones (cc'd) said that libvirt not remembering labels/uid causes issues for libguestfs that requires workarounds. Rich, do you have links to threads or bug reports where this is described in more detail? From the end user distro perspective, the main place I have historically heard people complain about this is basically: * download $ISO to home, owned by uid=crobinso * point virt-manager at it, which uses qemu:///system * VM starts, $ISO chown'd to uid=qemu * VM stops, $ISO chown'd to uid=root * Now there's a root owned image in your homedir. Worse, if you have a /media directory somewhere shared over http or some other service, owned as a non-root user, then changing to root owner can disrupt that access. This issue definitely annoys users. Unfortunately remember_owner doesn't help here because it's limited to RW media, which generally is less often shared than things like ISOs. I'm interested in hearing other concrete usecases that are solved by remember_owner (or at one time we thought would be solved by this) (in the mean time I will review your patches tomorrow) Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] security_stack: Perform rollback if one of stacked drivers fails
See 5/5 for explanation. Michal Prívozník (5): security: Pass @migrated to virSecurityManagerSetAllLabel security: Rename virSecurityManagerGetDriver() to virSecurityManagerGetVirtDriver() security: Introduce virSecurityManagerGetDriver() security_stack: Turn list of nested drivers into a doubly linked list security_stack: Perform rollback if one of stacked drivers fails src/lxc/lxc_process.c| 2 +- src/qemu/qemu_process.c | 3 +- src/qemu/qemu_security.c | 6 +- src/qemu/qemu_security.h | 3 +- src/security/security_apparmor.c | 3 +- src/security/security_dac.c | 3 +- src/security/security_driver.h | 3 +- src/security/security_manager.c | 17 ++- src/security/security_manager.h | 4 +- src/security/security_nop.c | 3 +- src/security/security_selinux.c | 9 +- src/security/security_stack.c| 220 +-- tests/qemusecuritytest.c | 2 +- tests/securityselinuxlabeltest.c | 2 +- 14 files changed, 222 insertions(+), 58 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list