Re: [libvirt] RFC: sVirt disk isolation with network based storage
On 08/20/2014 11:17 AM, Daniel P. Berrange wrote: As everyone knows sVirt is our nice solution to isolating guest resources from other (malicious) guests through SELinux labelling of the appropriate files / device nodes. This has been pretty effective since we introduced it to libvirt. In the last year or two, particularly in the cloud arena, there has been a big shift towards use of network based storage. Initially we were relying on kernel drivers / FUSE layers that exposed this network storage as devices or nodes in the host filesystem, so sVirt still stood a chance of being useful if the devices /FUSE layer supported labelling. Now though QEMU has native support for talking to gluster, ceph/rbd, iscsi and even nfs servers. This support is increasingly used in preference to using the kernel drivers / FUSE layers since it provides a simpler and thus (in theory) better performing I/O path for the network storage and does not require any privileged setup tasks on the host ahead of time. The problem is that I beleive this is blowing a decent sized hole in our sVirt isolation story. eg when we launch QEMU with an argument like this: -drive 'file=rbd:pool/image:auth_supported=none:\ mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\ mon3.example.org\:6322,if=virtio,format=raw' We are trusting QEMU to only ever access the disk volume 'pool/image'. There are, in all likelihood, many 100's or 1000's of disk images on the server it is connecting to and nothing is stopping QEMU from accessing any of them AFAICT. There is no currrently implemented mechanism by which the sVirt label that QEMU runs under is made available to the remote RBD server to use for enforcement, nor any way in which libvirt could tell the RBD server which label was applied for which disk. The same seems to apply for Gluster, iSCSI, and NFS too when accessed directly from a network client inside the QEMU process. As it stands the only approach I see for isolating each virtual machines disk(s) from other virtual machines is to make use of user authentication with these services. eg each virtual machine would need to have its own dedicated user account on the RBD/Gluster/iSCSI/NFS server, and the disk volumes for the VM would have to be made accessible solely to that user account. Assuming such user account / disk mapping exists in the servers today that can be made to work but it is an incredibly awful solution to deal with when VMs are being dynamically created deleted very frequently. Today apps like OpenStack just have a single RBD username and password for everything they do. Any virtual machines running with RBD storage on OpenStack thus have no sVirt protection for their disk images AFAICT. To protect images OpenStack would have to dynamically create delete new user accounts on the RBD server setup disk access for them. I don't see that kind of approach being viable. IIUC, there is some mechanism at the IP stack level where the kernel can take the SELinux label of the process that establishes the network connection and pass it across to the server. If there was a way in the RBD API for libvirt to label the volumes, then potentially we could have a system where the RBD server did sVirt enforcement, based on the instructions from libvirt the label of the client process. Thoughts on what to do about this ? Network based storage, where the network client is inside each QEMU server, is here to stay so I don't think we can ignore the problem long term. Regards, Daniel I think we should setup a meeting to discuss this and figure out our option. We need a mechanism for libvirt to send the labels of the process and images to the remote server and then we need an enforcement mechanism to only allow the process label to interact with the file image. SELinux could do this if each vm has a separate process running on the server interacting with the image. Otherwise the server needs to do some kind of enforcement on its own. We could use some form of labeled networking for transmitting the MCS Label of qemu to the server or we would need to extend the protocol to send the label down. There is two ways to handle labeled networking.The most common labeling standard,CIPSO, only sends the MCS portion of the label. The second form can send the entire label of the process, but it is seldom used and requires Labeled IPSEC. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: sVirt disk isolation with network based storage
Adding Paul Moore since he is the labelled networking expert. On 08/20/2014 11:17 AM, Daniel P. Berrange wrote: As everyone knows sVirt is our nice solution to isolating guest resources from other (malicious) guests through SELinux labelling of the appropriate files / device nodes. This has been pretty effective since we introduced it to libvirt. In the last year or two, particularly in the cloud arena, there has been a big shift towards use of network based storage. Initially we were relying on kernel drivers / FUSE layers that exposed this network storage as devices or nodes in the host filesystem, so sVirt still stood a chance of being useful if the devices /FUSE layer supported labelling. Now though QEMU has native support for talking to gluster, ceph/rbd, iscsi and even nfs servers. This support is increasingly used in preference to using the kernel drivers / FUSE layers since it provides a simpler and thus (in theory) better performing I/O path for the network storage and does not require any privileged setup tasks on the host ahead of time. The problem is that I beleive this is blowing a decent sized hole in our sVirt isolation story. eg when we launch QEMU with an argument like this: -drive 'file=rbd:pool/image:auth_supported=none:\ mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\ mon3.example.org\:6322,if=virtio,format=raw' We are trusting QEMU to only ever access the disk volume 'pool/image'. There are, in all likelihood, many 100's or 1000's of disk images on the server it is connecting to and nothing is stopping QEMU from accessing any of them AFAICT. There is no currrently implemented mechanism by which the sVirt label that QEMU runs under is made available to the remote RBD server to use for enforcement, nor any way in which libvirt could tell the RBD server which label was applied for which disk. The same seems to apply for Gluster, iSCSI, and NFS too when accessed directly from a network client inside the QEMU process. As it stands the only approach I see for isolating each virtual machines disk(s) from other virtual machines is to make use of user authentication with these services. eg each virtual machine would need to have its own dedicated user account on the RBD/Gluster/iSCSI/NFS server, and the disk volumes for the VM would have to be made accessible solely to that user account. Assuming such user account / disk mapping exists in the servers today that can be made to work but it is an incredibly awful solution to deal with when VMs are being dynamically created deleted very frequently. Today apps like OpenStack just have a single RBD username and password for everything they do. Any virtual machines running with RBD storage on OpenStack thus have no sVirt protection for their disk images AFAICT. To protect images OpenStack would have to dynamically create delete new user accounts on the RBD server setup disk access for them. I don't see that kind of approach being viable. IIUC, there is some mechanism at the IP stack level where the kernel can take the SELinux label of the process that establishes the network connection and pass it across to the server. If there was a way in the RBD API for libvirt to label the volumes, then potentially we could have a system where the RBD server did sVirt enforcement, based on the instructions from libvirt the label of the client process. Thoughts on what to do about this ? Network based storage, where the network client is inside each QEMU server, is here to stay so I don't think we can ignore the problem long term. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] virt-login-shell joins users into lxc container.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 12/23/2013 05:44 PM, Eric Blake wrote: On 12/23/2013 03:17 PM, Eric Blake wrote: +if (!(conf = virConfReadFile(login_shell_path, 0))) + goto cleanup; ...and non-root invariably fails here, since login_shell_path (/etc/libvirt/virt-login-shell.conf) is buried inside a directory that is not searchable by either root or virtlogin. Ah, I see - non-root fails here if run unprivileged (such as under gdb), but when run setuid it has the permissions of root and can read the file just fine. Maybe need to give it cap_dac_read_search? /* Overrides all DAC restrictions regarding read and search on files and directories, including ACL restrictions if [_POSIX_ACL] is defined. Excluding DAC access covered by CAP_LINUX_IMMUTABLE. */ #define CAP_DAC_READ_SEARCH 2 Then again, when run as setuid, it's not even getting past virInitialize(). :( At least I managed to figure out how to debug things: I recompiled with a sleep() at the beginning, gave my just-compiled binary the same setuid permissions as the installed binary, and then attach gdb (as root, since non-root can't ptrace a running setuid binary for obvious reasons). So I suspect that the failure in virInitialize() is yet more fallout from the CVE-2013-4400 patches being untested. -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlLFgzgACgkQrlYvE4MpobNyiACfRJWSEAnfiKooQS7ujZnkmAiq +JIAoLmKB5nZl+Nj6QSHww870OOZJhK/ =4uBh -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Be more clever and verbose about localization-initialization.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 11/04/2013 04:11 PM, Eric Blake wrote: On 10/08/2013 01:35 AM, Fuchs, Andreas wrote: I'd argue _for_ starting up libvirtd in case of errorous LC_* info. Since it is not a user-facing application but a system daemon, I think the impact of wrong language is small, but the benefit of having the daemon starting realiably is quite high. I still think that someone setting up the wrong language is a case of admin error, and that failing to start is appropriate. But this issue just caused a second bugzilla today: https://bugzilla.redhat.com/show_bug.cgi?id=1026514 so you aren't the only one to have hit the issue. However, in reading that bug, Dan Walsh (now cc'd) apparently didn't even find the stderr message that tried to alert him to why libvirtd was exiting early. Whether or not we apply your patch, there's the meta-issue that if libvirtd under systemd outputs an error to its stderr, where does that message go and how does an admin find out why libvirtd exited early? Oct 14 17:43:48 redsox.boston.devel.redhat.com systemd[1]: Starting Virtualization daemon... Oct 14 17:43:48 redsox.boston.devel.redhat.com systemd[1]: Started Virtualization daemon. Oct 14 17:43:48 redsox.boston.devel.redhat.com libvirtd[646]: /usr/sbin/libvirtd: initialization failed Oct 14 17:43:48 redsox.boston.devel.redhat.com systemd[1]: libvirtd.service: main process exited, code=exited, status=1/FAILURE Oct 14 17:43:48 redsox.boston.devel.redhat.com systemd[1]: Unit libvirtd.service entered failed state. Oct 14 17:43:48 redsox.boston.devel.redhat.com systemd[1]: libvirtd.service holdoff time over, scheduling restart. Oct 14 17:43:48 redsox.boston.devel.redhat.com systemd[1]: Stopping Virtualization daemon... Oct 14 17:43:48 redsox.boston.devel.redhat.com systemd[1]: Starting Virtualization daemon... Oct 14 17:43:48 redsox.boston.devel.redhat.com systemd[1]: Started Virtualization daemon. This is what I was getting. libvirt initialization failed. Of course when I ran it myself it worked fine. Since the user session had the correct LANG set. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.15 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlJ4EUYACgkQrlYvE4MpobOpPQCfb6oN3Vxw48ccyG2eVBwV3Xks G5sAoIftVHsM1NSq705OB0H+hCpeTFuJ =T8QH -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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) -
Re: [libvirt] [sandbox PATCH 1/2] Add virt-sandbox -s inherit, to execute the sandbox from the parent.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/15/2013 04:48 AM, Daniel P. Berrange wrote: On Tue, Aug 13, 2013 at 01:10:11PM -0400, Dan Walsh wrote: This will allow us to run sandbox as the calling process, If I am running a shell as staff_u:unconfined_r:unconfined_t:s0, and I execute virt-sandbox -c lxc/// -- /bin/sh /bin/sh will run as staff_u:unconfined_r:unconfined_t:s0 --- bin/virt-sandbox-service.pod | 6 +- bin/virt-sandbox.c | 9 - configure.ac | 1 + libvirt-sandbox.spec.in | 1 + libvirt-sandbox/Makefile.am | 2 ++ libvirt-sandbox/libvirt-sandbox-config.c | 14 ++ m4/virt-selinux.m4 | 11 +++ 7 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 m4/virt-selinux.m4 You've taken what was previously 3 separate patches fixing 3 separate bugs, and merged them into one giant patch. This is really bad - separate functional fixes must always be kept as separate patches. The actual changes look good, but please split it back up into 3 separate patches repost. Daniel Not quite sure what you are talking about, I sent two patches, the inherit patch included some fixes to the virt-sandbox-service.pod, which I will split out. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlIMvsUACgkQrlYvE4MpobOhLgCeMWUeZe0Q4QUVbyQ7qEIIdkpO jAsAn1H65pp8mgCfDiF/gUBm7P8rfjgH =jziw -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Patch set to add virt-sandbox -s inherit and fixes for man pages.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 for some reason my git-sendmail keeps failing. [sandbox PATCH 1/3] Add virt-sandbox -s inherit, to execute the [sandbox PATCH 2/3] Add comment about LIBVIRT_DEFAULT_URI to [sandbox PATCH 3/3] virt-sandbox-service.pod did not mention upgrade -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlH7zjAACgkQrlYvE4MpobPBIQCgvOYtY0ccFTUNBNA4tWWQs02t tYwAn15nXX9WhTyG0Piw4QVYwide9/RZ =g+dS -END PGP SIGNATURE- From fcf2e72b78b66075ca5f061423a259e058f4f39d Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Wed, 31 Jul 2013 17:04:58 -0400 Subject: [sandbox PATCH 1/3] Add virt-sandbox -s inherit, to execute the sandbox from the parent. This will allow us to run sandbox as the calling process, If I am running a shell as staff_u:unconfined_r:unconfined_t:s0, and I execute virt-sandbox -c lxc/// -- /bin/sh /bin/sh will run as staff_u:unconfined_r:unconfined_t:s0 --- bin/virt-sandbox.c | 4 libvirt-sandbox/libvirt-sandbox-config.c | 14 ++ 2 files changed, 18 insertions(+) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index b51465d..9a75f3c 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -403,6 +403,10 @@ USER:ROLE:TYPE:LEVEL, instead of the default base context. To set a completely static label. For example, static,label=system_u:system_r:svirt_t:s0:c412,c355 +=item inherit + +Inherit the context from the process that is executing virt-sandbox. + =back =item B-p, B--privileged diff --git a/libvirt-sandbox/libvirt-sandbox-config.c b/libvirt-sandbox/libvirt-sandbox-config.c index ccdb3bc..8e8ac65 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.c +++ b/libvirt-sandbox/libvirt-sandbox-config.c @@ -27,6 +27,8 @@ #include glib/gi18n.h #include libvirt-sandbox/libvirt-sandbox.h +#include errno.h +#include selinux/selinux.h /** * SECTION: libvirt-sandbox-config @@ -1521,6 +1523,18 @@ gboolean gvir_sandbox_config_set_security_opts(GVirSandboxConfig *config, gvir_sandbox_config_set_security_dynamic(config, TRUE); } else if (g_str_equal(tmp, static)) { gvir_sandbox_config_set_security_dynamic(config, FALSE); +} else if (g_str_equal(tmp, inherit)) { +gvir_sandbox_config_set_security_dynamic(config, FALSE); +security_context_t scon; +if (getcon(scon) 0) { +g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, +_(Unable to get SELinux context of user: %s), +strerror(errno)); +return FALSE; +} +gvir_sandbox_config_set_security_label(config, scon); +freecon(scon); + } else { g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, _(Unknown security option '%s'), tmp); -- 1.8.3.1 From f94804786ca1b41d2bb8c58ba04d6412ec49f3ae Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Wed, 31 Jul 2013 17:36:21 -0400 Subject: [sandbox PATCH 2/3] Add comment about LIBVIRT_DEFAULT_URI to virt-sandbox man page --- bin/virt-sandbox.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 9a75f3c..26eefcf 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -280,6 +280,7 @@ not allowed to open any other files. Set the libvirt connection URI, defaults to qemu:///session if omitted. Currently only the QEMU and LXC drivers are supported. +Alternatively the CLIBVIRT_DEFAULT_URI environment variable can be set, or the config file C/etc/libvirt/libvirt.conf can have a default URI set. =item B-n NAME, B--name=NAME -- 1.8.3.1 From af40cc741f69b335975f36801efe91f822a2b8cc Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Thu, 1 Aug 2013 11:09:51 -0400 Subject: [sandbox PATCH 3/3] virt-sandbox-service.pod did not mention upgrade Also still had references to start, stop and list --- bin/virt-sandbox-service.pod | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bin/virt-sandbox-service.pod b/bin/virt-sandbox-service.pod index 32caad9..b317ad4 100644 --- a/bin/virt-sandbox-service.pod +++ b/bin/virt-sandbox-service.pod @@ -4,7 +4,7 @@ virt-sandbox-service - Secure container tool =head1 SYNOPSIS - {create,clone,connect,delete,execute,list,reload,start,stop} + {create,clone,connect,delete,execute,reload,upgrade} commands: @@ -20,6 +20,8 @@ virt-sandbox-service - Secure container tool reload Reload a running sandbox container +upgrade Upgrade the sandbox container + =head1 DESCRIPTION virt-sandbox-service is used to provision secure sandboxed system services. @@ -52,7 +54,7 @@ supported currently). =head1 SEE ALSO -Clibvirt(8), Cselinux(8), Csystemd(8), Cvirt-sandbox(1), Cvirt-sandbox-service-create(1), Cvirt-sandbox-service-clone(1),
[libvirt] Updated patch for virt-login-shell for joing libvirt lxc containers.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 THis patch fixes all of Eric's and Daniels comments. [PATCH] virt-login-shell joins users into lxc container. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlH7zp8ACgkQrlYvE4MpobNx3gCbBtxw7T4fzIfHSyEEKKyjojXR BUUAoOToptiTOi+RC6Bdcp+zvg/xzfRh =7zpw -END PGP SIGNATURE- From 01c7ab48e720f34c2aa891a8fa07812b1c66c316 Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Fri, 28 Jun 2013 13:50:58 -0400 Subject: [PATCH] virt-login-shell joins users into lxc container. Openshift wants to have their gears stuck into a container when they login to the system. virt-login-shell will join a running gear with the username of the person running it, or attempt to start the container if it is not running. (Currently containers do not exist if they are not running, so I can not test this feature. But the code is there). This tool needs to be setuid since joining a container (nsjoin) requires privs. The root user is not allowed to execute this command. When this tool is run by a normal user it will only join the users container. Only users who are listed as valid_users in /etc/libvirt/virt-login-shell.conf are allowed to join containers using this tool. By default no users are allowed. --- .gitignore | 1 + libvirt.spec.in | 3 + po/POTFILES.in | 1 + tools/Makefile.am | 30 +++- tools/virt-login-shell.c| 350 tools/virt-login-shell.conf | 26 tools/virt-login-shell.pod | 62 7 files changed, 472 insertions(+), 1 deletion(-) create mode 100644 tools/virt-login-shell.c create mode 100644 tools/virt-login-shell.conf create mode 100644 tools/virt-login-shell.pod diff --git a/.gitignore b/.gitignore index 738c6ba..d848107 100644 --- a/.gitignore +++ b/.gitignore @@ -215,6 +215,7 @@ /tools/libvirt-guests.init /tools/libvirt-guests.service /tools/libvirt-guests.sh +/tools/virt-login-shell /tools/virsh /tools/virsh-*-edit.c /tools/virt-*-validate diff --git a/libvirt.spec.in b/libvirt.spec.in index fce7f91..fd8f532 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2006,14 +2006,17 @@ fi %doc AUTHORS ChangeLog.gz NEWS README COPYING COPYING.LESSER TODO %config(noreplace) %{_sysconfdir}/libvirt/libvirt.conf +%config(noreplace) %{_sysconfdir}/libvirt/virt-login-shell.conf %{_mandir}/man1/virsh.1* %{_mandir}/man1/virt-xml-validate.1* %{_mandir}/man1/virt-pki-validate.1* %{_mandir}/man1/virt-host-validate.1* +%{_mandir}/man1/virt-login-shell.1* %{_bindir}/virsh %{_bindir}/virt-xml-validate %{_bindir}/virt-pki-validate %{_bindir}/virt-host-validate +%attr(4755, root, root) %{_bindir}/virt-login-shell %{_libdir}/lib*.so.* %if %{with_dtrace} diff --git a/po/POTFILES.in b/po/POTFILES.in index 1fd84af..884b70a 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -231,3 +231,4 @@ tools/virt-host-validate-common.c tools/virt-host-validate-lxc.c tools/virt-host-validate-qemu.c tools/virt-host-validate.c +tools/virt-login-shell.c diff --git a/tools/Makefile.am b/tools/Makefile.am index 644a86d..00c582a 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -37,6 +37,7 @@ EXTRA_DIST = \ virt-pki-validate.in\ virt-sanlock-cleanup.in\ virt-sanlock-cleanup.8\ + virt-login-shell.pod\ virsh.pod \ libvirt-guests.sysconf\ virsh-edit.c \ @@ -52,8 +53,11 @@ EXTRA_DIST = \ DISTCLEANFILES = +confdir = $(sysconfdir)/libvirt +conf_DATA = virt-login-shell.conf + bin_SCRIPTS = virt-xml-validate virt-pki-validate -bin_PROGRAMS = virsh virt-host-validate +bin_PROGRAMS = virsh virt-host-validate virt-login-shell libexec_SCRIPTS = libvirt-guests.sh if WITH_SANLOCK @@ -65,6 +69,7 @@ dist_man1_MANS = \ virt-host-validate.1 \ virt-pki-validate.1 \ virt-xml-validate.1 \ + virt-login-shell.1 \ virsh.1 if WITH_SANLOCK dist_man8_MANS = virt-sanlock-cleanup.8 @@ -128,6 +133,24 @@ virt_host_validate_CFLAGS = \ $(COVERAGE_CFLAGS)\ $(NULL) +virt_login_shell_SOURCES = \ + virt-login-shell.conf\ + virt-login-shell.c + +virt_login_shell_LDFLAGS = $(COVERAGE_LDFLAGS) +virt_login_shell_LDADD = \ + $(STATIC_BINARIES)\ + $(PIE_LDFLAGS) \ + $(RELRO_LDFLAGS) \ + ../src/libvirt.la\ + ../src/libvirt-lxc.la\ + ../gnulib/lib/libgnu.la + +virt_login_shell_CFLAGS = \ + $(WARN_CFLAGS) \ + $(PIE_CFLAGS) \ + $(COVERAGE_CFLAGS) + virsh_SOURCES = \ console.c console.h\ virsh.c virsh.h \ @@ -189,6 +212,11 @@ virsh_win_icon.$(OBJEXT): virsh_win_icon.rc --output-format coff --output $@ endif +virt-login-shell.1: virt-login-shell.pod $(top_srcdir)/configure.ac + $(AM_V_GEN)$(POD2MAN) $ $(srcdir)/$@ \ + if grep 'POD ERROR' $(srcdir)/$@ ; then \ + rm $(srcdir)/$@; exit 1; fi + virsh.1: virsh.pod
Re: [libvirt] Patch set to add virt-sandbox -s inherit and fixes for man pages.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/02/2013 11:51 AM, Daniel P. Berrange wrote: On Fri, Aug 02, 2013 at 11:20:16AM -0400, Daniel J Walsh wrote: for some reason my git-sendmail keeps failing. Here's the wrapper script I used for that $ cat $HOME/usr/bin/git-spam #!/bin/sh dohelp() { echo syntax: $0 TO-ADDR REV-LIST } if [ -z $2 ]; then dohelp; exit 1 fi TO=$1 REV=$2 shift shift git send-email --compose --to $TO --smtp-server=smtp.corp.redhat.com --no-chain-reply-to $REV $@ Assuming you do your work on a branch, then you can just run git-spam libvir-list@redhat.com master.. If you were doing your work on master directly, then you'd have to use git-spam libvir-list@redhat.com origin/master.. Or explicitly specify the starting commit hash. [sandbox PATCH 1/3] Add virt-sandbox -s inherit, to execute the [sandbox PATCH 2/3] Add comment about LIBVIRT_DEFAULT_URI to [sandbox PATCH 3/3] virt-sandbox-service.pod did not mention upgrade From fcf2e72b78b66075ca5f061423a259e058f4f39d Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Wed, 31 Jul 2013 17:04:58 -0400 Subject: [sandbox PATCH 1/3] Add virt-sandbox -s inherit, to execute the sandbox from the parent. This will allow us to run sandbox as the calling process, If I am running a shell as staff_u:unconfined_r:unconfined_t:s0, and I execute virt-sandbox -c lxc/// -- /bin/sh /bin/sh will run as staff_u:unconfined_r:unconfined_t:s0 --- bin/virt-sandbox.c | 4 libvirt-sandbox/libvirt-sandbox-config.c | 14 ++ 2 files changed, 18 insertions(+) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index b51465d..9a75f3c 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -403,6 +403,10 @@ USER:ROLE:TYPE:LEVEL, instead of the default base context. To set a completely static label. For example, static,label=system_u:system_r:svirt_t:s0:c412,c355 +=item inherit + +Inherit the context from the process that is executing virt-sandbox. + =back =item B-p, B--privileged diff --git a/libvirt-sandbox/libvirt-sandbox-config.c b/libvirt-sandbox/libvirt-sandbox-config.c index ccdb3bc..8e8ac65 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.c +++ b/libvirt-sandbox/libvirt-sandbox-config.c @@ -27,6 +27,8 @@ #include glib/gi18n.h #include libvirt-sandbox/libvirt-sandbox.h +#include errno.h +#include selinux/selinux.h /** * SECTION: libvirt-sandbox-config @@ -1521,6 +1523,18 @@ gboolean gvir_sandbox_config_set_security_opts(GVirSandboxConfig *config, gvir_sandbox_config_set_security_dynamic(config, TRUE); } else if (g_str_equal(tmp, static)) { gvir_sandbox_config_set_security_dynamic(config, FALSE); +} else if (g_str_equal(tmp, inherit)) { + gvir_sandbox_config_set_security_dynamic(config, FALSE); + security_context_t scon; +if (getcon(scon) 0) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _(Unable to get SELinux context of user: %s), + strerror(errno)); +return FALSE; +} + gvir_sandbox_config_set_security_label(config, scon); + freecon(scon); Looks good. I wonder if we should also have an explicit 'unconfined' string to simplify life for people who want to run the container entirely unconfined ? eg avoid them needing the verbose -s static,label=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 + } else { g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, _(Unknown security option '%s'), tmp); You'll need to re-add the code to detect selinux in configure, since we had got rid of that previously. eg create m4/virt-selinux.m4 containing AC_DEFUN([LIBVIRT_SANDBOX_SELINUX], [ fail=0 old_LIBS=$LIBS old_CFLAGS=$CFLAGS AC_CHECK_HEADER([selinux/selinux.h],[],[fail=1]) AC_CHECK_LIB([selinux], [fgetfilecon],[],[fail=1]) LIBS=$old_LIBS CFLAGS=$old_CFLAGS test $fail = 1 AC_MSG_ERROR([You must install the libselinux development package in order to compile libvirt-sandbox]) ]) And then add LIBVIRT_SANDBOX_SELINUX to configure.ac, and update libvirt-sandbox/Makefile.am to include SELINUX_CFLAGS and SELINUX_LIBS. And make libvirt-sandbox.spec.in have a BuildRequires: libselinux-devel diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 9a75f3c..26eefcf 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -280,6 +280,7 @@ not allowed to open any other files. Set the libvirt connection URI, defaults to qemu:///session if omitted. Currently only the QEMU and LXC drivers are supported. +Alternatively the CLIBVIRT_DEFAULT_URI environment variable can be set, or the config file C/etc/libvirt/libvirt.conf can have a default URI set. Can you add line wrap at appropriate places From af40cc741f69b335975f36801efe91f822a2b8cc Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Thu, 1 Aug 2013 11:09:51 -0400 Subject: [sandbox PATCH 3/3] virt-sandbox-service.pod did not mention
Re: [libvirt] [PATCH] virt-login-shell joins users into lxc container.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I can't seem to get the error reporting to turn on, what am I doing wrong., if (virInitialize() 0) { fprintf(stderr, _(Failed to initialize libvirt)); return EXIT_FAILURE; } if (virErrorInitialize() 0) { fprintf(stderr, _(Failed to initialize libvirt Error Handling)); return EXIT_FAILURE; } virSetErrorFunc(NULL, NULL); virReportSystemError(EINVAL, %s, _(Test)); And I get no output, I thought I would get error on stderr? -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlHyX7cACgkQrlYvE4MpobNrbQCgmR5hp4Lz9pgCv93V2Fb6r0ec VZsAn13t/fiFqoOEmb6PIb5xa3Gzbr9o =fhdb -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-login-shell joins users into lxc container.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/26/2013 07:40 AM, Daniel P. Berrange wrote: On Fri, Jul 26, 2013 at 07:38:31AM -0400, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I can't seem to get the error reporting to turn on, what am I doing wrong., if (virInitialize() 0) { fprintf(stderr, _(Failed to initialize libvirt)); return EXIT_FAILURE; } if (virErrorInitialize() 0) { fprintf(stderr, _(Failed to initialize libvirt Error Handling)); return EXIT_FAILURE; } virSetErrorFunc(NULL, NULL); virReportSystemError(EINVAL, %s, _(Test)); And I get no output, I thought I would get error on stderr? You would, except that you just turned off printing to stderr by calling virSetErrorFunc in that way. Daniel Am I misreading this? * virSetErrorFunc: * @userData: pointer to the user data provided in the handler callback * @handler: the function to get called in case of error or NULL * * Set a library global error handling function, if @handler is NULL, * it will reset to default printing on stderr. The error raised there * are those for which no handler at the connection level could caught. */ Looks like setting handler to Null reset default printing on stderr? But I am getting no output whether or not I set this. I am attaching a hacked virt-login-shell.c which gives me no output. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlHyozwACgkQrlYvE4MpobMjAACePralBci9M6O0wshnO1+bXXVC a4EAn1/cfC8ng8XlLTO9DpiFetmDr9wv =+h5o -END PGP SIGNATURE- /* * virt-login-shell.c: a shell to connect to a container * * Copyright (C) 2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public * License along with this library. If not, see * http://www.gnu.org/licenses/. * * Daniel Walsh dwa...@redhat.com */ #include stdio.h #include stdlib.h #include virerror.h #define VIR_FROM_THIS VIR_FROM_NONE int main(int argc, char **argv) { virReportSystemError(1, %s %d %s, Test1, argc, argv[0]); if (virInitialize() 0) { fprintf(stderr, _(Failed to initialize libvirt Error Handling)); return EXIT_FAILURE; } virReportSystemError(EINVAL, %s, Test2); if (virErrorInitialize() 0) { fprintf(stderr, _(Failed to initialize libvirt Error Handling)); return EXIT_FAILURE; } virReportSystemError(EINVAL, %s, Test3); virSetErrorFunc(NULL, NULL); virReportSystemError(EINVAL, %s, Test4); virSetErrorLogPriorityFunc(NULL); virReportSystemError(EINVAL, %s, Test5); } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-login-shell joins users into lxc container.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/25/2013 01:23 PM, Eric Blake wrote: On 07/25/2013 11:09 AM, Eric Blake wrote: ACK to this patch. Technically since we're post freeze we shouldn't commit this until 1.1.2, but since this is an entirely new program perhaps we could make an exception here ? Thoughts ? It was posted pre-freeze; the only reason it didn't make freeze was lack of timely review. That said, since the patch adds a setuid binary, I would like to review it as a second set of eyes before we push it. The code was sent to a few reviewers, but I have not heard back, I will resend the latest code. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlHxX4oACgkQrlYvE4MpobMKhgCguv0ydgBjcEkC5Kxf/rt3OAwv u4gAn0bA6VM83MJm91dc5gOSC9gJpdcu =lA4e -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] libvirt patch to write a mcs translation file to /run/setrans directory
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 05/17/2013 05:52 AM, Daniel P. Berrange wrote: On Wed, May 15, 2013 at 02:36:32PM -0400, dwa...@redhat.com wrote: From: Dan Walsh dwa...@redhat.com mcstransd is a translation tool that can translate MCS Labels into human understandable code. I have patched it to watch for translation files in the /run/setrans directory. This allows us to run commands like ps -eZ and see system_u:system_r:svirt_t:Fedora18 rather then system_u:system_r:svirt_t:s0:c1,c2. When used with containers it would make an easy way to list all processes within a container using ps -eZ | grep Fedora18 --- src/security/security_selinux.c | 59 - 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5d108b9..cbcd013 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -83,6 +83,57 @@ virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr, virDomainTPMDefPtr tpm); +static int +virSecuritySELinuxAddMCSFile(const char *name, + const char *label) +{ +int ret = -1; +char *tmp = NULL; + context_t con = NULL; + +if (virAsprintf(tmp, %s/%s, SELINUX_TRANS_DIR, name) 0) { +virReportOOMError(); + return -1; +} +if (! (con = context_new(label))) { + virReportSystemError(errno, %s, + _(unable to allocate security context)); +goto cleanup; +} +if (virFileWriteStr(tmp, context_range_get(con), 0) 0) { + virReportSystemError(errno, + _(unable to create MCS file %s), tmp); +goto cleanup; +} +ret = 0; + +cleanup: +VIR_FREE(tmp); +context_free(con); +return ret; +} + +static int +virSecuritySELinuxRemoveMCSFile(const char *name) +{ + char *tmp=NULL; +int ret = -1; +if (virAsprintf(tmp, %s/%s, SELINUX_TRANS_DIR, name) 0) { +virReportOOMError(); + return -1; +} +if (unlink(tmp) 0 errno != ENOENT) { + virReportSystemError(errno, + _(Unable to remove MCS file %s), tmp); +goto cleanup; +} +ret = 0; + +cleanup: +VIR_FREE(tmp); +return ret; +} + /* * Returns 0 on success, 1 if already reserved, or -1 on fatal error */ @@ -1953,7 +2004,7 @@ virSecuritySELinuxReleaseSecurityLabel(virSecurityManagerPtr mgr, } VIR_FREE(secdef-imagelabel); -return 0; +return virSecuritySELinuxRemoveMCSFile(def-name); } @@ -2047,10 +2098,16 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN return -1; } +if (virSecuritySELinuxAddMCSFile(def-name, secdef-label) 0) { + if (security_getenforce() == 1) +return -1; +} + As you mentioned offlist, this is not going to work because the SetProcessLabel function is called in a child process, where you can't guarantee to see the host's /run directory. Instead it should be done in the GenSecurityLabel function which is called from a safe context. Daniel I did get this to work last night by moving the location of the virSecurityManagerSetProcessLabel to happen in the PivorRoot code before calling lxcContainerMountAllFS Which overmounts the /run directory. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlGWMYQACgkQrlYvE4MpobO9LgCePeIBlJuCTONdoAgeRk11EFE1 saYAnjX5ViWMMTXDI9qDlk59wlE6+3F8 =ju8u -END PGP SIGNATURE- From 3faf3644d44771f49b61fb5cf453d1321f8c0272 Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Thu, 16 May 2013 21:21:05 -0400 Subject: [PATCH 2/2] libvirt writes an mcs translation file to /run/setrans directory mcstransd is a translation tool that can translate MCS Labels into human understandable code. I have patched it to watch for translation files in the /run/setrans directory. This allows us to run commands like ps -eZ and see system_u:system_r:svirt_t:Fedora18 rather then system_u:system_r:svirt_t:s0:c1,c2. When used with containers it would make an easy way to list all processes within a container using ps -eZ | grep Fedora18 --- src/lxc/lxc_container.c | 8 +++--- src/security/security_selinux.c | 57 - 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 48ccc09..cb6ae6a 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1829,6 +1829,10 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, if (lxcContainerPopulateDevices(ttyPaths, nttyPaths) 0) goto cleanup; +VIR_DEBUG(Setting up security labeling); +if (virSecurityManagerSetProcessLabel(securityDriver, vmDef) 0) +goto cleanup; + /* Sets up any non-root mounts from guest config */ if
Re: [libvirt] [PATCH] Change label of fusefs mounted at /proc/meminfo in lxc containers
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 05/16/2013 12:09 PM, Daniel P. Berrange wrote: On Thu, May 16, 2013 at 05:04:06PM +0100, Daniel P. Berrange wrote: On Wed, May 15, 2013 at 10:35:48AM -0400, dwa...@redhat.com wrote: From: Dan Walsh dwa...@redhat.com We do not want to allow contained applications to be able to read fusefs_t. So we want /proc/meminfo label to match the system default proc_t. Fix checking of error codes --- src/lxc/lxc_container.c | 24 1 file changed, 24 insertions(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8e1d10a..8c0edb0 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -52,6 +52,10 @@ # include blkid/blkid.h #endif +#if WITH_SELINUX +# include selinux/selinux.h +#endif + #include virerror.h #include virlog.h #include lxc_container.h @@ -761,6 +765,26 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def, def-name)) 0) return ret; +#if WITH_SELINUX +if (is_selinux_enabled() 0) { + security_context_t scon; +ret = getfilecon(/proc/meminfo, scon); +if (ret 0) { + virReportSystemError(errno, + _(Failed to get security context of %s for /proc/meminfo mount point), + meminfo_path); +return ret; +} +ret = setfilecon(meminfo_path, scon); +freecon(scon); +if (ret 0) { +virReportSystemError(errno, + _(Failed to set security context of %s for /proc/meminfo mount point), + meminfo_path); So I'm not unable to start any containers with this patch applied: 2013-05-16 16:01:47.835+: 1: error : lxcContainerMountProcFuse:787 : Failed to set security context of /.oldroot//run/libvirt/lxc/busy.fuse/meminfo for /proc/meminfo mount point: Operation not supported What distro + kernel version did you test this with ? I'm using current F19 with kernel 3.9.0-0.rc8.git0.2.fc19.x86_64 when I see the failure Indeed, even trying to change it manually fails # chcon system_u:object_r:proc_t:s0 /proc/8180/root/proc/meminfo chcon: failed to change context of ‘/proc/8180/root/proc/meminfo’ to ‘system_u:object_r:proc_t:s0’: Operation not supported Daniel Revert the patch, I could swear this was working before, but it is not now. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlGVSSYACgkQrlYvE4MpobMxAQCgwgqT48d06kj5pnndBcAB+FHn 0TkAn3bBKSduOnIIrNTcTkAOLBtpWjje =gVQV -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Reworked patch set to add UID/GID support for containers
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/19/2013 05:58 AM, Daniel P. Berrange wrote: On Thu, Apr 18, 2013 at 02:34:49PM -0400, dwa...@redhat.com wrote: Combined all UID/GID patches, taken into account Dan Berrange feedback. Now UID will be based off the current UID. Sandbox Shell is no longer used when using LXC containers. Connect will now just execute a shell within the container. [sandbox PATCH 1/6] Add UID/GID support for use with interactive [sandbox PATCH 2/6] We should not turn on the sanbox shell by [sandbox PATCH 3/6] Only create the destination directory if it does What happened to the other 3 patches ? Daniel Should have been a new set, just resent. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlFxK6AACgkQrlYvE4MpobNxgwCfS6DmEPjyBo7kKmCJKuJYsRri VXYAoNpUDQCOECcA6h3YUGtIPKlI+EjU =gl8G -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 11/15] Refactor Container class into Container and ServiceContainer Class.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/05/2013 07:10 AM, Daniel P. Berrange wrote: On Wed, Apr 03, 2013 at 07:17:29PM -0400, Dan Walsh wrote: This way we can share common methods between the ServiceContainer and the InteractiveContainer (Patch to be added) --- bin/virt-sandbox-service | 754 --- 1 file changed, 385 insertions(+), 369 deletions(-) container.set_copy(args.copy) -if args.network: -for net in args.network: -container.add_network(net) +for net in args.network: +container.add_network(net) Hmm, I had the 'if args.network' because this would raise an error about 'args.network' not existing, if no --network args were provided on the command line. Are you sure this still works when no --network args are used ? ACK if this issue is not a problem anymore, or if this chunk is reverted. Daniel This command initializes the network to [], the previous code did not specify a default which implies default=None. My code added a default to an empty list which is why the code above works. - -parser.add_argument(-N, --network, dest=network, - -action=SetNet, - -help=_(Specify the network configuration)) +parser.add_argument(-n, --network, dest=network, +action=SetNet, default=[], +help=_(Specify the network configuration)) -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlFf+4gACgkQrlYvE4MpobMM1ACfZAawIVppXng3pGd3KBxUTEHI B5cAniiPBz6DAglamMzv+wSYK296jj/E =v9ro -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/16] Change all internal functions that use __METHOD to use _METHOD. __METHOD's.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/03/2013 07:28 AM, Daniel P. Berrange wrote: On Tue, Apr 02, 2013 at 06:11:23PM -0400, Dan Walsh wrote: Python makes assumptions about __METHOD names that will break some of the other patches that I am adding, involving inheritance of classes. The _METHODS are treated the same as any methods, but still give maintainers an idea that they should not be used. None of this code is public API, so I'm not sure there's any point in even using '_' - everything is internal. So I'd just remove all use of leading '_' in method names. Daniel I was using them more as an indicator to myself and future maintainers which interfaces to use publicly. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlFcOHYACgkQrlYvE4MpobMdtACggGLOeNQ330fcqzq0tXIAxxin KdEAoLZqMFkYoW4fuAZo8KKJYkB2xyvP =YHFN -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/16] Add InteractiveContainer support. First use case will be OpenShift.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/03/2013 08:47 AM, Daniel P. Berrange wrote: On Tue, Apr 02, 2013 at 06:11:29PM -0400, Dan Walsh wrote: Differentiating on which kind of container to create based off of the --command == InteractiveContainer --unitfile == ServiceContainer Resorted create args to be shown aphabetically except for the --command and --unitfile which I want to come at the end. --- bin/virt-sandbox-service | 139 +-- 1 file changed, 99 insertions(+), 40 deletions(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index f4d0eff..b559cf5 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -413,6 +413,45 @@ class Container: mount = LibvirtSandbox.ConfigMountRam.new(dest, size); self.config.add_mount(mount) +class InteractiveContainer(Container): +def __init__(self, name=None, uri = lxc:///, path = Container.DEFAULT_PATH, config=None, create=False): +Container.__init__(self, name, uri, path, config, create) + +if create: +self.config = LibvirtSandbox.ConfigInteractive.new(name) + +def _gen_filesystems(self): +Container._gen_filesystems(self) + self.add_bind_mount(self.dest, self.path) + +def _create(self): + # +# Create an InteractiveContainer +# + Container.create(self) +self._gen_filesystems() + +if self.image: +self._create_image() + self._umount() +sys.stdout.write(_(Created sandbox container image %s\n) % self.image) +else: + sys.stdout.write(_(Created sandbox container dir %s\n) % self.dest) + self.save_config() + +def create(self): +try: + self._create() +except Exception, e: +try: + self._delete() +except Exception, e2: +pass + raise e + +def set_command(self, command): + self.config.set_command(command) + class ServiceContainer(Container): IGNORE_DIRS= [ /var/run/, /etc/logrotate.d/, /etc/pam.d ] DEFAULT_DIRS = [ /etc, /var ] @@ -836,19 +875,26 @@ MB = int(100) def delete(args): config = read_config(args.name) -container = ServiceContainer(uri=args.uri, config = config) +if isinstance(config, gi.repository.LibvirtSandbox.ConfigInteractive): + container = InteractiveContainer(uri=args.uri, config = config) + else: +container = ServiceContainer(uri=args.uri, config = config) container.delete() def create(args): -container = ServiceContainer(name = args.name, uri=args.uri, create = True) -container.set_copy(args.copy) +if args.command: +container = InteractiveContainer(name = args.name, uri=args.uri, create = True) + container.set_command(args.command.split()) This is bad because it does not take account of shell quoting rules so if you have a command /usr/bin/foo some bar You'll create ['/usr/bin/foo', 'some', 'bar'] @@ -1022,28 +1075,34 @@ def gen_create_args(subparser): parser = subparser.add_parser(create, help=_(create a sandbox container)) +parser.add_argument(-n, --network, dest=network, + action=SetNet, default=[], +help=_(Specify the network configuration)) -parser.add_argument(-N, --network, dest=network, - action=SetNet, -help=_(Specify the network configuration)) You've changed '-N' into '-n' - please don't - the use of '-N' was intentionale to match 'virt-sandbox' command line arg names. -image = parser.add_argument_group(Create On Disk Image File) - - image.add_argument(-i, --imagesize, dest=imagesize, default = None, - action=SizeAction, - help=_(create image of this many megabytes.)) - parser.add_argument(-C, --copy, default=False, - action=store_true, -help=_(copy content from /etc and /var directories that will be mounted within the sandbox)) + ctype = parser.add_argument_group(_(Type of sandbox container to create)) +group = ctype.add_mutually_exclusive_group(required=True) +group.add_argument(-c, --command, + dest=command, default=None, +help=_(Command to run within the sandbox container)) IMHO it is better to make this work like virt-sandbox, where you can pass the command + its args as args on the end of the command line eg instead of virt-sandbox-service create -c /usr/bin/foo bar wizz We should allow virt-sandbox-service create -c /usr/bin/foo bar wizz virt-sandbox-service create -c -- /usr/bin/foo bar wizz so we don't have todo parsing of the -c arg - we just get the string list straight from the sys.argv Daniel Shouldn't the syntax be virt-sandbox-service create foo1 -- /usr/bin/foo bar wizz And virt-sandbox-service create -u httpd.service httpd1 -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird -
[libvirt] This patch adds the label to lxc-enter-namespace
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 lxc-enter-namespace allows a process from outside a container to start a process inside a container. One problem with the current code is the process running within the container would run with the label of the process that created it. For example if the admin process is running as unconfined_t and executes the following command # virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 29 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 47 ? 00:00:00 ps Note the ps command is running as unconfined_t, After this patch, virsh -c lxc:/// lxc-enter-namespace dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 38 ? 00:00:00 ps I also add a --nolabel command to virsh, which can go back to the original behaviour. virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 37 ? 00:00:00 ps One problem I had when I originally did the patch is lxcDomainGetSecurityLabel was returning the incorrect label, I needed the label of the initpid within the container not its parent process, so I changed this function to match OpenNamespaces function. One last strangeness, about half the time I run this, virsh hangs and never returns. Seems like if (conn-driver-domainGetSecurityLabel(domain, seclabel) == 0) { Gets hung up. I have attached the strace in out1.gz -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlE476kACgkQrlYvE4MpobMW8QCeMwkx5uzMgQdJbNqnyiOa62+Y JNIAnA8ZZRhjlqMIRAy5/RbMc1g3Wxv1 =cSsv -END PGP SIGNATURE- From e231257274c4716ea69a630b5613b91e04ebc813 Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Tue, 22 Jan 2013 17:27:31 -0500 Subject: [PATCH 6/6] Set the label by default when entering a namespace, when using SELinux. Any lxc process that runs within the namespace should be running with the context of the namespace. I also added a --nolabel qualifier to virsh so you can run an unconfined_t label in the namespace and not transition. --- include/libvirt/libvirt-lxc.h | 4 src/libvirt-lxc.c | 44 ++- tools/virsh-domain.c | 8 +++- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-lxc.h b/include/libvirt/libvirt-lxc.h index f2c87fb..2fad400 100644 --- a/include/libvirt/libvirt-lxc.h +++ b/include/libvirt/libvirt-lxc.h @@ -32,6 +32,10 @@ extern C { # endif +typedef enum { +SECURITY_LABEL = 1 +} NamespaceFlags; + int virDomainLxcOpenNamespace(virDomainPtr domain, int **fdlist, unsigned int flags); diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c index f580c3c..2972721 100644 --- a/src/libvirt-lxc.c +++ b/src/libvirt-lxc.c @@ -29,6 +29,9 @@ #include virlog.h #include virprocess.h #include datatypes.h +#ifdef WITH_SELINUX +#include selinux/selinux.h +#endif #define VIR_FROM_THIS VIR_FROM_NONE @@ -103,6 +106,36 @@ error: return -1; } +static int +virDomainSetDefaultSecurityLabel(virDomainPtr domain) +{ +int rc = 0; +virSecurityLabelPtr seclabel; +if (VIR_ALLOC(seclabel) 0) +return -1; + +if (virDomainGetSecurityLabel(domain, seclabel)) +return -1; + +#ifdef WITH_SELINUX +VIR_DEBUG(setexeccon %s, seclabel-label); +rc = setexeccon(seclabel-label); +if (rc) +VIR_ERROR(_(Failed to set security context to %s), seclabel-label); +#endif + +VIR_FREE(seclabel); + +return rc; +} + +static void +virDomainResetSecurityLabel(void) +{ +#ifdef WITH_SELINUX +(void) setexeccon(NULL); +#endif +} /** * virDomainLxcEnterNamespace: @@ -135,7 +168,12 @@ virDomainLxcEnterNamespace(virDomainPtr domain, { int i; -virCheckFlags(0, -1); +
Re: [libvirt] This patch adds the label to lxc-enter-namespace
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 01/25/2013 02:39 PM, Daniel J Walsh wrote: (2nd pass) lxc-enter-namespace allows a process from outside a container to start a process inside a container. One problem with the current code is the process running within the container would run with the label of the process that created it. For example if the admin process is running as unconfined_t and executes the following command # virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 29 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 47 ? 00:00:00 ps Note the ps command is running as unconfined_t, After this patch, virsh -c lxc:/// lxc-enter-namespace dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 38 ? 00:00:00 ps I also add a --nolabel command to virsh, which can go back to the original behaviour. virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 37 ? 00:00:00 ps Everything seems to be working perfectly now. Any comment on this? -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlEL6iwACgkQrlYvE4MpobN4lACfZF6cBMngf7e9jJGuNkH9HfXC tiAAoKNC7IuHy5yNrnwKmtS104FeryVl =N0pN -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] These patches needed to mount the securityfs in containers.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Simple patch to make sure /sys/kernel/security is mounted inside the container. Systemd attempts to use/mount this file system if it is not present. One of these days I will figure out how to merge patches. securityfs has to be mounted after /sys... Also want to mount it readonly. (Sent patches to wrong list, originally) -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlEL6yUACgkQrlYvE4MpobOTuwCfWkrq/wiPQKPG4y69fIhjDqqr riQAnis3qdRKzRJIpB4PbPGXTgR3nrKt =D+T5 -END PGP SIGNATURE- From 502f11954550bdd67fdc4b668f7ed2317449 Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Tue, 6 Nov 2012 13:26:50 -0500 Subject: [PATCH 2/5] Add securityfs mounted on /sys/kernel/security for containers --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8faa664..e06313e 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -521,6 +521,7 @@ static int lxcContainerMountBasicFS(bool pivotRoot, { proc, /proc, proc, NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV }, { /proc/sys, /proc/sys, NULL, NULL, MS_BIND }, { /proc/sys, /proc/sys, NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, +{ securityfs, /sys/kernel/security, securityfs, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, { sysfs, /sys, sysfs, NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV }, { sysfs, /sys, sysfs, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, #if HAVE_SELINUX -- 1.8.0 From ead9b3e6f81eccb133b7cca5ef0b83595f5aa132 Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Tue, 6 Nov 2012 15:07:21 -0500 Subject: [PATCH 3/5] Allow lxc_container to mount securityfs within the container --- src/lxc/lxc_container.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 17f685d..9030c27 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -521,9 +521,10 @@ static int lxcContainerMountBasicFS(bool pivotRoot, { proc, /proc, proc, NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV }, { /proc/sys, /proc/sys, NULL, NULL, MS_BIND }, { /proc/sys, /proc/sys, NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, -{ securityfs, /sys/kernel/security, securityfs, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, { sysfs, /sys, sysfs, NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV }, { sysfs, /sys, sysfs, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, +{ securityfs, /sys/kernel/security, securityfs, NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV }, +{ securityfs, /sys/kernel/security, securityfs, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, #if HAVE_SELINUX { SELINUX_MOUNT, SELINUX_MOUNT, selinuxfs, NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV }, { SELINUX_MOUNT, SELINUX_MOUNT, NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, -- 1.8.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] This patch adds the label to lxc-enter-namespace
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 (2nd pass) lxc-enter-namespace allows a process from outside a container to start a process inside a container. One problem with the current code is the process running within the container would run with the label of the process that created it. For example if the admin process is running as unconfined_t and executes the following command # virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 29 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 47 ? 00:00:00 ps Note the ps command is running as unconfined_t, After this patch, virsh -c lxc:/// lxc-enter-namespace dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 38 ? 00:00:00 ps I also add a --nolabel command to virsh, which can go back to the original behaviour. virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 37 ? 00:00:00 ps Everything seems to be working perfectly now. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlEC320ACgkQrlYvE4MpobPDjwCfTLjGarwDOLA333vE+XVp0Zj2 LYkAn3WGX3h309/kJejbE3uvXnIUCKJV =rwfM -END PGP SIGNATURE- diff --git a/include/libvirt/libvirt-lxc.h b/include/libvirt/libvirt-lxc.h index f2c87fb..257637b 100644 --- a/include/libvirt/libvirt-lxc.h +++ b/include/libvirt/libvirt-lxc.h @@ -43,6 +43,9 @@ int virDomainLxcEnterNamespace(virDomainPtr domain, int **oldfdlist, unsigned int flags); +int virDomainLxcGetSecurityLabel(virDomainPtr domain, + virSecurityLabelPtr seclabel); + # ifdef __cplusplus } # endif diff --git a/python/generator.py b/python/generator.py index f853d77..f98818e 100755 --- a/python/generator.py +++ b/python/generator.py @@ -551,6 +551,7 @@ skip_function = ( lxc_skip_function = ( virDomainLxcEnterNamespace, + virDomainLxcGetSecurityLabel, ) qemu_skip_function = ( #virDomainQemuAttach, diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c index f580c3c..a4aff59 100644 --- a/src/libvirt-lxc.c +++ b/src/libvirt-lxc.c @@ -41,6 +41,57 @@ __LINE__, info) /** + * virDomainLxcGetSecurityLabel: + * @domain: a domain object + * @seclabel: pointer to a virSecurityLabel structure + * + * This API is LXC specific, so it will only work with hypervisor + * connections to the LXC driver. + * + * Get the security label associated with the container @domain. + * + * Returns 0 on success, or -1 on error + */ +int +virDomainLxcGetSecurityLabel(virDomainPtr domain, + virSecurityLabelPtr seclabel) +{ +virConnectPtr conn; + +VIR_DEBUG(domain=%p, domain); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; +} + +conn = domain-conn; + +if (conn-flags VIR_CONNECT_RO) { + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; +} + +if (conn-driver-domainGetSecurityLabel) { + + if (conn-driver-domainGetSecurityLabel(domain, + seclabel) 0) + goto error; + + return 0; +} + +virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(conn); +return -1; +} + +/** * virDomainLxcOpenNamespace: * @domain: a domain object * @fdlist: pointer to an array to be filled with FDs diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1fe8039..fd60712 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1125,6 +1125,7 @@ static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secla { virLXCDriverPtr driver = dom-conn-privateData; virDomainObjPtr vm; +virLXCDomainObjPrivatePtr priv; int ret = -1; lxcDriverLock(driver); @@ -1162,8 +1163,14 @@ static int
[libvirt] This patch adds the label to lxc-enter-namespace
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 (Resend to the correct list) lxc-enter-namespace allows a process from outside a container to start a process inside a container. One problem with the current code is the process running within the container would run with the label of the process that created it. For example if the admin process is running as unconfined_t and executes the following command # virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 29 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 47 ? 00:00:00 ps Note the ps command is running as unconfined_t, After this patch, virsh -c lxc:/// lxc-enter-namespace dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 38 ? 00:00:00 ps I also add a --nolabel command to virsh, which can go back to the original behaviour. virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 37 ? 00:00:00 ps One problem I had when I originally did the patch is lxcDomainGetSecurityLabel was returning the incorrect label, I needed the label of the initpid within the container not its parent process, so I changed this function to match OpenNamespaces function. One last strangeness, about half the time I run this, virsh hangs and never returns. Seems like if (conn-driver-domainGetSecurityLabel(domain, seclabel) == 0) { Gets hung up. I have attached the strace in out1.gz -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlEBO2EACgkQrlYvE4MpobMS5ACg3Ih4Iu0lD9BofF4iP0QXarAL jpQAoLyWWNhnnFw2TRDJsXqvrTTVujyZ =hUZ/ -END PGP SIGNATURE- From e231257274c4716ea69a630b5613b91e04ebc813 Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Tue, 22 Jan 2013 17:27:31 -0500 Subject: [PATCH 6/6] Set the label by default when entering a namespace, when using SELinux. Any lxc process that runs within the namespace should be running with the context of the namespace. I also added a --nolabel qualifier to virsh so you can run an unconfined_t label in the namespace and not transition. --- include/libvirt/libvirt-lxc.h | 4 src/libvirt-lxc.c | 44 ++- tools/virsh-domain.c | 8 +++- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-lxc.h b/include/libvirt/libvirt-lxc.h index f2c87fb..2fad400 100644 --- a/include/libvirt/libvirt-lxc.h +++ b/include/libvirt/libvirt-lxc.h @@ -32,6 +32,10 @@ extern C { # endif +typedef enum { +SECURITY_LABEL = 1 +} NamespaceFlags; + int virDomainLxcOpenNamespace(virDomainPtr domain, int **fdlist, unsigned int flags); diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c index f580c3c..2972721 100644 --- a/src/libvirt-lxc.c +++ b/src/libvirt-lxc.c @@ -29,6 +29,9 @@ #include virlog.h #include virprocess.h #include datatypes.h +#ifdef WITH_SELINUX +#include selinux/selinux.h +#endif #define VIR_FROM_THIS VIR_FROM_NONE @@ -103,6 +106,36 @@ error: return -1; } +static int +virDomainSetDefaultSecurityLabel(virDomainPtr domain) +{ +int rc = 0; +virSecurityLabelPtr seclabel; +if (VIR_ALLOC(seclabel) 0) +return -1; + +if (virDomainGetSecurityLabel(domain, seclabel)) +return -1; + +#ifdef WITH_SELINUX +VIR_DEBUG(setexeccon %s, seclabel-label); +rc = setexeccon(seclabel-label); +if (rc) +VIR_ERROR(_(Failed to set security context to %s), seclabel-label); +#endif + +VIR_FREE(seclabel); + +return rc; +} + +static void +virDomainResetSecurityLabel(void) +{ +#ifdef WITH_SELINUX +(void) setexeccon(NULL); +#endif +} /** * virDomainLxcEnterNamespace: @@ -135,7 +168,12 @@ virDomainLxcEnterNamespace(virDomainPtr domain, { int i; -
Re: [libvirt] [PATCH] Log an audit message with the LXC init pid
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 11/20/2012 12:52 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Currently the LXC driver logs audit messages when a container is started or stopped. These audit messages, however, contain the PID of the libvirt_lxc supervisor process. To enable sysadmins to correlate with audit messages generated by processes /inside/ the container, we need to include the container init process PID. We can't do this in the main 'start' audit message, since the init PID is not available at that point. Instead we output a completely new audit record, that lists both PIDs. type=VIRT_CONTROL msg=audit(1353433750.071:363): pid=20180 uid=0 auid=501 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='virt=lxc op=init vm=busy uuid=dda7b947-0846-1759-2873-0f375df7d7eb vm-pid=20371 init-pid=20372 exe=/home/berrange/src/virt/libvirt/daemon/.libs/lt-libvirtd hostname=? addr=? terminal=pts/6 res=success' --- src/conf/domain_audit.c | 26 ++ src/conf/domain_audit.h | 3 +++ src/libvirt_private.syms | 1 + src/lxc/lxc_controller.c | 31 ++- src/lxc/lxc_monitor.c| 23 +++ src/lxc/lxc_monitor.h| 5 + src/lxc/lxc_process.c| 8 src/lxc/lxc_protocol.x | 7 ++- 8 files changed, 102 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 0f3924a..34cd92e 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -605,6 +605,32 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) virDomainAuditLifecycle(vm, start, reason, success); } +void +virDomainAuditInit(virDomainObjPtr vm, + unsigned long long initpid) +{ +char uuidstr[VIR_UUID_STRING_BUFLEN]; +char *vmname; +const char *virt; + +virUUIDFormat(vm-def-uuid, uuidstr); + +if (!(vmname = virAuditEncode(vm, vm-def-name))) { + VIR_WARN(OOM while encoding audit message); +return; +} + + if (!(virt = virDomainVirtTypeToString(vm-def-virtType))) { + VIR_WARN(Unexpected virt type %d while encoding audit message, vm-def-virtType); +virt = ?; +} + + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, true, + virt=%s op=init %s uuid=%s vm-pid=%lld init-pid=%lld, + virt, vmname, uuidstr, (long long)vm-pid, initpid); + +VIR_FREE(vmname); +} void virDomainAuditStop(virDomainObjPtr vm, const char *reason) diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index db35d09..94b1198 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -31,6 +31,9 @@ void virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void virDomainAuditInit(virDomainObjPtr vm, +unsigned long long pid) +ATTRIBUTE_NONNULL(1); void virDomainAuditStop(virDomainObjPtr vm, const char *reason) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5a07139..23369e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -258,6 +258,7 @@ virDomainAuditCgroupPath; virDomainAuditDisk; virDomainAuditFS; virDomainAuditHostdev; +virDomainAuditInit; virDomainAuditMemory; virDomainAuditNet; virDomainAuditNetDevice; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 4746897..65d117a 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -123,6 +123,7 @@ struct _virLXCController { /* Server socket */ virNetServerPtr server; +bool firstClient; virNetServerClientPtr client; virNetServerProgramPtr prog; bool inShutdown; @@ -132,6 +133,8 @@ struct _virLXCController { #include lxc_controller_dispatch.h static void virLXCControllerFree(virLXCControllerPtr ctrl); +static int virLXCControllerEventSendInit(virLXCControllerPtr ctrl, + pid_t initpid); static void virLXCControllerQuitTimer(int timer ATTRIBUTE_UNUSED, void *opaque) { @@ -152,6 +155,7 @@ static virLXCControllerPtr virLXCControllerNew(const char *name) goto no_memory; ctrl-timerShutdown = -1; +ctrl-firstClient = true; if (!(ctrl-name = strdup(name))) goto no_memory; @@ -588,6 +592,11 @@ static void *virLXCControllerClientPrivateNew(virNetServerClientPtr client, virNetServerClientSetCloseHook(client, virLXCControllerClientCloseHook); VIR_DEBUG(Got new client %p, client); ctrl-client = client; + +if (ctrl-initpid ctrl-firstClient) + virLXCControllerEventSendInit(ctrl, ctrl-initpid); +ctrl-firstClient = false; + return dummy; } @@ -1283,8 +1292,10 @@ virLXCControllerEventSend(virLXCControllerPtr ctrl, { virNetMessagePtr msg; -if (!ctrl-client) +if (!ctrl-client) { + VIR_WARN(Dropping event %d becuase libvirtd is not connected, procnr); return; +} VIR_DEBUG(Send event %d
Re: [libvirt] [virt-devel] This patch removes the mknod capability from Linux Containers.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 11/01/2012 04:07 PM, Eric Blake wrote: [originally posted to the wrong list] On 11/01/2012 12:57 PM, Daniel J Walsh wrote: 0001-Linux-Containers-are-not-allowed-to-create-device-no.patch From 3913ef4148728430cc9df79b84d5ec44130f4ac8 Mon Sep 17 00:00:00 2001 From: rhatdan dwa...@redhat.com I'll adjust the author attribution to match other patches of yours (we generally prefer 'git shortlog' to list full names). Date: Thu, 1 Nov 2012 14:54:39 -0400 Subject: [PATCH] Linux Containers are not allowed to create device nodes. This needs to be done before the container starts. Turning off the mknod capabilty is noticed by systemd, which will s/capabilty/capability/ no longer attempt to create device nodes. Missing a blank line, so 'git log' tries to treat this as a really long subject line. This eliminates SELinux AVC messages and ugly failure messages in the journal. --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 2789c17..8faa664 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1717,6 +1717,7 @@ static int lxcContainerDropCapabilities(bool keepReboot ATTRIBUTE_UNUSED) CAPNG_INHERITABLE | CAPNG_BOUNDING_SET, CAP_SYS_MODULE, /* No kernel module loading */ CAP_SYS_TIME, /* No changing the clock */ + CAP_MKNOD, /* No creating device nodes */ CAP_AUDIT_CONTROL, /* No messing with auditing status */ CAP_MAC_ADMIN, /* No messing with LSM config */ keepReboot ? -1 : CAP_SYS_BOOT, /* No use of reboot */ Makes sense to me. ACK; I'll clean it up and push in time for 1.0.0. Thanks, sorry about the git problems. Not sure where it is getting rhatdan from. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCS4y4ACgkQrlYvE4MpobOxtACgyk8NswhXnUM4ZAFvVfLETsPI a/0Anj3YzHDqYpJW8EibFHYXq9ugXzZf =exMM -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] selinux: Don't fail RestoreAll if file doesn't have a default label
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/23/2012 10:55 AM, Cole Robinson wrote: On 10/23/2012 06:56 AM, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/22/2012 04:13 PM, Cole Robinson wrote: On 10/22/2012 11:51 AM, Eric Blake wrote: On 10/21/2012 02:44 PM, Cole Robinson wrote: When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO. Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling. Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 1 file changed, 4 insertions(+) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) } if (getContext(newpath, buf.st_mode, fcon) 0) { +/* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN(cannot lookup default selinux label for %s, newpath); + rc = 0; In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact? I don't know if we can just 'remove' a label, we have to replace it with a different label, right? If I create a file under /mnt/foo the catch all label is unconfined_u:object_r:file_t:s0 but not sure if we can hardcode that. dwalsh, is there a way to programmatically determine the fallback default label? - Cole Another option would be to get the label before the virtual machine starts and then restore it to the old label, if we have kept the label around. That would take some work. I think it's fair to say that if users have a file with a non default label, and they want it restored after svirt usage, they just install their own custom policy so our equivalent of 'restorecon' does the right thing. - Cole One other option would be to grab the label before you change it and then set it back you are done. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCH6WsACgkQrlYvE4MpobPnQACfWXTb/5Gff2V012e9wdN8B7Xs AxEAoNeEzC/tdVOyLS/d8ZD6W0wkIgY6 =h9TN -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] selinux: Don't fail RestoreAll if file doesn't have a default label
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/22/2012 04:13 PM, Cole Robinson wrote: On 10/22/2012 11:51 AM, Eric Blake wrote: On 10/21/2012 02:44 PM, Cole Robinson wrote: When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO. Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling. Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 1 file changed, 4 insertions(+) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) } if (getContext(newpath, buf.st_mode, fcon) 0) { +/* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN(cannot lookup default selinux label for %s, newpath); +rc = 0; In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact? I don't know if we can just 'remove' a label, we have to replace it with a different label, right? If I create a file under /mnt/foo the catch all label is unconfined_u:object_r:file_t:s0 but not sure if we can hardcode that. dwalsh, is there a way to programmatically determine the fallback default label? - Cole Another option would be to get the label before the virtual machine starts and then restore it to the old label, if we have kept the label around. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCGd9kACgkQrlYvE4MpobOTqgCfX7IlWThWpHRbPIopZ4BUIerB e/MAn2YNe6wBNA6X7OGjDHqIqGv2E+7B =78kR -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] selinux: Don't fail RestoreAll if file doesn't have a default label
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/22/2012 04:13 PM, Cole Robinson wrote: On 10/22/2012 11:51 AM, Eric Blake wrote: On 10/21/2012 02:44 PM, Cole Robinson wrote: When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO. Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling. Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 1 file changed, 4 insertions(+) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) } if (getContext(newpath, buf.st_mode, fcon) 0) { +/* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN(cannot lookup default selinux label for %s, newpath); +rc = 0; In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact? I don't know if we can just 'remove' a label, we have to replace it with a different label, right? If I create a file under /mnt/foo the catch all label is unconfined_u:object_r:file_t:s0 but not sure if we can hardcode that. dwalsh, is there a way to programmatically determine the fallback default label? - Cole I would guess you could walk the stack until you got a file system with a label. dirs(newpath) ... For example in /mnt/a/b/c Check /mnt/a/b/c, then /mnt/a/b then /mnt/a then /mnt then / It should definitely not set it to file_t. I will send this question off to the selinux list to see if they have any better suggestions. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCGd0YACgkQrlYvE4MpobORcACgg8Ctj7tX5qtlSEtyOzaOArvw rWUAniGx8b73oIhFJpssbid3oexmO46i =Vz87 -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Patch to default selinuxfs mount point to /sys/fs/selinux
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Currently if you build on a machine that does not support SELinux we end up with the default mount point being /selinux, since this is moved to /sys/fs/selinux, we should start defaulting there. I believe this is causing a bug in libvirt-lxc when /selinux does not exists, even though /sys/fs/selinux exists. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlBl6F8ACgkQrlYvE4MpobPz0ACgxPdENx/KDtQY7YGT7BDXoLP3 AVIAoJwUvTib72U0VJ3dnVoGU0PYjMXZ =XU8L -END PGP SIGNATURE- diff --git a/.gnulib b/.gnulib index 440a1db..dbd9144 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42 +Subproject commit dbd914496c99c52220e5f5ba4121d6cb55fb3beb diff --git a/configure.ac b/configure.ac index ae26de7..5cc5cbe 100644 --- a/configure.ac +++ b/configure.ac @@ -1455,11 +1455,10 @@ fi if test $with_selinux = yes; then AC_MSG_CHECKING([SELinux mount point]) if test $with_selinux_mount = check || test -z $with_selinux_mount; then -if test -d /sys/fs/selinux ; then - SELINUX_MOUNT=/sys/fs/selinux -else - SELINUX_MOUNT=/selinux -fi + SELINUX_MOUNT=/sys/fs/selinux + if ! test -d ${SELINUX_MOUNT} test -d /selinux ; then + SELINUX_MOUNT=/selinux + fi else SELINUX_MOUNT=$with_selinux_mount fi -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] Honour current process label when generating SELinux labels
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/16/2012 11:41 AM, Viktor Mihajlovski wrote: On 08/10/2012 03:47 PM, Daniel P. Berrange wrote: This patch series makes a number of changes to the SELinux label generation code. This is intended to make it fully honour the current process label when generating VM labels, so that dynamic label generation works better with custom policies, or confined user accounts. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Unfortunately I am not selinux-savvy enough to understand exactly why, but I cannot start guests any more after pulling master. The issue is that the virtual disk's security context (a block device in this case) cannot be set, message shown below. 012-08-16 15:02:18.891+: 1536: error : virSecuritySELinuxSetFileconHelper:652 : unable to set security context 'system_u:system_r:svirt_image_t:s0:c786,c986' on '/dev/disk/by-path/ccw-0.0.3770-part1': Invalid argument Prior to that the security context would have looked like this system_u:object_r:svirt_image_t:s0:c153,c923, i.e. using object_r instead of system_r. I am running on RHEL 6.2, not sure whether this is relevant. Yes the security context should be system_u:object_r:svirt_image_t:s0:c786,c986 These patches should have just affected the Process label not the file label. On the file label we should alter the role on the file label to include object_r. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlAtMVIACgkQrlYvE4MpobMYqQCgz+d7yeXhYXTz0IGFIsRYUqJl GGgAniHHX21m7D5BHZgeMHskS8zww4B1 =Ex2S -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] I missed this tmpfs when I built the previous patch
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 We are mounting a tmpfs before mounting the cgroup file systems, we need to make sure this tmpfs is labeled correctly. This patch fixes the problem, Hopefully formatted correctly, it did pass the syntax check. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlAHERUACgkQrlYvE4MpobMkEwCfRjadP1RmB5YtvkQnwX11KTDN d5kAnjdI6VHNh3v4TopAsEWe73gcT9Ii =V8DO -END PGP SIGNATURE- From 53052da4966b003f3f7e2f3a23097050bc6091d7 Mon Sep 17 00:00:00 2001 From: Dan Walsh dwa...@redhat.com Date: Wed, 18 Jul 2012 15:36:20 -0400 Subject: [PATCH 12/12] lxcContainerMountCGroups also mounts a tmpfs that needs to be labeled. This patch passes down the sec_mount_options to the lxcContainerMountCGroups function and then mounts the tmpfs with the correct label. --- src/lxc/lxc_container.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 92a3bf9..6fdf359 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1341,9 +1341,11 @@ cleanup: static int lxcContainerMountCGroups(struct lxcContainerCGroup *mounts, -size_t nmounts) +size_t nmounts, +char * sec_mount_options) { size_t i; +char *opts = NULL; VIR_DEBUG(Mounting cgroups at '%s', VIR_CGROUP_SYSFS_MOUNT); @@ -1354,12 +1356,20 @@ static int lxcContainerMountCGroups(struct lxcContainerCGroup *mounts, return -1; } -if (mount(tmpfs, VIR_CGROUP_SYSFS_MOUNT, tmpfs, MS_NOSUID|MS_NODEV|MS_NOEXEC, mode=755) 0) { +if (virAsprintf(opts, +mode=755,size=65536%s,(sec_mount_options ? sec_mount_options : )) 0 ) { +virReportOOMError(); +return -1; +} + +if (mount(tmpfs, VIR_CGROUP_SYSFS_MOUNT, tmpfs, MS_NOSUID|MS_NODEV|MS_NOEXEC, opts) 0) { +VIR_FREE(opts); virReportSystemError(errno, _(Failed to mount %s on %s type %s), tmpfs, VIR_CGROUP_SYSFS_MOUNT, tmpfs); return -1; } +VIR_FREE(opts); for (i = 0 ; i nmounts ; i++) { if (mounts[i].linkDest) { @@ -1433,7 +1443,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, /* Now we can re-mount the cgroups controllers in the * same configuration as before */ -if (lxcContainerMountCGroups(mounts, nmounts) 0) +if (lxcContainerMountCGroups(mounts, nmounts, sec_mount_options) 0) goto cleanup; /* Mounts /dev/pts */ @@ -1512,7 +1522,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, /* Now we can re-mount the cgroups controllers in the * same configuration as before */ -if (lxcContainerMountCGroups(mounts, nmounts) 0) +if (lxcContainerMountCGroups(mounts, nmounts, sec_mount_options) 0) goto cleanup; VIR_DEBUG(Mounting completed); -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] We need this patch to mount all tmpfs file systems with the correct context.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Basically within a Secure Linux Container (virt-sandbox) we want all content that the process within the container can write to be labeled the same. We are labeling the physical disk correctly but when we create RAM based file systems libvirt is not labeling them, and they are defaulting to tmpfs_t, which will will not allow the processes to write. This patch labels the RAM based file systems correctly. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlAFiTwACgkQrlYvE4MpobPA1ACghFq3nxmNmHP/WEq1vSwjtoin VFoAnAlxgPISuIPiAPSFUL0CjiiSXDzw =xFCU -END PGP SIGNATURE- diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 145accb..b9bae36 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -425,9 +425,8 @@ err: } -static int lxcContainerMountBasicFS(virDomainDefPtr def, -bool pivotRoot, -virSecurityManagerPtr securityDriver) +static int lxcContainerMountBasicFS(bool pivotRoot, +char *sec_mount_options) { const struct { const char *src; @@ -493,10 +492,8 @@ static int lxcContainerMountBasicFS(virDomainDefPtr def, * and don't want to DOS the entire OS RAM usage */ -char *mount_options = virSecurityManagerGetMountOptions(securityDriver, def); ignore_value(virAsprintf(opts, - mode=755,size=65536%s,(mount_options ? mount_options : ))); -VIR_FREE(mount_options); + mode=755,size=65536%s,(sec_mount_options ? sec_mount_options : ))); if (!opts) { virReportOOMError(); goto cleanup; @@ -1001,12 +998,14 @@ cleanup: } -static int lxcContainerMountFSTmpfs(virDomainFSDefPtr fs) +static int lxcContainerMountFSTmpfs(virDomainFSDefPtr fs, +char *sec_mount_options) { int ret = -1; char *data = NULL; -if (virAsprintf(data, size=%lldk, fs-usage) 0) { +if (virAsprintf(data, +size=%lldk%s, fs-usage, (sec_mount_options ? sec_mount_options : )) 0) { virReportOOMError(); goto cleanup; } @@ -1043,7 +1042,8 @@ cleanup: static int lxcContainerMountFS(virDomainFSDefPtr fs, - const char *srcprefix) + const char *srcprefix, + char *sec_mount_options) { switch (fs-type) { case VIR_DOMAIN_FS_TYPE_MOUNT: @@ -1055,7 +1055,7 @@ static int lxcContainerMountFS(virDomainFSDefPtr fs, return -1; break; case VIR_DOMAIN_FS_TYPE_RAM: -if (lxcContainerMountFSTmpfs(fs) 0) +if (lxcContainerMountFSTmpfs(fs, sec_mount_options) 0) return -1; break; case VIR_DOMAIN_FS_TYPE_BIND: @@ -1082,7 +1082,8 @@ static int lxcContainerMountFS(virDomainFSDefPtr fs, static int lxcContainerMountAllFS(virDomainDefPtr vmDef, const char *dstprefix, - bool skipRoot) + bool skipRoot, + char *sec_mount_options) { size_t i; VIR_DEBUG(Mounting %s %d, dstprefix, skipRoot); @@ -1093,7 +1094,7 @@ static int lxcContainerMountAllFS(virDomainDefPtr vmDef, STREQ(vmDef-fss[i]-dst, /)) continue; -if (lxcContainerMountFS(vmDef-fss[i], dstprefix) 0) +if (lxcContainerMountFS(vmDef-fss[i], dstprefix, sec_mount_options) 0) return -1; } @@ -1401,7 +1402,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, virDomainFSDefPtr root, char **ttyPaths, size_t nttyPaths, - virSecurityManagerPtr securityDriver) + char *sec_mount_options) { struct lxcContainerCGroup *mounts = NULL; size_t nmounts = 0; @@ -1427,7 +1428,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Mounts the core /proc, /sys, etc filesystems */ -if (lxcContainerMountBasicFS(vmDef, true, securityDriver) 0) +if (lxcContainerMountBasicFS(true, sec_mount_options) 0) goto cleanup; /* Now we can re-mount the cgroups controllers in the @@ -1444,7 +1445,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Sets up any non-root mounts from guest config */ -if (lxcContainerMountAllFS(vmDef, /.oldroot, true) 0) +if (lxcContainerMountAllFS(vmDef, /.oldroot, true, sec_mount_options) 0) goto cleanup; /* Gets rid of all remaining mounts from host OS, including
Re: [libvirt] This patch creates a mount point for libvirt-lxc containers to mount on the destination system if it does not exist.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/25/2012 05:56 AM, Daniel P. Berrange wrote: On Fri, Jun 22, 2012 at 09:59:58AM -0400, Daniel J Walsh wrote: Currently libvirt-lxc checks to see if the destination exists and is a directory. If it is not a directory then the mount fails. Since libvirt-lxc can bind mount files on an inode, this patch is needed to allow us to bind mount files on files. Currently we want to bind mount on top of /etc/machine-id, and /etc/adjtime If the destination of the mount point does not exists, it checks if the src is a directory and then attempts to create a directory, otherwise it creates an empty file for the destination. The code will then bind mount over the destination. Current code blows up if the destination was not a directory. We want to be able to bind mount files on files. Sorry if you are seeing this patch for the second time, since I sent it to the wrong list. diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 24b1017..6cd8760 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -648,17 +665,37 @@ static int lxcContainerMountFSBind(virDomainFSDefPtr fs, { char *src = NULL; int ret = -1; +struct stat st; if (virAsprintf(src, %s%s, srcprefix, fs-src) 0) { virReportOOMError(); goto cleanup; } -if (virFileMakePath(fs-dst) 0) { - virReportSystemError(errno, - _(Failed to create %s), - fs-dst); -goto cleanup; +if ((stat(fs-dst, st) 0) (errno == ENOENT)) { + if (stat(src, st) = 0) { We should be reporting errors if either of the stat() calls fails. +if (S_ISDIR(st.st_mode)) { +if (virFileMakePath(fs-dst) 0) { + virReportSystemError(errno, + _(Failed to create %s), + fs-dst); +goto cleanup; +} + } else { +/* Create Empty file for target mount point */ +int fd = open(fs-dst, O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK, 0666); +if (fd = 0) { +close(fd); This needs to be VIR_CLOSE have it exit stats checked, since close() can fail (particularly on NFS). +} else { +if (errno != EEXIST) { + virReportSystemError(errno, + _(Failed to create %s), + fs-dst); +goto cleanup; +} + } +} +} I'm applying the following variation on your patch which fixes the issues I mention above: -if (virFileMakePath(fs-dst) 0) { - virReportSystemError(errno, - _(Failed to create %s), - fs-dst); -goto cleanup; +if (stat(fs-dst, st) 0) { +if (errno != ENOENT) { +virReportSystemError(errno, _(Unable to stat bind target %s), + fs-dst); +goto cleanup; +} +/* ENOENT = create the target dir or file */ +if (stat(src, st) 0) { +virReportSystemError(errno, _(Unable to stat bind source %s), + src); +goto cleanup; +} +if (S_ISDIR(st.st_mode)) { +if (virFileMakePath(fs-dst) 0) { + virReportSystemError(errno, + _(Failed to create %s), + fs-dst); + goto cleanup; +} +} else { +/* Create Empty file for target mount point */ +int fd = open(fs-dst, O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK, 0666); +if (fd 0) { + if (errno != EEXIST) { +virReportSystemError(errno, + _(Failed to create bind target %s), + fs-dst); +goto cleanup; +} + } +if (VIR_CLOSE(fd) 0) { + virReportSystemError(errno, + _(Failed to close bind target %s), + fs-dst); +goto cleanup; +} +} Daniel Great thanks. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk/oZDsACgkQrlYvE4MpobMa6ACg30+YwXBo1R8Tk7HjRGCEqqJN DQQAn0t26CHFffhWv73PZhmg1JSx7xRv =iM5A -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] Use private data struct in SELinux driver
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 05/11/2012 10:43 PM, Stefan Berger wrote: On 05/11/2012 06:10 AM, Daniel P. Berrange wrote: From: Daniel Walshdwa...@redhat.com Currently the SELinux driver stores its state in a set of global variables. This switches it to use a private data struct instead. This will enable different instances to have their own data. Signed-off-by: Daniel P. Berrangeberra...@redhat.com --- +SELinuxInitialize(virSecurityManagerPtr mgr) { [...] -ptr = strchrnul(default_image_context, '\n'); -if (*ptr == '\n') { +ptr = strchrnul(data-file_context, '\n'); +if (ptr *ptr == '\n') { *ptr = '\0'; -strcpy(default_content_context, ptr+1); - ptr = strchrnul(default_content_context, '\n'); -if (*ptr == '\n') +data-content_context = strdup(ptr+1); +if (!data-content_context) +goto error; virReportOOMError ? @@ -264,13 +277,11 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, goto cleanup; } -if (!def-seclabel.norelabel) { -def-seclabel.imagelabel = SELinuxGenNewContext(default_image_context, mcs); -if (!def-seclabel.imagelabel) { - virSecurityReportError(VIR_ERR_INTERNAL_ERROR, - _(cannot generate selinux context for %s), mcs); -goto cleanup; -} +def-seclabel.imagelabel = SELinuxGenNewContext(data-file_context, mcs); +if (!def-seclabel.imagelabel) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _(cannot generate selinux context for %s), mcs); +goto cleanup; } There was this check if (!def-seclabel.norelabel) that's now gone. Was this removed by accident? ACK with nit fixed. norelabel indicates that the Physical disk files/images should not be relabeled. When we create a tmpfs file system lxc containers always need to set an initial label on them. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk+uM7oACgkQrlYvE4MpobOFPACfZ/tDVzatSSoGkVUDEzICFmPE +1IAoNg7FX9wknCvZWFc9e7eLpN5SrZR =RQi1 -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Mount fresh instance of sysfs in LXC
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 05/10/2012 04:25 PM, Eric Blake wrote: On 05/10/2012 10:17 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Currently to make sysfs readonly, we remount the existing instance and then bind it readonly. Unfortunately this means sysfs is still showing device objects wrt the host OS namespace. We need it to reflect the container namespace, so we must mount a completely new instance of it. * src/lxc/lxc_container.c: Mount fresh sysfs instance --- src/lxc/lxc_container.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index b953646..77d33e1 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -441,8 +441,7 @@ static int lxcContainerMountBasicFS(lxc_child_argv_t *argv, const char *srcprefi { false, proc, /proc, proc, NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV }, { false, /proc/sys, /proc/sys, NULL, NULL, MS_BIND }, { false, /proc/sys, /proc/sys, NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, - { true, /sys, /sys, NULL, NULL, MS_BIND }, -{ true, /sys, /sys, NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, +{ false, sysfs, /sys, sysfs, NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY }, ACK. I have been testing this and it is working correctly now. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk+sKAwACgkQrlYvE4MpobOC1QCfeMOTUX9B60JkLhEn49/+Af3Z wboAn3AzoUL1n3eoQOW2MO58leX7/Nzw =47G8 -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add support for RAM filesystems for LXC
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 05/09/2012 11:49 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Introduce a new syntax for filesystems to allow use of a RAM filesystem filesystem type='ram' source usage='1024'/ target dir='/mnt'/ /filesystem The usasge is in KB to limit consumption of host memory. Do we want to allow user to specify Mount Options? rw,nosuid,nodev Or permissions root,root - 1777 -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk+qpLoACgkQrlYvE4MpobM+PACg1fjP6kQkUw3Oa5JXqXDlHiQ6 WP0An130+Te4ebR28JB5cUoVxDaIA2Ni =kaTs -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] This patch mounts tmpfs on /run iff /run directory exists in libvirt-lxc containers.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 We do not want to share /run with containers in order to prevent information leakage and applications within the containers attempting to communicate with applications outside of the container. It uses the same mount options used for /dev. We also want to bind mount over /var/run directory since this will either be a symbolic link to /run but on some installations /run is bind mounted over /var/run. If we just mount /run we are not guaranteed the /var/run will have the same content. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk+j7skACgkQrlYvE4MpobNSKQCfY2yGP/S+piUJ9VNtSjrliFTp ucAAoLJOazpcZvBRFnQUa7uqhh+tRagb =TjAb -END PGP SIGNATURE- diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 88f8a21..4cbe4b9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -539,6 +519,28 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot) devfs, /dev, tmpfs); goto cleanup; } + +/* Mount /run with a tmpfs iff it exists. Bind mount /run + over /var/run to make sure they point to the same directory +*/ +if ((access(/run, F_OK) == 0)) { +VIR_DEBUG(Mount tmpfs on /run type=tmpfs flags=%x, opts=%s, + MS_NOSUID, opts); +if (mount(tmpfs, /run, tmpfs, MS_NOSUID | MS_NODEV , opts) 0) { +virReportSystemError(errno, + _(Failed to mount %s on %s type %s), + tmpfs, /run, tmpfs); +goto cleanup; +} + +VIR_DEBUG(Mount /run on /var/run type=bind); +if (mount(/run, /var/run, run, MS_BIND , NULL) 0) { +virReportSystemError(errno, + _(Failed to mount %s on %s), + /run, /var/run); +goto cleanup; +} +} } rc = 0; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] These two machines look like they have dontaudit rules disabled.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 audit_log-ex-std-node22.prod.rhcloud.com-2012-03-12 audit_log-ex-std-node24.prod.rhcloud.com-2012-03-12 semodule -B Will turn dontaudit rules back on. 22:31:32.791:507663) : avc: denied { siginh } for pid=15258 comm=trap-user scontext=system_u:system_r:sshd_t:s0-s0:c0.c1023 tcontext=unconfined_u:system_r:libra_t:s0:c5,c641 tclass=process grep siginh * | audit2allow #= sshd_t == # This avc has a dontaudit rule in the current policy allow sshd_t libra_t:process siginh; -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk9fIj4ACgkQrlYvE4MpobM44gCeJEqC+EV3HN57pL2j/hv9hMYO cewAnjYiI6hehUpwqVEQJ3bX4Dz3eS95 =GqCQ -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Postgresql binding to other localhosts by libra instances.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I see several postgresql instances trying to bind to 127.0.0.1 audit_log-ex-lg-node4.prod.rhcloud.com-2012-03-12 audit_log-ex-std-node18.prod.rhcloud.com-2012-03-12 audit_log-ex-std-node5.prod.rhcloud.com-2012-03-12 uid=6b44af7291524783ad6ed1bc1b55aed5 uid=8d3252d15512409c97f9d3b9167cc2bc uid=e1f70ff7ee3a438a8d513ca953a3dc7c -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEUEARECAAYFAk9fJIsACgkQrlYvE4MpobNSPACYlgWtte2TcatDKgsfaVbz8WSY XQCcCatA688MoFasF5sQUpQ6DSaNVKU= =WEVQ -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Improve flexibility of SELinux labelling
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/27/2011 08:20 AM, Daniel P. Berrange wrote: This patch series adds two new features - The ability to override 'system_u:system_r:svirt_t:s0' from /etc/selinux/targeted/contexts/virtual_domain_context using the guest XML - The ability to use dynamic relabelling of resources, in combo with static VM label assignment. The latter is useful for management applications which want to be in full control of assigning VM labels (so that they can be unique across an entire cluster of hosts for example), while still benefiting from automatic relabelling of resources in the XML. I think you might want to be a little more flexible with this. I see where you would want 4 ways of doing this. Dynamic with /etc/selinux/targeted/contexts/virtual_domain_context Dynamic with alternate TYPE, Meaning I could specify system_u:system_r:svirt_apache_t:s0 and then libvirt would select a MCS label for this context and launch system_u:system_r:svirt_apache_t:s0:c1,c257 Static with no relabel. Static with relabel. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk4JuxgACgkQrlYvE4MpobMIyACeMEHG5Iv2fP15pexyss34wsGF dGsAn1gKtRuMeuVKBdU4TJL6Ar1Kl1ZB =V6qL -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Improve flexibility of SELinux labelling
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/28/2011 08:23 AM, Daniel P. Berrange wrote: On Tue, Jun 28, 2011 at 07:29:28AM -0400, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/27/2011 08:20 AM, Daniel P. Berrange wrote: This patch series adds two new features - The ability to override 'system_u:system_r:svirt_t:s0' from /etc/selinux/targeted/contexts/virtual_domain_context using the guest XML - The ability to use dynamic relabelling of resources, in combo with static VM label assignment. The latter is useful for management applications which want to be in full control of assigning VM labels (so that they can be unique across an entire cluster of hosts for example), while still benefiting from automatic relabelling of resources in the XML. I think you might want to be a little more flexible with this. I see where you would want 4 ways of doing this. We already do options 1 and 3. These two patches I post let us also support options 2 and 4, so I think we're sorted. Dynamic with /etc/selinux/targeted/contexts/virtual_domain_context seclabel type='dynamic'/ Dynamic with alternate TYPE, Meaning I could specify system_u:system_r:svirt_apache_t:s0 and then libvirt would select a MCS label for this context and launch system_u:system_r:svirt_apache_t:s0:c1,c257 seclabel type='dynamic' baselabelsystem_u:system_r:svirt_apache_t:s0/baselabel /seclabel Static with no relabel. seclabel type='static' relabel='no' labelsystem_u:system_r:svirt_apache_t:s0:c1,c257/label /seclabel Static with relabel. seclabel type='static' relabel='yes' labelsystem_u:system_r:svirt_apache_t:s0:c1,c257/label /seclabel Regards, Daniel Great thanks. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk4JzzMACgkQrlYvE4MpobOlQQCgl14dE0FPEWwNUW+YdsF6dV4w w8oAoJLvSuGlJuc6T7avEUyz1JyzfnG9 =QKcR -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: extending sVirt to confine host apps which talk to libvirtd
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/08/2011 06:34 AM, Daniel P. Berrange wrote: On Mon, Jun 06, 2011 at 02:51:15PM -0400, Daniel J Walsh wrote: On 06/06/2011 10:41 AM, Daniel P. Berrange wrote: Technical Notes / Issues 1. Adding new SELinux security classes / access vectors The selinux security classes are defined in /usr/include/selinux/flask.h and access vectors in /usr/include/selinux/av_permissions.h Both of these files are automatically by a script in the selinux reference policy code '$serefpolicy/policy/flask/flask.py'. The master data files are in the same directory, 'access_vectors' and 'security_classes'. Once generated, the headers need to be manually copied into the libselinux package sources. You do not need to do this anymore. libselinux does not care about the access vectors, they are named in your application.Well AFAICT, when I invoke avc_has_perm(), the security_class_t and access_vector_t parameters are integer constants that come from selinux/flask.h and selinux/av_permissions.h respectively. While I could just pick some unused numbers myself and hardcode in the libvirt source, it seems like it is better to have them recorded in those header files deployed by libselinux, so libvirt's usage doesn't clash with some other future app. There are calls now that ask the kernel for the correct numbers. man security_av_string 3. Security labelling config modes When creating a guest the following XML snippets can be used. a. Default type, dynamic MCS, automatic relabelling seclabel type='selinux' mode='dynamic' relabel='yes'/ b. Custom type, dynamic MCS, automatic relabelling seclabel type='selinux' mode='hybrid' relabel='yes' labelsystem_u:system_r:mysvirt_t/label imagelabelsystem_u:object_r:mysvirt_image_t/imagelabel /seclabel Yes this would be cool, although I am not sure you need an image label, since the MCS separation would still work on svirt_image_t. Would make policy writing easier and selection easier if you did not change the type of the image file. I would at least allow for the admin to not specify a image label. Yes, now that you mention it, including the imagelabel does seem redundant here. Once we can have multiple different base labels, should we control uniqueness just on the MCS category, or the full label. I believe we still want the former. eg, we should forbid two different guests to run: 'myfirst_svirt_t:c123,435' 'myother_svirt_t:c123,435' Yes I would keep one table of MCS Strings unique and never allow two process the same label. c. Default type, dynamic MCS, no relabelling seclabel type='selinux' mode='dynamic' relabel='no'/ Does this mode make any sense, since admin doesn't know MCS category upfront ? Possibly only useful if the guest only has readonly disks. But you don't relabel on readonly correct, since this is a shared resource. I would say this would not be used. We do actually relabel readonly content, but to the shared label (if it doesn't already have the right shared label). Ok d. Custom type, dynamic MCS, no relabelling seclabel type='selinux' mode='hybrid' relabel='no' labelsystem_u:system_r:mysvirt_t/label /seclabel Same question about whether it makes sense I don't think this makes sense. e. Custom type, static MCS, auto relabelling seclabel type='selinux' mode='static' relabel='yes' labelsystem_u:system_r:mysvirt_t:s0:c123,c456/label imagelabelsystem_u:system_r:mysvirt_image_t:s0:c123,c456/imagelabel /seclabel This is fine, not sure it is legal in MLS world. Although I guess we could change the label to SystemHigh when not in use. Yep, or just have MLS policy block this scenario. f. Custom type, static MCS, no relabelling seclabel type='selinux' mode='static' relabel='no' labelsystem_u:system_r:mysvirt_t:s0:c123,c456/label /seclabel We have this now, this is static labeling. Correct, options a. and f. are the two current configs we support. One last thing to think about is since libvirt can now be run under the users context, in certain situations, libvirt should examine the range of MLS/MCS labels associated with it and make sure that it can only assign MCS labels within this range. For example if I am a user running as staff_t:s0-s0:c500 libvirt should only pick random labels between 0-500. Ah, interesting point. Daniel -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk3ww3AACgkQrlYvE4MpobND+wCgoI2cK6qFOLspFe+MltnlqjwA hmwAnAuYytuZMf2tDPJphrQJ16IxXJjq =ShL0 -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: extending sVirt to confine host apps which talk to libvirtd
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/06/2011 10:41 AM, Daniel P. Berrange wrote: What follows is a document outlining some thoughts I've been having on extending sVirt to allow confinement of applications which talk to libvirtd on the host, primarily focusing on use of SELinux, but also allowing a simple non-SElinux RBAC mechanism. Securing KVM virtualization hosts with MAC == This document looks at the task of securing KVM virtualizaton hosts using mandatory access control technologies, with focus on SELinux. At the time of writing there have been two phases of development, and this document makes proposals for a third phase. Phase 1: circa 2006 --- Goal: Protect the host from a compromised virtual machine. The first phase of development had the modest goal of protecting the host from attack by a compromised virtual machine. To achieve this, the KVM processes are configured such that they will run under a confined security context ('virt_t' in the SELinux reference policy), which blocks access to any host resources not labelled ('virt_image_t') for use by virtual machines. The primary limitations of this initial implementation is that while the virtual host is secured, there is no protection between virtual machines. This can be considered a regression in isolation as compared to that offered by non-virtualized hosts. The second limitation is that the virtualization admin has to take care to ensure the host resources intended for use by the virtual machines are correctly labelled. This is a manual setup taks unless the images are kept in a preset location (/var/lib/libvirt/images in the SELinux reference policy). Phase 2: March 2009 --- Goal: Protect virtual machines from each other The second phase of development has the goal of providing isolation between virtual machines that is comparable to that achieved between physical machines. This piece of work is commonly referred to as svirt. The achieve this, the KVM processes are each configured to run under a dedicated security context, which blocks access to any resources not explicitly assigned to that virtual machine. In the SELinux implementation, the base context svirt_t has a unique MCS category (c240,c955) appended to form a unique security context system_u:system_r:svirt_t:s0:c240,c955. For each host resource to be assigned to the virtual machine, the base context svirt_image_t is combined with the same MCS category to form a unique resource security context system_u:object_r:svirt_image_t:s0:c240,c955. The assignment of virtual machine security contexts and labelling of resources can be done statically by the administrator / management application, or dynamically by the libvirtd daemon. The latter removes much of the administrator burden. The second phase has addressed the major guest security limitation of the first phase, and eased the burden placed on host administors. Attention can now focus on the security of the host management software stack. Client applications communicate with the libvirtd daemon using a simple sockets based RPC protocol. Thus operations initiated by client applications which run under one security context are in fact invoked under the libvirtd daemon's security context. Since the libvirtd daemon is a highly privileged, almost unconfined process, this provides a means for applications to elevate their privileges. A second problem with the current model is seen when looking at guest migration between hosts. During migration, there are two QEMU processes running for the same virtual machine, one process on each host. The dynamic assignment of MCS values to form unique security contexts is done on a per host basis, so there is no guarantee that the VM on host A will be using (or be able to use) the same security context on the target host of migration. This is not neccessarily a problem if the guest is using block devices, since block device inode labels are only visible to a single host. With a shared filesystem that supports SELinux labelling, like GFS2, both QEMU processes must run in the same security context to allow them both to access the associated files. Phase 3: June 2011 -- Goal: Protect virtual machines from host applications The third phase of development has the primary goal of honouring the confinement of client applications talking to libvirtd, when performing operations on virtual machines and other managed objects (storage pools, host devices, virtual networks, secrets, etc). Every application connecting to libvirt has an associated security context. Every object managed by libvirtd will have an associated security context. When an operation is invoked via a libvirt API the client application security context will be checked against the target object context, before
Re: [libvirt] [PATCH 6/6] Try much harder to restore disk file labels
On 09/01/2009 02:20 PM, Stephen Smalley wrote: On Tue, 2009-09-01 at 13:33 -0400, Stephen Smalley wrote: On Tue, 2009-09-01 at 18:10 +0100, Daniel P. Berrange wrote: On Tue, Sep 01, 2009 at 01:00:13PM -0400, Stephen Smalley wrote: On Tue, 2009-09-01 at 16:28 +0100, Daniel P. Berrange wrote: * src/security_selinux.c: matchpath() may well return NULL for many directories, to try and fallback to using parent directory label in that scenario. When have you seen this happen? matchpathcon() ultimately should fall back to the top-level regex (/.*) and map any otherwise unmatched files to default_t, and should generally have a fallback regex for each subtree (e.g. any file under /dev that isn't otherwise matched would get device_t). So I wouldn't expect this to happen. That describes what I always imagined would happen, but in my recent testing it doesn't appear to be doing that in practice in some cases eg, running matchpathcon(/media/usbdisk/virtual-images/demo.img) returned an error. Likewise for /sys/bus/pci/devices/.NN.NN.NN/*. I was testing this on a Fedora 11 host. Perhaps the policy is simply missing some rules Ah, I had forgotten about the behavior in the case where the path is marked with none in the file_contexts configuration, in which case matchpathcon() does return NULL as you describe. That gets used for two situations: - mount point directories for filesystems that do not support labeling, like /sys and /selinux. That should be obsoleted by the support in kernels = 2.6.30 to report seclabel support in /proc/mounts, which gets used by setfiles when available. So we can likely remove those none entries going forward. - directories that contain runtime-created files whose labels we do not wish to reset, like /tmp or /var/run. Those entries are still necessary to avoid clobbering runtime labels on such files upon a relabel. So I guess you do need this. Actually, why can't you just save the original file label somewhere (i.e. call getfilecon() and store that) and then just use that original label when restoring the label. Then you don't need to use matchpathcon() or this extra logic at all. Also, files will inherit their SELinux type from the parent directory by default upon creation unless a type transition rule is specified, so it isn't clear why you need to replicate this copying from parent behavior in the application. This code is being run upon VM shutdown, to get rid of the dynamic label that was assigned at VM startup. THe file already exists, so the default rule for file creation wouldn't come into effect at this point, so I was aiming to replicate that behaviour for the existing file. If we could guarentee that matchpathcon($PATH) would always return something usable, I would happily kill this code In Rawhide, these are the files that are labeled as none Which ones should I eliminate. I tend to agree with Steven, the best solution to the problem would be MAKEFILE/DEVICE getfilecon(path, oldcon) setfilecon(path, svirtcon) run virtual machine, end virtual machine setfilecon(path, oldcon) /sys/.* none /tmp/.* none /mnt/[^/]*/.* none /proc/.*none /media/[^/]*/.* none /dev/pts(/.*)? none /var/tmp/.* none /usr/tmp/.* none /selinux/.* none /var/run/.*\.*pid none /lost\+found/.* none /tmp/\.ICE-unix/.* -s none /tmp/\.X11-unix/.* -s none /var/lost\+found/.* none /usr/lost\+found/.* none /tmp/lost\+found/.* none /var/spool/cron/[^/]* -- none /var/spool/fcron/.* none /boot/lost\+found/.*none /var/tmp/lost\+found/.* none /usr/local/lost\+found/.* none /var/named/chroot/proc(/.*)?none /var/lib/nfs/rpc_pipefs(/.*)? none /var/spool/cron/crontabs/.* -- none /sys-d none /proc -d none /selinux-d none /\.journal none /var/\.journal none /usr/\.journal none /tmp/\.journal none /boot/\.journal none /usr/local/\.journalnone -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make SELinuxSecurityDriverProbe() fail on Fedora 10
On 04/16/2009 06:54 AM, Daniel P. Berrange wrote: On Thu, Apr 16, 2009 at 11:44:48AM +0100, Mark McLoughlin wrote: Running make check on F10, I get: libvir: Security Labeling error : SELinuxInitialize: cannot open SELinux virtual domain context file /etc/selinux/targeted/contexts/virtual_domain_context: No such file or directory Failed to start security driverFAIL: seclabeltest Seems virtual_domain_context isn't available on F10. IMHO that's a Fedora packaging bug. We already probe for the presence fo the selinux_virtual_domain_context_path() method, which didn't exist in Fedora 10. So if that method has now appeared, but without the files it requires in order to work that seems like a policy bug to me. Daniel Yes it is a policy bug, libselinux versus selinux-policy. Working on backporting the F11 policy into F10. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Thoughts on svirt configuration files.
On 04/07/2009 03:27 PM, Daniel P. Berrange wrote: On Mon, Apr 06, 2009 at 03:05:57PM -0400, Daniel J Walsh wrote: Currently we do not want to hard code virtual image names into libvirt, so libvirt and virtual-manager can use libselinux to get the default image label and process label. svirt_t and svirt_image_t. The idea was one policy writer might want his virtual images labeled differently than another. One problem with this is I added to interfaces one for the domain, and one for the image label. Now we realize we have other images. We have process Label- svirt_t:MCS Exclusive RW Image - svirt_image_t:MCS Shared RW Image - svirt_image_t:s0 Read Only Image - virt_content_t:s0 So I am suggesting that we remove the virtual_image_context file and allowing policy writers to define context in the virtual_domain_context files but have multiple records and multiple fields. Something like a space separated list where each field corresponds to above. system_u:system_r:svirt_t:s0 system_u:object_r:svirt_image_t:s0 system_u:object_r:svirt_image_t:s0 system_u:object_r:virt_content_t:s0 Then you could add optional types with similar fields system_u:system_r:svirt_nonet_t:s0 system_u:object_r:svirt_image_t:s0 system_u:object_r:svirt_image_t:s0 system_u:object_r:virt_content_t:s0 How would libvirt decide which of the records to use, or what the semantics of each were ? Or are you thinking this info is just to allow for verification of user supplied labels in the XML ? No I was thinking the user would specify the image label, and if dynamic, libvirt would assign the other three labels. I guess a close approximation of this would be the /etc/selinux/targeted/context/users directory, where each file includes multiple context. So we could add /etc/selinux/targeted/context/virt/ default: domain_label = system_u:system_r:svirt_t:s0 exclusive_image_label = system_u:object_r:svirt_image_t:s0 shared_image_label = system_u:object_r:svirt_image_t:s0 readonly_image_label = system_u:object_r:virt_content_t:s0 svirt_nonet_t: domain_label = system_u:system_r:svirt_nonet_t:s0 exclusive_image_label = system_u:object_r:svirt_nonet_image_t:s0 shared_image_label = system_u:object_r:svirt_image_t:s0 readonly_image_label = system_u:object_r:virt_content_t:s0 The more I think about this, it might be a waste, though. If we just change the libvirtd_t to allow the user to select a alternate domain_label and always set the images the same, it would probably work for most everyone. For those that this does not work would have to use static labeling. /etc/selinux/targeted/context/virtual_domain_context default_domain_label = system_u:system_r:svirt_t:s0 exclusive_image_label = system_u:object_r:svirt_image_t:s0 shared_image_label = system_u:object_r:svirt_image_t:s0 readonly_image_label = system_u:object_r:virt_content_t:s0 Since SELinux just returns a path, the virt team could choose the format of the file if a space separated list is not addequate. (xml?) Name Value Pairs? Policy writers would have to enter the format that is chosen. I'd favour using the simple config format we have for /etc/libvirt/libvirt.conf handled by the src/conf.h APIs. This is actually originates from Xen where it used /etc/xen/$GUEST config files, which were in fact python code. Our parser basically allows a subset of just strings and lists, which seems like it'd be sufficent for this case, and also allow the SELinux python libs to easily read the files too Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Thoughts on svirt configuration files.
Currently we do not want to hard code virtual image names into libvirt, so libvirt and virtual-manager can use libselinux to get the default image label and process label. svirt_t and svirt_image_t. The idea was one policy writer might want his virtual images labeled differently than another. One problem with this is I added to interfaces one for the domain, and one for the image label. Now we realize we have other images. We have process Label- svirt_t:MCS Exclusive RW Image - svirt_image_t:MCS Shared RW Image - svirt_image_t:s0 Read Only Image - virt_content_t:s0 So I am suggesting that we remove the virtual_image_context file and allowing policy writers to define context in the virtual_domain_context files but have multiple records and multiple fields. Something like a space separated list where each field corresponds to above. system_u:system_r:svirt_t:s0 system_u:object_r:svirt_image_t:s0 system_u:object_r:svirt_image_t:s0 system_u:object_r:virt_content_t:s0 Then you could add optional types with similar fields system_u:system_r:svirt_nonet_t:s0 system_u:object_r:svirt_image_t:s0 system_u:object_r:svirt_image_t:s0 system_u:object_r:virt_content_t:s0 Since SELinux just returns a path, the virt team could choose the format of the file if a space separated list is not addequate. (xml?) Name Value Pairs? Policy writers would have to enter the format that is chosen. Thoughts? I am thinking we might eventually want to allow an admin to select dynamic labels but specify alternative types. So svirt_t would be default but if someone wanted svirt_nonet_t, they could choose that also and get separation with a different type. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Reworked the XML verification patch for svirt
Main goal of this patch it to verify data being written to xml and inform the user when he makes a mistake. Only able to verify static/dynamic in domain_conf. Added verify code to qemu_driver.c for model and potentially label. diff --git a/src/Makefile.am b/src/Makefile.am index d5aac11..8d53740 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -170,9 +170,9 @@ STORAGE_HELPER_DISK_SOURCES = \ SECURITY_DRIVER_SOURCES = \ security.h security.c -SECURITY_DRIVER_SELINUX_SOURCES = \ - security_selinux.h security_selinux.c - +if WITH_SECDRIVER_SELINUX +SECURITY_DRIVER_SOURCES += security_selinux.h security_selinux.c +endif NODE_DEVICE_DRIVER_SOURCES = \ node_device.c node_device.h @@ -390,9 +390,6 @@ endif libvirt_driver_security_la_SOURCES = $(SECURITY_DRIVER_SOURCES) noinst_LTLIBRARIES += libvirt_driver_security.la libvirt_la_LIBADD += libvirt_driver_security.la -if WITH_SECDRIVER_SELINUX -libvirt_driver_security_la_SOURCES += $(SECURITY_DRIVER_SELINUX_SOURCES) -endif # Add all conditional sources just in case... EXTRA_DIST += \ diff --git a/src/domain_conf.c b/src/domain_conf.c index 2696449..c9259db 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -1859,29 +1859,44 @@ virSecurityLabelDefParseXML(virConnectPtr conn, if (virXPathNode(conn, ./seclabel, ctxt) == NULL) return 0; +p = virXPathStringLimit(conn, string(./seclabel/@model), + VIR_SECURITY_MODEL_BUFLEN-1, ctxt); +if (p == NULL) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, +%s, _(missing security model)); + goto error; +} +def-seclabel.model = p; + p = virXPathStringLimit(conn, string(./seclabel/@type), VIR_SECURITY_LABEL_BUFLEN-1, ctxt); -if (p == NULL) -goto error; -if ((def-seclabel.type = virDomainSeclabelTypeFromString(p)) 0) +if (p == NULL) { +virDomainReportError(conn, VIR_ERR_XML_ERROR, + %s, _(missing security type)); goto error; +} +def-seclabel.type = virDomainSeclabelTypeFromString(p); VIR_FREE(p); +if (def-seclabel.type 0) { +virDomainReportError(conn, VIR_ERR_XML_ERROR, + _(invalid security type)); +goto error; +} /* Only parse details, if using static labels, or * if the 'live' VM XML is requested */ if (def-seclabel.type == VIR_DOMAIN_SECLABEL_STATIC || !(flags VIR_DOMAIN_XML_INACTIVE)) { -p = virXPathStringLimit(conn, string(./seclabel/@model), -VIR_SECURITY_MODEL_BUFLEN-1, ctxt); -if (p == NULL) -goto error; -def-seclabel.model = p; p = virXPathStringLimit(conn, string(./seclabel/label[1]), VIR_SECURITY_LABEL_BUFLEN-1, ctxt); -if (p == NULL) +if (p == NULL) { +virDomainReportError(conn, VIR_ERR_XML_ERROR, + _(security label is missing)); goto error; +} + def-seclabel.label = p; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a5f9f92..5c88009 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -248,6 +248,7 @@ free_qparam_set; # security.h +virSecurityDriverVerify; virSecurityDriverStartup; virSecurityDriverInit; virSecurityDriverSetDOI; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index b2974bb..cd48193 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2115,6 +2115,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; +if (virSecurityDriverVerify(conn, def) 0) + goto cleanup; + vm = virDomainFindByName(driver-domains, def-name); if (vm) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, @@ -3398,6 +3401,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { VIR_DOMAIN_XML_INACTIVE))) goto cleanup; +if (virSecurityDriverVerify(conn, def) 0) + goto cleanup; + vm = virDomainFindByName(driver-domains, def-name); if (vm) { virDomainObjUnlock(vm); diff --git a/src/security.c b/src/security.c index e2bd20a..72aadf4 100644 --- a/src/security.c +++ b/src/security.c @@ -28,6 +28,25 @@ static virSecurityDriverPtr security_drivers[] = { }; int +virSecurityDriverVerify(virConnectPtr conn, virDomainDefPtr def) +{ +unsigned int i; +const virSecurityLabelDefPtr secdef = def-seclabel; + +if (STREQ(secdef-model, none)) +
[libvirt] Added seclabeltest to run under test suite.
diff --git a/tests/.gitignore b/tests/.gitignore index 9d809c9..4f33d0b 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -15,6 +15,7 @@ nodedevxml2xmltest nodeinfotest statstest qparamtest +seclabeltest *.gcda *.gcno *.exe diff --git a/tests/Makefile.am b/tests/Makefile.am index 28b2737..48db913 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -54,7 +54,7 @@ EXTRA_DIST = \ nodedevschemadata noinst_PROGRAMS = virshtest conftest \ -nodeinfotest statstest qparamtest +nodeinfotest statstest qparamtest seclabeltest if WITH_XEN noinst_PROGRAMS += xml2sexprtest sexpr2xmltest \ @@ -97,6 +97,7 @@ EXTRA_DIST += $(test_scripts) TESTS = virshtest \ nodeinfotest \ statstest \ + seclabeltest \ qparamtest \ $(test_scripts) @@ -203,6 +204,10 @@ statstest_SOURCES = \ statstest.c testutils.h testutils.c statstest_LDADD = $(LDADDS) +seclabeltest_SOURCES = \ + seclabeltest.c +seclabeltest_LDADD = ../src/libvirt_driver_security.la $(LDADDS) + qparamtest_SOURCES = \ qparamtest.c testutils.h testutils.c qparamtest_LDADD = $(LDADDS) diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c new file mode 100644 index 000..f068a00 --- /dev/null +++ b/tests/seclabeltest.c @@ -0,0 +1,60 @@ +#include config.h + +#include unistd.h +#include stdlib.h +#include stdio.h +#include string.h +#include errno.h +#include security.h + +int +main (int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) +{ +int ret; + +const char *doi, *model; +virSecurityDriverPtr security_drv; + +ret = virSecurityDriverStartup (security_drv, selinux); +if (ret == -1) +{ +fprintf (stderr, Failed to start security driver); +exit (-1); +} +/* No security driver wanted to be enabled: just return */ +if (ret == -2) +return 0; + +model = virSecurityDriverGetModel (security_drv); +if (!model) +{ +fprintf (stderr, Failed to copy secModel model: %s, + strerror (errno)); +exit (-1); +} + +doi = virSecurityDriverGetDOI (security_drv); +if (!doi) +{ +fprintf (stderr, Failed to copy secModel DOI: %s, + strerror (errno)); +exit (-1); +} + +virConnectPtr conn; +conn = virConnectOpen (NULL); +if (conn == NULL) +{ +fprintf (stderr, First virConnectOpen() failed\n); +exit (1); +} +virSecurityDriverSetDOI (conn, security_drv, 1); +doi = virSecurityDriverGetDOI (security_drv); +if (strcmp (doi, 1) != 0) +{ +fprintf (stderr, Failed to set secModel DOI: %s, strerror (errno)); +exit (1); +} +virConnectClose (conn); +return 0; +} -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Reworked the XML verification patch for svirt
On 04/01/2009 12:50 PM, Daniel P. Berrange wrote: On Wed, Apr 01, 2009 at 10:33:56AM -0400, Daniel J Walsh wrote: Main goal of this patch it to verify data being written to xml and inform the user when he makes a mistake. Only able to verify static/dynamic in domain_conf. Added verify code to qemu_driver.c for model and potentially label. diff --git a/src/Makefile.am b/src/Makefile.am index d5aac11..8d53740 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -170,9 +170,9 @@ STORAGE_HELPER_DISK_SOURCES = \ SECURITY_DRIVER_SOURCES = \ security.h security.c -SECURITY_DRIVER_SELINUX_SOURCES = \ - security_selinux.h security_selinux.c - +if WITH_SECDRIVER_SELINUX +SECURITY_DRIVER_SOURCES += security_selinux.h security_selinux.c +endif NODE_DEVICE_DRIVER_SOURCES = \ node_device.c node_device.h @@ -390,9 +390,6 @@ endif libvirt_driver_security_la_SOURCES = $(SECURITY_DRIVER_SOURCES) noinst_LTLIBRARIES += libvirt_driver_security.la libvirt_la_LIBADD += libvirt_driver_security.la -if WITH_SECDRIVER_SELINUX -libvirt_driver_security_la_SOURCES += $(SECURITY_DRIVER_SELINUX_SOURCES) -endif What's the purpose of this change ? The reason for keeping an explicit variable name SECURITY_DRIVER_SELINUX_SOURCES was that those sounds need to be added to EXTRA_DIST, even when the compilation of the selinux bits is turned off - so that 'make dist' always gets all files. diff --git a/src/domain_conf.c b/src/domain_conf.c index 2696449..c9259db 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -1859,29 +1859,44 @@ virSecurityLabelDefParseXML(virConnectPtr conn, if (virXPathNode(conn, ./seclabel, ctxt) == NULL) return 0; +p = virXPathStringLimit(conn, string(./seclabel/@model), + VIR_SECURITY_MODEL_BUFLEN-1, ctxt); +if (p == NULL) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, +%s, _(missing security model)); + goto error; +} +def-seclabel.model = p; + p = virXPathStringLimit(conn, string(./seclabel/@type), VIR_SECURITY_LABEL_BUFLEN-1, ctxt); -if (p == NULL) -goto error; -if ((def-seclabel.type = virDomainSeclabelTypeFromString(p)) 0) +if (p == NULL) { +virDomainReportError(conn, VIR_ERR_XML_ERROR, + %s, _(missing security type)); goto error; +} +def-seclabel.type = virDomainSeclabelTypeFromString(p); VIR_FREE(p); +if (def-seclabel.type 0) { +virDomainReportError(conn, VIR_ERR_XML_ERROR, + _(invalid security type)); +goto error; +} /* Only parse details, if using static labels, or * if the 'live' VM XML is requested */ if (def-seclabel.type == VIR_DOMAIN_SECLABEL_STATIC || !(flags VIR_DOMAIN_XML_INACTIVE)) { -p = virXPathStringLimit(conn, string(./seclabel/@model), -VIR_SECURITY_MODEL_BUFLEN-1, ctxt); -if (p == NULL) -goto error; -def-seclabel.model = p; p = virXPathStringLimit(conn, string(./seclabel/label[1]), VIR_SECURITY_LABEL_BUFLEN-1, ctxt); -if (p == NULL) +if (p == NULL) { +virDomainReportError(conn, VIR_ERR_XML_ERROR, + _(security label is missing)); goto error; +} + def-seclabel.label = p; } ACK, this looks fine diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a5f9f92..5c88009 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -248,6 +248,7 @@ free_qparam_set; # security.h +virSecurityDriverVerify; virSecurityDriverStartup; virSecurityDriverInit; virSecurityDriverSetDOI; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index b2974bb..cd48193 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2115,6 +2115,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; +if (virSecurityDriverVerify(conn, def) 0) + goto cleanup; + vm = virDomainFindByName(driver-domains, def-name); if (vm) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, @@ -3398,6 +3401,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { VIR_DOMAIN_XML_INACTIVE))) goto cleanup; +if (virSecurityDriverVerify(conn, def) 0) + goto cleanup; + vm = virDomainFindByName(driver-domains, def-name); if (vm) { virDomainObjUnlock(vm); diff --git a/src/security.c b/src/security.c index
[libvirt] Patch to allow setting of svirt XML.
This patch fixes the seclabel handling in domain_conf.c to allow virt-manager to set the seclabel model, type and label. Also adds missing error messages when the xml is incorrect. How much verification should we be doing on this? I have another patch that verifies the model as being a known model and a patch to verify the label is a correct label. (IE SELinux verifies the label is understood by the kernel.) The problem with this second patch is it sucks in security.[ch], security_selinux.[ch] into the libvirt_lxc. Should I be doing this? --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -1859,12 +1859,28 @@ virSecurityLabelDefParseXML(virConnectPtr conn, if (virXPathNode(conn, ./seclabel, ctxt) == NULL) return 0; +p = virXPathStringLimit(conn, string(./seclabel/@model), +VIR_SECURITY_MODEL_BUFLEN-1, ctxt); +if (p == NULL) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, +%s, _(missing seclabel model)); + goto error; +} +def-seclabel.model = p; + p = virXPathStringLimit(conn, string(./seclabel/@type), VIR_SECURITY_LABEL_BUFLEN-1, ctxt); -if (p == NULL) +if (p == NULL) { +virDomainReportError(conn, VIR_ERR_XML_ERROR, + %s, _(missing seclabel type)); goto error; -if ((def-seclabel.type = virDomainSeclabelTypeFromString(p)) 0) +} + +if ((def-seclabel.type = virDomainSeclabelTypeFromString(p)) 0) { +virDomainReportError(conn, VIR_ERR_XML_ERROR, + _(unknown seclabel type %s), p); goto error; +} VIR_FREE(p); /* Only parse details, if using static labels, or @@ -1872,16 +1888,14 @@ virSecurityLabelDefParseXML(virConnectPtr conn, */ if (def-seclabel.type == VIR_DOMAIN_SECLABEL_STATIC || !(flags VIR_DOMAIN_XML_INACTIVE)) { -p = virXPathStringLimit(conn, string(./seclabel/@model), -VIR_SECURITY_MODEL_BUFLEN-1, ctxt); -if (p == NULL) -goto error; -def-seclabel.model = p; p = virXPathStringLimit(conn, string(./seclabel/label[1]), VIR_SECURITY_LABEL_BUFLEN-1, ctxt); -if (p == NULL) -goto error; +if (p == NULL) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, +_(seclabel label is too long)); + goto error; +} def-seclabel.label = p; } @@ -1890,8 +1904,11 @@ virSecurityLabelDefParseXML(virConnectPtr conn, !(flags VIR_DOMAIN_XML_INACTIVE)) { p = virXPathStringLimit(conn, string(./seclabel/imagelabel[1]), VIR_SECURITY_LABEL_BUFLEN-1, ctxt); -if (p == NULL) +if (p == NULL) { +virDomainReportError(conn, VIR_ERR_XML_ERROR, + _(seclabel image label is too long)); goto error; +} def-seclabel.imagelabel = p; } diff --git a/src/security_selinux.c b/src/security_selinux.c index 1708d55..5937f48 100644 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] svirt fix for schema on domain.rng
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 107215c..21bd2b2 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -34,6 +34,12 @@ attribute name='model' text/ /attribute + attribute name='type' +choice + valuedynamic/value + valuestatic/value +/choice + /attribute element name='label' text/ /element -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] I have no idea why the current version of libvirt works for anyone in enforcing mode.
On 03/17/2009 11:52 AM, Daniel P. Berrange wrote: On Fri, Mar 13, 2009 at 10:19:44AM +, Daniel P. Berrange wrote: On Thu, Mar 12, 2009 at 01:39:13PM -0400, Daniel J Walsh wrote: Libvirt is executing qemu requiring it to execute pulseaudio which would require the folowing permissions, #= svirt_t == allow svirt_t admin_home_t:dir setattr; allow svirt_t admin_home_t:file { read write }; allow svirt_t pulseaudio_port_t:tcp_socket name_connect; allow svirt_t svirt_tmpfs_t:file read; allow svirt_t user_tmpfs_t:file read; Since qemu(svirt_t) is not allowed these permissions, pulseaudio crashes and qemu dies. I don't see it crashing - when I run with a guest with a sound device attached, I see the AVC denials, and QEMU just carries on without a active sound backend AFAICT. FYI I can now reproduce the problem and there's lots of things going on here :-( First, QEMU isn't crashing, libvirtd is killing it because it hangs. It is hanging because pulseaudio can't startup, and a bug in pulseaudio causes it to then get stuck on a condition variable wait that is never notified. THis causes the whole QEMU I/O loop to hang. Pulseaudio has this idea of a 'system instance' which the admin can start to provide a permanently active daemon, thus avoiding the need to autostart. This isn't setup to work at all in Fedora though, and there's no obvious way to disable autostart either :-( In theory though, if we could get autostart to be disabled, then we could allow QEMU access to the shared 'system instance' of PulseAudio just by making the SELinux policy allow connection to its UNIX domain socket in /var/run/. It still tries to create some junk in /root/ even when using the system instance which it really needs to not do. That said it seems to be OK if this stuff fails and still connects to the daemon. I believe you need to run without sound if you are running as root. We can't disable sound unconditonally for root, because not everyone will be using SELinux so its still valid to allow sound cards. I think the focus has to be on stopping QEMU from crashing. It might actually be an SDL bug, rather than a QEMU bug, because I believe its SDL that is responsible for opening the sound devices. I'm going to commit the following temporary patch to Fedora 11 rawhide libvirt builds only, to disable sound cards when SELinux is active and running as root. This will avoid the AVC denials, while we spend more time trying to get PA 'system instance' to work better, without autostart Index: src/qemu_conf.c === RCS file: /data/cvs/libvirt/src/qemu_conf.c,v retrieving revision 1.138 diff -u -p -r1.138 qemu_conf.c --- src/qemu_conf.c 16 Mar 2009 13:54:26 - 1.138 +++ src/qemu_conf.c 17 Mar 2009 15:50:10 - @@ -757,6 +757,20 @@ int qemudBuildCommandLine(virConnectPtr char uuid[VIR_UUID_STRING_BUFLEN]; char domid[50]; char *pidfile; +int skipSound = 0; + +if (driver-securityDriver +driver-securityDriver-name +STREQ(driver-securityDriver-name, selinux) +getuid() == 0) { +static int soundWarned = 0; +skipSound = 1; +if (vm-def-nsounds +!soundWarned) { +soundWarned = 1; +VIR_WARN0(Sound cards for VMs are disabled while SELinux security model is active); +} +} uname_normalize(ut); @@ -1364,7 +1378,8 @@ int qemudBuildCommandLine(virConnectPtr } /* Add sound hardware */ -if (vm-def-nsounds) { +if (vm-def-nsounds +!skipSound) { int size = 100; char *modstr; if (VIR_ALLOC_N(modstr, size+1) 0) Looks good to me. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Problem with the current svirt patch
On 03/13/2009 11:45 AM, Daniel P. Berrange wrote: On Fri, Mar 13, 2009 at 11:03:26AM -0400, Daniel J Walsh wrote: The current svirt patch relabels all disk to the image_t:MCS, which is incorrect. Read Only Disks and Sharable Disks should not be labeled. Also when libvirt is completed running the image it needs to relabel the image back to something sane. Right now it is labeling everything imagelabel:s0, including phisical disk partitions. I considered two ways of labeling the disk back. We can either grab the label when libvirt starts and change it back to this label when ever an image completes or we can ask the system what the label should be. (matcpathcon). I originally coded up the first, but quickly realized if anything went wrong with libvirt labeling like a crash, the labels on disk could be wrong. And libvirt would continuously set them to this wrong label. With matchpathcon, libvirt will at least set them to something sane. So this patch Removes labeling of readonly and shared disks and restores the images label to the system default when the image completes. I would really like to get this in ASAP. Since currently libvirt is relabeing the cdrom to virt_image_t when it is complete as well as physical disks. ACK this all looks sane to me. Daniel Is this going to be merged in? -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] The gui should drop the connection if libvirt stops for any reason like service libvirt stop.
diff -u virtManager/engine.py~ virtManager/engine.py --- virtManager/engine.py~ 2009-03-16 14:49:16.0 -0400 +++ virtManager/engine.py 2009-03-16 14:58:05.0 -0400 @@ -158,6 +158,7 @@ logging.error((Could not refresh connection %s\n % (uri)) + str(sys.exc_info()[0]) + \ + str(sys.exc_info()[1]) + \n + \ traceback.format_exc(sys.exc_info()[2])) +self.connections[uri][connection].close() return 1 def change_timer_interval(self,ignore1,ignore2,ignore3,ignore4): -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] I have no idea why the current version of libvirt works for anyone in enforcing mode.
On 03/13/2009 06:19 AM, Daniel P. Berrange wrote: On Thu, Mar 12, 2009 at 01:39:13PM -0400, Daniel J Walsh wrote: Libvirt is executing qemu requiring it to execute pulseaudio which would require the folowing permissions, #= svirt_t == allow svirt_t admin_home_t:dir setattr; allow svirt_t admin_home_t:file { read write }; allow svirt_t pulseaudio_port_t:tcp_socket name_connect; allow svirt_t svirt_tmpfs_t:file read; allow svirt_t user_tmpfs_t:file read; Since qemu(svirt_t) is not allowed these permissions, pulseaudio crashes and qemu dies. I don't see it crashing - when I run with a guest with a sound device attached, I see the AVC denials, and QEMU just carries on without a active sound backend AFAICT. I believe you need to run without sound if you are running as root. We can't disable sound unconditonally for root, because not everyone will be using SELinux so its still valid to allow sound cards. I think the focus has to be on stopping QEMU from crashing. It might actually be an SDL bug, rather than a QEMU bug, because I believe its SDL that is responsible for opening the sound devices. Daniel How about if we check if you are running with svirt then don't execute the code. Since I do not want to deal with these avc messages. Either they will happen always and I have to dontaudit them in which case a compromised svirt attacking the /root directory would be dontaudited, or people are going to see avc's all the time. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Problem with the current svirt patch
The current svirt patch relabels all disk to the image_t:MCS, which is incorrect. Read Only Disks and Sharable Disks should not be labeled. Also when libvirt is completed running the image it needs to relabel the image back to something sane. Right now it is labeling everything imagelabel:s0, including phisical disk partitions. I considered two ways of labeling the disk back. We can either grab the label when libvirt starts and change it back to this label when ever an image completes or we can ask the system what the label should be. (matcpathcon). I originally coded up the first, but quickly realized if anything went wrong with libvirt labeling like a crash, the labels on disk could be wrong. And libvirt would continuously set them to this wrong label. With matchpathcon, libvirt will at least set them to something sane. So this patch Removes labeling of readonly and shared disks and restores the images label to the system default when the image completes. I would really like to get this in ASAP. Since currently libvirt is relabeing the cdrom to virt_image_t when it is complete as well as physical disks. diff --git a/src/qemu_driver.c b/src/qemu_driver.c index e73bd3a..7d84344 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3756,7 +3756,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto cleanup; } if (driver-securityDriver) -driver-securityDriver-domainSetSecurityImageLabel(dom-conn, vm, dev); +driver-securityDriver-domainSetSecurityImageLabel(dom-conn, vm, dev-data.disk); break; default: @@ -3892,7 +3892,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom, dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) { ret = qemudDomainDetachPciDiskDevice(dom-conn, vm, dev); if (driver-securityDriver) -driver-securityDriver-domainRestoreSecurityImageLabel(dom-conn, vm, dev); +driver-securityDriver-domainRestoreSecurityImageLabel(dom-conn, dev-data.disk); } else qemudReportError(dom-conn, dom, NULL, VIR_ERR_NO_SUPPORT, diff --git a/src/security.h b/src/security.h index ac8d690..8cc2c17 100644 --- a/src/security.h +++ b/src/security.h @@ -32,11 +32,10 @@ typedef virSecurityDriverStatus (*virSecurityDriverProbe) (void); typedef int (*virSecurityDriverOpen) (virConnectPtr conn, virSecurityDriverPtr drv); typedef int (*virSecurityDomainRestoreImageLabel) (virConnectPtr conn, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev); + virDomainDiskDefPtr disk); typedef int (*virSecurityDomainSetImageLabel) (virConnectPtr conn, virDomainObjPtr vm, - virDomainDeviceDefPtr dev); + virDomainDiskDefPtr disk); typedef int (*virSecurityDomainGenLabel) (virConnectPtr conn, virDomainObjPtr sec); typedef int (*virSecurityDomainGetLabel) (virConnectPtr conn, diff --git a/src/security_selinux.c b/src/security_selinux.c index 9e6a442..bbe4bac 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -269,7 +269,7 @@ SELinuxGetSecurityLabel(virConnectPtr conn, } static int -SELinuxSetFilecon(virConnectPtr conn, char *path, char *tcon) +SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon) { char ebuf[1024]; @@ -288,28 +288,47 @@ SELinuxSetFilecon(virConnectPtr conn, char *path, char *tcon) static int SELinuxRestoreSecurityImageLabel(virConnectPtr conn, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev) + virDomainDiskDefPtr disk) { -const virSecurityLabelDefPtr secdef = vm-def-seclabel; +struct stat buf; +security_context_t fcon = NULL; +int rc = -1; +char *newpath = NULL; +const char *path = disk-src; -if (secdef-imagelabel) { -return SELinuxSetFilecon(conn, dev-data.disk-src, default_image_context); +if (disk-readonly || disk-shared) return 0; + +if (lstat(path, buf) != 0) return -1; + +if (S_ISLNK(buf.st_mode)) { +newpath = calloc(1, buf.st_size + 1); +if (!newpath) return -1; + +if (readlink(path, newpath, buf.st_size) 0) goto err; +path = newpath; +if (stat(path, buf) != 0) goto err; } -return 0; + +if (matchpathcon(path, buf.st_mode, fcon) == 0) { +rc = SELinuxSetFilecon(conn, path, fcon); +} +err: +free(fcon); +free(newpath); +return rc; } static int SELinuxSetSecurityImageLabel(virConnectPtr conn, virDomainObjPtr vm, -
Re: [libvirt] I have no idea why the current version of libvirt works for anyone in enforcing mode.
On 03/13/2009 11:16 AM, Cole Robinson wrote: Daniel J Walsh wrote: On 03/13/2009 09:49 AM, Daniel P. Berrange wrote: On Fri, Mar 13, 2009 at 09:44:15AM -0400, Daniel J Walsh wrote: On 03/13/2009 06:19 AM, Daniel P. Berrange wrote: On Thu, Mar 12, 2009 at 01:39:13PM -0400, Daniel J Walsh wrote: Libvirt is executing qemu requiring it to execute pulseaudio which would require the folowing permissions, #= svirt_t == allow svirt_t admin_home_t:dir setattr; allow svirt_t admin_home_t:file { read write }; allow svirt_t pulseaudio_port_t:tcp_socket name_connect; allow svirt_t svirt_tmpfs_t:file read; allow svirt_t user_tmpfs_t:file read; Since qemu(svirt_t) is not allowed these permissions, pulseaudio crashes and qemu dies. I don't see it crashing - when I run with a guest with a sound device attached, I see the AVC denials, and QEMU just carries on without a active sound backend AFAICT. I believe you need to run without sound if you are running as root. We can't disable sound unconditonally for root, because not everyone will be using SELinux so its still valid to allow sound cards. I think the focus has to be on stopping QEMU from crashing. It might actually be an SDL bug, rather than a QEMU bug, because I believe its SDL that is responsible for opening the sound devices. Daniel How about if we check if you are running with svirt then don't execute the code. Since I do not want to deal with these avc messages. Either they will happen always and I have to dontaudit them in which case a compromised svirt attacking the /root directory would be dontaudited, or people are going to see avc's all the time. For that scenario I think it'd be better to make virt-manager prevent addition of sound hardware, since its in a position to give feedback to the user telling them why sound devices aren't allowed. Daniel Well there is no protocol currently to tell virt-manager that the libvirt is running with svirt. I tried to remove a audio device via virt-manager and it does nothing. Also what happens when virt-manager configures a remote libvirt? Does the sound card automatically get added? What does 'does nothing' mean? We can't hotunplug a sound card, you will need to restart the VM for the changes to take effect. virt-manager out of the box does not add a sound card for remote VMs, only local. The default can be changed via Edit-Preferences. - Cole On a running vm, I tried to remove the sound card and nothing happened. I just tried again on a stopped vm and it got removed. If it is not able to remove the sound card on a running VM, then the remove button should be greyed out or it should remove it from the XML and explain to me it will happen on the next restart of the VM. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] I have no idea why the current version of libvirt works for anyone in enforcing mode.
Libvirt is executing qemu requiring it to execute pulseaudio which would require the folowing permissions, #= svirt_t == allow svirt_t admin_home_t:dir setattr; allow svirt_t admin_home_t:file { read write }; allow svirt_t pulseaudio_port_t:tcp_socket name_connect; allow svirt_t svirt_tmpfs_t:file read; allow svirt_t user_tmpfs_t:file read; Since qemu(svirt_t) is not allowed these permissions, pulseaudio crashes and qemu dies. I believe you need to run without sound if you are running as root. diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 03f710f..e0ab039 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1334,8 +1334,13 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT(-full-screen); } -/* Add sound hardware */ -if (vm-def-nsounds) { +/* Add sound hardware iff you are not running as root */ +/* Configuring sound devices in QEMU when doing remote provisioning is + pretty useless really. We need to tunnelling of audio stream from + the QEMU instance to the client machine, over VNC / SPICE, or a + parallel network audio transport. */ + +if (getuid() vm-def-nsounds) { int size = 100; char *modstr; if (VIR_ALLOC_N(modstr, size+1) 0) -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Updated James Morris patch to apply to libvirt-0.6.0 version
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Fri, Feb 27, 2009 at 03:37:55PM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Another patch off latest repository. This patch does not require the XML to include a label, although this is still supported. Implemented most of the comments from Jim. make check and make syntax-check passes, Added seclabeltest.c to run in tests, Updated capability.rng, although not really sure I did it right. This patch will generate random MCS Labels and relabels the image files to match. Seems to work well on F11. I had a few problems with label generation on my F11 machine - perhaps you have a newer version of the patch than the one I applied. I found I need the following additional patch.. - Make domainGenSecurityLabel() give diagnostics for each type of error instead of using generic error message in caller - Change logic bug 'c1 == c2' to 'c1 c2' - Change 'c%d,c%d' to 'c%d.c%d' - it doesn't like labels with the form c210,c502 only wanting c210.c502 This does not make sense. c210,c502 is valid. c210.c502 means include the range. c210, c211, c212...c502. - Fix use of STREQ - no need for == 0 in there I am reworking this code to use INT instead of strings. - Use VIR_FREE/VIR_ALLOC for memory mgmt With this I can successfully start several VMs, and see them all using different contexts, and see the files labelled # ps -xZ | grep qemu | awk '{print $1}' system_u:system_r:qemu_t:s0:c35.c537 system_u:system_r:qemu_t:s0:c210.c502 # ls -Zl /var/lib/libvirt/images/ total 504 -rwxr-xr-x. 1 system_u:object_r:virt_image_t:s0:c210.c502 root root 1073741824 2009-03-03 12:15 demo2.img -rwxr-xr-x. 1 system_u:object_r:virt_image_t:s0:c35.c537 root root 1073741824 2009-03-03 11:49 demo.img Daniel Index: src/qemu_driver.c === RCS file: /data/cvs/libvirt/src/qemu_driver.c,v retrieving revision 1.212 diff -u -p -r1.212 qemu_driver.c --- src/qemu_driver.c 3 Mar 2009 12:03:44 - 1.212 +++ src/qemu_driver.c 3 Mar 2009 12:25:47 - @@ -1316,13 +1316,11 @@ static int qemudStartVMDaemon(virConnect /* If you are using a SecurityDriver and there was no security label in database, then generate a security label for isolation */ -if (vm-def-seclabel.label == NULL driver-securityDriver) { -if (driver-securityDriver-domainGenSecurityLabel(vm) 0) { -qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - %s, _(Unable to generate Security Label)); -return -1; -} -} +if (vm-def-seclabel.label == NULL + driver-securityDriver + driver-securityDriver-domainGenSecurityLabel + driver-securityDriver-domainGenSecurityLabel(conn, vm) 0) +return -1; FD_ZERO(keepfd); Index: src/security.h === RCS file: /data/cvs/libvirt/src/security.h,v retrieving revision 1.1 diff -u -p -r1.1 security.h --- src/security.h3 Mar 2009 09:44:42 - 1.1 +++ src/security.h3 Mar 2009 12:25:47 - @@ -37,7 +37,8 @@ typedef int (*virSecurityDomainRestoreIm typedef int (*virSecurityDomainSetImageLabel) (virConnectPtr conn, virDomainObjPtr vm, virDomainDeviceDefPtr dev); -typedef int (*virSecurityDomainGenLabel) (virDomainObjPtr sec); +typedef int (*virSecurityDomainGenLabel) (virConnectPtr conn, + virDomainObjPtr sec); typedef int (*virSecurityDomainGetLabel) (virConnectPtr conn, virDomainObjPtr vm, virSecurityLabelPtr sec); Index: src/security_selinux.c === RCS file: /data/cvs/libvirt/src/security_selinux.c,v retrieving revision 1.1 diff -u -p -r1.1 security_selinux.c --- src/security_selinux.c3 Mar 2009 10:06:49 - 1.1 +++ src/security_selinux.c3 Mar 2009 12:25:47 - @@ -24,6 +24,9 @@ #include util.h #include memory.h + +#define VIR_FROM_THIS VIR_FROM_SECURITY + static char default_domain_context[1024]; static char default_image_context[1024]; #define SECURITY_SELINUX_VOID_DOI 0 @@ -45,10 +48,11 @@ mcsAdd(const char *mcs) struct MCS *ptr; for (ptr = mcsList; ptr; ptr = ptr-next) { -if (STREQ(ptr-mcs, mcs) == 0) +if (STREQ(ptr-mcs, mcs)) return -1; } -ptr = malloc(sizeof(struct MCS)); +if (VIR_ALLOC(ptr) 0) +return -1; ptr-mcs = strdup(mcs); ptr-next = mcsList; mcsList = ptr; @@ -62,7 +66,7 @@ mcsRemove(const char *mcs) struct MCS *ptr = NULL
[libvirt] Re: PATCH: Mark seclabel as dynamic generated, or statically pre-defined
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: This patch implements the behaviour I was refering to earlier, whereby the domain XML explicitly says whether the security label is a statically pre-defined one, or dynamically generated on VM boot by libvirtd So when creating a new guest, apps like virt-install have 2 options: - Leave out the seclabel tag completely - If no security driver is active, just works as normal unconfined VM I want to make sure of terminology. The VM will be confined, but will not be separated. IE the host machine will be protected from the VM, while other VM's will not be. svirt is concerned with protecting VM's from each other. Normal SELinux protects the Apache server from rogue VMs. If we want to have a truly unconfined vm, we need additional policy. I would suggest permissive mode or a permissive domain. semanage permissive -a qemu_t Would give you this. - If a security driver is active, a dynamic seclabel is generated seclabel type='dynamic' model='selinux' labelsystem_u:system_r:qemu_t:s0:c424,c719/label imagelabelsystem_u:object_r:virt_image_t:s0:c424,c719/imagelabel /seclabel - Add an explicit seclabel tag with type='static' attribute - Security driver uses the defined label imagelabel seclabel type='static' model='selinux' labelsystem_u:system_r:qemu_t:s0:c25,c100/label imagelabelsystem_u:system_r:virt_image_t:s0:c25,c100/imagelabel /seclabel Not sure imagelabel is required in the second case since, libvirt will not change the label. This is the biggest difference between static and dynamic, static will never run setfilecon, while dynamic runs it at startup and shutdown. DYNAMIC: start setfilecon(image, imagelabel) setexeccon(label) exec() Finish ChildExit() setfilecon(image, DEFAULT_LABEL) Exit() STATIC: start setexecon(label) exec() Finish: ChildExit() Exit() A static seclabel is visible in the XML, at all times, whether the VM is active or inactive. A dynamic seclabel is only visible when the VM is running, since it is auto-generated at VM boot. If you migrate the VM, or save/restore it, the dynamic seclabel will change on each boot. The seclabel isn't visible when not running, or if asking for the inactive XML dump This patch implements parsing of the 'type' attribute, and makes the seclabel generation key off this attribute. It also adds the 'imagelabel' XML element, since that was being used internally, but was not including in the XML output, or parsing routines, making it impossible to specify a pre-defined image label or see the dyanmic one domain_conf.c | 65 +++-- domain_conf.h |9 +++ qemu_driver.c | 13 -- security_selinux.c |1 4 files changed, 73 insertions(+), 15 deletions(-) Daniel Index: src/domain_conf.c === RCS file: /data/cvs/libvirt/src/domain_conf.c,v retrieving revision 1.70 diff -u -p -r1.70 domain_conf.c --- src/domain_conf.c 3 Mar 2009 09:44:42 - 1.70 +++ src/domain_conf.c 3 Mar 2009 15:36:02 - @@ -168,6 +168,10 @@ VIR_ENUM_IMPL(virDomainState, VIR_DOMAIN shutoff, crashed) +VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST, + dynamic, + static) + #define virDomainReportError(conn, code, fmt...) \ virReportErrorHelper(conn, VIR_FROM_DOMAIN, code, __FILE__,\ __FUNCTION__, __LINE__, fmt) @@ -1847,24 +1851,46 @@ static int virDomainLifecycleParseXML(vi static int virSecurityLabelDefParseXML(virConnectPtr conn, const virDomainDefPtr def, -xmlXPathContextPtr ctxt) +xmlXPathContextPtr ctxt, +int flags) { char *p; if (virXPathNode(conn, ./seclabel, ctxt) == NULL) return 0; -p = virXPathStringLimit(conn, string(./seclabel/label[1]), +p = virXPathStringLimit(conn, string(./seclabel/@type), VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) goto error; -def-seclabel.label = p; - -p = virXPathStringLimit(conn, string(./seclabel/@model), -VIR_SECURITY_MODEL_BUFLEN-1, ctxt); -if (p == NULL) +if ((def-seclabel.type = virDomainSeclabelTypeFromString(p)) 0) goto error; -def-seclabel.model = p; +VIR_FREE(p); + +/* Only parse details, if using static labels, or + * if the 'live' VM XML is requested + */ +if (def-seclabel.type == VIR_DOMAIN_SECLABEL_STATIC || +
Re: [libvirt] Updated James Morris patch to apply to libvirt-0.6.0 version
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Another patch off latest repository. This patch does not require the XML to include a label, although this is still supported. Implemented most of the comments from Jim. make check and make syntax-check passes, Added seclabeltest.c to run in tests, Updated capability.rng, although not really sure I did it right. This patch will generate random MCS Labels and relabels the image files to match. Seems to work well on F11. I will back port some policy to allow it to work on F10. I think we need a mechanism in libvirtd.conf to turn this off. And allow perhaps three modes. svirt=Disabled. No Security Driver. svirt=MLS (Requires context in xml, no relabel of disks) svirt=Standard, (If no XML label, then random generate one and reset file context). How should I read config from libvirt.conf and and not enable he SecurityModel? http://people.fedoraproject.org/~dwalsh/SELinux/svirt.patch -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmoTyMACgkQrlYvE4MpobPuHwCgkJqZenEwCWov96tTv+h3x8ec wmEAoMecJotrN009adtO3JOmkNLR3uXN =waHN -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Playing around with libvirt/virt-manager.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Mon, Feb 23, 2009 at 05:46:36PM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Tue, Feb 17, 2009 at 04:52:08PM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Looks like qemu launched from libvirt wants to create pulseaudito files under /root/.pulse directory. Hmm, that sounds bad - it should not do this. Seems strange, and we might want to consider changing the homedir for each qemu launched by libvirt. /var/run/libvirt/qemu/DOMAIN for example. It seems qemu has to be able to write here or it blows up. What version of QEMU is this with - I think that needs to be fixed in QEMU Will add selinux policy for now. I'd prefer not - AFAIK, QEMU should not be doing this - if PulseAudio is desired when running as root, then the admin should start it ahead of time, not have QEMU auto-spawn it. PA should only auto-spawn itself if running non-root in the desktop session IMHO. Daniel For some reason it is also trying to create /root/.kde directory and then link a socket to /tmp/ksocket-root. Everything seems to be caused by sound. I hacked out a libvirt that does not add the -esound qualifier to qemu and every thing works correctly in svirt with SELinux in enforcing mode. Not really sure what the proper way to handle this? Should libvirt be execing qemu with the sound device if it is running as root? Will this work with the sound devices? What happens if libvirt is remote? Configuring sound devices in QEMU when doing remote provisioning is pretty useless really. We need to tunnelling of audio stream from the QEMU instance to the client machine, over VNC / SPICE, or a parallel network audio transport. I'm inclined to say we should set the SDL env variable to disable sound for instances run as root, and only use sound when launching the per-user unprivileged instances which are able to properly integrate with the sound daemon provided by the desktop session (ESD / PulseAudio / KDE) Daniel How about a patch like this for now, took some liberties with your quote? git diff origin qemu_conf.c diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 6f58ee8..32cdba2 100644 - --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1327,8 +1327,13 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT(-full-screen); } - -/* Add sound hardware */ - -if (vm-def-nsounds) { +/* Add sound hardware iff you are not running as root */ +/* Configuring sound devices in QEMU when doing remote provisioning is + pretty useless really. We need to tunnelling of audio stream from + the QEMU instance to the client machine, over VNC / SPICE, or a + parallel network audio transport. */ + +if (getuid() vm-def-nsounds) { int size = 100; char *modstr; if (VIR_ALLOC_N(modstr, size+1) 0) -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmkBEYACgkQrlYvE4MpobPD4QCeP/VNSkebMd7b86t+n8fx+T+s +w8AoJGkLnrd1hLUxFU/6o9zRSv2WQUE =4CAZ -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Patch to python-virtinst to allow it to choose svirt labels
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Fri, Feb 20, 2009 at 01:52:31PM -0500, Daniel J Walsh wrote: +def _default_seclabels(self): +try: +fd = open(selinux.selinux_virtual_domain_context_path(), 'r') +except OSError, (err_no, msg): +raise RuntimeError, \ +failed to SELinux virtual domains context: %s: %s %s % (selinux.selinux_virtual_domain_context_path(),err_no, msg) + +label = fd.read() +fd.close() +try: +fd = open(selinux.selinux_virtual_image_context_path(), 'r') +except OSError, (err_no, msg): +raise RuntimeError, \ +failed to SELinux virtual domains context: %s: %s %s % (selinux.selinux_virtual_domain_context_path(), err_no, msg) + +image = fd.read() +fd.close() + +return (label, image) Opening local files in virt-install code is an approach we're trying to get rid of, because it prevents you doing remote provisioning. eg running virt-install on your laptop to provision on a server in the data center. Ok the only goal here is a mechanism for the selinux policy writer to tell the tools what label to run virtual processes as, and what label to label there images. +def is_conflict_seclabel(self, conn, seclabel): + +check if security label is in use by any other VMs on passed +connection. + +@param conn: connection to check for collisions on +@type conn: libvirt.virConnect + +@param seclabel: Security Label +@type str: Security label + +@return: True if a collision, False otherwise +@rtype: C{bool} + +if not seclabel: +return False + +vms = [] +# get working domain's name +ids = conn.listDomainsID() +for i in ids: +try: +vm = conn.lookupByID(i) +vms.append(vm) +except libvirt.libvirtError: +# guest probably in process of dieing +logging.warn(Failed to lookup domain id %d % i) +# get defined domain +names = conn.listDefinedDomains() +for name in names: +try: +vm = conn.lookupByName(name) +vms.append(vm) +except libvirt.libvirtError: +# guest probably in process of dieing +logging.warn(Failed to lookup domain name %s % name) + +count = 0 +for vm in vms: +doc = None +try: +doc = libxml2.parseDoc(vm.XMLDesc(0)) +except: +continue +ctx = doc.xpathNewContext() +try: +try: +label = ctx.xpathEval(/domain/seclabel/label/) +if label[0].content == seclabel: +count += 1 +break +except: +continue +finally: +if ctx is not None: +ctx.xpathFreeContext() +if doc is not None: +doc.freeDoc() +if count 0: +return True +else: +return False + +def _get_random_mcs(self): +f1 = random.randrange(1024) +f2 = random.randrange(1024) +if f1 f2: +return s0:c%s,c%s % (f1, f2) +else: +if f1 == f2: +return s0:c%s % f1 +else: +return s0:c%s,c%s % (f2, f1) + +def gen_seclabels(self): +mcs = self._get_random_mcs() +con = self.default_seclabel.split(':') +seclabel = %s:%s:%s:%s % (con[0], con[1], con[2], mcs) +con = self.default_imagelabel.split(':') +imagelabel = %s:%s:%s:%s % (con[0], con[1], con[2], mcs) +return (seclabel, imagelabel) Now that I look at this I'm not so sure its a good idea to have the client code responsible for allocating the 'mcs' level part of the label. I think virt-install should only specify the first bit of the label 'root:system_r:qemu_t' and that libvirt should be doing allocation of the mcs level on the fly at domain startup. For sVirt 1.0 our assumption is that mcs levels will only be unique within scope of running VMs on a each host machine. If we are including the mcs level in the label we pass into the XML, we are going to create a number of headaches for ourselves. - The inactive config written in /etc/libvirt/qemu contains an allocate mcs level even though it doesn't need it unless it is running. One of 500,000, which we can bounce up by adding one or more categories, currently we are only using 2, we could use as many as 1024. - If someone copies this config to another machine, then the mcs level in the config
Re: [libvirt] Updated James Morris patch to apply to libvirt-0.6.0 version
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Jim Meyering wrote: Daniel P. Berrange wrote: Just spotted one serious problem we need to address. The method 'qemudStartVMDaemon' quoted here is where we set the security label: ... Good catch. To use this, we'd make qemudStartVM() pass in a virExecHook callback which does the call to qemudDomainSetSecurityLabel() Daniel diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -291,6 +291,7 @@ virEnumToString; virEventAddHandle; virEventRemoveHandle; virExec; +virExecWithHook; Looks right to me. ACK. This patch looks good to me and I have already rebased the svirt patch to match, waiting for this to get applied, then I will resubmit my patch. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmiqhYACgkQrlYvE4MpobMkFACeMWEmQfwKMe4Cn7NNikPk9f3+ zlMAoK7DeqbpTKho4Kw/bZtuA86vtMJK =eVSb -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: Patch to python-virtinst to allow it to choose svirt labels
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Cole Robinson wrote: Daniel J Walsh wrote: The patch didn't apply to latest upstream (there has been a lot of code movement recently). I rediffed the patch to apply against current tip, and made a few minor changes that don't change the overall result (mentioned below). Also found at least one big bug in python-virtinst, VirtualDisk.py was dropping the / between dirname and basename of installation object, when you told it to create the object. This is already fixed upstream. You also had a minor bug fix in the Installer class that is fixed as well, so I dropped both pieces. Ok, My patch was against the F11 released version obviously. I think we want to have a big switch stored in libvirt somewhere saying whether or not we want isolated virtual machines. I think this should really be at the management tool level (i.e, virt-manager). libvirt should be dumb in this respect, being passed a label via the xml and doing with it what it's told. I disagree. The management of labeling and the database are too difficult, since the user might later want to turn it on. We would not be able to change from one setting to the other if the labels and labeling are not in place. The current rawhide policy would work with SELinux whether or not the libvirt calls the setexeccon call. So we can easily turn on the separated virtual machines and turn it off. I figure, virt-manager can have an option in Edit-Preferences, something like Isolate virtual machines with SELinux. Defaults to on. If selinux isn't running, we disable the option with a tooltip explaining why (or maybe hide it altogether). If the option is enabled, virt-manager will assign labels to VMs at install time, and check all active connections to avoid label collisions. More advanced behavior can come later (assigning specific labels, some sort of collision resolution with VMs on new connections, etc.) But now if you turn it off after adding a couple of machines, you would have some with labels and some without. Updated patch attached, I'll reply with patch specific comments later. Thanks, Cole -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmi5jgACgkQrlYvE4MpobMY3QCfQjDAIyTDzwv7AnAu5GqycZoh GZAAn1Q8oFb5bxDAuvov8jmYnX3OkrkA =y1Y1 -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Playing around with libvirt/virt-manager.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Tue, Feb 17, 2009 at 04:52:08PM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Looks like qemu launched from libvirt wants to create pulseaudito files under /root/.pulse directory. Hmm, that sounds bad - it should not do this. Seems strange, and we might want to consider changing the homedir for each qemu launched by libvirt. /var/run/libvirt/qemu/DOMAIN for example. It seems qemu has to be able to write here or it blows up. What version of QEMU is this with - I think that needs to be fixed in QEMU Will add selinux policy for now. I'd prefer not - AFAIK, QEMU should not be doing this - if PulseAudio is desired when running as root, then the admin should start it ahead of time, not have QEMU auto-spawn it. PA should only auto-spawn itself if running non-root in the desktop session IMHO. Daniel For some reason it is also trying to create /root/.kde directory and then link a socket to /tmp/ksocket-root. Everything seems to be caused by sound. I hacked out a libvirt that does not add the -esound qualifier to qemu and every thing works correctly in svirt with SELinux in enforcing mode. Not really sure what the proper way to handle this? Should libvirt be execing qemu with the sound device if it is running as root? Will this work with the sound devices? What happens if libvirt is remote? -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmjJ0wACgkQrlYvE4MpobOmAgCg2fWTndW+0Y7ttDqLEydWc3uf HkMAoM/VBEaS+elMoY0WarYV2xJPpW6A =DP/9 -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Updated James Morris patch to apply to libvirt-0.6.0 version
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: Just spotted one serious problem we need to address. The method 'qemudStartVMDaemon' quoted here is where we set the security label: On Tue, Feb 17, 2009 at 11:20:17AM -0500, Daniel J Walsh wrote: @@ -1178,6 +1237,16 @@ static int qemudStartVMDaemon(virConnect return -1; } +/* + * Set up the security label for the domain here, before doing + * too much else. + */ +if (qemudDomainSetSecurityLabel(conn, driver, vm) 0) { +qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _(Failed to set security label)); +return -1; +} + if (qemudExtractVersionInfo(emulator, NULL, qemuCmdFlags) 0) { Which ultimately calls the following method, which sets the context to use for the next exec call. +static int +SELinuxSecurityDomainSetSecurityLabel(virConnectPtr conn, + virSecurityDriverPtr drv, + const virSecurityLabelDefPtr secdef) +{ +/* TODO: verify DOI */ + +if (!STREQ(drv-name, secdef-model)) { +virSecurityReportError(conn, VIR_ERR_ERROR, + _(%s: security label driver mismatch: + \'%s\' model configured for domain, but + hypervisor driver is \'%s\'.), + __func__, secdef-model, drv-name); +return -1; +} + +if (setexeccon(secdef-label) == -1) { +virSecurityReportError(conn, VIR_ERR_ERROR, + _(%s: unable to set security context + '\%s\': %s.), __func__, secdef-label, + strerror(errno)); +return -1; +} +return 0; +} The problem is that between the point where we set the exec context, and the place where QEMU is finally exec'd, there is scope for several other programs to be exec'd. Also there are other threads running concurrently, and I'm under whether 'setexeccon' scope is per thread or per process. I think we need to move place where we set the exec context to after the fork() call, ideally to be the very last call made before the actual execve(). We do not currently have an easy way todo this, but I have the exact same problem in my patches to integrate with cgroups - I need to add the new PID to the appropriate cgroup immediately before exec'ing. So i suggest the following patch whichs a generic callback to the virExec() call, so we can implant the neccessary logic after the fork() and just before the real execve(), and safely in the child process. To use this, we'd make qemudStartVM() pass in a virExecHook callback which does the call to qemudDomainSetSecurityLabel() Daniel diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -291,6 +291,7 @@ virEnumToString; virEventAddHandle; virEventRemoveHandle; virExec; +virExecWithHook; virSetNonBlock; virFormatMacAddr; virGetHostname; diff --git a/src/util.c b/src/util.c --- a/src/util.c +++ b/src/util.c @@ -199,7 +199,10 @@ __virExec(virConnectPtr conn, const fd_set *keepfd, pid_t *retpid, int infd, int *outfd, int *errfd, - int flags) { + int flags, + virExecHook hook, + void *data) +{ pid_t pid; int null, i, openmax; int pipeout[2] = {-1,-1}; @@ -411,6 +414,9 @@ __virExec(virConnectPtr conn, childerr != childout) close(childerr); +if (hook) +(hook)(data); + if (envp) execve(argv[0], (char **) argv, (char**)envp); else @@ -445,13 +451,16 @@ __virExec(virConnectPtr conn, } int -virExec(virConnectPtr conn, -const char *const*argv, -const char *const*envp, -const fd_set *keepfd, -pid_t *retpid, -int infd, int *outfd, int *errfd, -int flags) { +virExecWithHook(virConnectPtr conn, +const char *const*argv, +const char *const*envp, +const fd_set *keepfd, +pid_t *retpid, +int infd, int *outfd, int *errfd, +int flags, +virExecHook hook, +void *data) +{ char *argv_str; if ((argv_str = virArgvToString(argv)) == NULL) { @@ -462,7 +471,21 @@ virExec(virConnectPtr conn, VIR_FREE(argv_str); return __virExec(conn, argv, envp, keepfd, retpid, infd, outfd, errfd, - flags); + flags, hook, data); +} + +int +virExec(virConnectPtr conn, +const char *const*argv, +const char *const*envp
[libvirt] Patch to python-virtinst to allow it to choose svirt labels
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Also found at least one big bug in python-virtinst, VirtualDisk.py was dropping the / between dirname and basename of installation object, when you told it to create the object. I think we want to have a big switch stored in libvirt somewhere saying whether or not we want isolated virtual machines. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkme++4ACgkQrlYvE4MpobM5ewCeP3iaq3HwT/Rw71E2YElbxKyg 66gAoJaCUkQkBvJz80wjztYiwOGsAKaj =GlDN -END PGP SIGNATURE- diff -r -u virtinst-0.400.1/virtinst/Guest.py virtinst-0.400.1.new/virtinst/Guest.py --- virtinst-0.400.1/virtinst/Guest.py 2009-01-26 14:33:25.0 -0500 +++ virtinst-0.400.1.new/virtinst/Guest.py 2009-02-19 17:36:35.0 -0500 @@ -32,6 +32,8 @@ import __builtin__ import CapabilitiesParser import VirtualDevice +import selinux +import random import osdict from VirtualDisk import VirtualDisk @@ -315,8 +317,9 @@ self._install_disk = None # VirtualDisk that contains install media if type is None: -type = xen -self.type = type +self.type = xen +else: +self.type = type if not location is None: self.location = location @@ -526,6 +529,7 @@ self._vcpus = None self._cpuset = None self._graphics_dev = None +self._seclabel = None self._os_type = None self._os_variant = None @@ -552,12 +556,34 @@ self.disknode = None # this needs to be set in the subclass +self.default_seclabel , self.default_imagelabel = self._default_seclabels() + +while self._seclabel == None: +seclabel, imagelabel = self.gen_seclabels() +if self.is_conflict_seclabel(self.conn, seclabel): +continue +self.set_seclabel(seclabel) +self.set_imagelabel(imagelabel) + def get_installer(self): return self._installer def set_installer(self, val): self._installer = val installer = property(get_installer, set_installer) +# Security context used to secure guest image +def get_imagelabel(self): +return self._imagelabel +def set_imagelabel(self, val): +self._imagelabel = val +imagelabel = property(get_imagelabel, set_imagelabel) + +# Security context used to secure guest process +def get_seclabel(self): +return self._seclabel +def set_seclabel(self, val): +self._seclabel = val +seclabel = property(get_seclabel, set_seclabel) def get_type(self): return self._installer.type @@ -565,7 +591,6 @@ self._installer.type = val type = property(get_type, set_type) - # Domain name of the guest def get_name(self): return self._name @@ -750,7 +775,7 @@ if enabled not in (True, False): raise ValueError, _(Graphics enabled must be True or False) -if enabled == True: +if enabled: gdev = VirtualGraphics(type=gtype) if port: gdev.port = port @@ -807,9 +832,23 @@ Ensure that devices are setup for disk in self._install_disks: disk.setup(progresscb) +# Not sure of this, might want to put this in VirtualDisk class +selinux.setfilecon(disk.path, self._imagelabel) for nic in self._install_nics: nic.setup(self.conn) +def _get_seclabel_xml(self): +xml = +if self._seclabel != None: +xml = + seclabel model='selinux' +label%s/label +image%s/image + /seclabel + % ( self._seclabel, self._imagelabel) +print xml +return xml + def _get_disk_xml(self, install=True): Return xml for disk devices (Must be implemented in subclass) raise NotImplementedError @@ -899,6 +938,7 @@ devices %(devices)s /devices + %(secxml)s /domain % { type: self.type, name: self.name, \ @@ -909,7 +949,8 @@ maxramkb: self.maxmemory * 1024, \ devices: self._get_device_xml(install), \ osblob: osblob, \ -action: action } +action: action, \ +secxml: self._get_seclabel_xml()} def start_install(self, consolecb=None, meter=None, removeOld=False, @@ -1026,6 +1067,108 @@ if self.domain is not None: raise RuntimeError, _(Domain has already been started!) +def _default_seclabels(self): +try: +fd = open(selinux.selinux_virtual_domain_context_path(), 'r') +except OSError, (err_no, msg): +raise RuntimeError, \ +failed to SELinux virtual domains context: %s: %s %s % (selinux.selinux_virtual_domain_context_path(),err_no, msg) + +label = fd.read() +fd.close() +try: +fd =
Re: [libvirt] Playing around with libvirt/virt-manager.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Tue, Feb 17, 2009 at 04:52:08PM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Looks like qemu launched from libvirt wants to create pulseaudito files under /root/.pulse directory. Hmm, that sounds bad - it should not do this. Seems strange, and we might want to consider changing the homedir for each qemu launched by libvirt. /var/run/libvirt/qemu/DOMAIN for example. It seems qemu has to be able to write here or it blows up. What version of QEMU is this with - I think that needs to be fixed in QEMU qemu-0.9.1-12.f11 Will add selinux policy for now. I'd prefer not - AFAIK, QEMU should not be doing this - if PulseAudio is desired when running as root, then the admin should start it ahead of time, not have QEMU auto-spawn it. PA should only auto-spawn itself if running non-root in the desktop session IMHO. Daniel I can not get this to run without adding policy, if qemu is denied access to /root it blows up. Sadly, It needs r/w access so running two qemu with different labels, the second will blow up since it can not write to the pulseaudio files created by the first. Added bug https://bugzilla.redhat.com/show_bug.cgi?id=486112 I still think it might be a good idea to create a homedir for each qemu from libvirt, this way they can read/write contents and as new libraries get sucked into qemu, it will just work HOME=/var/run/libvirt/qemu/DOMAIN qemu-kvm ... -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmcIj4ACgkQrlYvE4MpobORlQCdElHKK4GPNFkP/ktx/ppHheZM 4ZwAoJAKiIgRvcmkFZJ9ArirwTtI0qOR =fOX7 -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Updated James Morris patch to apply to libvirt-0.6.0 version
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Jim Meyering wrote: Daniel J Walsh dwa...@redhat.com wrote: [I removed the 1900+ lines of useless context] Ok, I have added your patches and make syntax-check succeeds except it does not like po_check --- po/POTFILES.in +++ po/POTFILES.in @@ -22,8 +22,6 @@ src/qemu_conf.c src/qemu_driver.c src/remote_internal.c -src/security.c -src/security_selinux.c src/storage_backend.c src/storage_backend_disk.c src/storage_backend_fs.c Makefile.maint: you have changed the set of files with translatable diagnostics; apply the above patch Since these files add translations, what do I need to do to get this to pass? That check passes for me, but I'm using git. Are you using CVS? If so, have you cvs-added those two files? If you have not, that would explain it. Otherwise, the check (which runs a script to search all version-controlled files) may need to be adjusted. Ok that is what I missed, I was not checking them in before running the test. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmbINsACgkQrlYvE4MpobP4FgCffugZzfPrm/it38q0wVLsJRiN T6cAn1/Vdfmhoo2UF8hbaq2c9hH9/3qP =1iaw -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Playing around with libvirt/virt-manager.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Looks like qemu launched from libvirt wants to create pulseaudito files under /root/.pulse directory. Seems strange, and we might want to consider changing the homedir for each qemu launched by libvirt. /var/run/libvirt/qemu/DOMAIN for example. It seems qemu has to be able to write here or it blows up. Will add selinux policy for now. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmbMYgACgkQrlYvE4MpobNSNACeMXYavKlquo7mgnZLVk6GqwFh Of4AnjbAIWQ6xb62ofG/opEGEqhz6IMF =MFto -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] I have been looking over James Morris patches for svirt.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 What is the process to get them into libvirt? I have begun to look at the second componant of the libvirt change. James patch, allows libvirt to read the SELinux context out of the xml database and execute qemu with the context. The second componant is to pass the context of the image(s) and allow libvirt to not only set the image, but also update the default labels on disk, so a relabel will not change the context. I have some patches to do this, but want to make sure the original patches are acceptable? The last changes and perhaps the most difficult is figuring out how to get the labels into the XML database in the first place. I guess virt_manager will somehow figure out what the labels should be and assign them. Dan -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmMeZEACgkQrlYvE4MpobMZcwCePmE1aF3zJURS2up7ERoxNU4V Ho8AoIwBMN7/f4zBCIDTpbnLg13FfPOG =rHH5 -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] (resend) Problems with virt-manager checking access on virtual images.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Met with Cole this morning and we talked about how SELinux can cause people headaches when installing virtual images from random locations. User downloads a iso image to his home directory and then uses virt-manager to install it. Problem is when the user has the whole thing configured, virt-manager tells libvirt to install. It executes qemu and SELinux prevents qemu from reading the iso image because it is labeled user_home_t and qemu is not allowed to read the contents of the home directory. qemu blows up with permission denied and the user is at a loss to what just happened. As we talked we realised this is not just an SELinux problem, but would also happen if a use had an nfs homedir or potentially a samba home directory where root was not allowed access. Also pam_namespace would cause problems, in the /tmp or /home/dwalsh would not be the same for root as they are for the user. One solution to the SELinux problem is to have a label that virt-manager could apply to the iso image (virt_content_ro_t). This would allow qemu to access the file as long as it had search access to the path to the image. solving most of these problems. But the user could still have an access problem that would be tough to diagnose. We came up with the idea of a running a simple helper application to check read access to the image file. During the install, virt-manager could tell libvirt to verify access by executing qemuaccess /home/dwalsh/windows.iso. If this executable was labeled qemu_exec_t like the other qemu images the same SELinux transitions would happen and we could instantly figure out if SELinux was going to cause problems. As a side benefit we could also check if NFS or samba would cause a problems. If qemuaccess failed, virt-manager could put up a diagnostic message suggesting SELinux, NFS, or Samba might be a problem, and the user could move the iso image to some directory like /var/lib/libvirt/isos/, where libirt would have access. I have attached a version that could solve the problem. Comments? Dan -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmC9NAACgkQrlYvE4MpobO8egCgpOlWtlSSrC+TPK41fWC9YPWg xwoAn2zYpk5ODoGhl5PXnwkltBKVjO1m =PYqR -END PGP SIGNATURE- #include stdio.h #include stdlib.h #include string.h #include errno.h main(int argc, char **argv) { int rc = 0; int i=0; if (argc 2) { fprintf(stderr, %s: image file(s) required\n, argv[0]); exit(-1); } for(i=1; i argc; i++) { FILE *fp = fopen(argv[i], r); if (!fp) { fprintf(stderr, %s: Can not read %s: %s\n, argv[0], argv[i], strerror(errno)); rc = -1; } else { fclose(fp); } } return rc; } qemuaccess.c.sig Description: Binary data qemuaccess.c.sig Description: PGP signature qemuaccess.c.sig.sig Description: PGP signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] (resend) Problems with virt-manager checking access on virtual images.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Fri, Jan 30, 2009 at 07:38:40AM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Met with Cole this morning and we talked about how SELinux can cause people headaches when installing virtual images from random locations. User downloads a iso image to his home directory and then uses virt-manager to install it. Problem is when the user has the whole thing configured, virt-manager tells libvirt to install. It executes qemu and SELinux prevents qemu from reading the iso image because it is labeled user_home_t and qemu is not allowed to read the contents of the home directory. qemu blows up with permission denied and the user is at a loss to what just happened. As we talked we realised this is not just an SELinux problem, but would also happen if a use had an nfs homedir or potentially a samba home directory where root was not allowed access. Also pam_namespace would cause problems, in the /tmp or /home/dwalsh would not be the same for root as they are for the user. One solution to the SELinux problem is to have a label that virt-manager could apply to the iso image (virt_content_ro_t). This would allow qemu to access the file as long as it had search access to the path to the image. solving most of these problems. But the user could still have an access problem that would be tough to diagnose. We came up with the idea of a running a simple helper application to check read access to the image file. During the install, virt-manager could tell libvirt to verify access by executing qemuaccess /home/dwalsh/windows.iso. If this executable was labeled qemu_exec_t like the other qemu images the same SELinux transitions would happen and we could instantly figure out if SELinux was going to cause problems. As a side benefit we could also check if NFS or samba would cause a problems. If qemuaccess failed, virt-manager could put up a diagnostic message suggesting SELinux, NFS, or Samba might be a problem, and the user could move the iso image to some directory like /var/lib/libvirt/isos/, where libirt would have access. I don't particularly like the idea of running another program to check this because SELinux context isn't the only thing which will potentially prevent QEMU accessing the disk image. CGroups device policy make prevent it. A setuid() call in QEMU to drop privileges, may prevent it. It may be asked to open it read-write, and only be able to open it read-only . It may want to take a lock on the image, but it already be locked, and so on. QEMU does already report pretty much the same error message as the qemuaccess program does when it is unable to access a disk image # virsh start demo libvir: QEMU error : internal error unable to start guest: qemu: could not open disk image /home/berrange/Fedora-9-i686-Live.iso IMHO, this all stems from a mistake I made when designing the original virt-manager UI. Namely, I should never have used a generic file dialog which allows selection of disk images anywhere on the host. It was wrong for general disk images, in just the same way that its wrong for ISO images. And absolutely useless for remote provisioning. With the storage pool APIs we have the opportunity to fix this in a way helps not only the SELinux case, but also the app in general. Instead of showing a generic file dialog, we should just show a dialog displaying the configured storage pools, and allow the user to pick an ISO out of one of the the pools. virt-manager should offer to remember which storage pool contains installable ISO images, and which should be used by default for disk images. We already have /var/lib/libvirt/images by default for disk images, and should add /var/lib/libvirt/isos too. If a user downloads an ISO to their home directory, we could easily have a option in the UI for 'Import an ISO file to the pool' which would move it into the correct location. NB, here I am talking about use of the system-wide QEMU driver instance of 'qemu:///system', which runs privileged on the host. This is really intended at server deployments of virt, but we have been happening to use it for general ad-hoc desktop usage too in Fedora. There is also a per-user 'qemu://session' UI where both libvirtd and the QEMU guests run unprivileged as that user's UID. In that scenario the storage pools should use something like $HOME/VirtualMachines/images and $HOME/VirtualMachines/ISOs'as the default pools for disk ISOs respectively. I would like to see Fedora use qemu:///session by default for generic desktop virt usage, in which case everything runs as that UID, and keeps everything in $HOME. Regards, Daniel We discussed this also. The only problem I see here is the additional disk usage, since you would permanently keeping the iso, even though you might only be using it once. What about removable media
Re: [libvirt] (resend) Problems with virt-manager checking access on virtual images.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Fri, Jan 30, 2009 at 09:06:38AM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: I don't particularly like the idea of running another program to check this because SELinux context isn't the only thing which will potentially prevent QEMU accessing the disk image. CGroups device policy make prevent it. A setuid() call in QEMU to drop privileges, may prevent it. It may be asked to open it read-write, and only be able to open it read-only . It may want to take a lock on the image, but it already be locked, and so on. QEMU does already report pretty much the same error message as the qemuaccess program does when it is unable to access a disk image # virsh start demo libvir: QEMU error : internal error unable to start guest: qemu: could not open disk image /home/berrange/Fedora-9-i686-Live.iso IMHO, this all stems from a mistake I made when designing the original virt-manager UI. Namely, I should never have used a generic file dialog which allows selection of disk images anywhere on the host. It was wrong for general disk images, in just the same way that its wrong for ISO images. And absolutely useless for remote provisioning. With the storage pool APIs we have the opportunity to fix this in a way helps not only the SELinux case, but also the app in general. Instead of showing a generic file dialog, we should just show a dialog displaying the configured storage pools, and allow the user to pick an ISO out of one of the the pools. virt-manager should offer to remember which storage pool contains installable ISO images, and which should be used by default for disk images. We already have /var/lib/libvirt/images by default for disk images, and should add /var/lib/libvirt/isos too. If a user downloads an ISO to their home directory, we could easily have a option in the UI for 'Import an ISO file to the pool' which would move it into the correct location. NB, here I am talking about use of the system-wide QEMU driver instance of 'qemu:///system', which runs privileged on the host. This is really intended at server deployments of virt, but we have been happening to use it for general ad-hoc desktop usage too in Fedora. There is also a per-user 'qemu://session' UI where both libvirtd and the QEMU guests run unprivileged as that user's UID. In that scenario the storage pools should use something like $HOME/VirtualMachines/images and $HOME/VirtualMachines/ISOs'as the default pools for disk ISOs respectively. I would like to see Fedora use qemu:///session by default for generic desktop virt usage, in which case everything runs as that UID, and keeps everything in $HOME. We discussed this also. The only problem I see here is the additional disk usage, since you would permanently keeping the iso, even though you might only be using it once. What about removable media, cdrom and usb key isos. Do you want to copy them to the /var/lib/libvirt/isos directory also? If think if someone downloads a large ISO its fairly likely they'll want to keep it around for some amount of time, rather than risk having to download it again. That said, our storage pools APIs can trivally be used to delete it if it is no longer required. For removable media, we're just directly attaching the block device, so we can't avoid the need to re-label from fixed_dev_t (or equiva) to virt_image_t - though having a virt_image_ro_t for readonly access would be preferrable for cases where we're explicitly attaching it to the guest in read-only mode. USB keys I'm not so sure about - in the common case they'll be FAT filesystem which doesn't support labelling at all. So we'd either have to ensure its mounted with a suitable context= mount option, or get the policy to to allow read-only access to the default mount context for FAT filesystems. The /var/lib/libvirt/isos directly is just a plain directory based storage pool. We also have concept of filesystem based storage pools (eg a local device, mounted in some directory) which can we directly manage. Likewise NFS mounts we can handle directly, mounting a remote directoy from a server in a specific place, so we can ensure a context= mount option is used. So I think a combination of factors - Default directories for local host stored file based images ISO which get correct context automatically - libvirt has to set contenxt on any block device nodes which need to be assigned - Setting mount context for removable disks / changing policy to allow read-only access to mounted removable disks. Are the isos in this directory going to be Read Only or can some qemus read/write them? Any files used to back the QEMU CDROM device should be restricted to be read-only, since we need ability to safely assign the same ISO to multiple guests concurrently - for concurrent
[libvirt] Re: SELinux SVirt/Qemu problems with current qemu design.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Tue, Jan 13, 2009 at 05:18:46PM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 As I begin to work on the svirt lock down of the qemu process, I am seeing a disturbing problem. The qemu binaries are being used to both setup the guest image environment and then to run the guest image. With 'setup' vs 'run' terminology, my understanding is that by 'setup' you are refering to the initial stage which QEMU opens the disk device backing files, possibly creates TAP devices (or uses existing TAP fds) creates Pseudo TTYs, etc ? In other words, this distinction is similar to the traditional idea of a daemon starting as root, opening all its resources, and then dropping its privs user 'nobody' or equiv ? https://bugzilla.redhat.com/show_bug.cgi?id=478901 https://bugzilla.redhat.com/show_bug.cgi?id=479614 https://bugzilla.redhat.com/show_bug.cgi?id=478734 qemu is also being used to install the images. Trying to make a distinction between install vs post-install is inventing something that doesn't really exist The concept of 'install' does not have any real meaning in the host - it is entirely a guest OS concept. All the host knows / cares about is that it is booting a guest with zero or more disks. One of these may happen to be a CDROM device, which may happen to have a OS install image on it. It could just as easily be a live CD image. Or it could just be a CDROM given to a running installed guest. Or there may be no CDROM device, the guest PXE boot installing. In all the scenarios the host setup / guest boot works the same way - QEMU is given an emulated CDROM device, backed by the real host CDROM device, or an ISO image. Or there may be no. This is no different to setup of virtual harddisks, The key here is that whatever file/device is backing the virtual CDROM, it needs to be correctly labelled to allow QEMU to either read from it, or read+write from it (we allow RO disks, which CDROM typically are). This is the same labelling problem as for virtual harddisks, although its probably more common for users to have ISOs in unexpected places. The installation app should really deal with this problem, perhaps by saying there shall always be a '$HOME/ISO Images' directory which we could just add to the SELinux policy to allow read-only access for QEMU. Then virt-manager would only allow use of ISOs from that directory, or offer to move the ISO there re-label The problem with this is the act of installing an image or setting up the environment an image runs within requires much more privileges then actually running the image If qemu program is required to be able to modify the host machines network or to read iso images from anywhere on the host system, then providing real security on the guest operating system process is limited. SELinux runs best when one processes forks/execs another process this allows us to run the two processes under different labels. Each process with the privileges required to run. An example environment would be the following process labels libvirt_t-qemu_setup_t-qemu_t Where qemu_t can only manage the files labeled with virt_image_t. qemu_t would run the guest OS. SELinux can allow a process to change its own label to drop priviledge but this is considered less desirable from a security point of view. Yes and as I stated we have the ability to Drop Priv. But dropping privs is the less then ideal situation since it relies on the App to do the right thing, which has not always proven to be correct in daemons. The complication comes in when you want to hot-plug devices (ie adding extra network cards, harddisks disks, change CDROM media, attach PCI or USB devices from the host). If we have a setup phase and a runtime phase, this basically prevents all hotplug capabilities. There is a tradeoff between functionality we allow qemu_t to have vs security restrictions. eg, we could write policy to prevent QEMU creating TAP devices / changing bridging itself, because 95% of the time libvirt will take care of that, and pass an open FD to QEMU. Depends if we're happy to exclude those other 5% of users. Hotplug could potentially be made to work if we made use of UNIX domain sockets, and their file descriptor passing, to pass an open FD of a tap device to a running QEMU instance. I think labeling is a better solution for file access. Not sure about tap devices. Do they become device nodes that we could create and label such that only a particular qemu could gain access? Perhaps you could do a similar trick with disks, passing a pre-opened FD for the backing file of the disk, so you could avoid giving QEMU the privileges to open / create files - merely read/write existing file handles Save/migration is another potential complication where the running VM needs more privileges, ie to create
[libvirt] Re: SELinux SVirt/Qemu problems with current qemu design.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 James Morris wrote: On Wed, 14 Jan 2009, Daniel J Walsh wrote: I think labeling can be done to allow the access to directories, and files. So libvirt could go in an label a file/directory in such a way that the running qemu_t:s0.c10 can read or read/write the file/directory. Same with the ability to create save images, as long as the labeling is correct. The only problem I see here is the searching of the directory path to the location of the directories. If we want to allow users to store files/directories anywhere, we end up having to allow qemu_t the ability to at least search every directory on the system, and potentially read them. Having the ability to read a directory is sometimes valuable, for a hacker. I thought the virt-manager etc. tools were moving toward using standardized directories and not allowing users to put VM images just anywhere. This is more the iso images used to install virt images can be anywhere. So a user copies a iso image to his home directory and then installs the iso using virt-manager. Currently qemu_t would need to read user_home_t to make this work. If virt-manager/libvirt were to relabel the iso file to virt_image_t then qemu_t would be able to read it, iff it could search all of the parent directories. Daniel, has brought up the fact that additional files/directories could be added to the image via virt_manager, He is suggesting that virt-manager/libvirt would label images something like virt_image_t or virt_image_ro_t. With Svirt, these would also need the categories added. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkluVPcACgkQrlYvE4MpobPSSACg6eaZhuA+9teDqVN7ebRQkVV2 LTUAn0vKMh9TdHDvJOuT0iIeT3krHeP/ =Q/VZ -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: SELinux SVirt/Qemu problems with current qemu design.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Itamar Heim wrote: From: libvir-list-boun...@redhat.com [mailto:libvir-list- boun...@redhat.com] On Behalf Of Daniel J Walsh I think labeling can be done to allow the access to directories, and files. So libvirt could go in an label a file/directory in such a way that the running qemu_t:s0.c10 can read or read/write the file/directory. Same with the ability to create save images, as long as the labeling is correct. The only problem I see here is the searching of the directory path to the location of the directories. If we want to allow users to store files/directories anywhere, we end up having to allow qemu_t the ability to at least search every directory on the system, and potentially read them. Having the ability to read a directory is sometimes valuable, for a hacker. [IH] I don't see us avoiding per-vm policy. The policy has to be dynamic - customized before launching qemu, and able to change during process life-cycle for events like hot-plug and migration. I don't see the need for the qemu process to change its permissions, since the managing libvirt can change the policy during process life (I assume selinux policy can change while a process is already running). The reason the policy has to be created just before process launch is that the VM can be run on different hosts, and I don't think it makes sense to synchronize the policy of all possible guests on all hosts all the time (and the policy is enforced at host level, not storage level for example). Also, we need to cover the SAN use case, where each image is probably an LV in LVM. SELinux has to levels of flexibility here device/file labeling and booleans. Having virt-manager modifying the policy on the fly would be quite a bit more complicated. lvm should be handled just by setting the labels on the /dev/mapper/VolGroup00-LogVol00 For example. If this is labeled virt_image_t then qemu_t can read/write it. If it is default labeled to fixed_disk_device_t, then it will not be allowed. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkluVyMACgkQrlYvE4MpobPu2QCg5HB5U1DsFa3gTnb+T78dj2iT hPoAn21jAg46ohyybQhwoJrjihR6puU5 =Ew2C -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] SELinux SVirt/Qemu problems with current qemu design.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 As I begin to work on the svirt lock down of the qemu process, I am seeing a disturbing problem. The qemu binaries are being used to both setup the guest image environment and then to run the guest image. https://bugzilla.redhat.com/show_bug.cgi?id=478901 https://bugzilla.redhat.com/show_bug.cgi?id=479614 https://bugzilla.redhat.com/show_bug.cgi?id=478734 qemu is also being used to install the images. The problem with this is the act of installing an image or setting up the environment an image runs within requires much more privileges then actually running the image If qemu program is required to be able to modify the host machines network or to read iso images from anywhere on the host system, then providing real security on the guest operating system process is limited. SELinux runs best when one processes forks/execs another process this allows us to run the two processes under different labels. Each process with the privileges required to run. An example environment would be the following process labels libvirt_t-qemu_setup_t-qemu_t Where qemu_t can only manage the files labeled with virt_image_t. qemu_t would run the guest OS. SELinux can allow a process to change its own label to drop priviledge but this is considered less desirable from a security point of view. Dan -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkltE0YACgkQrlYvE4MpobOY/wCg3wNp7miY6PLy3IkFae1M4uGk KgkAnieeGbpAUUA2/Af5oZxx9zShtZgs =LmbV -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] sVirt v0.10 - initial prototype
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Atsushi SAKAI wrote: Hi, James I have a question just for interest. The security context stores like /etc/selinux/targeted/contexts/files/file_contexts. But you are storeing the domain security label on libvirt specific XML. Of course, this is good for libvirt POV. Is it permitted for SELinux policy POV? By the way, I want to see the further discussion of the Requirements. Requirements not yet addressed include: Thanks Atsushi SAKAI Both /etc/selinux/targeted/contexts/files/file_contexts and libvirt specific XML are just indicators of the way the system should be setup. The kernel policy is the final arbiter of the policy. So if the kernel does not understand virt_image_t in /etc/selinux/targeted/contexts/files/file_contexts, it will not allow tools like rpm or restorecon to set it as a file context. Similarly if the process label qemu_t is specified in the libvirt xml, and the kernel policy does not know what a qemu_t is, it will not allow libvirt to start a processes labeled qemu_t. James Morris [EMAIL PROTECTED] wrote: [snip] The domain security label configuration format is as follows: # virsh dumpxml sys1 domain seclabel model='selinux' labelsystem_u:system_r:virtd_t:s0/label policytypetargeted/policytype /seclabel /domain It's possible to query the security label of a running domain via virsh: # virsh dominfo sys1 Id: 1 Name: sys1 UUID: fa3c8e06-0877-2a08-06fd-f2479b7bacb4 OS Type:hvm State: running CPU(s): 1 CPU time: 11.4s Max memory: 524288 kB Used memory:524288 kB Autostart: disable Security label: system_u:system_r:virtd_t:s0 (selinux/targeted/enforcing) The security label is deterimed via the new virDomainGetSecLabel() API method, which is transported over RPC to the backend driver (qemu in this case), and is entirely independent of the local security model, if any. e.g. this command could be run remotely from an entirely different platform: you just see what's happening on the remote system, as with other attributes of the domain. Feedback on the design thus far is sought before proceeding to more comprehensive integration. In particular, I'd be interested in any thoughts on the placement of the security labeling driver within libvirt, as this seems to be the most critical architectural issue (I've already refactored this aspect once). Currently, the idea is to attach the security labeling driver to the virt driver, rather than implement it independently as a top-level component as in the case of other types of drivers (e.g. storage). This is because process-based security labeling is highly dependent on the kind of virtualization in use, and may not make sense at all in some cases (e.g. when using a non-Linux hypervisor, or containers). In the case of qemu, a security labeling driver is added to qemud: @@ -63,6 +64,7 @@ struct qemud_driver { char *vncListen; virCapsPtr caps; +virSecLabelDriverPtr secLabelDriver; }; and then initialized during qemud startup from qemudSecLabelInit(). During initialization, any available security labeling drivers are probed, and the first one which thinks it should be used is installed. Top-level libvirt API calls are then dispatched to the active security labeling driver via the backend virt driver, as necessary. Note that the security labeling framework in this release is always built-in -- it can be made a compile-time option later if desired. Requirements not yet addressed include: - Labeling of resources and generally comprehensive labeling management - Automatic labeling (e.g. for the simple isolation use-case) - Integration of labeling support into higher-level management tools such as virt-manager - Integration with the audit subsystem to help with administration and debugging - Domain of interpretation (DOI) checking/translation - Python bindings As mentioned, the goal at this stage is to get feedback on the underlying design: comments welcome! - James -- James Morris [EMAIL PROTECTED] -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [EMAIL PROTECTED] with the words unsubscribe selinux without quotes as the message. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkkKCHgACgkQrlYvE4MpobO44ACdG8CX5Y6ptaUTd2RtP4rtjaTo u1sAniwNi1WoYUuxkO3zM9A8hNUGLhTO =jtTR -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [RFC] sVirt v0.10 - initial prototype
James Morris wrote: This is a request for comments on the initial prototype release of sVirt, a project to add security labeling support to Linux-based virtualization. Project page: http://www.selinuxproject.org/page/SVirt Previous libvirt discussions: High-level requirements: https://www.redhat.com/archives/libvir-list/2008-August/msg00255.html XML security labels: https://www.redhat.com/archives/libvir-list/2008-August/msg00740.html A patch for libvirt is attached; and also included in a release tarball at http://namei.org/svirt/. See 'readme.txt' there for more details on building and running the code. The purpose of this release is to establish a proof of concept of applying security labels to VMs, and for discussion of the underlying technical approach. With this release, it is possible to define a security label for a kvm/qemu domain in its XML configuration ('virsh edit'), launch the domain and have it transition to the specified security label ('virsh start'), then query the security label of the running domain ('virsh dominfo'). The following changes were made to libvirt: 1. Implementing a pluggable security label driver framework; 2. Implementing an SELinux security label driver for (1); 3. Wiring the security label framework into the Qemu driver; 4. Implementing basic libvirt API calls for initializing the driver, and getting/setting domain security labels; 5. Extending the domain XML configuration to include security labels; 6. Adding domain security label display/edit/dump support to virsh. One of the design principles I've followed with this is to reduce or eliminate configuration wherever possible. If a variety of security labeling drivers are present, libvirtd automatically detects which one to enable and enables it. e.g. if SELinux is enabled on the system, the SELinux labeling driver is enabled automatically when livbirtd starts. Another is to treat security labels as opaque unless they're actually being used for security purposes (e.g. to launch the domain). So, virsh and the domain configuration code currently do not need to semantically interpet security labels, just understand their format. This should suit the initial simple goal of isolated domains, which only requires security labels to be distinct. The domain security label configuration format is as follows: # virsh dumpxml sys1 domain seclabel model='selinux' labelsystem_u:system_r:virtd_t:s0/label policytypetargeted/policytype /seclabel /domain It's possible to query the security label of a running domain via virsh: # virsh dominfo sys1 Id: 1 Name: sys1 UUID: fa3c8e06-0877-2a08-06fd-f2479b7bacb4 OS Type:hvm State: running CPU(s): 1 CPU time: 11.4s Max memory: 524288 kB Used memory:524288 kB Autostart: disable Security label: system_u:system_r:virtd_t:s0 (selinux/targeted/enforcing) The security label is deterimed via the new virDomainGetSecLabel() API method, which is transported over RPC to the backend driver (qemu in this case), and is entirely independent of the local security model, if any. e.g. this command could be run remotely from an entirely different platform: you just see what's happening on the remote system, as with other attributes of the domain. Feedback on the design thus far is sought before proceeding to more comprehensive integration. In particular, I'd be interested in any thoughts on the placement of the security labeling driver within libvirt, as this seems to be the most critical architectural issue (I've already refactored this aspect once). Currently, the idea is to attach the security labeling driver to the virt driver, rather than implement it independently as a top-level component as in the case of other types of drivers (e.g. storage). This is because process-based security labeling is highly dependent on the kind of virtualization in use, and may not make sense at all in some cases (e.g. when using a non-Linux hypervisor, or containers). In the case of qemu, a security labeling driver is added to qemud: @@ -63,6 +64,7 @@ struct qemud_driver { char *vncListen; virCapsPtr caps; +virSecLabelDriverPtr secLabelDriver; }; and then initialized during qemud startup from qemudSecLabelInit(). During initialization, any available security labeling drivers are probed, and the first one which thinks it should be used is installed. Top-level libvirt API calls are then dispatched to the active security labeling driver via the backend virt driver, as necessary. Note that the security labeling framework in this release is always built-in -- it can be made a compile-time option later if desired. Requirements not yet addressed include: - Labeling of resources and generally comprehensive
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
James Morris wrote: On Tue, 12 Aug 2008, Daniel P. Berrange wrote: Do we instead add the info the udev rules, so when /dev is populated at boot time by udev the device nodes get the desired initial labelling ? Or do we manually chcon() the device at the time we boot the VM ? Dan Walsh has mentioned wanting to label the device at VM launch so that MCS labels can be dynamically assigned. This raises some other possible issues such as revoking any existing access (Linux doesn't have general revocation) and having the security of the system depend on whatever is performing the relabel (although we can enforce relabelfrom/relabelto permissions). I wonder if existing work/concepts related to MLS device allocation would be useful here. See: http://sourceforge.net/projects/devallocator/ - James The experimenting I have done has been around labeling of the virt_image and the process with mcs labels to prevent one process from touching another process/image with a different MCS label. system_u:system_r:qemu_t:s0:c1 can read/write system_u:system_r:virt_image_t:s0:c1 But can not read/write system_u:system_r:virt_image_t:s0:c2 or communicate with process system_u:system_r:qemu_t:s0:c2 The idea would be to have libvirt look at the labeling of the image file and lauch the qemu process with the correct type and matching MCS label. So a more advanced image might be labeled system_u:system_r:virt_image_nonet_t:s0:c1 and libvirt would launch system_u:system_r:qemu_nonet_t:s0:c2 I have not looked into which devices on the host machine might need higher levels of protection. In Fedora 9/Rawhide libvirt runs as virtd_t and has a transition rule on qemu_exec_t to qemu_t. So all virtual machines run with system_u:system_r:qemu_t:s0 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 09:20:41AM -0400, Daniel J Walsh wrote: James Morris wrote: On Tue, 12 Aug 2008, Daniel P. Berrange wrote: Do we instead add the info the udev rules, so when /dev is populated at boot time by udev the device nodes get the desired initial labelling ? Or do we manually chcon() the device at the time we boot the VM ? Dan Walsh has mentioned wanting to label the device at VM launch so that MCS labels can be dynamically assigned. This raises some other possible issues such as revoking any existing access (Linux doesn't have general revocation) and having the security of the system depend on whatever is performing the relabel (although we can enforce relabelfrom/relabelto permissions). I wonder if existing work/concepts related to MLS device allocation would be useful here. See: http://sourceforge.net/projects/devallocator/ - James The experimenting I have done has been around labeling of the virt_image and the process with mcs labels to prevent one process from touching another process/image with a different MCS label. system_u:system_r:qemu_t:s0:c1 can read/write system_u:system_r:virt_image_t:s0:c1 But can not read/write system_u:system_r:virt_image_t:s0:c2 or communicate with process system_u:system_r:qemu_t:s0:c2 The idea would be to have libvirt look at the labeling of the image file and lauch the qemu process with the correct type and matching MCS label. That's not going to fly for VMs without disks in the host - either totally diskless VMs, or VMs using iSCSI/NFS network blockdevices / root FS. Daniel We could store the label to run qemu for a particular image in the libvirt database. But this mechanism would have to match up with the labeling on disk or remote storage. system_u:system_r:qemu_nfs_t:s0:c1 can read/write system_u:system_r:nfs_t:s0 Or you have rules that state if virtd_t wants to start an image labeled nfs_t it will use qemu_nfs_t You could still use the MCS label to prevent processes from attacking each other, but if the remote storage does not support labelling you will not be able to prevent them from attacking each others image files. I think libvirt being SELinux aware and have it decide which context to run qemu at is the important point. The arguement about whether it needs to store the SELinux label in its database or base it off the label of the image can be hashed out later. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 09:54:23AM -0400, Daniel J Walsh wrote: Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 09:20:41AM -0400, Daniel J Walsh wrote: The experimenting I have done has been around labeling of the virt_image and the process with mcs labels to prevent one process from touching another process/image with a different MCS label. system_u:system_r:qemu_t:s0:c1 can read/write system_u:system_r:virt_image_t:s0:c1 But can not read/write system_u:system_r:virt_image_t:s0:c2 or communicate with process system_u:system_r:qemu_t:s0:c2 The idea would be to have libvirt look at the labeling of the image file and lauch the qemu process with the correct type and matching MCS label. That's not going to fly for VMs without disks in the host - either totally diskless VMs, or VMs using iSCSI/NFS network blockdevices / root FS. Daniel We could store the label to run qemu for a particular image in the libvirt database. But this mechanism would have to match up with the labeling on disk or remote storage. Ok, one minor point worth mentioning is that libvirt does not have a general purpose database of configurations. The way VM configuration is stored is hypervisor-specific. In the Xen/OpenVZ/VMWware case we pass the config straight through to XenD which takes care of persisting it. In QEMU/LXC case we store the XML config files in /etc/libvirt. Or you have rules that state if virtd_t wants to start an image labeled nfs_t it will use qemu_nfs_t You could still use the MCS label to prevent processes from attacking each other, but if the remote storage does not support labelling you will not be able to prevent them from attacking each others image files. We don't need to restrict ourselves to a single NFS type qemu_nfs_t/nfs_t. A single NFS server can export many directories each of which can be mounted with a different context. Yes and no. The mountpoint can be labeled differently at the directory level I believe. So you would have to store each image in it's own directory and mount at that level in order for mount -o context to work. I think libvirt being SELinux aware and have it decide which context to run qemu at is the important point. The argument about whether it needs to store the SELinux label in its database or base it off the label of the image can be hashed out later. So the important thing is that the label is represented in the libvirt XML format, and this XML config is persisted in a manner which is most applicable for the virtualization driver in question. While I know James wants to target KVM primarily, we need make sure the approach we take doesn't paint ourselves into a corner wrt supporting other virt platforms like Xen. I probably should have used datastore rather then database. Wherever this data is stored it should be protected as libvirt will be making security decisions based on the data. (Of course this is controlled by kernel policy. the kernel would prevent virtd_t for execing an application as sshd_t for example. Our guiding rule with libvirt is that for every capability we add, we need to come up with a conceptual model that is applicable to all virtualization drivers, even if we only implement it for one particular driver. Isn't libvirt going to be execing q program like qemu_t to run xen images? If yes then this should all work with the defined mechanism. Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list