[libvirt] [PATCH] selinux: distinguish failure to label from request to avoid label

[Daniel P. Berrange]

On Mon, Aug 12, 2013 at 10:19:47PM -0600, Eric Blake wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=924153
> 
> Commit 904e05a2 (v0.9.9) added a per-<disk> seclabel element with
> an attribute relabel='no' in order to try and minimize the
> impact of shutdown delays when an NFS server disappears.  The idea
> was that if a disk is on NFS and can't be labeled in the first
> place, there is no need to attempt the (no-op) relabel on domain
> shutdown.  Unfortunately, the way this was implemented was by
> modifying the domain XML so that the optimization would survive
> libvirtd restart, but in a way that is indistinguishable from an
> explicit user setting.  Furthermore, once the setting is turned
> on, libvirt avoids attempts at labeling, even for operations like
> snapshot or blockcopy where the chain is being extended or pivoted
> onto non-NFS, where SELinux labeling is once again possible.  As
> a result, it was impossible to do a blockcopy to pivot from an
> NFS image file onto a local file.
> 
> The solution is to separate the semantics of a chain that must
> not be labeled (which the user can set even on persistent domains)
> vs. the optimization of not attempting a relabel on cleanup (a
> live-only annotation), and using only the user's explicit notation
> rather than the optimization as the decision on whether to skip
> a label attempt in the first place.  When upgrading an older
> libvirtd to a newer, an NFS volume will still attempt the relabel;
> but as the avoidance of a relabel was only an optimization, this
> shouldn't cause any problems.
> 
> In the ideal future, libvirt will eventually have XML describing
> EVERY file in the backing chain, with each file having a separate
> <seclabel> element.  At that point, libvirt will be able to track
> more closely which files need a relabel attempt at shutdown.  But
> until we reach that point, the single <seclabel> for the entire
> <disk> chain is treated as a hint - when a chain has only one
> file, then we know it is accurate; but if the chain has more than
> one file, we have to attempt relabel in spite of the attribute,
> in case part of the chain is local and SELinux mattered for that
> portion of the chain.
> 
> * src/conf/domain_conf.h (_virSecurityDeviceLabelDef): Add new
> member.
> * src/conf/domain_conf.c (virSecurityDeviceLabelDefParseXML):
> Parse it, for live images only.
> (virSecurityDeviceLabelDefFormat): Output it.
> (virDomainDiskDefParseXML, virDomainChrSourceDefParseXML)
> (virDomainDiskSourceDefFormat, virDomainChrDefFormat)
> (virDomainDiskDefFormat): Pass flags on through.
> * src/security/security_selinux.c
> (virSecuritySELinuxRestoreSecurityImageLabelInt): Honor labelskip
> when possible.
> (virSecuritySELinuxSetSecurityFileLabel): Set labelskip, not
> norelabel, if labeling fails.
> * docs/formatdomain.html.in (seclabel): Document new xml.
> * docs/schemas/domaincommon.rng (devSeclabel): Allow it in RNG.
> * tests/qemuxml2argvdata/qemuxml2argv-seclabel-*-skiplabel.xml:
> * tests/qemuxml2argvdata/qemuxml2argv-seclabel-*-skiplabel.args:
> * tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-*-skiplabel.xml:
> New test files.
> * tests/qemuxml2argvtest.c (mymain): Run the new tests.
> * tests/qemuxml2xmltest.c (mymain): Likewise.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> 
>  docs/formatdomain.html.in                          |  6 ++-
>  docs/schemas/domaincommon.rng                      | 27 +++++++------
>  src/conf/domain_conf.c                             | 47 ++++++++++++++++------
>  src/conf/domain_conf.h                             |  3 +-
>  src/security/security_selinux.c                    | 10 ++++-
>  .../qemuxml2argv-seclabel-dynamic-skiplabel.args   |  5 +++
>  .../qemuxml2argv-seclabel-dynamic-skiplabel.xml    | 32 +++++++++++++++
>  .../qemuxml2argv-seclabel-static-skiplabel.args    |  5 +++
>  .../qemuxml2argv-seclabel-static-skiplabel.xml     | 33 +++++++++++++++
>  tests/qemuxml2argvtest.c                           |  2 +
>  .../qemuxml2xmlout-seclabel-dynamic-skiplabel.xml  | 31 ++++++++++++++
>  tests/qemuxml2xmltest.c                            |  8 ++--
>  12 files changed, 178 insertions(+), 31 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-skiplabel.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-skiplabel.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-static-skiplabel.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-static-skiplabel.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-skiplabel.xml

ACK


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