From 28fdfd20f2699f51d5bcfe97dfc4deddca6d7e6e Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Fri, 3 Apr 2020 10:28:17 +0200 Subject: [PATCH] qemu: Label restore path outside of secdriver transactions As explained in the previous commit, we need to relabel the file we are restoring the domain from. That is the FD that is passed to QEMU. If the file is not under /dev then the file inside the namespace is the very same as the one in the host. And regardless of using transactions, the file will be relabeled. But, if the file is under /dev then when using transactions only the copy inside the namespace is relabeled and the one in the host is not. But QEMU is reading from the one in the host, actually. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1772838 Signed-off-by: Michal Privoznik Reviewed-by: Erik Skultety --- src/qemu/qemu_security.c | 7 +++++++ src/security/security_selinux.c | 23 +++++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 484fc34552..61b9e4f0e3 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -39,6 +39,13 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; pid_t pid = -1; + /* Explicitly run this outside of transaction. We really want to relabel + * the file in the host and not in the domain's namespace. */ + if (virSecurityManagerDomainSetPathLabelRO(driver->securityManager, + vm->def, + stdin_path) < 0) + goto cleanup; + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) pid = vm->pid; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 8aeb6e45a5..281c303296 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -3135,7 +3135,7 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr def, static int virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, - const char *stdin_path, + const char *stdin_path G_GNUC_UNUSED, bool chardevStdioLogd, bool migrated G_GNUC_UNUSED) { @@ -3231,11 +3231,6 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr, data->content_context, true) < 0) return -1; - if (stdin_path && - virSecuritySELinuxSetFilecon(mgr, stdin_path, - data->content_context, true) < 0) - return -1; - return 0; } @@ -3393,6 +3388,21 @@ virSecuritySELinuxDomainSetPathLabel(virSecurityManagerPtr mgr, return virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel, true); } +static int +virSecuritySELinuxDomainSetPathLabelRO(virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path) +{ + virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + + if (!path || !secdef || !secdef->relabel || data->skipAllLabel) + return 0; + + return virSecuritySELinuxSetFilecon(mgr, path, data->content_context, false); +} /* * virSecuritySELinuxSetFileLabels: @@ -3596,6 +3606,7 @@ virSecurityDriver virSecurityDriverSELinux = { .getBaseLabel = virSecuritySELinuxGetBaseLabel, .domainSetPathLabel = virSecuritySELinuxDomainSetPathLabel, + .domainSetPathLabelRO = virSecuritySELinuxDomainSetPathLabelRO, .domainSetSecurityChardevLabel = virSecuritySELinuxSetChardevLabel, .domainRestoreSecurityChardevLabel = virSecuritySELinuxRestoreChardevLabel, -- 2.47.2