On 12.09.2014 00:20, Laszlo Ersek wrote:
On 09/11/14 16:25, 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@xxxxxxxxxx>
---
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.
Regarding the seclabel / context, I agree that it should have a label
consistent with other disk image files; for qemu it's just a -drive
after all. The hunk in question looks consistent with the rest of
"src/security/security_selinux.c".
Acked-by: Laszlo Ersek <lersek@xxxxxxxxxx>
Thanks, applied.
... Unfortunately the patch is insufficient. Commit 742b08e3 added directory
%{_localstatedir}/lib/libvirt/qemu/nvram/
to two sub-packages:
%files daemon
%files daemon-driver-qemu
I assume that was a correct thing to do (it is consistent with the rest
of the directories being listed for both subpackages).
However, this recent patch (commit 37d8c75f) changes the 0750 permission
bits to 0711 only in the "daemon" subpackage, and the directory in the
daemon-driver-qemu subpackage remains with 0750. In my testing, the
daemon package's modification doesn't take effect at all. In fact the
daemon RPM doesn't even seem to contain the nvram directory.
Ah, thanks for catching that. I'm pushing the obvious trivial fix. Thanks!
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list