Bug 856951

Fixed upstream with:
commit ede89aab64ba9eeb953f4b895bfaba13f64ab5ed
Author: Peter Krempa <pkrempa>
Date:   Tue Sep 18 12:20:41 2012 +0200

    security: Don't ignore errors when parsing DAC security labels
    
    The DAC security driver silently ignored errors when parsing the DAC
    label and used default values instead.
    
    With a domain containing the following label definition:
    
    <seclabel type='static' model='dac' relabel='yes'>
      <label>sdfklsdjlfjklsdjkl</label>
    </seclabel>
    
    the domain would start normaly but the disk images would be still owned
    by root and no error was displayed.
    
    This patch changes the behavior if the parsing of the label fails (note
    that a not present label is not a failure and in this case the default
    label should be used) the error isn't masked but is raised that causes
    the domain start to fail with a descriptive error message:
    
    virsh #  start tr
    error: Failed to start domain tr
    error: internal error invalid argument: failed to parse DAC seclabel
    'sdfklsdjlfjklsdjkl' for domain 'tr'
    
    I also changed the error code to "invalid argument" from "internal
    error" and tweaked the various error messages to contain correct and
    useful information.

Now the invalid label is rejected on domain start.

pkgs:
libvirt-0.10.2-1.el6.x86_64
kernel-2.6.32-279.el6.x86_64
qemu-kvm-rhev-0.12.1.2-2.313.el6.x86_64

steps:
scenario 1:
1. edit domain and add invalid static dac seclable
# virsh edit $domain
...
<seclabel type='static' model='dac' relabel='yes'>
<label>sdfklsdjlfjklsdjkl</label>
</seclabel>
...

2. start domain
# virsh start libvirt_test_api
error: Failed to start domain libvirt_test_api
error: internal error invalid argument: failed to parse DAC seclabel 'sdfklsdjlfjklsdjkl' for domain 'libvirt_test_api'

scenario 2:
1. edit domain with static non-exist user:group dac seclable
# virsh edit $domain
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>aaa:aaa</label>
  </seclabel>
...

2. start domain
# virsh start libvirt_test_api
error: Failed to start domain libvirt_test_api
error: internal error invalid argument: failed to parse DAC seclabel 'aaa:aaa' for domain 'libvirt_test_api'

scenario 3:
1. start domain with valid user:group
# virsh edit $domain
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>qemu:qemu</label>
  </seclabel>
...

2. start domain
# virsh start libvirt_test_api
error: Failed to start domain libvirt_test_api
error: internal error invalid argument: failed to parse DAC seclabel 'qemu:qemu' for domain 'libvirt_test_api'

This is expected for now, only uid:gid is supported, upstream is discussing of the patch which also parse user name/group. Maybe a bug should filed to track this.


scenario 4:
1. start domain with valid user:group in id
# virsh edit $domain
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>107:107</label>
  </seclabel>
...

2. start domain
# virsh start libvirt_test_api
Domain libvirt_test_api started

3. check xml and img
# virsh dumpxml libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>107:107</label>
    <imagelabel>107:107</imagelabel>
  </seclabel>
...

# ll -Z /var/lib/libvirt/images/libvirt_test_api 
-rw-r--r--. qemu qemu unconfined_u:object_r:svirt_image_t:s0:c199,c861 /var/lib/libvirt/images/libvirt_test_api

scenario 5:
1. When user/group as default which is qemu:qemu in qemu.conf, edit domain with static dac seclable as 0:0

# virsh edit $domain
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>0:0</label>
  </seclabel>
...

2. start domain
# virsh start libvirt_test_api
error: Failed to start domain libvirt_test_api
error: internal error Process exited while reading console log output: 2012-09-25 02:50:25.346+0000: 30212: debug : virFileClose:72 : Closed fd 21
2012-09-25 02:50:25.346+0000: 30212: debug : virFileClose:72 : Closed fd 28
2012-09-25 02:50:25.351+0000: 30212: debug : virFileClose:72 : Closed fd 3
bind(unix:/var/lib/libvirt/qemu/libvirt_test_api.monitor): Permission denied
chardev: opening backend "socket" failed

This is expected I think, as qemu will fail to open a socket with root permission.

