[libvirt PATCH] apparmor: Enable passt support
passt provides an AppArmor abstraction that covers all the inner details of its operation, so we can simply import that and add the libvirt-specific parts on top: namely, passt needs to be able to create a socket and pid file, while the libvirt daemon needs to be able to kill passt. Signed-off-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> --- src/security/apparmor/libvirt-qemu | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 9af1333b22..44056b5f14 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -185,6 +185,21 @@ /usr/{lib,lib64}/libswtpm_libtpms.so mr, /usr/lib/@{multiarch}/libswtpm_libtpms.so mr, + # support for passt network back-end + /usr/bin/passt Cx -> passt, + + profile passt { + /usr/bin/passt r, + + signal (receive) set=("term") peer=/usr/sbin/libvirtd, + signal (receive) set=("term") peer=libvirtd, + signal (receive) set=("term") peer=virtqemud, + + owner /{,var/}run/libvirt/qemu/passt/* rw, + + include if exists <abstractions/passt> + } + # for save and resume /{usr/,}bin/dash rmix, /{usr/,}bin/dd rmix, -- 2.39.2
On Tue, Mar 07, 2023 at 08:02:37PM +0100, Andrea Bolognani wrote:
passt provides an AppArmor abstraction that covers all the inner details of its operation, so we can simply import that and add the libvirt-specific parts on top: namely, passt needs to be able to create a socket and pid file, while the libvirt daemon needs to be able to kill passt.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> --- src/security/apparmor/libvirt-qemu | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 9af1333b22..44056b5f14 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -185,6 +185,21 @@ /usr/{lib,lib64}/libswtpm_libtpms.so mr, /usr/lib/@{multiarch}/libswtpm_libtpms.so mr,
+ # support for passt network back-end + /usr/bin/passt Cx -> passt, + + profile passt { + /usr/bin/passt r, + + signal (receive) set=("term") peer=/usr/sbin/libvirtd, + signal (receive) set=("term") peer=libvirtd,
What's the rationale for having both qualified & unqualified here, but not below ?
+ signal (receive) set=("term") peer=virtqemud, + + owner /{,var/}run/libvirt/qemu/passt/* rw, + + include if exists <abstractions/passt> + }
With 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 :|
On Tue, Mar 07, 2023 at 07:04:25PM +0000, Daniel P. Berrangé wrote:
On Tue, Mar 07, 2023 at 08:02:37PM +0100, Andrea Bolognani wrote:
+ # support for passt network back-end + /usr/bin/passt Cx -> passt, + + profile passt { + /usr/bin/passt r, + + signal (receive) set=("term") peer=/usr/sbin/libvirtd, + signal (receive) set=("term") peer=libvirtd,
What's the rationale for having both qualified & unqualified here, but not below ?
Cargo cult. That's what the top-level profile does, so I figured it would be good enough for the subprofile too. I've seen stuff like peer=(label=libvirtd) as well, but I haven't investigated the various notations and how exactly they differ. There's plenty of room for improvement in the AppArmor profile in general, but that's a task for another day :) -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Mar 07, 2023 at 01:28:41PM -0800, Andrea Bolognani wrote:
On Tue, Mar 07, 2023 at 07:04:25PM +0000, Daniel P. Berrangé wrote:
On Tue, Mar 07, 2023 at 08:02:37PM +0100, Andrea Bolognani wrote:
+ # support for passt network back-end + /usr/bin/passt Cx -> passt, + + profile passt { + /usr/bin/passt r, + + signal (receive) set=("term") peer=/usr/sbin/libvirtd, + signal (receive) set=("term") peer=libvirtd,
What's the rationale for having both qualified & unqualified here, but not below ?
Cargo cult. That's what the top-level profile does, so I figured it would be good enough for the subprofile too.
I've seen stuff like peer=(label=libvirtd) as well, but I haven't investigated the various notations and how exactly they differ.
It looks like using the label= version is only needed when additional information is passed, for example in unix (send, receive) type=stream addr=none peer=(label=unconfined addr=none), Each profile has its own name attached as a label, so peer=libvirtd and peer=(label=libvirtd) are effectively equivalent. Similarly, the binary running under a profile also becomes a label, making peer=libvirtd and peer=/usr/sbin/libvirtd also equivalent, which means that having both in the profile is redundant.
There's plenty of room for improvement in the AppArmor profile in general, but that's a task for another day :)
Based on the above, I'm working on a patch that cleans up the situation by sticking with the simpler form. Plus some other stuff. I don't want to make such a cleanup patch a requirement for this functionally relevant one though, especially in the not entirely remote chance that I have completely misunderstood how this stuff work O:-) So I'd prefer to keep the cargo cult line for now, and clean everything up at once later. -- Andrea Bolognani / Red Hat / Virtualization
On Wed, Mar 08, 2023 at 09:23:19AM -0800, Andrea Bolognani wrote:
On Tue, Mar 07, 2023 at 01:28:41PM -0800, Andrea Bolognani wrote:
On Tue, Mar 07, 2023 at 07:04:25PM +0000, Daniel P. Berrangé wrote:
On Tue, Mar 07, 2023 at 08:02:37PM +0100, Andrea Bolognani wrote:
+ # support for passt network back-end + /usr/bin/passt Cx -> passt, + + profile passt { + /usr/bin/passt r, + + signal (receive) set=("term") peer=/usr/sbin/libvirtd, + signal (receive) set=("term") peer=libvirtd,
What's the rationale for having both qualified & unqualified here, but not below ?
Cargo cult. That's what the top-level profile does, so I figured it would be good enough for the subprofile too.
I've seen stuff like peer=(label=libvirtd) as well, but I haven't investigated the various notations and how exactly they differ.
It looks like using the label= version is only needed when additional information is passed, for example in
unix (send, receive) type=stream addr=none peer=(label=unconfined addr=none),
Each profile has its own name attached as a label, so
peer=libvirtd
and
peer=(label=libvirtd)
are effectively equivalent. Similarly, the binary running under a profile also becomes a label, making
peer=libvirtd
and
peer=/usr/sbin/libvirtd
also equivalent, which means that having both in the profile is redundant.
There's plenty of room for improvement in the AppArmor profile in general, but that's a task for another day :)
Based on the above, I'm working on a patch that cleans up the situation by sticking with the simpler form. Plus some other stuff.
I don't want to make such a cleanup patch a requirement for this functionally relevant one though, especially in the not entirely remote chance that I have completely misunderstood how this stuff work O:-)
So I'd prefer to keep the cargo cult line for now, and clean everything up at once later.
Sure, if its already inconsistent, then this makes it no worse... Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With 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 :|
participants (2)
-
Andrea Bolognani -
Daniel P. Berrangé