[PATCH] virt-aa-helper: use 'include if exists' on .files

Georgia Garcia posted 1 patch 4 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20240604173456.3533611-1-georgia.garcia@canonical.com
src/security/virt-aa-helper.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH] virt-aa-helper: use 'include if exists' on .files
Posted by Georgia Garcia 4 months, 2 weeks ago
Change the 'include' in the AppArmor policy to use 'include if exists'
when including <uuid>.files. Note that 'if exists' is only available
after AppArmor 3.0, therefore a #ifdef check must be added.

When the <uuid>.files is not present, there are some failures in the
AppArmor tools like the following, since they expect the file to exist
when using 'include':

ERROR: Include file /etc/apparmor.d/libvirt/libvirt-8534a409-a460-4fab-a2dd-0e1dce4ff273.files not found

Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
---
 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 0374581f07..402cbd9602 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1564,7 +1564,12 @@ main(int argc, char **argv)
         /* create the profile from TEMPLATE */
         if (ctl->cmd == 'c' || purged) {
             char *tmp = NULL;
-            tmp = g_strdup_printf("  #include <libvirt/%s.files>\n", ctl->uuid);
+#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);
 
             if (ctl->dryrun) {
                 vah_info(profile);
-- 
2.34.1
Re: [PATCH] virt-aa-helper: use 'include if exists' on .files
Posted by Michal Prívozník 4 months, 1 week ago
On 6/4/24 19:34, Georgia Garcia wrote:
> Change the 'include' in the AppArmor policy to use 'include if exists'
> when including <uuid>.files. Note that 'if exists' is only available
> after AppArmor 3.0, therefore a #ifdef check must be added.
> 
> When the <uuid>.files is not present, there are some failures in the
> AppArmor tools like the following, since they expect the file to exist
> when using 'include':
> 
> ERROR: Include file /etc/apparmor.d/libvirt/libvirt-8534a409-a460-4fab-a2dd-0e1dce4ff273.files not found

When can this ever happen? I thought libvirt creates this file for each
domain running.

Michal
Re: [PATCH] virt-aa-helper: use 'include if exists' on .files
Posted by Georgia Garcia 4 months, 1 week ago
On Mon, 2024-06-10 at 15:03 +0200, Michal Prívozník wrote:
> On 6/4/24 19:34, Georgia Garcia wrote:
> > Change the 'include' in the AppArmor policy to use 'include if exists'
> > when including <uuid>.files. Note that 'if exists' is only available
> > after AppArmor 3.0, therefore a #ifdef check must be added.
> > 
> > When the <uuid>.files is not present, there are some failures in the
> > AppArmor tools like the following, since they expect the file to exist
> > when using 'include':
> > 
> > ERROR: Include file /etc/apparmor.d/libvirt/libvirt-8534a409-a460-4fab-a2dd-0e1dce4ff273.files not found
> 
> When can this ever happen? I thought libvirt creates this file for each
> domain running.

The file does not exist when the domain is not running, so if you're
running an apparmor tool like aa-genprof, they scan all profiles under
/etc/apparmor.d/ and they expect a valid state for the policies - which
these don't have because they include a file that does not exist unless
the domain is running.

> 
> Michal
> 

Thanks,
Georgia
Re: [PATCH] virt-aa-helper: use 'include if exists' on .files
Posted by Michal Prívozník 4 months, 1 week ago
On 6/10/24 16:29, Georgia Garcia wrote:
> On Mon, 2024-06-10 at 15:03 +0200, Michal Prívozník wrote:
>> On 6/4/24 19:34, Georgia Garcia wrote:
>>> Change the 'include' in the AppArmor policy to use 'include if exists'
>>> when including <uuid>.files. Note that 'if exists' is only available
>>> after AppArmor 3.0, therefore a #ifdef check must be added.
>>>
>>> When the <uuid>.files is not present, there are some failures in the
>>> AppArmor tools like the following, since they expect the file to exist
>>> when using 'include':
>>>
>>> ERROR: Include file /etc/apparmor.d/libvirt/libvirt-8534a409-a460-4fab-a2dd-0e1dce4ff273.files not found
>>
>> When can this ever happen? I thought libvirt creates this file for each
>> domain running.
> 
> The file does not exist when the domain is not running, so if you're
> running an apparmor tool like aa-genprof, they scan all profiles under
> /etc/apparmor.d/ and they expect a valid state for the policies - which
> these don't have because they include a file that does not exist unless
> the domain is running.


Fair enough.

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

and merged. Congratulations on your first libvirt contribution!

Michal