Re: Printing runtime DAC seclabel in the XML

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

 



On Sat, Apr 23, 2016 at 01:10:01PM -0400, Cole Robinson wrote:
On 04/22/2016 01:28 AM, Martin Kletzander wrote:
On Thu, Apr 21, 2016 at 10:16:41AM -0400, Cole Robinson wrote:
On 04/21/2016 06:08 AM, Martin Kletzander wrote:
On Wed, Apr 20, 2016 at 07:57:05PM -0400, Cole Robinson wrote:
I'm looking in the code to see why runtime VM dac seclabel values aren't
printed in the active XML. They are filled in, but the domain XML formatter
explicitly skips it:

   /* To avoid backward compatibility issues, suppress DAC and 'none' labels
    * that are automatically generated.
    */
   if ((STREQ_NULLABLE(def->model, "dac") ||
        STREQ_NULLABLE(def->model, "none")) && def->implicit)
       return;

The relevant bit is from here:

commit 990e46c4542349f838e001d30638872576c389e9
Author: Marcelo Cerri <mhcerri@xxxxxxxxxxxxxxxxxx>
Date:   Fri Aug 31 13:40:41 2012 +0200

   conf: Avoid formatting auto-generated DAC labels

And I think comment elsewhere in domain_conf.c explains what that's all
about:

   /* libvirt versions prior to 0.10.0 support just a single seclabel element
    * in guest's XML and model attribute can be suppressed if type is none or
    * type is dynamic, baselabel is not defined and INACTIVE flag is set.
    *
    * To avoid compatibility issues, for this specific case the first model
    * defined in host's capabilities is used as model for the seclabel.
    */

Just dropping the the model == "dac" check above seems to accomplish what I'm
after, but it's not strictly back compatible. That said, libvirt has
supported

I think this has nothing to do with backward compatibility, but forward
compatibility (which we care about for "some" time).


That's not my reading of the patch + thread. I think it was that new libvirt
starting printing two <seclabel> blocks for active XML. If you then do a
managedsave, downgrade libvirt, and attempt a restore (or live migration to an
older version), old libvirt errors when it sees 2 <seclabel> blocks. So back
compat here to me meant 'generate XML that old libvirt will accept'


Yes, so we have the same understanding, but the terminology is
confusing.  Backward compatibility is about supporting old
XML/API/whatever in new libvirt.  Forward compatibility means old
libvirt will work with newer XML/API/whatever.

To be even surer, I checked Wikipedia [1] =)


Sorry for the confusion

So we are talking about the same thing.  And about that, I don't think
we need to care about older libvirt.  Most of the XMLs won't work
because of other things anyway.  Especially versions lower than 0.10.0
as oldest CentOS we consider supported has 0.10.2 IIRC and most requests
and bugreports for other distros are from newer versions as well.


Again there's some subtlety here: a config with <sound
model='BRAND-NEW-MODEL'/>  obviously isn't going to work on older libvirt that
didn't support it. It's completely acceptable to say 'sorry you can't migrate
that config to older libvirt'.

However I believe the original reason that runtime DAC wasn't put in the XML
is because of this:

- start with a working XML config on libvirt < 0.10.0
- update libvirt
- start your VM, runtime XML now contains DAC labels
- save your VM
- downgrade libvirt back to 0.10.0
- try to restore your VM, libvirt chokes on double <seclabel>

This is the same issue we had when <input type='keyboard'/> started being
printed in the XML. That's why we have this chunk in InputDefFormat

   /* don't format keyboard into migratable XML for backward compatibility */
   if (def->type == VIR_DOMAIN_INPUT_TYPE_KBD &&
       flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)
       return 0;

That said, I don't think we really need to retain that type of migration
compat going all the way back to < 0.10.0, but I want to make sure we are on
the same page.


I think we are.

...actually maybe that's the better solution here is just to make the DAC
check dependent on VIR_DOMAIN_DEF_FORMAT_MIGRATABLE... runtime seclabel stuff
really isn't interesting for migration anyways. I'll send a patch like that


That'd be for the best, probably.

What are you trying to achieve?  It works for me, the default seclabel
is not exposed, but if I change it, it is dumped for running, shut-off
or whatever domain there is.


the issue: start a VM with no explicit seclabel config, dump the running XML,
you'll see something like this:

 <seclabel type='dynamic' model='selinux' relabel='yes'>
   <label>unconfined_u:unconfined_r:svirt_t:s0:c325,c747</label>
   <imagelabel>unconfined_u:object_r:svirt_image_t:s0:c325,c747</imagelabel>
 </seclabel>

but no <seclabel> block for DAC labels, which seems inconsistent


Well, this is just about the point of view.  SELinux labels are
dynamically generated and the user cannot predict what the categories
will be and so on.  But for DAC, the domain will be running under the
default user and group set in qemu.conf.

Don't get me wrong, I'm all for exposing all the labels, but I see one
problem with that that we need to make sure is deal with.  Currently, if
you change the default in qemu.conf it will apply to all your domains.
However, if we add it there and save it to the disk, the default uid:gid
won't follow the change in qemu.conf even if it was just left at the
default state.


I'm not sure I'm understanding correctly... this is only about runtime XML
reporting. The qemu.conf bits aren't listed in the inactive XML, similar to
how it works for selinux labeling. If a VM is running with user=foo and you
change qemu.conf setting to use user=bar, the VM will pick that change up next
time is started.

Do you mean if the VM is defined with its runtime XML, the DAC value is now
hardcoded for the VM and doesn't abide qemu.conf anymore? That's mostly not
true... since the runtime XML is type=dynamic, so if a user inadvertently
defines the VMs runtime XML, the only bits that get hardcoded are:

 <seclabel type='dynamic' model='selinux' relabel='yes'/>
 <seclabel type='dynamic' model='dac' relabel='yes'/>

and the specific labels are thrown out.


Oh, right, my bad, you're right :)

Thanks,
Cole

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]