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

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



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/30/2013 08:07 AM, Daniel P. Berrange wrote:
> 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.
> 
Well we could put the code into check if the type exists else use
svirt_lxc_file_t.  (BTW Aliased in latest code.)
>> 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.
> 
Well the idea is to allow other policy writers to write policy that would use
different types, rather then hard code them into programs.  Dominick Grift is
experimenting with other types of SELinux Policy, and any time he has a hard
coded type, it breaks his code.   Obviously we need to move more types out of
this code to make it fully functional.


>> +    } + +    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.
> 
Ok.
>> +    } +}
>> 
>> 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
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlJJcQcACgkQrlYvE4MpobPFxgCffiA+hIVft0cip0uOg/ac/nTY
NXQAoLpB7b3f9nODTOq6nStA+7gBe6GG
=DKzO
-----END PGP SIGNATURE-----


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