11:20 a.m.
This fixes the last issue preventing qemu:///system spice GL from working
out of the box: chown'ing the rendernode path so we have permissions
to open it.
We skip this if mount namespaces are disabled, so the chown'ing won't
interfere with other rendernode users on the host.
https://bugzilla.redhat.com/show_bug.cgi?id=1460804
v2:
Add the MOUNT_NAMESPACE handling
Drop DAC restore of rendernode
Cole Robinson (2):
security: add MANAGER_MOUNT_NAMESPACE flag
security: dac: relabel spice rendernode
src/qemu/qemu_driver.c | 2 ++
src/security/security_dac.c | 68 +++++++++++++++++++++++++++++++++++++++++
src/security/security_dac.h | 3 ++
src/security/security_manager.c | 4 ++-
src/security/security_manager.h | 1 +
5 files changed, 77 insertions(+), 1 deletion(-)
--
2.13.5
11:20 a.m.
New subject: [libvirt] [PATCH v2 1/2] security: add MANAGER_MOUNT_NAMESPACE flag
The VIR_SECURITY_MANAGER_MOUNT_NAMESPACE flag informs the DAC driver
if mount namespaces are in use for the VM. Will be used for future
changes.
Wire it up in the qemu driver
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_driver.c | 2 ++
src/security/security_dac.c | 10 ++++++++++
src/security/security_dac.h | 3 +++
src/security/security_manager.c | 4 +++-
src/security/security_manager.h | 1 +
5 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2ba6c80c4..ea1a85b41 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -419,6 +419,8 @@ qemuSecurityInit(virQEMUDriverPtr driver)
if (virQEMUDriverIsPrivileged(driver)) {
if (cfg->dynamicOwnership)
flags |= VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP;
+ if (virBitmapIsBitSet(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT))
+ flags |= VIR_SECURITY_MANAGER_MOUNT_NAMESPACE;
if (!(mgr = qemuSecurityNewDAC(QEMU_DRIVER_NAME,
cfg->user,
cfg->group,
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index ca7a6af6d..507be44a2 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -57,6 +57,7 @@ struct _virSecurityDACData {
gid_t *groups;
int ngroups;
bool dynamicOwnership;
+ bool mountNamespace;
char *baselabel;
virSecurityManagerDACChownCallback chownCallback;
};
@@ -238,6 +239,15 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
}
void
+virSecurityDACSetMountNamespace(virSecurityManagerPtr mgr,
+ bool mountNamespace)
+{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ priv->mountNamespace = mountNamespace;
+}
+
+
+void
virSecurityDACSetChownCallback(virSecurityManagerPtr mgr,
virSecurityManagerDACChownCallback chownCallback)
{
diff --git a/src/security/security_dac.h b/src/security/security_dac.h
index 846cefbb5..97681c961 100644
--- a/src/security/security_dac.h
+++ b/src/security/security_dac.h
@@ -32,6 +32,9 @@ int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
bool dynamic);
+void virSecurityDACSetMountNamespace(virSecurityManagerPtr mgr,
+ bool mountNamespace);
+
void virSecurityDACSetChownCallback(virSecurityManagerPtr mgr,
virSecurityManagerDACChownCallback chownCallback);
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 95b995230..e43c99d4f 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -146,7 +146,8 @@ virSecurityManagerNewDAC(const char *virtDriver,
virSecurityManagerPtr mgr;
virCheckFlags(VIR_SECURITY_MANAGER_NEW_MASK |
- VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP, NULL);
+ VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP |
+ VIR_SECURITY_MANAGER_MOUNT_NAMESPACE, NULL);
mgr = virSecurityManagerNewDriver(&virSecurityDriverDAC,
virtDriver,
@@ -161,6 +162,7 @@ virSecurityManagerNewDAC(const char *virtDriver,
}
virSecurityDACSetDynamicOwnership(mgr, flags &
VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP);
+ virSecurityDACSetMountNamespace(mgr, flags &
VIR_SECURITY_MANAGER_MOUNT_NAMESPACE);
virSecurityDACSetChownCallback(mgr, chownCallback);
return mgr;
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 01296d339..08fb89203 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -36,6 +36,7 @@ typedef enum {
VIR_SECURITY_MANAGER_REQUIRE_CONFINED = 1 << 2,
VIR_SECURITY_MANAGER_PRIVILEGED = 1 << 3,
VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP = 1 << 4,
+ VIR_SECURITY_MANAGER_MOUNT_NAMESPACE = 1 << 5,
} virSecurityManagerNewFlags;
# define VIR_SECURITY_MANAGER_NEW_MASK \
--
2.13.5
11:20 a.m.
New subject: [libvirt] [PATCH v2 2/2] security: dac: relabel spice rendernode
For a logged in user this a path like /dev/dri/renderD128 will have
default ownership root:video which won't work for the qemu:qemu user,
so we need to chown it.
We only do this when mount namespaces are enabled in the qemu driver,
so the chown'ing doesn't interfere with other users of the shared
render node path
https://bugzilla.redhat.com/show_bug.cgi?id=1460804
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
The restore bit is also motivated by a bug I hit when testing this:
DAC /dev/* permissions are 'restored' to root:root even with mount
namespaces enabled:
https://bugzilla.redhat.com/show_bug.cgi?id=1485719
src/security/security_dac.c | 58 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 507be44a2..349dbe81d 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1381,6 +1381,54 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr,
static int
+virSecurityDACSetGraphicsLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainGraphicsDefPtr gfx)
+
+{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityLabelDefPtr seclabel;
+ uid_t user;
+ gid_t group;
+
+ /* Skip chowning the shared render file if namespaces are disabled */
+ if (!priv->mountNamespace)
+ return 0;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+ if (seclabel && !seclabel->relabel)
+ return 0;
+
+ if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0)
+ return -1;
+
+ if (gfx->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
+ gfx->data.spice.gl == VIR_TRISTATE_BOOL_YES &&
+ gfx->data.spice.rendernode) {
+ if (virSecurityDACSetOwnership(priv, NULL,
+ gfx->data.spice.rendernode,
+ user, group) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+
+static int
+virSecurityDACRestoreGraphicsLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+ virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainGraphicsDefPtr gfx ATTRIBUTE_UNUSED)
+
+{
+ /* The only graphics labelling we do is dependent on mountNamespaces,
+ in which case 'restoring' the label doesn't actually accomplish
+ anything, so there's nothing to do here */
+ return 0;
+}
+
+
+static int
virSecurityDACSetInputLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virDomainInputDefPtr input)
@@ -1491,6 +1539,11 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
rc = -1;
}
+ for (i = 0; i < def->ngraphics; i++) {
+ if (virSecurityDACRestoreGraphicsLabel(mgr, def, def->graphics[i]) < 0)
+ return -1;
+ }
+
for (i = 0; i < def->ninputs; i++) {
if (virSecurityDACRestoreInputLabel(mgr, def, def->inputs[i]) < 0)
rc = -1;
@@ -1611,6 +1664,11 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
return -1;
}
+ for (i = 0; i < def->ngraphics; i++) {
+ if (virSecurityDACSetGraphicsLabel(mgr, def, def->graphics[i]) < 0)
+ return -1;
+ }
+
for (i = 0; i < def->ninputs; i++) {
if (virSecurityDACSetInputLabel(mgr, def, def->inputs[i]) < 0)
return -1;
--
2.13.5
7:37 a.m.
On Sun, 2017-08-27 at 12:20 -0400, Cole Robinson wrote:
This fixes the last issue preventing qemu:///system spice GL from
working
out of the box: chown'ing the rendernode path so we have permissions
to open it.
We skip this if mount namespaces are disabled, so the chown'ing won't
interfere with other rendernode users on the host.
https://bugzilla.redhat.com/show_bug.cgi?id=1460804
v2:
Add the MOUNT_NAMESPACE handling
Drop DAC restore of rendernode
Cole Robinson (2):
security: add MANAGER_MOUNT_NAMESPACE flag
security: dac: relabel spice rendernode
src/qemu/qemu_driver.c | 2 ++
src/security/security_dac.c | 68 +++++++++++++++++++++++++++++++++++++++++
src/security/security_dac.h | 3 ++
src/security/security_manager.c | 4 ++-
src/security/security_manager.h | 1 +
5 files changed, 77 insertions(+), 1 deletion(-)
Looks reasonable and works as expected on my Fedora 26
installation, so for the entire series:
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
You should document this in the release notes, though :)
--
Andrea Bolognani / Red Hat / Virtualization
2636
days inactive
2644
days old
3 comments
2 participants
participants (2)
-
Andrea Bolognani -
Cole Robinson