From 71f2434fe79c88d75f6236e82998a04f3dee26ff Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Thu, 27 Jun 2013 17:07:22 +0100 Subject: [PATCH] Add access control filtering of interface objects 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 --- src/conf/interface_conf.h | 3 + src/interface/interface_backend_netcf.c | 267 ++++++++++++++++++++---- src/interface/interface_backend_udev.c | 56 +++-- 3 files changed, 272 insertions(+), 54 deletions(-) diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index ed6986c5d8..d60867bd1b 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 9aa673d20a..760540ab63 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -209,6 +209,204 @@ static int netcfInterfaceClose(virConnectPtr conn) return 0; } +static int netcfConnectNumOfInterfacesImpl(virConnectPtr conn, + int status, + virInterfaceObjListFilter filter) +{ + struct interface_driver *driver = conn->interfacePrivateData; + int count; + int want = 0; + int ret = -1; + int i; + char **names = NULL; + + /* List all interfaces, in case we might support new filter flags + * beyond 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 host interfaces: %s%s%s"), + errmsg, details ? " - " : "", + details ? details : ""); + goto cleanup; + } + + 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 netcfConnectListInterfacesImpl(virConnectPtr conn, + int status, + char **const names, int nnames, + virInterfaceObjListFilter filter) +{ + struct interface_driver *driver = conn->interfacePrivateData; + int count = 0; + int want = 0; + int ret = -1; + int i; + char **allnames = NULL; + + 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; + } + + if (count == 0) { + ret = 0; + goto cleanup; + } + + 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; @@ -218,15 +416,9 @@ static int netcfConnectNumOfInterfaces(virConnectPtr conn) return -1; interfaceDriverLock(driver); - count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_ACTIVE); - 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 : ""); - } - + count = netcfConnectNumOfInterfacesImpl(conn, + NETCF_IFACE_ACTIVE, + virConnectNumOfInterfacesCheckACL); interfaceDriverUnlock(driver); return count; } @@ -240,17 +432,10 @@ static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, in return -1; interfaceDriverLock(driver); - - count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_IFACE_ACTIVE); - 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 interfaces: %s%s%s"), - errmsg, details ? " - " : "", - details ? details : ""); - } - + 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,7 +575,18 @@ netcfConnectListAllInterfaces(virConnectPtr conn, goto cleanup; } - /* XXX: Filter the result, need to be splitted once new filter flags + 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 split once new filter flags * except active|inactive are supported. */ if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) && diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index 95034bfd51..6270c9dc17 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) ||