[libvirt] Re: [et-mgmt-tools] Re: Patch to python-virtinst to allow it to choose svirt labels
Daniel J Walsh
dwalsh at redhat.com
Mon Feb 23 18:11:33 UTC 2009
-----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-----
More information about the libvir-list
mailing list