On Fri, Jan 13, 2017 at 11:12:43AM +0100, Michal Privoznik wrote:
When creating new /dev/* for qemu, we do chown() and copy ACLs to
create the exact copy from the original /dev. I though that
copying SELinux labels is not necessary as SELinux will chose the
sane defaults. Surprisingly, it does not leaving namespace with
the following labels:
crw-rw-rw-. root root system_u:object_r:tmpfs_t:s0 random
crw-------. root root system_u:object_r:tmpfs_t:s0 rtc0
drwxrwxrwt. root root system_u:object_r:tmpfs_t:s0 shm
crw-rw-rw-. root root system_u:object_r:tmpfs_t:s0 urandom
As a result, domain is unable to start:
error: internal error: process exited while connecting to monitor: Error in GnuTLS initialization: Failed to acquire random data.
qemu-kvm: cannot initialize crypto: Unable to initialize GNUTLS library: Failed to acquire random data.
The solution is to copy the SELinux labels as well.
Reported-by: Andrea Bolognani <abologna@xxxxxxxxxx>
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/qemu/qemu_domain.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1399dee0d..a29866673 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -63,6 +63,9 @@
#if defined(HAVE_SYS_MOUNT_H)
# include <sys/mount.h>
#endif
+#ifdef WITH_SELINUX
+# include <selinux/selinux.h>
+#endif
#include <libxml/xpathInternals.h>
@@ -6958,6 +6961,9 @@ qemuDomainCreateDevice(const char *device,
char *canonDevicePath = NULL;
struct stat sb;
int ret = -1;
+#ifdef WITH_SELINUX
+ char *tcon = NULL;
+#endif
if (virFileResolveAllLinks(device, &canonDevicePath) < 0) {
if (errno == ENOENT && allow_noent) {
@@ -7023,10 +7029,34 @@ qemuDomainCreateDevice(const char *device,
goto cleanup;
}
+#ifdef WITH_SELINUX
+ if (getfilecon_raw(canonDevicePath, &tcon) < 0 &&
+ (errno != ENOTSUP && errno != ENODATA)) {
+ virReportSystemError(errno,
+ _("Unable to get SELinux label on %s"), canonDevicePath);
+ goto cleanup;
+ }
+
+ if (tcon &&
+ setfilecon_raw(devicePath, (VIR_SELINUX_CTX_CONST char *) tcon) < 0) {
+ VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
+ if (errno != EOPNOTSUPP && errno != ENOTSUP) {
+ VIR_WARNINGS_RESET
+ virReportSystemError(errno,
+ _("Unable to set SELinux label on %s"),
+ devicePath);
+ goto cleanup;
+ }
+ }
+#endif
+
I think, instead of all the ifdefs, this should be a security driver API instead of being hardcoded in places. That way it will be processed properly by all the security drivers. For the purpose of this release, let's say I noticed this patch right after the release and hence didn't have time to NACK it properly on time. Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
