External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-12 12:55:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

qemu: Set label on vhostuser net device when hotplugging

Browse Source 
Attaching a newly created vhostuser port to a VM fails due to an
apparmor denial

internal error: unable to execute QEMU command 'chardev-add': Failed
to bind socket to /run/openvswitch/vhu838c4d29-c9: Permission denied

In the case of a net device type VIR_DOMAIN_NET_TYPE_VHOSTUSER, the
underlying chardev is not labeled in qemuDomainAttachNetDevice prior
to calling qemuMonitorAttachCharDev.

A simple fix would be to call qemuSecuritySetChardevLabel using the
embedded virDomainChrSourceDef in the virDomainNetDef vhostuser data,
but this incurs the risk of incorrectly restoring the label. E.g.
consider the DAC driver behavior with a vhostuser net device, which
uses a socket for the chardev backend. The DAC driver uses XATTRS to
store original labelling information, but XATTRS are not compatible
with sockets. Without the original labelling information, the socket
labels will be restored with root ownership, preventing other
less-privileged processes from connecting to the socket.

This patch avoids overloading chardev labelling with vhostuser net
devices by introducing virSecurityManager{Set,Restore}NetdevLabel,
which is currently only implemented for the apparmor driver. The
new APIs are then used to set and restore labels for the vhostuser
net devices.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Jim Fehlig 2021-07-27 18:13:36 -06:00
parent 3796e8e1ff
commit 2e8ebfe3fa
9 changed files with 250 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/libvirt_private.syms
View File

@ -1688,6 +1688,7 @@ virSecurityManagerRestoreHostdevLabel;
virSecurityManagerRestoreImageLabel;
virSecurityManagerRestoreInputLabel;
virSecurityManagerRestoreMemoryLabel;
virSecurityManagerRestoreNetdevLabel;
virSecurityManagerRestoreSavedStateLabel;
virSecurityManagerRestoreTPMLabels;
virSecurityManagerSetAllLabel;
@ -1699,6 +1700,7 @@ virSecurityManagerSetImageFDLabel;
virSecurityManagerSetImageLabel;
virSecurityManagerSetInputLabel;
virSecurityManagerSetMemoryLabel;
virSecurityManagerSetNetdevLabel;
virSecurityManagerSetProcessLabel;
virSecurityManagerSetSavedStateLabel;
virSecurityManagerSetSocketLabel;

13
src/qemu/qemu_hotplug.c
View File

