1. Patchew
  2. Libvirt
  3. View series

[PATCH] virt-aa-helper: delete dynamic files

Alessandro posted 1 patch 1 year ago
Failed in applying to current master (apply log)
There is a newer version of this series
src/security/virt-aa-helper.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH] virt-aa-helper: delete dynamic files
Posted by Alessandro 1 year ago 
We attempted multiple ways to clean up dynamic files; however, we must
preserve user overrides, which requires keeping the file
/etc/apparmor.d/libvirt/libvirt-uuid

This commit proposes to move user overrides into
/etc/apparmor.d/libvirt/libvirt-uuid.local and include it, if present,
unconditionally. When we stop the domain, we remove libvirt.uuid and
libvirt-uuid.files, whereas we preserve libvirt-uuid.local if present.

Applying the patch, it produces the following:

root@virt-hv-lab002:/etc/apparmor.d/libvirt# ls -1 libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033*
libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033
libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.files
root@virt-hv-lab002:/etc/apparmor.d/libvirt# cat libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033

profile libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033 flags=(attach_disconnected) {
  #include <abstractions/libvirt-qemu>
  #include if exists <libvirt/libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.files>
  #include if exists <libvirt/libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.local>

}
root@virt-hv-lab002:/etc/apparmor.d/libvirt# cat libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.files
  "/var/log/libvirt/**/testing-9a4be628.log" w,
  "/var/lib/libvirt/qemu/domain-testing-9a4be628/monitor.sock" rw,
  "/var/lib/libvirt/qemu/domain-4-testing-9a4be628/*" rw,
  "/var/run/libvirt/**/testing-9a4be628.pid" rwk,
  "/var/run/libvirt/**/*.tunnelmigrate.dest.testing-9a4be628" rw,
  "/var/lib/libvirt/images/testing-9a4be628.qcow2" rwk,
  "/var/lib/libvirt/images/noble-server-cloudimg-amd64.img" rk,
  # don't audit writes to readonly files
  deny "/var/lib/libvirt/images/noble-server-cloudimg-amd64.img" w,
  "/var/lib/libvirt/images/testing-9a4be628-ds.qcow2" rwk,
  "/usr/share/OVMF/OVMF_CODE_4M.fd" rk,
  # don't audit writes to readonly files
  deny "/usr/share/OVMF/OVMF_CODE_4M.fd" w,
  "/var/lib/libvirt/qemu/nvram/testing-9a4be628_VARS.fd" rwk,
  "/dev/vhost-net" rw,
  "/var/lib/libvirt/qemu/domain-4-testing-9a4be628/{,**}" rwk,
  "/run/libvirt/qemu/channel/4-testing-9a4be628/{,**}" rwk,
  "/var/lib/libvirt/qemu/domain-4-testing-9a4be628/master-key.aes" rwk,
  "/dev/net/tun" rwk,
  "/dev/userfaultfd" rwk,

Fixes: https://gitlab.com/libvirt/libvirt/-/issues/451

