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

Re: [libvirt] [PATCH] conf: Fix parsing of seclabels without model



On Thu, Aug 30, 2012 at 03:31:05PM -0300, Marcelo Cerri wrote:
> On 08/30/2012 03:20 PM, Daniel P. Berrange wrote:
> >On Thu, Aug 30, 2012 at 03:17:09PM -0300, Marcelo Cerri wrote:
> >>On 08/30/2012 03:03 PM, Daniel P. Berrange wrote:
> >>>On Thu, Aug 30, 2012 at 07:12:26PM +0200, Jiri Denemark wrote:
> >>>>On Thu, Aug 30, 2012 at 13:19:31 -0300, Marcelo Cerri wrote:
> >>>>>With this patch libvirt tries to assign a model to seclabels when model
> >>>>>is missing. Libvirt will look up at host's capabilities and assign a
> >>>>>model in order to each seclabel that doesn't have a model assigned.
> >>>>>
> >>>>>This patch fixes:
> >>>>>
> >>>>>1. The problem with existing guests that have a seclabel defined in its XML.
> >>>>>2. A XML parse error when a guest is restored.
> >>>>>
> >>>>>Signed-off-by: Marcelo Cerri <mhcerri linux vnet ibm com>
> >>>>>---
> >>>>>  src/conf/domain_conf.c | 56 ++++++++++++++++++++++++++------------------------
> >>>>>  1 file changed, 29 insertions(+), 27 deletions(-)
> >>>>
> >>>>I think this is trying to fix the issue at a wrong place. It's not that XML
> >>>>generated by older libvirtd is not correctly parsed by current libvirtd. The
> >>>>problem is that *current* libvirtd creates an XML that it cannot parse back.
> >>>>Thus we should rather fix the code that formats the XML.
> >>>>
> >>>>On that front, I'm concerned about migration compatibility of this new
> >>>>security driver code. If we just blindly emit <seclabel type='dynamic'
> >>>>model='dac' relabel='yes'> element into the XML, I'm pretty sure an older
> >>>>libvirtd will complain about it even though the element was not used to do
> >>>>anything special that would be done anyway (that is, if labels are the default
> >>>>qemu_user:qemu_group).
> >>>
> >>>Yes, we should not auto-add a <seclabel> for model=dac unless we have
> >>>configured it to auto-assign a private uid:gid pair per guest. If it is
> >>>operating in the mode where it just uses a fixed uid:gid pair we should
> >>>not emit the seclabel.
> >>>
> >>
> >>Can you explain which problem this auto-added <seclabel> for
> >>model=dac can create? I really can see a migration compatibility
> >>issue with it. When a <seclabel> for model=selinux is not defined
> >>for a guest, and SELinux driver is in use, a <seclabel> is also
> >>auto-added to this guest.
> >
> >An old libvirtd (ie < 0.10.0) already knows how to parse & accept
> >a <seclabel> for model=selinux. It will reject a <seclabel>
> >which has model=dac, if that is the first <seclabe> element present.
> >(it will of course ignore the 2nd/3rd/etc <seclabel> element, since
> >it only expected one to exist).  So if  model=dac is added as the
> >second <seclabel> back compat is ok. If the selinux/apparmour
> >security drivers are disabled though, the <seclabel> with model=dac
> >will be the first & only element. This will confuse old libvirtd.
> >
> 
> Ok. But in which scenario would this happen? It doesn't seem to make
> sense to save a guest with an earlier libvirt version and restore it
> in an older libvirt.

I wish that was the case, but unfortunately people do want todo
exactly that :-(  More particularly for live-migration betweeen
different releases of RHEL, but save+restore too.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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