@ -1203,6 +1203,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
g_autofree char *netdev_name = NULL;
g_autoptr(virConnect) conn = NULL;
virErrorPtr save_err = NULL;
bool teardownlabel = false;
/* If appropriate, grab a physical device from the configured
* network's pool of devices, or resolve bridge device name
@ -1343,6 +1344,9 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
&net->ifname) < 0)
goto cleanup;
if (qemuSecuritySetNetdevLabel(driver, vm, net) < 0)
goto cleanup;
teardownlabel = true;
break;
case VIR_DOMAIN_NET_TYPE_USER:
@ -1559,6 +1563,10 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
qemuDomainNetDeviceVportRemove(net);
}
if (teardownlabel &&
qemuSecurityRestoreNetdevLabel(driver, vm, net) < 0)
VIR_WARN("Unable to restore network device labelling on hotplug fail");
/* we had potentially pre-added the device to the domain
* device lists, if so we need to remove it (from def->nets
* and/or def->hostdevs) on failure
@ -4803,6 +4811,11 @@ qemuDomainRemoveNetDevice(virQEMUDriver *driver,
cfg->stateDir));
}
if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
if (qemuSecurityRestoreNetdevLabel(driver, vm, net) < 0)
VIR_WARN("Unable to restore security label on vhostuser char device");
}
qemuDomainNetDeviceVportRemove(net);
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {

59
src/qemu/qemu_security.c
View File

@ -439,6 +439,65 @@ qemuSecurityRestoreChardevLabel(virQEMUDriver *driver,
return ret;
}
int
qemuSecuritySetNetdevLabel(virQEMUDriver *driver,
virDomainObj *vm,
virDomainNetDef *net)
{
int ret = -1;
qemuDomainObjPrivate *priv = vm->privateData;
pid_t pid = -1;
if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
pid = vm->pid;
if (virSecurityManagerTransactionStart(driver->securityManager) < 0)
goto cleanup;
if (virSecurityManagerSetNetdevLabel(driver->securityManager,
vm->def, net) < 0)
goto cleanup;
if (virSecurityManagerTransactionCommit(driver->securityManager,
pid, priv->rememberOwner) < 0)
goto cleanup;
ret = 0;
cleanup:
virSecurityManagerTransactionAbort(driver->securityManager);
return ret;
}
int
qemuSecurityRestoreNetdevLabel(virQEMUDriver *driver,
virDomainObj *vm,
virDomainNetDef *net)
{
int ret = -1;
qemuDomainObjPrivate *priv = vm->privateData;
pid_t pid = -1;
if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
pid = vm->pid;
if (virSecurityManagerTransactionStart(driver->securityManager) < 0)
goto cleanup;
if (virSecurityManagerRestoreNetdevLabel(driver->securityManager,
vm->def, net) < 0)
goto cleanup;
if (virSecurityManagerTransactionCommit(driver->securityManager,
pid, priv->rememberOwner) < 0)
goto cleanup;
ret = 0;
cleanup:
virSecurityManagerTransactionAbort(driver->securityManager);
return ret;
}
/*
* qemuSecurityStartVhostUserGPU:

8
src/qemu/qemu_security.h
View File

@ -79,6 +79,14 @@ int qemuSecurityRestoreChardevLabel(virQEMUDriver *driver,
virDomainObj *vm,
virDomainChrDef *chr);
int qemuSecuritySetNetdevLabel(virQEMUDriver *driver,
virDomainObj *vm,
virDomainNetDef *net);
int qemuSecurityRestoreNetdevLabel(virQEMUDriver *driver,
virDomainObj *vm,
virDomainNetDef *net);
int qemuSecurityStartVhostUserGPU(virQEMUDriver *driver,
virDomainObj *vm,
virCommand *cmd,

61
src/security/security_apparmor.c
View File

@ -1053,6 +1053,64 @@ AppArmorRestoreChardevLabel(virSecurityManager *mgr,
return reload_profile(mgr, def, NULL, false);
}
static int
AppArmorSetNetdevLabel(virSecurityManager *mgr,
virDomainDef *def,
virDomainNetDef *net)
{
int ret = -1;
virSecurityLabelDef *secdef;
virDomainChrSourceDef *dev_source;
virDomainNetType actualType;
secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
if (!secdef)
return 0;
actualType = virDomainNetGetActualType(net);
if (actualType != VIR_DOMAIN_NET_TYPE_VHOSTUSER)
return 0;
dev_source = net->data.vhostuser;
switch ((virDomainChrType)dev_source->type) {
case VIR_DOMAIN_CHR_TYPE_UNIX:
ret = reload_profile(mgr, def, dev_source->data.file.path, true);
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
case VIR_DOMAIN_CHR_TYPE_PTY:
case VIR_DOMAIN_CHR_TYPE_PIPE:
case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
case VIR_DOMAIN_CHR_TYPE_NULL:
case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_STDIO:
case VIR_DOMAIN_CHR_TYPE_UDP:
case VIR_DOMAIN_CHR_TYPE_TCP:
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
case VIR_DOMAIN_CHR_TYPE_NMDM:
case VIR_DOMAIN_CHR_TYPE_LAST:
ret = 0;
break;
}
return ret;
}
static int
AppArmorRestoreNetdevLabel(virSecurityManager *mgr,
virDomainDef *def,
virDomainNetDef *net G_GNUC_UNUSED)
{
virSecurityLabelDef *secdef;
secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
if (!secdef)
return 0;
return reload_profile(mgr, def, NULL, false);
}
static int
AppArmorSetPathLabel(virSecurityManager *mgr,
virDomainDef *def,
@ -1168,6 +1226,9 @@ virSecurityDriver virAppArmorSecurityDriver = {
.domainSetSecurityChardevLabel = AppArmorSetChardevLabel,
.domainRestoreSecurityChardevLabel = AppArmorRestoreChardevLabel,
.domainSetSecurityNetdevLabel = AppArmorSetNetdevLabel,
.domainRestoreSecurityNetdevLabel = AppArmorRestoreNetdevLabel,
.domainSetSecurityImageFDLabel = AppArmorSetFDLabel,
.domainSetSecurityTapFDLabel = AppArmorSetFDLabel,

9
src/security/security_driver.h
View File

@ -157,6 +157,12 @@ typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManager *mgr,
virDomainDef *def);
typedef int (*virSecurityDomainRestoreTPMLabels) (virSecurityManager *mgr,
virDomainDef *def);
typedef int (*virSecurityDomainSetNetdevLabel) (virSecurityManager *mgr,
virDomainDef *def,
virDomainNetDef *net);
typedef int (*virSecurityDomainRestoreNetdevLabel) (virSecurityManager *mgr,
virDomainDef *def,
virDomainNetDef *net);
struct _virSecurityDriver {
@ -224,6 +230,9 @@ struct _virSecurityDriver {
virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels;
virSecurityDomainRestoreTPMLabels domainRestoreSecurityTPMLabels;
virSecurityDomainSetNetdevLabel domainSetSecurityNetdevLabel;
virSecurityDomainRestoreNetdevLabel domainRestoreSecurityNetdevLabel;
};
virSecurityDriver *virSecurityDriverLookup(const char *name,

38
src/security/security_manager.c
View File

@ -1297,6 +1297,44 @@ virSecurityManagerRestoreTPMLabels(virSecurityManager *mgr,
}
int
virSecurityManagerSetNetdevLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainNetDef *net)
{
int ret;
if (mgr->drv->domainSetSecurityNetdevLabel) {
virObjectLock(mgr);
ret = mgr->drv->domainSetSecurityNetdevLabel(mgr, vm, net);
virObjectUnlock(mgr);
return ret;
}
return 0;
}
int
virSecurityManagerRestoreNetdevLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainNetDef *net)
{
int ret;
if (mgr->drv->domainRestoreSecurityNetdevLabel) {
virObjectLock(mgr);
ret = mgr->drv->domainRestoreSecurityNetdevLabel(mgr, vm, net);
virObjectUnlock(mgr);
return ret;
}
return 0;
}
static int
cmpstringp(const void *p1, const void *p2)
{

8
src/security/security_manager.h
View File

@ -220,6 +220,14 @@ int virSecurityManagerSetTPMLabels(virSecurityManager *mgr,
int virSecurityManagerRestoreTPMLabels(virSecurityManager *mgr,
virDomainDef *vm);
int virSecurityManagerSetNetdevLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainNetDef *net);
int virSecurityManagerRestoreNetdevLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainNetDef *net);
typedef struct _virSecurityManagerMetadataLockState virSecurityManagerMetadataLockState;
struct _virSecurityManagerMetadataLockState {
size_t nfds; /* Captures size of both @fds and @paths */

