From: Cédric Bosdonnat <cedric.bosdonnat(a)free.fr&gt; use_apparmor() was first designed to be called from withing libvirtd, but libvirt_lxc also uses it. in libvirt_lxc, there is no need to check whether to use apparmor or not: just use it if possible. --- examples/apparmor/Makefile.am | 2 ++ examples/apparmor/TEMPLATE | 2 +- examples/apparmor/libvirt-lxc | 10 ++++++ src/security/security_apparmor.c | 10 +++--- src/security/virt-aa-helper.c | 69 +++++++++++++++++++++++++++++----------- 5 files changed, 69 insertions(+), 24 deletions(-) create mode 100644 examples/apparmor/libvirt-lxc diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am index 2630fef..a741e94 100644 --- a/examples/apparmor/Makefile.am +++ b/examples/apparmor/Makefile.am @@ -17,6 +17,7 @@ EXTRA_DIST= \ TEMPLATE \ libvirt-qemu \ + libvirt-lxc \ usr.lib.libvirt.virt-aa-helper \ usr.sbin.libvirtd @@ -30,6 +31,7 @@ apparmor_DATA = \ abstractionsdir = $(apparmordir)/abstractions abstractions_DATA = \ libvirt-qemu \ + libvirt-lxc \ $(NULL) templatesdir = $(apparmordir)/libvirt diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE index 008a221..187dec5 100644 --- a/examples/apparmor/TEMPLATE +++ b/examples/apparmor/TEMPLATE @@ -5,5 +5,5 @@ #include <tunables/global> profile LIBVIRT_TEMPLATE { - #include <abstractions/libvirt-qemu> + #include <abstractions/libvirt-driver> } diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc new file mode 100644 index 0000000..47f27b1 --- /dev/null +++ b/examples/apparmor/libvirt-lxc @@ -0,0 +1,10 @@ +# Last Modified: Fri Feb 7 13:01:36 2014 + + #include <abstractions/base> + + /usr/sbin/cron PUx, + /usr/lib/systemd/systemd PUx, + + /usr/lib/libsystemd-*.so.* mr, + /usr/lib/libudev-*.so.* mr, + /etc/ld.so.cache mr, diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 257e3dd..14dc707 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -246,6 +246,11 @@ use_apparmor(void) return rc; } + /* If libvirt_lxc is calling us, then consider apparmor is used + * and enforced. */ + if (strstr(libvirt_daemon, "libvirt_lxc")) + return 1; + if (access(APPARMOR_PROFILES_PATH, R_OK) != 0) goto cleanup; @@ -341,7 +346,7 @@ AppArmorSetSecuritySCSILabel(virSCSIDevicePtr dev ATTRIBUTE_UNUSED, /* Called on libvirtd startup to see if AppArmor is available */ static int -AppArmorSecurityManagerProbe(const char *virtDriver) +AppArmorSecurityManagerProbe(const char *virtDriver ATTRIBUTE_UNUSED) { char *template = NULL; int rc = SECURITY_DRIVER_DISABLE; @@ -349,9 +354,6 @@ AppArmorSecurityManagerProbe(const char *virtDriver) if (use_apparmor() < 0) return rc; - if (virtDriver && STREQ(virtDriver, "LXC")) - return rc; - /* see if template file exists */ if (virAsprintf(&template, "%s/TEMPLATE", APPARMOR_DIR "/libvirt") == -1) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b9282b4..e7f1359 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -328,18 +328,24 @@ update_include_file(const char *include_file, const char *included_files, */ static int create_profile(const char *profile, const char *profile_name, - const char *profile_files) + const char *profile_files, int virtType) { char *template; char *tcontent = NULL; char *pcontent = NULL; char *replace_name = NULL; char *replace_files = NULL; + char *replace_driver = NULL; const char *template_name = "\nprofile LIBVIRT_TEMPLATE"; const char *template_end = "\n}"; + const char *template_driver = "libvirt-driver"; int tlen, plen; int fd; int rc = -1; + const char *driver_name = "qemu"; + + if (virtType == VIR_DOMAIN_VIRT_LXC) + driver_name = "lxc"; if (virFileExists(profile)) { vah_error(NULL, 0, _("profile exists")); @@ -371,6 +377,11 @@ create_profile(const char *profile, const char *profile_name, goto clean_tcontent; } + if (strstr(tcontent, template_driver) == NULL) { + vah_error(NULL, 0, _("no replacement string in template")); + goto clean_tcontent; + } + /* '\nprofile <profile_name>\0' */ if (virAsprintfQuiet(&replace_name, "\nprofile %s", profile_name) == -1) { vah_error(NULL, 0, _("could not allocate memory for profile name")); @@ -378,14 +389,26 @@ create_profile(const char *profile, const char *profile_name, } /* '\n<profile_files>\n}\0' */ - if (virAsprintfQuiet(&replace_files, "\n%s\n}", profile_files) == -1) { + if ((virtType != VIR_DOMAIN_VIRT_LXC) && + virAsprintfQuiet(&replace_files, "\n%s\n}", profile_files) == -1) { vah_error(NULL, 0, _("could not allocate memory for profile files")); VIR_FREE(replace_name); goto clean_tcontent; } + /* 'libvirt-<driver_name>\0' */ + if (virAsprintfQuiet(&replace_driver, "libvirt-%s", driver_name) == -1) { + vah_error(NULL, 0, _("could not allocate memory for profile driver")); + VIR_FREE(replace_driver); + goto clean_tcontent; + } + plen = tlen + strlen(replace_name) - strlen(template_name) + - strlen(replace_files) - strlen(template_end) + 1; + strlen(replace_driver) - strlen(template_driver) + 1; + + if (virtType != VIR_DOMAIN_VIRT_LXC) + plen += strlen(replace_files) - strlen(template_end); + if (plen > MAX_FILE_LEN || plen < tlen) { vah_error(NULL, 0, _("invalid length for new profile")); goto clean_replace; @@ -398,10 +421,14 @@ create_profile(const char *profile, const char *profile_name, pcontent[0] = '\0'; strcpy(pcontent, tcontent); + if (replace_string(pcontent, plen, template_driver, replace_driver) < 0) + goto clean_all; + if (replace_string(pcontent, plen, template_name, replace_name) < 0) goto clean_all; - if (replace_string(pcontent, plen, template_end, replace_files) < 0) + if ((virtType != VIR_DOMAIN_VIRT_LXC) && + replace_string(pcontent, plen, template_end, replace_files) < 0) goto clean_all; /* write the file */ @@ -427,6 +454,7 @@ create_profile(const char *profile, const char *profile_name, clean_replace: VIR_FREE(replace_name); VIR_FREE(replace_files); + VIR_FREE(replace_driver); clean_tcontent: VIR_FREE(tcontent); end: @@ -666,8 +694,8 @@ caps_mockup(vahControl * ctl, const char *xmlStr) goto cleanup; ctl->hvm = virXPathString("string(./os/type[1])", ctxt); - if (!ctl->hvm || STRNEQ(ctl->hvm, "hvm")) { - vah_error(ctl, 0, _("os.type is not 'hvm'")); + if (!ctl->hvm) { + vah_error(ctl, 0, _("os.type is not defined")); goto cleanup; } arch = virXPathString("string(./os/type[1]/@arch)", ctxt); @@ -1234,18 +1262,20 @@ main(int argc, char **argv) if (vah_add_file(&buf, ctl->newfile, "rw") != 0) goto cleanup; } else { - virBufferAsprintf(&buf, " \"%s/log/libvirt/**/%s.log\" w,\n", - LOCALSTATEDIR, ctl->def->name); - virBufferAsprintf(&buf, " \"%s/lib/libvirt/**/%s.monitor\" rw,\n", - LOCALSTATEDIR, ctl->def->name); - virBufferAsprintf(&buf, " \"%s/run/libvirt/**/%s.pid\" rwk,\n", - LOCALSTATEDIR, ctl->def->name); - virBufferAsprintf(&buf, " \"/run/libvirt/**/%s.pid\" rwk,\n", - ctl->def->name); - virBufferAsprintf(&buf, " \"%s/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", - LOCALSTATEDIR, ctl->def->name); - virBufferAsprintf(&buf, " \"/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", - ctl->def->name); + if (ctl->def->virtType == VIR_DOMAIN_VIRT_QEMU) { + virBufferAsprintf(&buf, " \"%s/log/libvirt/**/%s.log\" w,\n", + LOCALSTATEDIR, ctl->def->name); + virBufferAsprintf(&buf, " \"%s/lib/libvirt/**/%s.monitor\" rw,\n", + LOCALSTATEDIR, ctl->def->name); + virBufferAsprintf(&buf, " \"%s/run/libvirt/**/%s.pid\" rwk,\n", + LOCALSTATEDIR, ctl->def->name); + virBufferAsprintf(&buf, " \"/run/libvirt/**/%s.pid\" rwk,\n", + ctl->def->name); + virBufferAsprintf(&buf, " \"%s/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", + LOCALSTATEDIR, ctl->def->name); + virBufferAsprintf(&buf, " \"/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", + ctl->def->name); + } if (ctl->files) virBufferAdd(&buf, ctl->files, -1); } @@ -1282,7 +1312,8 @@ main(int argc, char **argv) vah_info(ctl->uuid); vah_info(tmp); rc = 0; - } else if ((rc = create_profile(profile, ctl->uuid, tmp)) != 0) { + } else if ((rc = create_profile(profile, ctl->uuid, tmp, + ctl->def->virtType)) != 0) { vah_error(ctl, 0, _("could not create profile")); unlink(include_file); } -- 1.8.5.2

