[libvirt] [PATCH 0/5] Get AppArmor to work for LXC containers
This patch series, implements the AppArmor support for LXC containers. examples/apparmor/libvirt-lxc only allows the minimum, users will be able to add more in the generated profile. Cédric Bosdonnat (5): LXC driver: generate apparmor profiles for guests Make sure apparmor is started before libvirtd Set default lxc security_driver to none apparmor: add debug traces when changing profile. add support for apparmor in lxc-enter-namespace daemon/libvirtd.service.in | 1 + examples/apparmor/Makefile.am | 2 ++ examples/apparmor/TEMPLATE | 2 +- examples/apparmor/libvirt-lxc | 17 ++++++++++ src/libvirt-lxc.c | 13 ++++++++ src/lxc/lxc.conf | 2 ++ src/security/security_apparmor.c | 15 ++++++--- src/security/virt-aa-helper.c | 69 +++++++++++++++++++++++++++++----------- 8 files changed, 97 insertions(+), 24 deletions(-) create mode 100644 examples/apparmor/libvirt-lxc -- 1.8.5.2
From: Cédric Bosdonnat <cedric.bosdonnat@free.fr> 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
If apparmor security driver is enabled in either qemu or lxc driver configuration and libvirtd starts before AppArmor, it will fail. --- daemon/libvirtd.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index dc2433a..b50fca0 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -9,6 +9,7 @@ Before=libvirt-guests.service After=network.target After=dbus.service After=iscsid.service +After=apparmor.service Documentation=man:libvirtd(8) Documentation=http://libvirt.org -- 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, Feb 21, 2014 at 02:57:28PM +0100, Cédric Bosdonnat wrote:
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
This shouldn't be required. What is supposed to happen is that the security drivers are enabled by default, but the guests get given a label which is disabled. eg if you have SELinux security driver enabled, the LXC containers will get given: <seclabel type='none' model='selinux'/> Instead of what QEMU gets: <seclabel type='dynamic' model='selinux'/> The type='none' means do not confine the guest. I guess we never added support to the apparmour driver to honour the VIR_DOMAIN_SECLABEL_NONE value. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, 2014-02-21 at 14:23 +0000, Daniel P. Berrange wrote:
On Fri, Feb 21, 2014 at 02:57:28PM +0100, Cédric Bosdonnat wrote:
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
This shouldn't be required. What is supposed to happen is that the security drivers are enabled by default, but the guests get given a label which is disabled. eg if you have SELinux security driver enabled, the LXC containers will get given:
<seclabel type='none' model='selinux'/>
Instead of what QEMU gets:
<seclabel type='dynamic' model='selinux'/>
The type='none' means do not confine the guest. I guess we never added support to the apparmour driver to honour the VIR_DOMAIN_SECLABEL_NONE value.
I see. Then I'll need it add it instead of this patch. -- Cedric
The reason for these is that aa-status doesn't show the process using the profile as they are in another namespace. --- src/security/security_apparmor.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 14dc707..1c1b128 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -593,6 +593,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, goto cleanup; } + VIR_DEBUG("Changing AppArmor profile to %s", profile_name); if (aa_change_profile(profile_name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("error calling aa_change_profile()")); @@ -618,6 +619,7 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, { int rc = -1; char *profile_name = NULL; + char *cmd_str = NULL; virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); @@ -637,11 +639,14 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, if ((profile_name = get_profile_name(def)) == NULL) goto cleanup; + cmd_str = virCommandToString(cmd); + VIR_DEBUG("Changing AppArmor profile to %s on %s", profile_name, cmd_str); virCommandSetAppArmorProfile(cmd, profile_name); rc = 0; cleanup: VIR_FREE(profile_name); + VIR_FREE(cmd_str); return rc; } -- 1.8.5.2
--- 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
participants (3)
-
Cedric Bosdonnat -
Cédric Bosdonnat
-
Daniel P. Berrange