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 | 38 +++++++++++++++++++--------
src/security/virt-aa-helper.c | 45 ++++++++++++++++++++++++++------
2 files changed, 64 insertions(+), 19 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 91c51f6395..907b01577c 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -147,7 +147,8 @@ load_profile(virSecurityManager *mgr G_GNUC_UNUSED,
const char *profile,
virDomainDef *def,
const char *fn,
- bool append)
+ bool append,
+ bool runtime)
{
bool create = true;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
@@ -173,6 +174,8 @@ load_profile(virSecurityManager *mgr G_GNUC_UNUSED,
} else {
virCommandAddArgList(cmd, "-f", fn, NULL);
}
+ if (runtime)
+ virCommandAddArgList(cmd, "-t", NULL);
}
virCommandAddEnvFormat(cmd,
@@ -245,10 +248,11 @@ use_apparmor(void)
* NULL.
*/
static int
-reload_profile(virSecurityManager *mgr,
- virDomainDef *def,
- const char *fn,
- bool append)
+reload_runtime_profile(virSecurityManager *mgr,
+ virDomainDef *def,
+ const char *fn,
+ bool append,
+ bool runtime)
{
virSecurityLabelDef *secdef = virDomainDefGetSecurityLabelDef(
def, SECURITY_APPARMOR_NAME);
@@ -258,7 +262,7 @@ reload_profile(virSecurityManager *mgr,
/* Update the profile only if it is loaded */
if (profile_loaded(secdef->imagelabel) >= 0) {
- if (load_profile(mgr, secdef->imagelabel, def, fn, append) < 0) {
+ if (load_profile(mgr, secdef->imagelabel, def, fn, append, runtime) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot update AppArmor profile \'%1$s\'"),
secdef->imagelabel);
@@ -268,6 +272,18 @@ reload_profile(virSecurityManager *mgr,
return 0;
}
+/* reload the profile, adding read/write file specified by fn if it is not
+ * NULL.
+ */
+static int
+reload_profile(virSecurityManager *mgr,
+ virDomainDef *def,
+ const char *fn,
+ bool append)
+{
+ return reload_runtime_profile(mgr, def, fn, append, false);
+}
+
static int
AppArmorSetSecurityHostdevLabelHelper(const char *file, void *opaque)
{
@@ -388,7 +404,7 @@ AppArmorGenSecurityLabel(virSecurityManager *mgr G_GNUC_UNUSED,
secdef->model = g_strdup(SECURITY_APPARMOR_NAME);
/* Now that we have a label, load the profile into the kernel. */
- if (load_profile(mgr, secdef->label, def, NULL, false) < 0) {
+ if (load_profile(mgr, secdef->label, def, NULL, false, false) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot load AppArmor profile \'%1$s\'"),
secdef->label);
@@ -420,7 +436,7 @@ 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);
+ return reload_runtime_profile(mgr, def, incomingPath, true, true);
return 0;
}
@@ -1074,9 +1090,9 @@ AppArmorSetPathLabel(virSecurityManager *mgr,
if (allowSubtree) {
full_path = g_strdup_printf("%s/{,**}", path);
- rc = reload_profile(mgr, def, full_path, true);
+ rc = reload_runtime_profile(mgr, def, full_path, true, true);
} else {
- rc = reload_profile(mgr, def, path, true);
+ rc = reload_runtime_profile(mgr, def, path, true, true);
}
return rc;
@@ -1112,7 +1128,7 @@ AppArmorSetFDLabel(virSecurityManager *mgr,
return 0;
}
- return reload_profile(mgr, def, fd_path, true);
+ return reload_runtime_profile(mgr, def, fd_path, true, true);
}
static char *
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 1626d5a89c..3a217fa3d1 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -71,6 +71,7 @@ typedef struct {
virArch arch; /* machine architecture */
char *newfile; /* newly added file */
bool append; /* append to .files instead of rewrite */
+ bool runtime; /* file should be added to .runtime_files */
} vahControl;
static int
@@ -110,6 +111,7 @@ vah_usage(void)
" Extra File:\n"
" -f | --add-file <file> add file to a profile generated from XML\n"
" -F | --append-file <file> append file to an existing profile\n"
+ " -t | --runtime file is valid only during runtime\n"
"\n"), progname);
puts(_("This command is intended to be used by libvirtd and not used directly.\n"));
@@ -1356,10 +1358,11 @@ vahParseArgv(vahControl * ctl, int argc, char **argv)
{ "replace", 0, 0, 'r' },
{ "remove", 0, 0, 'R' },
{ "uuid", 1, 0, 'u' },
+ { "runtime", 0, 0, 't' },
{ 0, 0, 0, 0 },
};
- while ((arg = getopt_long(argc, argv, "acdDhrRH:b:u:p:f:F:", opt,
+ while ((arg = getopt_long(argc, argv, "acdDhrRH:b:u:p:f:F:t", opt,
&idx)) != -1) {
switch (arg) {
case 'a':
@@ -1396,6 +1399,9 @@ vahParseArgv(vahControl * ctl, int argc, char **argv)
PROFILE_NAME_SIZE) < 0)
vah_error(ctl, 1, _("error copying UUID"));
break;
+ case 't':
+ ctl->runtime = true;
+ break;
default:
vah_error(ctl, 1, _("unsupported option"));
break;
@@ -1445,9 +1451,16 @@ main(int argc, char **argv)
int rc = -1;
g_autofree char *profile = NULL;
g_autofree char *include_file = NULL;
+ g_autofree char *include_runtime_file = NULL;
off_t size;
bool purged = 0;
+#if defined(WITH_APPARMOR_3)
+ const char *ifexists = "if exists ";
+#else
+ const char *ifexists = "";
+#endif
+
if (virGettextInitialize() < 0 ||
virErrorInitialize() < 0) {
fprintf(stderr, _("%1$s: initialization failed\n"), argv[0]);
@@ -1479,13 +1492,16 @@ main(int argc, char **argv)
profile = g_strdup_printf("%s/%s", APPARMOR_DIR "/libvirt", ctl->uuid);
include_file = g_strdup_printf("%s/%s.files", APPARMOR_DIR "/libvirt", ctl->uuid);
+ include_runtime_file = g_strdup_printf("%s/%s.runtime_files", APPARMOR_DIR "/libvirt", ctl->uuid);
if (ctl->cmd == 'a') {
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(include_runtime_file);
+ }
} else if (ctl->cmd == 'c' || ctl->cmd == 'r') {
g_autofree char *included_files = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
@@ -1513,6 +1529,7 @@ main(int argc, char **argv)
if (vah_add_file(&buf, ctl->newfile, "rwk") != 0)
goto cleanup;
} else {
+ virBufferAsprintf(&buf, " #include %s<libvirt/%s.runtime_files>\n", ifexists, ctl->uuid);
if (ctl->def->virtType == VIR_DOMAIN_VIRT_QEMU ||
ctl->def->virtType == VIR_DOMAIN_VIRT_KQEMU ||
ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) {
@@ -1535,11 +1552,20 @@ main(int argc, char **argv)
/* (re)create the include file using included_files */
if (ctl->dryrun) {
- vah_info(include_file);
+ if (ctl->runtime)
+ vah_info(include_runtime_file);
+ else
+ vah_info(include_file);
vah_info(included_files);
rc = 0;
} else if (ctl->def->virtType == VIR_DOMAIN_VIRT_LXC) {
rc = 0;
+ } else if (ctl->runtime) {
+ /* runtime should only update include_runtime_file */
+ if ((rc = update_include_file(include_runtime_file,
+ included_files,
+ ctl->append)) != 0)
+ goto cleanup;
} else if ((rc = update_include_file(include_file,
included_files,
ctl->append)) != 0) {
@@ -1550,11 +1576,12 @@ main(int argc, char **argv)
/* create the profile from TEMPLATE */
if (ctl->cmd == 'c' || purged) {
g_autofree char *tmp = NULL;
-#if defined(WITH_APPARMOR_3)
- const char *ifexists = "if exists ";
-#else
- const char *ifexists = "";
-#endif
+
+ /* ideally libvirt-uuid.files and
+ * libvirt-uuid.runtime_files should be in libvirt-uuid.d/
+ * and the directory should be included instead, but how
+ * to deal with running domains when the libvirt-uuid
+ * profile is not recreated? */
tmp = g_strdup_printf(" #include %s<libvirt/%s.files>\n", ifexists, ctl->uuid);
if (ctl->dryrun) {
@@ -1566,6 +1593,7 @@ main(int argc, char **argv)
ctl->def->virtType)) != 0) {
vah_error(ctl, 0, _("could not create profile"));
unlink(include_file);
+ unlink(include_runtime_file);
}
}
@@ -1578,6 +1606,7 @@ main(int argc, char **argv)
/* cleanup */
if (rc != 0) {
unlink(include_file);
+ unlink(include_runtime_file);
if (ctl->cmd == 'c')
unlink(profile);
}