[libvirt] [sandbox PATCH] virt-sandbox patch to launch containers with proper label

[Daniel P. Berrange]

On Wed, Sep 25, 2013 at 04:50:23PM -0400, Dan Walsh wrote:
> virt-sandbox should be launching containers based off the lxc_context
> file from selinux-policy. I changed the hard coded paths to match the
> latest fedora assigned labels.
> 
> Fedora 20 SELinux Policy and beyond will have proper SELinux labels in its
> lxc_contexts file.
> ---
>  bin/virt-sandbox-service                  |  2 +-
>  bin/virt-sandbox-service-clone.pod        |  5 ++-
>  bin/virt-sandbox-service-create.pod       |  7 ++--
>  bin/virt-sandbox.c                        |  5 ++-
>  libvirt-sandbox/libvirt-sandbox-builder.c | 58 +++++++++++++++++++++++++------
>  5 files changed, 55 insertions(+), 22 deletions(-)
> 
> diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> index c4c4f54..b42fe08 100755
> --- a/bin/virt-sandbox-service
> +++ b/bin/virt-sandbox-service
> @@ -101,7 +101,7 @@ def copydirtree(src, dst):
>  class Container:
>      DEFAULT_PATH       = "/var/lib/libvirt/filesystems"
>      DEFAULT_IMAGE      = "/var/lib/libvirt/images/%s.raw"
> -    SELINUX_FILE_TYPE  = "svirt_lxc_file_t"
> +    SELINUX_FILE_TYPE  = "svirt_sandbox_file_t"

This change will make it impossible to use the new release on existing
distros since they won't have this new policy type. We need this to be
conditionally changed.

> diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c
> index 1335042..613161a 100644
> --- a/libvirt-sandbox/libvirt-sandbox-builder.c
> +++ b/libvirt-sandbox/libvirt-sandbox-builder.c
> @@ -67,6 +67,48 @@ gvir_sandbox_builder_error_quark(void)
>  {
>      return g_quark_from_static_string("gvir-sandbox-builder");
>  }
> +#include <selinux/selinux.h>
> +#include <errno.h>
> +static char line[1024];
> +
> +static const char *get_label(int type) {
> +    const char *path = selinux_lxc_contexts_path();
> +
> +    FILE *fp = fopen(path, "r");
> +    if (fp) {
> +        GType gt = gvir_config_domain_virt_type_get_type ();
> +        GEnumClass *cls = g_type_class_ref (gt);
> +        GEnumValue *val = g_enum_get_value (cls, type);
> +
> +        while (val && fgets(line, sizeof line, fp)) {
> +            int len = strlen(line);
> +            if (len > 2)
> +                continue;
> +            if (line[len-1] == '\n')
> +                line[len-1] = '\0';
> +            char *name = line;
> +            char *value = strchr(name, '=');
> +            if (!value)
> +                continue;
> +            *value = '\0';
> +            value++;
> +            if (strcmp(name,val->value_nick))
> +                continue;
> +            return value;
> +        }
> +        fclose(fp);

I'm not sure I really understand what this code is doing. You seem to
be opening /etc/selinux/targetted/context/lxc_contexts and then searching
for the type for LXC, QEMU or KVM. This doesn't really make sense to me.
I wonder what the point of any of this code us, when the switch statement
below looks to be sufficient.

> +    }
> +
> +    switch (type) {
> +    case GVIR_CONFIG_DOMAIN_VIRT_KVM:
> +        return "system_u:system_r:svirt_qemu_net_t:s0";

This should be 'svirt_kvm_net_t' otherwise you are granting the
KVM process permission to use execmem & execstack, which is
bad from a security POV.

> +    case GVIR_CONFIG_DOMAIN_VIRT_QEMU:
> +        return "system_u:system_r:svirt_qemu_net_t:s0";

This again looks like it might have back compat problems, if we
try to use this on old policy based systems. Though those hosts
were already broken due to svirt_t type not allowing sufficient
privileges, so perhaps its ok.

> +    case GVIR_CONFIG_DOMAIN_VIRT_LXC:
> +    default:
> +        return "system_u:system_r:svirt_lxc_net_t:s0";

The default case should report an error - we shouldn't assume that
if we add usermode linux or vmware support, that it'll want this
lxc type.

> +    }
> +}
>  
>  static gboolean gvir_sandbox_builder_construct_domain(GVirSandboxBuilder *builder,
>                                                        GVirSandboxConfig *config,
> @@ -335,17 +377,11 @@ static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil
>      if (gvir_sandbox_config_get_security_dynamic(config)) {
>          gvir_config_domain_seclabel_set_type(sec,
>                                               GVIR_CONFIG_DOMAIN_SECLABEL_DYNAMIC);
> -        if (label)
> -            gvir_config_domain_seclabel_set_baselabel(sec, label);
> -        else if (gvir_config_domain_get_virt_type(domain) ==
> -                 GVIR_CONFIG_DOMAIN_VIRT_LXC)
> -            gvir_config_domain_seclabel_set_baselabel(sec, "system_u:system_r:svirt_lxc_net_t:s0");
> -        else if (gvir_config_domain_get_virt_type(domain) ==
> -                 GVIR_CONFIG_DOMAIN_VIRT_QEMU)
> -            gvir_config_domain_seclabel_set_baselabel(sec, "system_u:system_r:svirt_tcg_t:s0");
> -        else if (gvir_config_domain_get_virt_type(domain) ==
> -                 GVIR_CONFIG_DOMAIN_VIRT_KVM)
> -            gvir_config_domain_seclabel_set_baselabel(sec, "system_u:system_r:svirt_t:s0");
> +        if (!label)
> +            label = get_label(gvir_config_domain_get_virt_type(domain));
> +
> +        gvir_config_domain_seclabel_set_baselabel(sec, label);
> +
>      } else {
>          gvir_config_domain_seclabel_set_type(sec,
>                                               GVIR_CONFIG_DOMAIN_SECLABEL_STATIC);


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 :|