From: Michal Privoznik <mprivozn@redhat.com> Here's the problem. When starting a QEMU domain, the process is split into several phases. A simplified process looks like this: 1) qemuProcessPrepareDomain() 1a) qemuSecurityGenLabel() 1b) generate run time paths 1c) qemuSecurityManagerLoadProfile() 2) qemuProcessPrepareHost() 2a) qemuSecurityDomainSetPathLabel() /* transitively */ 3) qemuProcessLaunch() 3a) qemuSecuritySetAllLabel() NB, step 2a) also contains helper processes used to set up host, that we want to run in the security context of the domain (e.g. swtpm_setup). This works well for SELinux and DAC because their APIs basically match 1:1 to ours. But it's not that simple with AppArmor. It doesn't contain any profile upfront, so one is generated in step 1a) (among with seclabel). But at that point, the domain definition has no run time info. This can then lead to some paths being left out (for instance, disks which source wasn't translated from storage pool/vol spec into a path). After previous commits, there's this new step 1c) which can actually load the profile. Therefore, move profile loading into .domainLoadProfile callback. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/135 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_apparmor.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 68ac39611f..98fad9034d 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -387,23 +387,36 @@ AppArmorGenSecurityLabel(virSecurityManager *mgr G_GNUC_UNUSED, if (!secdef->model) secdef->model = g_strdup(SECURITY_APPARMOR_NAME); - /* Now that we have a label, load the profile into the kernel. */ + return 0; +} + + +static int +AppArmorLoadProfile(virSecurityManager *mgr G_GNUC_UNUSED, + virDomainDef *def) +{ + virSecurityLabelDef *secdef = virDomainDefGetSecurityLabelDef(def, + SECURITY_APPARMOR_NAME); + + if (!secdef) + return 0; + + if ((secdef->type == VIR_DOMAIN_SECLABEL_STATIC) || + (secdef->type == VIR_DOMAIN_SECLABEL_NONE)) { + return 0; + } + if (load_profile(mgr, secdef->label, def, NULL, false) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot load AppArmor profile \'%1$s\'"), secdef->label); - goto err; + return -1; } return 0; - - err: - VIR_FREE(secdef->label); - VIR_FREE(secdef->imagelabel); - VIR_FREE(secdef->model); - return -1; } + static int AppArmorSetSecurityAllLabel(virSecurityManager *mgr, char *const *sharedFilesystems G_GNUC_UNUSED, @@ -1157,6 +1170,8 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainReserveSecurityLabel = AppArmorReserveSecurityLabel, .domainReleaseSecurityLabel = AppArmorReleaseSecurityLabel, + .domainLoadProfile = AppArmorLoadProfile, + .domainGetSecurityProcessLabel = AppArmorGetSecurityProcessLabel, .domainSetSecurityProcessLabel = AppArmorSetSecurityProcessLabel, .domainSetSecurityChildProcessLabel = AppArmorSetSecurityChildProcessLabel, -- 2.52.0