scenario 6:
Following the setting after scenario 5
1. modify qemu.conf with user/group as root/root.
...
user = "root"
group = "root"

2. start domain
# virsh start libvirt_test_api
Domain libvirt_test_api started

3. check domain img and xml
# virsh dumpxml libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>0:0</label>
    <imagelabel>0:0</imagelabel>
  </seclabel>
...

# ll -Z /var/lib/libvirt/images/libvirt_test_api 
-rw-r--r--. root root unconfined_u:object_r:svirt_image_t:s0:c48,c416 /var/lib/libvirt/images/libvirt_test_api

# ll -Z /var/lib/libvirt/qemu/libvirt_test_api.monitor 
srwxr-xr-x. root root unconfined_u:object_r:qemu_var_run_t:s0:c48,c416 /var/lib/libvirt/qemu/libvirt_test_api.monitor

scenario 7:
Following the setting after scenario 6
1. edit domain with uid:gid as 107:107
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>107:107</label>
  </seclabel>
...

2. start domain
# virsh start libvirt_test_api
error: Failed to start domain libvirt_test_api
error: internal error Process exited while reading console log output: 2012-09-25 03:01:34.460+0000: 30509: debug : virFileClose:72 : Closed fd 21
2012-09-25 03:01:34.460+0000: 30509: debug : virFileClose:72 : Closed fd 28
2012-09-25 03:01:34.464+0000: 30509: debug : virFileClose:72 : Closed fd 3
bind(unix:/var/lib/libvirt/qemu/libvirt_test_api.monitor): Permission denied
chardev: opening backend "socket" failed

# ll -Z /var/lib/libvirt/qemu/
drwxr-xr-x. root root system_u:object_r:qemu_var_run_t:s0 dump
srwxr-xr-x. root root unconfined_u:object_r:qemu_var_run_t:s0:c48,c416 myRHEL6.agent
drwxr-xr-x. root root system_u:object_r:qemu_var_run_t:s0 save
drwxr-xr-x. root root system_u:object_r:qemu_var_run_t:s0 snapshot

the monitor socket is not created, bit confusing here, shouldn't root have permission on qemu files? Maybe the logic is not right here.

Hi, Peter

Can you halp to confirm need we file a new bug for tracking user:group name problem in secnario 3, and the question I raised in secnario 7. 

Thanks!

In scenario 5 the failure to create the socket is a little bit unexpected for me, but that might be due to selinux labeling. Libvirt changes owner of the directory containing the socket files to the user who is specified in /etc/libvirt/qemu.conf configuration file. The DAC labeling code then changes an individual domain to the specified uid and gid. With <label>0:0</label> the domain is actually running with root permissions and should have DAC access to the directory (confirmed on a non-selinux host). The other way around as described in scenario 6 and 7 when /etc/libvirt/qemu.conf contains user=root, then the files are actualy owned by root and you're trying to access them as non root with a non-root user who doesn't have permissions as the default permissions for this directory are:

# ls -la /var/lib/libvirt/qemu/
drwxrwx---   5 root root  4096 Sep 25 10:46 .

I'd rise your scenario 7 question in scenario 5, but the cause will probably be selinux.

(In reply to comment #7)
> (In reply to comment #6)
> > As of filing a bug for parsing user and group names along with UID's:
> > Libvirt is now rebased for 6.4 and now if the code (that was posted
> > upstream) makes it into upstream libvirt, there's no guarantee it will be
> > backported. You may file a bug about this (requesting the backport) but I
> > can't guarantee the backport will be done.
> 
> This BZ is sufficient to request the backport--it's flagged as 6.4.0.
> 
> > Dave,
> > what's your opinion on backporting
> > http://www.redhat.com/archives/libvir-list/2012-September/msg01503.html once
> > it gets accepted upstream?
> 
> That depends on the backport, but I think at this point it's reasonable to
> take it.

The problem is verified in this bug.
Thanks for help.

A new bug is cloned for tracking DAC name problem (if I don't understand wrong here):
Bug 860519 - security: support for names on DAC labels
https://bugzilla.redhat.com/show_bug.cgi?id=860519

as in comment #4 and comment #8, this is verified.

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