6:54 a.m.
Pavel Hrdina (2):
security: introduce virSecurityManager(Set|Restore)ChardevLabel
qemu: fix security labeling for attach/detach of char devices
src/libvirt_private.syms | 2 ++
src/qemu/qemu_hotplug.c | 10 +++++++
src/qemu/qemu_security.c | 60 +++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_security.h | 8 ++++++
src/security/security_dac.c | 3 +++
src/security/security_driver.h | 11 ++++++++
src/security/security_manager.c | 40 +++++++++++++++++++++++++++
src/security/security_manager.h | 10 +++++++
src/security/security_nop.c | 20 ++++++++++++++
src/security/security_selinux.c | 3 +++
src/security/security_stack.c | 43 +++++++++++++++++++++++++++++
11 files changed, 210 insertions(+)
--
2.14.3
6:54 a.m.
New subject: [libvirt] [PATCH 1/2] security: introduce virSecurityManager(Set|Restore)ChardevLabel
SELinux and DAC drivers already have both functions but they were not
exported as public API of security manager.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/security/security_dac.c | 3 +++
src/security/security_driver.h | 11 +++++++++++
src/security/security_manager.c | 40 ++++++++++++++++++++++++++++++++++++++
src/security/security_manager.h | 10 ++++++++++
src/security/security_nop.c | 20 +++++++++++++++++++
src/security/security_selinux.c | 3 +++
src/security/security_stack.c | 43 +++++++++++++++++++++++++++++++++++++++++
8 files changed, 132 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 53057aa82e..de4ec4d442 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1276,6 +1276,7 @@ virSecurityManagerPreFork;
virSecurityManagerReleaseLabel;
virSecurityManagerReserveLabel;
virSecurityManagerRestoreAllLabel;
+virSecurityManagerRestoreChardevLabel;
virSecurityManagerRestoreDiskLabel;
virSecurityManagerRestoreHostdevLabel;
virSecurityManagerRestoreImageLabel;
@@ -1283,6 +1284,7 @@ virSecurityManagerRestoreInputLabel;
virSecurityManagerRestoreMemoryLabel;
virSecurityManagerRestoreSavedStateLabel;
virSecurityManagerSetAllLabel;
+virSecurityManagerSetChardevLabel;
virSecurityManagerSetChildProcessLabel;
virSecurityManagerSetDaemonSocketLabel;
virSecurityManagerSetDiskLabel;
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 52ca07a10f..609d2595b2 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -2155,4 +2155,7 @@ virSecurityDriver virSecurityDriverDAC = {
.getBaseLabel = virSecurityDACGetBaseLabel,
.domainSetPathLabel = virSecurityDACDomainSetPathLabel,
+
+ .domainSetSecurityChardevLabel = virSecurityDACSetChardevLabel,
+ .domainRestoreSecurityChardevLabel = virSecurityDACRestoreChardevLabel,
};
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 1b3070d06d..47dad8ba20 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -140,6 +140,14 @@ typedef int (*virSecurityDomainRestoreInputLabel)
(virSecurityManagerPtr mgr,
typedef int (*virSecurityDomainSetPathLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
const char *path);
+typedef int (*virSecurityDomainSetChardevLabel) (virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainChrSourceDefPtr dev_source,
+ bool chardevStdioLogd);
+typedef int (*virSecurityDomainRestoreChardevLabel) (virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainChrSourceDefPtr
dev_source,
+ bool chardevStdioLogd);
struct _virSecurityDriver {
@@ -201,6 +209,9 @@ struct _virSecurityDriver {
virSecurityDriverGetBaseLabel getBaseLabel;
virSecurityDomainSetPathLabel domainSetPathLabel;
+
+ virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel;
+ virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel;
};
virSecurityDriverPtr virSecurityDriverLookup(const char *name,
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 3cf12188a0..9249aba1fa 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -1152,3 +1152,43 @@ virSecurityManagerRestoreInputLabel(virSecurityManagerPtr mgr,
virReportUnsupportedError();
return -1;
}
+
+
+int
+virSecurityManagerSetChardevLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainChrSourceDefPtr dev_source,
+ bool chardevStdioLogd)
+{
+ if (mgr->drv->domainSetSecurityChardevLabel) {
+ int ret;
+ virObjectLock(mgr);
+ ret = mgr->drv->domainSetSecurityChardevLabel(mgr, def, dev_source,
+ chardevStdioLogd);
+ virObjectUnlock(mgr);
+ return ret;
+ }
+
+ virReportUnsupportedError();
+ return -1;
+}
+
+
+int
+virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainChrSourceDefPtr dev_source,
+ bool chardevStdioLogd)
+{
+ if (mgr->drv->domainRestoreSecurityChardevLabel) {
+ int ret;
+ virObjectLock(mgr);
+ ret = mgr->drv->domainRestoreSecurityChardevLabel(mgr, def, dev_source,
+ chardevStdioLogd);
+ virObjectUnlock(mgr);
+ return ret;
+ }
+
+ virReportUnsupportedError();
+ return -1;
+}
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 834c7f1593..013e3b9b18 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -184,4 +184,14 @@ int virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
const char *path);
+int virSecurityManagerSetChardevLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainChrSourceDefPtr dev_source,
+ bool chardevStdioLogd);
+
+int virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainChrSourceDefPtr dev_source,
+ bool chardevStdioLogd);
+
#endif /* VIR_SECURITY_MANAGER_H__ */
diff --git a/src/security/security_nop.c b/src/security/security_nop.c
index cfb032c686..ff739f8199 100644
--- a/src/security/security_nop.c
+++ b/src/security/security_nop.c
@@ -262,6 +262,23 @@ virSecurityDomainInputLabelNop(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
return 0;
}
+static int
+virSecurityDomainSetChardevLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+ virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainChrSourceDefPtr dev_source
ATTRIBUTE_UNUSED,
+ bool chardevStdioLogd ATTRIBUTE_UNUSED)
+{
+ return 0;
+}
+
+static int
+virSecurityDomainRestoreChardevLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+ virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainChrSourceDefPtr dev_source
ATTRIBUTE_UNUSED,
+ bool chardevStdioLogd ATTRIBUTE_UNUSED)
+{
+ return 0;
+}
virSecurityDriver virSecurityDriverNop = {
.privateDataLen = 0,
@@ -314,4 +331,7 @@ virSecurityDriver virSecurityDriverNop = {
.domainGetSecurityMountOptions = virSecurityDomainGetMountOptionsNop,
.getBaseLabel = virSecurityGetBaseLabel,
+
+ .domainSetSecurityChardevLabel = virSecurityDomainSetChardevLabelNop,
+ .domainRestoreSecurityChardevLabel = virSecurityDomainRestoreChardevLabelNop,
};
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index b677fbcda7..0815a02d18 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -3095,4 +3095,7 @@ virSecurityDriver virSecurityDriverSELinux = {
.getBaseLabel = virSecuritySELinuxGetBaseLabel,
.domainSetPathLabel = virSecuritySELinuxDomainSetPathLabel,
+
+ .domainSetSecurityChardevLabel = virSecuritySELinuxSetChardevLabel,
+ .domainRestoreSecurityChardevLabel = virSecuritySELinuxRestoreChardevLabel,
};
diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index cd916382b2..0375e7d89d 100644
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -719,6 +719,46 @@ virSecurityStackDomainSetPathLabel(virSecurityManagerPtr mgr,
return rc;
}
+static int
+virSecurityStackDomainSetChardevLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainChrSourceDefPtr dev_source,
+ bool chardevStdioLogd)
+{
+ virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityStackItemPtr item = priv->itemsHead;
+ int rc = 0;
+
+ for (; item; item = item->next) {
+ if (virSecurityManagerSetChardevLabel(item->securityManager,
+ def, dev_source,
+ chardevStdioLogd) < 0)
+ rc = -1;
+ }
+
+ return rc;
+}
+
+static int
+virSecurityStackDomainRestoreChardevLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainChrSourceDefPtr dev_source,
+ bool chardevStdioLogd)
+{
+ virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityStackItemPtr item = priv->itemsHead;
+ int rc = 0;
+
+ for (; item; item = item->next) {
+ if (virSecurityManagerRestoreChardevLabel(item->securityManager,
+ def, dev_source,
+ chardevStdioLogd) < 0)
+ rc = -1;
+ }
+
+ return rc;
+}
+
virSecurityDriver virSecurityDriverStack = {
.privateDataLen = sizeof(virSecurityStackData),
.name = "stack",
@@ -778,4 +818,7 @@ virSecurityDriver virSecurityDriverStack = {
.getBaseLabel = virSecurityStackGetBaseLabel,
.domainSetPathLabel = virSecurityStackDomainSetPathLabel,
+
+ .domainSetSecurityChardevLabel = virSecurityStackDomainSetChardevLabel,
+ .domainRestoreSecurityChardevLabel = virSecurityStackDomainRestoreChardevLabel,
};
--
2.14.3
6:54 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: fix security labeling for attach/detach of char devices
Commit e93d844b90 was not enough to fix the permission denied
issue. We need to apply security labels as well.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1465833
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 10 ++++++++
src/qemu/qemu_security.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_security.h | 8 +++++++
3 files changed, 78 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index cbc7af59b7..6ef28bf051 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1815,6 +1815,7 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
bool chardevAttached = false;
bool teardowncgroup = false;
bool teardowndevice = false;
+ bool teardownlabel = false;
char *tlsAlias = NULL;
char *secAlias = NULL;
bool need_release = false;
@@ -1835,6 +1836,10 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
goto cleanup;
teardowndevice = true;
+ if (qemuSecuritySetChardevLabel(driver, vm, chr) < 0)
+ goto cleanup;
+ teardownlabel = true;
+
if (qemuSetupChardevCgroup(vm, chr) < 0)
goto cleanup;
teardowncgroup = true;
@@ -1877,6 +1882,8 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
if (teardowncgroup && qemuTeardownChardevCgroup(vm, chr) < 0)
VIR_WARN("Unable to remove chr device cgroup ACL on hotplug
fail");
+ if (teardownlabel && qemuSecurityRestoreChardevLabel(driver, vm, chr)
< 0)
+ VIR_WARN("Unable to restore security label on char device");
if (teardowndevice && qemuDomainNamespaceTeardownChardev(driver, vm, chr)
< 0)
VIR_WARN("Unable to remove chr device from /dev");
}
@@ -4154,6 +4161,9 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
if (qemuTeardownChardevCgroup(vm, chr) < 0)
VIR_WARN("Failed to remove chr device cgroup ACL");
+ if (qemuSecurityRestoreChardevLabel(driver, vm, chr) < 0)
+ VIR_WARN("Unable to restore security label on char device");
+
if (qemuDomainNamespaceTeardownChardev(driver, vm, chr) < 0)
VIR_WARN("Unable to remove chr device from /dev");
diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index e7d2bbd5a3..2aced22d2d 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -364,3 +364,63 @@ qemuSecurityRestoreInputLabel(virDomainObjPtr vm,
virSecurityManagerTransactionAbort(driver->securityManager);
return ret;
}
+
+
+int
+qemuSecuritySetChardevLabel(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainChrDefPtr chr)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
+ virSecurityManagerTransactionStart(driver->securityManager) < 0)
+ goto cleanup;
+
+ if (virSecurityManagerSetChardevLabel(driver->securityManager,
+ vm->def,
+ chr->source,
+ priv->chardevStdioLogd) < 0)
+ goto cleanup;
+
+ if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
+ virSecurityManagerTransactionCommit(driver->securityManager,
+ vm->pid) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ virSecurityManagerTransactionAbort(driver->securityManager);
+ return ret;
+}
+
+
+int
+qemuSecurityRestoreChardevLabel(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainChrDefPtr chr)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
+ virSecurityManagerTransactionStart(driver->securityManager) < 0)
+ goto cleanup;
+
+ if (virSecurityManagerRestoreChardevLabel(driver->securityManager,
+ vm->def,
+ chr->source,
+ priv->chardevStdioLogd) < 0)
+ goto cleanup;
+
+ if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
+ virSecurityManagerTransactionCommit(driver->securityManager,
+ vm->pid) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ virSecurityManagerTransactionAbort(driver->securityManager);
+ return ret;
+}
diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h
index 76d63f06ec..d54ce6fead 100644
--- a/src/qemu/qemu_security.h
+++ b/src/qemu/qemu_security.h
@@ -76,6 +76,14 @@ int qemuSecuritySetInputLabel(virDomainObjPtr vm,
int qemuSecurityRestoreInputLabel(virDomainObjPtr vm,
virDomainInputDefPtr input);
+int qemuSecuritySetChardevLabel(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainChrDefPtr chr);
+
+int qemuSecurityRestoreChardevLabel(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainChrDefPtr chr);
+
/* Please note that for these APIs there is no wrapper yet. Do NOT blindly add
* new APIs here. If an API can touch a /dev file add a proper wrapper instead.
*/
--
2.14.3
11:47 a.m.
On Fri, Dec 01, 2017 at 01:54:30PM +0100, Pavel Hrdina wrote:
Pavel Hrdina (2):
security: introduce virSecurityManager(Set|Restore)ChardevLabel
qemu: fix security labeling for attach/detach of char devices
src/libvirt_private.syms | 2 ++
src/qemu/qemu_hotplug.c | 10 +++++++
src/qemu/qemu_security.c | 60 +++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_security.h | 8 ++++++
src/security/security_dac.c | 3 +++
src/security/security_driver.h | 11 ++++++++
src/security/security_manager.c | 40 +++++++++++++++++++++++++++
src/security/security_manager.h | 10 +++++++
src/security/security_nop.c | 20 ++++++++++++++
src/security/security_selinux.c | 3 +++
src/security/security_stack.c | 43 +++++++++++++++++++++++++++++
11 files changed, 210 insertions(+)
ACK series
I will refrain from commenting on whether these are bug fixes or style
cleanups.
Jan
2579
days inactive
2579
days old
3 comments
2 participants
participants (2)
-
Ján Tomko -
Pavel Hrdina