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: NDaniel P. Berrange <berrange@redhat.com>

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)

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 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 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,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) &&

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) ||