5:02 p.m.
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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.
Such is life with security filtering.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
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(-)
+++ 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.
s/case of/case/
s/except/beyond/
+
+ if (VIR_ALLOC_N(names, count) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
[Uggh - we still need to revisit the goal to hoist OOM reporting into
VIR_ALLOC and friends - nothing magic happened during my 3-week vacation
on that front. But that's a completely separate project.]
+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);
Can any of this be shared? You have:
long setup
foreach element
if not filtered
count++
and
long setup
foreach element
if not filtered
append object
where the long setup is the same. I think some of the other filtering
code has used a single implementation, where passing in NULL for the
**names parameter implies just counting, and passing in non-NULL implies
object appending. Then you wouldn't be duplicating as much code, making
it easier to add new filters in one place in the future.
+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);
Indentation is off.
@@ -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.
Pre-existing, but as long as you are here, s/splitted/split/. Or maybe
even nuke the comment, now that you have set up the framework for
additional filtering so it no longer applies.
@@ -412,6 +595,7 @@ netcfConnectListAllInterfaces(virConnectPtr
conn,
(MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) &&
(status & NETCF_IFACE_INACTIVE)))) {
ncf_if_free(iface);
+ iface = NULL;
continue;
Oooh tricky - you posted the CVE-2013-2218 patch embedded inside this
patch, prior to the embargo date, but no one picked up on it until now :)
At any rate, a consolidation patch between the count vs. list function
is probably worth a separate patch, at which point your code as-is looks
mostly okay. ACK with grammar/spacing nits fixed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org