No security_driver value could cause weird behavior, like using apparmor even though we don't want it. --- src/lxc/lxc.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lxc/lxc.conf b/src/lxc/lxc.conf index 8df4601..5eb0122 100644 --- a/src/lxc/lxc.conf +++ b/src/lxc/lxc.conf @@ -20,6 +20,8 @@ # to 'none' instead. # #security_driver = "selinux" +#security_driver = "apparmor" +security_driver = "none" # If set to non-zero, then the default security labeling # will make guests confined. If set to zero, then guests -- 1.8.5.2

On Fri, 2014-02-21 at 14:23 +0000, Daniel P. Berrange wrote: I see. Then I'll need it add it instead of this patch. -- Cedric

--- examples/apparmor/libvirt-lxc | 7 +++++++ src/libvirt-lxc.c | 13 +++++++++++++ 2 files changed, 20 insertions(+) diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc index 47f27b1..d404328 100644 --- a/examples/apparmor/libvirt-lxc +++ b/examples/apparmor/libvirt-lxc @@ -2,6 +2,13 @@ #include <abstractions/base> + # Needed for lxc-enter-namespace + capability sys_admin, + capability sys_chroot, + + # Added for lxc-enter-namespace --cmd /bin/bash + /bin/bash PUx, + /usr/sbin/cron PUx, /usr/lib/systemd/systemd PUx, diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c index 074809a..f10fafc 100644 --- a/src/libvirt-lxc.c +++ b/src/libvirt-lxc.c @@ -33,6 +33,9 @@ #ifdef WITH_SELINUX # include <selinux/selinux.h> #endif +#ifdef WITH_APPARMOR +# include <sys/apparmor.h> +#endif #define VIR_FROM_THIS VIR_FROM_NONE @@ -240,6 +243,16 @@ virDomainLxcEnterSecurityLabel(virSecurityModelPtr model, _("Support for SELinux is not enabled")); goto error; #endif + } else if (STREQ(model->model, "apparmor")) { +#ifdef WITH_APPARMOR + if (aa_change_profile(label->label) < 0) + virReportSystemError(errno, _("error changing profile to %s"), + label->label); +#else + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Support for AppArmor is not enabled")); + goto error; +#endif } else { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, _("Security model %s cannot be entered"), -- 1.8.5.2