[libvirt] Patch to launch virt-sandbox-containers with correct label.

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

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" def __init__(self, name=None, uri = "lxc:///", path = DEFAULT_PATH, config=None, create=False): self.uri = uri diff --git a/bin/virt-sandbox-service-clone.pod b/bin/virt-sandbox-service-clone.pod index cd261c4..3b4ecec 100644 --- a/bin/virt-sandbox-service-clone.pod +++ b/bin/virt-sandbox-service-clone.pod @@ -42,8 +42,7 @@ separated by commas. The following options are valid for SELinux Dynamically allocate an SELinux label, using the default base context. The default base context is system_u:system_r:svirt_lxc_net_t:s0 for LXC, -system_u:system_r:svirt_t:s0 for KVM, system_u:system_r:svirt_tcg_t:s0 -for QEMU. +system_u:system_r:svirt_qemu_net_t:s0 for KVM or QEMU. =item dynamic,label=USER:ROLE:TYPE:LEVEL @@ -53,7 +52,7 @@ USER:ROLE:TYPE:LEVEL, instead of the default base context. =item static,label=USER:ROLE:TYPE:LEVEL To set a completely static label. For example, -static,label=system_u:system_r:svirt_t:s0:c412,c355 +static,label=system_u:system_r:svirt_lxc_net_t:s0:c412,c355 =back diff --git a/bin/virt-sandbox-service-create.pod b/bin/virt-sandbox-service-create.pod index 2ab289a..4bdca32 100644 --- a/bin/virt-sandbox-service-create.pod +++ b/bin/virt-sandbox-service-create.pod @@ -61,7 +61,7 @@ Default: C<Login GID of UID>. Set SELinux file type to use within container. -Default: C<svirt_lxc_file_t>. +Default: C<svirt_sandbox_file_t>. =item B<-p PATH>, B<--path PATH> @@ -182,8 +182,7 @@ separated by commas. The following options are valid for SELinux Dynamically allocate an SELinux label, using the default base context. The default base context is system_u:system_r:svirt_lxc_net_t:s0 for LXC, -system_u:system_r:svirt_t:s0 for KVM, system_u:system_r:svirt_tcg_t:s0 -for QEMU. +system_u:system_r:svirt_qemu_net_t:s0 for KVM or QEMU. =item dynamic,label=USER:ROLE:TYPE:LEVEL @@ -193,7 +192,7 @@ USER:ROLE:TYPE:LEVEL, instead of the default base context. =item static,label=USER:ROLE:TYPE:LEVEL To set a completely static label. For example, -static,label=system_u:system_r:svirt_t:s0:c412,c355 +static,label=system_u:system_r:svirt_lxc_net_t:s0:c412,c355 =back diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index b16217b..f66b045 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -413,8 +413,7 @@ separated by commas. The following options are valid for SELinux Dynamically allocate an SELinux label, using the default base context. The default base context is system_u:system_r:svirt_lxc_net_t:s0 for LXC, -system_u:system_r:svirt_t:s0 for KVM, system_u:system_r:svirt_tcg_t:s0 -for QEMU. +system_u:system_r:svirt_qemu_net_t:s0 for KVM or QEMU. =item dynamic,label=USER:ROLE:TYPE:LEVEL @@ -424,7 +423,7 @@ USER:ROLE:TYPE:LEVEL, instead of the default base context. =item static,label=USER:ROLE:TYPE:LEVEL To set a completely static label. For example, -static,label=system_u:system_r:svirt_t:s0:c412,c355 +static,label=system_u:system_r:svirt_lxc_net_t:s0:c412,c355 =item inherit 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); + } + + switch (type) { + case GVIR_CONFIG_DOMAIN_VIRT_KVM: + return "system_u:system_r:svirt_qemu_net_t:s0"; + case GVIR_CONFIG_DOMAIN_VIRT_QEMU: + return "system_u:system_r:svirt_qemu_net_t:s0"; + case GVIR_CONFIG_DOMAIN_VIRT_LXC: + default: + return "system_u:system_r:svirt_lxc_net_t:s0"; + } +} 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); -- 1.8.3.1

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

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

On Mon, Sep 30, 2013 at 08:39:35AM -0400, Daniel J Walsh wrote:
-----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);
Your email client has completely mangled this quoted text. Please fix it to preserve line breaks / whitespace, as it makes reading the replies rather difficult.
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.
Yeah, but I'm not seeing how this /etc/selinux/targetted/context/lxc_contexts file content is working with this piece of code. With the updated policy I see $ cat /etc/selinux/targeted/contexts/lxc_contexts process = "system_u:system_r:svirt_lxc_net_t:s0" file = "system_u:object_r:svirt_lxc_file_t:s0" content = "system_u:object_r:virt_var_lib_t:s0" so this code which is looking for 'kvm' and 'qemu' strings in that file isn't doing anything useful 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 :|
participants (3)
-
Dan Walsh
-
Daniel J Walsh
-
Daniel P. Berrange