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

[libvirt] Re: [et-mgmt-tools] Re: Patch to python-virtinst to allow it to choose svirt labels



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


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