52
src/security/security_stack.c
View File

@ -963,6 +963,55 @@ virSecurityStackRestoreTPMLabels(virSecurityManager *mgr,
}
static int
virSecurityStackDomainSetNetdevLabel(virSecurityManager *mgr,
virDomainDef *def,
virDomainNetDef *net)
{
virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr);
virSecurityStackItem *item = priv->itemsHead;
for (; item; item = item->next) {
if (virSecurityManagerSetNetdevLabel(item->securityManager, def, net) < 0)
goto rollback;
}
return 0;
rollback:
for (item = item->prev; item; item = item->prev) {
if (virSecurityManagerRestoreNetdevLabel(item->securityManager,
def, net) < 0) {
VIR_WARN("Unable to restore netdev label after failed set label "
"call virDriver=%s driver=%s domain=%s",
virSecurityManagerGetVirtDriver(mgr),
virSecurityManagerGetDriver(item->securityManager),
def->name);
}
}
return -1;
}
static int
virSecurityStackDomainRestoreNetdevLabel(virSecurityManager *mgr,
virDomainDef *def,
virDomainNetDef *net)
{
virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr);
virSecurityStackItem *item = priv->itemsHead;
int rc = 0;
for (; item; item = item->next) {
if (virSecurityManagerRestoreNetdevLabel(item->securityManager,
def, net) < 0)
rc = -1;
}
return rc;
}
virSecurityDriver virSecurityDriverStack = {
.privateDataLen = sizeof(virSecurityStackData),
.name = "stack",
@ -1028,4 +1077,7 @@ virSecurityDriver virSecurityDriverStack = {
.domainSetSecurityTPMLabels = virSecurityStackSetTPMLabels,
.domainRestoreSecurityTPMLabels = virSecurityStackRestoreTPMLabels,
.domainSetSecurityNetdevLabel = virSecurityStackDomainSetNetdevLabel,
.domainRestoreSecurityNetdevLabel = virSecurityStackDomainRestoreNetdevLabel,
};