[libvirt] [PATCH 7/8] Add access control filtering of interface objects

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Ensure that all APIs which list interface objects filter them against the access control system. This makes the APIs for listing names and counting devices slightly less efficient, since we can't use the direct netcf APIs for these tasks. Instead we have to ask netcf for the full list of objects & iterate over the list filtering them out. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/conf/interface_conf.h | 3 + src/interface/interface_backend_netcf.c | 262 +++++++++++++++++++++++++++----- src/interface/interface_backend_udev.c | 56 +++++-- 3 files changed, 270 insertions(+), 51 deletions(-) diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index ed6986c..d60867b 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -211,6 +211,9 @@ char *virInterfaceDefFormat(const virInterfaceDefPtr def); void virInterfaceObjLock(virInterfaceObjPtr obj); void virInterfaceObjUnlock(virInterfaceObjPtr obj); +typedef bool (*virInterfaceObjListFilter)(virConnectPtr conn, + virInterfaceDefPtr def); + # define VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE \ (VIR_CONNECT_LIST_INTERFACES_ACTIVE | \ VIR_CONNECT_LIST_INTERFACES_INACTIVE) diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index a995816..abd11e2 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -209,48 +209,233 @@ static int netcfInterfaceClose(virConnectPtr conn) return 0; } -static int netcfConnectNumOfInterfaces(virConnectPtr conn) +static int netcfConnectNumOfInterfacesImpl(virConnectPtr conn, + int status, + virInterfaceObjListFilter filter) { - int count; struct interface_driver *driver = conn->interfacePrivateData; + int count; + int want = 0; + int ret = -1; + int i; + char **names = NULL; - if (virConnectNumOfInterfacesEnsureACL(conn) < 0) - return -1; - - interfaceDriverLock(driver); - count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_ACTIVE); + /* List all interfaces, in case of we might support new filter flags + * except active|inactive in future. + */ + count = ncf_num_of_interfaces(driver->netcf, status); if (count < 0) { const char *errmsg, *details; int errcode = ncf_error(driver->netcf, &errmsg, &details); virReportError(netcf_to_vir_err(errcode), - _("failed to get number of interfaces on host: %s%s%s"), - errmsg, details ? " - " : "", details ? details : ""); + _("failed to get number of host interfaces: %s%s%s"), + errmsg, details ? " - " : "", + details ? details : ""); + goto cleanup; } - interfaceDriverUnlock(driver); - return count; + if (count == 0) { + ret = 0; + goto cleanup; + } + + if (VIR_ALLOC_N(names, count) < 0) { + virReportOOMError(); + goto cleanup; + } + + if ((count = ncf_list_interfaces(driver->netcf, count, names, status)) < 0) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + virReportError(netcf_to_vir_err(errcode), + _("failed to list host interfaces: %s%s%s"), + errmsg, details ? " - " : "", + details ? details : ""); + goto cleanup; + } + + for (i = 0; i < count; i++) { + virInterfaceDefPtr def; + struct netcf_if *iface; + + iface = ncf_lookup_by_name(driver->netcf, names[i]); + if (!iface) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + if (errcode != NETCF_NOERROR) { + virReportError(netcf_to_vir_err(errcode), + _("couldn't find interface named '%s': %s%s%s"), + names[i], errmsg, + details ? " - " : "", details ? details : ""); + goto cleanup; + } else { + /* Ignore the NETCF_NOERROR, as the interface is very likely + * deleted by other management apps (e.g. virt-manager). + */ + VIR_WARN("couldn't find interface named '%s', might be " + "deleted by other process", names[i]); + continue; + } + } + + if (!(def = netcfGetMinimalDefForDevice(iface))) { + ncf_if_free(iface); + goto cleanup; + } + ncf_if_free(iface); + + if (!filter(conn, def)) { + virInterfaceDefFree(def); + continue; + } + virInterfaceDefFree(def); + + want++; + } + + ret = want; + +cleanup: + if (names) + for (i = 0; i < count; i++) + VIR_FREE(names[i]); + VIR_FREE(names); + return ret; } -static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, int nnames) + +static int netcfConnectListInterfacesImpl(virConnectPtr conn, + int status, + char **const names, int nnames, + virInterfaceObjListFilter filter) { struct interface_driver *driver = conn->interfacePrivateData; - int count; + int count = 0; + int want = 0; + int ret = -1; + int i; + char **allnames = NULL; - if (virConnectListInterfacesEnsureACL(conn) < 0) - return -1; + count = ncf_num_of_interfaces(driver->netcf, status); + if (count < 0) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + virReportError(netcf_to_vir_err(errcode), + _("failed to get number of host interfaces: %s%s%s"), + errmsg, details ? " - " : "", + details ? details : ""); + goto cleanup; + } - interfaceDriverLock(driver); + if (count == 0) { + ret = 0; + goto cleanup; + } - count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_IFACE_ACTIVE); - if (count < 0) { + if (VIR_ALLOC_N(allnames, count) < 0) { + virReportOOMError(); + goto cleanup; + } + + if ((count = ncf_list_interfaces(driver->netcf, count, allnames, status)) < 0) { const char *errmsg, *details; int errcode = ncf_error(driver->netcf, &errmsg, &details); virReportError(netcf_to_vir_err(errcode), _("failed to list host interfaces: %s%s%s"), errmsg, details ? " - " : "", details ? details : ""); + goto cleanup; + } + + if (count == 0) { + ret = 0; + goto cleanup; + } + + for (i = 0; i < count && want < nnames; i++) { + virInterfaceDefPtr def; + struct netcf_if *iface; + + iface = ncf_lookup_by_name(driver->netcf, allnames[i]); + if (!iface) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + if (errcode != NETCF_NOERROR) { + virReportError(netcf_to_vir_err(errcode), + _("couldn't find interface named '%s': %s%s%s"), + allnames[i], errmsg, + details ? " - " : "", details ? details : ""); + goto cleanup; + } else { + /* Ignore the NETCF_NOERROR, as the interface is very likely + * deleted by other management apps (e.g. virt-manager). + */ + VIR_WARN("couldn't find interface named '%s', might be " + "deleted by other process", allnames[i]); + continue; + } + } + + if (!(def = netcfGetMinimalDefForDevice(iface))) { + ncf_if_free(iface); + goto cleanup; + } + ncf_if_free(iface); + + if (!filter(conn, def)) { + virInterfaceDefFree(def); + continue; + } + virInterfaceDefFree(def); + + names[want++] = allnames[i]; + allnames[i] = NULL; + } + + ret = want; + +cleanup: + if (allnames) + for (i = 0; i < count; i++) + VIR_FREE(allnames[i]); + VIR_FREE(allnames); + if (ret < 0) { + for (i = 0; i < nnames; i++) + VIR_FREE(names[i]); } + return ret; +} + + +static int netcfConnectNumOfInterfaces(virConnectPtr conn) +{ + int count; + struct interface_driver *driver = conn->interfacePrivateData; + + if (virConnectNumOfInterfacesEnsureACL(conn) < 0) + return -1; + + interfaceDriverLock(driver); + count = netcfConnectNumOfInterfacesImpl(conn, + NETCF_IFACE_ACTIVE, + virConnectNumOfInterfacesCheckACL); + interfaceDriverUnlock(driver); + return count; +} + +static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, int nnames) +{ + struct interface_driver *driver = conn->interfacePrivateData; + int count; + + if (virConnectListInterfacesEnsureACL(conn) < 0) + return -1; + interfaceDriverLock(driver); + count = netcfConnectListInterfacesImpl(conn, + NETCF_IFACE_ACTIVE, + names, nnames, + virConnectListInterfacesCheckACL); interfaceDriverUnlock(driver); return count; @@ -265,16 +450,9 @@ static int netcfConnectNumOfDefinedInterfaces(virConnectPtr conn) return -1; interfaceDriverLock(driver); - count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_INACTIVE); - if (count < 0) { - const char *errmsg, *details; - int errcode = ncf_error(driver->netcf, &errmsg, &details); - virReportError(netcf_to_vir_err(errcode), - _("failed to get number of defined interfaces on host: %s%s%s"), - errmsg, details ? " - " : "", - details ? details : ""); - } - + count = netcfConnectNumOfInterfacesImpl(conn, + NETCF_IFACE_INACTIVE, + virConnectNumOfDefinedInterfacesCheckACL); interfaceDriverUnlock(driver); return count; } @@ -288,17 +466,10 @@ static int netcfConnectListDefinedInterfaces(virConnectPtr conn, char **const na return -1; interfaceDriverLock(driver); - - count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_IFACE_INACTIVE); - if (count < 0) { - const char *errmsg, *details; - int errcode = ncf_error(driver->netcf, &errmsg, &details); - virReportError(netcf_to_vir_err(errcode), - _("failed to list host defined interfaces: %s%s%s"), - errmsg, details ? " - " : "", - details ? details : ""); - } - + count = netcfConnectListInterfacesImpl(conn, + NETCF_IFACE_INACTIVE, + names, nnames, + virConnectListDefinedInterfacesCheckACL); interfaceDriverUnlock(driver); return count; @@ -373,6 +544,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn, } for (i = 0; i < count; i++) { + virInterfaceDefPtr def; iface = ncf_lookup_by_name(driver->netcf, names[i]); if (!iface) { const char *errmsg, *details; @@ -403,6 +575,17 @@ netcfConnectListAllInterfaces(virConnectPtr conn, goto cleanup; } + if (!(def = netcfGetMinimalDefForDevice(iface))) + goto cleanup; + + if (!virConnectListAllInterfacesCheckACL(conn, def)) { + ncf_if_free(iface); + iface = NULL; + virInterfaceDefFree(def); + continue; + } + virInterfaceDefFree(def); + /* XXX: Filter the result, need to be splitted once new filter flags * except active|inactive are supported. */ @@ -412,6 +595,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn, (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) && (status & NETCF_IFACE_INACTIVE)))) { ncf_if_free(iface); + iface = NULL; continue; } diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index 68e1e2f..e063702 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -181,7 +181,8 @@ udevInterfaceClose(virConnectPtr conn) } static int -udevNumOfInterfacesByStatus(virConnectPtr conn, virUdevStatus status) +udevNumOfInterfacesByStatus(virConnectPtr conn, virUdevStatus status, + virInterfaceObjListFilter filter) { struct udev_iface_driver *driverState = conn->interfacePrivateData; struct udev *udev = udev_ref(driverState->udev); @@ -208,7 +209,18 @@ udevNumOfInterfacesByStatus(virConnectPtr conn, virUdevStatus status) /* For each item so we can count */ udev_list_entry_foreach(dev_entry, devices) { - count++; + struct udev_device *dev; + const char *path; + virInterfaceDefPtr def; + + path = udev_list_entry_get_name(dev_entry); + dev = udev_device_new_from_syspath(udev, path); + + def = udevGetMinimalDefForDevice(dev); + if (filter(conn, def)) + count++; + udev_device_unref(dev); + virInterfaceDefFree(def); } cleanup: @@ -223,7 +235,8 @@ static int udevListInterfacesByStatus(virConnectPtr conn, char **const names, int names_len, - virUdevStatus status) + virUdevStatus status, + virInterfaceObjListFilter filter) { struct udev_iface_driver *driverState = conn->interfacePrivateData; struct udev *udev = udev_ref(driverState->udev); @@ -251,6 +264,7 @@ udevListInterfacesByStatus(virConnectPtr conn, udev_list_entry_foreach(dev_entry, devices) { struct udev_device *dev; const char *path; + virInterfaceDefPtr def; /* Ensure we won't exceed the size of our array */ if (count > names_len) @@ -258,13 +272,18 @@ udevListInterfacesByStatus(virConnectPtr conn, path = udev_list_entry_get_name(dev_entry); dev = udev_device_new_from_syspath(udev, path); - if (VIR_STRDUP(names[count], udev_device_get_sysname(dev)) < 0) { - udev_device_unref(dev); - goto error; + + def = udevGetMinimalDefForDevice(dev); + if (filter(conn, def)) { + if (VIR_STRDUP(names[count], udev_device_get_sysname(dev)) < 0) { + udev_device_unref(dev); + virInterfaceDefFree(def); + goto error; + } + count++; } udev_device_unref(dev); - - count++; + virInterfaceDefFree(def); } udev_enumerate_unref(enumerate); @@ -289,7 +308,8 @@ udevConnectNumOfInterfaces(virConnectPtr conn) if (virConnectNumOfInterfacesEnsureACL(conn) < 0) return -1; - return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_ACTIVE); + return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_ACTIVE, + virConnectNumOfInterfacesCheckACL); } static int @@ -301,7 +321,8 @@ udevConnectListInterfaces(virConnectPtr conn, return -1; return udevListInterfacesByStatus(conn, names, names_len, - VIR_UDEV_IFACE_ACTIVE); + VIR_UDEV_IFACE_ACTIVE, + virConnectListInterfacesCheckACL); } static int @@ -310,7 +331,8 @@ udevConnectNumOfDefinedInterfaces(virConnectPtr conn) if (virConnectNumOfDefinedInterfacesEnsureACL(conn) < 0) return -1; - return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_INACTIVE); + return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_INACTIVE, + virConnectNumOfDefinedInterfacesCheckACL); } static int @@ -322,7 +344,8 @@ udevConnectListDefinedInterfaces(virConnectPtr conn, return -1; return udevListInterfacesByStatus(conn, names, names_len, - VIR_UDEV_IFACE_INACTIVE); + VIR_UDEV_IFACE_INACTIVE, + virConnectListDefinedInterfacesCheckACL); } #define MATCH(FLAG) (flags & (FLAG)) @@ -400,6 +423,7 @@ udevConnectListAllInterfaces(virConnectPtr conn, const char *path; const char *name; const char *macaddr; + virInterfaceDefPtr def; path = udev_list_entry_get_name(dev_entry); dev = udev_device_new_from_syspath(udev, path); @@ -407,6 +431,14 @@ udevConnectListAllInterfaces(virConnectPtr conn, macaddr = udev_device_get_sysattr_value(dev, "address"); status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); + def = udevGetMinimalDefForDevice(dev); + if (!virConnectListAllInterfacesCheckACL(conn, def)) { + udev_device_unref(dev); + virInterfaceDefFree(def); + continue; + } + virInterfaceDefFree(def); + /* Filter the results */ if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) && !((MATCH(VIR_CONNECT_LIST_INTERFACES_ACTIVE) && status) || -- 1.8.1.4