On Fri, Apr 03, 2020 at 05:58:02PM +0200, Michal Privoznik wrote:
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).
Since this patch is only adding a generic driver API rather than a specific
security backend API, I'd move ^this last paragraph to the next patch, I
actually appreciated the info there much more than here during the review.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
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 e276f55bb1..2c63f37fc2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1523,6 +1523,7 @@ virSecurityDriverLookup;
# security/security_manager.h
virSecurityManagerCheckAllLabel;
virSecurityManagerClearSocketLabel;
+virSecurityManagerDomainSetIncomingPathLabel;
IncomingPath sounds a bit confusing to me, I'd go with something like HostPath
instead, in patch 3/3 you also have an explicit commentary why we're using the
respective backend API for this rather than simply using SetPathLabel.
virSecurityManagerDomainSetPathLabel;
virSecurityManagerGenLabel;
virSecurityManagerGetBaseLabel;
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 3353955813..6cbe8613c9 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 (*virSecurityDomainSetIncomingPathLabel) (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;
+ virSecurityDomainSetIncomingPathLabel domainSetIncomingPathLabel;
virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel;
virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index fe032746ff..a76b953ee4 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -1077,6 +1077,35 @@ virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr,
}
+/**
+ * virSecurityManagerDomainSetIncomingPathLabel:
+ * @mgr: security manager object
+ * @vm: domain definition object
+ * @path: path to label
+ *
+ * This function relabels given @path so that @vm can restore for
maybe add "host" @path here to make it even clearer.
+ * it. This allows the driver backend to use different label than
+ * virSecurityManagerDomainSetPathLabel().
+ *
+ * Returns: 0 on success, -1 on error.
+ */
+int
+virSecurityManagerDomainSetIncomingPathLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ const char *path)
+{
+ if (mgr->drv->domainSetIncomingPathLabel) {
+ int ret;
+ virObjectLock(mgr);
+ ret = mgr->drv->domainSetIncomingPathLabel(mgr, vm, path);
+ virObjectUnlock(mgr);
+ return ret;
+ }
+
+ return 0;
+}
Looks good and works:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>