There is a common misconception when writing AppArmor policy that
[0-9]* applies * to the [0-9] class, but that's not the case. For this
example, [0-9]* matches a single digit followed by any number of
characters except for /
Create a UUID variable that uses the following format 8-4-4-4-12.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
---
src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 5 ++++-
src/security/apparmor/usr.sbin.libvirtd.in | 7 +++++--
src/security/apparmor/usr.sbin.virtqemud.in | 6 ++++--
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
index 44645c6989..90a8b7072c 100644
--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
+++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
@@ -1,5 +1,8 @@
#include <tunables/global>
+@{hextet}=[0-9a-f][0-9a-f][0-9a-f][0-9a-f]
+@{UUID}=@{hextet}@{hextet}-@{hextet}-@{hextet}-@{hextet}-@{hextet}@{hextet}@{hextet}
+
profile virt-aa-helper @libexecdir@/virt-aa-helper {
#include <abstractions/base>
#include <abstractions/openssl>
@@ -44,7 +47,7 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper {
/{usr/,}{s,}bin/apparmor_parser Ux,
@sysconfdir@/apparmor.d/libvirt/* r,
- @sysconfdir@/apparmor.d/libvirt/libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]* rw,
+ @sysconfdir@/apparmor.d/libvirt/libvirt-@{UUID}* rw,
# for backingstore -- allow access to non-hidden files in @{HOME} as well
# as storage pools
diff --git a/src/security/apparmor/usr.sbin.libvirtd.in b/src/security/apparmor/usr.sbin.libvirtd.in
index 70e586895f..3659ddc219 100644
--- a/src/security/apparmor/usr.sbin.libvirtd.in
+++ b/src/security/apparmor/usr.sbin.libvirtd.in
@@ -1,4 +1,7 @@
#include <tunables/global>
+
+@{hextet}=[0-9a-f][0-9a-f][0-9a-f][0-9a-f]
+@{UUID}=@{hextet}@{hextet}-@{hextet}-@{hextet}-@{hextet}-@{hextet}@{hextet}@{hextet}
@{LIBVIRT}="libvirt"
profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) {
@@ -72,7 +75,7 @@ profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) {
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]*),
+ unix (send, receive) type=stream addr=none peer=(label=libvirt-@{UUID}),
# unconfined also required if guests run without security module
unix (send, receive) type=stream addr=none peer=(label=unconfined),
@@ -115,7 +118,7 @@ profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) {
/etc/xen/scripts/** rmix,
# allow changing to our UUID-based named profiles
- change_profile -> @{LIBVIRT}-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*,
+ change_profile -> @{LIBVIRT}-@{UUID},
/usr/{lib,lib64,lib/qemu,libexec,libexec/qemu}/qemu-bridge-helper Cx -> qemu_bridge_helper,
# child profile for bridge helper process
diff --git a/src/security/apparmor/usr.sbin.virtqemud.in b/src/security/apparmor/usr.sbin.virtqemud.in
index 42fa4813da..86b23465b6 100644
--- a/src/security/apparmor/usr.sbin.virtqemud.in
+++ b/src/security/apparmor/usr.sbin.virtqemud.in
@@ -1,5 +1,7 @@
#include <tunables/global>
@{LIBVIRT}="libvirt"
+@{hextet}=[0-9a-f][0-9a-f][0-9a-f][0-9a-f]
+@{UUID}=@{hextet}@{hextet}-@{hextet}-@{hextet}-@{hextet}-@{hextet}@{hextet}@{hextet}
profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) {
#include <abstractions/base>
@@ -71,7 +73,7 @@ profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) {
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]*),
+ unix (send, receive) type=stream addr=none peer=(label=libvirt-@{UUID}),
# unconfined also required if guests run without security module
unix (send, receive) type=stream addr=none peer=(label=unconfined),
@@ -109,7 +111,7 @@ profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) {
/etc/libvirt/hooks/** rmix,
# allow changing to our UUID-based named profiles
- change_profile -> @{LIBVIRT}-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*,
+ change_profile -> @{LIBVIRT}-@{UUID},
/usr/{lib,lib64,lib/qemu,libexec,libexec/qemu}/qemu-bridge-helper Cx -> qemu_bridge_helper,
# child profile for bridge helper process
--
2.43.0
On 1/7/25 08:23, Georgia Garcia wrote: > There is a common misconception when writing AppArmor policy that > [0-9]* applies * to the [0-9] class, but that's not the case. For this > example, [0-9]* matches a single digit followed by any number of > characters except for / > > Create a UUID variable that uses the following format 8-4-4-4-12. > > Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com> > --- > src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 5 ++++- > src/security/apparmor/usr.sbin.libvirtd.in | 7 +++++-- > src/security/apparmor/usr.sbin.virtqemud.in | 6 ++++-- > 3 files changed, 13 insertions(+), 5 deletions(-) This patch seems fine to me. Did you notice the issue by code inspection, or does it fix an observed error? If the latter, we should mention it in the commit message. Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim > > diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in > index 44645c6989..90a8b7072c 100644 > --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in > +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in > @@ -1,5 +1,8 @@ > #include <tunables/global> > > +@{hextet}=[0-9a-f][0-9a-f][0-9a-f][0-9a-f] > +@{UUID}=@{hextet}@{hextet}-@{hextet}-@{hextet}-@{hextet}-@{hextet}@{hextet}@{hextet} > + > profile virt-aa-helper @libexecdir@/virt-aa-helper { > #include <abstractions/base> > #include <abstractions/openssl> > @@ -44,7 +47,7 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper { > /{usr/,}{s,}bin/apparmor_parser Ux, > > @sysconfdir@/apparmor.d/libvirt/* r, > - @sysconfdir@/apparmor.d/libvirt/libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]* rw, > + @sysconfdir@/apparmor.d/libvirt/libvirt-@{UUID}* rw, > > # for backingstore -- allow access to non-hidden files in @{HOME} as well > # as storage pools > diff --git a/src/security/apparmor/usr.sbin.libvirtd.in b/src/security/apparmor/usr.sbin.libvirtd.in > index 70e586895f..3659ddc219 100644 > --- a/src/security/apparmor/usr.sbin.libvirtd.in > +++ b/src/security/apparmor/usr.sbin.libvirtd.in > @@ -1,4 +1,7 @@ > #include <tunables/global> > + > +@{hextet}=[0-9a-f][0-9a-f][0-9a-f][0-9a-f] > +@{UUID}=@{hextet}@{hextet}-@{hextet}-@{hextet}-@{hextet}-@{hextet}@{hextet}@{hextet} > @{LIBVIRT}="libvirt" > > profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) { > @@ -72,7 +75,7 @@ profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) { > 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]*), > + unix (send, receive) type=stream addr=none peer=(label=libvirt-@{UUID}), > # unconfined also required if guests run without security module > unix (send, receive) type=stream addr=none peer=(label=unconfined), > > @@ -115,7 +118,7 @@ profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) { > /etc/xen/scripts/** rmix, > > # allow changing to our UUID-based named profiles > - change_profile -> @{LIBVIRT}-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*, > + change_profile -> @{LIBVIRT}-@{UUID}, > > /usr/{lib,lib64,lib/qemu,libexec,libexec/qemu}/qemu-bridge-helper Cx -> qemu_bridge_helper, > # child profile for bridge helper process > diff --git a/src/security/apparmor/usr.sbin.virtqemud.in b/src/security/apparmor/usr.sbin.virtqemud.in > index 42fa4813da..86b23465b6 100644 > --- a/src/security/apparmor/usr.sbin.virtqemud.in > +++ b/src/security/apparmor/usr.sbin.virtqemud.in > @@ -1,5 +1,7 @@ > #include <tunables/global> > @{LIBVIRT}="libvirt" > +@{hextet}=[0-9a-f][0-9a-f][0-9a-f][0-9a-f] > +@{UUID}=@{hextet}@{hextet}-@{hextet}-@{hextet}-@{hextet}-@{hextet}@{hextet}@{hextet} > > profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) { > #include <abstractions/base> > @@ -71,7 +73,7 @@ profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) { > 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]*), > + unix (send, receive) type=stream addr=none peer=(label=libvirt-@{UUID}), > # unconfined also required if guests run without security module > unix (send, receive) type=stream addr=none peer=(label=unconfined), > > @@ -109,7 +111,7 @@ profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) { > /etc/libvirt/hooks/** rmix, > > # allow changing to our UUID-based named profiles > - change_profile -> @{LIBVIRT}-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*, > + change_profile -> @{LIBVIRT}-@{UUID}, > > /usr/{lib,lib64,lib/qemu,libexec,libexec/qemu}/qemu-bridge-helper Cx -> qemu_bridge_helper, > # child profile for bridge helper process
On Tue, 2025-01-07 at 17:04 -0700, Jim Fehlig wrote: > On 1/7/25 08:23, Georgia Garcia wrote: > > There is a common misconception when writing AppArmor policy that > > [0-9]* applies * to the [0-9] class, but that's not the case. For this > > example, [0-9]* matches a single digit followed by any number of > > characters except for / > > > > Create a UUID variable that uses the following format 8-4-4-4-12. > > > > Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com> > > --- > > src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 5 ++++- > > src/security/apparmor/usr.sbin.libvirtd.in | 7 +++++-- > > src/security/apparmor/usr.sbin.virtqemud.in | 6 ++++-- > > 3 files changed, 13 insertions(+), 5 deletions(-) > > This patch seems fine to me. Did you notice the issue by code inspection, or > does it fix an observed error? If the latter, we should mention it in the commit > message. > It was indeed by code inspection. Since the rules were broader than needed, we wouldn't see errors related to this in normal libvirt use. I'm just restricting it to what was the intended behavior. Thank you, Georgia > Reviewed-by: Jim Fehlig <jfehlig@suse.com> > > Regards, > Jim
On 1/8/25 04:45, Georgia Garcia wrote: > On Tue, 2025-01-07 at 17:04 -0700, Jim Fehlig wrote: >> On 1/7/25 08:23, Georgia Garcia wrote: >>> There is a common misconception when writing AppArmor policy that >>> [0-9]* applies * to the [0-9] class, but that's not the case. For this >>> example, [0-9]* matches a single digit followed by any number of >>> characters except for / >>> >>> Create a UUID variable that uses the following format 8-4-4-4-12. >>> >>> Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com> >>> --- >>> src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 5 ++++- >>> src/security/apparmor/usr.sbin.libvirtd.in | 7 +++++-- >>> src/security/apparmor/usr.sbin.virtqemud.in | 6 ++++-- >>> 3 files changed, 13 insertions(+), 5 deletions(-) >> >> This patch seems fine to me. Did you notice the issue by code inspection, or >> does it fix an observed error? If the latter, we should mention it in the commit >> message. >> > > It was indeed by code inspection. Since the rules were broader than > needed, we wouldn't see errors related to this in normal libvirt use. > I'm just restricting it to what was the intended behavior. Thanks for confirming. I've pushed patches 1-3 since they are independent improvements from the issue you're trying to fix in patch 4. Regards, Jim
© 2016 - 2025 Red Hat, Inc.