Bug 860519

Summary: security: support for names on DAC labels
Product: Red Hat Enterprise Linux 6 Reporter: Wayne Sun <gsun>
Component: libvirtAssignee: Peter Krempa <pkrempa>
Status: CLOSED ERRATA QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 6.4CC: acathrow, dallan, dyasny, dyuan, gsun, mzhan, rwu, veillard, xuzhang, ydu
Target Milestone: rc   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: libvirt-0.10.2-3.el6 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 856951 Environment:
Last Closed: 2013-02-21 07:24:16 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Comment 2 Peter Krempa 2012-09-26 09:42:24 UTC
This bug requests adding support for the following usage pattern:

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'

The current code supports only numerical IDs for the DAC seclabel. A feature to add support for usernames was submitted for upstream review: http://www.redhat.com/archives/libvir-list/2012-September/msg01503.html and is waiting for a final review iteration after the author sends a new version.

Comment 4 Peter Krempa 2012-10-03 10:26:29 UTC
This feature was added upstream by:

commit 0d0a7cdcf5ee64922b5966370833e992afb09eba
Author: Marcelo Cerri <mhcerri.ibm.com>
Date:   Tue Oct 2 14:57:37 2012 -0300

    doc: update description about security labels on formatdomain.html
    
    This patch adds a brief description about labels for each security
    driver.

commit 60469dd10f97fcba60f0eeeae6965b83fbb9d652
Author: Marcelo Cerri <mhcerri.ibm.com>
Date:   Tue Oct 2 14:57:36 2012 -0300

    security: also parse user/group names instead of just IDs for DAC labels
    
    The DAC driver is missing parsing of group and user names for DAC labels
    and currently just parses uid and gid. This patch extends it to support
    names, so the following security label definition is now valid:
    
      <seclabel type='static' model='dac' relabel='yes'>
          <label>qemu:qemu</label>
          <imagelabel>qemu:qemu</imagelabel>
      </seclabel>
    
    When it tries to parse an owner or a group, it first tries to resolve it as
    a name, if it fails or it's an invalid user/group name then it tries to
    parse it as an UID or GID. A leading '+' can also be used for both owner and
    group to force it to be parsed as IDs, so the following example is also
    valid:
    
      <seclabel type='static' model='dac' relabel='yes'>
          <label>+101:+101</label>
          <imagelabel>+101:+101</imagelabel>
      </seclabel>
    
    This ensures that UID 101 and GUI 101 will be used instead of an user or
    group named "101".

$ git describe --tags
v0.10.2-89-g0d0a7cd

Comment 12 Jiri Denemark 2012-10-08 13:29:36 UTC
Moving back to ASSIGNED until additional fixes are ready.

Comment 15 Wayne Sun 2012-10-16 03:46:19 UTC
pkgs:
libvirt-0.10.2-3.el6.x86_64
qemu-kvm-rhev-0.12.1.2-2.317.el6.x86_64
kernel-2.6.32-279.el6.x86_64

steps:
1. edit domain add following xml:
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>qemu:qemu</label>
  </seclabel>
...

2. start domain
# virsh start libvirt_test_api
Domain libvirt_test_api started

# virsh dumpxml libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>qemu:qemu</label>
    <imagelabel>qemu:qemu</imagelabel>
  </seclabel>
...

3. add uid:gid with a plus
# virsh destroy libvirt_test_api
Domain libvirt_test_api destroyed

# virsh edit libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>+107:107</label>
  </seclabel>
...

Domain libvirt_test_api XML configuration edited.

4. start domain and check
# virsh start libvirt_test_api
Domain libvirt_test_api started

# virsh dumpxml libvirt_test_api
  <seclabel type='static' model='dac' relabel='yes'>
    <label>+107:107</label>
    <imagelabel>+107:107</imagelabel>
  </seclabel>

# ps aux|grep libvirt_test_api
qemu     17621 61.3  0.2 1456976 298024 ?      Sl   11:08   0:15 /usr/libexec/qemu-kvm -name libvirt_test_api -S -M rhel6.4.0 -enable-kvm -m 1024 -smp 1,sockets=1,cores=1,threads=1 -uuid 05867c1a-afeb-300e-e55e-2673391ae080 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/libvirt_test_api.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/libvirt-test-api,if=none,id=drive-virtio-disk0,format=qcow2 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -netdev tap,fd=22,id=hostnet0,vhost=on,vhostfd=23 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=54:52:00:45:c3:8a,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -vnc 127.0.0.1:0 -vga cirrus -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6

# ll -Z /var/lib/libvirt/images/libvirt-test-api 
-rw-r--r--. qemu qemu unconfined_u:object_r:svirt_image_t:s0:c75,c651 /var/lib/libvirt/images/libvirt-test-api

5. mix name with id with plus
# virsh destroy libvirt_test_api
Domain libvirt_test_api destroyed

