Re: [libvirt] [PATCH 0/12] Improve security driver handling & QEMU DAC management

On Tue, Jan 26, 2010 at 02:28:45PM +0200, Dan Kenigsberg wrote: > On Tue, Jan 26, 2010 at 10:23:01AM +0000, Daniel P. Berrange wrote: > > On Tue, Jan 26, 2010 at 12:06:58PM +0200, Dan Kenigsberg wrote: > > > On Wed, Jan 20, 2010 at 03:14:57PM +0000, Daniel P. Berrange wrote: > > > > This patch series does some work on te security drivers, and the QEMU code > > > > for managing DAC permissions on files. > > > > > > > > The core goal is to turn the QEMU driver DAC file management code into a > > > > security driver. Instead of QEMU calling into the SELinux/AppArmour drivers > > > > directly, a stacked driver module is introduced. This delegates all operations > > > > to first the QEMU DAC driver, and then the main SELinux/AppArmour driver. > > > > The end result is that all the permissions management code is removed from > > > > the QEMU driver, and we're left with just simple security driver calls. > > > > > > > > In the process of this a number of flaws in the current hotplug code were > > > > found, and code was generally tidied up with a view to making it easier to > > > > manage. > > > > > > > > Finally, we add the ability to turn off the QEMU DAC file managment code, > > > > and also deal gracefully with failures to change ownership (eg on NFS with > > > > root squash, or readonly FS). > > > > > > hmmm, there's another problem which this patch set does not address: > > > error : virStorageFileGetMetadata:415 : cannot open file '/deep/into/my/root/squashing/export': Permission denied > > > > > > With dynamic_ownership = 0, libvirt down not mess with chown, but it now > > > assumes that it can read disk images. > > > > That is a not unreasonable assumption. The model we want to support is one > > where > > > > - App uses virStoragePool APIs to setup the NFS mount point on the host > > - App uses virStorageVol APIs to create the new volume with correct > > ownership at time of creation > > - libvirt starts the guest, without needing chown() due to step 2 > > getting ownership correct at time of creation. > > > > If libvirt can't read the directories leading upto the image, then it > > would never have been able to create the volume in the first place. The > > patchs we have done have aimed at avoiding root squash problems which > > prevent chmod/chown of files between root & non-root. We still assume > > that we can read files / browse directories. > > Understood. But if I am handling storage myself, I have a regression > when I switch to libvirt from the sordid art of playing with qemu > directly. I don't want to make my repository world-readable, and I don't > want (yet) to use libvirt storage API. > > Do I have a third option? virStorageFileGetMetadata() is used to determine if the disk configured for a guest has a backing file (eg qcow linked images). This is called from the DAC driver to setup ownership, and from the SELinux driver to setup labelling. You say you've already disabled the DAC driver ownership so it shouldn't be hitting it there. Thus I assume it must the SELinux driver that's causing the error. > Note that before this patchset, I could apply a patch disabling chown's > and I managed to start domains over root squashing nfs. I'm surprised if that's true, because I don't believe my patches changed this behaviour of the SELinux driver - it was always calling into the virStorageFileGetMetadata() method to determine backing store, so should always have hit the issue you describe