[libvirt] [PATCH 1/2] nvram: Fix permissions

[Ján Tomko]

On 09/11/2014 04:25 PM, Michal Privoznik wrote:
> On 11.09.2014 16:07, Laszlo Ersek wrote:
>> On 09/11/14 14:09, Michal Privoznik wrote:
>>> I've noticed two problem with the automatically created NVRAM varstore
>>> file. The first, even though I run qemu as root:root for some reason I
>>> get Permission denied when trying to open the _VARS.fd file. The
>>> problem is, the upper directory misses execute permissions, which in
>>> combination with us dropping some capabilities result in EPERM.
>>>
>>> The next thing is, that if I switch SELinux to enforcing mode, I get
>>> another EPERM because the vars file is not labeled correctly. It is
>>> passed to qemu as disk and hence should be labelled as disk. QEMU may
>>> write to it eventually, so this is different to kernel or initrd.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>   libvirt.spec.in                 | 2 +-
>>>   src/security/security_selinux.c | 5 ++++-
>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>>> index a6a58cf..ecf160b 100644
>>> --- a/libvirt.spec.in
>>> +++ b/libvirt.spec.in
>>> @@ -1938,7 +1938,7 @@ exit 0
>>>   %dir %attr(0750, %{qemu_user}, %{qemu_group})
>>> %{_localstatedir}/lib/libvirt/qemu/
>>>   %dir %attr(0750, %{qemu_user}, %{qemu_group})
>>> %{_localstatedir}/lib/libvirt/qemu/channel/
>>>   %dir %attr(0750, %{qemu_user}, %{qemu_group})
>>> %{_localstatedir}/lib/libvirt/qemu/channel/target/
>>> -%dir %attr(0750, %{qemu_user}, %{qemu_group})
>>> %{_localstatedir}/lib/libvirt/qemu/nvram/
>>> +%dir %attr(0711, %{qemu_user}, %{qemu_group})
>>> %{_localstatedir}/lib/libvirt/qemu/nvram/
>>>   %dir %attr(0750, %{qemu_user}, %{qemu_group})
>>> %{_localstatedir}/cache/libvirt/qemu/
>>>   %{_datadir}/augeas/lenses/libvirtd_qemu.aug
>>>   %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
>>> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
>>> index bf67fb5..3db2b27 100644
>>> --- a/src/security/security_selinux.c
>>> +++ b/src/security/security_selinux.c
>>> @@ -2300,8 +2300,11 @@
>>> virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr,
>>>                                        mgr) < 0)
>>>           return -1;
>>>
>>> +    /* This is different than kernel or initrd. The nvram store
>>> +     * is really a disk, qemu can read and write to it. */
>>>       if (def->os.loader && def->os.loader->nvram &&
>>> -        virSecuritySELinuxSetFilecon(def->os.loader->nvram,
>>> data->content_context) < 0)
>>> +        secdef && secdef->imagelabel &&
>>> +        virSecuritySELinuxSetFilecon(def->os.loader->nvram,
>>> secdef->imagelabel) < 0)
>>>           return -1;
>>>
>>>       if (def->os.kernel &&
>>>
>>
>> Good detective work!
>>
>> Regarding the g+x,o+x change on
>> %{_localstatedir}/lib/libvirt/qemu/nvram. This change theoretically
>> allows a qemu instance to "probe" for the presence of "foreign" varstore
>> files (it won't be able to open any, but eg. error codes for open()
>> would differ between ENOENT vs. EACCES, and stat() would fail vs.
>> succeed). However I think we can live with this, and anyway, it's simply
>> impossible to open a file in directory D if directory D doesn't provide
>> the user with search permission. So that looks like a must.
> 
> Indeed. Moreover, I was first surprised that even if was running qemu under
> root:root I got EACCES. Problem was, libvirt drops nearly all caps (even
> CAP_SYS_ADMIN) after fork and prior to executing qemu binary.
> 

I believe CAP_DAC_OVERRIDE is the capability.

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140911/78d1e265/attachment-0001.sig>