Re: [libvirt] [PATCH 2/2] Rework lxc apparmor profile

On Fri, 2014-07-11 at 16:08 +0000, Serge Hallyn wrote: > Quoting Cédric Bosdonnat (cbosdonnat(a)suse.com): > > 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) > > Hi, > > something looks weird here. Is this (two lines above and 5 lines below) > intended, or are there missing lines? 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.

> > + > > + 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 > > > > -- > > libvir-list mailing list > > libvir-list(a)redhat.com > > https://www.redhat.com/mailman/listinfo/libvir-list > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list