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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Cédric Bosdonnat (cbosdonnat@xxxxxxxx):
> 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?

> +
> +    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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]