--- src/security/virt-aa-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..d563b98 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1342,7 +1342,8 @@ main(int argc, char **argv) vah_info(include_file); vah_info(included_files); rc = 0; - } else if ((rc = update_include_file(include_file, + } else if (ctl->def->virtType != VIR_DOMAIN_VIRT_LXC && + (rc = update_include_file(include_file, included_files, ctl->append)) != 0) goto cleanup; -- 1.8.4.5

Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default. This profile allows quite a lot, but strives to restrict access to dangerous resources. Removing the explicit authorizations to bash, systemd and cron files, forces them to keep the lxc profile for all applications inside the container. PUx permissions where leading to running systemd (and others tasks) unconfined. Put the generic files, network and capabilities restrictions directly in the TEMPLATE.lxc: this way, users can restrict them on a per container basis. --- examples/apparmor/Makefile.am | 6 +- examples/apparmor/TEMPLATE.lxc | 15 ++++ examples/apparmor/{TEMPLATE => TEMPLATE.qemu} | 2 +- examples/apparmor/libvirt-lxc | 119 +++++++++++++++++++++++--- src/security/security_apparmor.c | 20 +++-- src/security/virt-aa-helper.c | 29 +------ 6 files changed, 148 insertions(+), 43 deletions(-) create mode 100644 examples/apparmor/TEMPLATE.lxc rename examples/apparmor/{TEMPLATE => TEMPLATE.qemu} (75%) diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am index a741e94..7a20e16 100644 --- a/examples/apparmor/Makefile.am +++ b/examples/apparmor/Makefile.am @@ -15,7 +15,8 @@ ## <http://www.gnu.org/licenses/>;. EXTRA_DIST= \ - TEMPLATE \ + TEMPLATE.qemu \ + TEMPLATE.lxc \ libvirt-qemu \ libvirt-lxc \ usr.lib.libvirt.virt-aa-helper \ @@ -36,6 +37,7 @@ abstractions_DATA = \ templatesdir = $(apparmordir)/libvirt templates_DATA = \ - TEMPLATE \ + TEMPLATE.qemu \ + TEMPLATE.lxc \ $(NULL) endif WITH_APPARMOR_PROFILES diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc new file mode 100644 index 0000000..7b64885 --- /dev/null +++ b/examples/apparmor/TEMPLATE.lxc @@ -0,0 +1,15 @@ +# +# This profile is for the domain whose UUID matches this file. +# + +#include <tunables/global> + +profile LIBVIRT_TEMPLATE { + #include <abstractions/libvirt-lxc> + + # Globally allows everything to run under this profile + # These can be narrowed depending on the container's use. + file, + capability, + network, +} diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu similarity index 75% rename from examples/apparmor/TEMPLATE rename to examples/apparmor/TEMPLATE.qemu index 187dec5..008a221 100644 --- a/examples/apparmor/TEMPLATE +++ b/examples/apparmor/TEMPLATE.qemu @@ -5,5 +5,5 @@ #include <tunables/global> profile LIBVIRT_TEMPLATE { - #include <abstractions/libvirt-driver> + #include <abstractions/libvirt-qemu> } diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc index d404328..4bfb503 100644 --- a/examples/apparmor/libvirt-lxc +++ b/examples/apparmor/libvirt-lxc @@ -2,16 +2,115 @@ #include <abstractions/base> - # Needed for lxc-enter-namespace - capability sys_admin, - capability sys_chroot, + umount, - # Added for lxc-enter-namespace --cmd /bin/bash - /bin/bash PUx, + # ignore DENIED message on / remount + deny mount options=(ro, remount) -> /, - /usr/sbin/cron PUx, - /usr/lib/systemd/systemd PUx, + # allow tmpfs mounts everywhere + mount fstype=tmpfs, - /usr/lib/libsystemd-*.so.* mr, - /usr/lib/libudev-*.so.* mr, - /etc/ld.so.cache mr, + # allow mqueue mounts everywhere + mount fstype=mqueue, + + # allow fuse mounts everywhere + mount fstype=fuse.*, + + # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted + mount fstype=binfmt_misc -> /proc/sys/fs/binfmt_misc/, + deny @{PROC}/sys/fs/** wklx, + + # allow efivars to be mounted, writing to it will be blocked though + mount fstype=efivarfs -> /sys/firmware/efi/efivars/, + + # block some other dangerous paths + deny @{PROC}/sysrq-trigger rwklx, + deny @{PROC}/mem rwklx, + deny @{PROC}/kmem rwklx, + + # deny writes in /sys except for /sys/fs/cgroup, also allow + # fusectl, securityfs and debugfs to be mounted there (read-only) + mount fstype=fusectl -> /sys/fs/fuse/connections/, + mount fstype=securityfs -> /sys/kernel/security/, + mount fstype=debugfs -> /sys/kernel/debug/, + mount fstype=proc -> /proc/, + mount fstype=sysfs -> /sys/, + deny /sys/firmware/efi/efivars/** rwklx, + deny /sys/kernel/security/** rwklx, + + # generated by: lxc-generate-aa-rules.py container-rules.base + deny /proc/sys/[^kn]*{,/**} wklx, + deny /proc/sys/k[^e]*{,/**} wklx, + deny /proc/sys/ke[^r]*{,/**} wklx, + deny /proc/sys/ker[^n]*{,/**} wklx, + deny /proc/sys/kern[^e]*{,/**} wklx, + deny /proc/sys/kerne[^l]*{,/**} wklx, + deny /proc/sys/kernel/[^smhd]*{,/**} wklx, + deny /proc/sys/kernel/d[^o]*{,/**} wklx, + deny /proc/sys/kernel/do[^m]*{,/**} wklx, + deny /proc/sys/kernel/dom[^a]*{,/**} wklx, + deny /proc/sys/kernel/doma[^i]*{,/**} wklx, + deny /proc/sys/kernel/domai[^n]*{,/**} wklx, + deny /proc/sys/kernel/domain[^n]*{,/**} wklx, + deny /proc/sys/kernel/domainn[^a]*{,/**} wklx, + deny /proc/sys/kernel/domainna[^m]*{,/**} wklx, + deny /proc/sys/kernel/domainnam[^e]*{,/**} wklx, + deny /proc/sys/kernel/domainname?*{,/**} wklx, + deny /proc/sys/kernel/h[^o]*{,/**} wklx, + deny /proc/sys/kernel/ho[^s]*{,/**} wklx, + deny /proc/sys/kernel/hos[^t]*{,/**} wklx, + deny /proc/sys/kernel/host[^n]*{,/**} wklx, + deny /proc/sys/kernel/hostn[^a]*{,/**} wklx, + deny /proc/sys/kernel/hostna[^m]*{,/**} wklx, + deny /proc/sys/kernel/hostnam[^e]*{,/**} wklx, + deny /proc/sys/kernel/hostname?*{,/**} wklx, + deny /proc/sys/kernel/m[^s]*{,/**} wklx, + deny /proc/sys/kernel/ms[^g]*{,/**} wklx, + deny /proc/sys/kernel/msg*/** wklx, + deny /proc/sys/kernel/s[^he]*{,/**} wklx, + deny /proc/sys/kernel/se[^m]*{,/**} wklx, + deny /proc/sys/kernel/sem*/** wklx, + deny /proc/sys/kernel/sh[^m]*{,/**} wklx, + deny /proc/sys/kernel/shm*/** wklx, + deny /proc/sys/kernel?*{,/**} wklx, + deny /proc/sys/n[^e]*{,/**} wklx, + deny /proc/sys/ne[^t]*{,/**} wklx, + deny /proc/sys/net?*{,/**} wklx, + deny /sys/[^fdc]*{,/**} wklx, + deny /sys/c[^l]*{,/**} wklx, + deny /sys/cl[^a]*{,/**} wklx, + deny /sys/cla[^s]*{,/**} wklx, + deny /sys/clas[^s]*{,/**} wklx, + deny /sys/class/[^n]*{,/**} wklx, + deny /sys/class/n[^e]*{,/**} wklx, + deny /sys/class/ne[^t]*{,/**} wklx, + deny /sys/class/net?*{,/**} wklx, + deny /sys/class?*{,/**} wklx, + deny /sys/d[^e]*{,/**} wklx, + deny /sys/de[^v]*{,/**} wklx, + deny /sys/dev[^i]*{,/**} wklx, + deny /sys/devi[^c]*{,/**} wklx, + deny /sys/devic[^e]*{,/**} wklx, + deny /sys/device[^s]*{,/**} wklx, + deny /sys/devices/[^v]*{,/**} wklx, + deny /sys/devices/v[^i]*{,/**} wklx, + deny /sys/devices/vi[^r]*{,/**} wklx, + deny /sys/devices/vir[^t]*{,/**} wklx, + deny /sys/devices/virt[^u]*{,/**} wklx, + deny /sys/devices/virtu[^a]*{,/**} wklx, + deny /sys/devices/virtua[^l]*{,/**} wklx, + deny /sys/devices/virtual/[^n]*{,/**} wklx, + deny /sys/devices/virtual/n[^e]*{,/**} wklx, + deny /sys/devices/virtual/ne[^t]*{,/**} wklx, + deny /sys/devices/virtual/net?*{,/**} wklx, + deny /sys/devices/virtual?*{,/**} wklx, + deny /sys/devices?*{,/**} wklx, + deny /sys/f[^s]*{,/**} wklx, + deny /sys/fs/[^c]*{,/**} wklx, + deny /sys/fs/c[^g]*{,/**} wklx, + deny /sys/fs/cg[^r]*{,/**} wklx, + deny /sys/fs/cgr[^o]*{,/**} wklx, + deny /sys/fs/cgro[^u]*{,/**} wklx, + deny /sys/fs/cgrou[^p]*{,/**} wklx, + deny /sys/fs/cgroup?*{,/**} wklx, + deny /sys/fs?*{,/**} wklx, diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 1e2a38b..778d233 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -351,26 +351,36 @@ AppArmorSetSecuritySCSILabel(virSCSIDevicePtr dev ATTRIBUTE_UNUSED, static int AppArmorSecurityManagerProbe(const char *virtDriver ATTRIBUTE_UNUSED) { - char *template = NULL; + char *template_qemu = NULL; + char *template_lxc = NULL; int rc = SECURITY_DRIVER_DISABLE; if (use_apparmor() < 0) return rc; /* see if template file exists */ - if (virAsprintf(&template, "%s/TEMPLATE", + if (virAsprintf(&template_qemu, "%s/TEMPLATE.qemu", APPARMOR_DIR "/libvirt") == -1) return rc; - if (!virFileExists(template)) { + if (virAsprintf(&template_lxc, "%s/TEMPLATE.lxc", + APPARMOR_DIR "/libvirt") == -1) + + if (!virFileExists(template_qemu)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("template \'%s\' does not exist"), template_qemu); + goto cleanup; + } + if (!virFileExists(template_lxc)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("template \'%s\' does not exist"), template); + _("template \'%s\' does not exist"), template_lxc); goto cleanup; } rc = SECURITY_DRIVER_ENABLE; cleanup: - VIR_FREE(template); + VIR_FREE(template_qemu); + VIR_FREE(template_lxc); return rc; } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index d563b98..2a09145 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -336,24 +336,20 @@ create_profile(const char *profile, const char *profile_name, 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")); goto end; } - if (virAsprintfQuiet(&template, "%s/TEMPLATE", APPARMOR_DIR "/libvirt") < 0) { + + if (virAsprintfQuiet(&template, "%s/TEMPLATE.%s", APPARMOR_DIR "/libvirt", + virDomainVirtTypeToString(virtType)) < 0) { vah_error(NULL, 0, _("template name exceeds maximum length")); goto end; } @@ -378,11 +374,6 @@ 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")); @@ -397,15 +388,7 @@ create_profile(const char *profile, const char *profile_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_driver) - strlen(template_driver) + 1; + plen = tlen + strlen(replace_name) - strlen(template_name) + 1; if (virtType != VIR_DOMAIN_VIRT_LXC) plen += strlen(replace_files) - strlen(template_end); @@ -422,9 +405,6 @@ 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; @@ -455,7 +435,6 @@ 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: -- 1.8.4.5

On Fri, 2014-07-11 at 16:08 +0000, Serge Hallyn wrote: What looks weird? the fact that there are now 2 template files instead of one? I did that only to avoid hard-coding the default rules for lxc containers that I didn't want to go into the abstraction file. -- Cedric

Quoting Cédric Bosdonnat (cbosdonnat(a)suse.com): Hi, this being a verbatim copy from lxc's policy, is there any plan for keeping the policy uptodate as the lxc policy is updated? Does lxc-enter-namespace --cmd /bin/bash still work? (I would expect so) (This remains a bug, a 'goto cleanup' is needed here)

Quoting Cedric Bosdonnat (cbosdonnat(a)suse.com): Oh, bother. I think I just deleted that. Assuming that was the only change, Acked-by: Serge E. Hallyn <serge.hallyn(a)ubuntu.com&gt;