Some rules are generated dynamically during boot and added to the AppArmor policy. An example of that is macvtap devices that call the AppArmorSetFDLabel hook to add a rule for the tap device path. Since this information is dynamic, it is not available in the xml config, therefore whenever a "Restore" hook is called, the entire profile is regenerated by virt-aa-helper based only the information from the VM definition, so the dynamic information is lost. This patch stores the dynamically generated rules while the domain is running and reloads them whenever there's a restore operation. Note that there are no hooks for restoring FD labels, so that information is not removed from the set of rules while the domain is running. Closes: https://gitlab.com/libvirt/libvirt/-/issues/692 Signed-off-by: Georgia Garcia <georgia.garcia@xxxxxxxxxxxxx> --- src/security/security_apparmor.c | 208 +++++++++++++++++++++++++++++-- 1 file changed, 200 insertions(+), 8 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 07e95ec81d..ae5815fb0d 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -60,6 +60,24 @@ struct SDPDOP { virDomainDef *def; }; +typedef struct _virSecurityAppArmorLabel virSecurityAppArmorLabel; +struct _virSecurityAppArmorLabel { + GPtrArray *paths; + char *name; +}; + +typedef struct _virSecurityAppArmorLabelList virSecurityAppArmorLabelList; +struct _virSecurityAppArmorLabelList { + GPtrArray *labels; + virMutex lock; +}; + +virSecurityAppArmorLabelList *labelList = NULL; + +static int AppArmorRestorePolicy(virSecurityManager *mgr, + virDomainDef *def, + char *seclabel); + /* * profile_status returns '-2' on error, '-1' if not loaded, '0' if loaded * @@ -273,6 +291,119 @@ reload_profile(virSecurityManager *mgr, secdef->imagelabel); return -1; } + if (!append) { + if (AppArmorRestorePolicy(mgr, def, secdef->imagelabel) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot restore rules from AppArmor profile \'%1$s\'"), + secdef->imagelabel); + return -1; + } + } + + } + return 0; +} + +static int +AppArmorInitLabelList(void) +{ + if (labelList) + return 0; + + labelList = g_new0(virSecurityAppArmorLabelList, 1); + labelList->labels = g_ptr_array_new(); + if (virMutexInit(&labelList->lock) < 0) { + virReportSystemError(errno, "%s", + _("Failed to initialize AppArmor mutex")); + return -1; + } + return 0; +} + +static void +AppArmorFreeLabelList(void) +{ + size_t i; + if (!labelList) + return; + + VIR_WITH_MUTEX_LOCK_GUARD(&labelList->lock) { + for (i = 0; i < labelList->labels->len; i++) { + virSecurityAppArmorLabel *label = g_ptr_array_index(labelList->labels, i); + g_ptr_array_free(label->paths, TRUE); + g_free(label->name); + } + g_ptr_array_free(labelList->labels, TRUE); + } + virMutexDestroy(&labelList->lock); + g_free(labelList); + labelList = NULL; +} + +static int +AppArmorAppendPathToLabelList(char *seclabel, + const char *path) +{ + size_t i; + char *new; + + if (!path) + return 0; + + if (!labelList) { + if (AppArmorInitLabelList() < 0) + return -1; + } + + VIR_WITH_MUTEX_LOCK_GUARD(&labelList->lock) { + for (i = 0; i < labelList->labels->len; i++) { + virSecurityAppArmorLabel *label = g_ptr_array_index(labelList->labels, i); + if (STREQ(seclabel, label->name)) { + new = g_strdup(path); + g_ptr_array_add(label->paths, new); + } + } + } + return 0; +} + +static int +AppArmorLoadStoredPath(virSecurityAppArmorLabel *label, + virSecurityManager *mgr, + virDomainDef *def, + char *seclabel) +{ + size_t i; + for (i = 0; i < label->paths->len; i++) { + if (load_profile(mgr, seclabel, def, g_ptr_array_index(label->paths, i), true) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot update AppArmor profile \'%1$s\' \'%2$s\'"), + seclabel, (char *)g_ptr_array_index(label->paths, i)); + return -1; + } + } + return 0; +} + +static int +AppArmorRestorePolicy(virSecurityManager *mgr, + virDomainDef *def, + char *seclabel) +{ + size_t i; + + if (!labelList) { + return 0; + } + + VIR_WITH_MUTEX_LOCK_GUARD(&labelList->lock) { + for (i = 0; i < labelList->labels->len; i++) { + virSecurityAppArmorLabel *label = g_ptr_array_index(labelList->labels, i); + if (STREQ(seclabel, label->name)) { + if (AppArmorLoadStoredPath(label, mgr, def, seclabel) < 0) + return -1; + } + } } return 0; } @@ -330,12 +461,13 @@ AppArmorSecurityManagerProbe(const char *virtDriver G_GNUC_UNUSED) static int AppArmorSecurityManagerOpen(virSecurityManager *mgr G_GNUC_UNUSED) { - return 0; + return AppArmorInitLabelList(); } static int AppArmorSecurityManagerClose(virSecurityManager *mgr G_GNUC_UNUSED) { + AppArmorFreeLabelList(); return 0; } @@ -364,6 +496,7 @@ AppArmorGenSecurityLabel(virSecurityManager *mgr G_GNUC_UNUSED, g_autofree char *profile_name = NULL; virSecurityLabelDef *secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + virSecurityAppArmorLabel *label; if (!secdef) return 0; @@ -404,6 +537,18 @@ AppArmorGenSecurityLabel(virSecurityManager *mgr G_GNUC_UNUSED, goto err; } + if (!labelList) { + if (AppArmorInitLabelList() < 0) + goto err; + } + + label = g_new0(virSecurityAppArmorLabel, 1); + label->paths = g_ptr_array_new(); + label->name = g_strdup(profile_name); + VIR_WITH_MUTEX_LOCK_GUARD(&labelList->lock) { + g_ptr_array_add(labelList->labels, label); + } + return 0; err: @@ -421,6 +566,7 @@ AppArmorSetSecurityAllLabel(virSecurityManager *mgr, bool chardevStdioLogd G_GNUC_UNUSED, bool migrated G_GNUC_UNUSED) { + int rc; virSecurityLabelDef *secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); if (!secdef || !secdef->relabel) @@ -428,8 +574,15 @@ AppArmorSetSecurityAllLabel(virSecurityManager *mgr, /* Reload the profile if incomingPath is specified. Note that GenSecurityLabel() will have already been run. */ - if (incomingPath) - return reload_profile(mgr, def, incomingPath, true); + if (incomingPath) { + rc = reload_profile(mgr, def, incomingPath, true); + if (AppArmorAppendPathToLabelList(secdef->imagelabel, incomingPath) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not store path")); + return -1; + } + return rc; + } return 0; } @@ -495,6 +648,7 @@ AppArmorRestoreSecurityAllLabel(virSecurityManager *mgr G_GNUC_UNUSED, bool migrated G_GNUC_UNUSED, bool chardevStdioLogd G_GNUC_UNUSED) { + size_t i; int rc = 0; virSecurityLabelDef *secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); @@ -508,6 +662,21 @@ AppArmorRestoreSecurityAllLabel(virSecurityManager *mgr G_GNUC_UNUSED, _("could not remove profile for \'%1$s\'"), secdef->label); } + if (labelList) { + VIR_WITH_MUTEX_LOCK_GUARD(&labelList->lock) { + for (i = 0; i < labelList->labels->len; i++) { + virSecurityAppArmorLabel *label; + label = g_ptr_array_index(labelList->labels, i); + if (STREQ(secdef->label, label->name)) { + g_free(label->name); + g_ptr_array_free(label->paths, TRUE); + g_ptr_array_remove_index(labelList->labels, i); + break; + } + } + } + } + } return rc; } @@ -1082,15 +1251,26 @@ AppArmorSetPathLabel(virSecurityManager *mgr, { int rc = -1; char *full_path = NULL; + virSecurityLabelDef *secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); if (allowSubtree) { full_path = g_strdup_printf("%s/{,**}", path); - rc = reload_profile(mgr, def, full_path, true); - VIR_FREE(full_path); } else { - rc = reload_profile(mgr, def, path, true); + full_path = g_strdup(path); + } + + rc = reload_profile(mgr, def, full_path, true); + if (rc == 0) { + if (AppArmorAppendPathToLabelList(secdef->imagelabel, full_path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not store path")); + rc = -1; + } } + VIR_FREE(full_path); return rc; } @@ -1107,6 +1287,7 @@ AppArmorSetFDLabel(virSecurityManager *mgr, virDomainDef *def, int fd) { + int rc = 0; char *proc = NULL; char *fd_path = NULL; @@ -1121,10 +1302,21 @@ AppArmorSetFDLabel(virSecurityManager *mgr, if (virFileResolveLink(proc, &fd_path) < 0) { /* it's a deleted file, presumably. Ignore? */ VIR_WARN("could not find path for descriptor %s, skipping", proc); - return 0; + goto err; } - return reload_profile(mgr, def, fd_path, true); + rc = reload_profile(mgr, def, fd_path, true); + if (rc == 0) { + if (AppArmorAppendPathToLabelList(secdef->imagelabel, fd_path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not store path")); + rc = -1; + } + } + g_free(fd_path); + err: + g_free(proc); + return rc; } static char * -- 2.34.1
