Bug 851491

Although the following patch can prevent libvirtd crash, users can't successfull start previous guest due to a security context error:

# virsh start foo
error: Failed to start domain foo
error: unable to set security context 'unconfined_u:object_r:svirt_image_t' on '/var/lib/libvirt/images/foo': Invalid argument

# ll -Z /var/lib/libvirt/images/foo
-rw-r--r--. root tty unconfined_u:object_r:virt_image_t:s0 /var/lib/libvirt/images/foo

Notes, the image's group is 'tty' now, it's not expected.

---
 src/security/security_dac.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 925498f..52286f7 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -898,7 +898,7 @@ virSecurityDACGenLabel(virSecurityManagerPtr mgr,
         return rc;
     }
 
-    if (!seclabel->norelabel) {
+    if (!seclabel->norelabel && seclabel->label) {
         if (seclabel->imagelabel == NULL) {
             seclabel->imagelabel = strdup(seclabel->label);
             if (seclabel->imagelabel == NULL) {
-- 
1.7.1

Michal, Is this bug relevant with DAC regression issues? (In reply to comment #3)
> Although the following patch can prevent libvirtd crash, users can't
> successfull start previous guest due to a security context error:
> 
> # virsh start foo
> error: Failed to start domain foo
> error: unable to set security context 'unconfined_u:object_r:svirt_image_t'
> on '/var/lib/libvirt/images/foo': Invalid argument
> 
> # ll -Z /var/lib/libvirt/images/foo
> -rw-r--r--. root tty unconfined_u:object_r:virt_image_t:s0
> /var/lib/libvirt/images/foo
> 
> Notes, the image's group is 'tty' now, it's not expected.

Michal, I saw the upstream is discussing DAC regression question, Is this bug relevant with DAC regression issues?

Alex,

yes. Running a git bisect showed it's caused by this commit:

6d6bff3a46315c4f868191e315165d155cbacb93 is the first bad commit
commit 6d6bff3a46315c4f868191e315165d155cbacb93
Author: Marcelo Cerri <mhcerri.ibm.com>
Date:   Wed Aug 15 19:10:38 2012 -0300

    Support for multiple default security drivers in QEMU config
    
    This patch updates the key "security_driver" in QEMU config to suport
    both a sigle default driver or a list of default drivers. This ensures
    that it will remain compatible with older versions of the config file.
    
    Signed-off-by: Marcelo Cerri <mhcerri.ibm.com>

which is a part of DAC code rewrite. So it's relevant to DAC regression.

Crash fixed with upstream commit:

commit f2b241e6a76a2b01f002bf1f71ee321b40bd2ee2
Author: Peter Krempa <pkrempa>
Date:   Tue Aug 28 18:27:31 2012 +0200

    security_dac: Avoid segfault when no label is requested
    
    When no DAC "label" was requested for a domain the DAC manager tried to
    strdup a NULL string causing a segfault.

after fixing the crash a second problem emerged caused by passing uninitialized values to function setting uid and gid of the spawned process and images that it uses that is fixed by:

commit 3c2f5e3ede67e9349c68422ef2d867cb64ef4329
Author: Peter Krempa <pkrempa>
Date:   Tue Aug 28 18:29:38 2012 +0200

    security_dac: Don't return uninitialised value when parsing seclabels
    
    When starting a machine the DAC security driver tries to set the UID and
    GID of the newly spawned process. This worked as desired if the desired
    label was set. When the label was missing a logical bug in
    virSecurityDACGenLabel() caused that uninitialised values were used as
    uid and gid for the new process.
    
    With this patch, default values (from qemu driver configuration)
    are used if the label is not found.

and

commit f2f0af3960742434a35348628fccbc8169805df3
Author: Peter Krempa <pkrempa>
Date:   Wed Aug 29 01:15:04 2012 +0200

    security_dac: Don't return uninitialised uid and gid for image labels
    
    As in the previous commit, images are also chowned to uninitialised
    uid and gid if the label is not present.

With these patches libvirt now behaves as in previous versions:
- the daemon does not segfault
- the spawned process has correct uid and gid
- images used by the process are chowned to correct uid and gid
- starting of the machine _still fails_ with "Invalid argument" while setting selinux labels on images:

*libvirt version: 0.9.11.4, package: 3.fc17
$ virsh -c qemu:///system start test
error: Failed to start domain test
error: unable to set security context 'system_u:object_r:svirt_image_t' on '/var/lib/libvirt/images/q.img': Invalid argument

*current upstream version, compiled from git:
$ virsh -c qemu:///system start test
error: Failed to start domain test
error: unable to set security context 'unconfined_u:object_r:svirt_image_t' on '/var/lib/libvirt/images/q.img': Invalid argument

(Note that the labels are different - probably because the git version isn't labeled correctly -, but the setting still fails)

To verify this bug, It's important to check that the spawned process and all it's images have correct UNIX permissions as those were affected by problems covered in this fix.

Moving to POST.

(In reply to comment #6)
> 
> With these patches libvirt now behaves as in previous versions:
> - the daemon does not segfault
> - the spawned process has correct uid and gid
> - images used by the process are chowned to correct uid and gid
> - starting of the machine _still fails_ with "Invalid argument" while
> setting selinux labels on images:

It should not fail, this problem is fixed in bug:
https://bugzilla.redhat.com/show_bug.cgi?id=823857

And verified on libvirt-0.10.0-0rc0.el6.x86_64 , so some fix may broke what's done in bug 823857. 

> 
> *libvirt version: 0.9.11.4, package: 3.fc17
> $ virsh -c qemu:///system start test
> error: Failed to start domain test
> error: unable to set security context 'system_u:object_r:svirt_image_t' on
> '/var/lib/libvirt/images/q.img': Invalid argument
> 
> *current upstream version, compiled from git:
> $ virsh -c qemu:///system start test
> error: Failed to start domain test
> error: unable to set security context 'unconfined_u:object_r:svirt_image_t'
> on '/var/lib/libvirt/images/q.img': Invalid argument
> 
> (Note that the labels are different - probably because the git version isn't
> labeled correctly -, but the setting still fails)
> 
> To verify this bug, It's important to check that the spawned process and all
> it's images have correct UNIX permissions as those were affected by problems
> covered in this fix.
> 
> Moving to POST.

pkgs:
libvirt-0.10.0-1.el6.x86_64

steps:
1. set "security_default_confined = 0" in qemu.conf
# vim /etc/libvirt/qemu.conf

# service libvirtd restart
Stopping libvirtd daemon:                                  [  OK  ]
Starting libvirtd daemon:                                  [  OK  ]

2. start a domain
# virsh start dom_test
error: Failed to start domain dom_test
error: unable to set security context 'unconfined_u:object_r:svirt_image_t' on '/var/lib/libvirt/images/dom_test': Invalid argument

libvirtd not crash here.
Invalid argument is another problem need fix, cloned a new bug for tracking this:
https://bugzilla.redhat.com/show_bug.cgi?id=853043

3. set selinux permissive and start domain
# setenforce 0
# sestatus
SELinux status:                 enabled
SELinuxfs mount:                /selinux
Current mode:                   permissive
Mode from config file:          enforcing
Policy version:                 24
Policy from config file:        targeted

# virsh start dom_test
Domain dom_test started

4. check with img and proce

# ll -Z /var/lib/libvirt/images/dom_test 
-rw-r--r--. qemu qemu unconfined_u:object_r:svirt_image_t /var/lib/libvirt/images/dom_test

# ps Zaux|grep dom_test
unconfined_u:system_r:qemu_t:s0-s0:c0.c1023 qemu 12730 24.8  4.8 3534288 386864 ? Sl 18:33   0:19 /usr/libexec/qemu-kvm -name dom_test -S -M rhel6.2.0 -enable-kvm -m 3000 -mem-prealloc -mem-path /var/hugepages/libvirt/qemu -smp 2,sockets=2,cores=1,threads=1 -uuid 0c5659b7-59f8-b5d9-199c-d14255038f91 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/dom_test.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive file=/var/lib/libvirt/images/dom_test,if=none,id=drive-virtio-disk0,format=raw,cache=none -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive if=none,media=cdrom,id=drive-ide0-0-0,readonly=on,format=raw -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -netdev tap,fd=23,id=hostnet0,vhost=on,vhostfd=24 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=54:52:00:98:92:8d,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -vnc 127.0.0.1:0 -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
ss

uid/gid working fine with both img and process. So this bug is fixed.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2013-0276.html