From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> Insert calls to the ACL checking APIs in all interface driver entrypoints. Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> --- src/Makefile.am | 5 +- src/interface/interface_backend_netcf.c | 115 ++++++++++++++++++++++++++++++++ src/interface/interface_backend_udev.c | 85 ++++++++++++++++++++--- 3 files changed, 196 insertions(+), 9 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index a76c27e..8e60612 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1248,7 +1248,10 @@ noinst_LTLIBRARIES += libvirt_driver_interface.la # Stateful, so linked to daemon instead #libvirt_la_BUILT_LIBADD += libvirt_driver_interface.la endif -libvirt_driver_interface_la_CFLAGS = -I$(top_srcdir)/src/conf $(AM_CFLAGS) +libvirt_driver_interface_la_CFLAGS = \ + -I$(top_srcdir)/src/access \ + -I$(top_srcdir)/src/conf \ + $(AM_CFLAGS) libvirt_driver_interface_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_interface_la_LIBADD = if WITH_NETCF diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index d626017..a995816 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -31,6 +31,8 @@ #include "interface_conf.h" #include "viralloc.h" #include "virlog.h" +#include "virstring.h" +#include "viraccessapicheck.h" #define VIR_FROM_THIS VIR_FROM_INTERFACE @@ -52,6 +54,36 @@ static void interfaceDriverUnlock(struct interface_driver *driver) virMutexUnlock(&driver->lock); } +/* + * Get a minimal virInterfaceDef containing enough metadata + * for access control checks to be performed. Currently + * this implies existance of name and mac address attributes + */ +static virInterfaceDef * ATTRIBUTE_NONNULL(1) +netcfGetMinimalDefForDevice(struct netcf_if *iface) +{ + virInterfaceDef *def; + + /* Allocate our interface definition structure */ + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (VIR_STRDUP(def->name, ncf_if_name(iface)) < 0) + goto cleanup; + + if (VIR_STRDUP(def->mac, ncf_if_mac_string(iface)) < 0) + goto cleanup; + + return def; + +cleanup: + virInterfaceDefFree(def); + return NULL; +} + + static int netcf_to_vir_err(int netcf_errcode) { switch (netcf_errcode) @@ -182,6 +214,9 @@ static int netcfConnectNumOfInterfaces(virConnectPtr conn) int count; struct interface_driver *driver = conn->interfacePrivateData; + if (virConnectNumOfInterfacesEnsureACL(conn) < 0) + return -1; + interfaceDriverLock(driver); count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_ACTIVE); if (count < 0) { @@ -201,6 +236,9 @@ static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, in struct interface_driver *driver = conn->interfacePrivateData; int count; + if (virConnectListInterfacesEnsureACL(conn) < 0) + return -1; + interfaceDriverLock(driver); count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_IFACE_ACTIVE); @@ -223,6 +261,9 @@ static int netcfConnectNumOfDefinedInterfaces(virConnectPtr conn) int count; struct interface_driver *driver = conn->interfacePrivateData; + if (virConnectNumOfDefinedInterfacesEnsureACL(conn) < 0) + return -1; + interfaceDriverLock(driver); count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_INACTIVE); if (count < 0) { @@ -243,6 +284,9 @@ static int netcfConnectListDefinedInterfaces(virConnectPtr conn, char **const na struct interface_driver *driver = conn->interfacePrivateData; int count; + if (virConnectListDefinedInterfacesEnsureACL(conn) < 0) + return -1; + interfaceDriverLock(driver); count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_IFACE_INACTIVE); @@ -279,6 +323,9 @@ netcfConnectListAllInterfaces(virConnectPtr conn, virCheckFlags(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE, -1); + if (virConnectListAllInterfacesEnsureACL(conn) < 0) + return -1; + interfaceDriverLock(driver); /* List all interfaces, in case of we might support new filter flags @@ -414,6 +461,7 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn, struct interface_driver *driver = conn->interfacePrivateData; struct netcf_if *iface; virInterfacePtr ret = NULL; + virInterfaceDefPtr def = NULL; interfaceDriverLock(driver); iface = ncf_lookup_by_name(driver->netcf, name); @@ -432,10 +480,17 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn, goto cleanup; } + if (!(def = netcfGetMinimalDefForDevice(iface))) + goto cleanup; + + if (virInterfaceLookupByNameEnsureACL(conn, def) < 0) + goto cleanup; + ret = virGetInterface(conn, ncf_if_name(iface), ncf_if_mac_string(iface)); cleanup: ncf_if_free(iface); + virInterfaceDefFree(def); interfaceDriverUnlock(driver); return ret; } @@ -447,6 +502,7 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr conn, struct netcf_if *iface; int niface; virInterfacePtr ret = NULL; + virInterfaceDefPtr def = NULL; interfaceDriverLock(driver); niface = ncf_lookup_by_mac_string(driver->netcf, macstr, 1, &iface); @@ -472,10 +528,18 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr conn, goto cleanup; } + + if (!(def = netcfGetMinimalDefForDevice(iface))) + goto cleanup; + + if (virInterfaceLookupByMACStringEnsureACL(conn, def) < 0) + goto cleanup; + ret = virGetInterface(conn, ncf_if_name(iface), ncf_if_mac_string(iface)); cleanup: ncf_if_free(iface); + virInterfaceDefFree(def); interfaceDriverUnlock(driver); return ret; } @@ -520,6 +584,9 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo, goto cleanup; } + if (virInterfaceGetXMLDescEnsureACL(ifinfo->conn, ifacedef) < 0) + goto cleanup; + ret = virInterfaceDefFormat(ifacedef); if (!ret) { /* error was already reported */ @@ -554,6 +621,9 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn, goto cleanup; } + if (virInterfaceDefineXMLEnsureACL(conn, ifacedef) < 0) + goto cleanup; + xmlstr = virInterfaceDefFormat(ifacedef); if (!xmlstr) { /* error was already reported */ @@ -584,6 +654,7 @@ cleanup: static int netcfInterfaceUndefine(virInterfacePtr ifinfo) { struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; + virInterfaceDefPtr def = NULL; int ret = -1; interfaceDriverLock(driver); @@ -594,6 +665,13 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo) { goto cleanup; } + + if (!(def = netcfGetMinimalDefForDevice(iface))) + goto cleanup; + + if (virInterfaceUndefineEnsureACL(ifinfo->conn, def) < 0) + goto cleanup; + ret = ncf_if_undefine(iface); if (ret < 0) { const char *errmsg, *details; @@ -607,6 +685,7 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo) { cleanup: ncf_if_free(iface); + virInterfaceDefFree(def); interfaceDriverUnlock(driver); return ret; } @@ -616,6 +695,7 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo, { struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; + virInterfaceDefPtr def = NULL; int ret = -1; virCheckFlags(0, -1); @@ -628,6 +708,13 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo, goto cleanup; } + + if (!(def = netcfGetMinimalDefForDevice(iface))) + goto cleanup; + + if (virInterfaceCreateEnsureACL(ifinfo->conn, def) < 0) + goto cleanup; + ret = ncf_if_up(iface); if (ret < 0) { const char *errmsg, *details; @@ -641,6 +728,7 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo, cleanup: ncf_if_free(iface); + virInterfaceDefFree(def); interfaceDriverUnlock(driver); return ret; } @@ -650,6 +738,7 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo, { struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; + virInterfaceDefPtr def = NULL; int ret = -1; virCheckFlags(0, -1); @@ -662,6 +751,13 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo, goto cleanup; } + + if (!(def = netcfGetMinimalDefForDevice(iface))) + goto cleanup; + + if (virInterfaceDestroyEnsureACL(ifinfo->conn, def) < 0) + goto cleanup; + ret = ncf_if_down(iface); if (ret < 0) { const char *errmsg, *details; @@ -675,6 +771,7 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo, cleanup: ncf_if_free(iface); + virInterfaceDefFree(def); interfaceDriverUnlock(driver); return ret; } @@ -684,6 +781,7 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo) struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; unsigned int flags = 0; + virInterfaceDefPtr def = NULL; int ret = -1; interfaceDriverLock(driver); @@ -694,6 +792,13 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo) goto cleanup; } + + if (!(def = netcfGetMinimalDefForDevice(iface))) + goto cleanup; + + if (virInterfaceIsActiveEnsureACL(ifinfo->conn, def) < 0) + goto cleanup; + if (ncf_if_status(iface, &flags) < 0) { const char *errmsg, *details; int errcode = ncf_error(driver->netcf, &errmsg, &details); @@ -708,6 +813,7 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo) cleanup: ncf_if_free(iface); + virInterfaceDefFree(def); interfaceDriverUnlock(driver); return ret; } @@ -720,6 +826,9 @@ static int netcfInterfaceChangeBegin(virConnectPtr conn, unsigned int flags) virCheckFlags(0, -1); /* currently flags must be 0 */ + if (virInterfaceChangeBeginEnsureACL(conn) < 0) + return -1; + interfaceDriverLock(driver); ret = ncf_change_begin(driver->netcf, 0); @@ -743,6 +852,9 @@ static int netcfInterfaceChangeCommit(virConnectPtr conn, unsigned int flags) virCheckFlags(0, -1); /* currently flags must be 0 */ + if (virInterfaceChangeCommitEnsureACL(conn) < 0) + return -1; + interfaceDriverLock(driver); ret = ncf_change_commit(driver->netcf, 0); @@ -766,6 +878,9 @@ static int netcfInterfaceChangeRollback(virConnectPtr conn, unsigned int flags) virCheckFlags(0, -1); /* currently flags must be 0 */ + if (virInterfaceChangeRollbackEnsureACL(conn) < 0) + return -1; + interfaceDriverLock(driver); ret = ncf_change_rollback(driver->netcf, 0); diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index a6c7fde..68e1e2f 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -31,6 +31,7 @@ #include "interface_conf.h" #include "viralloc.h" #include "virstring.h" +#include "viraccessapicheck.h" #define VIR_FROM_THIS VIR_FROM_INTERFACE @@ -61,6 +62,36 @@ virUdevStatusString(virUdevStatus status) return ""; } +/* + * Get a minimal virInterfaceDef containing enough metadata + * for access control checks to be performed. Currently + * this implies existance of name and mac address attributes + */ +static virInterfaceDef * ATTRIBUTE_NONNULL(1) +udevGetMinimalDefForDevice(struct udev_device *dev) +{ + virInterfaceDef *def; + + /* Allocate our interface definition structure */ + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (VIR_STRDUP(def->name, udev_device_get_sysname(dev)) < 0) + goto cleanup; + + if (VIR_STRDUP(def->mac, udev_device_get_sysattr_value(dev, "address")) < 0) + goto cleanup; + + return def; + +cleanup: + virInterfaceDefFree(def); + return NULL; +} + + static struct udev_enumerate * ATTRIBUTE_NONNULL(1) udevGetDevices(struct udev *udev, virUdevStatus status) { @@ -255,6 +286,9 @@ error: static int udevConnectNumOfInterfaces(virConnectPtr conn) { + if (virConnectNumOfInterfacesEnsureACL(conn) < 0) + return -1; + return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_ACTIVE); } @@ -263,6 +297,9 @@ udevConnectListInterfaces(virConnectPtr conn, char **const names, int names_len) { + if (virConnectListInterfacesEnsureACL(conn) < 0) + return -1; + return udevListInterfacesByStatus(conn, names, names_len, VIR_UDEV_IFACE_ACTIVE); } @@ -270,6 +307,9 @@ udevConnectListInterfaces(virConnectPtr conn, static int udevConnectNumOfDefinedInterfaces(virConnectPtr conn) { + if (virConnectNumOfDefinedInterfacesEnsureACL(conn) < 0) + return -1; + return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_INACTIVE); } @@ -278,6 +318,9 @@ udevConnectListDefinedInterfaces(virConnectPtr conn, char **const names, int names_len) { + if (virConnectListDefinedInterfacesEnsureACL(conn) < 0) + return -1; + return udevListInterfacesByStatus(conn, names, names_len, VIR_UDEV_IFACE_INACTIVE); } @@ -302,6 +345,9 @@ udevConnectListAllInterfaces(virConnectPtr conn, virCheckFlags(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE, -1); + if (virConnectListAllInterfacesEnsureACL(conn) < 0) + return -1; + /* Grab a udev reference */ udev = udev_ref(driverState->udev); @@ -412,8 +458,8 @@ udevInterfaceLookupByName(virConnectPtr conn, const char *name) struct udev_iface_driver *driverState = conn->interfacePrivateData; struct udev *udev = udev_ref(driverState->udev); struct udev_device *dev; - const char *macaddr; virInterfacePtr ret = NULL; + virInterfaceDefPtr def = NULL; /* get a device reference based on the device name */ dev = udev_device_new_from_subsystem_sysname(udev, "net", name); @@ -424,12 +470,18 @@ udevInterfaceLookupByName(virConnectPtr conn, const char *name) goto cleanup; } - macaddr = udev_device_get_sysattr_value(dev, "address"); - ret = virGetInterface(conn, name, macaddr); + if (!(def = udevGetMinimalDefForDevice(dev))) + goto cleanup; + + if (virInterfaceLookupByNameEnsureACL(conn, def) < 0) + goto cleanup; + + ret = virGetInterface(conn, def->name, def->mac); udev_device_unref(dev); cleanup: udev_unref(udev); + virInterfaceDefFree(def); return ret; } @@ -442,7 +494,7 @@ udevInterfaceLookupByMACString(virConnectPtr conn, const char *macstr) struct udev_enumerate *enumerate = NULL; struct udev_list_entry *dev_entry; struct udev_device *dev; - const char *name; + virInterfaceDefPtr def = NULL; virInterfacePtr ret = NULL; enumerate = udevGetDevices(udev, VIR_UDEV_IFACE_ALL); @@ -480,14 +532,21 @@ udevInterfaceLookupByMACString(virConnectPtr conn, const char *macstr) } dev = udev_device_new_from_syspath(udev, udev_list_entry_get_name(dev_entry)); - name = udev_device_get_sysname(dev); - ret = virGetInterface(conn, name, macstr); + + if (!(def = udevGetMinimalDefForDevice(dev))) + goto cleanup; + + if (virInterfaceLookupByMACStringEnsureACL(conn, def) < 0) + goto cleanup; + + ret = virGetInterface(conn, def->name, def->mac); udev_device_unref(dev); cleanup: if (enumerate) udev_enumerate_unref(enumerate); udev_unref(udev); + virInterfaceDefFree(def); return ret; } @@ -1044,6 +1103,9 @@ udevInterfaceGetXMLDesc(virInterfacePtr ifinfo, if (!ifacedef) goto cleanup; + if (virInterfaceGetXMLDescEnsureACL(ifinfo->conn, ifacedef) < 0) + goto cleanup; + xmlstr = virInterfaceDefFormat(ifacedef); virInterfaceDefFree(ifacedef); @@ -1061,7 +1123,8 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) struct udev_iface_driver *driverState = ifinfo->conn->interfacePrivateData; struct udev *udev = udev_ref(driverState->udev); struct udev_device *dev; - int status; + virInterfaceDefPtr def = NULL; + int status = -1; dev = udev_device_new_from_subsystem_sysname(udev, "net", ifinfo->name); @@ -1069,10 +1132,15 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) virReportError(VIR_ERR_NO_INTERFACE, _("couldn't find interface named '%s'"), ifinfo->name); - status = -1; goto cleanup; } + if (!(def = udevGetMinimalDefForDevice(dev))) + goto cleanup; + + if (virInterfaceIsActiveEnsureACL(ifinfo->conn, def) < 0) + goto cleanup; + /* Check if it's active or not */ status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); @@ -1080,6 +1148,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) cleanup: udev_unref(udev); + virInterfaceDefFree(def); return status; } -- 1.8.1.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list