Bug 714997

Summary: virsh creates images and directories with the wrong ownership and permissions
Product: [Community] Virtualization Tools Reporter: Lex van Roon <r3boot>
Component: libvirtAssignee: Libvirt Maintainers <libvirt-maint>
Status: CLOSED CURRENTRELEASE QA Contact:
Severity: medium Docs Contact:
Priority: unspecified    
Version: unspecifiedCC: crobinso, janne.savikko, tharbaug, xen-maint
Target Milestone: ---   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-03-23 13:01:08 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Lex van Roon 2011-06-21 14:43:02 UTC
Description of problem:
When creating a directory-based storage pool with virsh, the ownership and permissions are set to root:root, 0700/0600. This causes all tools running as a privileged user to fail (most notably, virt-install). The same thing happens when trying to create an image within a directory-based storage pool.

Version-Release number of selected component (if applicable):
libvirt: 0.9.2-1
OS: Debian/Squeeze

How reproducible:
Fully reproducible.

Steps to Reproduce:
1. Create a directory-based storage pool
2. Build the directory-based storage pool
2a. Create a image within a directory-based storage pool
  
Actual results:

==> directory-based storage pool:
$ id
uid=1000(user) gid=1000(user) groups=1000(user),24(cdrom),25(floppy),27(sudo),29(audio),30(dip),44(video),46(plugdev),108(bluetooth),111(netdev),123(libvirt)

$ virsh pool-define-as virtpool dir --target /var/lib/libvirt/images/virtpool
Pool virtpool defined

$ virsh pool-build virtpool
Pool virtpool built

$ ls -lad /var/lib/libvirt/images/virtpool
drwx------ 1 root root 0 Jun 21 16:21 /var/lib/libvirt/images/virtpool

==> image within a directory-based storage pool:
$ sudo chown libvirt-qemu:libvirt /var/lib/libvirt/images/virtpool
$ sudo chmod 0770 /var/lib/libvirt/images/virtpool
$ ls -lad /var/lib/libvirt/images/virtpool
drwxrwx--- 1 libvirt-qemu libvirt 24 Jun 21 16:29 /var/lib/libvirt/images/virtpool

$ virsh vol-create-as virtpool machine1.img 3G --format qcow2
Vol machine1.img created

$ ls -la /var/lib/libvirt/images/virtpool/machine1.img
ls: cannot access /var/lib/libvirt/images/virtpool/machine1.img: Permission denied

$ sudo ls -la /var/lib/libvirt/images/virtpool/machine1.img
-rw------- 1 root root 197120 Jun 21 16:37 /var/lib/libvirt/images/virtpool/machine1.img

Expected results:
A directory having ownerships and permissions usable by libvirt, something like libvirt-qemu:libvirt, 0770.

Additional info:
It would be nice to be able to specify the owner and the permissions when defining a pool and/or a volume.

Comment 1 Janne Savikko 2011-10-30 19:28:42 UTC
Description of problem:
See above.

Version-Release number of selected component (if applicable):
libvirt: 0.9.2-4ubuntu15
OS: Ubuntu/Oneiric

Additional info:
It seems that libvirt (or virsh/virt-manager) doesn't care about storage pool template's permissions even though I have them defined. virsh sets permissions to 0600 root:root, virt-manager to 0600 102:105.


$ virsh pool-list --details
Name     State    Autostart  Persistent   Capacity  Allocation  Available
-------------------------------------------------------------------------
default  running  yes        yes         220.01 GB    29.70 GB  190.31 GB 

$ virsh pool-dumpxml default
<pool type='dir'>
  <name>default</name>
  <uuid>8235850b-ffaf-0dc3-11b1-3be5b36e7b97</uuid>
  <capacity>236228661240</capacity>
  <allocation>31886737400</allocation>
  <available>204341923832</available>
  <source>
  </source>
  <target>
    <path>/var/lib/libvirt/images</path>
    <permissions>
      <mode>0640</mode>
      <owner>102</owner>
      <group>105</group>
    </permissions>
  </target>
</pool>

Comment 2 Thayne Harbaugh 2014-12-03 19:24:13 UTC
It appears that this is due to
security/security_dac.c:virSecurityDACRestoreSecurityFileLabelInternal()
which indiscriminately sets the owner:group to 0:0 regardless of what it is
supposed to be:

  static int
  virSecurityDACRestoreSecurityFileLabelInternal(virSecurityDACDataPtr priv,
                                                 virStorageSourcePtr src,
                                                 const char *path)
  {
      VIR_INFO("Restoring DAC user and group on '%s'",
               NULLSTR(src ? src->path : path));
  
      /* XXX record previous ownership */
      return virSecurityDACSetOwnershipInternal(priv, src, path, 0, 0);
  }

Notice the nice comment: XXX record previous ownership.  Someone knew that
it would be a problem and glossed over it.

Something like this might help:

  static int
  virSecurityDACRestoreSecurityFileLabelInternal(virSecurityDACDataPtr priv,
                                                 virStorageSourcePtr src,
                                                 const char *path)
  {
      uid_t uid = 0;
      gid_t gid = 0;
  
      VIR_INFO("priv(%p)", priv);
  
      if (priv) {
          uid = priv->user;
          gid = priv->group;
      }
  
      VIR_INFO("Restoring DAC user and group on '%s'",
               NULLSTR(src ? src->path : path));
  
      /* XXX record previous ownership */
      return virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid);
  }

But my concern is that it still doesn't remember what it should be - it
just sets it to a value.  If I understood more about *why* the ownership
needed to be changed back-and-forth I'd make a real patch.

Comment 3 Thayne Harbaugh 2014-12-03 19:29:34 UTC
My guess is that 1004673 is related to this:

  https://bugzilla.redhat.com/show_bug.cgi?id=1004673

Comment 4 Thayne Harbaugh 2014-12-03 19:58:48 UTC
A temporary solution is to disable ownership shuffling in /etc/libvirt/libvirtd.conf:

  dynamic_ownership = 0

Comment 5 Cole Robinson 2016-03-23 13:01:08 UTC
There's a few things in here. The original report about default storage pool permissions is no longer valid, upstream uses 0755 now.

The latter comments about libvirt not resetting file ownership to the original owner is a separate issue and is tracked in https://bugzilla.redhat.com/show_bug.cgi?id=636156

Closing as CURRENTRELEASE since the original reported issue is fixed