Signed-off-by: Alessandro <alessandro@0x65c.net>
---
 src/security/virt-aa-helper.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 034c042..8f044a1 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1495,8 +1495,10 @@ main(int argc, char **argv)
         rc = parserLoad(ctl->uuid);
     } else if (ctl->cmd == 'R' || ctl->cmd == 'D') {
         rc = parserRemove(ctl->uuid);
-        if (ctl->cmd == 'D')
+        if (ctl->cmd == 'D'){
             unlink(include_file);
+            unlink(profile);
+        }
     } else if (ctl->cmd == 'c' || ctl->cmd == 'r') {
         g_autofree char *included_files = NULL;
         g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
@@ -1561,12 +1563,15 @@ main(int argc, char **argv)
         /* create the profile from TEMPLATE */
         if (ctl->cmd == 'c' || purged) {
             g_autofree char *tmp = NULL;
+            g_autofree char *tmp_local = NULL:
 #if defined(WITH_APPARMOR_3)
             const char *ifexists = "if exists ";
 #else
             const char *ifexists = "";
 #endif
             tmp = g_strdup_printf("  #include %s<libvirt/%s.files>\n", ifexists, ctl->uuid);
+            tmp_local = g_strdup_printf("  #include %s<libvirt/%s.local>\n", ifexists, ctl->uuid);
+            tmp = g_strconcat(tmp,tmp_local, NULL);
 
             if (ctl->dryrun) {
                 vah_info(profile);
-- 
2.49.0
Re: [PATCH] virt-aa-helper: delete dynamic files
Posted by Daniel P. Berrangé via Devel 1 year ago 
On Mon, Mar 31, 2025 at 03:01:18PM +0200, Alessandro wrote:
> We attempted multiple ways to clean up dynamic files; however, we must
> preserve user overrides, which requires keeping the file
> /etc/apparmor.d/libvirt/libvirt-uuid
> 
> This commit proposes to move user overrides into
> /etc/apparmor.d/libvirt/libvirt-uuid.local and include it, if present,
> unconditionally. When we stop the domain, we remove libvirt.uuid and
> libvirt-uuid.files, whereas we preserve libvirt-uuid.local if present.
> 
> Applying the patch, it produces the following:
> 
> root@virt-hv-lab002:/etc/apparmor.d/libvirt# ls -1 libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033*
> libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033
> libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.files
> root@virt-hv-lab002:/etc/apparmor.d/libvirt# cat libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033
> 
> profile libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033 flags=(attach_disconnected) {
>   #include <abstractions/libvirt-qemu>
>   #include if exists <libvirt/libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.files>
>   #include if exists <libvirt/libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.local>
> 
> }
> root@virt-hv-lab002:/etc/apparmor.d/libvirt# cat libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.files
>   "/var/log/libvirt/**/testing-9a4be628.log" w,
>   "/var/lib/libvirt/qemu/domain-testing-9a4be628/monitor.sock" rw,
>   "/var/lib/libvirt/qemu/domain-4-testing-9a4be628/*" rw,
>   "/var/run/libvirt/**/testing-9a4be628.pid" rwk,
>   "/var/run/libvirt/**/*.tunnelmigrate.dest.testing-9a4be628" rw,
>   "/var/lib/libvirt/images/testing-9a4be628.qcow2" rwk,
>   "/var/lib/libvirt/images/noble-server-cloudimg-amd64.img" rk,
>   # don't audit writes to readonly files
>   deny "/var/lib/libvirt/images/noble-server-cloudimg-amd64.img" w,
>   "/var/lib/libvirt/images/testing-9a4be628-ds.qcow2" rwk,
>   "/usr/share/OVMF/OVMF_CODE_4M.fd" rk,
>   # don't audit writes to readonly files
>   deny "/usr/share/OVMF/OVMF_CODE_4M.fd" w,
>   "/var/lib/libvirt/qemu/nvram/testing-9a4be628_VARS.fd" rwk,
>   "/dev/vhost-net" rw,
>   "/var/lib/libvirt/qemu/domain-4-testing-9a4be628/{,**}" rwk,
>   "/run/libvirt/qemu/channel/4-testing-9a4be628/{,**}" rwk,
>   "/var/lib/libvirt/qemu/domain-4-testing-9a4be628/master-key.aes" rwk,
>   "/dev/net/tun" rwk,
>   "/dev/userfaultfd" rwk,
> 
> Fixes: https://gitlab.com/libvirt/libvirt/-/issues/451
> 
> Signed-off-by: Alessandro <alessandro@0x65c.net>
> ---
>  src/security/virt-aa-helper.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 034c042..8f044a1 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1495,8 +1495,10 @@ main(int argc, char **argv)
>          rc = parserLoad(ctl->uuid);
>      } else if (ctl->cmd == 'R' || ctl->cmd == 'D') {
>          rc = parserRemove(ctl->uuid);
> -        if (ctl->cmd == 'D')
> +        if (ctl->cmd == 'D'){
>              unlink(include_file);
> +            unlink(profile);
> +        }
>      } else if (ctl->cmd == 'c' || ctl->cmd == 'r') {
>          g_autofree char *included_files = NULL;
>          g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> @@ -1561,12 +1563,15 @@ main(int argc, char **argv)
>          /* create the profile from TEMPLATE */
>          if (ctl->cmd == 'c' || purged) {
>              g_autofree char *tmp = NULL;
> +            g_autofree char *tmp_local = NULL:
>  #if defined(WITH_APPARMOR_3)
>              const char *ifexists = "if exists ";
>  #else
>              const char *ifexists = "";
>  #endif
>              tmp = g_strdup_printf("  #include %s<libvirt/%s.files>\n", ifexists, ctl->uuid);
> +            tmp_local = g_strdup_printf("  #include %s<libvirt/%s.local>\n", ifexists, ctl->uuid);
> +            tmp = g_strconcat(tmp,tmp_local, NULL);

This leaks memory, as it overwrites the original 'tmp' pointer
without releasing it.

>  
>              if (ctl->dryrun) {
>                  vah_info(profile);
> -- 
> 2.49.0
> 

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 :|
Re: [PATCH] virt-aa-helper: delete dynamic files
Posted by Alessandro 1 year ago 
On Mon, 31 Mar 2025 at 18:00, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Mar 31, 2025 at 03:01:18PM +0200, Alessandro wrote:
> > We attempted multiple ways to clean up dynamic files; however, we must
> > preserve user overrides, which requires keeping the file
> > /etc/apparmor.d/libvirt/libvirt-uuid
> >
> > This commit proposes to move user overrides into
> > /etc/apparmor.d/libvirt/libvirt-uuid.local and include it, if present,
> > unconditionally. When we stop the domain, we remove libvirt.uuid and
> > libvirt-uuid.files, whereas we preserve libvirt-uuid.local if present.
> >
> > Applying the patch, it produces the following:
> >
> > root@virt-hv-lab002:/etc/apparmor.d/libvirt# ls -1 libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033*
> > libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033
> > libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.files
> > root@virt-hv-lab002:/etc/apparmor.d/libvirt# cat libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033
> >
> > profile libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033 flags=(attach_disconnected) {
> >   #include <abstractions/libvirt-qemu>
> >   #include if exists <libvirt/libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.files>
> >   #include if exists <libvirt/libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.local>
> >
> > }
> > root@virt-hv-lab002:/etc/apparmor.d/libvirt# cat libvirt-e7424556-ffc1-4f6e-bafa-84e66c4dc033.files
> >   "/var/log/libvirt/**/testing-9a4be628.log" w,
> >   "/var/lib/libvirt/qemu/domain-testing-9a4be628/monitor.sock" rw,
> >   "/var/lib/libvirt/qemu/domain-4-testing-9a4be628/*" rw,
> >   "/var/run/libvirt/**/testing-9a4be628.pid" rwk,
> >   "/var/run/libvirt/**/*.tunnelmigrate.dest.testing-9a4be628" rw,
> >   "/var/lib/libvirt/images/testing-9a4be628.qcow2" rwk,
> >   "/var/lib/libvirt/images/noble-server-cloudimg-amd64.img" rk,
> >   # don't audit writes to readonly files
> >   deny "/var/lib/libvirt/images/noble-server-cloudimg-amd64.img" w,
> >   "/var/lib/libvirt/images/testing-9a4be628-ds.qcow2" rwk,
> >   "/usr/share/OVMF/OVMF_CODE_4M.fd" rk,
> >   # don't audit writes to readonly files
> >   deny "/usr/share/OVMF/OVMF_CODE_4M.fd" w,
> >   "/var/lib/libvirt/qemu/nvram/testing-9a4be628_VARS.fd" rwk,
> >   "/dev/vhost-net" rw,
> >   "/var/lib/libvirt/qemu/domain-4-testing-9a4be628/{,**}" rwk,
> >   "/run/libvirt/qemu/channel/4-testing-9a4be628/{,**}" rwk,
> >   "/var/lib/libvirt/qemu/domain-4-testing-9a4be628/master-key.aes" rwk,
> >   "/dev/net/tun" rwk,
> >   "/dev/userfaultfd" rwk,
> >
> > Fixes: https://gitlab.com/libvirt/libvirt/-/issues/451
> >
> > Signed-off-by: Alessandro <alessandro@0x65c.net>
> > ---
> >  src/security/virt-aa-helper.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> > index 034c042..8f044a1 100644
> > --- a/src/security/virt-aa-helper.c
> > +++ b/src/security/virt-aa-helper.c
> > @@ -1495,8 +1495,10 @@ main(int argc, char **argv)
> >          rc = parserLoad(ctl->uuid);
> >      } else if (ctl->cmd == 'R' || ctl->cmd == 'D') {
> >          rc = parserRemove(ctl->uuid);
> > -        if (ctl->cmd == 'D')
> > +        if (ctl->cmd == 'D'){
> >              unlink(include_file);
> > +            unlink(profile);
> > +        }
> >      } else if (ctl->cmd == 'c' || ctl->cmd == 'r') {
> >          g_autofree char *included_files = NULL;
> >          g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> > @@ -1561,12 +1563,15 @@ main(int argc, char **argv)
> >          /* create the profile from TEMPLATE */
> >          if (ctl->cmd == 'c' || purged) {
> >              g_autofree char *tmp = NULL;
> > +            g_autofree char *tmp_local = NULL:
> >  #if defined(WITH_APPARMOR_3)
> >              const char *ifexists = "if exists ";
> >  #else
> >              const char *ifexists = "";
> >  #endif
> >              tmp = g_strdup_printf("  #include %s<libvirt/%s.files>\n", ifexists, ctl->uuid);
> > +            tmp_local = g_strdup_printf("  #include %s<libvirt/%s.local>\n", ifexists, ctl->uuid);
> > +            tmp = g_strconcat(tmp,tmp_local, NULL);
>
> This leaks memory, as it overwrites the original 'tmp' pointer
> without releasing it.

Thank you for the review.
I sent out another patch that should, hopefully, fix this.

>
> >
> >              if (ctl->dryrun) {
> >                  vah_info(profile);
> > --
> > 2.49.0
> >
>
> 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 :|

Thank you,
Best regards
A.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.