[PATCH] apparmor: fix qemu_bridge_helper for named profile

Christian Ehrhardt posted 1 patch 4 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200130072126.27131-1-christian.ehrhardt@canonical.com
src/security/apparmor/usr.sbin.libvirtd | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] apparmor: fix qemu_bridge_helper for named profile
Posted by Christian Ehrhardt 4 years, 2 months ago
Since a3ab6d42 "apparmor: convert libvirtd profile to a named profile"
the detection of the subelement for qemu_bridge_helper is wrong.

In combination with the older 123cc3e1 "apparmor: allow
/usr/lib/qemu/qemu-bridge-helper" it now detects qemu-bridge-helper no
more with its path, but instead as a proper subelement of the named profile
like: label=libvirtd//qemu_bridge_helper

In the same fashion the reverse rule in the qemu_bridge_helper
sub-profile still uses the path and not the named profile label.

Triggering denies like:
apparmor="DENIED" operation="file_inherit"
  profile="libvirtd//qemu_bridge_helper" pid=5629 comm="qemu-bridge-hel"
  family="unix" sock_type="stream" protocol=0 requested_mask="send receive"
  denied_mask="send receive" addr=none peer_addr=none peer="libvirtd"

This patch fixes the unix socket rules for the communication between
libvirtd and qemu-bridge-helper to match that.

Fixes: a3ab6d42d825499af44b8f19f9299e150d9687bc
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1655111

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 src/security/apparmor/usr.sbin.libvirtd | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd
index 29f9936ad9..172972e525 100644
--- a/src/security/apparmor/usr.sbin.libvirtd
+++ b/src/security/apparmor/usr.sbin.libvirtd
@@ -62,8 +62,8 @@ profile libvirtd /usr/sbin/libvirtd flags=(attach_disconnected) {
   signal (send) set=("kill", "term") peer=unconfined,
 
   # For communication/control to qemu-bridge-helper
-  unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/libvirtd//qemu_bridge_helper),
-  signal (send) set=("term") peer=/usr/sbin/libvirtd//qemu_bridge_helper,
+  unix (send, receive) type=stream addr=none peer=(label=libvirtd//qemu_bridge_helper),
+  signal (send) set=("term") peer=libvirtd//qemu_bridge_helper,
 
   # allow connect with openGraphicsFD, direction reversed in newer versions
   unix (send, receive) type=stream addr=none peer=(label=libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*),
@@ -121,7 +121,7 @@ profile libvirtd /usr/sbin/libvirtd flags=(attach_disconnected) {
    network inet stream,
 
    # For communication/control from libvirtd
-   unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/libvirtd),
+   unix (send, receive) type=stream addr=none peer=(label=libvirtd),
    signal (receive) set=("term") peer=/usr/sbin/libvirtd,
    signal (receive) set=("term") peer=libvirtd,
 
-- 
2.25.0


Re: [PATCH] apparmor: fix qemu_bridge_helper for named profile
Posted by Michal Privoznik 4 years, 2 months ago
On 1/30/20 8:21 AM, Christian Ehrhardt wrote:
> Since a3ab6d42 "apparmor: convert libvirtd profile to a named profile"
> the detection of the subelement for qemu_bridge_helper is wrong.
> 
> In combination with the older 123cc3e1 "apparmor: allow
> /usr/lib/qemu/qemu-bridge-helper" it now detects qemu-bridge-helper no
> more with its path, but instead as a proper subelement of the named profile
> like: label=libvirtd//qemu_bridge_helper
> 
> In the same fashion the reverse rule in the qemu_bridge_helper
> sub-profile still uses the path and not the named profile label.
> 
> Triggering denies like:
> apparmor="DENIED" operation="file_inherit"
>    profile="libvirtd//qemu_bridge_helper" pid=5629 comm="qemu-bridge-hel"
>    family="unix" sock_type="stream" protocol=0 requested_mask="send receive"
>    denied_mask="send receive" addr=none peer_addr=none peer="libvirtd"
> 
> This patch fixes the unix socket rules for the communication between
> libvirtd and qemu-bridge-helper to match that.
> 
> Fixes: a3ab6d42d825499af44b8f19f9299e150d9687bc
> Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1655111
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>   src/security/apparmor/usr.sbin.libvirtd | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

Re: [PATCH] apparmor: fix qemu_bridge_helper for named profile
Posted by Christian Ehrhardt 4 years, 1 month ago
On Thu, Jan 30, 2020 at 8:29 AM Michal Privoznik <mprivozn@redhat.com>
wrote:

> On 1/30/20 8:21 AM, Christian Ehrhardt wrote:
> > Since a3ab6d42 "apparmor: convert libvirtd profile to a named profile"
> > the detection of the subelement for qemu_bridge_helper is wrong.
> >
> > In combination with the older 123cc3e1 "apparmor: allow
> > /usr/lib/qemu/qemu-bridge-helper" it now detects qemu-bridge-helper no
> > more with its path, but instead as a proper subelement of the named
> profile
> > like: label=libvirtd//qemu_bridge_helper
> >
> > In the same fashion the reverse rule in the qemu_bridge_helper
> > sub-profile still uses the path and not the named profile label.
> >
> > Triggering denies like:
> > apparmor="DENIED" operation="file_inherit"
> >    profile="libvirtd//qemu_bridge_helper" pid=5629 comm="qemu-bridge-hel"
> >    family="unix" sock_type="stream" protocol=0 requested_mask="send
> receive"
> >    denied_mask="send receive" addr=none peer_addr=none peer="libvirtd"
> >
> > This patch fixes the unix socket rules for the communication between
> > libvirtd and qemu-bridge-helper to match that.
> >
> > Fixes: a3ab6d42d825499af44b8f19f9299e150d9687bc
> > Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1655111
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > ---
> >   src/security/apparmor/usr.sbin.libvirtd | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
>
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
>

Thanks for the review!

Nothing else came up in discussions here and in local tests it seems to
work fine as well.
Pushed to the repository


> Michal
>
>

-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd