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

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

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

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

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 <crobinso@redhat.com>

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list