[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.000000000 -0500 +++ virtinst-0.400.1.new/virtinst/Guest.py 2009-02-19 17:36:35.000000000 -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 = 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) + + 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) + def _set_defaults(self): if self.uuid is None: while 1: diff -r -u virtinst-0.400.1/virtinst/VirtualDisk.py virtinst-0.400.1.new/virtinst/VirtualDisk.py --- virtinst-0.400.1/virtinst/VirtualDisk.py 2009-01-26 14:33:25.000000000 -0500 +++ virtinst-0.400.1.new/virtinst/VirtualDisk.py 2009-02-19 19:43:44.000000000 -0500 @@ -172,8 +172,6 @@ """ return "%s:%s" %(self.type, self.path) - - def _get_path(self): return self._path def _set_path(self, val, validate=True): @@ -413,7 +411,7 @@ newpath = self.vol_object.path() elif self.vol_install: newpath = _util.get_xml_path(self.vol_install.pool.XMLDesc(0), - "/pool/target/path") + \ + "/pool/target/path") + "/" + \ self.vol_install.name if newpath and newpath != self.path:

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.
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 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.) Updated patch attached, I'll reply with patch specific comments later. Thanks, Cole

Cole Robinson wrote:
Daniel J Walsh wrote:
<snip>
Updated patch attached, I'll reply with patch specific comments later.
Thanks, Cole
Two major pieces missing from this patch: 1) Some way to skip all this. If selinux isn't enabled on the host, or the user doesn't request labeling, we shouldn't do the work. 2) Support in CapabilitiesParser for the host security policy, otherwise we have no way of knowing if selinux is enabled on a remote host.
diff -r 026a37ccd417 virtinst/Guest.py --- a/virtinst/Guest.py Sat Feb 21 14:36:07 2009 -0500 +++ b/virtinst/Guest.py Sat Feb 21 15:09:49 2009 -0500 @@ -27,6 +27,8 @@ import libvirt import CapabilitiesParser import VirtualGraphics +import selinux
This import should be moved into gen_seclabels, and probably wrapped in a try...except block, and if the import fails (libselinux-python isn't isntalled) we just skip adding the labels to the xml. This will make it easier for other distros who may not want the selinux support to avoid problems.
+import random
import osdict from virtinst import _virtinst as _ @@ -71,6 +73,8 @@ self._cpuset = None self._graphics_dev = None self._consolechild = None + self._seclabel = None + self._imagelabel = None
self._os_type = None self._os_variant = None @@ -101,6 +105,16 @@
self._caps = CapabilitiesParser.parse(self.conn.getCapabilities())
+ (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 @@ -110,6 +124,20 @@ 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) + # Domain name of the guest def get_name(self): return self._name @@ -407,6 +435,19 @@ xml = _util.xml_append(xml, sound_dev.get_xml_config()) return xml
+ 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 + +
This doesn't look like it matches the latest svirt libvirt patches: I don't see an <image> tag in those.
def _get_device_xml(self, install=True): xml = ""
@@ -494,6 +535,7 @@ <devices> %(devices)s </devices> + %(secxml)s </domain> """ % { "type": self.type, "name": self.name, \ @@ -504,7 +546,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, @@ -537,6 +580,8 @@ """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)
Yes, this should be in the VirtualDisk class, in the setup method. VirtualDisk would also need an imagelabel attribute in that case. It may be nice to also keep the Guest one, rather than force the user to fill in the imagelabel for every VirtualDisk object being added. Either way, this will break the remote case as is.
for nic in self._install_nics: nic.setup(self.conn)
@@ -631,6 +676,80 @@ 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 find 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 find SELinux virtual image " + "context: %s: %s %s" % + (selinux.selinux_virtual_domain_context_path(), + err_no, msg))) +
What version of libselinux-python were these functions added in? They don't seem present on F9 at least. Also, this labeling would only be relevant on the local host. Maybe this info should be exposed by libvirt capabilities? If we do get this info from capabilities, then we may as well just remove the dependency on libselinux-python, and just use 'chcon' directly. The rest looks reasonable. Thanks, Cole

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Cole Robinson wrote:
Cole Robinson wrote:
Daniel J Walsh wrote:
<snip>
Updated patch attached, I'll reply with patch specific comments later.
Thanks, Cole
Two major pieces missing from this patch:
1) Some way to skip all this. If selinux isn't enabled on the host, or the user doesn't request labeling, we shouldn't do the work.
2) Support in CapabilitiesParser for the host security policy, otherwise we have no way of knowing if selinux is enabled on a remote host.
diff -r 026a37ccd417 virtinst/Guest.py --- a/virtinst/Guest.py Sat Feb 21 14:36:07 2009 -0500 +++ b/virtinst/Guest.py Sat Feb 21 15:09:49 2009 -0500 @@ -27,6 +27,8 @@ import libvirt import CapabilitiesParser import VirtualGraphics +import selinux
This import should be moved into gen_seclabels, and probably wrapped in a try...except block, and if the import fails (libselinux-python isn't isntalled) we just skip adding the labels to the xml. This will make it easier for other distros who may not want the selinux support to avoid problems.
+import random
import osdict from virtinst import _virtinst as _ @@ -71,6 +73,8 @@ self._cpuset = None self._graphics_dev = None self._consolechild = None + self._seclabel = None + self._imagelabel = None
self._os_type = None self._os_variant = None @@ -101,6 +105,16 @@
self._caps = CapabilitiesParser.parse(self.conn.getCapabilities())
+ (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 @@ -110,6 +124,20 @@ 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) + # Domain name of the guest def get_name(self): return self._name @@ -407,6 +435,19 @@ xml = _util.xml_append(xml, sound_dev.get_xml_config()) return xml
+ 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 + +
This doesn't look like it matches the latest svirt libvirt patches: I don't see an <image> tag in those.
def _get_device_xml(self, install=True): xml = ""
@@ -494,6 +535,7 @@ <devices> %(devices)s </devices> + %(secxml)s </domain> """ % { "type": self.type, "name": self.name, \ @@ -504,7 +546,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, @@ -537,6 +580,8 @@ """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)
Yes, this should be in the VirtualDisk class, in the setup method. VirtualDisk would also need an imagelabel attribute in that case. It may be nice to also keep the Guest one, rather than force the user to fill in the imagelabel for every VirtualDisk object being added.
Either way, this will break the remote case as is.
for nic in self._install_nics: nic.setup(self.conn)
@@ -631,6 +676,80 @@ 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 find 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 find SELinux virtual image " + "context: %s: %s %s" % + (selinux.selinux_virtual_domain_context_path(), + err_no, msg))) +
What version of libselinux-python were these functions added in? They don't seem present on F9 at least.
No they require rawhide libselinux. I just added the default label files for svirt/F11.
Also, this labeling would only be relevant on the local host. Maybe this info should be exposed by libvirt capabilities?
Not sure what this means.
If we do get this info from capabilities, then we may as well just remove the dependency on libselinux-python, and just use 'chcon' directly.
libselinux changes are to ask the policy writer what it should call the virtual process and the virtual image. We can back port the libselinux changes/policy changes to provide this functionality to F10 and potentially RHEL5 if necessary.
The rest looks reasonable.
Thanks, Cole
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmi5tQACgkQrlYvE4MpobOmWgCghmUFQPTGiw/l2zF2W3tbyyFn 4vEAoLVeV5FlN+e42pmVn5ZAC3QqfyBs =48C/ -----END PGP SIGNATURE-----

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

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.
+ 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. - If someone copies this config to another machine, then the mcs level in the config may no longer be unique on the target machine - If you live migrate a VM, again you have problem that the mcs used on the source may already be in use on the target. - Save/restore across machines also has uniqueness issues The way virt-install is allocating the mcs level here is also open to race condition if more than one VM provisioning operation is taking place concurrently. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

-----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 may no longer be unique on the target machine - If you live migrate a VM, again you have problem that the mcs used on the source may already be in use on the target. - Save/restore across machines also has uniqueness issues
The way virt-install is allocating the mcs level here is also open to race condition if more than one VM provisioning operation is taking place concurrently.
Daniel
This is the way I was originally thinking, but there are a couple of problems with this. libvirt would need to relabel all of the images on the fly when it chooses the MCS Level. So if I have multiple disks I would need to run through them and execute a setfilecon on each image. What happens to the labels when them image is complete? Do I rerun through the images setting the labels to something that no qemu can read/write? system_u:object_r:virt_image_t:SystemHigh? Since we do not want to accidently allow a later qemu to gain access. What about shared storage? If I have multiple libvirt running on shared storage, they could both pick out the same MCS Label and set labels on the images allowing the remote machine access. Now since we are allowing for ~500,000 labels, this is limited risk, but it does exist. Finally the current solution does allow for MLS. In MLS the administrator wants to say that these images will run as TopSecret and do not care about isolation. So we would need a mechanism to override libvirt choosing a label on the fly and allow the user to force the label. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmkF48ACgkQrlYvE4MpobPNpwCgz3hleD/Q88P3MQalmSPSfsqk LFoAni64GmLHsLaU+fCHChIlBI8Z9DBO =WScn -----END PGP SIGNATURE-----
participants (3)
-
Cole Robinson
-
Daniel J Walsh
-
Daniel P. Berrange