From 55cbb94e2e033766a4132cf7a8f16e4966166bef Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Fri, 3 Apr 2020 14:31:35 +0200 Subject: [PATCH] security: Introduce virSecurityManagerDomainSetPathLabelRO This API allows drivers to separate out handling of @stdin_path of virSecurityManagerSetAllLabel(). The thing is, the QEMU driver uses transactions for virSecurityManagerSetAllLabel() which relabels devices from inside of domain's namespace. This is what we usually want. Except when resuming domain from a file. The file is opened before any namespace is set up and the FD is passed to QEMU to read the migration stream from. Because of this, the file lives outside of the namespace and if it so happens that the file is a block device (i.e. it lives under /dev) its copy will be created in the namespace. But the FD that is passed to QEMU points to the original living in the host and not in the namespace. So relabeling the file inside the namespace helps nothing. But if we have a separate API for relabeling the restore file then the QEMU driver can continue calling virSecurityManagerSetAllLabel() with transactions enabled and call this new API without transactions. We already have an API for relabeling a single file (virSecurityManagerDomainSetPathLabel()) but in case of SELinux it uses @imagelabel (which allows RW access) and we want to use @content_context (which allows RO access). Signed-off-by: Michal Privoznik Reviewed-by: Erik Skultety --- src/libvirt_private.syms | 1 + src/security/security_driver.h | 4 ++++ src/security/security_manager.c | 29 +++++++++++++++++++++++++++++ src/security/security_manager.h | 4 ++++ src/security/security_stack.c | 21 +++++++++++++++++++++ 5 files changed, 59 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ec367653d5..182a570382 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1525,6 +1525,7 @@ virSecurityDriverLookup; virSecurityManagerCheckAllLabel; virSecurityManagerClearSocketLabel; virSecurityManagerDomainSetPathLabel; +virSecurityManagerDomainSetPathLabelRO; virSecurityManagerGenLabel; virSecurityManagerGetBaseLabel; virSecurityManagerGetDOI; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 3353955813..d23b64668d 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -140,6 +140,9 @@ typedef int (*virSecurityDomainSetPathLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, const char *path, bool allowSubtree); +typedef int (*virSecurityDomainSetPathLabelRO) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path); typedef int (*virSecurityDomainSetChardevLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainChrSourceDefPtr dev_source, @@ -211,6 +214,7 @@ struct _virSecurityDriver { virSecurityDriverGetBaseLabel getBaseLabel; virSecurityDomainSetPathLabel domainSetPathLabel; + virSecurityDomainSetPathLabelRO domainSetPathLabelRO; virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel; virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index fe032746ff..6bf297470c 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1077,6 +1077,35 @@ virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr, } +/** + * virSecurityManagerDomainSetPathLabelRO: + * @mgr: security manager object + * @vm: domain definition object + * @path: path to label + * + * This function relabels given @path for read only access, which + * is in contrast with virSecurityManagerDomainSetPathLabel() which + * gives read write access. + * + * Returns: 0 on success, -1 on error. + */ +int +virSecurityManagerDomainSetPathLabelRO(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path) +{ + if (mgr->drv->domainSetPathLabelRO) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetPathLabelRO(mgr, vm, path); + virObjectUnlock(mgr); + return ret; + } + + return 0; +} + + /** * virSecurityManagerSetMemoryLabel: * @mgr: security manager object diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 7699bcbc6f..2c5fa3ee15 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -189,6 +189,10 @@ int virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr, const char *path, bool allowSubtree); +int virSecurityManagerDomainSetPathLabelRO(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path); + int virSecurityManagerSetChardevLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainChrSourceDefPtr dev_source, diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 073876daff..165303a1f8 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -825,6 +825,26 @@ virSecurityStackDomainSetPathLabel(virSecurityManagerPtr mgr, return rc; } + +static int +virSecurityStackDomainSetPathLabelRO(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerDomainSetPathLabelRO(item->securityManager, + vm, path) < 0) + rc = -1; + } + + return rc; +} + + static int virSecurityStackDomainSetChardevLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, @@ -985,6 +1005,7 @@ virSecurityDriver virSecurityDriverStack = { .getBaseLabel = virSecurityStackGetBaseLabel, .domainSetPathLabel = virSecurityStackDomainSetPathLabel, + .domainSetPathLabelRO = virSecurityStackDomainSetPathLabelRO, .domainSetSecurityChardevLabel = virSecurityStackDomainSetChardevLabel, .domainRestoreSecurityChardevLabel = virSecurityStackDomainRestoreChardevLabel,