# virsh edit libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>qemu:+107</label>
  </seclabel>
... 

6. start and check
# virsh start libvirt_test_api
Domain libvirt_test_api started

# virsh dumpxml libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>qemu:+107</label>
    <imagelabel>qemu:+107</imagelabel>
  </seclabel>
...

# ps aux|grep qemu
qemu     18100 11.3  0.2 1385196 289660 ?      Sl   11:14   0:15 /usr/libexec/qemu-kvm -name libvirt_test_api -S -M rhel6.4.0 -enable-kvm -m 1024 -smp 1,sockets=1,cores=1,threads=1 -uuid 05867c1a-afeb-300e-e55e-2673391ae080 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/libvirt_test_api.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/libvirt-test-api,if=none,id=drive-virtio-disk0,format=qcow2 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -netdev tap,fd=22,id=hostnet0,vhost=on,vhostfd=23 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=54:52:00:45:c3:8a,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -vnc 127.0.0.1:0 -vga cirrus -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6

# ll -Z /var/lib/libvirt/images/
-rw-r--r--. qemu qemu unconfined_u:object_r:svirt_image_t:s0:c7,c786 libvirt-test-api

7. test with non-exist group name or gid
# virsh destroy libvirt_test_api
Domain libvirt_test_api destroyed

# virsh edit libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>qemu:+1000</label>
  </seclabel>
... 

# virsh start libvirt_test_api
Domain libvirt_test_api started

# virsh dumpxml libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>qemu:+1000</label>
    <imagelabel>qemu:+1000</imagelabel>
  </seclabel>
...

# ll -Z /var/lib/libvirt/images/
-rw-r--r--. qemu 1000 unconfined_u:object_r:svirt_image_t:s0:c196,c515 libvirt-test-api

# ps aux|grep qemu
qemu     18437  4.7  0.2 1383140 330380 ?      Sl   11:18   0:15 /usr/libexec/qemu-kvm -name libvirt_test_api -S -M rhel6.4.0 -enable-kvm -m 1024 -smp 1,sockets=1,cores=1,threads=1 -uuid 05867c1a-afeb-300e-e55e-2673391ae080 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/libvirt_test_api.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/libvirt-test-api,if=none,id=drive-virtio-disk0,format=qcow2 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -netdev tap,fd=22,id=hostnet0,vhost=on,vhostfd=23 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=54:52:00:45:c3:8a,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -vnc 127.0.0.1:0 -vga cirrus -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6


This is little unexpected here, group 1000 did not exist on system, but the domain can start and the img also with group 1000. Since gid not blocking starting domain here, this could be ok, but when with unexist group name later this fail, so there might have a problem.

# virsh edit libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>107:vdsm</label>
  </seclabel>
... 

Domain libvirt_test_api XML configuration edited.

# virsh start libvirt_test_api
error: Failed to start domain libvirt_test_api
error: internal error invalid argument: Failed to parse group 'vdsm'


8. test with invalid uid or user name
Now try with a noexist uid:
# virsh destroy libvirt_test_api
Domain libvirt_test_api destroyed

# virsh edit libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>+36:+1000</label>
  </seclabel>
...
Domain libvirt_test_api XML configuration edited.

# virsh start libvirt_test_api
error: Failed to start domain libvirt_test_api
error: internal error internal error getpwuid_r failed to retrieve data for uid '36'

# virsh edit libvirt_test_api
...
  <seclabel type='static' model='dac' relabel='yes'>
    <label>vdsm:+1000</label>
  </seclabel>
...
Domain libvirt_test_api XML configuration edited.

# virsh start libvirt_test_api
error: Failed to start domain libvirt_test_api
error: internal error invalid argument: Failed to parse user 'vdsm'

This is expected on user uid or name.

Hi Peter,

Can you help explain the problem in step 7 ? 

thanks!

Comment 16 Peter Krempa 2012-10-16 13:25:50 UTC
When using numeric ID's the uid/gid can be changed to any value even "non-existing" uids/gids. The kernel doesn't care about this behavior as it uses numeric ID's. 

When using string names, the names have to be converted to uid/gid at first. When doing this, the group must exist to retrieve uid/gid for it before doing the changes to images.

The behavior you encountered is therefore fully expected.

Comment 17 Wayne Sun 2012-10-17 03:30:36 UTC
(In reply to comment #16)
> When using numeric ID's the uid/gid can be changed to any value even
> "non-existing" uids/gids. The kernel doesn't care about this behavior as it
> uses numeric ID's. 
> 
> When using string names, the names have to be converted to uid/gid at first.
> When doing this, the group must exist to retrieve uid/gid for it before
> doing the changes to images.
> 
> The behavior you encountered is therefore fully expected.

Thanks to explain this. 
This is fixed.

Comment 18 errata-xmlrpc 2013-02-21 07:24:16 UTC
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